* [PATCH v2 0/2] block: warn once for each partition in bio_check_ro() @ 2023-11-21 11:19 Yu Kuai 2023-11-21 11:19 ` [PATCH v2 1/2] block: introduce new field flags in block_device Yu Kuai 2023-11-21 11:19 ` [PATCH v2 2/2] block: warn once for each partition in bio_check_ro() Yu Kuai 0 siblings, 2 replies; 5+ messages in thread From: Yu Kuai @ 2023-11-21 11:19 UTC (permalink / raw) To: ming.lei, axboe Cc: linux-block, linux-kernel, yukuai3, yukuai1, yi.zhang, yangerkun From: Yu Kuai <yukuai3@huawei.com> Changes in v2: - don't use test/set_bit() for new field, because unsigned long will cause that some field can't be placed in the first cacheline(64 bytes), use unsigned int for new field and test/set/clear it like 'bio->bi_flags'. Yu Kuai (2): block: introduce new field flags in block_device block: warn once for each partition in bio_check_ro() block/bdev.c | 15 ++++++++------- block/blk-core.c | 21 +++++++++++++++------ block/genhd.c | 15 +++++++++++---- block/ioctl.c | 6 +++++- include/linux/blk_types.h | 28 ++++++++++++++++++++++------ include/linux/blkdev.h | 5 +++-- 6 files changed, 64 insertions(+), 26 deletions(-) -- 2.39.2 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2 1/2] block: introduce new field flags in block_device 2023-11-21 11:19 [PATCH v2 0/2] block: warn once for each partition in bio_check_ro() Yu Kuai @ 2023-11-21 11:19 ` Yu Kuai 2023-11-21 9:33 ` Ming Lei 2023-11-21 11:19 ` [PATCH v2 2/2] block: warn once for each partition in bio_check_ro() Yu Kuai 1 sibling, 1 reply; 5+ messages in thread From: Yu Kuai @ 2023-11-21 11:19 UTC (permalink / raw) To: ming.lei, axboe Cc: linux-block, linux-kernel, yukuai3, yukuai1, yi.zhang, yangerkun From: Yu Kuai <yukuai3@huawei.com> There are multiple switches in struct block_device, use separate bool fields for them is not gracefully. Add a new field flags and replace swithes to a bit, there are no functional changes, and preare to add a new switch in the next patch. In order to keep flags in the first cacheline, define it as u32 and add new helpers to set/clear/test each bit like 'bio->bi_flags'. Signed-off-by: Yu Kuai <yukuai3@huawei.com> --- block/bdev.c | 15 ++++++++------- block/blk-core.c | 7 ++++--- block/genhd.c | 15 +++++++++++---- block/ioctl.c | 6 +++++- include/linux/blk_types.h | 27 +++++++++++++++++++++------ include/linux/blkdev.h | 5 +++-- 6 files changed, 52 insertions(+), 23 deletions(-) diff --git a/block/bdev.c b/block/bdev.c index fc8d28d77495..6b420cbd4f3d 100644 --- a/block/bdev.c +++ b/block/bdev.c @@ -408,10 +408,10 @@ struct block_device *bdev_alloc(struct gendisk *disk, u8 partno) bdev->bd_partno = partno; bdev->bd_inode = inode; bdev->bd_queue = disk->queue; - if (partno) - bdev->bd_has_submit_bio = disk->part0->bd_has_submit_bio; + if (partno && bdev_flagged(disk->part0, BD_FLAG_HAS_SUBMIT_BIO)) + bdev_set_flag(bdev, BD_FLAG_HAS_SUBMIT_BIO); else - bdev->bd_has_submit_bio = false; + bdev_clear_flag(bdev, BD_FLAG_HAS_SUBMIT_BIO); bdev->bd_stats = alloc_percpu(struct disk_stats); if (!bdev->bd_stats) { iput(inode); @@ -612,7 +612,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_flagged(bdev, BD_FLAG_WRITE_HOLDER)) unblock = true; } if (!whole->bd_holders) @@ -625,7 +625,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_FLAG_WRITE_HOLDER); } } @@ -878,9 +878,10 @@ struct bdev_handle *bdev_open_by_dev(dev_t dev, 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_flagged(bdev, BD_FLAG_WRITE_HOLDER) && (disk->event_flags & DISK_EVENT_FLAG_BLOCK_ON_EXCL_WRITE)) { - bdev->bd_write_holder = true; + bdev_set_flag(bdev, BD_FLAG_WRITE_HOLDER); unblock_events = false; } } diff --git a/block/blk-core.c b/block/blk-core.c index fdf25b8d6e78..f9f8b12ba626 100644 --- a/block/blk-core.c +++ b/block/blk-core.c @@ -482,7 +482,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_flagged(part, BD_FLAG_MAKE_IT_FAIL) && + should_fail(&fail_make_request, bytes); } static int __init fail_make_request_debugfs(void) @@ -595,7 +596,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_flagged(bio->bi_bdev, BD_FLAG_HAS_SUBMIT_BIO)) { blk_mq_submit_bio(bio); } else if (likely(bio_queue_enter(bio) == 0)) { struct gendisk *disk = bio->bi_bdev->bd_disk; @@ -703,7 +704,7 @@ void submit_bio_noacct_nocheck(struct bio *bio) */ if (current->bio_list) bio_list_add(¤t->bio_list[0], bio); - else if (!bio->bi_bdev->bd_has_submit_bio) + else if (!bdev_flagged(bio->bi_bdev, BD_FLAG_HAS_SUBMIT_BIO)) __submit_bio_noacct_mq(bio); else __submit_bio_noacct(bio); diff --git a/block/genhd.c b/block/genhd.c index c9d06f72c587..57f96c0c8da0 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_FLAG_HAS_SUBMIT_BIO); /* * If the driver provides an explicit major number it also must provide @@ -1062,7 +1063,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_flagged(dev_to_bdev(dev), BD_FLAG_MAKE_IT_FAIL)); } ssize_t part_fail_store(struct device *dev, @@ -1071,8 +1073,13 @@ 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_clear_flag(dev_to_bdev(dev), BD_FLAG_MAKE_IT_FAIL); + else + bdev_set_flag(dev_to_bdev(dev), BD_FLAG_MAKE_IT_FAIL); + + } return count; } diff --git a/block/ioctl.c b/block/ioctl.c index 4160f4e6bd5b..a64440f4c96b 100644 --- a/block/ioctl.c +++ b/block/ioctl.c @@ -394,7 +394,11 @@ static int blkdev_roset(struct block_device *bdev, unsigned cmd, if (ret) return ret; } - bdev->bd_read_only = n; + + if (!n) + bdev_clear_flag(bdev, BD_FLAG_READ_ONLY); + else + bdev_set_flag(bdev, BD_FLAG_READ_ONLY); return 0; } diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h index 52e264d5a830..f7a7965eeb25 100644 --- a/include/linux/blk_types.h +++ b/include/linux/blk_types.h @@ -37,6 +37,11 @@ struct bio_crypt_ctx; #define PAGE_SECTORS (1 << PAGE_SECTORS_SHIFT) #define SECTOR_MASK (PAGE_SECTORS - 1) +#define BD_FLAG_READ_ONLY 0 /* read-only-policy */ +#define BD_FLAG_WRITE_HOLDER 1 +#define BD_FLAG_HAS_SUBMIT_BIO 2 +#define BD_FLAG_MAKE_IT_FAIL 3 + struct block_device { sector_t bd_start_sect; sector_t bd_nr_sectors; @@ -44,10 +49,8 @@ struct block_device { struct request_queue * bd_queue; struct disk_stats __percpu *bd_stats; unsigned long bd_stamp; - bool bd_read_only; /* read-only policy */ + unsigned int bd_flags; u8 bd_partno; - bool bd_write_holder; - bool bd_has_submit_bio; dev_t bd_dev; atomic_t bd_openers; spinlock_t bd_size_lock; /* for bd_inode->i_size updates */ @@ -63,9 +66,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 @@ -83,6 +83,21 @@ struct block_device { #define bdev_kobj(_bdev) \ (&((_bdev)->bd_device.kobj)) +static inline bool bdev_flagged(struct block_device *bdev, unsigned int bit) +{ + return bdev->bd_flags & (1U << bit); +} + +static inline void bdev_set_flag(struct block_device *bdev, unsigned int bit) +{ + bdev->bd_flags |= (1U << bit); +} + +static inline void bdev_clear_flag(struct block_device *bdev, unsigned int bit) +{ + bdev->bd_flags &= ~(1U << bit); +} + /* * 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 9f6c3373f9fc..be1a39c0f8db 100644 --- a/include/linux/blkdev.h +++ b/include/linux/blkdev.h @@ -743,13 +743,14 @@ void disk_uevent(struct gendisk *disk, enum kobject_action action); static inline int get_disk_ro(struct gendisk *disk) { - return disk->part0->bd_read_only || + return bdev_flagged(disk->part0, BD_FLAG_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_flagged(bdev, BD_FLAG_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] 5+ messages in thread
* Re: [PATCH v2 1/2] block: introduce new field flags in block_device 2023-11-21 11:19 ` [PATCH v2 1/2] block: introduce new field flags in block_device Yu Kuai @ 2023-11-21 9:33 ` Ming Lei 2023-11-21 11:06 ` Yu Kuai 0 siblings, 1 reply; 5+ messages in thread From: Ming Lei @ 2023-11-21 9:33 UTC (permalink / raw) To: Yu Kuai; +Cc: axboe, linux-block, linux-kernel, yukuai3, yi.zhang, yangerkun On Tue, Nov 21, 2023 at 07:19:26PM +0800, Yu Kuai wrote: > From: Yu Kuai <yukuai3@huawei.com> > > There are multiple switches in struct block_device, use separate bool > fields for them is not gracefully. Add a new field flags and replace > swithes to a bit, there are no functional changes, and preare to add > a new switch in the next patch. > > In order to keep flags in the first cacheline, define it as u32 and add > new helpers to set/clear/test each bit like 'bio->bi_flags'. > > Signed-off-by: Yu Kuai <yukuai3@huawei.com> > --- > block/bdev.c | 15 ++++++++------- > block/blk-core.c | 7 ++++--- > block/genhd.c | 15 +++++++++++---- > block/ioctl.c | 6 +++++- > include/linux/blk_types.h | 27 +++++++++++++++++++++------ > include/linux/blkdev.h | 5 +++-- > 6 files changed, 52 insertions(+), 23 deletions(-) > > diff --git a/block/bdev.c b/block/bdev.c > index fc8d28d77495..6b420cbd4f3d 100644 > --- a/block/bdev.c > +++ b/block/bdev.c > @@ -408,10 +408,10 @@ struct block_device *bdev_alloc(struct gendisk *disk, u8 partno) > bdev->bd_partno = partno; > bdev->bd_inode = inode; > bdev->bd_queue = disk->queue; > - if (partno) > - bdev->bd_has_submit_bio = disk->part0->bd_has_submit_bio; > + if (partno && bdev_flagged(disk->part0, BD_FLAG_HAS_SUBMIT_BIO)) > + bdev_set_flag(bdev, BD_FLAG_HAS_SUBMIT_BIO); > else > - bdev->bd_has_submit_bio = false; > + bdev_clear_flag(bdev, BD_FLAG_HAS_SUBMIT_BIO); > bdev->bd_stats = alloc_percpu(struct disk_stats); > if (!bdev->bd_stats) { > iput(inode); > @@ -612,7 +612,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_flagged(bdev, BD_FLAG_WRITE_HOLDER)) > unblock = true; > } > if (!whole->bd_holders) > @@ -625,7 +625,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_FLAG_WRITE_HOLDER); > } > } > > @@ -878,9 +878,10 @@ struct bdev_handle *bdev_open_by_dev(dev_t dev, 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_flagged(bdev, BD_FLAG_WRITE_HOLDER) && > (disk->event_flags & DISK_EVENT_FLAG_BLOCK_ON_EXCL_WRITE)) { > - bdev->bd_write_holder = true; > + bdev_set_flag(bdev, BD_FLAG_WRITE_HOLDER); > unblock_events = false; > } > } > diff --git a/block/blk-core.c b/block/blk-core.c > index fdf25b8d6e78..f9f8b12ba626 100644 > --- a/block/blk-core.c > +++ b/block/blk-core.c > @@ -482,7 +482,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_flagged(part, BD_FLAG_MAKE_IT_FAIL) && > + should_fail(&fail_make_request, bytes); > } > > static int __init fail_make_request_debugfs(void) > @@ -595,7 +596,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_flagged(bio->bi_bdev, BD_FLAG_HAS_SUBMIT_BIO)) { > blk_mq_submit_bio(bio); > } else if (likely(bio_queue_enter(bio) == 0)) { > struct gendisk *disk = bio->bi_bdev->bd_disk; > @@ -703,7 +704,7 @@ void submit_bio_noacct_nocheck(struct bio *bio) > */ > if (current->bio_list) > bio_list_add(¤t->bio_list[0], bio); > - else if (!bio->bi_bdev->bd_has_submit_bio) > + else if (!bdev_flagged(bio->bi_bdev, BD_FLAG_HAS_SUBMIT_BIO)) > __submit_bio_noacct_mq(bio); > else > __submit_bio_noacct(bio); > diff --git a/block/genhd.c b/block/genhd.c > index c9d06f72c587..57f96c0c8da0 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_FLAG_HAS_SUBMIT_BIO); > > /* > * If the driver provides an explicit major number it also must provide > @@ -1062,7 +1063,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_flagged(dev_to_bdev(dev), BD_FLAG_MAKE_IT_FAIL)); > } > > ssize_t part_fail_store(struct device *dev, > @@ -1071,8 +1073,13 @@ 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_clear_flag(dev_to_bdev(dev), BD_FLAG_MAKE_IT_FAIL); > + else > + bdev_set_flag(dev_to_bdev(dev), BD_FLAG_MAKE_IT_FAIL); > + > + } > > return count; > } > diff --git a/block/ioctl.c b/block/ioctl.c > index 4160f4e6bd5b..a64440f4c96b 100644 > --- a/block/ioctl.c > +++ b/block/ioctl.c > @@ -394,7 +394,11 @@ static int blkdev_roset(struct block_device *bdev, unsigned cmd, > if (ret) > return ret; > } > - bdev->bd_read_only = n; > + > + if (!n) > + bdev_clear_flag(bdev, BD_FLAG_READ_ONLY); > + else > + bdev_set_flag(bdev, BD_FLAG_READ_ONLY); > return 0; > } > > diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h > index 52e264d5a830..f7a7965eeb25 100644 > --- a/include/linux/blk_types.h > +++ b/include/linux/blk_types.h > @@ -37,6 +37,11 @@ struct bio_crypt_ctx; > #define PAGE_SECTORS (1 << PAGE_SECTORS_SHIFT) > #define SECTOR_MASK (PAGE_SECTORS - 1) > > +#define BD_FLAG_READ_ONLY 0 /* read-only-policy */ > +#define BD_FLAG_WRITE_HOLDER 1 > +#define BD_FLAG_HAS_SUBMIT_BIO 2 > +#define BD_FLAG_MAKE_IT_FAIL 3 > + > struct block_device { > sector_t bd_start_sect; > sector_t bd_nr_sectors; > @@ -44,10 +49,8 @@ struct block_device { > struct request_queue * bd_queue; > struct disk_stats __percpu *bd_stats; > unsigned long bd_stamp; > - bool bd_read_only; /* read-only policy */ > + unsigned int bd_flags; > u8 bd_partno; > - bool bd_write_holder; > - bool bd_has_submit_bio; > dev_t bd_dev; > atomic_t bd_openers; > spinlock_t bd_size_lock; /* for bd_inode->i_size updates */ As I mentioned, you can convert the above/original 4 fields into single 'unsigned int', otherwise one extra 'int' filed is added. Then, 'bd_size_lock' is moved to the 2nd cache line, which isn't big deal since the field isn't touched in fast path. Anyway, it can be one future cleanup, so: Reviewed-by: Ming Lei <ming.lei@redhat.com> Thanks, Ming ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 1/2] block: introduce new field flags in block_device 2023-11-21 9:33 ` Ming Lei @ 2023-11-21 11:06 ` Yu Kuai 0 siblings, 0 replies; 5+ messages in thread From: Yu Kuai @ 2023-11-21 11:06 UTC (permalink / raw) To: Ming Lei, Yu Kuai Cc: axboe, linux-block, linux-kernel, yi.zhang, yangerkun, yukuai (C) Hi, 在 2023/11/21 17:33, Ming Lei 写道: > On Tue, Nov 21, 2023 at 07:19:26PM +0800, Yu Kuai wrote: >> From: Yu Kuai <yukuai3@huawei.com> >> >> There are multiple switches in struct block_device, use separate bool >> fields for them is not gracefully. Add a new field flags and replace >> swithes to a bit, there are no functional changes, and preare to add >> a new switch in the next patch. >> >> In order to keep flags in the first cacheline, define it as u32 and add >> new helpers to set/clear/test each bit like 'bio->bi_flags'. >> >> Signed-off-by: Yu Kuai <yukuai3@huawei.com> >> --- >> block/bdev.c | 15 ++++++++------- >> block/blk-core.c | 7 ++++--- >> block/genhd.c | 15 +++++++++++---- >> block/ioctl.c | 6 +++++- >> include/linux/blk_types.h | 27 +++++++++++++++++++++------ >> include/linux/blkdev.h | 5 +++-- >> 6 files changed, 52 insertions(+), 23 deletions(-) >> >> diff --git a/block/bdev.c b/block/bdev.c >> index fc8d28d77495..6b420cbd4f3d 100644 >> --- a/block/bdev.c >> +++ b/block/bdev.c >> @@ -408,10 +408,10 @@ struct block_device *bdev_alloc(struct gendisk *disk, u8 partno) >> bdev->bd_partno = partno; >> bdev->bd_inode = inode; >> bdev->bd_queue = disk->queue; >> - if (partno) >> - bdev->bd_has_submit_bio = disk->part0->bd_has_submit_bio; >> + if (partno && bdev_flagged(disk->part0, BD_FLAG_HAS_SUBMIT_BIO)) >> + bdev_set_flag(bdev, BD_FLAG_HAS_SUBMIT_BIO); >> else >> - bdev->bd_has_submit_bio = false; >> + bdev_clear_flag(bdev, BD_FLAG_HAS_SUBMIT_BIO); >> bdev->bd_stats = alloc_percpu(struct disk_stats); >> if (!bdev->bd_stats) { >> iput(inode); >> @@ -612,7 +612,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_flagged(bdev, BD_FLAG_WRITE_HOLDER)) >> unblock = true; >> } >> if (!whole->bd_holders) >> @@ -625,7 +625,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_FLAG_WRITE_HOLDER); >> } >> } >> >> @@ -878,9 +878,10 @@ struct bdev_handle *bdev_open_by_dev(dev_t dev, 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_flagged(bdev, BD_FLAG_WRITE_HOLDER) && >> (disk->event_flags & DISK_EVENT_FLAG_BLOCK_ON_EXCL_WRITE)) { >> - bdev->bd_write_holder = true; >> + bdev_set_flag(bdev, BD_FLAG_WRITE_HOLDER); >> unblock_events = false; >> } >> } >> diff --git a/block/blk-core.c b/block/blk-core.c >> index fdf25b8d6e78..f9f8b12ba626 100644 >> --- a/block/blk-core.c >> +++ b/block/blk-core.c >> @@ -482,7 +482,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_flagged(part, BD_FLAG_MAKE_IT_FAIL) && >> + should_fail(&fail_make_request, bytes); >> } >> >> static int __init fail_make_request_debugfs(void) >> @@ -595,7 +596,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_flagged(bio->bi_bdev, BD_FLAG_HAS_SUBMIT_BIO)) { >> blk_mq_submit_bio(bio); >> } else if (likely(bio_queue_enter(bio) == 0)) { >> struct gendisk *disk = bio->bi_bdev->bd_disk; >> @@ -703,7 +704,7 @@ void submit_bio_noacct_nocheck(struct bio *bio) >> */ >> if (current->bio_list) >> bio_list_add(¤t->bio_list[0], bio); >> - else if (!bio->bi_bdev->bd_has_submit_bio) >> + else if (!bdev_flagged(bio->bi_bdev, BD_FLAG_HAS_SUBMIT_BIO)) >> __submit_bio_noacct_mq(bio); >> else >> __submit_bio_noacct(bio); >> diff --git a/block/genhd.c b/block/genhd.c >> index c9d06f72c587..57f96c0c8da0 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_FLAG_HAS_SUBMIT_BIO); >> >> /* >> * If the driver provides an explicit major number it also must provide >> @@ -1062,7 +1063,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_flagged(dev_to_bdev(dev), BD_FLAG_MAKE_IT_FAIL)); >> } >> >> ssize_t part_fail_store(struct device *dev, >> @@ -1071,8 +1073,13 @@ 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_clear_flag(dev_to_bdev(dev), BD_FLAG_MAKE_IT_FAIL); >> + else >> + bdev_set_flag(dev_to_bdev(dev), BD_FLAG_MAKE_IT_FAIL); >> + >> + } >> >> return count; >> } >> diff --git a/block/ioctl.c b/block/ioctl.c >> index 4160f4e6bd5b..a64440f4c96b 100644 >> --- a/block/ioctl.c >> +++ b/block/ioctl.c >> @@ -394,7 +394,11 @@ static int blkdev_roset(struct block_device *bdev, unsigned cmd, >> if (ret) >> return ret; >> } >> - bdev->bd_read_only = n; >> + >> + if (!n) >> + bdev_clear_flag(bdev, BD_FLAG_READ_ONLY); >> + else >> + bdev_set_flag(bdev, BD_FLAG_READ_ONLY); >> return 0; >> } >> >> diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h >> index 52e264d5a830..f7a7965eeb25 100644 >> --- a/include/linux/blk_types.h >> +++ b/include/linux/blk_types.h >> @@ -37,6 +37,11 @@ struct bio_crypt_ctx; >> #define PAGE_SECTORS (1 << PAGE_SECTORS_SHIFT) >> #define SECTOR_MASK (PAGE_SECTORS - 1) >> >> +#define BD_FLAG_READ_ONLY 0 /* read-only-policy */ >> +#define BD_FLAG_WRITE_HOLDER 1 >> +#define BD_FLAG_HAS_SUBMIT_BIO 2 >> +#define BD_FLAG_MAKE_IT_FAIL 3 >> + >> struct block_device { >> sector_t bd_start_sect; >> sector_t bd_nr_sectors; >> @@ -44,10 +49,8 @@ struct block_device { >> struct request_queue * bd_queue; >> struct disk_stats __percpu *bd_stats; >> unsigned long bd_stamp; >> - bool bd_read_only; /* read-only policy */ >> + unsigned int bd_flags; >> u8 bd_partno; >> - bool bd_write_holder; >> - bool bd_has_submit_bio; >> dev_t bd_dev; >> atomic_t bd_openers; >> spinlock_t bd_size_lock; /* for bd_inode->i_size updates */ > > As I mentioned, you can convert the above/original 4 fields into single > 'unsigned int', otherwise one extra 'int' filed is added. Then, 'bd_size_lock' > is moved to the 2nd cache line, which isn't big deal since the field isn't > touched in fast path. Anyway, it can be one future cleanup, so: Thanks for the explanation, if you don't mind, I can change bd_flags to unsigned short to keep this behaviour in the next version. Thanks, Kuai > > Reviewed-by: Ming Lei <ming.lei@redhat.com> > > Thanks, > Ming > > > . > ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2 2/2] block: warn once for each partition in bio_check_ro() 2023-11-21 11:19 [PATCH v2 0/2] block: warn once for each partition in bio_check_ro() Yu Kuai 2023-11-21 11:19 ` [PATCH v2 1/2] block: introduce new field flags in block_device Yu Kuai @ 2023-11-21 11:19 ` Yu Kuai 1 sibling, 0 replies; 5+ messages in thread From: Yu Kuai @ 2023-11-21 11:19 UTC (permalink / raw) To: ming.lei, axboe Cc: linux-block, linux-kernel, yukuai3, yukuai1, yi.zhang, yangerkun From: Yu Kuai <yukuai3@huawei.com> Commit 1b0a151c10a6 ("blk-core: use pr_warn_ratelimited() in bio_check_ro()") fix message storm by limit the rate, however, there will still be lots of message in the long term. Fix it better by warn once for each partition. Signed-off-by: Yu Kuai <yukuai3@huawei.com> --- block/blk-core.c | 14 +++++++++++--- include/linux/blk_types.h | 1 + 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/block/blk-core.c b/block/blk-core.c index f9f8b12ba626..6575f684d41e 100644 --- a/block/blk-core.c +++ b/block/blk-core.c @@ -502,9 +502,17 @@ static inline void bio_check_ro(struct bio *bio) if (op_is_write(bio_op(bio)) && bdev_read_only(bio->bi_bdev)) { if (op_is_flush(bio->bi_opf) && !bio_sectors(bio)) return; - pr_warn_ratelimited("Trying to write to read-only block-device %pg\n", - bio->bi_bdev); - /* Older lvm-tools actually trigger this */ + + if (bdev_flagged(bio->bi_bdev, BD_FLAG_RO_WARNED)) + return; + + bdev_set_flag(bio->bi_bdev, BD_FLAG_RO_WARNED); + /* + * Use ioctl to set underlying disk of raid/dm to read-only + * will trigger this. + */ + pr_warn("Trying to write to read-only block-device %pg\n", + bio->bi_bdev); } } diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h index f7a7965eeb25..07abd0165226 100644 --- a/include/linux/blk_types.h +++ b/include/linux/blk_types.h @@ -41,6 +41,7 @@ struct bio_crypt_ctx; #define BD_FLAG_WRITE_HOLDER 1 #define BD_FLAG_HAS_SUBMIT_BIO 2 #define BD_FLAG_MAKE_IT_FAIL 3 +#define BD_FLAG_RO_WARNED 4 struct block_device { sector_t bd_start_sect; -- 2.39.2 ^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-11-21 11:06 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-11-21 11:19 [PATCH v2 0/2] block: warn once for each partition in bio_check_ro() Yu Kuai 2023-11-21 11:19 ` [PATCH v2 1/2] block: introduce new field flags in block_device Yu Kuai 2023-11-21 9:33 ` Ming Lei 2023-11-21 11:06 ` Yu Kuai 2023-11-21 11:19 ` [PATCH v2 2/2] block: warn once for each partition in bio_check_ro() Yu Kuai
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox