[PATCH v6] net: atlantic: always deep reset on pm op, fixing up my null deref regression

[PATCH v6] net: atlantic: always deep reset on pm op, fixing up my null deref regression

[Manuel Ullmann]

From 18dc080d8d4a30d0fcb45f24fd15279cc87c47d5 Mon Sep 17 00:00:00 2001
Date: Wed, 4 May 2022 21:30:44 +0200

The impact of this regression is the same for resume that I saw on
thaw: the kernel hangs and nothing except SysRq rebooting can be done.

Fixes regression in commit cbe6c3a8f8f4 ("net: atlantic: invert deep
par in pm functions, preventing null derefs"), where I disabled deep
pm resets in suspend and resume, trying to make sense of the
atl_resume_common() deep parameter in the first place.

It turns out, that atlantic always has to deep reset on pm
operations. Even though I expected that and tested resume, I screwed
up by kexec-rebooting into an unpatched kernel, thus missing the
breakage.

This fixup obsoletes the deep parameter of atl_resume_common, but I
leave the cleanup for the maintainers to post to mainline.

Suspend and hibernation were successfully tested by the reporters.

Fixes: cbe6c3a8f8f4 ("net: atlantic: invert deep par in pm functions, preventing null derefs")
Link: https://lore.kernel.org/regressions/9-Ehc_xXSwdXcvZqKD5aSqsqeNj5Izco4MYEwnx5cySXVEc9-x_WC4C3kAoCqNTi-H38frroUK17iobNVnkLtW36V6VWGSQEOHXhmVMm5iQ=@protonmail.com/
Reported-by: Jordan Leppert <jordanleppert@protonmail.com>
Reported-by: Holger Hoffstaette <holger@applied-asynchrony.com>
Tested-by: Jordan Leppert <jordanleppert@protonmail.com>
Tested-by: Holger Hoffstaette <holger@applied-asynchrony.com>
CC: <stable@vger.kernel.org> # 5.10+
Signed-off-by: Manuel Ullmann <labre@posteo.de>
---
I'm very sorry for this regression. It would be nice, if this could
reach mainline before 5.18 release, if applicable. This restores the
original suspend behaviour, while keeping the fix for hibernation. The
fix for hibernation might not be the root cause, but still is the most
simple fix for backporting to stable while the root cause is unknown
to the maintainers.

Changes in v2:
Patch formatting fixes
~ Fix Fixes tag
~ Simplify stable Cc tag
~ Fix Signed-off-by tag

Changes in v3:
~ Prefixed commit reference with "commit" aka I managed to use
  checkpatch.pl.
~ Added Tested-by tags for the testing reporters.
~ People start to get annoyed by my patch revision spamming. Should be
  the last one.

Changes in v4:
~ Moved patch changelog to comment section
~ Use unicode ndash for patch changelog list to avoid confusion with
  diff in editors
~ Expanded comment
~ Targeting net-next by subject

Changes in v5:
~ Changed my MTA transfer encoding to 8 bit instead of
  quoted-printable. Git should like this a bit more.

Changes in v6:
~ Reducing content to 7 bit chars, because nipa did not apply v4 and v5, while
  git does against a fresh net-next HEAD. Maybe it chokes on the
  additional bit.
~ Omitting target tree to resemble the last passing patch version the most.

 drivers/net/ethernet/aquantia/atlantic/aq_pci_func.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)


base-commit: 672c0c5173427e6b3e2a9bbb7be51ceeec78093a

diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_pci_func.c b/drivers/net/ethernet/aquantia/atlantic/aq_pci_func.c
index 3a529ee8c834..831833911a52 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_pci_func.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_pci_func.c
@@ -449,7 +449,7 @@ static int aq_pm_freeze(struct device *dev)
 
 static int aq_pm_suspend_poweroff(struct device *dev)
 {
-	return aq_suspend_common(dev, false);
+	return aq_suspend_common(dev, true);
 }
 
 static int aq_pm_thaw(struct device *dev)
