Re: [PATCH v3 1/4] block/badblocks: change some members of badblocks to bool

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

 





在 2023/6/21 22:02, Ashok Raj 写道:
On Thu, Jun 22, 2023 at 01:20:49AM +0800, linan666@xxxxxxxxxxxxxxx wrote:
From: Li Nan <linan122@xxxxxxxxxx>

"changed" and "unacked_exist" are used as boolean type. Change the type
of them to bool. And reorder fields to reduce memory hole.

minor nit: If you use a .gitorderfile to list .h before .c it will help review them in
order.


I will config my git.

I don't know if its even worth doing this manual compaction unless you are
storing the entire struct in some flash or its in a sensitive cache
thrashing structure.


Yeah, it is worthless to manual compaction.

bool is useful that it makes the code easier to read and can eliminate some
class of bugs that you would otherwise use !! operator.


No functional changed intended.

Signed-off-by: Li Nan <linan122@xxxxxxxxxx>
---
  block/badblocks.c         | 14 +++++++-------
  include/linux/badblocks.h | 10 +++++-----
  2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/block/badblocks.c b/block/badblocks.c
index 3afb550c0f7b..1b4caa42c5f1 100644
--- a/block/badblocks.c
+++ b/block/badblocks.c
@@ -141,7 +141,7 @@ static void badblocks_update_acked(struct badblocks *bb)
  	}
if (!unacked)
-		bb->unacked_exist = 0;
+		bb->unacked_exist = false;
  }
/**
@@ -302,9 +302,9 @@ int badblocks_set(struct badblocks *bb, sector_t s, int sectors,
  		}
  	}
- bb->changed = 1;
+	bb->changed = true;
  	if (!acknowledged)
-		bb->unacked_exist = 1;
+		bb->unacked_exist = true;
  	else
  		badblocks_update_acked(bb);
  	write_sequnlock_irqrestore(&bb->lock, flags);
@@ -414,7 +414,7 @@ int badblocks_clear(struct badblocks *bb, sector_t s, int sectors)
  	}
badblocks_update_acked(bb);
-	bb->changed = 1;
+	bb->changed = true;
  out:
  	write_sequnlock_irq(&bb->lock);
  	return rv;
@@ -435,7 +435,7 @@ void ack_all_badblocks(struct badblocks *bb)
  		return;
  	write_seqlock_irq(&bb->lock);
- if (bb->changed == 0 && bb->unacked_exist) {
+	if (bb->changed == false && bb->unacked_exist) {

	if (!bb->changed && bb->unacked_exist)

I will change it in next version.



  		u64 *p = bb->page;
  		int i;
@@ -447,7 +447,7 @@ void ack_all_badblocks(struct badblocks *bb)
  				p[i] = BB_MAKE(start, len, 1);
  			}
  		}
-		bb->unacked_exist = 0;
+		bb->unacked_exist = false;
  	}
  	write_sequnlock_irq(&bb->lock);
  }
@@ -493,7 +493,7 @@ ssize_t badblocks_show(struct badblocks *bb, char *page, int unack)
  				length << bb->shift);
  	}
  	if (unack && len == 0)
-		bb->unacked_exist = 0;
+		bb->unacked_exist = false;
if (read_seqretry(&bb->lock, seq))
  		goto retry;
diff --git a/include/linux/badblocks.h b/include/linux/badblocks.h
index 2426276b9bd3..c2723f97d22d 100644
--- a/include/linux/badblocks.h
+++ b/include/linux/badblocks.h
@@ -27,15 +27,15 @@
  struct badblocks {
  	struct device *dev;	/* set by devm_init_badblocks */
  	int count;		/* count of bad blocks */
-	int unacked_exist;	/* there probably are unacknowledged
-				 * bad blocks.  This is only cleared
-				 * when a read discovers none
-				 */
  	int shift;		/* shift from sectors to block size
  				 * a -ve shift means badblocks are
  				 * disabled.*/
+	bool unacked_exist;	/* there probably are unacknowledged
+				 * bad blocks.  This is only cleared
+				 * when a read discovers none

read of what?

"... when a read of unacknowledged bad blocks discovers none"

Would this be better?

Thank for your suggestion.


+				 */
+	bool changed;
  	u64 *page;		/* badblock list */
-	int changed;
  	seqlock_t lock;
  	sector_t sector;
  	sector_t size;		/* in sectors */
--
2.39.2


.

--
Thanks,
Nan




[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