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

Johan Hovold johan at kernel.org
Wed Feb 8 11:55:34 UTC 2017


On Wed, Feb 08, 2017 at 11:43:57AM +0000, Bryan O'Donoghue wrote:
> On 08/02/17 09:43, Johan Hovold wrote:
> > 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 at kernel.org>
> >>>
> >>> 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.

> > 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.
> 
> I'm just wondering what impact it has instead of
> wait_event_interruptible() more/less overhead (I suspect more) - and I
> reckoned you'd not be on for that change - so only made a change on the
> asynchronous path.

Yes, your approach with unconditional scheduling of a work struct for
every operation was needlessly inefficient and I did not want that for
the synchronous path as it would definitely be a regression (even if we
would have gone with your patches as an intermediate step for the
asynchronous case).

My approach does not suffer from such overhead so can therefore be used
as a generic mechanism (there's exactly one timer involved whether we
use wait_event_interruptible_timer or not).

Johan


More information about the devel mailing list