@@ -459,7 +459,7 @@ static int aq_pm_thaw(struct device *dev)
 
 static int aq_pm_resume_restore(struct device *dev)
 {
-	return atl_resume_common(dev, false);
+	return atl_resume_common(dev, true);
 }
 
 static const struct dev_pm_ops aq_pm_ops = {

base-commit: 672c0c5173427e6b3e2a9bbb7be51ceeec78093a
-- 
2.35.1

Re: [PATCH v6] net: atlantic: always deep reset on pm op, fixing up my null deref regression

[Paolo Abeni]

On Sun, 2022-05-08 at 00:36 +0000, Manuel Ullmann wrote:
> From 18dc080d8d4a30d0fcb45f24fd15279cc87c47d5 Mon Sep 17 00:00:00 2001
> Date: Wed, 4 May 2022 21:30:44 +0200
> 
> The impact of this regression is the same for resume that I saw on
> thaw: the kernel hangs and nothing except SysRq rebooting can be done.
> 
> Fixes regression in commit cbe6c3a8f8f4 ("net: atlantic: invert deep
> par in pm functions, preventing null derefs"), where I disabled deep
> pm resets in suspend and resume, trying to make sense of the
> atl_resume_common() deep parameter in the first place.
> 
> It turns out, that atlantic always has to deep reset on pm
> operations. Even though I expected that and tested resume, I screwed
> up by kexec-rebooting into an unpatched kernel, thus missing the
> breakage.
> 
> This fixup obsoletes the deep parameter of atl_resume_common, but I
> leave the cleanup for the maintainers to post to mainline.
> 
> Suspend and hibernation were successfully tested by the reporters.
> 
> Fixes: cbe6c3a8f8f4 ("net: atlantic: invert deep par in pm functions, preventing null derefs")
> Link: https://lore.kernel.org/regressions/9-Ehc_xXSwdXcvZqKD5aSqsqeNj5Izco4MYEwnx5cySXVEc9-x_WC4C3kAoCqNTi-H38frroUK17iobNVnkLtW36V6VWGSQEOHXhmVMm5iQ=@protonmail.com/
> Reported-by: Jordan Leppert <jordanleppert@protonmail.com>
> Reported-by: Holger Hoffstaette <holger@applied-asynchrony.com>
> Tested-by: Jordan Leppert <jordanleppert@protonmail.com>
> Tested-by: Holger Hoffstaette <holger@applied-asynchrony.com>
> CC: <stable@vger.kernel.org> # 5.10+
> Signed-off-by: Manuel Ullmann <labre@posteo.de>
> ---
> I'm very sorry for this regression. It would be nice, if this could
> reach mainline before 5.18 release, if applicable. This restores the
> original suspend behaviour, while keeping the fix for hibernation. The
> fix for hibernation might not be the root cause, but still is the most
> simple fix for backporting to stable while the root cause is unknown
> to the maintainers.
> 
> Changes in v2:
> Patch formatting fixes
> ~ Fix Fixes tag
> ~ Simplify stable Cc tag
> ~ Fix Signed-off-by tag
> 
> Changes in v3:
> ~ Prefixed commit reference with "commit" aka I managed to use
>   checkpatch.pl.
> ~ Added Tested-by tags for the testing reporters.
> ~ People start to get annoyed by my patch revision spamming. Should be
>   the last one.
> 
> Changes in v4:
> ~ Moved patch changelog to comment section
> ~ Use unicode ndash for patch changelog list to avoid confusion with
>   diff in editors
> ~ Expanded comment
> ~ Targeting net-next by subject
> 
> Changes in v5:
> ~ Changed my MTA transfer encoding to 8 bit instead of
>   quoted-printable. Git should like this a bit more.
> 
> Changes in v6:
> ~ Reducing content to 7 bit chars, because nipa did not apply v4 and v5, while
>   git does against a fresh net-next HEAD. Maybe it chokes on the
>   additional bit.
> ~ Omitting target tree to resemble the last passing patch version the most.

For future submission, please always specify the target tree (which is
-net in this case). No need to resubmit: I'm applying it.

Thanks,

Paolo

Re: [PATCH v6] net: atlantic: always deep reset on pm op, fixing up my null deref regression

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (master)
by Paolo Abeni <pabeni@redhat.com>:

On Sun, 08 May 2022 00:36:46 +0000 you wrote:
> >From 18dc080d8d4a30d0fcb45f24fd15279cc87c47d5 Mon Sep 17 00:00:00 2001
> Date: Wed, 4 May 2022 21:30:44 +0200
> 
> The impact of this regression is the same for resume that I saw on
> thaw: the kernel hangs and nothing except SysRq rebooting can be done.
> 
> Fixes regression in commit cbe6c3a8f8f4 ("net: atlantic: invert deep
> par in pm functions, preventing null derefs"), where I disabled deep
> pm resets in suspend and resume, trying to make sense of the
> atl_resume_common() deep parameter in the first place.
> 
> [...]

Here is the summary with links:
  - [v6] net: atlantic: always deep reset on pm op, fixing up my null deref regression
    https://git.kernel.org/netdev/net/c/1809c30b6e5a

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH v6] net: atlantic: always deep reset on pm op, fixing up my null deref regression

[Manuel Ullmann]

Paolo Abeni <pabeni@redhat.com> writes:

> On Sun, 2022-05-08 at 00:36 +0000, Manuel Ullmann wrote:
>> [...]
>
> For future submission, please always specify the target tree (which is
> -net in this case). No need to resubmit: I'm applying it.
>
> Thanks,
>
> Paolo

Ah, okay. So next branches are reserved for new changes, while non-next
may be targeted by fixups or at least regression fixes. I can't recall a
definition for this in the documentation, but that might be me. I'll
keep this in mind for the future. Thanks for applying it and bearing
with me.

Regards,
Manuel