Re: [PATCH v2 1/4] ALSA: core: Add async signal helpers

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, 01 Aug 2022 10:05:59 +0200,
Jaroslav Kysela wrote:
> 
> On 28. 07. 22 14:59, Takashi Iwai wrote:
> > Currently the call of kill_fasync() from an interrupt handler might
> > lead to potential spin deadlocks, as spotted by syzkaller.
> > Unfortunately, it's not so trivial to fix this lock chain as it's
> > involved with the tasklist_lock that is touched in allover places.
> > 
> > As a temporary workaround, this patch provides the way to defer the
> > async signal notification in a work.  The new helper functions,
> > snd_fasync_helper() and snd_kill_faync() are replacements for
> > fasync_helper() and kill_fasync(), respectively.  In addition,
> > snd_fasync_free() needs to be called at the destructor of the relevant
> > file object.
> > 
> > Signed-off-by: Takashi Iwai <tiwai@xxxxxxx>
> 
> ...
> 
> > +void snd_kill_fasync(struct snd_fasync *fasync, int signal, int poll)
> > +{
> > +	unsigned long flags;
> > +
> > +	if (!fasync || !fasync->on)
> > +		return;
> > +	spin_lock_irqsave(&snd_fasync_lock, flags);
> > +	fasync->signal = signal;
> > +	fasync->poll = poll;
> > +	list_move(&fasync->list, &snd_fasync_list);
> > +	schedule_work(&snd_fasync_work);
> > +	spin_unlock_irqrestore(&snd_fasync_lock, flags);
> > +}
> 
> The schedule_work() may be called outside the spinlock - it calls
> queue_work_on() / __queue_work() which has already own protection for
> the concurrent execution.

It can be outside, too, but scheduling earlier reduces the possible
unnecessary scheduling.  Suppose that a list is added while the work
is already running in another CPU.  If we call schedule_work() outside
this lock, it might be already the time after the work has processed
the queued item, and hence it can be a superfluous scheduling call.


thanks,

Takashi



[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Pulse Audio]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux