[PATCH V2 0/3] block: make sure discard/writesame bio is aligned with logical block size

[PATCH V2 0/3] block: make sure discard/writesame bio is aligned with logical block size

[Ming Lei]

Hi,

The 1st & 3rd patch fixes bio size alignment issue.

The 2nd patch cleans up __blkdev_issue_discard() a bit.

V2:
	- introduce helper of bio_allowed_max_sectors()
	- add commit log for patch 2


Ming Lei (3):
  block: make sure discard bio is aligned with logical block size
  block: cleanup __blkdev_issue_discard()
  block: make sure writesame bio is aligned with logical block size

 block/blk-lib.c   | 26 +++++++-------------------
 block/blk-merge.c |  3 ++-
 block/blk.h       | 10 ++++++++++
 3 files changed, 19 insertions(+), 20 deletions(-)

Cc: Rui Salvaterra <rsalvaterra@gmail.com>
Cc: stable@vger.kernel.org
Cc: Mike Snitzer <snitzer@redhat.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Xiao Ni <xni@redhat.com>
Cc: Mariusz Dabrowski <mariusz.dabrowski@intel.com>
-- 
2.9.5

[PATCH V2 1/3] block: make sure discard bio is aligned with logical block size

[Ming Lei]

Obviously the created discard bio has to be aligned with logical block size.

This patch introduces the helper of bio_allowed_max_sectors() for
this purpose.

Fixes: 744889b7cbb56a6 ("block: don't deal with discard limit in blkdev_issue_discard()")
Fixes: a22c4d7e34402cc ("block: re-add discard_granularity and alignment checks")
Reported-by: Rui Salvaterra <rsalvaterra@gmail.com>
Cc: Rui Salvaterra <rsalvaterra@gmail.com>
Cc: stable@vger.kernel.org
Cc: Mike Snitzer <snitzer@redhat.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Xiao Ni <xni@redhat.com>
Cc: Mariusz Dabrowski <mariusz.dabrowski@intel.com>
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 block/blk-lib.c   |  3 +--
 block/blk-merge.c |  3 ++-
 block/blk.h       | 10 ++++++++++
 3 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/block/blk-lib.c b/block/blk-lib.c
