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

Al Viro
2024-04-29T17:02:13Z

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)