Re: [PATCH 10/12] badblocks: return boolen from badblocks_set() and badblocks_clear()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

Just two simple coding styes below.

在 2025/02/21 16:11, Zheng Qixing 写道:
From: Zheng Qixing <zhengqixing@xxxxxxxxxx>

Change the return type of badblocks_set() and badblocks_clear()
from int to bool, indicating success or failure. Specifically:

- _badblocks_set() and _badblocks_clear() functions now return
true for success and false for failure.
- All calls to these functions have been updated to handle the
new boolean return type.
- This change improves code clarity and ensures a more consistent
handling of success and failure states.

Signed-off-by: Zheng Qixing <zhengqixing@xxxxxxxxxx>
---
  block/badblocks.c             | 37 +++++++++++++++++------------------
  drivers/block/null_blk/main.c | 17 ++++++++--------
  drivers/md/md.c               | 35 +++++++++++++++++----------------
  drivers/nvdimm/badrange.c     |  2 +-
  include/linux/badblocks.h     |  6 +++---
  5 files changed, 49 insertions(+), 48 deletions(-)

diff --git a/block/badblocks.c b/block/badblocks.c
index 79d91be468c4..8f057563488a 100644
--- a/block/badblocks.c
+++ b/block/badblocks.c
@@ -836,8 +836,8 @@ static bool try_adjacent_combine(struct badblocks *bb, int prev)
  }
