On Mon, 17 Jan 2022 18:11:42 +0100, Pierre-Louis Bossart wrote: > > > > On 1/17/22 2:49 AM, Takashi Iwai wrote: > > On Sun, 16 Jan 2022 12:18:17 +0100, > > Christophe JAILLET wrote: > >> > >> The commit below states that dpcm_be_connect() may be called from atomic > >> context. It changes a GFP_KERNEL into a GFP_ATOMIC to deal with it. > >> > >> Another memory allocation is done in dpcm_create_debugfs_state() which is > >> called by dpcm_be_connect(). Also use GFP_ATOMIC there to be consistent > >> and be compliant with atomic context. > >> > >> Fixes: d8a9c6e1f676 ("ASoC: soc-pcm: use GFP_ATOMIC for dpcm structure") > >> Signed-off-by: Christophe JAILLET <christophe.jaillet@xxxxxxxxxx> > >> --- > >> Not clear to me how dpcm_be_connect() can be called from an atomic context, > >> though. But better safe than sorry. > > > > I don't think this no longer valid for the very latest code. > > The commit b7898396f4bb dropped the spurious dpcm_lock spinlock, so > > the code path you touched must be always sleepable. > > > > Similarly, the commit d8a9c6e1f676 can be reverted now. > > Can we really revert d8a9c6e1f676? > > We did propagate the non-atomic FE property to the BE, but if both FE > and BE are both atomic that constraint would be required, no? At the kzalloc() call in dpcm_be_connect, there is no spin lock involved. It's merely protected by card->pcm_mutex, instead. The spinlock is applied at the later call with snd_soc_pcm_stream_lock_irq() only for the list manipulations. (See it's *_irq(), not *_irqsave(); that means the context being sleepable at that point.) So, we can use GFP_KERNEL safely there. GFP_ATOMIC was needed in the past where dpcm_be_connect() itself is called in dpcm_lock spinlock. It was removed recently. Takashi > > > > > > thanks, > > > > Takashi > > > >> --- > >> sound/soc/soc-pcm.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c > >> index 7abfc48b26ca..1a536a2b9dc3 100644 > >> --- a/sound/soc/soc-pcm.c > >> +++ b/sound/soc/soc-pcm.c > >> @@ -212,7 +212,7 @@ static void dpcm_create_debugfs_state(struct snd_soc_dpcm *dpcm, int stream) > >> { > >> char *name; > >> > >> - name = kasprintf(GFP_KERNEL, "%s:%s", dpcm->be->dai_link->name, > >> + name = kasprintf(GFP_ATOMIC, "%s:%s", dpcm->be->dai_link->name, > >> stream ? "capture" : "playback"); > >> if (name) { > >> dpcm->debugfs_state = debugfs_create_dir( > >> -- > >> 2.32.0 > >> >