[alsa-devel] [PATCH 09/15] ALSA: line6: Add hwdep interface to access the POD control messages

[Takashi Iwai]

On Fri, 12 Aug 2016 18:43:30 +0200,
Andrej Kruták wrote:
> 
> On Fri, Aug 12, 2016 at 2:30 PM, Takashi Iwai <tiwai at suse.de> wrote:
> > On Fri, 12 Aug 2016 14:15:16 +0200,
> >>
> >> >> > Also, the blocking read/write control isn't usually done by a
> >> >> > semaphore.  Then you can handle the interrupt there.
> >> >> >
> >> >> >
> >> >>
> >> >> I actually wonder why, semaphores seemed perfect for this... Do you
> >> >> have some hints?
> >> >
> >> > Assume you want to interrupt the user-space app while it's being
> >> > blocked by the semaphore.  With your code, you can't.
> >> >
> >> >
> >>
> >> You can - down_interruptible() is there for this exact reason.
> >
> > There is another blocking path: you keep semaphore down after
> > line6_hwdep_read() until line6_hwdep_push_message().  What happens if
> > user-space interrupts during that, and line6_hwdep_push_message() is
> > delayed or stall by some reason?
> >
> 
> Actually, I think I don't see what's the "another path" here, could
> you please elaborate one more bit? I just want to make sure to not
> reimplement the same race using waitqueue...
> 
> What's the point then? line6_hwdep_push_message() could get not
> scheduled for some while. So until it calls up(), line6_hwdep_read()
> will block on down_interruptible(), or until signal (in which case
> user gets -ERESTARTSYS). After up() is called, there are data in
> buffer...

Well, what happens if user aborts before up() is called in
line6_hwdep_push_message()?  Now the driver calls close, and it frees
the memory.  What if, at the very same time,
line6_hwdep_push_message() is triggered?

> If line6_hwdep_read() returns after interrupt, no problem -
> the buffer will just continue to be filled and semaphore will be
> up()'d while there's free buffer space. Or until the device is
> closed...

Or, what if line6_hwdep_push_message() is triggered twice or more
before line6_hwdep_read() is called?  It will call up() twice or
more.  Then at this point, you call line6_hwdep_read() concurrently
from two threads...  How do they protect against each other?

> If I do the same via waitqueue, I will have the same problems, no?
> Maybe if you could post the steps where you see the race...

In your code, it's not clear that you're protecting from what.
A simple lock+wait loop shows it more easily, at least.


Takashi

> At the same time, looking at __down_common(), it just does the
> standard waitqueue stuff (TASK_INTERRUPTIBLE + schedule()) (+counter
> in down())... So do you have some other race in mind? I'm running in
> circles, so surely you must :-)
> 
> 
> Sorry if I sound like a moron... and thanks for you time!
> 
> -- 
> Andrej
>