Skip to contentSkip to conversation

packing struct block_device flags

RFC PATCH In review Al Viro 2 participants 24 messages Apr 28, 2024 – May 3, 2024

Subsystems (4)
VFS Block layer Library code Vsprintf
lore
Share & tools
lore

Al Viro

author maintainer
#0 lore raw

RFC PATCH 1/1 packing struct block_device flags

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 <[email protected]> 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(-)
author maintainer
#1 lore raw

PATCH 1/8 Use bdev_is_paritition() instead of open-coding it

Signed-off-by Al Viro <[email protected]>
---
 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(-)
block/blk-core.c +3 -2
index a16b5abdbbf5..20322abc6082 100644
@@ -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;
 	}
block/blk-mq.c +1 -1
index 32afb87efbd0..43bb8f50a07c 100644
@@ -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)]++;
include/linux/part_stat.h +1 -1
index abeba356bc3f..ac8c44dd8237 100644
@@ -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)
lib/vsprintf.c +1 -1
index 552738f14275..3f9f1b959ef0 100644
@@ -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';
Signature · 1 line
2.39.2
author maintainer
#3 lore raw

PATCH 2/8 wrapper for access to ->bd_partno

In reply to Al Viro

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

Signed-off-by Al Viro <[email protected]>
---
 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(-)
block/early-lookup.c +1 -1
index 3effbd0d35e9..3fb57f7d2b12 100644
@@ -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;
 	}
block/partitions/core.c +6 -6
index b11e88c82c8c..edd5309dc4ba 100644
@@ -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;
include/linux/blkdev.h +6 -1
index c3e8f7cf96be..32549d675955 100644
@@ -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 {
lib/vsprintf.c +1 -1
index 3f9f1b959ef0..cdd4e2314bfc 100644
@@ -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;
 }
Signature · 1 line
2.39.2
author maintainer
#4 lore raw

PATCH 3/8 bdev: infrastructure for flags

In reply to Al Viro

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 <[email protected]>
---
 block/bdev.c              |  2 +-
 include/linux/blk_types.h |  2 +-
 include/linux/blkdev.h    | 33 ++++++++++++++++++++++++++++++++-
 3 files changed, 34 insertions(+), 3 deletions(-)
block/bdev.c +1 -1
index 7a5f611c3d2e..0c55f7abaac3 100644
@@ -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)
include/linux/blk_types.h +1 -1
index cb1526ec44b5..274f7eb4138f 100644
@@ -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;
include/linux/blkdev.h +32 -1
index 32549d675955..b9dbfb811533 100644
@@ -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)
Signature · 1 line
2.39.2
author maintainer
#5 lore raw

PATCH 4/8 bdev: move ->bd_read_only to ->__bd_flags

In reply to Al Viro
Signed-off-by Al Viro <[email protected]>
---
 block/ioctl.c             | 5 ++++-
 include/linux/blk_types.h | 5 ++++-
 include/linux/blkdev.h    | 4 ++--
 3 files changed, 10 insertions(+), 4 deletions(-)
block/ioctl.c +4 -1
index 0c76137adcaa..be173e4ff43d 100644
@@ -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;
 }
include/linux/blk_types.h +4 -1
index 274f7eb4138f..1aa41b651614 100644
@@ -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.
include/linux/blkdev.h +2 -2
index b9dbfb811533..d556cec9224b 100644
@@ -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);
Signature · 1 line
2.39.2
author maintainer
#6 lore raw

PATCH 5/8 bdev: move ->bd_write_holder into ->__bd_flags

In reply to Al Viro
Signed-off-by Al Viro <[email protected]>
---
 block/bdev.c              | 9 +++++----
 include/linux/blk_types.h | 2 +-
 2 files changed, 6 insertions(+), 5 deletions(-)
block/bdev.c +5 -4
index 0c55f7abaac3..24c1dd6de8a9 100644
@@ -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;
 		}
 	}
include/linux/blk_types.h +1 -1
index 1aa41b651614..8a336053b5fa 100644
@@ -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,
 };
 
 /*
Signature · 1 line
2.39.2
author maintainer
#7 lore raw

PATCH 6/8 bdev: move ->bd_has_subit_bio to ->__bd_flags

In reply to Al Viro
Signed-off-by Al Viro <[email protected]>
---
 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(-)
block/bdev.c +2 -4
index 24c1dd6de8a9..9aa23620fe92 100644
@@ -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);
block/blk-core.c +2 -2
index 20322abc6082..f61460b65408 100644
@@ -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);
block/genhd.c +2 -1
index bb29a68e1d67..19cd1a31fa80 100644
@@ -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
include/linux/blk_types.h +1 -1
index 8a336053b5fa..c8f5364b24f1 100644
@@ -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,
 };
 
 /*
Signature · 1 line
2.39.2
author maintainer
#8 lore raw

PATCH 7/8 bdev: move ->bd_ro_warned to ->__bd_flags

In reply to Al Viro
Signed-off-by Al Viro <[email protected]>
---
 block/blk-core.c          | 5 +++--
 include/linux/blk_types.h | 2 +-
 2 files changed, 4 insertions(+), 3 deletions(-)
block/blk-core.c +3 -2
index f61460b65408..1be49be9fac4 100644
@@ -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.
include/linux/blk_types.h +1 -1
index c8f5364b24f1..59de93913cc4 100644
@@ -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,
 };
 
 /*
Signature · 1 line
2.39.2
author maintainer
#9 lore raw

PATCH 8/8 bdev: move ->bd_make_it_fail to ->__bd_flags

In reply to Al Viro
Signed-off-by Al Viro <[email protected]>
---
 block/blk-core.c          |  3 ++-
 block/genhd.c             | 12 ++++++++----
 include/linux/blk_types.h |  6 +++---
 3 files changed, 13 insertions(+), 8 deletions(-)
block/blk-core.c +2 -1
index 1be49be9fac4..1076336dd620 100644
@@ -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)
block/genhd.c +8 -4
index 19cd1a31fa80..0cce461952f6 100644
@@ -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;
 }
include/linux/blk_types.h +3 -3
index 59de93913cc4..98e1c2d28d60 100644
@@ -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
 };
 
 /*
Signature · 1 line
2.39.2
#10 lore raw
In reply to Al Viro
Quoted 10 lines · Al Viro

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 <[email protected]> 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.

Quoted 5 lines
	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.

author maintainer
#11 lore raw
Quoted 11 lines · Christoph Hellwig
Quoted 10 lines · Al Viro

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 <[email protected]> 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.

Quoted 7 lines
Quoted 5 lines
	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.

Quoted 9 lines

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.

author maintainer
#12 lore raw
Quoted 6 lines · Al Viro

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:

block/bdev.c +1 -1
index 9aa23620fe92..fae30eae7741 100644
@@ -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))
include/linux/blk_types.h +1 -1
index 98e1c2d28d60..a822911e28e5 100644
@@ -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 */
include/linux/blkdev.h +4 -20
index d556cec9224b..a8271497ac62 100644
@@ -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)
author maintainer
#13 lore raw
Quoted 9 lines · Al Viro
Quoted 6 lines · Al Viro

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.

Quoted 3 lines

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:

block/bdev.c +1 -1
index 9aa23620fe92..fae30eae7741 100644
@@ -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))
include/linux/blk_types.h +9 -11
index 98e1c2d28d60..84bfc702269a 100644
@@ -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.
include/linux/blkdev.h +7 -23
index d556cec9224b..1fe91231f85b 100644
@@ -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)
author maintainer
#14 lore raw
Quoted 10 lines · Al Viro
Quoted 9 lines · Al Viro
Quoted 6 lines · Al Viro

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.

Quoted 5 lines
Quoted 3 lines

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.

author maintainer
#15 lore raw
Quoted 11 lines · Al Viro
Quoted 10 lines · Al Viro
Quoted 9 lines · Al Viro
Quoted 6 lines · Al Viro

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.

Quoted 7 lines
Quoted 5 lines
Quoted 3 lines

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.

author maintainer
#16 lore raw

PATCH 1/8 Use bdev_is_paritition() instead of open-coding it

Reviewed-by Christoph Hellwig <[email protected]>
Signed-off-by Al Viro <[email protected]>
---
 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(-)
block/blk-core.c +3 -2
index a16b5abdbbf5..20322abc6082 100644
@@ -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;
 	}
block/blk-mq.c +1 -1
index 32afb87efbd0..43bb8f50a07c 100644
@@ -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)]++;
include/linux/part_stat.h +1 -1
index abeba356bc3f..ac8c44dd8237 100644
@@ -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)
lib/vsprintf.c +1 -1
index 552738f14275..3f9f1b959ef0 100644
@@ -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';
Signature · 1 line
2.39.2
author maintainer
#17 lore raw

PATCH 2/8 wrapper for access to ->bd_partno

In reply to Al Viro

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

Signed-off-by Al Viro <[email protected]>
---
 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(-)
block/early-lookup.c +1 -1
index 3effbd0d35e9..3fb57f7d2b12 100644
@@ -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;
 	}