index 76f867ea9a9b..d56fd159d2e8 100644
--- a/block/blk-lib.c
+++ b/block/blk-lib.c
@@ -57,8 +57,7 @@ int __blkdev_issue_discard(struct block_device *bdev, sector_t sector,
 
 		if (!req_sects)
 			goto fail;
-		if (req_sects > UINT_MAX >> 9)
-			req_sects = UINT_MAX >> 9;
+		req_sects = min(req_sects, bio_allowed_max_sectors(q));
 
 		end_sect = sector + req_sects;
 
diff --git a/block/blk-merge.c b/block/blk-merge.c
index 42a46744c11b..507fbaa6b4c0 100644
--- a/block/blk-merge.c
+++ b/block/blk-merge.c
@@ -90,7 +90,8 @@ static struct bio *blk_bio_discard_split(struct request_queue *q,
 	/* Zero-sector (unknown) and one-sector granularities are the same.  */
 	granularity = max(q->limits.discard_granularity >> 9, 1U);
 
-	max_discard_sectors = min(q->limits.max_discard_sectors, UINT_MAX >> 9);
+	max_discard_sectors = min(q->limits.max_discard_sectors,
+			bio_allowed_max_sectors(q));
 	max_discard_sectors -= max_discard_sectors % granularity;
 
 	if (unlikely(!max_discard_sectors)) {
diff --git a/block/blk.h b/block/blk.h
index a1841b8ff129..2680aa4fcd88 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -396,6 +396,16 @@ static inline unsigned long blk_rq_deadline(struct request *rq)
 }
 
 /*
+ * The max size one bio can handle is UINT_MAX becasue bvec_iter.bi_size
+ * is defined as 'unsigned int', meantime it has to aligned to with logical
+ * block size which is the minimum accepted unit by hardware.
+ */
+static inline unsigned int bio_allowed_max_sectors(struct request_queue *q)
+{
+	return round_down(UINT_MAX, queue_logical_block_size(q)) >> 9;
+}
+
+/*
  * Internal io_context interface
  */
 void get_io_context(struct io_context *ioc);
-- 
2.9.5

[PATCH V2 3/3] block: make sure writesame bio is aligned with logical block size

[Ming Lei]

Obviously the created writesame bio has to be aligned with logical block
size, and use bio_allowed_max_sectors() to retrieve this number.

Fixes: b49a0871be31a745b2ef ("block: remove split code in blkdev_issue_{discard,write_same}")
Cc: Rui Salvaterra <rsalvaterra@gmail.com>
Cc: stable@vger.kernel.org
Cc: Mike Snitzer <snitzer@redhat.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Xiao Ni <xni@redhat.com>
Cc: Mariusz Dabrowski <mariusz.dabrowski@intel.com>
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 block/blk-lib.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/blk-lib.c b/block/blk-lib.c
index d58d5d87dd88..e8b3bb9bf375 100644
--- a/block/blk-lib.c
+++ b/block/blk-lib.c
@@ -149,7 +149,7 @@ static int __blkdev_issue_write_same(struct block_device *bdev, sector_t sector,
 		return -EOPNOTSUPP;
 
 	/* Ensure that max_write_same_sectors doesn't overflow bi_size */
-	max_write_same_sectors = UINT_MAX >> 9;
+	max_write_same_sectors = bio_allowed_max_sectors(q);
 
 	while (nr_sects) {
 		bio = blk_next_bio(bio, 1, gfp_mask);
-- 
2.9.5

Re: [PATCH V2 0/3] block: make sure discard/writesame bio is aligned with logical block size

[Ming Lei]

On Mon, Oct 29, 2018 at 08:57:16PM +0800, Ming Lei wrote:
> Hi,
> 
> The 1st & 3rd patch fixes bio size alignment issue.
> 
> The 2nd patch cleans up __blkdev_issue_discard() a bit.
> 
> V2:
> 	- introduce helper of bio_allowed_max_sectors()
> 	- add commit log for patch 2
> 
> 
> Ming Lei (3):
>   block: make sure discard bio is aligned with logical block size
>   block: cleanup __blkdev_issue_discard()
>   block: make sure writesame bio is aligned with logical block size
> 
>  block/blk-lib.c   | 26 +++++++-------------------
>  block/blk-merge.c |  3 ++-
>  block/blk.h       | 10 ++++++++++
>  3 files changed, 19 insertions(+), 20 deletions(-)
> 
> Cc: Rui Salvaterra <rsalvaterra@gmail.com>
> Cc: stable@vger.kernel.org
> Cc: Mike Snitzer <snitzer@redhat.com>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Xiao Ni <xni@redhat.com>
> Cc: Mariusz Dabrowski <mariusz.dabrowski@intel.com>

V2 addresses Christoph's comment by introducing bio_allowed_max_sectors().

Ping...

Thanks,
Ming

Re: [PATCH V2 0/3] block: make sure discard/writesame bio is aligned with logical block size

[Rui Salvaterra]

On Mon, 5 Nov 2018 at 03:41, Ming Lei <ming.lei@redhat.com> wrote:
>
> V2 addresses Christoph's comment by introducing bio_allowed_max_sectors().
>
> Ping...
>
> Thanks,
> Ming

Hi, Ming,

Sorry for the delay. I tested your V2 against Linux 4.20-rc1 and
everything seems fine. FWIW, V2 is also

Tested-by: Rui Salvaterra <rsalvaterra@gmail.com>

Thanks again,

Rui

Re: [PATCH V2 0/3] block: make sure discard/writesame bio is aligned with logical block size

[Ming Lei]

On Tue, Nov 06, 2018 at 04:48:13PM +0000, Rui Salvaterra wrote:
> On Mon, 5 Nov 2018 at 03:41, Ming Lei <ming.lei@redhat.com> wrote:
> >
> > V2 addresses Christoph's comment by introducing bio_allowed_max_sectors().
> >
> > Ping...
> >
> > Thanks,
> > Ming
> 
> Hi, Ming,
> 
> Sorry for the delay. I tested your V2 against Linux 4.20-rc1 and
> everything seems fine. FWIW, V2 is also
> 
> Tested-by: Rui Salvaterra <rsalvaterra@gmail.com>

Rui, thanks for your test on V2.

Jens, what do you think about this patchset?

Thanks,
Ming

Re: [PATCH V2 0/3] block: make sure discard/writesame bio is aligned with logical block size

[Jens Axboe]

On 11/6/18 5:55 PM, Ming Lei wrote:
> On Tue, Nov 06, 2018 at 04:48:13PM +0000, Rui Salvaterra wrote:
>> On Mon, 5 Nov 2018 at 03:41, Ming Lei <ming.lei@redhat.com> wrote:
>>>
>>> V2 addresses Christoph's comment by introducing bio_allowed_max_sectors().
>>>
>>> Ping...
>>>
>>> Thanks,
>>> Ming
>>
>> Hi, Ming,
>>
>> Sorry for the delay. I tested your V2 against Linux 4.20-rc1 and
>> everything seems fine. FWIW, V2 is also
>>
>> Tested-by: Rui Salvaterra <rsalvaterra@gmail.com>
> 
> Rui, thanks for your test on V2.
> 
> Jens, what do you think about this patchset?

I waffled on this a bit... I generally don't like cleanup patches for
fixes that are going into the current series.  Those kinds of patches
should be as short and sweet as possible, and cleanup part usually ends
up being the problematic one, ironically enough. But it looks fine to
me, so I've put it in.

-- 
Jens Axboe