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

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

 



On 08/02/17 11:55, Johan Hovold wrote:
> 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@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.
> 
>>> 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).

Yes, I'm wondering what impact switching away from wait_for_completion*
in favour of add/cancel timer has for every operation.

It's probably irrelevant.

> 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).

Hmm yes, I was surprised that the first feedback from you on this was an
alternative patch but, since you feel strongly about doing it this way,
it's fine with me.

Acked-by: Bryan O'Donoghue <pure.logic@xxxxxxxxxxxxxxxxx>

-- 
bod
_______________________________________________
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