Re: [PATCH 2/2] mdadm: raid10.c Remove near atomic break

[NeilBrown <neilb@suse.com>]

[-- Attachment #1: Type: text/plain, Size: 1948 bytes --]

On Fri, Nov 04 2016, Robert LeBlanc wrote:

> This is always triggered for small reads preventing spreading the reads
> across all available drives. The comments are also confusing as it is
> supposed to apply only to 'far' layouts, but really only applies to 'near'
> layouts. Since there isn't problems with 'far' layouts, there shouldn't
> be a problem for 'near' layouts either. This change fairly distributes
> reads across all drives where before only came from the first drive.

Why is "fairness" an issue?
The current code will use a device if it finds that it is completely
idle. i.e. if nr_pending is 0.
Why is that ever the wrong thing to do?

Does your testing show that overall performance is improved?  If so,
that would certainly be useful.
But it isn't clear (to me) that simply spreading the load more "fairly"
is a worthy goal.

Thanks,
NeilBrown


>
> Signed-off-by: Robert LeBlanc <robert@leblancnet.us>
> ---
>  drivers/md/raid10.c | 7 -------
>  1 file changed, 7 deletions(-)
>
> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
> index be1a9fc..8d83802 100644
> --- a/drivers/md/raid10.c
> +++ b/drivers/md/raid10.c
> @@ -777,13 +777,6 @@ static struct md_rdev *read_balance(struct r10conf *conf,
>  		if (!do_balance)
>  			break;
>  
> -		/* This optimisation is debatable, and completely destroys
> -		 * sequential read speed for 'far copies' arrays.  So only
> -		 * keep it for 'near' arrays, and review those later.
> -		 */
> -		if (geo->near_copies > 1 && !atomic_read(&rdev->nr_pending))
> -			break;
> -
>  		/* for far > 1 always use the lowest address */
>  		if (geo->far_copies > 1)
>  			new_distance = r10_bio->devs[slot].addr;
> -- 
> 2.10.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-raid" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 800 bytes --]