Re: [PATCH] blk: improve order of bio handling in generic_make_request()

[Jack Wang]

On 06.03.2017 05:40, NeilBrown wrote:
> On Fri, Mar 03 2017, Jack Wang wrote:
>>
>> Thanks Neil for pushing the fix.
>>
>> We can optimize generic_make_request a little bit:
>> - assign bio_list struct hold directly instead init and merge
>> - remove duplicate code
>>
>> I think better to squash into your fix.
> 
> Hi Jack,
>  I don't object to your changes, but I'd like to see a response from
>  Jens first.
>  My preference would be to get the original patch in, then other changes
>  that build on it, such as this one, can be added.  Until the core
>  changes lands, any other work is pointless.
> 
>  Of course if Jens wants a this merged before he'll apply it, I'll
>  happily do that.
> 
> Thanks,
> NeilBrown

Hi Neil,

Totally agree, let's wait for Jens's decision.

Hi Jens,

Please consider this fix also for stable 4.3+ 

Thanks,
Jack Wang

> 
> 
> 
>> ---
>>  block/blk-core.c | 9 ++-------
>>  1 file changed, 2 insertions(+), 7 deletions(-)
>>
>> diff --git a/block/blk-core.c b/block/blk-core.c
>> index 3bc7202..b29b7e5 100644
>> --- a/block/blk-core.c
>> +++ b/block/blk-core.c
>> @@ -2147,8 +2147,7 @@ blk_qc_t generic_make_request(struct bio *bio)
>>  			struct bio_list lower, same, hold;
>>  
>>  			/* Create a fresh bio_list for all subordinate requests */
>> -			bio_list_init(&hold);
>> -			bio_list_merge(&hold, &bio_list_on_stack);
>> +			hold = bio_list_on_stack;
>>  			bio_list_init(&bio_list_on_stack);
>>  
>>  			ret = q->make_request_fn(q, bio);
>> @@ -2168,14 +2167,10 @@ blk_qc_t generic_make_request(struct bio *bio)
>>  			bio_list_merge(&bio_list_on_stack, &lower);
>>  			bio_list_merge(&bio_list_on_stack, &same);
>>  			bio_list_merge(&bio_list_on_stack, &hold);
>> -
>> -			bio = bio_list_pop(current->bio_list);
>>  		} else {
>> -			struct bio *bio_next = bio_list_pop(current->bio_list);
>> -
>>  			bio_io_error(bio);
>> -			bio = bio_next;
>>  		}
>> +		bio = bio_list_pop(current->bio_list);
>>  	} while (bio);
>>  	current->bio_list = NULL; /* deactivate */
>>  
>> -- 
>>
>> Regards,
>> Jack