block/partitions/core.c +6 -6
index b11e88c82c8c..edd5309dc4ba 100644
@@ -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;
include/linux/blkdev.h +6 -1
index c3e8f7cf96be..32549d675955 100644
@@ -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 {
lib/vsprintf.c +1 -1
index 3f9f1b959ef0..cdd4e2314bfc 100644
@@ -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;
 }
Signature · 1 line
2.39.2
author maintainer
#18 lore raw

PATCH v2 3/8 bdev: infrastructure for flags

In reply to Al Viro

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 <[email protected]>
---
 block/bdev.c              |  2 +-
 include/linux/blk_types.h |  3 ++-
 include/linux/blkdev.h    | 17 ++++++++++++++++-
 3 files changed, 19 insertions(+), 3 deletions(-)
block/bdev.c +1 -1
index 7a5f611c3d2e..2ec223315500 100644
@@ -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)
include/linux/blk_types.h +2 -1
index cb1526ec44b5..04f92737ab08 100644
@@ -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;
include/linux/blkdev.h +16 -1
index 32549d675955..99917e5860fd 100644
@@ -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)
Signature · 1 line
2.39.2
author maintainer
#19 lore raw

PATCH v2 4/8 bdev: move ->bd_read_only to ->__bd_flags

In reply to Al Viro
Signed-off-by Al Viro <[email protected]>
---
 block/ioctl.c             | 5 ++++-
 include/linux/blk_types.h | 2 +-
 include/linux/blkdev.h    | 4 ++--
 3 files changed, 7 insertions(+), 4 deletions(-)
block/ioctl.c +4 -1
index 0c76137adcaa..be173e4ff43d 100644
@@ -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;
 }
include/linux/blk_types.h +1 -1
index 04f92737ab08..f70dd31cbcd1 100644
@@ -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;
include/linux/blkdev.h +2 -2
index 99917e5860fd..1fe91231f85b 100644
@@ -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);
Signature · 1 line
2.39.2
author maintainer
#20 lore raw

PATCH v2 5/8 bdev: move ->bd_write_holder into ->__bd_flags

In reply to Al Viro
Signed-off-by Al Viro <[email protected]>
---
 block/bdev.c              | 9 +++++----
 include/linux/blk_types.h | 2 +-
 2 files changed, 6 insertions(+), 5 deletions(-)
block/bdev.c +5 -4
index 2ec223315500..9df9a59f0900 100644
@@ -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;
 		}
 	}
include/linux/blk_types.h +1 -1
index f70dd31cbcd1..e45a490d488e 100644
@@ -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 */
Signature · 1 line
2.39.2
author maintainer
#21 lore raw

PATCH v2 6/8 bdev: move ->bd_has_subit_bio to ->__bd_flags

In reply to Al Viro

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 <[email protected]>
---
 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(-)
block/bdev.c +2 -4
index 9df9a59f0900..fae30eae7741 100644
@@ -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);
block/blk-core.c +2 -2
index 20322abc6082..f61460b65408 100644
@@ -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);
block/genhd.c +2 -1
index bb29a68e1d67..19cd1a31fa80 100644
@@ -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
include/linux/blk_types.h +1 -1
index e45a490d488e..11b9e8eeb79f 100644
@@ -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 */
Signature · 1 line
2.39.2
author maintainer
#22 lore raw

PATCH v2 7/8 bdev: move ->bd_ro_warned to ->__bd_flags

In reply to Al Viro
Signed-off-by Al Viro <[email protected]>
---
 block/blk-core.c          | 5 +++--
 include/linux/blk_types.h | 2 +-
 2 files changed, 4 insertions(+), 3 deletions(-)
block/blk-core.c +3 -2
index f61460b65408..1be49be9fac4 100644
@@ -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.
include/linux/blk_types.h +1 -1
index 11b9e8eeb79f..4e0c8785090c 100644
@@ -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
Signature · 1 line
2.39.2
author maintainer
#23 lore raw

PATCH v2 8/8 bdev: move ->bd_make_it_fail to ->__bd_flags

In reply to Al Viro
Signed-off-by Al Viro <[email protected]>
---
 block/blk-core.c          |  3 ++-
 block/genhd.c             | 12 ++++++++----
 include/linux/blk_types.h |  6 +++---
 3 files changed, 13 insertions(+), 8 deletions(-)
block/blk-core.c +2 -1
index 1be49be9fac4..1076336dd620 100644
@@ -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)
block/genhd.c +8 -4
index 19cd1a31fa80..0cce461952f6 100644
@@ -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;
 }
include/linux/blk_types.h +3 -3
index 4e0c8785090c..5bb7805927ac 100644
@@ -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
Signature · 1 line
2.39.2