Re: [PATCH] staging: greybus: operation: add generic timeout support

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

 



On Tue, Feb 07, 2017 at 02:31:04PM +0000, Bryan O'Donoghue wrote:
> On 07/02/17 14:19, Johan Hovold wrote:
> > On Mon, Jan 23, 2017 at 01:04:14PM +0100, Johan Hovold wrote:
> >> Add a struct timer_list to struct gb_operation and use that to implement
> >> generic operation timeouts.
> >>
> >> This simplifies the synchronous operation handling somewhat while also
> >> providing a generic timeout mechanism that drivers can use for
> >> asynchronous operations.
> >>
> >> Signed-off-by: Johan Hovold <johan@xxxxxxxxxx>
> > 
> > Greg,
> > 
> > I believe you can apply this one now. This is the right way to implement
> > operation timeouts, and it is independent of Bryan's patches converting
> > loopback to use the new interface, which can be applied on top when they
> > are ready.
> > 
> > Thanks,
> > Johan
> > 
> 
> Haven't really had time to look at this but, since it's the way you want
> to do it, I guess go ahead.

That's alright. As I pointed out in my review of your patches, using
delayed work to unconditionally try to cancel every operation is
needlessly inefficient (remember that an operation timing out is also
the exceptional case) and you run into problems with synchronisation
during connection tear down (and even risk of deadlocks if using a
single workqueue).

My approach using a single timer which either times out or is cancelled
upon completion is about as efficient as this can get, and therefore
also allows the current synchronous-operation implementation to be built
on top of this generic mechanism.

Johan
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel



[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux