FAILED: patch "[PATCH] block: don't deal with discard limit in" failed to apply to 4.14-stable tree

FAILED: patch "[PATCH] block: don't deal with discard limit in" failed to apply to 4.14-stable tree

[gregkh]

The patch below does not apply to the 4.14-stable tree.
If someone wants it applied there, or to any other stable or longterm
tree, then please email the backport, including the original git commit
id to <stable@vger.kernel.org>.

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

>From 744889b7cbb56a64f957e65ade7cb65fe3f35714 Mon Sep 17 00:00:00 2001
From: Ming Lei <ming.lei@redhat.com>
Date: Fri, 12 Oct 2018 15:53:10 +0800
Subject: [PATCH] block: don't deal with discard limit in
 blkdev_issue_discard()

blk_queue_split() does respect this limit via bio splitting, so no
need to do that in blkdev_issue_discard(), then we can align to
normal bio submit(bio_add_page() & submit_bio()).

More importantly, this patch fixes one issue introduced in a22c4d7e34402cc
("block: re-add discard_granularity and alignment checks"), in which
zero discard bio may be generated in case of zero alignment.

Fixes: a22c4d7e34402ccdf3 ("block: re-add discard_granularity and alignment checks")
Cc: stable@vger.kernel.org
Cc: Ming Lin <ming.l@ssi.samsung.com>
Cc: Mike Snitzer <snitzer@redhat.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Xiao Ni <xni@redhat.com>
Tested-by: Mariusz Dabrowski <mariusz.dabrowski@intel.com>
Signed-off-by: Ming Lei <ming.lei@redhat.com>
Signed-off-by: Jens Axboe <axboe@kernel.dk>

diff --git a/block/blk-lib.c b/block/blk-lib.c
index d1b9dd03da25..bbd44666f2b5 100644
--- a/block/blk-lib.c
+++ b/block/blk-lib.c
@@ -29,9 +29,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 granularity;
 	unsigned int op;
-	int alignment;
 	sector_t bs_mask;
 
 	if (!q)
@@ -54,38 +52,16 @@ int __blkdev_issue_discard(struct block_device *bdev, sector_t sector,
 	if ((sector | nr_sects) & bs_mask)
 		return -EINVAL;
 
-	/* Zero-sector (unknown) and one-sector granularities are the same.  */
-	granularity = max(q->limits.discard_granularity >> 9, 1U);
-	alignment = (bdev_discard_alignment(bdev) >> 9) % granularity;
-
 	while (nr_sects) {
-		unsigned int req_sects;
-		sector_t end_sect, tmp;
+		unsigned int req_sects = nr_sects;
+		sector_t end_sect;
 
-		/*
-		 * Issue in chunks of the user defined max discard setting,
-		 * ensuring that bi_size doesn't overflow
-		 */
-		req_sects = min_t(sector_t, nr_sects,
-					q->limits.max_discard_sectors);
 		if (!req_sects)
 			goto fail;
 		if (req_sects > UINT_MAX >> 9)
 			req_sects = UINT_MAX >> 9;
 
-		/*
-		 * If splitting a request, and the next starting sector would be
-		 * misaligned, stop the discard at the previous aligned sector.
-		 */
 		end_sect = sector + req_sects;
-		tmp = end_sect;
-		if (req_sects < nr_sects &&
-		    sector_div(tmp, granularity) != alignment) {
-			end_sect = end_sect - alignment;
-			sector_div(end_sect, granularity);
-			end_sect = end_sect * granularity + alignment;
-			req_sects = end_sect - sector;
-		}
 
 		bio = next_bio(bio, 0, gfp_mask);
 		bio->bi_iter.bi_sector = sector;

Re: FAILED: patch "[PATCH] block: don't deal with discard limit in" failed to apply to 4.14-stable tree

[Sudip Mukherjee]

[-- Attachment #1: Type: text/plain, Size: 747 bytes --]

Hi Greg,

On Thu, Nov 08, 2018 at 09:23:10AM -0800, gregkh@linuxfoundation.org wrote:
> 
> The patch below does not apply to the 4.14-stable tree.
> If someone wants it applied there, or to any other stable or longterm
> tree, then please email the backport, including the original git commit
> id to <stable@vger.kernel.org>.

Instead of backporting it was easier to add the commits it depends on.
1) af097f5d199e ("block: break discard submissions into the user defined size")
It is not marked for stable but seems it can be in stable.

2) b88aef36b87c ("block: fix infinite loop if the device loses discard capability")
It is marked for stable but is not there in 4.14-stable.

All are attached to this mail for you to apply.

--
Regards
Sudip

[-- Attachment #2: 0001-block-break-discard-submissions-into-the-user-define.patch --]
[-- Type: text/x-diff, Size: 1522 bytes --]

>From 1be4fbd23b620d384219ba0a1b2ed14a09ac1c01 Mon Sep 17 00:00:00 2001
From: Jens Axboe <axboe@kernel.dk>
Date: Tue, 8 May 2018 15:09:41 -0600
Subject: [PATCH 1/3] block: break discard submissions into the user defined
 size

commit af097f5d199e2aa3ab3ef777f0716e487b8f7b08 upstream

Don't build discards bigger than what the user asked for, if the
user decided to limit the size by writing to 'discard_max_bytes'.

Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
Reviewed-by: Omar Sandoval <osandov@fb.com>
Signed-off-by: Jens Axboe <axboe@kernel.dk>
Signed-off-by: Sudip Mukherjee <sudipm.mukherjee@gmail.com>
---
 block/blk-lib.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/block/blk-lib.c b/block/blk-lib.c
index 2bc544ce3d2e..53a45663e688 100644
--- a/block/blk-lib.c
+++ b/block/blk-lib.c
@@ -59,10 +59,16 @@ int __blkdev_issue_discard(struct block_device *bdev, sector_t sector,
 		unsigned int req_sects;
 		sector_t end_sect, tmp;
 
-		/* Make sure bi_size doesn't overflow */
-		req_sects = min_t(sector_t, nr_sects, UINT_MAX >> 9);
+		/*
+		 * Issue in chunks of the user defined max discard setting,
+		 * ensuring that bi_size doesn't overflow
+		 */
+		req_sects = min_t(sector_t, nr_sects,
+					q->limits.max_discard_sectors);
+		if (req_sects > UINT_MAX >> 9)
+			req_sects = UINT_MAX >> 9;
 
-		/**
+		/*
 		 * If splitting a request, and the next starting sector would be
 		 * misaligned, stop the discard at the previous aligned sector.
 		 */
-- 
2.11.0


[-- Attachment #3: 0002-block-fix-infinite-loop-if-the-device-loses-discard-.patch --]
[-- Type: text/x-diff, Size: 1599 bytes --]

>From d4a1fe129ccf82570d7add250b77a9b9ee878561 Mon Sep 17 00:00:00 2001
From: Mikulas Patocka <mpatocka@redhat.com>
Date: Tue, 3 Jul 2018 13:34:22 -0400
Subject: [PATCH 2/3] block: fix infinite loop if the device loses discard
 capability

commit b88aef36b87c9787a4db724923ec4f57dfd513f3 upstream

If __blkdev_issue_discard is in progress and a device mapper device is
reloaded with a table that doesn't support discard,
q->limits.max_discard_sectors is set to zero. This results in infinite
loop in __blkdev_issue_discard.

This patch checks if max_discard_sectors is zero and aborts with
-EOPNOTSUPP.

Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
Tested-by: Zdenek Kabelac <mpatocka@redhat.com>
Cc: stable@vger.kernel.org
Signed-off-by: Jens Axboe <axboe@kernel.dk>
Signed-off-by: Sudip Mukherjee <sudipm.mukherjee@gmail.com>
---
 block/blk-lib.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/block/blk-lib.c b/block/blk-lib.c
index 53a45663e688..0bdc77888dc5 100644
--- a/block/blk-lib.c
+++ b/block/blk-lib.c
@@ -65,6 +65,8 @@ int __blkdev_issue_discard(struct block_device *bdev, sector_t sector,
 		 */
 		req_sects = min_t(sector_t, nr_sects,
 					q->limits.max_discard_sectors);
+		if (!req_sects)
+			goto fail;
 		if (req_sects > UINT_MAX >> 9)
 			req_sects = UINT_MAX >> 9;
 
@@ -102,6 +104,14 @@ int __blkdev_issue_discard(struct block_device *bdev, sector_t sector,
 
 	*biop = bio;
 	return 0;
+
+fail:
+	if (bio) {
+		submit_bio_wait(bio);
+		bio_put(bio);
+	}
+	*biop = NULL;
+	return -EOPNOTSUPP;
 }
 EXPORT_SYMBOL(__blkdev_issue_discard);
 
-- 
2.11.0


[-- Attachment #4: 0003-block-don-t-deal-with-discard-limit-in-blkdev_issue_.patch --]
[-- Type: text/x-diff, Size: 2969 bytes --]

>From 4fe24d18d61cdbf07db6969e506c1751c0fdfc42 Mon Sep 17 00:00:00 2001
From: Ming Lei <ming.lei@redhat.com>
Date: Fri, 12 Oct 2018 15:53:10 +0800
Subject: [PATCH 3/3] block: don't deal with discard limit in
 blkdev_issue_discard()

commit 744889b7cbb56a64f957e65ade7cb65fe3f35714 upstream

blk_queue_split() does respect this limit via bio splitting, so no
need to do that in blkdev_issue_discard(), then we can align to
normal bio submit(bio_add_page() & submit_bio()).

More importantly, this patch fixes one issue introduced in a22c4d7e34402cc
("block: re-add discard_granularity and alignment checks"), in which
zero discard bio may be generated in case of zero alignment.

Fixes: a22c4d7e34402ccdf3 ("block: re-add discard_granularity and alignment checks")
Cc: stable@vger.kernel.org
Cc: Ming Lin <ming.l@ssi.samsung.com>
Cc: Mike Snitzer <snitzer@redhat.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Xiao Ni <xni@redhat.com>
Tested-by: Mariusz Dabrowski <mariusz.dabrowski@intel.com>
Signed-off-by: Ming Lei <ming.lei@redhat.com>
Signed-off-by: Jens Axboe <axboe@kernel.dk>
Signed-off-by: Sudip Mukherjee <sudipm.mukherjee@gmail.com>
---
 block/blk-lib.c | 28 ++--------------------------
 1 file changed, 2 insertions(+), 26 deletions(-)

diff --git a/block/blk-lib.c b/block/blk-lib.c
index 0bdc77888dc5..3e334b4575ab 100644
--- a/block/blk-lib.c
+++ b/block/blk-lib.c
@@ -29,9 +29,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 granularity;
 	unsigned int op;
-	int alignment;
 	sector_t bs_mask;
 
 	if (!q)
@@ -51,38 +49,16 @@ int __blkdev_issue_discard(struct block_device *bdev, sector_t sector,
 	if ((sector | nr_sects) & bs_mask)
 		return -EINVAL;
 
-	/* Zero-sector (unknown) and one-sector granularities are the same.  */
-	granularity = max(q->limits.discard_granularity >> 9, 1U);
-	alignment = (bdev_discard_alignment(bdev) >> 9) % granularity;
-
 	while (nr_sects) {
-		unsigned int req_sects;
-		sector_t end_sect, tmp;
+		unsigned int req_sects = nr_sects;
+		sector_t end_sect;
 
-		/*
-		 * Issue in chunks of the user defined max discard setting,
-		 * ensuring that bi_size doesn't overflow
-		 */
-		req_sects = min_t(sector_t, nr_sects,
-					q->limits.max_discard_sectors);
 		if (!req_sects)
 			goto fail;
 		if (req_sects > UINT_MAX >> 9)
 			req_sects = UINT_MAX >> 9;
 
-		/*
-		 * If splitting a request, and the next starting sector would be
-		 * misaligned, stop the discard at the previous aligned sector.
-		 */
 		end_sect = sector + req_sects;
-		tmp = end_sect;
-		if (req_sects < nr_sects &&
-		    sector_div(tmp, granularity) != alignment) {
-			end_sect = end_sect - alignment;
-			sector_div(end_sect, granularity);
-			end_sect = end_sect * granularity + alignment;
-			req_sects = end_sect - sector;
-		}
 
 		bio = next_bio(bio, 0, gfp_mask);
 		bio->bi_iter.bi_sector = sector;
-- 
2.11.0

Re: FAILED: patch "[PATCH] block: don't deal with discard limit in" failed to apply to 4.14-stable tree

[Sasha Levin]

On Thu, Dec 20, 2018 at 08:53:47PM +0000, Sudip Mukherjee wrote:
>Hi Greg,
>
>On Thu, Nov 08, 2018 at 09:23:10AM -0800, gregkh@linuxfoundation.org wrote:
>>
>> The patch below does not apply to the 4.14-stable tree.
>> If someone wants it applied there, or to any other stable or longterm
>> tree, then please email the backport, including the original git commit
>> id to <stable@vger.kernel.org>.
>
>Instead of backporting it was easier to add the commits it depends on.
>1) af097f5d199e ("block: break discard submissions into the user defined size")
>It is not marked for stable but seems it can be in stable.
>
>2) b88aef36b87c ("block: fix infinite loop if the device loses discard capability")
>It is marked for stable but is not there in 4.14-stable.
>
>All are attached to this mail for you to apply.

I've queued both for 4.14 and 4.9. It seems that 4.4 needs something
more complex than that.

--
Thanks,
Sasha

Re: FAILED: patch "[PATCH] block: don't deal with discard limit in" failed to apply to 4.14-stable tree

[Sudip Mukherjee]

[-- Attachment #1: Type: text/plain, Size: 1091 bytes --]

On Fri, Dec 21, 2018 at 06:20:26PM -0500, Sasha Levin wrote:
> On Thu, Dec 20, 2018 at 08:53:47PM +0000, Sudip Mukherjee wrote:
> > Hi Greg,
> > 
> > On Thu, Nov 08, 2018 at 09:23:10AM -0800, gregkh@linuxfoundation.org wrote:
> > > 
> > > The patch below does not apply to the 4.14-stable tree.
> > > If someone wants it applied there, or to any other stable or longterm
> > > tree, then please email the backport, including the original git commit
> > > id to <stable@vger.kernel.org>.
> > 
> > Instead of backporting it was easier to add the commits it depends on.
> > 1) af097f5d199e ("block: break discard submissions into the user defined size")
> > It is not marked for stable but seems it can be in stable.
> > 
> > 2) b88aef36b87c ("block: fix infinite loop if the device loses discard capability")
> > It is marked for stable but is not there in 4.14-stable.
> > 
> > All are attached to this mail for you to apply.
> 
> I've queued both for 4.14 and 4.9. It seems that 4.4 needs something
> more complex than that.

Backported patch attached for 4.4-stable tree.

--
Regards
Sudip

[-- Attachment #2: 0001-block-break-discard-submissions-into-the-user-define.patch --]
[-- Type: text/x-diff, Size: 1397 bytes --]

>From 2377ab657d423978108dfdaacd443adfa2613d2b Mon Sep 17 00:00:00 2001
From: Jens Axboe <axboe@kernel.dk>
Date: Tue, 8 May 2018 15:09:41 -0600
Subject: [PATCH 1/2] block: break discard submissions into the user defined size

commit af097f5d199e2aa3ab3ef777f0716e487b8f7b08 upstream

Don't build discards bigger than what the user asked for, if the
user decided to limit the size by writing to 'discard_max_bytes'.

Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
Reviewed-by: Omar Sandoval <osandov@fb.com>
Signed-off-by: Jens Axboe <axboe@kernel.dk>
Signed-off-by: Sudip Mukherjee <sudipm.mukherjee@gmail.com>
---
 block/blk-lib.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/block/blk-lib.c b/block/blk-lib.c
index 9ebf65379556..73ca3ef8ca5a 100644
--- a/block/blk-lib.c
+++ b/block/blk-lib.c
@@ -81,8 +81,14 @@ int blkdev_issue_discard(struct block_device *bdev, sector_t sector,
 			break;
 		}
 
-		/* Make sure bi_size doesn't overflow */
-		req_sects = min_t(sector_t, nr_sects, UINT_MAX >> 9);
+		/*
+		 * Issue in chunks of the user defined max discard setting,
+		 * ensuring that bi_size doesn't overflow
+		 */
+		req_sects = min_t(sector_t, nr_sects,
+					q->limits.max_discard_sectors);
+		if (req_sects > UINT_MAX >> 9)
+			req_sects = UINT_MAX >> 9;
 
 		/*
 		 * If splitting a request, and the next starting sector would be
-- 
2.11.0


[-- Attachment #3: 0002-block-re-add-discard_granularity-and-alignment-check.patch --]
[-- Type: text/x-diff, Size: 3362 bytes --]

>From 2e507841428638b7fac3cea954e675b780a861a3 Mon Sep 17 00:00:00 2001
From: Ming Lin <ming.l@ssi.samsung.com>
Date: Thu, 22 Oct 2015 09:59:42 -0700
Subject: [PATCH 2/2] block: re-add discard_granularity and alignment checks

commit 744889b7cbb56a64f957e65ade7cb65fe3f35714 upstream

In commit b49a087("block: remove split code in
blkdev_issue_{discard,write_same}"), discard_granularity and alignment
checks were removed. Ideally, with bio late splitting, the upper layers
shouldn't need to depend on device's limits.

Christoph reported a discard regression on the HGST Ultrastar SN100 NVMe
device when mkfs.xfs. We have not found the root cause yet.

This patch re-adds discard_granularity and alignment checks by reverting
the related changes in commit b49a087. The good thing is now we can
remove the 2G discard size cap and just use UINT_MAX to avoid bi_size
overflow.

Reviewed-by: Christoph Hellwig <hch@lst.de>
Tested-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Ming Lin <ming.l@ssi.samsung.com>
Reviewed-by: Mike Snitzer <snitzer@redhat.com>
Signed-off-by: Jens Axboe <axboe@fb.com>
Signed-off-by: Sudip Mukherjee <sudipm.mukherjee@gmail.com>
---
 block/blk-lib.c | 28 ++--------------------------
 1 file changed, 2 insertions(+), 26 deletions(-)

diff --git a/block/blk-lib.c b/block/blk-lib.c
index 73ca3ef8ca5a..a42e680ef8f7 100644
--- a/block/blk-lib.c
+++ b/block/blk-lib.c
@@ -43,8 +43,6 @@ int blkdev_issue_discard(struct block_device *bdev, sector_t sector,
 	DECLARE_COMPLETION_ONSTACK(wait);
 	struct request_queue *q = bdev_get_queue(bdev);
 	int type = REQ_WRITE | REQ_DISCARD;
-	unsigned int granularity;
-	int alignment;
 	struct bio_batch bb;
 	struct bio *bio;
 	int ret = 0;
@@ -56,10 +54,6 @@ int blkdev_issue_discard(struct block_device *bdev, sector_t sector,
 	if (!blk_queue_discard(q))
 		return -EOPNOTSUPP;
 
-	/* Zero-sector (unknown) and one-sector granularities are the same.  */
-	granularity = max(q->limits.discard_granularity >> 9, 1U);
-	alignment = (bdev_discard_alignment(bdev) >> 9) % granularity;
-
 	if (flags & BLKDEV_DISCARD_SECURE) {
 		if (!blk_queue_secdiscard(q))
 			return -EOPNOTSUPP;
@@ -72,8 +66,8 @@ int blkdev_issue_discard(struct block_device *bdev, sector_t sector,
 
 	blk_start_plug(&plug);
 	while (nr_sects) {
-		unsigned int req_sects;
-		sector_t end_sect, tmp;
+		unsigned int req_sects = nr_sects;
+		sector_t end_sect;
 
 		bio = bio_alloc(gfp_mask, 1);
 		if (!bio) {
@@ -81,28 +75,10 @@ int blkdev_issue_discard(struct block_device *bdev, sector_t sector,
 			break;
 		}
 
-		/*
-		 * Issue in chunks of the user defined max discard setting,
-		 * ensuring that bi_size doesn't overflow
-		 */
-		req_sects = min_t(sector_t, nr_sects,
-					q->limits.max_discard_sectors);
 		if (req_sects > UINT_MAX >> 9)
 			req_sects = UINT_MAX >> 9;
 
-		/*
-		 * If splitting a request, and the next starting sector would be
-		 * misaligned, stop the discard at the previous aligned sector.
-		 */
 		end_sect = sector + req_sects;
-		tmp = end_sect;
-		if (req_sects < nr_sects &&
-		    sector_div(tmp, granularity) != alignment) {
-			end_sect = end_sect - alignment;
-			sector_div(end_sect, granularity);
-			end_sect = end_sect * granularity + alignment;
-			req_sects = end_sect - sector;
-		}
 
 		bio->bi_iter.bi_sector = sector;
 		bio->bi_end_io = bio_batch_end_io;
-- 
2.11.0

Re: FAILED: patch "[PATCH] block: don't deal with discard limit in" failed to apply to 4.14-stable tree

[Greg KH]

On Sat, Dec 22, 2018 at 10:40:21PM +0000, Sudip Mukherjee wrote:
> On Fri, Dec 21, 2018 at 06:20:26PM -0500, Sasha Levin wrote:
> > On Thu, Dec 20, 2018 at 08:53:47PM +0000, Sudip Mukherjee wrote:
> > > Hi Greg,
> > > 
> > > On Thu, Nov 08, 2018 at 09:23:10AM -0800, gregkh@linuxfoundation.org wrote:
> > > > 
> > > > The patch below does not apply to the 4.14-stable tree.
> > > > If someone wants it applied there, or to any other stable or longterm
> > > > tree, then please email the backport, including the original git commit
> > > > id to <stable@vger.kernel.org>.
> > > 
> > > Instead of backporting it was easier to add the commits it depends on.
> > > 1) af097f5d199e ("block: break discard submissions into the user defined size")
> > > It is not marked for stable but seems it can be in stable.
> > > 
> > > 2) b88aef36b87c ("block: fix infinite loop if the device loses discard capability")
> > > It is marked for stable but is not there in 4.14-stable.
> > > 
> > > All are attached to this mail for you to apply.
> > 
> > I've queued both for 4.14 and 4.9. It seems that 4.4 needs something
> > more complex than that.
> 
> Backported patch attached for 4.4-stable tree.

Thanks for these, now queued up.

greg k-h