Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   linan666@huaweicloud.com
To:     axboe@kernel.dk, linan122@huawei.com, vishal.l.verma@intel.com,
        dan.j.williams@intel.com
Cc:     linux-block@vger.kernel.org, linux-kernel@vger.kernel.org,
        yukuai3@huawei.com, yi.zhang@huawei.com, houtao1@huawei.com,
        yangerkun@huawei.com
Subject: [PATCH v2 07/11] block/badblocks: check bb->count instead of 'hi > lo'
Date:   Thu,  4 May 2023 20:48:24 +0800
Message-Id: <20230504124828.679770-8-linan666@huaweicloud.com>
In-Reply-To: <20230504124828.679770-1-linan666@huaweicloud.com>
References: <20230504124828.679770-1-linan666@huaweicloud.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

From: Li Nan <linan122@huawei.com>

'hi > lo' only holds when bb->count is 0. Check it is complicated after
dichotomy. Check bb->count instead. And this will make future fix more
convenient.

No functional change intended.

Signed-off-by: Li Nan <linan122@huawei.com>
---
 block/badblocks.c | 155 +++++++++++++++++++++++-----------------------
 1 file changed, 79 insertions(+), 76 deletions(-)

diff --git a/block/badblocks.c b/block/badblocks.c
index c11eb869f2f3..3cb8513cbd7f 100644
--- a/block/badblocks.c
+++ b/block/badblocks.c
@@ -187,28 +187,32 @@ int badblocks_set(struct badblocks *bb, sector_t s, int sectors,
 	p = bb->page;
 	lo = 0;
 	hi = bb->count;
-	/* Find the last range that starts at-or-before 's' */
-	while (hi - lo > 1) {
-		int mid = (lo + hi) / 2;
-		sector_t a = BB_OFFSET(p[mid]);
-
-		if (a <= s)
-			lo = mid;
-		else
-			hi = mid;
-	}
-	if (hi > lo && BB_OFFSET(p[lo]) > s)
-		hi = lo;
+	if (bb->count) {
+		sector_t a;
+		sector_t e;
+		int ack;
+
+		/* Find the last range that starts at-or-before 's' */
+		while (hi - lo > 1) {
+			int mid = (lo + hi) / 2;
+
+			a = BB_OFFSET(p[mid]);
+			if (a <= s)
+				lo = mid;
+			else
+				hi = mid;
+		}
 
-	if (hi > lo) {
 		/* we found a range that might merge with the start
 		 * of our new range
 		 */
-		sector_t a = BB_OFFSET(p[lo]);
-		sector_t e = a + BB_LEN(p[lo]);
-		int ack = BB_ACK(p[lo]);
+		a = BB_OFFSET(p[lo]);
+		e = a + BB_LEN(p[lo]);
+		ack = BB_ACK(p[lo]);
 
-		if (e >= s) {
+		if (a > s) {
+			hi = lo;
+		} else if (e >= s) {
 			/* Yes, we can merge with a previous range */
 			if (s == a && s + sectors >= e)
 				/* new range covers old */
@@ -232,72 +236,71 @@ int badblocks_set(struct badblocks *bb, sector_t s, int sectors,
 			}
 			sectors = e - s;
 		}
-	}
-	if (sectors && hi < bb->count) {
-		/* 'hi' points to the first range that starts after 's'.
-		 * Maybe we can merge with the start of that range
-		 */
-		sector_t a = BB_OFFSET(p[hi]);
-		sector_t e = a + BB_LEN(p[hi]);
-		int ack = BB_ACK(p[hi]);
-
-		if (a <= s + sectors) {
-			/* merging is possible */
-			if (e <= s + sectors) {
-				/* full overlap */
-
-				e = s + sectors;
-				ack = acknowledged;
-			} else
-				ack = ack && acknowledged;
-
-			a = s;
-			if (e - a <= BB_MAX_LEN) {
-				p[hi] = BB_MAKE(a, e-a, ack);
-				s = e;
-			} else {
-				p[hi] = BB_MAKE(a, BB_MAX_LEN, ack);
-				s = a + BB_MAX_LEN;
+		if (sectors && hi < bb->count) {
+			/* 'hi' points to the first range that starts after 's'.
+			 * Maybe we can merge with the start of that range
+			 */
+			a = BB_OFFSET(p[hi]);
+			e = a + BB_LEN(p[hi]);
+			ack = BB_ACK(p[hi]);
+
+			if (a <= s + sectors) {
+				/* merging is possible */
+				if (e <= s + sectors) {
+					/* full overlap */
+					e = s + sectors;
+					ack = acknowledged;
+				} else
+					ack = ack && acknowledged;
+
+				a = s;
+				if (e - a <= BB_MAX_LEN) {
+					p[hi] = BB_MAKE(a, e-a, ack);
+					s = e;
+				} else {
+					p[hi] = BB_MAKE(a, BB_MAX_LEN, ack);
+					s = a + BB_MAX_LEN;
+				}
+				sectors = e - s;
+				lo = hi;
+				hi++;
+				changed = true;
 			}
-			sectors = e - s;
-			lo = hi;
-			hi++;
-			changed = true;
 		}
-	}
-	if (sectors == 0 && hi < bb->count) {
-		/* we might be able to combine lo and hi */
-		/* Note: 's' is at the end of 'lo' */
-		sector_t loa = BB_OFFSET(p[lo]), hia = BB_OFFSET(p[hi]);
-		sector_t hie = hia + BB_LEN(p[hi]);
-		int newlen = max(s, hie) - loa;
-		int ack = BB_ACK(p[lo]) && BB_ACK(p[hi]);
-
-		if (s >= hia) {
-			while (s >= hie) {
-				/* lo contains hi, just remove hi */
-				memmove(p + hi, p + hi + 1,
-					(bb->count - hi - 1) * 8);
-				bb->count--;
-				if (hi >= bb->count)
-					break;
-				hia = BB_OFFSET(p[hi]);
-				hie = hia + BB_LEN(p[hi]);
-			}
-			if (s >= hia && hi < bb->count) {
-				if (newlen > BB_MAX_LEN) {
-					p[lo] = BB_MAKE(loa, BB_MAX_LEN, ack);
-					p[hi] = BB_MAKE(loa + BB_MAX_LEN,
-							newlen - BB_MAX_LEN,
-							BB_ACK(p[hi]));
-				} else {
-					p[lo] = BB_MAKE(loa, newlen, ack);
+		if (sectors == 0 && hi < bb->count) {
+			/* we might be able to combine lo and hi */
+			/* Note: 's' is at the end of 'lo' */
+			sector_t loa = BB_OFFSET(p[lo]), hia = BB_OFFSET(p[hi]);
+			sector_t hie = hia + BB_LEN(p[hi]);
+			int newlen = max(s, hie) - loa;
+
+			ack = BB_ACK(p[lo]) && BB_ACK(p[hi]);
+			if (s >= hia) {
+				while (s >= hie) {
+					/* lo contains hi, just remove hi */
 					memmove(p + hi, p + hi + 1,
 						(bb->count - hi - 1) * 8);
 					bb->count--;
+					if (hi >= bb->count)
+						break;
+					hia = BB_OFFSET(p[hi]);
+					hie = hia + BB_LEN(p[hi]);
+				}
+				if (s >= hia && hi < bb->count) {
+					if (newlen > BB_MAX_LEN) {
+						p[lo] = BB_MAKE(loa, BB_MAX_LEN, ack);
+						p[hi] = BB_MAKE(loa + BB_MAX_LEN,
+								newlen - BB_MAX_LEN,
+								BB_ACK(p[hi]));
+					} else {
+						p[lo] = BB_MAKE(loa, newlen, ack);
+						memmove(p + hi, p + hi + 1,
+							(bb->count - hi - 1) * 8);
+						bb->count--;
+					}
 				}
+				changed = true;
 			}
-			changed = true;
 		}
 	}
 	while (sectors) {
-- 
2.31.1