Re: [PATCHES][RFC] packing struct block_device flags

[Al Viro <viro@xxxxxxxxxxxxxxxxxx>]

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)