On Sat, Oct 17, 2015 at 04:34:28PM +0000, Sell, Timothy C wrote: > > How can you guarantee it? Please document that somehow, I don't see it > > here in this patch how that can be true, but maybe I'm not looking close > > enough... > > Sure; I'll add some code comments. Basically they will amount to this: > * The kref is initialized to 1 in _probe(), and the book-end for that is > the devdata_put() in _remove(). > * devdata_get() / devdata_put() is called to keep the ref valid during > interrupt processing in visorinput_channel_interrupt(). The visorbus > model guarantees that at most 1 thread of execution will ever be > running this function at a time. > * We are guaranteed that the kref will be >= 1 when entering > visorinput_channel_interrupt(), and that no other thread of execution can > do a kref_put() at that point, because the devdata_put() > in _remove() is only done AFTER disabling interrupts. Once > interrupts are disabled, NO thread of execution will be running > visorinput_channel_interrupt(). So the lifetime of this kref mirrors the lifetime of the struct device? Why have a kref at all then? > * "[PATCH 6/9] staging: unisys: visorinput: respond to resolution changes on-the-fly" > (a subsequent patch in this series) > requires that devdata be accessed from a workqueue, but does the > required devdata_get() before queuing the work (which occurs in > visorinput_channel_interrupt(), where we know the kref count is actually >= 2), > and devdata_put() after the work is processed. Because devdata_get() is > NEVER done at a time when it is possible that the kref is 0, we do NOT > require a lock. > * In actuality, there is only 1 scenario when devdata_get()/_put() could be > running on > 1 thread of execution simultaneously: The devdata_put() > performed after processing of the workitem can occur asynchronously with > the devdata_put in _remove(). But since those both involve only kref_put(), > we are safe calling them on multiple threads without a lock, as only the last > one will be calling release(), due to atomic operation in kref_put(). > > I don't see a case for needing a lock, but of course it's entirely possible > that I've missed something. > > Honestly, I was a bit surprised to see how FEW usages of > kref_put_mutex() and kref_put_spinlock_irqsave() there actually were > in the kernel. Well, most of the usages are probably wrong, let's not continue to write bad code :) thanks, greg k-h _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
