On Sat, Dec 13, 2014 at 12:12:19AM +0100, AH wrote: > Mark Brown wrote on 12.12.2014 17:36: > >On Wed, Dec 10, 2014 at 04:48:19PM +0100, Andrzej Hajda wrote: > >>+ kfree(ptask); > >>+ > >>+ if (empty) > >>+ break; > >>+ > >>+ track_process_task(track, task); > >...we then go and do some other stuff, including processing that task, > >without the lock or or any other means I can see of excluding other > >users before going round and removing the task. This seems to leave us > >vulnerable to double execution. > No, if you look at track_add_task function you will see that the queue is > processed only if it is initially empty, otherwise the task is only added to > the queue, so it will be processed after processing earlier tasks. > So the rule is that if someone add task to the queue it checks if the queue > is empty, in such case it process all tasks from the queue until > the queue becomes empty, even the tasks added by other processed. > This way all tasks are serialized. This is all pretty fiddly and seems fragile - if nothing else the code seems undercommented since the above is only going to be apparent with following through multiple functions and we're relying on both owner and list emptiness with more than one place where a task can get processed. > >I'm also unclear what is supposed to happen if adding a notification > >races with removing the thing being watched. > The sequence should be always as follows: > 1. create thing, then call track_up(thing). > ... > 2. call track_down(thing) then remove thing. > If we put 1 into probe and 2 into remove callback of the driver it will be > safe - we are synchronised by device_lock. But if, for some reason, we want > to create object after probe we should do own synchronization or just put > device_lock around 1. The same applies if we want to remove > object earlier. This is the comment above about. I will expand it to more > verbose explanation. You can't rely on the device lock here since this isn't tied to kobjects or anything at all - it's a freestanding interface someone could pick up and use in another context. Besides, that isn't really my concern - my concern is what happens if something asks to wait for
Attachment:
signature.asc
Description: Digital signature
