[PATCH 0/4] Set max_discard_sectors maximal value to UINT_MAX>>9

[PATCH 0/4] Set max_discard_sectors maximal value to UINT_MAX>>9

[Gwendal Grignou]

Without this change, 'blkdiscard /dev/mmcblk0' would panic:
Because max_discard_sectors was set to 4G sectors, discard request issued
on behalf of blkdiscard would be merged, leading to request larger than 4GB:
[  169.035929] __end_that: dev mmcblk0: type=1, flags=122c8081
[  169.041464]   sector 0, nr/cnr 0/4294966272
[  169.045689]   bio ebb7a380, biotail ebb7aa80, buffer   (null), len 0
[  169.052106] blk_update_request: bio idx 0 >= vcnt 0
[  169.057068] request botched: dev mmcblk0: type=1, flags=122c8081
[  169.063138]   sector 0, nr/cnr 0/4294966272
[  169.067436]   bio ebb7a380, biotail ebb7aa80, buffer   (null), len 0
The IOCTL would be stuck in the kernel, a watchdog would reboot the machine.

After this change, blkdiscard works fine. With blktrace/blkparse, I verified
the discard requests are sane: 8387584 is 4GB in sectors rounded down.
Example of a ~16GB device:
 9368  Q   D 0 + 8387584 [blkdiscard]
    0  m   N cfq9368 alloced
 9368  G   D 0 + 8387584 [blkdiscard]
 9368  P   N [blkdiscard]
 9368  Q   D 8387584 + 8387584 [blkdiscard]
 9368  G   D 8387584 + 8387584 [blkdiscard]
 9368  Q   D 16775168 + 8387584 [blkdiscard]
 9368  G   D 16775168 + 8387584 [blkdiscard]
 9368  Q   D 25162752 + 5372928 [blkdiscard]
 9368  G   D 25162752 + 5372928 [blkdiscard]
 9368  I   D 0 + 8387584 [blkdiscard]
    0  m   N cfq9368 insert_request
    0  m   N cfq9368 add_to_rr
 9368  I   D 8387584 + 8387584 [blkdiscard]
    0  m   N cfq9368 insert_request
 9368  I   D 16775168 + 8387584 [blkdiscard]
    0  m   N cfq9368 insert_request
 9368  I   D 25162752 + 5372928 [blkdiscard]
    0  m   N cfq9368 insert_request
 9368  U   N [blkdiscard] 4
 ...

I fixed the other locations where max_discard_sectors was set to
UINT_MAX and checked the other settings were in units of 512 bytes
sectors.

Gwendal Grignou (4):
  Limit max_discard_sectors to UINT_MAX>>9
  Limit max_discard_sectors to UINT_MAX>>9
  Limit max_discard_sectors to UINT_MAX>>9
  Limit max_discard_sectors to UINT_MAX>>9

 block/blk-lib.c           | 2 +-
 drivers/block/nbd.c       | 2 +-
 drivers/mmc/core/core.c   | 6 ++++--
 drivers/mtd/mtd_blkdevs.c | 2 +-
 4 files changed, 7 insertions(+), 5 deletions(-)

-- 
1.9.1.423.g4596e3a

[PATCH 1/4] Limit max_discard_sectors to UINT_MAX>>9

[Gwendal Grignou]

max_discard_sectors can not be larger than UINT_MAX>>9,
otherwise, there is a risk that discard requests would be merged
into a request larger than 4GB.

Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
---
 drivers/block/nbd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index 2dc3b51..f30128b 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -859,7 +859,7 @@ static int __init nbd_init(void)
 		 */
 		queue_flag_set_unlocked(QUEUE_FLAG_NONROT, disk->queue);
 		disk->queue->limits.discard_granularity = 512;
-		disk->queue->limits.max_discard_sectors = UINT_MAX;
+		disk->queue->limits.max_discard_sectors = UINT_MAX >> 9;
 		disk->queue->limits.discard_zeroes_data = 0;
 		blk_queue_max_hw_sectors(disk->queue, 65536);
 		disk->queue->limits.max_sectors = 256;
-- 
1.9.1.423.g4596e3a

[PATCH 2/4] Limit max_discard_sectors to UINT_MAX>>9

[Gwendal Grignou]

max_discard_sectors can not be larger than UINT_MAX>>9,
otherwise, there is a risk that discard requests would be merged
into a request larger than 4GB.

Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
---
 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 9b5b561..67bb0e7 100644
--- a/block/blk-lib.c
+++ b/block/blk-lib.c
@@ -64,7 +64,7 @@ int blkdev_issue_discard(struct block_device *bdev, sector_t sector,
 	 * Ensure that max_discard_sectors is of the proper
 	 * granularity, so that requests stay aligned after a split.
 	 */
-	max_discard_sectors = min(q->limits.max_discard_sectors, UINT_MAX >> 9);
+	max_discard_sectors = q->limits.max_discard_sectors;
 	max_discard_sectors -= max_discard_sectors % granularity;
 	if (unlikely(!max_discard_sectors)) {
 		/* Avoid infinite loop below. Being cautious never hurts. */
-- 
1.9.1.423.g4596e3a

[PATCH 3/4] Limit max_discard_sectors to UINT_MAX>>9

[Gwendal Grignou]

max_discard_sectors can not be larger than UINT_MAX>>9,
otherwise, there is a risk that discard requests would be merged
into a request larger than 4GB.
Ensure that max_discard_sectors is in unit of sectors.

Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
---
 drivers/mmc/core/core.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 57a2b40..46e7ced 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -2153,7 +2153,7 @@ static unsigned int mmc_do_calc_max_discard(struct mmc_card *card,
 	if (qty == 1)
 		return 1;
 
-	/* Convert qty to sectors */
+	/* Convert qty to bytes */
 	if (card->erase_shift)
 		max_discard = --qty << card->erase_shift;
 	else if (mmc_card_sd(card))
@@ -2170,7 +2170,7 @@ unsigned int mmc_calc_max_discard(struct mmc_card *card)
 	unsigned int max_discard, max_trim;
 
 	if (!host->max_discard_to)
-		return UINT_MAX;
+		return UINT_MAX >> 9;
 
 	/*
 	 * Without erase_group_def set, MMC erase timeout depends on clock
@@ -2188,6 +2188,8 @@ unsigned int mmc_calc_max_discard(struct mmc_card *card)
 	} else if (max_discard < card->erase_size) {
 		max_discard = 0;
 	}
+	/* convert max_discard to sectors */
+	max_discard >>= 9;
 	pr_debug("%s: calculated max. discard sectors %u for timeout %u ms\n",
 		 mmc_hostname(host), max_discard, host->max_discard_to);
 	return max_discard;
-- 
1.9.1.423.g4596e3a

[PATCH 4/4] Limit max_discard_sectors to UINT_MAX>>9

[Gwendal Grignou]

max_discard_sectors can not be larger than UINT_MAX>>9,
otherwise, there is a risk that discard requests would be merged
into a request larger than 4GB.

Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
---
 drivers/mtd/mtd_blkdevs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/mtd_blkdevs.c b/drivers/mtd/mtd_blkdevs.c
index 5073cbc..e74090c 100644
--- a/drivers/mtd/mtd_blkdevs.c
+++ b/drivers/mtd/mtd_blkdevs.c
@@ -416,7 +416,7 @@ int add_mtd_blktrans_dev(struct mtd_blktrans_dev *new)
 
 	if (tr->discard) {
 		queue_flag_set_unlocked(QUEUE_FLAG_DISCARD, new->rq);
-		new->rq->limits.max_discard_sectors = UINT_MAX;
+		new->rq->limits.max_discard_sectors = UINT_MAX >> 9;
 	}
 
 	gd->queue = new->rq;
-- 
1.9.1.423.g4596e3a