Re: [PATCH 4/9] md: factor out the rdev overlaps check from rdev_size_store

From: Guoqing Jiang <guoqing.jiang@linux.dev>
To: Christoph Hellwig <hch@lst.de>, Song Liu <song@kernel.org>
Cc: Logan Gunthorpe <logang@deltatee.com>,
	linux-raid@vger.kernel.org, linux-block@vger.kernel.org
Subject: Re: [PATCH 4/9] md: factor out the rdev overlaps check from rdev_size_store
Date: Fri, 15 Jul 2022 17:01:43 +0800	[thread overview]
Message-ID: <787a004b-321b-2b6c-3e54-d0587e3ad02e@linux.dev> (raw)
In-Reply-To: <20220713113125.2232975-5-hch@lst.de>

On 7/13/22 7:31 PM, Christoph Hellwig wrote:
> This splits the code into nicely readable chunks and also avoids
> the refcount inc/dec manipulations.
>
> Signed-off-by: Christoph Hellwig<hch@lst.de>
> ---
>   drivers/md/md.c | 84 +++++++++++++++++++++++--------------------------
>   1 file changed, 39 insertions(+), 45 deletions(-)
>
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 3127dcb8102ce..5346135ab51c8 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -3344,14 +3344,33 @@ rdev_size_show(struct md_rdev *rdev, char *page)
>   	return sprintf(page, "%llu\n", (unsigned long long)rdev->sectors / 2);
>   }
>   
> -static int overlaps(sector_t s1, sector_t l1, sector_t s2, sector_t l2)
> +static int md_rdevs_overlap(struct md_rdev *a, struct md_rdev *b)
>   {
>   	/* check if two start/length pairs overlap */
> -	if (s1+l1 <= s2)
> -		return 0;
> -	if (s2+l2 <= s1)
> -		return 0;
> -	return 1;
> +	if (a->data_offset + a->sectors <= b->data_offset)
> +		return false;
> +	if (b->data_offset + b->sectors <= a->data_offset)
> +		return false;
> +	return true;
> +}

Given it returns true or false, better to change the return type to bool.

> +
> +static bool md_rdev_overlaps(struct md_rdev *rdev)
> +{

The two names (md_rdevs_overlap/md_rdev_overlaps) are quite similar ...

Thanks,
Guoqing