Re: [PATCH 26/27] block: decouple REQ_OP_SECURE_ERASE from REQ_OP_DISCARD
On 4/9/22 12:50 PM, Christoph Hellwig wrote: Secure erase is a very different operation from discard in that it is a data integrity operation vs hint. Fully split the limits and helper infrastructure to make the separation more clear. Signed-off-by: Christoph Hellwig Reviewed-by: Martin K. Petersen Acked-by: Christoph Böhmwalder [drbd] Acked-by: Ryusuke Konishi [nifs2] Acked-by: Coly Li [drbd] Hi Christoph, My ACK is for bcache, not drbd here. Thanks. Coly Li Acked-by: David Sterba [btrfs]
Re: [PATCH 22/27] block: refactor discard bio size limiting
On 4/6/22 2:05 PM, Christoph Hellwig wrote:
Move all the logic to limit the discard bio size into a common helper
so that it is better documented.
Signed-off-by: Christoph Hellwig
Acked-by: Coly Li
Thanks for the change.
Coly Li
---
block/blk-lib.c | 59 -
block/blk.h | 14
2 files changed, 29 insertions(+), 44 deletions(-)
diff --git a/block/blk-lib.c b/block/blk-lib.c
index 237d60d8b5857..2ae32a722851c 100644
--- a/block/blk-lib.c
+++ b/block/blk-lib.c
@@ -10,6 +10,32 @@
#include "blk.h"
+static sector_t bio_discard_limit(struct block_device *bdev, sector_t sector)
+{
+ unsigned int discard_granularity =
+ bdev_get_queue(bdev)->limits.discard_granularity;
+ sector_t granularity_aligned_sector;
+
+ if (bdev_is_partition(bdev))
+ sector += bdev->bd_start_sect;
+
+ granularity_aligned_sector =
+ round_up(sector, discard_granularity >> SECTOR_SHIFT);
+
+ /*
+* Make sure subsequent bios start aligned to the discard granularity if
+* it needs to be split.
+*/
+ if (granularity_aligned_sector != sector)
+ return granularity_aligned_sector - sector;
+
+ /*
+* Align the bio size to the discard granularity to make splitting the
bio
+* at discard granularity boundaries easier in the driver if needed.
+*/
+ return round_down(UINT_MAX, discard_granularity) >> SECTOR_SHIFT;
+}
+
int __blkdev_issue_discard(struct block_device *bdev, sector_t sector,
sector_t nr_sects, gfp_t gfp_mask, int flags,
struct bio **biop)
@@ -17,7 +43,7 @@ int __blkdev_issue_discard(struct block_device *bdev,
sector_t sector,
struct request_queue *q = bdev_get_queue(bdev);
struct bio *bio = *biop;
unsigned int op;
- sector_t bs_mask, part_offset = 0;
+ sector_t bs_mask;
if (bdev_read_only(bdev))
return -EPERM;
@@ -48,36 +74,9 @@ int __blkdev_issue_discard(struct block_device *bdev,
sector_t sector,
if (!nr_sects)
return -EINVAL;
- /* In case the discard request is in a partition */
- if (bdev_is_partition(bdev))
- part_offset = bdev->bd_start_sect;
-
while (nr_sects) {
- sector_t granularity_aligned_lba, req_sects;
- sector_t sector_mapped = sector + part_offset;
-
- granularity_aligned_lba = round_up(sector_mapped,
- q->limits.discard_granularity >> SECTOR_SHIFT);
-
- /*
-* Check whether the discard bio starts at a discard_granularity
-* aligned LBA,
-* - If no: set (granularity_aligned_lba - sector_mapped) to
-* bi_size of the first split bio, then the second bio will
-* start at a discard_granularity aligned LBA on the device.
-* - If yes: use bio_aligned_discard_max_sectors() as the max
-* possible bi_size of the first split bio. Then when this bio
-* is split in device drive, the split ones are very probably
-* to be aligned to discard_granularity of the device's queue.
-*/
- if (granularity_aligned_lba == sector_mapped)
- req_sects = min_t(sector_t, nr_sects,
- bio_aligned_discard_max_sectors(q));
- else
- req_sects = min_t(sector_t, nr_sects,
- granularity_aligned_lba -
sector_mapped);
-
- WARN_ON_ONCE((req_sects << 9) > UINT_MAX);
+ sector_t req_sects =
+ min(nr_sects, bio_discard_limit(bdev, sector));
bio = blk_next_bio(bio, bdev, 0, op, gfp_mask);
bio->bi_iter.bi_sector = sector;
diff --git a/block/blk.h b/block/blk.h
index 8ccbc6e076369..1fdc1d28e6d60 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -346,20 +346,6 @@ static inline unsigned int bio_allowed_max_sectors(struct
request_queue *q)
return round_down(UINT_MAX, queue_logical_block_size(q)) >> 9;
}
-/*
- * The max bio size which is aligned to q->limits.discard_granularity. This
- * is a hint to split large discard bio in generic block layer, then if device
- * driver needs to split the discard bio into smaller ones, their bi_size can
- * be very probably and easily aligned to discard_granularity of the device's
- * queue.
- */
-static inline unsigned int bio_aligned_discard_max_sectors(
- struct request_queue *q)
-{
- return round_down(UINT_MAX, q->limits.discard_granularity) >>
- SECTOR_SHIFT;
-}
-
/*
* Internal io_context interface
*/
Re: [PATCH 26/27] block: uncouple REQ_OP_SECURE_ERASE from REQ_OP_DISCARD
On 4/6/22 2:05 PM, Christoph Hellwig wrote: Secure erase is a very different operation from discard in that it is a data integrity operation vs hint. Fully split the limits and helper infrastructure to make the separation more clear. Signed-off-by: Christoph Hellwig For the bcache part, Acked-by: Coly Li Thanks. Coly Li --- block/blk-core.c| 2 +- block/blk-lib.c | 64 - block/blk-mq-debugfs.c | 1 - block/blk-settings.c| 16 +++- block/fops.c| 2 +- block/ioctl.c | 43 +++ drivers/block/drbd/drbd_receiver.c | 5 ++- drivers/block/rnbd/rnbd-clt.c | 4 +- drivers/block/rnbd/rnbd-srv-dev.h | 2 +- drivers/block/xen-blkback/blkback.c | 15 +++ drivers/block/xen-blkback/xenbus.c | 5 +-- drivers/block/xen-blkfront.c| 5 ++- drivers/md/bcache/alloc.c | 2 +- drivers/md/dm-table.c | 8 ++-- drivers/md/dm-thin.c| 4 +- drivers/md/md.c | 2 +- drivers/md/raid5-cache.c| 6 +-- drivers/mmc/core/queue.c| 2 +- drivers/nvme/target/io-cmd-bdev.c | 2 +- drivers/target/target_core_file.c | 2 +- drivers/target/target_core_iblock.c | 2 +- fs/btrfs/extent-tree.c | 4 +- fs/ext4/mballoc.c | 2 +- fs/f2fs/file.c | 16 fs/f2fs/segment.c | 2 +- fs/jbd2/journal.c | 2 +- fs/nilfs2/sufile.c | 4 +- fs/nilfs2/the_nilfs.c | 4 +- fs/ntfs3/super.c| 2 +- fs/xfs/xfs_discard.c| 2 +- fs/xfs/xfs_log_cil.c| 2 +- include/linux/blkdev.h | 27 +++- mm/swapfile.c | 6 +-- 33 files changed, 168 insertions(+), 99 deletions(-) [snipped] diff --git a/drivers/md/bcache/alloc.c b/drivers/md/bcache/alloc.c index 097577ae3c471..ce13c272c3872 100644 --- a/drivers/md/bcache/alloc.c +++ b/drivers/md/bcache/alloc.c @@ -336,7 +336,7 @@ static int bch_allocator_thread(void *arg) mutex_unlock(&ca->set->bucket_lock); blkdev_issue_discard(ca->bdev, bucket_to_sector(ca->set, bucket), - ca->sb.bucket_size, GFP_KERNEL, 0); + ca->sb.bucket_size, GFP_KERNEL); mutex_lock(&ca->set->bucket_lock); } [snipped]
Re: [PATCH 25/27] block: remove QUEUE_FLAG_DISCARD
On 4/6/22 2:05 PM, Christoph Hellwig wrote: Just use a non-zero max_discard_sectors as an indicator for discard support, similar to what is done for write zeroes. The only places where needs special attention is the RAID5 driver, which must clear discard support for security reasons by default, even if the default stacking rules would allow for it. Signed-off-by: Christoph Hellwig For the bcache part, Acked-by: Coly Li Thanks. Coly Li --- arch/um/drivers/ubd_kern.c| 2 -- block/blk-mq-debugfs.c| 1 - drivers/block/drbd/drbd_nl.c | 15 --- drivers/block/loop.c | 2 -- drivers/block/nbd.c | 3 --- drivers/block/null_blk/main.c | 1 - drivers/block/rbd.c | 1 - drivers/block/rnbd/rnbd-clt.c | 2 -- drivers/block/virtio_blk.c| 2 -- drivers/block/xen-blkfront.c | 2 -- drivers/block/zram/zram_drv.c | 1 - drivers/md/bcache/super.c | 1 - drivers/md/dm-table.c | 5 + drivers/md/dm-thin.c | 2 -- drivers/md/dm.c | 1 - drivers/md/md-linear.c| 9 - drivers/md/raid0.c| 7 --- drivers/md/raid1.c| 14 -- drivers/md/raid10.c | 14 -- drivers/md/raid5.c| 12 drivers/mmc/core/queue.c | 1 - drivers/mtd/mtd_blkdevs.c | 1 - drivers/nvme/host/core.c | 6 ++ drivers/s390/block/dasd_fba.c | 1 - drivers/scsi/sd.c | 2 -- include/linux/blkdev.h| 2 -- 26 files changed, 7 insertions(+), 103 deletions(-) [snipped] diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c index 296f200b2e208..2f49e31142f62 100644 --- a/drivers/md/bcache/super.c +++ b/drivers/md/bcache/super.c @@ -973,7 +973,6 @@ static int bcache_device_init(struct bcache_device *d, unsigned int block_size, blk_queue_flag_set(QUEUE_FLAG_NONROT, d->disk->queue); blk_queue_flag_clear(QUEUE_FLAG_ADD_RANDOM, d->disk->queue); - blk_queue_flag_set(QUEUE_FLAG_DISCARD, d->disk->queue); blk_queue_write_cache(q, true, true); [snipped]
Re: [PATCH 23/27] block: add a bdev_max_discard_sectors helper
On 4/6/22 2:05 PM, Christoph Hellwig wrote:
Add a helper to query the number of sectors support per each discard bio
based on the block device and use this helper to stop various places from
poking into the request_queue to see if discard is supported and if so how
much. This mirrors what is done e.g. for write zeroes as well.
Signed-off-by: Christoph Hellwig
For the bcache part,
Acked-by: Coly Li
Thanks.
Coly Li
---
block/blk-core.c| 2 +-
block/blk-lib.c | 2 +-
block/ioctl.c | 3 +--
drivers/block/drbd/drbd_main.c | 2 +-
drivers/block/drbd/drbd_nl.c| 12 +++-
drivers/block/drbd/drbd_receiver.c | 5 ++---
drivers/block/loop.c| 9 +++--
drivers/block/rnbd/rnbd-srv-dev.h | 6 +-
drivers/block/xen-blkback/xenbus.c | 2 +-
drivers/md/bcache/request.c | 4 ++--
drivers/md/bcache/super.c | 2 +-
drivers/md/bcache/sysfs.c | 2 +-
drivers/md/dm-cache-target.c| 9 +
drivers/md/dm-clone-target.c| 9 +
drivers/md/dm-io.c | 2 +-
drivers/md/dm-log-writes.c | 3 +--
drivers/md/dm-raid.c| 9 ++---
drivers/md/dm-table.c | 4 +---
drivers/md/dm-thin.c| 9 +
drivers/md/dm.c | 2 +-
drivers/md/md-linear.c | 4 ++--
drivers/md/raid0.c | 2 +-
drivers/md/raid1.c | 6 +++---
drivers/md/raid10.c | 8
drivers/md/raid5-cache.c| 2 +-
drivers/target/target_core_device.c | 8 +++-
fs/btrfs/extent-tree.c | 4 ++--
fs/btrfs/ioctl.c| 2 +-
fs/exfat/file.c | 2 +-
fs/exfat/super.c| 10 +++---
fs/ext4/ioctl.c | 10 +++---
fs/ext4/super.c | 10 +++---
fs/f2fs/f2fs.h | 3 +--
fs/f2fs/segment.c | 6 ++
fs/fat/file.c | 2 +-
fs/fat/inode.c | 10 +++---
fs/gfs2/rgrp.c | 2 +-
fs/jbd2/journal.c | 7 ++-
fs/jfs/ioctl.c | 2 +-
fs/jfs/super.c | 8 ++--
fs/nilfs2/ioctl.c | 2 +-
fs/ntfs3/file.c | 2 +-
fs/ntfs3/super.c| 2 +-
fs/ocfs2/ioctl.c| 2 +-
fs/xfs/xfs_discard.c| 2 +-
fs/xfs/xfs_super.c | 12
include/linux/blkdev.h | 5 +
mm/swapfile.c | 17 ++---
48 files changed, 87 insertions(+), 163 deletions(-)
[snipped]
diff --git a/drivers/md/bcache/request.c b/drivers/md/bcache/request.c
index fdd0194f84dd0..e27f67f06a428 100644
--- a/drivers/md/bcache/request.c
+++ b/drivers/md/bcache/request.c
@@ -1005,7 +1005,7 @@ static void cached_dev_write(struct cached_dev *dc,
struct search *s)
bio_get(s->iop.bio);
if (bio_op(bio) == REQ_OP_DISCARD &&
- !blk_queue_discard(bdev_get_queue(dc->bdev)))
+ !bdev_max_discard_sectors(dc->bdev))
goto insert_data;
/* I/O request sent to backing device */
@@ -1115,7 +1115,7 @@ static void detached_dev_do_request(struct bcache_device
*d, struct bio *bio,
bio->bi_private = ddip;
if ((bio_op(bio) == REQ_OP_DISCARD) &&
- !blk_queue_discard(bdev_get_queue(dc->bdev)))
+ !bdev_max_discard_sectors(dc->bdev))
bio->bi_end_io(bio);
else
submit_bio_noacct(bio);
diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
index bf3de149d3c9f..296f200b2e208 100644
--- a/drivers/md/bcache/super.c
+++ b/drivers/md/bcache/super.c
@@ -2350,7 +2350,7 @@ static int register_cache(struct cache_sb *sb, struct
cache_sb_disk *sb_disk,
ca->bdev->bd_holder = ca;
ca->sb_disk = sb_disk;
- if (blk_queue_discard(bdev_get_queue(bdev)))
+ if (bdev_max_discard_sectors((bdev)))
ca->discard = CACHE_DISCARD(&ca->sb);
ret = cache_alloc(ca);
diff --git a/drivers/md/bcache/sysfs.c b/drivers/md/bcache/sysfs.c
index d1029d71ff3bc..c6f677059214d 100644
--- a/drivers/md/bcache/sysfs.c
+++ b/drivers/md/bcache/sysfs.c
@@ -1151,7 +1151,7 @@ STORE(__bch_cache)
if (attr == &sysfs_discard) {
bool v = strtoul_or_return(buf);
- if (blk_queue_discard(bdev_get_queue(ca->bdev)))
+ if (bdev_max_discard_sectors(ca->bdev))
ca->discard = v;
if (v != CACHE_DISCARD(&ca->sb)) {
[snipped]
Re: [PATCH 02/10] bcache: add error handling support for add_disk()
On 8/28/21 3:18 AM, Luis Chamberlain wrote:
We never checked for errors on add_disk() as this function
returned void. Now that this is fixed, use the shiny new
error handling.
This driver doesn't do any unwinding with blk_cleanup_disk()
even on errors after add_disk() and so we follow that
tradition.
Signed-off-by: Luis Chamberlain
Acked-by: Coly Li
Thanks.
---
drivers/md/bcache/super.c | 17 -
1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
index f2874c77ff79..f0c32cdd6594 100644
--- a/drivers/md/bcache/super.c
+++ b/drivers/md/bcache/super.c
@@ -1082,7 +1082,9 @@ int bch_cached_dev_run(struct cached_dev *dc)
closure_sync(&cl);
}
- add_disk(d->disk);
+ ret = add_disk(d->disk);
+ if (ret)
+ goto out;
bd_link_disk_holder(dc->bdev, dc->disk.disk);
/*
* won't show up in the uevent file, use udevadm monitor -e instead
@@ -1534,10 +1536,11 @@ static void flash_dev_flush(struct closure *cl)
static int flash_dev_run(struct cache_set *c, struct uuid_entry *u)
{
+ int err = -ENOMEM;
struct bcache_device *d = kzalloc(sizeof(struct bcache_device),
GFP_KERNEL);
if (!d)
- return -ENOMEM;
+ goto err_ret;
closure_init(&d->cl, NULL);
set_closure_fn(&d->cl, flash_dev_flush, system_wq);
@@ -1551,9 +1554,12 @@ static int flash_dev_run(struct cache_set *c, struct
uuid_entry *u)
bcache_device_attach(d, c, u - c->uuids);
bch_sectors_dirty_init(d);
bch_flash_dev_request_init(d);
- add_disk(d->disk);
+ err = add_disk(d->disk);
+ if (err)
+ goto err;
- if (kobject_add(&d->kobj, &disk_to_dev(d->disk)->kobj, "bcache"))
+ err = kobject_add(&d->kobj, &disk_to_dev(d->disk)->kobj, "bcache");
+ if (err)
goto err;
bcache_device_link(d, c, "volume");
@@ -1567,7 +1573,8 @@ static int flash_dev_run(struct cache_set *c, struct
uuid_entry *u)
return 0;
err:
kobject_put(&d->kobj);
- return -ENOMEM;
+err_ret:
+ return err;
}
static int flash_devs_run(struct cache_set *c)
Re: [PATCH 13/45] block: add a bdev_kobj helper
On 11/24/20 9:27 PM, Christoph Hellwig wrote:
> Add a little helper to find the kobject for a struct block_device.
>
> Signed-off-by: Christoph Hellwig
> Reviewed-by: Greg Kroah-Hartman
> Reviewed-by: Jan Kara
> Reviewed-by: Johannes Thumshirn
For the bcache part, Acked-by: Coly Li
Thanks.
Coly Li
> ---
> drivers/md/bcache/super.c | 7 ++-
> drivers/md/md.c | 4 +---
> fs/block_dev.c| 6 +++---
> fs/btrfs/sysfs.c | 15 +++
> include/linux/blk_types.h | 3 +++
> 5 files changed, 12 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
> index 46a00134a36ae1..a6a5e21e4fd136 100644
> --- a/drivers/md/bcache/super.c
> +++ b/drivers/md/bcache/super.c
> @@ -1447,8 +1447,7 @@ static int register_bdev(struct cache_sb *sb, struct
> cache_sb_disk *sb_disk,
> goto err;
>
> err = "error creating kobject";
> - if (kobject_add(&dc->disk.kobj, &part_to_dev(bdev->bd_part)->kobj,
> - "bcache"))
> + if (kobject_add(&dc->disk.kobj, bdev_kobj(bdev), "bcache"))
> goto err;
> if (bch_cache_accounting_add_kobjs(&dc->accounting, &dc->disk.kobj))
> goto err;
> @@ -2342,9 +2341,7 @@ static int register_cache(struct cache_sb *sb, struct
> cache_sb_disk *sb_disk,
> goto err;
> }
>
> - if (kobject_add(&ca->kobj,
> - &part_to_dev(bdev->bd_part)->kobj,
> - "bcache")) {
> + if (kobject_add(&ca->kobj, bdev_kobj(bdev), "bcache")) {
> err = "error calling kobject_add";
> ret = -ENOMEM;
> goto out;
[snipped]
Re: [PATCH 23/45] block: remove i_bdev
On 11/24/20 9:27 PM, Christoph Hellwig wrote:
> Switch the block device lookup interfaces to directly work with a dev_t
> so that struct block_device references are only acquired by the
> blkdev_get variants (and the blk-cgroup special case). This means that
> we not don't need an extra reference in the inode and can generally
> simplify handling of struct block_device to keep the lookups contained
> in the core block layer code.
>
> Signed-off-by: Christoph Hellwig
For the bcache part, Acked-by: Coly Li
Thanks.
Coly Li
> ---
> block/ioctl.c| 3 +-
> drivers/block/loop.c | 8 +-
> drivers/md/bcache/super.c| 20 +-
> drivers/md/dm-table.c| 9 +-
> drivers/mtd/mtdsuper.c | 17 +-
> drivers/target/target_core_file.c| 6 +-
> drivers/usb/gadget/function/storage_common.c | 8 +-
> fs/block_dev.c | 195 +--
> fs/btrfs/volumes.c | 13 +-
> fs/inode.c | 3 -
> fs/internal.h| 7 +-
> fs/io_uring.c| 10 +-
> fs/pipe.c| 5 +-
> fs/quota/quota.c | 19 +-
> fs/statfs.c | 2 +-
> fs/super.c | 37 ++--
> include/linux/blkdev.h | 2 +-
> include/linux/fs.h | 1 -
> 18 files changed, 114 insertions(+), 251 deletions(-)
>
[snipped]
> diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
> index a6a5e21e4fd136..c55d3c58a7ef55 100644
> --- a/drivers/md/bcache/super.c
> +++ b/drivers/md/bcache/super.c
> @@ -2380,38 +2380,38 @@ kobj_attribute_write(register,
> register_bcache);
> kobj_attribute_write(register_quiet, register_bcache);
> kobj_attribute_write(pendings_cleanup, bch_pending_bdevs_cleanup);
>
> -static bool bch_is_open_backing(struct block_device *bdev)
> +static bool bch_is_open_backing(dev_t dev)
> {
> struct cache_set *c, *tc;
> struct cached_dev *dc, *t;
>
> list_for_each_entry_safe(c, tc, &bch_cache_sets, list)
> list_for_each_entry_safe(dc, t, &c->cached_devs, list)
> - if (dc->bdev == bdev)
> + if (dc->bdev->bd_dev == dev)
> return true;
> list_for_each_entry_safe(dc, t, &uncached_devices, list)
> - if (dc->bdev == bdev)
> + if (dc->bdev->bd_dev == dev)
> return true;
> return false;
> }
>
> -static bool bch_is_open_cache(struct block_device *bdev)
> +static bool bch_is_open_cache(dev_t dev)
> {
> struct cache_set *c, *tc;
>
> list_for_each_entry_safe(c, tc, &bch_cache_sets, list) {
> struct cache *ca = c->cache;
>
> - if (ca->bdev == bdev)
> + if (ca->bdev->bd_dev == dev)
> return true;
> }
>
> return false;
> }
>
> -static bool bch_is_open(struct block_device *bdev)
> +static bool bch_is_open(dev_t dev)
> {
> - return bch_is_open_cache(bdev) || bch_is_open_backing(bdev);
> + return bch_is_open_cache(dev) || bch_is_open_backing(dev);
> }
>
> struct async_reg_args {
> @@ -2535,9 +2535,11 @@ static ssize_t register_bcache(struct kobject *k,
> struct kobj_attribute *attr,
> sb);
> if (IS_ERR(bdev)) {
> if (bdev == ERR_PTR(-EBUSY)) {
> - bdev = lookup_bdev(strim(path));
> + dev_t dev;
> +
> mutex_lock(&bch_register_lock);
> - if (!IS_ERR(bdev) && bch_is_open(bdev))
> + if (lookup_bdev(strim(path), &dev) == 0 &&
> + bch_is_open(dev))
> err = "device already registered";
> else
> err = "device busy";
[snipped]
Re: [PATCH 30/45] block: remove the nr_sects field in struct hd_struct
On 11/24/20 9:27 PM, Christoph Hellwig wrote: > Now that the hd_struct always has a block device attached to it, there is > no need for having two size field that just get out of sync. > > Additional the field in hd_struct did not use proper serializiation, > possibly allowing for torn writes. By only using the block_device field > this problem also gets fixed. > > Signed-off-by: Christoph Hellwig > Reviewed-by: Greg Kroah-Hartman For the bcache part, Acked-by: Coly Li Thanks. Coly Li > --- > block/bio.c| 4 +- > block/blk-core.c | 2 +- > block/blk.h| 53 -- > block/genhd.c | 55 +++--- > block/partitions/core.c| 17 --- > drivers/block/loop.c | 1 - > drivers/block/nbd.c| 2 +- > drivers/block/xen-blkback/common.h | 4 +- > drivers/md/bcache/super.c | 2 +- > drivers/s390/block/dasd_ioctl.c| 4 +- > drivers/target/target_core_pscsi.c | 7 +-- > fs/block_dev.c | 73 +- > fs/f2fs/super.c| 2 +- > fs/pstore/blk.c| 2 +- > include/linux/genhd.h | 29 +++- > kernel/trace/blktrace.c| 2 +- > 16 files changed, 60 insertions(+), 199 deletions(-) > [snipped] > diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c > index c55d3c58a7ef55..04fa40868fbe10 100644 > --- a/drivers/md/bcache/super.c > +++ b/drivers/md/bcache/super.c > @@ -1408,7 +1408,7 @@ static int cached_dev_init(struct cached_dev *dc, > unsigned int block_size) > q->limits.raid_partial_stripes_expensive; > > ret = bcache_device_init(&dc->disk, block_size, > - dc->bdev->bd_part->nr_sects - dc->sb.data_offset, > + bdev_nr_sectors(dc->bdev) - dc->sb.data_offset, >dc->bdev, &bcache_cached_ops); > if (ret) > return ret; [snipped]
Re: [PATCH 38/45] block: switch partition lookup to use struct block_device
On 11/24/20 9:27 PM, Christoph Hellwig wrote:
> Use struct block_device to lookup partitions on a disk. This removes
> all usage of struct hd_struct from the I/O path, and this allows removing
> the percpu refcount in struct hd_struct.
>
> Signed-off-by: Christoph Hellwig
For the bcache part, Acked-by: Coly Li
> ---
> block/bio.c| 4 +-
> block/blk-core.c | 66 ++
> block/blk-flush.c | 2 +-
> block/blk-mq.c | 9 ++--
> block/blk-mq.h | 7 ++--
> block/blk.h| 4 +-
> block/genhd.c | 56 +
> block/partitions/core.c| 7 +---
> drivers/block/drbd/drbd_receiver.c | 2 +-
> drivers/block/drbd/drbd_worker.c | 2 +-
> drivers/block/zram/zram_drv.c | 2 +-
> drivers/md/bcache/request.c| 4 +-
> drivers/md/dm.c| 4 +-
> drivers/md/md.c| 4 +-
> drivers/nvme/target/admin-cmd.c| 20 -
> fs/ext4/super.c| 18 +++-
> fs/ext4/sysfs.c| 10 +
> fs/f2fs/f2fs.h | 2 +-
> fs/f2fs/super.c| 6 +--
> include/linux/blkdev.h | 8 ++--
> include/linux/genhd.h | 4 +-
> include/linux/part_stat.h | 17
> 22 files changed, 120 insertions(+), 138 deletions(-)
[snipped]
> diff --git a/drivers/md/bcache/request.c b/drivers/md/bcache/request.c
> index afac8d07c1bd00..85b1f2a9b72d68 100644
> --- a/drivers/md/bcache/request.c
> +++ b/drivers/md/bcache/request.c
> @@ -475,7 +475,7 @@ struct search {
> unsigned intread_dirty_data:1;
> unsigned intcache_missed:1;
>
> - struct hd_struct*part;
> + struct block_device *part;
> unsigned long start_time;
>
> struct btree_op op;
> @@ -1073,7 +1073,7 @@ struct detached_dev_io_private {
> unsigned long start_time;
> bio_end_io_t*bi_end_io;
> void*bi_private;
> - struct hd_struct*part;
> + struct block_device *part;
> };
[snipped]
Re: [PATCH 19/20] bcache: remove a superflous lookup_bdev all
On 11/18/20 5:10 PM, Greg KH wrote: > On Wed, Nov 18, 2020 at 04:54:51PM +0800, Coly Li wrote: >> On 11/18/20 4:47 PM, Christoph Hellwig wrote: >>> Don't bother to call lookup_bdev for just a slightly different error >>> message without any functional change. >>> >>> Signed-off-by: Christoph Hellwig ist >> >> Hi Christoph, >> >> NACK. This removing error message is frequently triggered and observed, >> and distinct a busy device and an already registered device is important >> (the first one is critical error and second one is not). >> >> Remove such error message will be a functional regression. > > What normal operation causes this error message to be emitted? And what > can a user do with it? When there was bug and the caching or backing device was not unregistered successfully, people could see "device busy"; and if it was because the device registered again, it could be "already registered". Without the different message, people may think the device is always busy but indeed it isn't. he motivation of the patch is OK to me, but we need to make the logical consistent, otherwise we will have similar bug report for bogus warning dmesg from bcache users in soon future. Coly Li
Re: [PATCH 19/20] bcache: remove a superflous lookup_bdev all
On 11/18/20 4:47 PM, Christoph Hellwig wrote:
> Don't bother to call lookup_bdev for just a slightly different error
> message without any functional change.
>
> Signed-off-by: Christoph Hellwig ist
Hi Christoph,
NACK. This removing error message is frequently triggered and observed,
and distinct a busy device and an already registered device is important
(the first one is critical error and second one is not).
Remove such error message will be a functional regression.
Coly Li
> ---
> drivers/md/bcache/super.c | 44 +--
> 1 file changed, 1 insertion(+), 43 deletions(-)
>
> diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
> index e5db2cdd114112..5c531ed7785280 100644
> --- a/drivers/md/bcache/super.c
> +++ b/drivers/md/bcache/super.c
> @@ -2380,40 +2380,6 @@ kobj_attribute_write(register,
> register_bcache);
> kobj_attribute_write(register_quiet, register_bcache);
> kobj_attribute_write(pendings_cleanup, bch_pending_bdevs_cleanup);
>
> -static bool bch_is_open_backing(struct block_device *bdev)
> -{
> - struct cache_set *c, *tc;
> - struct cached_dev *dc, *t;
> -
> - list_for_each_entry_safe(c, tc, &bch_cache_sets, list)
> - list_for_each_entry_safe(dc, t, &c->cached_devs, list)
> - if (dc->bdev == bdev)
> - return true;
> - list_for_each_entry_safe(dc, t, &uncached_devices, list)
> - if (dc->bdev == bdev)
> - return true;
> - return false;
> -}
> -
> -static bool bch_is_open_cache(struct block_device *bdev)
> -{
> - struct cache_set *c, *tc;
> -
> - list_for_each_entry_safe(c, tc, &bch_cache_sets, list) {
> - struct cache *ca = c->cache;
> -
> - if (ca->bdev == bdev)
> - return true;
> - }
> -
> - return false;
> -}
> -
> -static bool bch_is_open(struct block_device *bdev)
> -{
> - return bch_is_open_cache(bdev) || bch_is_open_backing(bdev);
> -}
> -
> struct async_reg_args {
> struct delayed_work reg_work;
> char *path;
> @@ -2535,15 +2501,7 @@ static ssize_t register_bcache(struct kobject *k,
> struct kobj_attribute *attr,
> sb);
> if (IS_ERR(bdev)) {
> if (bdev == ERR_PTR(-EBUSY)) {
> - bdev = lookup_bdev(strim(path));
> - mutex_lock(&bch_register_lock);
> - if (!IS_ERR(bdev) && bch_is_open(bdev))
> - err = "device already registered";
> - else
> - err = "device busy";
> - mutex_unlock(&bch_register_lock);
> - if (!IS_ERR(bdev))
> - bdput(bdev);
> + err = "device busy";
> if (attr == &ksysfs_register_quiet)
> goto done;
> }
>
Re: [PATCH RFC PKS/PMEM 48/58] drivers/md: Utilize new kmap_thread()
On 2020/10/12 13:28, Ira Weiny wrote: > On Sat, Oct 10, 2020 at 10:20:34AM +0800, Coly Li wrote: >> On 2020/10/10 03:50, ira.we...@intel.com wrote: >>> From: Ira Weiny >>> >>> These kmap() calls are localized to a single thread. To avoid the over >>> head of global PKRS updates use the new kmap_thread() call. >>> >> >> Hi Ira, >> >> There were a number of options considered. >> >> 1) Attempt to change all the thread local kmap() calls to kmap_atomic() >> 2) Introduce a flags parameter to kmap() to indicate if the mapping >> should be global or not >> 3) Change ~20-30 call sites to 'kmap_global()' to indicate that they >> require a global mapping of the pages >> 4) Change ~209 call sites to 'kmap_thread()' to indicate that the >> mapping is to be used within that thread of execution only >> >> >> I copied the above information from patch 00/58 to this message. The >> idea behind kmap_thread() is fine to me, but as you said the new api is >> very easy to be missed in new code (even for me). I would like to be >> supportive to option 2) introduce a flag to kmap(), then we won't forget >> the new thread-localized kmap method, and people won't ask why a >> _thread() function is called but no kthread created. > > Thanks for the feedback. > > I'm going to hold off making any changes until others weigh in. FWIW, I kind > of like option 2 as well. But there is already kmap_atomic() so it seemed > like > kmap_() was more in line with the current API. I understand it now, the idea is fine to me. Acked-by: Coly Li Thanks. Coly Li
Re: [PATCH RFC PKS/PMEM 48/58] drivers/md: Utilize new kmap_thread()
On 2020/10/10 03:50, ira.we...@intel.com wrote:
> From: Ira Weiny
>
> These kmap() calls are localized to a single thread. To avoid the over
> head of global PKRS updates use the new kmap_thread() call.
>
Hi Ira,
There were a number of options considered.
1) Attempt to change all the thread local kmap() calls to kmap_atomic()
2) Introduce a flags parameter to kmap() to indicate if the mapping
should be global or not
3) Change ~20-30 call sites to 'kmap_global()' to indicate that they
require a global mapping of the pages
4) Change ~209 call sites to 'kmap_thread()' to indicate that the
mapping is to be used within that thread of execution only
I copied the above information from patch 00/58 to this message. The
idea behind kmap_thread() is fine to me, but as you said the new api is
very easy to be missed in new code (even for me). I would like to be
supportive to option 2) introduce a flag to kmap(), then we won't forget
the new thread-localized kmap method, and people won't ask why a
_thread() function is called but no kthread created.
Thanks.
Coly Li
> Cc: Coly Li (maintainer:BCACHE (BLOCK LAYER CACHE))
> Cc: Kent Overstreet (maintainer:BCACHE (BLOCK
> LAYER CACHE))
> Signed-off-by: Ira Weiny
> ---
> drivers/md/bcache/request.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/md/bcache/request.c b/drivers/md/bcache/request.c
> index c7cadaafa947..a4571f6d09dd 100644
> --- a/drivers/md/bcache/request.c
> +++ b/drivers/md/bcache/request.c
> @@ -44,10 +44,10 @@ static void bio_csum(struct bio *bio, struct bkey *k)
> uint64_t csum = 0;
>
> bio_for_each_segment(bv, bio, iter) {
> - void *d = kmap(bv.bv_page) + bv.bv_offset;
> + void *d = kmap_thread(bv.bv_page) + bv.bv_offset;
>
> csum = bch_crc64_update(csum, d, bv.bv_len);
> - kunmap(bv.bv_page);
> + kunmap_thread(bv.bv_page);
> }
>
> k->ptr[KEY_PTRS(k)] = csum & (~0ULL >> 1);
>
Re: [Xen-devel] [PATCH v3] block: refactor duplicated macros
On 2020/3/11 8:22 上午, Matteo Croce wrote: > The macros PAGE_SECTORS, PAGE_SECTORS_SHIFT and SECTOR_MASK are defined > several times in different flavours across the whole tree. > Define them just once in a common header. > > While at it, replace replace "PAGE_SHIFT - 9" with "PAGE_SECTORS_SHIFT" too > and rename SECTOR_MASK to PAGE_SECTORS_MASK. > > Signed-off-by: Matteo Croce Hi Matteo, For the bcache part, it looks good to me. Acked-by: Coly Li > --- > v3: > As Guoqing Jiang suggested, replace "PAGE_SHIFT - 9" with "PAGE_SECTORS_SHIFT" > > v2: > As Dan Williams suggested: > > #define PAGE_SECTORS_MASK(~(PAGE_SECTORS - 1)) > > block/blk-lib.c | 2 +- > block/blk-settings.c | 4 ++-- > block/partition-generic.c| 2 +- > drivers/block/brd.c | 3 --- > drivers/block/null_blk_main.c| 14 +- > drivers/block/zram/zram_drv.c| 8 > drivers/block/zram/zram_drv.h| 2 -- > drivers/dax/super.c | 2 +- > drivers/md/bcache/util.h | 2 -- > drivers/md/dm-bufio.c| 6 +++--- > drivers/md/dm-integrity.c| 10 +- > drivers/md/dm-table.c| 2 +- > drivers/md/md.c | 4 ++-- > drivers/md/raid1.c | 2 +- > drivers/md/raid10.c | 2 +- > drivers/md/raid5-cache.c | 10 +- > drivers/md/raid5.h | 2 +- > drivers/mmc/core/host.c | 3 ++- > drivers/nvme/host/fc.c | 2 +- > drivers/nvme/target/loop.c | 2 +- > drivers/scsi/xen-scsifront.c | 4 ++-- > fs/erofs/internal.h | 2 +- > fs/ext2/dir.c| 2 +- > fs/iomap/buffered-io.c | 2 +- > fs/libfs.c | 2 +- > fs/nfs/blocklayout/blocklayout.h | 2 -- > fs/nilfs2/dir.c | 2 +- > include/linux/blkdev.h | 4 > include/linux/device-mapper.h| 1 - > mm/page_io.c | 4 ++-- > mm/swapfile.c| 12 ++-- > 31 files changed, 56 insertions(+), 65 deletions(-) > [snipped] > diff --git a/drivers/md/bcache/util.h b/drivers/md/bcache/util.h > index c029f7443190..55196e0f37c3 100644 > --- a/drivers/md/bcache/util.h > +++ b/drivers/md/bcache/util.h > @@ -15,8 +15,6 @@ > > #include "closure.h" > > -#define PAGE_SECTORS (PAGE_SIZE / 512) > - > struct closure; > > #ifdef CONFIG_BCACHE_DEBUG [snipped] -- Coly Li ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
