All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCHES][RFC] packing struct block_device flags
@ 2024-04-28  5:12 Al Viro
  2024-04-28  5:14 ` [PATCH 1/8] Use bdev_is_paritition() instead of open-coding it Al Viro
                   ` (8 more replies)
  0 siblings, 9 replies; 24+ messages in thread
From: Al Viro @ 2024-04-28  5:12 UTC (permalink / raw)
  To: linux-fsdevel
  Cc: Yu Kuai, linux-block, Christian Brauner, Christoph Hellwig, Jens Axboe

	We have several bool members in struct block_device.
It would be nice to pack that stuff, preferably without
blowing the cacheline boundaries.

	That had been suggested a while ago, and initial
implementation by Yu Kuai <yukuai1@huaweicloud.com> ran into
objections re atomicity, along with the obvious suggestion to
use unsigned long and test_bit/set_bit/clear_bit for access.
Unfortunately, that *does* blow the cacheline boundaries.

	However, it's not hard to do that without bitops;
we have an 8-bit assign-once partition number nearby, and
folding it into a 32-bit member leaves us up to 24 bits for
flags.  Using cmpxchg for setting/clearing flags is not
hard, and 32bit cmpxchg is supported everywhere.

	Series below does that conversion.  Please, review.
Individual patches in followups, the branch is in
git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs.git #work.bd_flags

Shortlog:
Al Viro (8):
      Use bdev_is_paritition() instead of open-coding it
      wrapper for access to ->bd_partno
      bdev: infrastructure for flags
      bdev: move ->bd_read_only to ->__bd_flags
      bdev: move ->bd_write_holder into ->__bd_flags
      bdev: move ->bd_has_subit_bio to ->__bd_flags
      bdev: move ->bd_ro_warned to ->__bd_flags
      bdev: move ->bd_make_it_fail to ->__bd_flags

Diffstat:
 block/bdev.c              | 17 ++++++++---------
 block/blk-core.c          | 17 ++++++++++-------
 block/blk-mq.c            |  2 +-
 block/early-lookup.c      |  2 +-
 block/genhd.c             | 15 ++++++++++-----
 block/ioctl.c             |  5 ++++-
 block/partitions/core.c   | 12 ++++++------
 include/linux/blk_types.h | 19 +++++++++++--------
 include/linux/blkdev.h    | 42 +++++++++++++++++++++++++++++++++++++++---
 include/linux/part_stat.h |  2 +-
 lib/vsprintf.c            |  4 ++--
 11 files changed, 93 insertions(+), 44 deletions(-)

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 1/8] Use bdev_is_paritition() instead of open-coding it
  2024-04-28  5:12 [PATCHES][RFC] packing struct block_device flags Al Viro
@ 2024-04-28  5:14 ` Al Viro
  2024-04-29  5:19   ` Christoph Hellwig
  2024-04-28  5:15 ` [PATCH 2/8] wrapper for access to ->bd_partno Al Viro
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 24+ messages in thread
From: Al Viro @ 2024-04-28  5:14 UTC (permalink / raw)
  To: linux-fsdevel
  Cc: Yu Kuai, linux-block, Christian Brauner, Christoph Hellwig, Jens Axboe

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 block/blk-core.c          | 5 +++--
 block/blk-mq.c            | 2 +-
 include/linux/part_stat.h | 2 +-
 lib/vsprintf.c            | 2 +-
 4 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index a16b5abdbbf5..20322abc6082 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -759,7 +759,8 @@ void submit_bio_noacct(struct bio *bio)
 	if (!bio_flagged(bio, BIO_REMAPPED)) {
 		if (unlikely(bio_check_eod(bio)))
 			goto end_io;
-		if (bdev->bd_partno && unlikely(blk_partition_remap(bio)))
+		if (bdev_is_partition(bdev) &&
+		    unlikely(blk_partition_remap(bio)))
 			goto end_io;
 	}
 
@@ -989,7 +990,7 @@ void update_io_ticks(struct block_device *part, unsigned long now, bool end)
 		if (likely(try_cmpxchg(&part->bd_stamp, &stamp, now)))
 			__part_stat_add(part, io_ticks, end ? now - stamp : 1);
 	}
-	if (part->bd_partno) {
+	if (bdev_is_partition(part)) {
 		part = bdev_whole(part);
 		goto again;
 	}
diff --git a/block/blk-mq.c b/block/blk-mq.c
index 32afb87efbd0..43bb8f50a07c 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -92,7 +92,7 @@ static bool blk_mq_check_inflight(struct request *rq, void *priv)
 	struct mq_inflight *mi = priv;
 
 	if (rq->part && blk_do_io_stat(rq) &&
-	    (!mi->part->bd_partno || rq->part == mi->part) &&
+	    (!bdev_is_partition(mi->part) || rq->part == mi->part) &&
 	    blk_mq_rq_state(rq) == MQ_RQ_IN_FLIGHT)
 		mi->inflight[rq_data_dir(rq)]++;
 
diff --git a/include/linux/part_stat.h b/include/linux/part_stat.h
index abeba356bc3f..ac8c44dd8237 100644
--- a/include/linux/part_stat.h
+++ b/include/linux/part_stat.h
@@ -59,7 +59,7 @@ static inline void part_stat_set_all(struct block_device *part, int value)
 
 #define part_stat_add(part, field, addnd)	do {			\
 	__part_stat_add((part), field, addnd);				\
-	if ((part)->bd_partno)						\
+	if (bdev_is_partition(part))					\
 		__part_stat_add(bdev_whole(part), field, addnd);	\
 } while (0)
 
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 552738f14275..3f9f1b959ef0 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -966,7 +966,7 @@ char *bdev_name(char *buf, char *end, struct block_device *bdev,
 
 	hd = bdev->bd_disk;
 	buf = string(buf, end, hd->disk_name, spec);
-	if (bdev->bd_partno) {
+	if (bdev_is_partition(bdev)) {
 		if (isdigit(hd->disk_name[strlen(hd->disk_name)-1])) {
 			if (buf < end)
 				*buf = 'p';
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 2/8] wrapper for access to ->bd_partno
  2024-04-28  5:12 [PATCHES][RFC] packing struct block_device flags Al Viro
  2024-04-28  5:14 ` [PATCH 1/8] Use bdev_is_paritition() instead of open-coding it Al Viro
@ 2024-04-28  5:15 ` Al Viro
  2024-04-28  5:16 ` [PATCH 3/8] bdev: infrastructure for flags Al Viro
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 24+ messages in thread
From: Al Viro @ 2024-04-28  5:15 UTC (permalink / raw)
  To: linux-fsdevel
  Cc: Yu Kuai, linux-block, Christian Brauner, Christoph Hellwig, Jens Axboe

On the next step it's going to get folded into a field where flags will go.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 block/early-lookup.c    |  2 +-
 block/partitions/core.c | 12 ++++++------
 include/linux/blkdev.h  |  7 ++++++-
 lib/vsprintf.c          |  2 +-
 4 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/block/early-lookup.c b/block/early-lookup.c
index 3effbd0d35e9..3fb57f7d2b12 100644
--- a/block/early-lookup.c
+++ b/block/early-lookup.c
@@ -78,7 +78,7 @@ static int __init devt_from_partuuid(const char *uuid_str, dev_t *devt)
 		 * to the partition number found by UUID.
 		 */
 		*devt = part_devt(dev_to_disk(dev),
-				  dev_to_bdev(dev)->bd_partno + offset);
+				  bdev_partno(dev_to_bdev(dev)) + offset);
 	} else {
 		*devt = dev->devt;
 	}
