Re: [PATCH] ALSA: dummy: Defer hrtimer data freeing until runtime detach

[Takashi Iwai <tiwai@suse.de>]

On Wed, 14 Oct 2026 10:35:24 +0200,
Cen Zhang wrote:
> 
> Hi Takashi,
> 
> >
> > Hmm, it's not clear how this happens.  It looks like a typical
> > obscuration by AI reports...
> >
> > Basically dummy_hrtimer_sync_stop should handled the hrtimer cancel,
> > but maybe the problem is rather its lack at releasing?  If so, the
> > patch below should cover instead.
> >
> >
> > thanks,
> >
> > Takashi
> 
> Thank you for looking at this. My commit message did not explain the
> scenario clearly. Sorry for the confusion.
> 
> The observed failure involves a PREPARE ioctl on a surviving linked
> substream. The test opens two dummy capture substreams, A and B, with
> the hrtimer backend selected, configures both with HW_PARAMS, and links
> them with LINK. It then performs the final close of A while issuing
> PREPARE on B. It does not issue START.
> 
> On the current for-next source, the close path is:
> 
>   snd_pcm_release_substream(A)
>     snd_pcm_drop(A)
>     do_hw_free(A)
>       snd_pcm_sync_stop(A, true)
>     dummy_pcm_close(A)
>       dummy_hrtimer_free(A)
>         kfree(dpcm)
>     pcm_release_private(A)
>       snd_pcm_unlink(A)
>     snd_pcm_detach_substream(A)
> 
> So the close path already synchronizes the timer before freeing dpcm.
> The gap is between driver close and unlink. snd_pcm_drop() leaves the
> linked group in SETUP. The release path calls do_hw_free() directly;
> unlike the HW_FREE ioctl wrapper, that helper does not change the
> runtime state to OPEN. A therefore remains linked and in a state that
> snd_pcm_pre_prepare() accepts until the later unlink.
> 
> The relevant ordering is:
> 
>   Target A final close              PREPARE on linked peer B
>   --------------------              ------------------------
>   snd_pcm_drop(): group -> SETUP
>   do_hw_free(): synchronize timer
>   dummy_pcm_close(): free dpcm
>                                     Take link read lock
>                                     Walk group, including A
>                                     Accept A's SETUP state
>                                     dummy_pcm_prepare(A)
>                                       Read freed timer_ops
>   pcm_release_private()
>     snd_pcm_unlink(): link write lock
> 
> PREPARE holds the link read lock and the participating runtime buffer
> mutexes. Driver close does not hold those locks across the free. Timer
> cancellation waits for timer callbacks, but does not wait for this
> separate PREPARE action. The later unlink is what prevents another
> group action from visiting A.
> 
> >
> > -- 8< --
> > --- a/sound/core/pcm_native.c
> > +++ b/sound/core/pcm_native.c
> > @@ -2301,6 +2301,8 @@ static int snd_pcm_drop(struct snd_pcm_substream *substream)
> >         result = snd_pcm_stop(substream, SNDRV_PCM_STATE_SETUP);
> >         /* runtime->control->appl_ptr = runtime->status->hw_ptr; */
> >
> > +       snd_pcm_sync_stop(substream, true);
> > +
> >         return result;
> >  }
> >
> 
> Synchronizing a pending hrtimer callback before release is sensible.
> For the linked PREPARE ordering above, though, I think an additional
> lifetime step may still be needed. PREPARE on B can start after the
> extra sync_stop() has completed and after A's close has freed dpcm,
> while A is still linked. It can then reach A's prepare callback as
> shown above. Waiting for timer callbacks alone does not exclude that
> later group action.

That's not what I didn't understand: do both A and B use the same
device?  The release shouldn't happen until all detached, so the
prepare and the release can't happen concurrently for the same device.

If A and B are for different devices, they don't interfere with each
other in the case of dummy driver.


Takashi