Re: [RFC PATCH] md/raid10: refactor some codes from raid10_write_request

[Guoqing Jiang <gqjiang@suse.com>]

On 03/15/2017 12:48 AM, Shaohua Li wrote:
> On Mon, Mar 13, 2017 at 05:23:59PM +0800, Guoqing Jiang wrote:
>> Previously, we clone both bio and repl_bio in raid10_write_request,
>> then add the cloned bio to plug->pending or conf->pending_bio_list
>> based on plug or not, and most of the logics are same for the two
>> conditions.
>>
>> So introduce handle_clonebio (a better name is welcome) for it, and
>> use replacement parameter to distinguish the difference. No functional
>> changes in the patch.
>>
>> Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
>> ---
>> Another reason for it is to improve the readability of code, but
>> I didn't touch raid10 before so this is labeled as RFC.
>>
>>   drivers/md/raid10.c | 172 ++++++++++++++++++++++------------------------------
>>   1 file changed, 72 insertions(+), 100 deletions(-)
>>
>> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
>> index b1b1f982a722..02d8eff8d26e 100644
>> --- a/drivers/md/raid10.c
>> +++ b/drivers/md/raid10.c
>> @@ -1188,18 +1188,81 @@ static void raid10_read_request(struct mddev *mddev, struct bio *bio,
>>   	return;
>>   }
>>   
>> -static void raid10_write_request(struct mddev *mddev, struct bio *bio,
>> -				 struct r10bio *r10_bio)
>> +static void handle_clonebio(struct mddev *mddev, struct r10bio *r10_bio,
>> +			    struct bio *bio, int i, int replacement,
>> +			    int max_sectors)
> Maybe raid10_write_one_disk?

Thanks for review, I will use it unless a better name is found.

>   Please replace 'i' with a meaningful name and
> change to 'boo' for replacement.

Ok, I would replace 'i' with 'n_copy', what is the meaning of 'boo'?
IMO, replacement fits better here, thanks.

+		if (r10_bio->devs[i].bio)
+			handle_clonebio(mddev, r10_bio, bio, i, 0, max_sectors);
+		if (r10_bio->devs[i].repl_bio)
+			handle_clonebio(mddev, r10_bio, bio, i, 1, max_sectors);



Regards,
Guoqing