mbox series

[0/2] ASoC: SOF: remove unregister calls from shutdown

Message ID 20221209114529.3909192-1-kai.vehmanen@linux.intel.com (mailing list archive)
Headers show
Series ASoC: SOF: remove unregister calls from shutdown | expand

Message

Kai Vehmanen Dec. 9, 2022, 11:45 a.m. UTC
This patchset is an alternative solution to problems
reported by Ricardo Ribalda <ribalda@chromium.org> and
Zhen Ni <nizhen@uniontech.com>, as discussed in

- "[PATCH] ALSA: core: Fix deadlock when shutdown a frozen userspace"
 https://mailman.alsa-project.org/pipermail/alsa-devel/2022-November/209248.html

- "[PATCH] ASoc: SOF: Fix sof-audio-pci-intel-tgl shutdown timeout during hibernation"
  https://mailman.alsa-project.org/pipermail/alsa-devel/2022-December/209685.html

It was raised by Oliver Neukum <oneukum@suse.com> that kernel should
not let user-space stall the shutdown process in any scenario and the
unregister call in current SOF shutdown code is not right.

This series reverts the ASoC SOF patch to unregister clients and
the machine drivers at shutdown. To avoid bringing back old bugs,
the series includes a patch to fix S5 entry on certain Intel
platforms.

Kai Vehmanen (2):
  ASoC: SOF: Intel: pci-tgl: unblock S5 entry if DMA stop has failed"
  ASoC: SOF: Revert: "core: unregister clients and machine drivers in
    .shutdown"

 sound/soc/sof/core.c          |  9 -----
 sound/soc/sof/intel/hda-dsp.c | 72 +++++++++++++++++++++++++++++++++++
 sound/soc/sof/intel/hda.h     |  1 +
 sound/soc/sof/intel/tgl.c     |  2 +-
 4 files changed, 74 insertions(+), 10 deletions(-)


base-commit: e85b1f5a9769ac30f4d2f6fb1cdcd9570c38e0c1

Comments

Mark Brown Dec. 14, 2022, 1:32 p.m. UTC | #1
On Fri, 09 Dec 2022 13:45:27 +0200, Kai Vehmanen wrote:
> This patchset is an alternative solution to problems
> reported by Ricardo Ribalda <ribalda@chromium.org> and
> Zhen Ni <nizhen@uniontech.com>, as discussed in
> 
> - "[PATCH] ALSA: core: Fix deadlock when shutdown a frozen userspace"
>  https://mailman.alsa-project.org/pipermail/alsa-devel/2022-November/209248.html
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/2] ASoC: SOF: Intel: pci-tgl: unblock S5 entry if DMA stop has failed"
      commit: 2aa2a5ead0ee0a358bf80a2984a641d1bf2adc2a
[2/2] ASoC: SOF: Revert: "core: unregister clients and machine drivers in .shutdown"
      commit: 44fda61d2bcfb74a942df93959e083a4e8eff75f

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark
Thorsten Leemhuis Dec. 19, 2022, 9:41 a.m. UTC | #2
Hi, this is your Linux kernel regression tracker.

On 14.12.22 14:32, Mark Brown wrote:
> On Fri, 09 Dec 2022 13:45:27 +0200, Kai Vehmanen wrote:
>> This patchset is an alternative solution to problems
>> reported by Ricardo Ribalda <ribalda@chromium.org> and
>> Zhen Ni <nizhen@uniontech.com>, as discussed in
>>
>> - "[PATCH] ALSA: core: Fix deadlock when shutdown a frozen userspace"
>>  https://mailman.alsa-project.org/pipermail/alsa-devel/2022-November/209248.html
>>
>> [...]
> 
> Applied to
> 
>    https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
> 
> Thanks!
> 
> [1/2] ASoC: SOF: Intel: pci-tgl: unblock S5 entry if DMA stop has failed"
>       commit: 2aa2a5ead0ee0a358bf80a2984a641d1bf2adc2a
> [2/2] ASoC: SOF: Revert: "core: unregister clients and machine drivers in .shutdown"
>       commit: 44fda61d2bcfb74a942df93959e083a4e8eff75f
> 
> All being well this means that it will be integrated into the linux-next
> tree (usually sometime in the next 24 hours) and sent to Linus during
> the next merge window (or sooner if it is a bug fix), however if
> problems are discovered then the patch may be dropped or reverted.
> [...]

