On Fri, Aug 12, 2016 at 10:01 PM, Takashi Iwai <tiwai@xxxxxxx> wrote: > 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@xxxxxxx> 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? > Right, the 'active' flag is cleary not a good lock... >> 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? > There's read_lock, unfortunately after "fixing" the sleep-in-atomic in line6_hwdep_read(), it's broken now... >> 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. > Okay, I'll try to rethink/rework it, simplify the code, and get back. Thanks! -- Andrej _______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxx http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
