Re: [Cluster-devel] [PATCH 23/27] block: add a bdev_max_discard_sectors helper

2022-04-08 Thread Ryusuke Konishi
On Wed, Apr 6, 2022 at 11: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 
> ---
...
> diff --git a/drivers/target/target_core_device.c 
> b/drivers/target/target_core_device.c
> index 16e775bcf4a7c..7d510e4231713 100644
> --- a/drivers/target/target_core_device.c
> +++ b/drivers/target/target_core_device.c
> @@ -829,9 +829,7 @@ struct se_device *target_alloc_device(struct se_hba *hba, 
> const char *name)
>  }
>
>  /*
> - * Check if the underlying struct block_device request_queue supports
> - * the QUEUE_FLAG_DISCARD bit for UNMAP/WRITE_SAME in SCSI + TRIM
> - * in ATA and we need to set TPE=1

> + * Check if the underlying struct block_device request_queue supports disard.
>   */

Here was a typo:

 s/disard/discard/

On Thu, Apr 7, 2022 at 12:19 AM Andreas Gruenbacher  wrote:
> If I'm misreading things, could you please document that
> bdev_max_discard_sectors() != 0 implies that discard is supported?

I got the same impression.   Checking the discard support with
bdev_max_discard_sectors() != 0 seems a bit unclear than before.

Thanks,
Ryusuke Konishi



Re: [Cluster-devel] [PATCH 23/27] block: add a bdev_max_discard_sectors helper

2022-04-07 Thread David Sterba
On Wed, Apr 06, 2022 at 08:05:12AM +0200, 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 
> ---
>  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 +++-

For

>  fs/btrfs/extent-tree.c  |  4 ++--
>  fs/btrfs/ioctl.c|  2 +-

Acked-by: David Sterba 



Re: [Cluster-devel] [PATCH 23/27] block: add a bdev_max_discard_sectors helper

2022-04-06 Thread Coly Li

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(>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 == _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(>sb)) {



[snipped]



Re: [Cluster-devel] [PATCH 23/27] block: add a bdev_max_discard_sectors helper

2022-04-06 Thread Andreas Gruenbacher
On Wed, Apr 6, 2022 at 8:07 AM 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 
> ---
>  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(-)
>
> diff --git a/block/blk-core.c b/block/blk-core.c
> index 937bb6b863317..b5c3a8049134c 100644
> --- a/block/blk-core.c
> +++ b/block/blk-core.c
> @@ -820,7 +820,7 @@ void submit_bio_noacct(struct bio *bio)
>
> switch (bio_op(bio)) {
> case REQ_OP_DISCARD:
> -   if (!blk_queue_discard(q))
> +   if (!bdev_max_discard_sectors(bdev))
> goto not_supported;
> break;
> case REQ_OP_SECURE_ERASE:
> diff --git a/block/blk-lib.c b/block/blk-lib.c
> index 2ae32a722851c..8b4b66d3a9bfc 100644
> --- a/block/blk-lib.c
> +++ b/block/blk-lib.c
> @@ -53,7 +53,7 @@ int __blkdev_issue_discard(struct block_device *bdev, 
> sector_t sector,
> return -EOPNOTSUPP;
> op = REQ_OP_SECURE_ERASE;
> } else {
> -   if (!blk_queue_discard(q))
> +   if (!bdev_max_discard_sectors(bdev))
> return -EOPNOTSUPP;
> op = REQ_OP_DISCARD;
> }
> diff --git a/block/ioctl.c b/block/ioctl.c
> index ad3771b268b81..c2cd3ba5290ce 100644
> --- a/block/ioctl.c
> +++ b/block/ioctl.c
> @@ -87,14 +87,13 @@ static int blk_ioctl_discard(struct block_device *bdev, 
> fmode_t mode,
>  {
> uint64_t range[2];
> uint64_t start, len;
> -   struct request_queue *q = bdev_get_queue(bdev);
> struct inode *inode = bdev->bd_inode;
> int err;
>
> if (!(mode & FMODE_WRITE))
> return -EBADF;
>
> -   if (!blk_queue_discard(q))
> +   if (!bdev_max_discard_sectors(bdev))
> return -EOPNOTSUPP;
>
> if (copy_from_user(range, (void __user *)arg, sizeof(range)))
> diff --git a/drivers/block/drbd/drbd_main.c b/drivers/block/drbd/drbd_main.c
> index 9d43aadde19ad..8fd89a1b0b7b3 100644
> --- a/drivers/block/drbd/drbd_main.c
> +++ b/drivers/block/drbd/drbd_main.c
> @@ -942,7 +942,7 @@ int drbd_send_sizes(struct drbd_peer_device *peer_device, 
> int trigger_reply, enu
> cpu_to_be32(bdev_alignment_offset(bdev));
> p->qlim->io_min =