diff mbox series

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

Message ID 20220713113125.2232975-5-hch@lst.de (mailing list archive)
State New, archived
Headers show
Series [1/9] md: fix error handling in md_alloc | expand

Commit Message

Christoph Hellwig July 13, 2022, 11:31 a.m. UTC 
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(-)

Comments

Guoqing Jiang July 15, 2022, 9:01 a.m. UTC | #1 
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
diff mbox series

Patch

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;
+}
+
+static bool md_rdev_overlaps(struct md_rdev *rdev)
+{
+	struct mddev *mddev;
+	struct md_rdev *rdev2;
+
+	spin_lock(&all_mddevs_lock);
+	list_for_each_entry(mddev, &all_mddevs, all_mddevs) {
+		rdev_for_each(rdev2, mddev) {
+			if (rdev != rdev2 && rdev->bdev == rdev2->bdev &&
+			    md_rdevs_overlap(rdev, rdev2)) {
+				spin_unlock(&all_mddevs_lock);
+				return true;
+			}
+		}
+	}
+	spin_unlock(&all_mddevs_lock);
+	return false;
 }
 
 static int strict_blocks_to_sectors(const char *buf, sector_t *sectors)
@@ -3403,46 +3422,21 @@  rdev_size_store(struct md_rdev *rdev, const char *buf, size_t len)
 		return -EINVAL; /* component must fit device */
 
 	rdev->sectors = sectors;
-	if (sectors > oldsectors && my_mddev->external) {
-		/* Need to check that all other rdevs with the same
-		 * ->bdev do not overlap.  'rcu' is sufficient to walk
-		 * the rdev lists safely.
-		 * This check does not provide a hard guarantee, it
-		 * just helps avoid dangerous mistakes.
-		 */
-		struct mddev *mddev;
-		int overlap = 0;
-		struct list_head *tmp;
 
-		rcu_read_lock();
-		for_each_mddev(mddev, tmp) {
-			struct md_rdev *rdev2;
-
-			rdev_for_each(rdev2, mddev)
-				if (rdev->bdev == rdev2->bdev &&
-				    rdev != rdev2 &&
-				    overlaps(rdev->data_offset, rdev->sectors,
-					     rdev2->data_offset,
-					     rdev2->sectors)) {
-					overlap = 1;
-					break;
-				}
-			if (overlap) {
-				mddev_put(mddev);
-				break;
-			}
-		}
-		rcu_read_unlock();
-		if (overlap) {
-			/* Someone else could have slipped in a size
-			 * change here, but doing so is just silly.
-			 * We put oldsectors back because we *know* it is
-			 * safe, and trust userspace not to race with
-			 * itself
-			 */
-			rdev->sectors = oldsectors;
-			return -EBUSY;
-		}
+	/*
+	 * Check that all other rdevs with the same bdev do not overlap.  This
+	 * check does not provide a hard guarantee, it just helps avoid
+	 * dangerous mistakes.
+	 */
+	if (sectors > oldsectors && my_mddev->external &&
+	    md_rdev_overlaps(rdev)) {
+		/*
+		 * Someone else could have slipped in a size change here, but
+		 * doing so is just silly.  We put oldsectors back because we
+		 * know it is safe, and trust userspace not to race with itself.
+		 */
+		rdev->sectors = oldsectors;
+		return -EBUSY;
 	}
 	return len;
 }