Re: [PATCH 11/18] swim: dequeue in-flight request

[Tejun Heo <tj@kernel.org>]

Hello, Sergei.

Sergei Shtylyov wrote:
>> Similar response as the if/else one on the other thread.  Is it really
>> any significantly better?  The 'duplication' here is basically one
>> liner
> 
>   Not true, it's 3-liner. I wouldn't bother with one liner.

The final result is...

	req = blk_fetch_request(q);
	while (req) {
		int err = -EIO;

That looks like one liner to me.

>> after the peek/fetch change
> 
>   The peek/fetch code itself is duplicated. :-/

Do you mean by inlining?  Please note that blk_fetch_request() is not
inlined anymore after Fujita's bidi end_request cleanup patches.

>> and when the duplication is minimal,
>> I usually find it clearer to put the loop condition at the while
>> clause itself.
> 
>   No problem, we could just keep an old form of *while* loop.
> 
>> If you think it's significantly better,
> 
>   I do hink it avoids duplicating peek/fetch code.
> 
>> please go ahead and submit the patch but to me the change you're
>> proposing is
>> basically cosmetic and not even a clearly better one at that.
> 
> Should probably look at the resulting assembly to see how much it's
> differrent.

Do you seriously think it's worthwhile to optimize the request loop
according to assembly generation?  That sounds like a bad case of over
(micro) optimization to me.  It's not gonna make any noticeable
difference and the changes you make today can be irrelevant or even
deterimental tomorrow depending on any number of parameters.  Please
don't do it for performance reasons.

Thanks.

-- 
tejun