I noticed a regression report in bugzilla:
https://bugzilla.kernel.org/show_bug.cgi?id=216820

```
> My laptop started to hang on hibernation (sleep and shutdown are
> fine). I bisected it to commit 83bfc7e793b555291785136c3ae86abcdc046887,
> which appears to be related to ALSA.
```

That's a commit the second patch from this series reverts. To my
untrained eyes it thus looks a lot like these change will resolve the
reported issue, which made me wonder:

* these patches afaics are not yet in mainline, is the plan to still
send it this cycle?

* there are no "CC: <stable..." tags in these patches. Is the plan to
manually ask for a backport? Or how can we get the regression fixed in
older releases?

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)

P.S.: As the Linux kernel's regression tracker I deal with a lot of
reports and sometimes miss something important when writing mails like
this. If that's the case here, don't hesitate to tell me in a public
reply, it's in everyone's interest to set the public record straight.
Mark Brown Dec. 19, 2022, 1:04 p.m. UTC | #3
On Mon, Dec 19, 2022 at 10:41:41AM +0100, Thorsten Leemhuis wrote:

> * there are no "CC: <stable..." tags in these patches. Is the plan to
> manually ask for a backport? Or how can we get the regression fixed in
> older releases?

Speak to the stable maintainers I guess, or hope their bot picks the
commits up.
Kai Vehmanen Dec. 20, 2022, 11:41 a.m. UTC | #4
Hi,

On Mon, 19 Dec 2022, Mark Brown wrote:

> On Mon, Dec 19, 2022 at 10:41:41AM +0100, Thorsten Leemhuis wrote:
> 
> > * there are no "CC: <stable..." tags in these patches. Is the plan to
> > manually ask for a backport? Or how can we get the regression fixed in
> > older releases?
> 
> Speak to the stable maintainers I guess, or hope their bot picks the
> commits up.

thanks Thorsten for the notice. These patches do lack the "Fixes:" tag, so 
it's possible the bots will not pick these up. I can follow up and send 
these to stable if this does not happen.

Br, Kai
Greg KH Dec. 20, 2022, 11:47 a.m. UTC | #5
On Tue, Dec 20, 2022 at 01:41:01PM +0200, Kai Vehmanen wrote:
> Hi,
> 
> On Mon, 19 Dec 2022, Mark Brown wrote:
> 
> > On Mon, Dec 19, 2022 at 10:41:41AM +0100, Thorsten Leemhuis wrote:
> > 
> > > * there are no "CC: <stable..." tags in these patches. Is the plan to
> > > manually ask for a backport? Or how can we get the regression fixed in
> > > older releases?
> > 
> > Speak to the stable maintainers I guess, or hope their bot picks the
> > commits up.
> 
> thanks Thorsten for the notice. These patches do lack the "Fixes:" tag, so 
> it's possible the bots will not pick these up. I can follow up and send 
> these to stable if this does not happen.

"Fixes:" guarantees nothing, please NEVER rely on that.  As per the
kernel documentation for the last 18+ years, you have to tag a commit
with the "Cc: stable@..." tag to ensure that it gets picked up properly.

thanks,

greg k-h
Thorsten Leemhuis Dec. 20, 2022, 11:53 a.m. UTC | #6
On 20.12.22 12:41, Kai Vehmanen wrote:
> 
> On Mon, 19 Dec 2022, Mark Brown wrote:
> 
>> On Mon, Dec 19, 2022 at 10:41:41AM +0100, Thorsten Leemhuis wrote:
>>
>>> * there are no "CC: <stable..." tags in these patches. Is the plan to
>>> manually ask for a backport? Or how can we get the regression fixed in
>>> older releases?
>>
>> Speak to the stable maintainers I guess, or hope their bot picks the
>> commits up.
> 
> thanks Thorsten for the notice. These patches do lack the "Fixes:" tag, so 
> it's possible the bots will not pick these up. I can follow up and send 
> these to stable if this does not happen.

Thanks, that would be great, I try to stay out of that business, as
actual developers of the code in question are in the best position to
judge and handle things like this.

Ciao, Thorsten