diff --git a/block/partitions/core.c b/block/partitions/core.c
index b11e88c82c8c..edd5309dc4ba 100644
--- a/block/partitions/core.c
+++ b/block/partitions/core.c
@@ -173,7 +173,7 @@ static struct parsed_partitions *check_partition(struct gendisk *hd)
 static ssize_t part_partition_show(struct device *dev,
 				   struct device_attribute *attr, char *buf)
 {
-	return sprintf(buf, "%d\n", dev_to_bdev(dev)->bd_partno);
+	return sprintf(buf, "%d\n", bdev_partno(dev_to_bdev(dev)));
 }
 
 static ssize_t part_start_show(struct device *dev,
@@ -250,7 +250,7 @@ static int part_uevent(const struct device *dev, struct kobj_uevent_env *env)
 {
 	const struct block_device *part = dev_to_bdev(dev);
 
-	add_uevent_var(env, "PARTN=%u", part->bd_partno);
+	add_uevent_var(env, "PARTN=%u", bdev_partno(part));
 	if (part->bd_meta_info && part->bd_meta_info->volname[0])
 		add_uevent_var(env, "PARTNAME=%s", part->bd_meta_info->volname);
 	return 0;
@@ -267,7 +267,7 @@ void drop_partition(struct block_device *part)
 {
 	lockdep_assert_held(&part->bd_disk->open_mutex);
 
-	xa_erase(&part->bd_disk->part_tbl, part->bd_partno);
+	xa_erase(&part->bd_disk->part_tbl, bdev_partno(part));
 	kobject_put(part->bd_holder_dir);
 
 	device_del(&part->bd_device);
@@ -338,8 +338,8 @@ static struct block_device *add_partition(struct gendisk *disk, int partno,
 	pdev->parent = ddev;
 
 	/* in consecutive minor range? */
-	if (bdev->bd_partno < disk->minors) {
-		devt = MKDEV(disk->major, disk->first_minor + bdev->bd_partno);
+	if (bdev_partno(bdev) < disk->minors) {
+		devt = MKDEV(disk->major, disk->first_minor + bdev_partno(bdev));
 	} else {
 		err = blk_alloc_ext_minor();
 		if (err < 0)
@@ -404,7 +404,7 @@ static bool partition_overlaps(struct gendisk *disk, sector_t start,
 
 	rcu_read_lock();
 	xa_for_each_start(&disk->part_tbl, idx, part, 1) {
-		if (part->bd_partno != skip_partno &&
+		if (bdev_partno(part) != skip_partno &&
 		    start < part->bd_start_sect + bdev_nr_sectors(part) &&
 		    start + length > part->bd_start_sect) {
 			overlap = true;
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index c3e8f7cf96be..32549d675955 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -720,6 +720,11 @@ void invalidate_disk(struct gendisk *disk);
 void set_disk_ro(struct gendisk *disk, bool read_only);
 void disk_uevent(struct gendisk *disk, enum kobject_action action);
 
+static inline u8 bdev_partno(const struct block_device *bdev)
+{
+	return bdev->bd_partno;
+}
+
 static inline int get_disk_ro(struct gendisk *disk)
 {
 	return disk->part0->bd_read_only ||
@@ -1095,7 +1100,7 @@ static inline int sb_issue_zeroout(struct super_block *sb, sector_t block,
 
 static inline bool bdev_is_partition(struct block_device *bdev)
 {
-	return bdev->bd_partno;
+	return bdev_partno(bdev) != 0;
 }
 
 enum blk_default_limits {
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 3f9f1b959ef0..cdd4e2314bfc 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -972,7 +972,7 @@ char *bdev_name(char *buf, char *end, struct block_device *bdev,
 				*buf = 'p';
 			buf++;
 		}
-		buf = number(buf, end, bdev->bd_partno, spec);
+		buf = number(buf, end, bdev_partno(bdev), spec);
 	}
 	return buf;
 }
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 3/8] bdev: infrastructure for flags
  2024-04-28  5:12 [PATCHES][RFC] packing struct block_device flags Al Viro
  2024-04-28  5:14 ` [PATCH 1/8] Use bdev_is_paritition() instead of open-coding it Al Viro
  2024-04-28  5:15 ` [PATCH 2/8] wrapper for access to ->bd_partno Al Viro
@ 2024-04-28  5:16 ` Al Viro
  2024-04-28  5:17 ` [PATCH 4/8] bdev: move ->bd_read_only to ->__bd_flags Al Viro
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 24+ messages in thread
From: Al Viro @ 2024-04-28  5:16 UTC (permalink / raw)
  To: linux-fsdevel
  Cc: Yu Kuai, linux-block, Christian Brauner, Christoph Hellwig, Jens Axboe

Replace bd_partno with a 32bit field (__bd_flags).  The lower 8 bits
contain the partition number, the upper 24 are for flags.

Helpers: bdev_{test,set,clear}_flag(bdev, flag), where flag numbers
are zero-based.  Use cmpxchg() to set/clear - all architectures support
it for u32.

NOTE: this commit does not actuall move any flags over there - they
are still bool fields.  As the result, it shifts the fields wrt
cacheline boundaries; that's going to be restored once the first
3 flags are dealt with.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 block/bdev.c              |  2 +-
 include/linux/blk_types.h |  2 +-
 include/linux/blkdev.h    | 33 ++++++++++++++++++++++++++++++++-
 3 files changed, 34 insertions(+), 3 deletions(-)

diff --git a/block/bdev.c b/block/bdev.c
index 7a5f611c3d2e..0c55f7abaac3 100644
--- a/block/bdev.c
+++ b/block/bdev.c
@@ -411,7 +411,7 @@ struct block_device *bdev_alloc(struct gendisk *disk, u8 partno)
 	mutex_init(&bdev->bd_fsfreeze_mutex);
 	spin_lock_init(&bdev->bd_size_lock);
 	mutex_init(&bdev->bd_holder_lock);
-	bdev->bd_partno = partno;
+	bdev->__bd_flags = partno;
 	bdev->bd_inode = inode;
 	bdev->bd_queue = disk->queue;
 	if (partno)
diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
index cb1526ec44b5..274f7eb4138f 100644
--- a/include/linux/blk_types.h
+++ b/include/linux/blk_types.h
@@ -45,8 +45,8 @@ struct block_device {
 	struct request_queue *	bd_queue;
 	struct disk_stats __percpu *bd_stats;
 	unsigned long		bd_stamp;
+	u32			__bd_flags;	// partition number + flags
 	bool			bd_read_only;	/* read-only policy */
-	u8			bd_partno;
 	bool			bd_write_holder;
 	bool			bd_has_submit_bio;
 	dev_t			bd_dev;
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 32549d675955..b9dbfb811533 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -722,7 +722,38 @@ void disk_uevent(struct gendisk *disk, enum kobject_action action);
 
 static inline u8 bdev_partno(const struct block_device *bdev)
 {
-	return bdev->bd_partno;
+	return bdev->__bd_flags & 0xff;
+}
+
+static inline bool bdev_test_flag(const struct block_device *bdev, int flag)
+{
+	return bdev->__bd_flags & (1 << (flag + 8));
+}
+
+static inline void bdev_set_flag(struct block_device *bdev, int flag)
+{
+	u32 v = bdev->__bd_flags;
+
+	for (;;) {
+		u32 w = cmpxchg(&bdev->__bd_flags, v, v | (1 << (flag + 8)));
+
+		if (v == w)
+			return;
+		v = w;
+	}
+}
+
+static inline void bdev_clear_flag(struct block_device *bdev, int flag)
+{
+	u32 v = bdev->__bd_flags;
+
+	for (;;) {
+		u32 w = cmpxchg(&bdev->__bd_flags, v, v & ~(1 << (flag + 8)));
+
+		if (v == w)
+			return;
+		v = w;
+	}
 }
 
 static inline int get_disk_ro(struct gendisk *disk)
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 4/8] bdev: move ->bd_read_only to ->__bd_flags
  2024-04-28  5:12 [PATCHES][RFC] packing struct block_device flags Al Viro
                   ` (2 preceding siblings ...)
  2024-04-28  5:16 ` [PATCH 3/8] bdev: infrastructure for flags Al Viro
@ 2024-04-28  5:17 ` Al Viro
  2024-04-28  5:18 ` [PATCH 5/8] bdev: move ->bd_write_holder into ->__bd_flags Al Viro
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 24+ messages in thread
From: Al Viro @ 2024-04-28  5:17 UTC (permalink / raw)
  To: linux-fsdevel
  Cc: Yu Kuai, linux-block, Christian Brauner, Christoph Hellwig, Jens Axboe

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 block/ioctl.c             | 5 ++++-
 include/linux/blk_types.h | 5 ++++-
 include/linux/blkdev.h    | 4 ++--
 3 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/block/ioctl.c b/block/ioctl.c
index 0c76137adcaa..be173e4ff43d 100644
--- a/block/ioctl.c
+++ b/block/ioctl.c
@@ -402,7 +402,10 @@ static int blkdev_roset(struct block_device *bdev, unsigned cmd,
 		if (ret)
 			return ret;
 	}
-	bdev->bd_read_only = n;
+	if (n)
+		bdev_set_flag(bdev, BD_READ_ONLY);
+	else
+		bdev_clear_flag(bdev, BD_READ_ONLY);
 	return 0;
 }
 
diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
index 274f7eb4138f..1aa41b651614 100644
--- a/include/linux/blk_types.h
+++ b/include/linux/blk_types.h
@@ -46,7 +46,6 @@ struct block_device {
 	struct disk_stats __percpu *bd_stats;
 	unsigned long		bd_stamp;
 	u32			__bd_flags;	// partition number + flags
-	bool			bd_read_only;	/* read-only policy */
 	bool			bd_write_holder;
 	bool			bd_has_submit_bio;
 	dev_t			bd_dev;
@@ -86,6 +85,10 @@ struct block_device {
 #define bdev_kobj(_bdev) \
 	(&((_bdev)->bd_device.kobj))
 
+enum {
+	BD_READ_ONLY,		// read-only policy
+};
+
 /*
  * Block error status values.  See block/blk-core:blk_errors for the details.
  * Alpha cannot write a byte atomically, so we need to use 32-bit value.
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index b9dbfb811533..d556cec9224b 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -758,13 +758,13 @@ static inline void bdev_clear_flag(struct block_device *bdev, int flag)
 
 static inline int get_disk_ro(struct gendisk *disk)
 {
-	return disk->part0->bd_read_only ||
+	return bdev_test_flag(disk->part0, BD_READ_ONLY) ||
 		test_bit(GD_READ_ONLY, &disk->state);
 }
 
 static inline int bdev_read_only(struct block_device *bdev)
 {
-	return bdev->bd_read_only || get_disk_ro(bdev->bd_disk);
+	return bdev_test_flag(bdev, BD_READ_ONLY) || get_disk_ro(bdev->bd_disk);
 }
 
 bool set_capacity_and_notify(struct gendisk *disk, sector_t size);
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 5/8] bdev: move ->bd_write_holder into ->__bd_flags
  2024-04-28  5:12 [PATCHES][RFC] packing struct block_device flags Al Viro
                   ` (3 preceding siblings ...)
  2024-04-28  5:17 ` [PATCH 4/8] bdev: move ->bd_read_only to ->__bd_flags Al Viro
@ 2024-04-28  5:18 ` Al Viro
  2024-04-28  5:19 ` [PATCH 6/8] bdev: move ->bd_has_subit_bio to ->__bd_flags Al Viro
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 24+ messages in thread
From: Al Viro @ 2024-04-28  5:18 UTC (permalink / raw)
  To: linux-fsdevel
  Cc: Yu Kuai, linux-block, Christian Brauner, Christoph Hellwig, Jens Axboe

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 block/bdev.c              | 9 +++++----
 include/linux/blk_types.h | 2 +-
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/block/bdev.c b/block/bdev.c
index 0c55f7abaac3..24c1dd6de8a9 100644
--- a/block/bdev.c
+++ b/block/bdev.c
@@ -624,7 +624,7 @@ static void bd_end_claim(struct block_device *bdev, void *holder)
 		bdev->bd_holder = NULL;
 		bdev->bd_holder_ops = NULL;
 		mutex_unlock(&bdev->bd_holder_lock);
-		if (bdev->bd_write_holder)
+		if (bdev_test_flag(bdev, BD_WRITE_HOLDER))
 			unblock = true;
 	}
 	if (!whole->bd_holders)
@@ -640,7 +640,7 @@ static void bd_end_claim(struct block_device *bdev, void *holder)
 	 */
 	if (unblock) {
 		disk_unblock_events(bdev->bd_disk);
-		bdev->bd_write_holder = false;
+		bdev_clear_flag(bdev, BD_WRITE_HOLDER);
 	}
 }
 
@@ -892,9 +892,10 @@ int bdev_open(struct block_device *bdev, blk_mode_t mode, void *holder,
 		 * writeable reference is too fragile given the way @mode is
 		 * used in blkdev_get/put().
 		 */
-		if ((mode & BLK_OPEN_WRITE) && !bdev->bd_write_holder &&
+		if ((mode & BLK_OPEN_WRITE) &&
+		    !bdev_test_flag(bdev, BD_WRITE_HOLDER) &&
 		    (disk->event_flags & DISK_EVENT_FLAG_BLOCK_ON_EXCL_WRITE)) {
-			bdev->bd_write_holder = true;
+			bdev_set_flag(bdev, BD_WRITE_HOLDER);
 			unblock_events = false;
 		}
 	}
diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
index 1aa41b651614..8a336053b5fa 100644
--- a/include/linux/blk_types.h
+++ b/include/linux/blk_types.h
@@ -46,7 +46,6 @@ struct block_device {
 	struct disk_stats __percpu *bd_stats;
 	unsigned long		bd_stamp;
 	u32			__bd_flags;	// partition number + flags
-	bool			bd_write_holder;
 	bool			bd_has_submit_bio;
 	dev_t			bd_dev;
 	struct inode		*bd_inode;	/* will die */
@@ -87,6 +86,7 @@ struct block_device {
 
 enum {
 	BD_READ_ONLY,		// read-only policy
+	BD_WRITE_HOLDER,
 };
 
 /*
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 6/8] bdev: move ->bd_has_subit_bio to ->__bd_flags
  2024-04-28  5:12 [PATCHES][RFC] packing struct block_device flags Al Viro
                   ` (4 preceding siblings ...)
  2024-04-28  5:18 ` [PATCH 5/8] bdev: move ->bd_write_holder into ->__bd_flags Al Viro
@ 2024-04-28  5:19 ` Al Viro
  2024-04-28  5:19 ` [PATCH 7/8] bdev: move ->bd_ro_warned " Al Viro
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 24+ messages in thread
From: Al Viro @ 2024-04-28  5:19 UTC (permalink / raw)
  To: linux-fsdevel
  Cc: Yu Kuai, linux-block, Christian Brauner, Christoph Hellwig, Jens Axboe

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 block/bdev.c              | 6 ++----
 block/blk-core.c          | 4 ++--
 block/genhd.c             | 3 ++-
 include/linux/blk_types.h | 2 +-
 4 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/block/bdev.c b/block/bdev.c
index 24c1dd6de8a9..9aa23620fe92 100644
--- a/block/bdev.c
+++ b/block/bdev.c
@@ -414,10 +414,8 @@ struct block_device *bdev_alloc(struct gendisk *disk, u8 partno)
 	bdev->__bd_flags = partno;
 	bdev->bd_inode = inode;
 	bdev->bd_queue = disk->queue;
-	if (partno)
-		bdev->bd_has_submit_bio = disk->part0->bd_has_submit_bio;
-	else
-		bdev->bd_has_submit_bio = false;
+	if (partno && bdev_test_flag(disk->part0, BD_HAS_SUBMIT_BIO))
+		bdev_set_flag(bdev, BD_HAS_SUBMIT_BIO);
 	bdev->bd_stats = alloc_percpu(struct disk_stats);
 	if (!bdev->bd_stats) {
 		iput(inode);
diff --git a/block/blk-core.c b/block/blk-core.c
index 20322abc6082..f61460b65408 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -615,7 +615,7 @@ static void __submit_bio(struct bio *bio)
 	if (unlikely(!blk_crypto_bio_prep(&bio)))
 		return;
 
-	if (!bio->bi_bdev->bd_has_submit_bio) {
+	if (!bdev_test_flag(bio->bi_bdev, BD_HAS_SUBMIT_BIO)) {
 		blk_mq_submit_bio(bio);
 	} else if (likely(bio_queue_enter(bio) == 0)) {
 		struct gendisk *disk = bio->bi_bdev->bd_disk;
@@ -723,7 +723,7 @@ void submit_bio_noacct_nocheck(struct bio *bio)
 	 */
 	if (current->bio_list)
 		bio_list_add(&current->bio_list[0], bio);
-	else if (!bio->bi_bdev->bd_has_submit_bio)
+	else if (!bdev_test_flag(bio->bi_bdev, BD_HAS_SUBMIT_BIO))
 		__submit_bio_noacct_mq(bio);
 	else
 		__submit_bio_noacct(bio);
diff --git a/block/genhd.c b/block/genhd.c
index bb29a68e1d67..19cd1a31fa80 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -413,7 +413,8 @@ int __must_check device_add_disk(struct device *parent, struct gendisk *disk,
 	elevator_init_mq(disk->queue);
 
 	/* Mark bdev as having a submit_bio, if needed */
-	disk->part0->bd_has_submit_bio = disk->fops->submit_bio != NULL;
+	if (disk->fops->submit_bio)
+		bdev_set_flag(disk->part0, BD_HAS_SUBMIT_BIO);
 
 	/*
 	 * If the driver provides an explicit major number it also must provide
diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
index 8a336053b5fa..c8f5364b24f1 100644
--- a/include/linux/blk_types.h
+++ b/include/linux/blk_types.h
@@ -46,7 +46,6 @@ struct block_device {
 	struct disk_stats __percpu *bd_stats;
 	unsigned long		bd_stamp;
 	u32			__bd_flags;	// partition number + flags
-	bool			bd_has_submit_bio;
 	dev_t			bd_dev;
 	struct inode		*bd_inode;	/* will die */
 
@@ -87,6 +86,7 @@ struct block_device {
 enum {
 	BD_READ_ONLY,		// read-only policy
 	BD_WRITE_HOLDER,
+	BD_HAS_SUBMIT_BIO,
 };
 
 /*
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 7/8] bdev: move ->bd_ro_warned to ->__bd_flags
  2024-04-28  5:12 [PATCHES][RFC] packing struct block_device flags Al Viro
                   ` (5 preceding siblings ...)
  2024-04-28  5:19 ` [PATCH 6/8] bdev: move ->bd_has_subit_bio to ->__bd_flags Al Viro
@ 2024-04-28  5:19 ` Al Viro
  2024-04-28  5:21 ` [PATCH 8/8] bdev: move ->bd_make_it_fail " Al Viro
  2024-04-29  5:23 ` [PATCHES][RFC] packing struct block_device flags Christoph Hellwig
  8 siblings, 0 replies; 24+ messages in thread
From: Al Viro @ 2024-04-28  5:19 UTC (permalink / raw)
  To: linux-fsdevel
  Cc: Yu Kuai, linux-block, Christian Brauner, Christoph Hellwig, Jens Axboe

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 block/blk-core.c          | 5 +++--
 include/linux/blk_types.h | 2 +-
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index f61460b65408..1be49be9fac4 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -514,10 +514,11 @@ static inline void bio_check_ro(struct bio *bio)
 		if (op_is_flush(bio->bi_opf) && !bio_sectors(bio))
 			return;
 
-		if (bio->bi_bdev->bd_ro_warned)
+		if (bdev_test_flag(bio->bi_bdev, BD_RO_WARNED))
 			return;
 
-		bio->bi_bdev->bd_ro_warned = true;
+		bdev_set_flag(bio->bi_bdev, BD_RO_WARNED);
+
 		/*
 		 * Use ioctl to set underlying disk of raid/dm to read-only
 		 * will trigger this.
diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
index c8f5364b24f1..59de93913cc4 100644
--- a/include/linux/blk_types.h
+++ b/include/linux/blk_types.h
@@ -65,7 +65,6 @@ struct block_device {
 #ifdef CONFIG_FAIL_MAKE_REQUEST
 	bool			bd_make_it_fail;
 #endif
-	bool			bd_ro_warned;
 	int			bd_writers;
 	/*
 	 * keep this out-of-line as it's both big and not needed in the fast
@@ -87,6 +86,7 @@ enum {
 	BD_READ_ONLY,		// read-only policy
 	BD_WRITE_HOLDER,
 	BD_HAS_SUBMIT_BIO,
+	BD_RO_WARNED,
 };
 
 /*
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 8/8] bdev: move ->bd_make_it_fail to ->__bd_flags
  2024-04-28  5:12 [PATCHES][RFC] packing struct block_device flags Al Viro
                   ` (6 preceding siblings ...)
  2024-04-28  5:19 ` [PATCH 7/8] bdev: move ->bd_ro_warned " Al Viro
@ 2024-04-28  5:21 ` Al Viro
  2024-04-29  5:23 ` [PATCHES][RFC] packing struct block_device flags Christoph Hellwig
  8 siblings, 0 replies; 24+ messages in thread
From: Al Viro @ 2024-04-28  5:21 UTC (permalink / raw)
  To: linux-fsdevel
  Cc: Yu Kuai, linux-block, Christian Brauner, Christoph Hellwig, Jens Axboe

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 block/blk-core.c          |  3 ++-
 block/genhd.c             | 12 ++++++++----
 include/linux/blk_types.h |  6 +++---
 3 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index 1be49be9fac4..1076336dd620 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -494,7 +494,8 @@ __setup("fail_make_request=", setup_fail_make_request);
 
 bool should_fail_request(struct block_device *part, unsigned int bytes)
 {
-	return part->bd_make_it_fail && should_fail(&fail_make_request, bytes);
+	return bdev_test_flag(part, BD_MAKE_IT_FAIL) &&
+	       should_fail(&fail_make_request, bytes);
 }
 
 static int __init fail_make_request_debugfs(void)
diff --git a/block/genhd.c b/block/genhd.c
index 19cd1a31fa80..0cce461952f6 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -1066,7 +1066,8 @@ static DEVICE_ATTR(diskseq, 0444, diskseq_show, NULL);
 ssize_t part_fail_show(struct device *dev,
 		       struct device_attribute *attr, char *buf)
 {
-	return sprintf(buf, "%d\n", dev_to_bdev(dev)->bd_make_it_fail);
+	return sprintf(buf, "%d\n",
+		       bdev_test_flag(dev_to_bdev(dev), BD_MAKE_IT_FAIL));
 }
 
 ssize_t part_fail_store(struct device *dev,
@@ -1075,9 +1076,12 @@ ssize_t part_fail_store(struct device *dev,
 {
 	int i;
 
-	if (count > 0 && sscanf(buf, "%d", &i) > 0)
-		dev_to_bdev(dev)->bd_make_it_fail = i;
-
+	if (count > 0 && sscanf(buf, "%d", &i) > 0) {
+		if (i)
+			bdev_set_flag(dev_to_bdev(dev), BD_MAKE_IT_FAIL);
+		else
+			bdev_clear_flag(dev_to_bdev(dev), BD_MAKE_IT_FAIL);
+	}
 	return count;
 }
 
diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
index 59de93913cc4..98e1c2d28d60 100644
--- a/include/linux/blk_types.h
+++ b/include/linux/blk_types.h
@@ -62,9 +62,6 @@ struct block_device {
 	struct mutex		bd_fsfreeze_mutex; /* serialize freeze/thaw */
 
 	struct partition_meta_info *bd_meta_info;
-#ifdef CONFIG_FAIL_MAKE_REQUEST
-	bool			bd_make_it_fail;
-#endif
 	int			bd_writers;
 	/*
 	 * keep this out-of-line as it's both big and not needed in the fast
@@ -87,6 +84,9 @@ enum {
 	BD_WRITE_HOLDER,
 	BD_HAS_SUBMIT_BIO,
 	BD_RO_WARNED,
+#ifdef CONFIG_FAIL_MAKE_REQUEST
+	BD_MAKE_IT_FAIL,
+#endif
 };
 
 /*
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH 1/8] Use bdev_is_paritition() instead of open-coding it
  2024-04-28  5:14 ` [PATCH 1/8] Use bdev_is_paritition() instead of open-coding it Al Viro
@ 2024-04-29  5:19   ` Christoph Hellwig
  0 siblings, 0 replies; 24+ messages in thread
From: Christoph Hellwig @ 2024-04-29  5:19 UTC (permalink / raw)
  To: Al Viro
  Cc: linux-fsdevel, Yu Kuai, linux-block, Christian Brauner,
	Christoph Hellwig, Jens Axboe

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCHES][RFC] packing struct block_device flags
  2024-04-28  5:12 [PATCHES][RFC] packing struct block_device flags Al Viro
                   ` (7 preceding siblings ...)
  2024-04-28  5:21 ` [PATCH 8/8] bdev: move ->bd_make_it_fail " Al Viro
@ 2024-04-29  5:23 ` Christoph Hellwig
  2024-04-29  7:31   ` Al Viro
  8 siblings, 1 reply; 24+ messages in thread
From: Christoph Hellwig @ 2024-04-29  5:23 UTC (permalink / raw)
  To: Al Viro
  Cc: linux-fsdevel, Yu Kuai, linux-block, Christian Brauner,
	Christoph Hellwig, Jens Axboe

On Sun, Apr 28, 2024 at 06:12:32AM +0100, Al Viro wrote:
> 	We have several bool members in struct block_device.
> It would be nice to pack that stuff, preferably without
> blowing the cacheline boundaries.
> 
> 	That had been suggested a while ago, and initial
> implementation by Yu Kuai <yukuai1@huaweicloud.com> ran into
> objections re atomicity, along with the obvious suggestion to
> use unsigned long and test_bit/set_bit/clear_bit for access.
> Unfortunately, that *does* blow the cacheline boundaries.
> 
> 	However, it's not hard to do that without bitops;
> we have an 8-bit assign-once partition number nearby, and
> folding it into a 32-bit member leaves us up to 24 bits for
> flags.  Using cmpxchg for setting/clearing flags is not
> hard, and 32bit cmpxchg is supported everywhere.

To me this looks pretty complicated and arcane.  How about we just
reclaim a little space in the block device and just keep bd_partno
and add an unsigned long base flags?

E.g. bd_meta_info is only used in slow path early boot and sysfs code
and just set for a few partitions.

Just turn it into a hash/xrray/whatever indexed by bd_dev and we've
already reclaimed enough space.


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCHES][RFC] packing struct block_device flags
  2024-04-29  5:23 ` [PATCHES][RFC] packing struct block_device flags Christoph Hellwig
@ 2024-04-29  7:31   ` Al Viro
  2024-04-29 17:02     ` Al Viro
  0 siblings, 1 reply; 24+ messages in thread
From: Al Viro @ 2024-04-29  7:31 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: linux-fsdevel, Yu Kuai, linux-block, Christian Brauner, Jens Axboe

On Mon, Apr 29, 2024 at 07:23:15AM +0200, Christoph Hellwig wrote:
> On Sun, Apr 28, 2024 at 06:12:32AM +0100, Al Viro wrote:
> > 	We have several bool members in struct block_device.
> > It would be nice to pack that stuff, preferably without
> > blowing the cacheline boundaries.
> > 
> > 	That had been suggested a while ago, and initial
> > implementation by Yu Kuai <yukuai1@huaweicloud.com> ran into
> > objections re atomicity, along with the obvious suggestion to
> > use unsigned long and test_bit/set_bit/clear_bit for access.
> > Unfortunately, that *does* blow the cacheline boundaries.
> > 
> > 	However, it's not hard to do that without bitops;
> > we have an 8-bit assign-once partition number nearby, and
> > folding it into a 32-bit member leaves us up to 24 bits for
> > flags.  Using cmpxchg for setting/clearing flags is not
> > hard, and 32bit cmpxchg is supported everywhere.
> 
> To me this looks pretty complicated and arcane.

cmpxchg for setting/clearing a bit in u32 is hardly arcane,
especially since these bits are rarely modified.  _Reading_
is done on hot paths, but that's cheap.

For more familiar form,
static inline void bdev_set_flag(struct block_device *bdev, int flag)
{
	u32 *p = &bdev->__bd_flags;
        u32 c, old;

	c = *p;
	while ((old = cmpxchg(p, c, c | (1 << (flag + 8)))) != c)
		c = old;
}

to keep it closer to e.g. generic_atomic_or() and its ilk (asm-generic/atomic.h)
or any number of similar places.

FWIW, we could go for atomic_t there and use
	atomic_read() & 0xff
for partno, with atomic_or()/atomic_and() for set/clear and
atomic_read() & constant for test.  That might slightly optimize
set/clear on some architectures, but setting/clearing flags is
nowhere near hot enough for that to make a difference.

> How about we just
> reclaim a little space in the block device and just keep bd_partno
> and add an unsigned long base flags?
> 
> E.g. bd_meta_info is only used in slow path early boot and sysfs code
> and just set for a few partitions.
> 
> Just turn it into a hash/xrray/whatever indexed by bd_dev and we've
> already reclaimed enough space.

The problem is not the total size, though - it's blowing the first
cacheline.  With struct device shoved into that thing, a word or two
in total size is noise; keeping the fields read on the hot paths within
the first 64 bytes is not.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCHES][RFC] packing struct block_device flags
  2024-04-29  7:31   ` Al Viro
@ 2024-04-29 17:02     ` Al Viro
  2024-04-29 18:13       ` Al Viro
  0 siblings, 1 reply; 24+ messages in thread
From: Al Viro @ 2024-04-29 17:02 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: linux-fsdevel, Yu Kuai, linux-block, Christian Brauner, Jens Axboe

On Mon, Apr 29, 2024 at 08:31:07AM +0100, Al Viro wrote:

> FWIW, we could go for atomic_t there and use
> 	atomic_read() & 0xff
> for partno, with atomic_or()/atomic_and() for set/clear and
> atomic_read() & constant for test.  That might slightly optimize
> set/clear on some architectures, but setting/clearing flags is
> nowhere near hot enough for that to make a difference.

Incremental for that (would be folded into 3/8 if we went that way)
is below; again, I'm not at all sure it's idiomatic enough to bother
with, but that should at least show what's going on:

diff --git a/block/bdev.c b/block/bdev.c
index 9aa23620fe92..fae30eae7741 100644
--- a/block/bdev.c
+++ b/block/bdev.c
@@ -411,7 +411,7 @@ struct block_device *bdev_alloc(struct gendisk *disk, u8 partno)
 	mutex_init(&bdev->bd_fsfreeze_mutex);
 	spin_lock_init(&bdev->bd_size_lock);
 	mutex_init(&bdev->bd_holder_lock);
-	bdev->__bd_flags = partno;
+	atomic_set(&bdev->__bd_flags, partno);
 	bdev->bd_inode = inode;
 	bdev->bd_queue = disk->queue;
 	if (partno && bdev_test_flag(disk->part0, BD_HAS_SUBMIT_BIO))
diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
index 98e1c2d28d60..a822911e28e5 100644
--- a/include/linux/blk_types.h
+++ b/include/linux/blk_types.h
@@ -45,7 +45,7 @@ struct block_device {
 	struct request_queue *	bd_queue;
 	struct disk_stats __percpu *bd_stats;
 	unsigned long		bd_stamp;
-	u32			__bd_flags;	// partition number + flags
+	atomic_t		__bd_flags;	// partition number + flags
 	dev_t			bd_dev;
 	struct inode		*bd_inode;	/* will die */
 
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index d556cec9224b..a8271497ac62 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -722,38 +722,22 @@ void disk_uevent(struct gendisk *disk, enum kobject_action action);
 
 static inline u8 bdev_partno(const struct block_device *bdev)
 {
-	return bdev->__bd_flags & 0xff;
+	return atomic_read(&bdev->__bd_flags) & 0xff;
 }
 
 static inline bool bdev_test_flag(const struct block_device *bdev, int flag)
 {
-	return bdev->__bd_flags & (1 << (flag + 8));
+	return atomic_read(&bdev->__bd_flags) & (1 << (flag + 8));
 }
 
 static inline void bdev_set_flag(struct block_device *bdev, int flag)
 {
-	u32 v = bdev->__bd_flags;
-
-	for (;;) {
-		u32 w = cmpxchg(&bdev->__bd_flags, v, v | (1 << (flag + 8)));
-
-		if (v == w)
-			return;
-		v = w;
-	}
+	atomic_or(1 << (flag + 8), &bdev->__bd_flags);
 }
 
 static inline void bdev_clear_flag(struct block_device *bdev, int flag)
 {
-	u32 v = bdev->__bd_flags;
-
-	for (;;) {
-		u32 w = cmpxchg(&bdev->__bd_flags, v, v & ~(1 << (flag + 8)));
-
-		if (v == w)
-			return;
-		v = w;
-	}
+	atomic_and(~(1 << (flag + 8)), &bdev->__bd_flags);
 }
 
 static inline int get_disk_ro(struct gendisk *disk)

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCHES][RFC] packing struct block_device flags
  2024-04-29 17:02     ` Al Viro
@ 2024-04-29 18:13       ` Al Viro
  2024-04-29 18:30         ` Al Viro
  0 siblings, 1 reply; 24+ messages in thread
From: Al Viro @ 2024-04-29 18:13 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: linux-fsdevel, Yu Kuai, linux-block, Christian Brauner, Jens Axboe

On Mon, Apr 29, 2024 at 06:02:09PM +0100, Al Viro wrote:
> On Mon, Apr 29, 2024 at 08:31:07AM +0100, Al Viro wrote:
> 
> > FWIW, we could go for atomic_t there and use
> > 	atomic_read() & 0xff
> > for partno, with atomic_or()/atomic_and() for set/clear and
> > atomic_read() & constant for test.  That might slightly optimize
> > set/clear on some architectures, but setting/clearing flags is
> > nowhere near hot enough for that to make a difference.
> 
> Incremental for that (would be folded into 3/8 if we went that way)
> is below; again, I'm not at all sure it's idiomatic enough to bother
> with, but that should at least show what's going on:

Or this, for that matter:

diff --git a/block/bdev.c b/block/bdev.c
index 9aa23620fe92..fae30eae7741 100644
--- a/block/bdev.c
+++ b/block/bdev.c
@@ -411,7 +411,7 @@ struct block_device *bdev_alloc(struct gendisk *disk, u8 partno)
 	mutex_init(&bdev->bd_fsfreeze_mutex);
 	spin_lock_init(&bdev->bd_size_lock);
 	mutex_init(&bdev->bd_holder_lock);
-	bdev->__bd_flags = partno;
+	atomic_set(&bdev->__bd_flags, partno);
 	bdev->bd_inode = inode;
 	bdev->bd_queue = disk->queue;
 	if (partno && bdev_test_flag(disk->part0, BD_HAS_SUBMIT_BIO))
diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
index 98e1c2d28d60..84bfc702269a 100644
--- a/include/linux/blk_types.h
+++ b/include/linux/blk_types.h
@@ -45,7 +45,15 @@ struct block_device {
 	struct request_queue *	bd_queue;
 	struct disk_stats __percpu *bd_stats;
 	unsigned long		bd_stamp;
-	u32			__bd_flags;	// partition number + flags
+	atomic_t		__bd_flags;
+#define BD_PARTNO		255	// lower 8 bits; assign-once
+#define BD_READ_ONLY		(1u<<8) // read-only policy
+#define BD_WRITE_HOLDER		(1u<<9)
+#define BD_HAS_SUBMIT_BIO	(1u<<10)
+#define BD_RO_WARNED		(1u<<11)
+#ifdef CONFIG_FAIL_MAKE_REQUEST
+#define BD_MAKE_IT_FAIL		(1u<<12)
+#endif
 	dev_t			bd_dev;
 	struct inode		*bd_inode;	/* will die */
 
@@ -79,16 +87,6 @@ struct block_device {
 #define bdev_kobj(_bdev) \
 	(&((_bdev)->bd_device.kobj))
 
-enum {
-	BD_READ_ONLY,		// read-only policy
-	BD_WRITE_HOLDER,
-	BD_HAS_SUBMIT_BIO,
-	BD_RO_WARNED,
-#ifdef CONFIG_FAIL_MAKE_REQUEST
-	BD_MAKE_IT_FAIL,
-#endif
-};
-
 /*
  * Block error status values.  See block/blk-core:blk_errors for the details.
  * Alpha cannot write a byte atomically, so we need to use 32-bit value.
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index d556cec9224b..1fe91231f85b 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -722,38 +722,22 @@ void disk_uevent(struct gendisk *disk, enum kobject_action action);
 
 static inline u8 bdev_partno(const struct block_device *bdev)
 {
-	return bdev->__bd_flags & 0xff;
+	return atomic_read(&bdev->__bd_flags) & BD_PARTNO;
 }
 
-static inline bool bdev_test_flag(const struct block_device *bdev, int flag)
+static inline bool bdev_test_flag(const struct block_device *bdev, unsigned flag)
 {
-	return bdev->__bd_flags & (1 << (flag + 8));
+	return atomic_read(&bdev->__bd_flags) & flag;
 }
 
-static inline void bdev_set_flag(struct block_device *bdev, int flag)
+static inline void bdev_set_flag(struct block_device *bdev, unsigned flag)
 {
-	u32 v = bdev->__bd_flags;
-
-	for (;;) {
-		u32 w = cmpxchg(&bdev->__bd_flags, v, v | (1 << (flag + 8)));
-
-		if (v == w)
-			return;
-		v = w;
-	}
+	atomic_or(flag, &bdev->__bd_flags);
 }
 
-static inline void bdev_clear_flag(struct block_device *bdev, int flag)
+static inline void bdev_clear_flag(struct block_device *bdev, unsigned flag)
 {
-	u32 v = bdev->__bd_flags;
-
-	for (;;) {
-		u32 w = cmpxchg(&bdev->__bd_flags, v, v & ~(1 << (flag + 8)));
-
-		if (v == w)
-			return;
-		v = w;
-	}
+	atomic_andnot(flag, &bdev->__bd_flags);
 }
 
 static inline int get_disk_ro(struct gendisk *disk)

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCHES][RFC] packing struct block_device flags
  2024-04-29 18:13       ` Al Viro
@ 2024-04-29 18:30         ` Al Viro
  2024-05-03  0:06           ` Al Viro
  0 siblings, 1 reply; 24+ messages in thread
From: Al Viro @ 2024-04-29 18:30 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: linux-fsdevel, Yu Kuai, linux-block, Christian Brauner, Jens Axboe

On Mon, Apr 29, 2024 at 07:13:00PM +0100, Al Viro wrote:
> On Mon, Apr 29, 2024 at 06:02:09PM +0100, Al Viro wrote:
> > On Mon, Apr 29, 2024 at 08:31:07AM +0100, Al Viro wrote:
> > 
> > > FWIW, we could go for atomic_t there and use
> > > 	atomic_read() & 0xff
> > > for partno, with atomic_or()/atomic_and() for set/clear and
> > > atomic_read() & constant for test.  That might slightly optimize
> > > set/clear on some architectures, but setting/clearing flags is
> > > nowhere near hot enough for that to make a difference.
> > 
> > Incremental for that (would be folded into 3/8 if we went that way)
> > is below; again, I'm not at all sure it's idiomatic enough to bother
> > with, but that should at least show what's going on:
> 
> Or this, for that matter:

See #work.bd_flags-2 for carve-up of that variant.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCHES][RFC] packing struct block_device flags
  2024-04-29 18:30         ` Al Viro
@ 2024-05-03  0:06           ` Al Viro
  2024-05-03  0:07             ` [PATCH 1/8] Use bdev_is_paritition() instead of open-coding it Al Viro
                               ` (7 more replies)
  0 siblings, 8 replies; 24+ messages in thread
From: Al Viro @ 2024-05-03  0:06 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: linux-fsdevel, Yu Kuai, linux-block, Christian Brauner, Jens Axboe

On Mon, Apr 29, 2024 at 07:30:41PM +0100, Al Viro wrote:
> On Mon, Apr 29, 2024 at 07:13:00PM +0100, Al Viro wrote:
> > On Mon, Apr 29, 2024 at 06:02:09PM +0100, Al Viro wrote:
> > > On Mon, Apr 29, 2024 at 08:31:07AM +0100, Al Viro wrote:
> > > 
> > > > FWIW, we could go for atomic_t there and use
> > > > 	atomic_read() & 0xff
> > > > for partno, with atomic_or()/atomic_and() for set/clear and
> > > > atomic_read() & constant for test.  That might slightly optimize
> > > > set/clear on some architectures, but setting/clearing flags is
> > > > nowhere near hot enough for that to make a difference.
> > > 
> > > Incremental for that (would be folded into 3/8 if we went that way)
> > > is below; again, I'm not at all sure it's idiomatic enough to bother
> > > with, but that should at least show what's going on:
> > 
> > Or this, for that matter:
> 
> See #work.bd_flags-2 for carve-up of that variant.

Ugh...  Forgot to push it out, sorry.  Branch pushed out now,
individual patches in followups.

Summary of changes compared to posted variant:
	__bd_flags is atomic_t now.
	atomic_{or,andnot} is used instead of cmpxchg().
	the constants (BD_READ_ONLY, etc.) are masks, not flag numbers.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 1/8] Use bdev_is_paritition() instead of open-coding it
  2024-05-03  0:06           ` Al Viro
@ 2024-05-03  0:07             ` Al Viro
  2024-05-03  0:07             ` [PATCH 2/8] wrapper for access to ->bd_partno Al Viro
                               ` (6 subsequent siblings)
  7 siblings, 0 replies; 24+ messages in thread
From: Al Viro @ 2024-05-03  0:07 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: linux-fsdevel, Yu Kuai, linux-block, Christian Brauner, Jens Axboe

Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 block/blk-core.c          | 5 +++--
 block/blk-mq.c            | 2 +-
 include/linux/part_stat.h | 2 +-
 lib/vsprintf.c            | 2 +-
 4 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index a16b5abdbbf5..20322abc6082 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -759,7 +759,8 @@ void submit_bio_noacct(struct bio *bio)
 	if (!bio_flagged(bio, BIO_REMAPPED)) {
 		if (unlikely(bio_check_eod(bio)))
 			goto end_io;
-		if (bdev->bd_partno && unlikely(blk_partition_remap(bio)))
+		if (bdev_is_partition(bdev) &&
+		    unlikely(blk_partition_remap(bio)))
 			goto end_io;
 	}
 
@@ -989,7 +990,7 @@ void update_io_ticks(struct block_device *part, unsigned long now, bool end)
 		if (likely(try_cmpxchg(&part->bd_stamp, &stamp, now)))
 			__part_stat_add(part, io_ticks, end ? now - stamp : 1);
 	}
-	if (part->bd_partno) {
+	if (bdev_is_partition(part)) {
 		part = bdev_whole(part);
 		goto again;
 	}
diff --git a/block/blk-mq.c b/block/blk-mq.c
index 32afb87efbd0..43bb8f50a07c 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -92,7 +92,7 @@ static bool blk_mq_check_inflight(struct request *rq, void *priv)
 	struct mq_inflight *mi = priv;
 
 	if (rq->part && blk_do_io_stat(rq) &&
-	    (!mi->part->bd_partno || rq->part == mi->part) &&
+	    (!bdev_is_partition(mi->part) || rq->part == mi->part) &&
 	    blk_mq_rq_state(rq) == MQ_RQ_IN_FLIGHT)
 		mi->inflight[rq_data_dir(rq)]++;
 
diff --git a/include/linux/part_stat.h b/include/linux/part_stat.h
index abeba356bc3f..ac8c44dd8237 100644
--- a/include/linux/part_stat.h
+++ b/include/linux/part_stat.h
@@ -59,7 +59,7 @@ static inline void part_stat_set_all(struct block_device *part, int value)
 
 #define part_stat_add(part, field, addnd)	do {			\
 	__part_stat_add((part), field, addnd);				\
-	if ((part)->bd_partno)						\
+	if (bdev_is_partition(part))					\
 		__part_stat_add(bdev_whole(part), field, addnd);	\
 } while (0)
 
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 552738f14275..3f9f1b959ef0 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -966,7 +966,7 @@ char *bdev_name(char *buf, char *end, struct block_device *bdev,
 
 	hd = bdev->bd_disk;
 	buf = string(buf, end, hd->disk_name, spec);
-	if (bdev->bd_partno) {
+	if (bdev_is_partition(bdev)) {
 		if (isdigit(hd->disk_name[strlen(hd->disk_name)-1])) {
 			if (buf < end)
 				*buf = 'p';
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 2/8] wrapper for access to ->bd_partno
  2024-05-03  0:06           ` Al Viro
  2024-05-03  0:07             ` [PATCH 1/8] Use bdev_is_paritition() instead of open-coding it Al Viro
@ 2024-05-03  0:07             ` Al Viro
  2024-05-03  0:08             ` [PATCH v2 3/8] bdev: infrastructure for flags Al Viro
                               ` (5 subsequent siblings)
  7 siblings, 0 replies; 24+ messages in thread
From: Al Viro @ 2024-05-03  0:07 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: linux-fsdevel, Yu Kuai, linux-block, Christian Brauner, Jens Axboe

On the next step it's going to get folded into a field where flags will go.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 block/early-lookup.c    |  2 +-
 block/partitions/core.c | 12 ++++++------
 include/linux/blkdev.h  |  7 ++++++-
 lib/vsprintf.c          |  2 +-
 4 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/block/early-lookup.c b/block/early-lookup.c
index 3effbd0d35e9..3fb57f7d2b12 100644
--- a/block/early-lookup.c
+++ b/block/early-lookup.c
@@ -78,7 +78,7 @@ static int __init devt_from_partuuid(const char *uuid_str, dev_t *devt)
 		 * to the partition number found by UUID.
 		 */
 		*devt = part_devt(dev_to_disk(dev),
-				  dev_to_bdev(dev)->bd_partno + offset);
+				  bdev_partno(dev_to_bdev(dev)) + offset);
 	} else {
 		*devt = dev->devt;
 	}
diff --git a/block/partitions/core.c b/block/partitions/core.c
index b11e88c82c8c..edd5309dc4ba 100644
--- a/block/partitions/core.c
+++ b/block/partitions/core.c
@@ -173,7 +173,7 @@ static struct parsed_partitions *check_partition(struct gendisk *hd)
 static ssize_t part_partition_show(struct device *dev,
 				   struct device_attribute *attr, char *buf)
 {
-	return sprintf(buf, "%d\n", dev_to_bdev(dev)->bd_partno);
+	return sprintf(buf, "%d\n", bdev_partno(dev_to_bdev(dev)));
 }
 
 static ssize_t part_start_show(struct device *dev,
@@ -250,7 +250,7 @@ static int part_uevent(const struct device *dev, struct kobj_uevent_env *env)
 {
 	const struct block_device *part = dev_to_bdev(dev);
 
-	add_uevent_var(env, "PARTN=%u", part->bd_partno);
+	add_uevent_var(env, "PARTN=%u", bdev_partno(part));
 	if (part->bd_meta_info && part->bd_meta_info->volname[0])
 		add_uevent_var(env, "PARTNAME=%s", part->bd_meta_info->volname);
 	return 0;
@@ -267,7 +267,7 @@ void drop_partition(struct block_device *part)
 {
 	lockdep_assert_held(&part->bd_disk->open_mutex);
 
-	xa_erase(&part->bd_disk->part_tbl, part->bd_partno);
+	xa_erase(&part->bd_disk->part_tbl, bdev_partno(part));
 	kobject_put(part->bd_holder_dir);
 
 	device_del(&part->bd_device);
@@ -338,8 +338,8 @@ static struct block_device *add_partition(struct gendisk *disk, int partno,
 	pdev->parent = ddev;
 
 	/* in consecutive minor range? */
-	if (bdev->bd_partno < disk->minors) {
-		devt = MKDEV(disk->major, disk->first_minor + bdev->bd_partno);
+	if (bdev_partno(bdev) < disk->minors) {
+		devt = MKDEV(disk->major, disk->first_minor + bdev_partno(bdev));
 	} else {
 		err = blk_alloc_ext_minor();
 		if (err < 0)
@@ -404,7 +404,7 @@ static bool partition_overlaps(struct gendisk *disk, sector_t start,
 
 	rcu_read_lock();
 	xa_for_each_start(&disk->part_tbl, idx, part, 1) {
-		if (part->bd_partno != skip_partno &&
+		if (bdev_partno(part) != skip_partno &&
 		    start < part->bd_start_sect + bdev_nr_sectors(part) &&
 		    start + length > part->bd_start_sect) {
 			overlap = true;
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index c3e8f7cf96be..32549d675955 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -720,6 +720,11 @@ void invalidate_disk(struct gendisk *disk);
 void set_disk_ro(struct gendisk *disk, bool read_only);
 void disk_uevent(struct gendisk *disk, enum kobject_action action);
 
+static inline u8 bdev_partno(const struct block_device *bdev)
+{
+	return bdev->bd_partno;
+}
+
 static inline int get_disk_ro(struct gendisk *disk)
 {
 	return disk->part0->bd_read_only ||
@@ -1095,7 +1100,7 @@ static inline int sb_issue_zeroout(struct super_block *sb, sector_t block,
 
 static inline bool bdev_is_partition(struct block_device *bdev)
 {
-	return bdev->bd_partno;
+	return bdev_partno(bdev) != 0;
 }
 
 enum blk_default_limits {
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 3f9f1b959ef0..cdd4e2314bfc 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -972,7 +972,7 @@ char *bdev_name(char *buf, char *end, struct block_device *bdev,
 				*buf = 'p';
 			buf++;
 		}
-		buf = number(buf, end, bdev->bd_partno, spec);
+		buf = number(buf, end, bdev_partno(bdev), spec);
 	}
 	return buf;
 }
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 3/8] bdev: infrastructure for flags
  2024-05-03  0:06           ` Al Viro
  2024-05-03  0:07             ` [PATCH 1/8] Use bdev_is_paritition() instead of open-coding it Al Viro
  2024-05-03  0:07             ` [PATCH 2/8] wrapper for access to ->bd_partno Al Viro
@ 2024-05-03  0:08             ` Al Viro
  2024-05-03  0:09             ` [PATCH v2 4/8] bdev: move ->bd_read_only to ->__bd_flags Al Viro
                               ` (4 subsequent siblings)
  7 siblings, 0 replies; 24+ messages in thread
From: Al Viro @ 2024-05-03  0:08 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: linux-fsdevel, Yu Kuai, linux-block, Christian Brauner, Jens Axboe

Replace bd_partno with a 32bit field (__bd_flags).  The lower 8 bits
contain the partition number, the upper 24 are for flags.

Helpers: bdev_{test,set,clear}_flag(bdev, flag), with atomic_or()
and atomic_andnot() used to set/clear.

NOTE: this commit does not actually move any flags over there - they
are still bool fields.  As the result, it shifts the fields wrt
cacheline boundaries; that's going to be restored once the first
3 flags are dealt with.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 block/bdev.c              |  2 +-
 include/linux/blk_types.h |  3 ++-
 include/linux/blkdev.h    | 17 ++++++++++++++++-
 3 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/block/bdev.c b/block/bdev.c
index 7a5f611c3d2e..2ec223315500 100644
--- a/block/bdev.c
+++ b/block/bdev.c
@@ -411,7 +411,7 @@ struct block_device *bdev_alloc(struct gendisk *disk, u8 partno)
 	mutex_init(&bdev->bd_fsfreeze_mutex);
 	spin_lock_init(&bdev->bd_size_lock);
 	mutex_init(&bdev->bd_holder_lock);
-	bdev->bd_partno = partno;
+	atomic_set(&bdev->__bd_flags, partno);
 	bdev->bd_inode = inode;
 	bdev->bd_queue = disk->queue;
 	if (partno)
diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
index cb1526ec44b5..04f92737ab08 100644
--- a/include/linux/blk_types.h
+++ b/include/linux/blk_types.h
@@ -45,8 +45,9 @@ struct block_device {
 	struct request_queue *	bd_queue;
 	struct disk_stats __percpu *bd_stats;
 	unsigned long		bd_stamp;
+	atomic_t		__bd_flags;	// partition number + flags
+#define BD_PARTNO		255	// lower 8 bits; assign-once
 	bool			bd_read_only;	/* read-only policy */
-	u8			bd_partno;
 	bool			bd_write_holder;
 	bool			bd_has_submit_bio;
 	dev_t			bd_dev;
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 32549d675955..99917e5860fd 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -722,7 +722,22 @@ void disk_uevent(struct gendisk *disk, enum kobject_action action);
 
 static inline u8 bdev_partno(const struct block_device *bdev)
 {
-	return bdev->bd_partno;
+	return atomic_read(&bdev->__bd_flags) & BD_PARTNO;
+}
+
+static inline bool bdev_test_flag(const struct block_device *bdev, unsigned flag)
+{
+	return atomic_read(&bdev->__bd_flags) & flag;
+}
+
+static inline void bdev_set_flag(struct block_device *bdev, unsigned flag)
+{
+	atomic_or(flag, &bdev->__bd_flags);
+}
+
+static inline void bdev_clear_flag(struct block_device *bdev, unsigned flag)
+{
+	atomic_andnot(flag, &bdev->__bd_flags);
 }
 
 static inline int get_disk_ro(struct gendisk *disk)
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 4/8] bdev: move ->bd_read_only to ->__bd_flags
  2024-05-03  0:06           ` Al Viro
                               ` (2 preceding siblings ...)
  2024-05-03  0:08             ` [PATCH v2 3/8] bdev: infrastructure for flags Al Viro
@ 2024-05-03  0:09             ` Al Viro
  2024-05-03  0:09             ` [PATCH v2 5/8] bdev: move ->bd_write_holder into ->__bd_flags Al Viro
                               ` (3 subsequent siblings)
  7 siblings, 0 replies; 24+ messages in thread
From: Al Viro @ 2024-05-03  0:09 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: linux-fsdevel, Yu Kuai, linux-block, Christian Brauner, Jens Axboe

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 block/ioctl.c             | 5 ++++-
 include/linux/blk_types.h | 2 +-
 include/linux/blkdev.h    | 4 ++--
 3 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/block/ioctl.c b/block/ioctl.c
index 0c76137adcaa..be173e4ff43d 100644
--- a/block/ioctl.c
+++ b/block/ioctl.c
@@ -402,7 +402,10 @@ static int blkdev_roset(struct block_device *bdev, unsigned cmd,
 		if (ret)
 			return ret;
 	}
-	bdev->bd_read_only = n;
+	if (n)
+		bdev_set_flag(bdev, BD_READ_ONLY);
+	else
+		bdev_clear_flag(bdev, BD_READ_ONLY);
 	return 0;
 }
 
diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
index 04f92737ab08..f70dd31cbcd1 100644
--- a/include/linux/blk_types.h
+++ b/include/linux/blk_types.h
@@ -47,7 +47,7 @@ struct block_device {
 	unsigned long		bd_stamp;
 	atomic_t		__bd_flags;	// partition number + flags
 #define BD_PARTNO		255	// lower 8 bits; assign-once
-	bool			bd_read_only;	/* read-only policy */
+#define BD_READ_ONLY		(1u<<8) // read-only policy
 	bool			bd_write_holder;
 	bool			bd_has_submit_bio;
 	dev_t			bd_dev;
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 99917e5860fd..1fe91231f85b 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -742,13 +742,13 @@ static inline void bdev_clear_flag(struct block_device *bdev, unsigned flag)
 
 static inline int get_disk_ro(struct gendisk *disk)
 {
-	return disk->part0->bd_read_only ||
+	return bdev_test_flag(disk->part0, BD_READ_ONLY) ||
 		test_bit(GD_READ_ONLY, &disk->state);
 }
 
 static inline int bdev_read_only(struct block_device *bdev)
 {
-	return bdev->bd_read_only || get_disk_ro(bdev->bd_disk);
+	return bdev_test_flag(bdev, BD_READ_ONLY) || get_disk_ro(bdev->bd_disk);
 }
 
 bool set_capacity_and_notify(struct gendisk *disk, sector_t size);
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 5/8] bdev: move ->bd_write_holder into ->__bd_flags
  2024-05-03  0:06           ` Al Viro
                               ` (3 preceding siblings ...)
  2024-05-03  0:09             ` [PATCH v2 4/8] bdev: move ->bd_read_only to ->__bd_flags Al Viro
@ 2024-05-03  0:09             ` Al Viro
  2024-05-03  0:10             ` [PATCH v2 6/8] bdev: move ->bd_has_subit_bio to ->__bd_flags Al Viro
                               ` (2 subsequent siblings)
  7 siblings, 0 replies; 24+ messages in thread
From: Al Viro @ 2024-05-03  0:09 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: linux-fsdevel, Yu Kuai, linux-block, Christian Brauner, Jens Axboe

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 block/bdev.c              | 9 +++++----
 include/linux/blk_types.h | 2 +-
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/block/bdev.c b/block/bdev.c
index 2ec223315500..9df9a59f0900 100644
--- a/block/bdev.c
+++ b/block/bdev.c
@@ -624,7 +624,7 @@ static void bd_end_claim(struct block_device *bdev, void *holder)
 		bdev->bd_holder = NULL;
 		bdev->bd_holder_ops = NULL;
 		mutex_unlock(&bdev->bd_holder_lock);
-		if (bdev->bd_write_holder)
+		if (bdev_test_flag(bdev, BD_WRITE_HOLDER))
 			unblock = true;
 	}
 	if (!whole->bd_holders)
@@ -640,7 +640,7 @@ static void bd_end_claim(struct block_device *bdev, void *holder)
 	 */
 	if (unblock) {
 		disk_unblock_events(bdev->bd_disk);
-		bdev->bd_write_holder = false;
+		bdev_clear_flag(bdev, BD_WRITE_HOLDER);
 	}
 }
 
@@ -892,9 +892,10 @@ int bdev_open(struct block_device *bdev, blk_mode_t mode, void *holder,
 		 * writeable reference is too fragile given the way @mode is
 		 * used in blkdev_get/put().
 		 */
-		if ((mode & BLK_OPEN_WRITE) && !bdev->bd_write_holder &&
+		if ((mode & BLK_OPEN_WRITE) &&
+		    !bdev_test_flag(bdev, BD_WRITE_HOLDER) &&
 		    (disk->event_flags & DISK_EVENT_FLAG_BLOCK_ON_EXCL_WRITE)) {
-			bdev->bd_write_holder = true;
+			bdev_set_flag(bdev, BD_WRITE_HOLDER);
 			unblock_events = false;
 		}
 	}
diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
index f70dd31cbcd1..e45a490d488e 100644
--- a/include/linux/blk_types.h
+++ b/include/linux/blk_types.h
@@ -48,7 +48,7 @@ struct block_device {
 	atomic_t		__bd_flags;	// partition number + flags
 #define BD_PARTNO		255	// lower 8 bits; assign-once
 #define BD_READ_ONLY		(1u<<8) // read-only policy
-	bool			bd_write_holder;
+#define BD_WRITE_HOLDER		(1u<<9)
 	bool			bd_has_submit_bio;
 	dev_t			bd_dev;
 	struct inode		*bd_inode;	/* will die */
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 6/8] bdev: move ->bd_has_subit_bio to ->__bd_flags
  2024-05-03  0:06           ` Al Viro
                               ` (4 preceding siblings ...)
  2024-05-03  0:09             ` [PATCH v2 5/8] bdev: move ->bd_write_holder into ->__bd_flags Al Viro
@ 2024-05-03  0:10             ` Al Viro
  2024-05-03  0:10             ` [PATCH v2 7/8] bdev: move ->bd_ro_warned " Al Viro
  2024-05-03  0:11             ` [PATCH v2 8/8] bdev: move ->bd_make_it_fail " Al Viro
  7 siblings, 0 replies; 24+ messages in thread
From: Al Viro @ 2024-05-03  0:10 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: linux-fsdevel, Yu Kuai, linux-block, Christian Brauner, Jens Axboe

In bdev_alloc() we have all flags initialized to false, so
assignment to ->bh_has_submit_bio n there is a no-op unless
we have partno != 0 and flag already set on entire device.

In device_add_disk() we have just allocated the block_device
in question and it had been a full-device one, so the flag
is guaranteed to be still clear when we get to assignment.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 block/bdev.c              | 6 ++----
 block/blk-core.c          | 4 ++--
 block/genhd.c             | 3 ++-
 include/linux/blk_types.h | 2 +-
 4 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/block/bdev.c b/block/bdev.c
index 9df9a59f0900..fae30eae7741 100644
--- a/block/bdev.c
+++ b/block/bdev.c
@@ -414,10 +414,8 @@ struct block_device *bdev_alloc(struct gendisk *disk, u8 partno)
 	atomic_set(&bdev->__bd_flags, partno);
 	bdev->bd_inode = inode;
 	bdev->bd_queue = disk->queue;
-	if (partno)
-		bdev->bd_has_submit_bio = disk->part0->bd_has_submit_bio;
-	else
-		bdev->bd_has_submit_bio = false;
+	if (partno && bdev_test_flag(disk->part0, BD_HAS_SUBMIT_BIO))
+		bdev_set_flag(bdev, BD_HAS_SUBMIT_BIO);
 	bdev->bd_stats = alloc_percpu(struct disk_stats);
 	if (!bdev->bd_stats) {
 		iput(inode);
diff --git a/block/blk-core.c b/block/blk-core.c
index 20322abc6082..f61460b65408 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -615,7 +615,7 @@ static void __submit_bio(struct bio *bio)
 	if (unlikely(!blk_crypto_bio_prep(&bio)))
 		return;
 
-	if (!bio->bi_bdev->bd_has_submit_bio) {
+	if (!bdev_test_flag(bio->bi_bdev, BD_HAS_SUBMIT_BIO)) {
 		blk_mq_submit_bio(bio);
 	} else if (likely(bio_queue_enter(bio) == 0)) {
 		struct gendisk *disk = bio->bi_bdev->bd_disk;
@@ -723,7 +723,7 @@ void submit_bio_noacct_nocheck(struct bio *bio)
 	 */
 	if (current->bio_list)
 		bio_list_add(&current->bio_list[0], bio);
-	else if (!bio->bi_bdev->bd_has_submit_bio)
+	else if (!bdev_test_flag(bio->bi_bdev, BD_HAS_SUBMIT_BIO))
 		__submit_bio_noacct_mq(bio);
 	else
 		__submit_bio_noacct(bio);
diff --git a/block/genhd.c b/block/genhd.c
index bb29a68e1d67..19cd1a31fa80 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -413,7 +413,8 @@ int __must_check device_add_disk(struct device *parent, struct gendisk *disk,
 	elevator_init_mq(disk->queue);
 
 	/* Mark bdev as having a submit_bio, if needed */
-	disk->part0->bd_has_submit_bio = disk->fops->submit_bio != NULL;
+	if (disk->fops->submit_bio)
+		bdev_set_flag(disk->part0, BD_HAS_SUBMIT_BIO);
 
 	/*
 	 * If the driver provides an explicit major number it also must provide
diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
index e45a490d488e..11b9e8eeb79f 100644
--- a/include/linux/blk_types.h
+++ b/include/linux/blk_types.h
@@ -49,7 +49,7 @@ struct block_device {
 #define BD_PARTNO		255	// lower 8 bits; assign-once
 #define BD_READ_ONLY		(1u<<8) // read-only policy
 #define BD_WRITE_HOLDER		(1u<<9)
-	bool			bd_has_submit_bio;
+#define BD_HAS_SUBMIT_BIO	(1u<<10)
 	dev_t			bd_dev;
 	struct inode		*bd_inode;	/* will die */
 
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 7/8] bdev: move ->bd_ro_warned to ->__bd_flags
  2024-05-03  0:06           ` Al Viro
                               ` (5 preceding siblings ...)
  2024-05-03  0:10             ` [PATCH v2 6/8] bdev: move ->bd_has_subit_bio to ->__bd_flags Al Viro
@ 2024-05-03  0:10             ` Al Viro
  2024-05-03  0:11             ` [PATCH v2 8/8] bdev: move ->bd_make_it_fail " Al Viro
  7 siblings, 0 replies; 24+ messages in thread
From: Al Viro @ 2024-05-03  0:10 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: linux-fsdevel, Yu Kuai, linux-block, Christian Brauner, Jens Axboe

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 block/blk-core.c          | 5 +++--
 include/linux/blk_types.h | 2 +-
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index f61460b65408..1be49be9fac4 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -514,10 +514,11 @@ static inline void bio_check_ro(struct bio *bio)
 		if (op_is_flush(bio->bi_opf) && !bio_sectors(bio))
 			return;
 
-		if (bio->bi_bdev->bd_ro_warned)
+		if (bdev_test_flag(bio->bi_bdev, BD_RO_WARNED))
 			return;
 
-		bio->bi_bdev->bd_ro_warned = true;
+		bdev_set_flag(bio->bi_bdev, BD_RO_WARNED);
+
 		/*
 		 * Use ioctl to set underlying disk of raid/dm to read-only
 		 * will trigger this.
diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
index 11b9e8eeb79f..4e0c8785090c 100644
--- a/include/linux/blk_types.h
+++ b/include/linux/blk_types.h
@@ -50,6 +50,7 @@ struct block_device {
 #define BD_READ_ONLY		(1u<<8) // read-only policy
 #define BD_WRITE_HOLDER		(1u<<9)
 #define BD_HAS_SUBMIT_BIO	(1u<<10)
+#define BD_RO_WARNED		(1u<<11)
 	dev_t			bd_dev;
 	struct inode		*bd_inode;	/* will die */
 
@@ -69,7 +70,6 @@ struct block_device {
 #ifdef CONFIG_FAIL_MAKE_REQUEST
 	bool			bd_make_it_fail;
 #endif
-	bool			bd_ro_warned;
 	int			bd_writers;
 	/*
 	 * keep this out-of-line as it's both big and not needed in the fast
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 8/8] bdev: move ->bd_make_it_fail to ->__bd_flags
  2024-05-03  0:06           ` Al Viro
                               ` (6 preceding siblings ...)
  2024-05-03  0:10             ` [PATCH v2 7/8] bdev: move ->bd_ro_warned " Al Viro
@ 2024-05-03  0:11             ` Al Viro
  7 siblings, 0 replies; 24+ messages in thread
From: Al Viro @ 2024-05-03  0:11 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: linux-fsdevel, Yu Kuai, linux-block, Christian Brauner, Jens Axboe

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 block/blk-core.c          |  3 ++-
 block/genhd.c             | 12 ++++++++----
 include/linux/blk_types.h |  6 +++---
 3 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index 1be49be9fac4..1076336dd620 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -494,7 +494,8 @@ __setup("fail_make_request=", setup_fail_make_request);
 
 bool should_fail_request(struct block_device *part, unsigned int bytes)
 {
-	return part->bd_make_it_fail && should_fail(&fail_make_request, bytes);
+	return bdev_test_flag(part, BD_MAKE_IT_FAIL) &&
+	       should_fail(&fail_make_request, bytes);
 }
 
 static int __init fail_make_request_debugfs(void)
diff --git a/block/genhd.c b/block/genhd.c
index 19cd1a31fa80..0cce461952f6 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -1066,7 +1066,8 @@ static DEVICE_ATTR(diskseq, 0444, diskseq_show, NULL);
 ssize_t part_fail_show(struct device *dev,
 		       struct device_attribute *attr, char *buf)
 {
-	return sprintf(buf, "%d\n", dev_to_bdev(dev)->bd_make_it_fail);
+	return sprintf(buf, "%d\n",
+		       bdev_test_flag(dev_to_bdev(dev), BD_MAKE_IT_FAIL));
 }
 
 ssize_t part_fail_store(struct device *dev,
@@ -1075,9 +1076,12 @@ ssize_t part_fail_store(struct device *dev,
 {
 	int i;
 
-	if (count > 0 && sscanf(buf, "%d", &i) > 0)
-		dev_to_bdev(dev)->bd_make_it_fail = i;
-
+	if (count > 0 && sscanf(buf, "%d", &i) > 0) {
+		if (i)
+			bdev_set_flag(dev_to_bdev(dev), BD_MAKE_IT_FAIL);
+		else
+			bdev_clear_flag(dev_to_bdev(dev), BD_MAKE_IT_FAIL);
+	}
 	return count;
 }
 
diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
index 4e0c8785090c..5bb7805927ac 100644
--- a/include/linux/blk_types.h
+++ b/include/linux/blk_types.h
@@ -51,6 +51,9 @@ struct block_device {
 #define BD_WRITE_HOLDER		(1u<<9)
 #define BD_HAS_SUBMIT_BIO	(1u<<10)
 #define BD_RO_WARNED		(1u<<11)
+#ifdef CONFIG_FAIL_MAKE_REQUEST
+#define BD_MAKE_IT_FAIL		(1u<<12)
+#endif
 	dev_t			bd_dev;
 	struct inode		*bd_inode;	/* will die */
 
@@ -67,9 +70,6 @@ struct block_device {
 	struct mutex		bd_fsfreeze_mutex; /* serialize freeze/thaw */
 
 	struct partition_meta_info *bd_meta_info;
-#ifdef CONFIG_FAIL_MAKE_REQUEST
-	bool			bd_make_it_fail;
-#endif
 	int			bd_writers;
 	/*
 	 * keep this out-of-line as it's both big and not needed in the fast
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2024-05-03  0:11 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-28  5:12 [PATCHES][RFC] packing struct block_device flags Al Viro
2024-04-28  5:14 ` [PATCH 1/8] Use bdev_is_paritition() instead of open-coding it Al Viro
2024-04-29  5:19   ` Christoph Hellwig
2024-04-28  5:15 ` [PATCH 2/8] wrapper for access to ->bd_partno Al Viro
2024-04-28  5:16 ` [PATCH 3/8] bdev: infrastructure for flags Al Viro
2024-04-28  5:17 ` [PATCH 4/8] bdev: move ->bd_read_only to ->__bd_flags Al Viro
2024-04-28  5:18 ` [PATCH 5/8] bdev: move ->bd_write_holder into ->__bd_flags Al Viro
2024-04-28  5:19 ` [PATCH 6/8] bdev: move ->bd_has_subit_bio to ->__bd_flags Al Viro
2024-04-28  5:19 ` [PATCH 7/8] bdev: move ->bd_ro_warned " Al Viro
2024-04-28  5:21 ` [PATCH 8/8] bdev: move ->bd_make_it_fail " Al Viro
2024-04-29  5:23 ` [PATCHES][RFC] packing struct block_device flags Christoph Hellwig
2024-04-29  7:31   ` Al Viro
2024-04-29 17:02     ` Al Viro
2024-04-29 18:13       ` Al Viro
2024-04-29 18:30         ` Al Viro
2024-05-03  0:06           ` Al Viro
2024-05-03  0:07             ` [PATCH 1/8] Use bdev_is_paritition() instead of open-coding it Al Viro
2024-05-03  0:07             ` [PATCH 2/8] wrapper for access to ->bd_partno Al Viro
2024-05-03  0:08             ` [PATCH v2 3/8] bdev: infrastructure for flags Al Viro
2024-05-03  0:09             ` [PATCH v2 4/8] bdev: move ->bd_read_only to ->__bd_flags Al Viro
2024-05-03  0:09             ` [PATCH v2 5/8] bdev: move ->bd_write_holder into ->__bd_flags Al Viro
2024-05-03  0:10             ` [PATCH v2 6/8] bdev: move ->bd_has_subit_bio to ->__bd_flags Al Viro
2024-05-03  0:10             ` [PATCH v2 7/8] bdev: move ->bd_ro_warned " Al Viro
2024-05-03  0:11             ` [PATCH v2 8/8] bdev: move ->bd_make_it_fail " Al Viro

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.