| Message ID | 20221127-snd-freeze-v8-0-3bc02d09f2ce@chromium.org |
|---|---|
| Headers | show |
| Series | ASoC: SOF: Fix deadlock when shutdown a frozen userspace | expand |
Hi Oliver Thanks for your review On Thu, 1 Dec 2022 at 13:29, Oliver Neukum <oneukum@suse.com> wrote: > > On 01.12.22 12:08, Ricardo Ribalda wrote: > > If we are shutting down due to kexec and the userspace is frozen, the > > system will stall forever waiting for userspace to complete. > > > > Do not wait for the clients to complete in that case. > > Hi, > > I am afraid I have to state that this approach is bad in every case, > not just this corner case. It basically means that user space can stall > the kernel for an arbitrary amount of time. And we cannot have that. > > Regards > Oliver This patchset does not modify this behaviour. It simply fixes the stall for kexec(). The patch that introduced the stall: 83bfc7e793b5 ("ASoC: SOF: core: unregister clients and machine drivers in .shutdown") was sent as a generalised version of: https://github.com/thesofproject/linux/pull/3388 AFAIK, we would need a similar patch for every single board.... which I am not sure it is doable in a reasonable timeframe. On the meantime this seems like a decent compromises. Yes, a miss-behaving userspace can still stall during suspend, but that was not introduced in this patch. Regards! >
Hi Oliver On Thu, 1 Dec 2022 at 14:22, 'Oliver Neukum' via Chromeos Kdump <chromeos-kdump@google.com> wrote: > > On 01.12.22 14:03, Ricardo Ribalda wrote: > > Hi, > > > This patchset does not modify this behaviour. It simply fixes the > > stall for kexec(). > > > > The patch that introduced the stall: > > 83bfc7e793b5 ("ASoC: SOF: core: unregister clients and machine drivers > > in .shutdown") > > That patch is problematic. I would go as far as saying that > it needs to be reverted. > It fixes a real issue. We have not had any complaints until we tried to kexec in the platform. I wont recommend reverting it until we have an alternative implementation. kexec is far less common than suspend/reboot. > > was sent as a generalised version of: > > https://github.com/thesofproject/linux/pull/3388 > > > > AFAIK, we would need a similar patch for every single board.... which > > I am not sure it is doable in a reasonable timeframe. > > > > On the meantime this seems like a decent compromises. Yes, a > > miss-behaving userspace can still stall during suspend, but that was > > not introduced in this patch. > > Well, I mean if you know what wrong then I'd say at least return to > a sanely broken state. > > The whole approach is wrong. You need to be able to deal with user > space talking to removed devices by returning an error and keeping > the resources association with the open file allocated until > user space calls close() In general, the whole shutdown is broken for all the subsystems ;). It is a complicated issue. Users handling fds, devices with DMAs in the middle of an operation, dma fences.... Unfortunately I am not that familiar with the sound subsystem to make a proper patch for this. > > Regards > Oliver > > > > -- > You received this message because you are subscribed to the Google Groups "Chromeos Kdump" group. > To unsubscribe from this group and stop receiving emails from it, send an email to chromeos-kdump+unsubscribe@google.com. > To view this discussion on the web, visit https://groups.google.com/a/google.com/d/msgid/chromeos-kdump/d3730d1d-6f92-700a-06c4-0e0a35e270b0%40suse.com.
Since: 83bfc7e793b5 ("ASoC: SOF: core: unregister clients and machine drivers in .shutdown") we wait for all the workloads to be completed during shutdown. This was done to avoid a stall once the device is started again. Unfortunately this has the side effect of stalling kexec(), if the userspace is frozen. Let's handle that case. To: Joel Fernandes <joel@joelfernandes.org> To: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> To: Liam Girdwood <lgirdwood@gmail.com> To: Peter Ujfalusi <peter.ujfalusi@linux.intel.com> To: Bard Liao <yung-chuan.liao@linux.intel.com> To: Ranjani Sridharan <ranjani.sridharan@linux.intel.com> To: Kai Vehmanen <kai.vehmanen@linux.intel.com> To: Daniel Baluta <daniel.baluta@nxp.com> To: Mark Brown <broonie@kernel.org> To: Jaroslav Kysela <perex@perex.cz> To: Takashi Iwai <tiwai@suse.com> To: Eric Biederman <ebiederm@xmission.com> To: Chromeos Kdump <chromeos-kdump@google.com> To: Steven Rostedt <rostedt@goodmis.org> To: Michael Ellerman <mpe@ellerman.id.au> To: Nicholas Piggin <npiggin@gmail.com> To: Christophe Leroy <christophe.leroy@csgroup.eu> To: "K. Y. Srinivasan" <kys@microsoft.com> To: Haiyang Zhang <haiyangz@microsoft.com> To: Wei Liu <wei.liu@kernel.org> To: Dexuan Cui <decui@microsoft.com> To: Thomas Gleixner <tglx@linutronix.de> To: Ingo Molnar <mingo@redhat.com> To: Borislav Petkov <bp@alien8.de> To: Dave Hansen <dave.hansen@linux.intel.com> To: x86@kernel.org To: "H. Peter Anvin" <hpa@zytor.com> To: Juergen Gross <jgross@suse.com> To: Boris Ostrovsky <boris.ostrovsky@oracle.com> To: Ard Biesheuvel <ardb@kernel.org> To: Bjorn Helgaas <bhelgaas@google.com> To: "Rafael J. Wysocki" <rafael@kernel.org> To: Pavel Machek <pavel@ucw.cz> To: Len Brown <len.brown@intel.com> Cc: stable@vger.kernel.org Cc: sound-open-firmware@alsa-project.org Cc: alsa-devel@alsa-project.org Cc: linux-kernel@vger.kernel.org Cc: kexec@lists.infradead.org Cc: linuxppc-dev@lists.ozlabs.org Cc: linux-hyperv@vger.kernel.org Cc: xen-devel@lists.xenproject.org Cc: linux-efi@vger.kernel.org Cc: linux-pci@vger.kernel.org Cc: linux-pm@vger.kernel.org Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> --- Changes in v8: - Wrap pm_freezing and kexec_inprogress in functions. - Do not run snd_sof_machine_unregister(sdev, pdata) during kexec (Thanks Kai). - Link to v7: https://lore.kernel.org/r/20221127-snd-freeze-v7-0-127c582f1ca4@chromium.org Changes in v7: - Fix commit message (Thanks Pierre-Louis). - Link to v6: https://lore.kernel.org/r/20221127-snd-freeze-v6-0-3e90553f64a5@chromium.org Changes in v6: - Check if we are in kexec with the userspace frozen. - Link to v5: https://lore.kernel.org/r/20221127-snd-freeze-v5-0-4ededeb08ba0@chromium.org Changes in v5: - Edit subject prefix. - Link to v4: https://lore.kernel.org/r/20221127-snd-freeze-v4-0-51ca64b7f2ab@chromium.org Changes in v4: - Do not call snd_sof_machine_unregister from shutdown. - Link to v3: https://lore.kernel.org/r/20221127-snd-freeze-v3-0-a2eda731ca14@chromium.org Changes in v3: - Wrap pm_freezing in a function. - Link to v2: https://lore.kernel.org/r/20221127-snd-freeze-v2-0-d8a425ea9663@chromium.org Changes in v2: - Only use pm_freezing if CONFIG_FREEZER . - Link to v1: https://lore.kernel.org/r/20221127-snd-freeze-v1-0-57461a366ec2@chromium.org --- Ricardo Ribalda (3): kexec: Refactor kexec_in_progress into a function freezer: refactor pm_freezing into a function. ASoC: SOF: Fix deadlock when shutdown a frozen userspace arch/powerpc/platforms/pseries/vio.c | 2 +- arch/x86/kernel/cpu/mshyperv.c | 6 +++--- arch/x86/xen/enlighten_hvm.c | 2 +- drivers/firmware/efi/efi.c | 2 +- drivers/pci/pci-driver.c | 2 +- include/linux/freezer.h | 3 ++- include/linux/kexec.h | 5 ++--- kernel/freezer.c | 3 +-- kernel/kexec_core.c | 12 ++++++++++-- kernel/power/process.c | 24 ++++++++++++++++++++---- sound/soc/sof/core.c | 9 ++++++--- 11 files changed, 48 insertions(+), 22 deletions(-) --- base-commit: 4312098baf37ee17a8350725e6e0d0e8590252d4 change-id: 20221127-snd-freeze-1ee143228326 Best regards,