/* Do exact work to set bad block range into the bad block table */
-static int _badblocks_set(struct badblocks *bb, sector_t s, int sectors,
-			  int acknowledged)
+static bool _badblocks_set(struct badblocks *bb, sector_t s, int sectors,
+			   int acknowledged)
  {
  	int len = 0, added = 0;
  	struct badblocks_context bad;
@@ -847,11 +847,11 @@ static int _badblocks_set(struct badblocks *bb, sector_t s, int sectors,
if (bb->shift < 0)
  		/* badblocks are disabled */
-		return 1;
+		return false;
if (sectors == 0)
  		/* Invalid sectors number */
-		return 1;
+		return false;
if (bb->shift) {
  		/* round the start down, and the end up */
@@ -977,7 +977,7 @@ static int _badblocks_set(struct badblocks *bb, sector_t s, int sectors,
write_sequnlock_irqrestore(&bb->lock, flags); - return sectors;
+	return sectors == 0;
  }
/*
@@ -1048,21 +1048,20 @@ static int front_splitting_clear(struct badblocks *bb, int prev,
  }
/* Do the exact work to clear bad block range from the bad block table */
-static int _badblocks_clear(struct badblocks *bb, sector_t s, int sectors)
+static bool _badblocks_clear(struct badblocks *bb, sector_t s, int sectors)
  {
  	struct badblocks_context bad;
  	int prev = -1, hint = -1;
  	int len = 0, cleared = 0;
-	int rv = 0;
  	u64 *p;
if (bb->shift < 0)
  		/* badblocks are disabled */
-		return 1;
+		return false;
if (sectors == 0)
  		/* Invalid sectors number */
-		return 1;
+		return false;
if (bb->shift) {
  		sector_t target;
@@ -1182,9 +1181,9 @@ static int _badblocks_clear(struct badblocks *bb, sector_t s, int sectors)
  	write_sequnlock_irq(&bb->lock);
if (!cleared)
-		rv = 1;
+		return false;
- return rv;
+	return true;
  }
/* Do the exact work to check bad blocks range from the bad block table */
@@ -1338,11 +1337,11 @@ EXPORT_SYMBOL_GPL(badblocks_check);
   * decide how best to handle it.
   *
   * Return:
- *  0: success
- *  other: failed to set badblocks (out of space)
+ *  true: success
+ *  false: failed to set badblocks (out of space)
   */
-int badblocks_set(struct badblocks *bb, sector_t s, int sectors,
-			int acknowledged)
+bool badblocks_set(struct badblocks *bb, sector_t s, int sectors,
+		   int acknowledged)
  {
  	return _badblocks_set(bb, s, sectors, acknowledged);
  }
@@ -1359,10 +1358,10 @@ EXPORT_SYMBOL_GPL(badblocks_set);
   * drop the remove request.
   *
   * Return:
- *  0: success
- *  1: failed to clear badblocks
+ *  true: success
+ *  false: failed to clear badblocks
   */
-int badblocks_clear(struct badblocks *bb, sector_t s, int sectors)
+bool badblocks_clear(struct badblocks *bb, sector_t s, int sectors)
  {
  	return _badblocks_clear(bb, s, sectors);
  }
@@ -1484,7 +1483,7 @@ ssize_t badblocks_store(struct badblocks *bb, const char *page, size_t len,
  		return -EINVAL;
  	}
- if (badblocks_set(bb, sector, length, !unack))
+	if (!badblocks_set(bb, sector, length, !unack))
  		return -ENOSPC;
  	else

Since we're here, can you also remove the else branch as well?

  		return len;
diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
index d94ef37480bd..623db72ad66b 100644
--- a/drivers/block/null_blk/main.c
+++ b/drivers/block/null_blk/main.c
@@ -559,14 +559,15 @@ static ssize_t nullb_device_badblocks_store(struct config_item *item,
  		goto out;
  	/* enable badblocks */
  	cmpxchg(&t_dev->badblocks.shift, -1, 0);
-	if (buf[0] == '+')
-		ret = badblocks_set(&t_dev->badblocks, start,
-			end - start + 1, 1);
-	else
-		ret = badblocks_clear(&t_dev->badblocks, start,
-			end - start + 1);
-	if (ret == 0)
-		ret = count;
+	if (buf[0] == '+') {
+		if (badblocks_set(&t_dev->badblocks, start,
+				  end - start + 1, 1))
+			ret = count;
+	} else {
+		if (badblocks_clear(&t_dev->badblocks, start,
+				    end - start + 1))

else if ()

Reviewed-by: Yu Kuai <yukuai3@xxxxxxxxxx>
Thanks
+			ret = count;
+	}
  out:
  	kfree(orig);
  	return ret;
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 30b3dbbce2d2..49d826e475cb 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -1748,7 +1748,7 @@ static int super_1_load(struct md_rdev *rdev, struct md_rdev *refdev, int minor_
  			count <<= sb->bblog_shift;
  			if (bb + 1 == 0)
  				break;
-			if (badblocks_set(&rdev->badblocks, sector, count, 1))
+			if (!badblocks_set(&rdev->badblocks, sector, count, 1))
  				return -EINVAL;
  		}
  	} else if (sb->bblog_offset != 0)
@@ -9846,7 +9846,6 @@ int rdev_set_badblocks(struct md_rdev *rdev, sector_t s, int sectors,
  		       int is_new)
  {
  	struct mddev *mddev = rdev->mddev;
-	int rv;
/*
  	 * Recording new badblocks for faulty rdev will force unnecessary
@@ -9862,33 +9861,35 @@ int rdev_set_badblocks(struct md_rdev *rdev, sector_t s, int sectors,
  		s += rdev->new_data_offset;
  	else
  		s += rdev->data_offset;
-	rv = badblocks_set(&rdev->badblocks, s, sectors, 0);
-	if (rv == 0) {
-		/* Make sure they get written out promptly */
-		if (test_bit(ExternalBbl, &rdev->flags))
-			sysfs_notify_dirent_safe(rdev->sysfs_unack_badblocks);
-		sysfs_notify_dirent_safe(rdev->sysfs_state);
-		set_mask_bits(&mddev->sb_flags, 0,
-			      BIT(MD_SB_CHANGE_CLEAN) | BIT(MD_SB_CHANGE_PENDING));
-		md_wakeup_thread(rdev->mddev->thread);
-		return 1;
-	} else
+
+	if (!badblocks_set(&rdev->badblocks, s, sectors, 0))
  		return 0;
+
+	/* Make sure they get written out promptly */
+	if (test_bit(ExternalBbl, &rdev->flags))
+		sysfs_notify_dirent_safe(rdev->sysfs_unack_badblocks);
+	sysfs_notify_dirent_safe(rdev->sysfs_state);
+	set_mask_bits(&mddev->sb_flags, 0,
+		      BIT(MD_SB_CHANGE_CLEAN) | BIT(MD_SB_CHANGE_PENDING));
+	md_wakeup_thread(rdev->mddev->thread);
+	return 1;
  }
  EXPORT_SYMBOL_GPL(rdev_set_badblocks);
int rdev_clear_badblocks(struct md_rdev *rdev, sector_t s, int sectors,
  			 int is_new)
  {
-	int rv;
  	if (is_new)
  		s += rdev->new_data_offset;
  	else
  		s += rdev->data_offset;
-	rv = badblocks_clear(&rdev->badblocks, s, sectors);
-	if ((rv == 0) && test_bit(ExternalBbl, &rdev->flags))
+
+	if (!badblocks_clear(&rdev->badblocks, s, sectors))
+		return 0;
+
+	if (test_bit(ExternalBbl, &rdev->flags))
  		sysfs_notify_dirent_safe(rdev->sysfs_badblocks);
-	return rv;
+	return 1;
  }
  EXPORT_SYMBOL_GPL(rdev_clear_badblocks);
diff --git a/drivers/nvdimm/badrange.c b/drivers/nvdimm/badrange.c
index a002ea6fdd84..ee478ccde7c6 100644
--- a/drivers/nvdimm/badrange.c
+++ b/drivers/nvdimm/badrange.c
@@ -167,7 +167,7 @@ static void set_badblock(struct badblocks *bb, sector_t s, int num)
  	dev_dbg(bb->dev, "Found a bad range (0x%llx, 0x%llx)\n",
  			(u64) s * 512, (u64) num * 512);
  	/* this isn't an error as the hardware will still throw an exception */
-	if (badblocks_set(bb, s, num, 1))
+	if (!badblocks_set(bb, s, num, 1))
  		dev_info_once(bb->dev, "%s: failed for sector %llx\n",
  				__func__, (u64) s);
  }
diff --git a/include/linux/badblocks.h b/include/linux/badblocks.h
index 670f2dae692f..8764bed9ff16 100644
--- a/include/linux/badblocks.h
+++ b/include/linux/badblocks.h
@@ -50,9 +50,9 @@ struct badblocks_context {
int badblocks_check(struct badblocks *bb, sector_t s, int sectors,
  		   sector_t *first_bad, int *bad_sectors);
-int badblocks_set(struct badblocks *bb, sector_t s, int sectors,
-			int acknowledged);
-int badblocks_clear(struct badblocks *bb, sector_t s, int sectors);
+bool badblocks_set(struct badblocks *bb, sector_t s, int sectors,
+		   int acknowledged);
+bool badblocks_clear(struct badblocks *bb, sector_t s, int sectors);
  void ack_all_badblocks(struct badblocks *bb);
  ssize_t badblocks_show(struct badblocks *bb, char *page, int unack);
  ssize_t badblocks_store(struct badblocks *bb, const char *page, size_t len,






[Index of Archives]     [Linux RAID]     [Linux SCSI]     [Linux ATA RAID]     [IDE]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Device Mapper]

  Powered by Linux