* [PATCH] mmc: core: don't set limits.discard_granularity as 0
@ 2020-09-30 16:08 Coly Li
2020-09-30 17:23 ` Adrian Hunter
0 siblings, 1 reply; 6+ messages in thread
From: Coly Li @ 2020-09-30 16:08 UTC (permalink / raw)
To: linux-mmc
Cc: linux-kernel, linux-block, Coly Li, Vicente Bergas, Adrian Hunter,
Ulf Hansson
In mmc_queue_setup_discard() the mmc driver queue's discard_granularity
might be set as 0 (when card->pref_erase > max_discard) while the mmc
device still declares to support discard operation. This is buggy and
triggered the following kernel warning message,
WARNING: CPU: 0 PID: 135 at __blkdev_issue_discard+0x200/0x294
CPU: 0 PID: 135 Comm: f2fs_discard-17 Not tainted 5.9.0-rc6 #1
Hardware name: Google Kevin (DT)
pstate: 00000005 (nzcv daif -PAN -UAO BTYPE=--)
pc : __blkdev_issue_discard+0x200/0x294
lr : __blkdev_issue_discard+0x54/0x294
sp : ffff800011dd3b10
x29: ffff800011dd3b10 x28: 0000000000000000 x27: ffff800011dd3cc4 x26: ffff800011dd3e18 x25: 000000000004e69b x24: 0000000000000c40 x23: ffff0000f1deaaf0 x22: ffff0000f2849200 x21: 00000000002734d8 x20: 0000000000000008 x19: 0000000000000000 x18: 0000000000000000 x17: 0000000000000000 x16: 0000000000000000 x15: 0000000000000000 x14: 0000000000000394 x13: 0000000000000000 x12: 0000000000000000 x11: 0000000000000000 x10: 00000000000008b0 x9 : ffff800011dd3cb0 x8 : 000000000004e69b x7 : 0000000000000000 x6 : ffff0000f1926400 x5 : ffff0000f1940800 x4 : 0000000000000000 x3 : 0000000000000c40 x2 : 0000000000000008 x1 : 00000000002734d8 x0 : 0000000000000000 Call trace:
__blkdev_issue_discard+0x200/0x294
__submit_discard_cmd+0x128/0x374
__issue_discard_cmd_orderly+0x188/0x244
__issue_discard_cmd+0x2e8/0x33c
issue_discard_thread+0xe8/0x2f0
kthread+0x11c/0x120
ret_from_fork+0x10/0x1c
---[ end trace e4c8023d33dfe77a ]---
This patch fixes the issue by setting discard_granularity as SECTOR_SIZE
instead of 0 when (card->pref_erase > max_discard) is true. Now no more
complain from __blkdev_issue_discard() for the improper value of discard
granularity.
Fixes: commit e056a1b5b67b ("mmc: queue: let host controllers specify maximum discard timeout")
Reported-by: Vicente Bergas <vicencb@gmail.com>
Signed-off-by: Coly Li <colyli@suse.de>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Ulf Hansson <ulf.hansson@linaro.org>
---
drivers/mmc/core/queue.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mmc/core/queue.c b/drivers/mmc/core/queue.c
index 6c022ef0f84d..350d0cc4ee62 100644
--- a/drivers/mmc/core/queue.c
+++ b/drivers/mmc/core/queue.c
@@ -190,7 +190,7 @@ static void mmc_queue_setup_discard(struct request_queue *q,
q->limits.discard_granularity = card->pref_erase << 9;
/* granularity must not be greater than max. discard */
if (card->pref_erase > max_discard)
- q->limits.discard_granularity = 0;
+ q->limits.discard_granularity = SECTOR_SIZE;
if (mmc_can_secure_erase_trim(card))
blk_queue_flag_set(QUEUE_FLAG_SECERASE, q);
}
--
2.26.2
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] mmc: core: don't set limits.discard_granularity as 0 2020-09-30 16:08 [PATCH] mmc: core: don't set limits.discard_granularity as 0 Coly Li @ 2020-09-30 17:23 ` Adrian Hunter 2020-10-01 4:36 ` Coly Li 0 siblings, 1 reply; 6+ messages in thread From: Adrian Hunter @ 2020-09-30 17:23 UTC (permalink / raw) To: Coly Li, linux-mmc; +Cc: linux-kernel, linux-block, Vicente Bergas, Ulf Hansson On 30/09/20 7:08 pm, Coly Li wrote: > In mmc_queue_setup_discard() the mmc driver queue's discard_granularity > might be set as 0 (when card->pref_erase > max_discard) while the mmc > device still declares to support discard operation. This is buggy and > triggered the following kernel warning message, > > WARNING: CPU: 0 PID: 135 at __blkdev_issue_discard+0x200/0x294 > CPU: 0 PID: 135 Comm: f2fs_discard-17 Not tainted 5.9.0-rc6 #1 > Hardware name: Google Kevin (DT) > pstate: 00000005 (nzcv daif -PAN -UAO BTYPE=--) > pc : __blkdev_issue_discard+0x200/0x294 > lr : __blkdev_issue_discard+0x54/0x294 > sp : ffff800011dd3b10 > x29: ffff800011dd3b10 x28: 0000000000000000 x27: ffff800011dd3cc4 x26: ffff800011dd3e18 x25: 000000000004e69b x24: 0000000000000c40 x23: ffff0000f1deaaf0 x22: ffff0000f2849200 x21: 00000000002734d8 x20: 0000000000000008 x19: 0000000000000000 x18: 0000000000000000 x17: 0000000000000000 x16: 0000000000000000 x15: 0000000000000000 x14: 0000000000000394 x13: 0000000000000000 x12: 0000000000000000 x11: 0000000000000000 x10: 00000000000008b0 x9 : ffff800011dd3cb0 x8 : 000000000004e69b x7 : 0000000000000000 x6 : ffff0000f1926400 x5 : ffff0000f1940800 x4 : 0000000000000000 x3 : 0000000000000c40 x2 : 0000000000000008 x1 : 00000000002734d8 x0 : 0000000000000000 Call trace: > __blkdev_issue_discard+0x200/0x294 > __submit_discard_cmd+0x128/0x374 > __issue_discard_cmd_orderly+0x188/0x244 > __issue_discard_cmd+0x2e8/0x33c > issue_discard_thread+0xe8/0x2f0 > kthread+0x11c/0x120 > ret_from_fork+0x10/0x1c > ---[ end trace e4c8023d33dfe77a ]--- > > This patch fixes the issue by setting discard_granularity as SECTOR_SIZE > instead of 0 when (card->pref_erase > max_discard) is true. Now no more > complain from __blkdev_issue_discard() for the improper value of discard > granularity. > > Fixes: commit e056a1b5b67b ("mmc: queue: let host controllers specify maximum discard timeout") That "Fixes" tag is a bit misleading. For some time, the block layer had no problem with discard_granularity of zero, and blk_bio_discard_split() still doesn't (see below). static struct bio *blk_bio_discard_split(struct request_queue *q, struct bio *bio, struct bio_set *bs, unsigned *nsegs) { unsigned int max_discard_sectors, granularity; int alignment; sector_t tmp; unsigned split_sectors; *nsegs = 1; /* Zero-sector (unknown) and one-sector granularities are the same. */ granularity = max(q->limits.discard_granularity >> 9, 1U); > Reported-by: Vicente Bergas <vicencb@gmail.com> > Signed-off-by: Coly Li <colyli@suse.de> > Cc: Adrian Hunter <adrian.hunter@intel.com> > Cc: Ulf Hansson <ulf.hansson@linaro.org> > --- > drivers/mmc/core/queue.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/mmc/core/queue.c b/drivers/mmc/core/queue.c > index 6c022ef0f84d..350d0cc4ee62 100644 > --- a/drivers/mmc/core/queue.c > +++ b/drivers/mmc/core/queue.c > @@ -190,7 +190,7 @@ static void mmc_queue_setup_discard(struct request_queue *q, > q->limits.discard_granularity = card->pref_erase << 9; > /* granularity must not be greater than max. discard */ > if (card->pref_erase > max_discard) > - q->limits.discard_granularity = 0; > + q->limits.discard_granularity = SECTOR_SIZE; > if (mmc_can_secure_erase_trim(card)) > blk_queue_flag_set(QUEUE_FLAG_SECERASE, q); > } > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mmc: core: don't set limits.discard_granularity as 0 2020-09-30 17:23 ` Adrian Hunter @ 2020-10-01 4:36 ` Coly Li 2020-10-01 6:14 ` Adrian Hunter 0 siblings, 1 reply; 6+ messages in thread From: Coly Li @ 2020-10-01 4:36 UTC (permalink / raw) To: Adrian Hunter Cc: linux-mmc, linux-kernel, linux-block, Vicente Bergas, Ulf Hansson, Martin K. Petersen On 2020/10/1 01:23, Adrian Hunter wrote: > On 30/09/20 7:08 pm, Coly Li wrote: >> In mmc_queue_setup_discard() the mmc driver queue's discard_granularity >> might be set as 0 (when card->pref_erase > max_discard) while the mmc >> device still declares to support discard operation. This is buggy and >> triggered the following kernel warning message, >> >> WARNING: CPU: 0 PID: 135 at __blkdev_issue_discard+0x200/0x294 >> CPU: 0 PID: 135 Comm: f2fs_discard-17 Not tainted 5.9.0-rc6 #1 >> Hardware name: Google Kevin (DT) >> pstate: 00000005 (nzcv daif -PAN -UAO BTYPE=--) >> pc : __blkdev_issue_discard+0x200/0x294 >> lr : __blkdev_issue_discard+0x54/0x294 >> sp : ffff800011dd3b10 >> x29: ffff800011dd3b10 x28: 0000000000000000 x27: ffff800011dd3cc4 x26: ffff800011dd3e18 x25: 000000000004e69b x24: 0000000000000c40 x23: ffff0000f1deaaf0 x22: ffff0000f2849200 x21: 00000000002734d8 x20: 0000000000000008 x19: 0000000000000000 x18: 0000000000000000 x17: 0000000000000000 x16: 0000000000000000 x15: 0000000000000000 x14: 0000000000000394 x13: 0000000000000000 x12: 0000000000000000 x11: 0000000000000000 x10: 00000000000008b0 x9 : ffff800011dd3cb0 x8 : 000000000004e69b x7 : 0000000000000000 x6 : ffff0000f1926400 x5 : ffff0000f1940800 x4 : 0000000000000000 x3 : 0000000000000c40 x2 : 0000000000000008 x1 : 00000000002734d8 x0 : 0000000000000000 Call trace: >> __blkdev_issue_discard+0x200/0x294 >> __submit_discard_cmd+0x128/0x374 >> __issue_discard_cmd_orderly+0x188/0x244 >> __issue_discard_cmd+0x2e8/0x33c >> issue_discard_thread+0xe8/0x2f0 >> kthread+0x11c/0x120 >> ret_from_fork+0x10/0x1c >> ---[ end trace e4c8023d33dfe77a ]--- >> >> This patch fixes the issue by setting discard_granularity as SECTOR_SIZE >> instead of 0 when (card->pref_erase > max_discard) is true. Now no more >> complain from __blkdev_issue_discard() for the improper value of discard >> granularity. >> >> Fixes: commit e056a1b5b67b ("mmc: queue: let host controllers specify maximum discard timeout") > > That "Fixes" tag is a bit misleading. For some time, the block layer had > no problem with discard_granularity of zero, and blk_bio_discard_split() > still doesn't (see below). > > static struct bio *blk_bio_discard_split(struct request_queue *q, > struct bio *bio, > struct bio_set *bs, > unsigned *nsegs) > { > unsigned int max_discard_sectors, granularity; > int alignment; > sector_t tmp; > unsigned split_sectors; > > *nsegs = 1; > > /* Zero-sector (unknown) and one-sector granularities are the same. */ > granularity = max(q->limits.discard_granularity >> 9, 1U); > From Documentation/block/queue-sysfs.rst, the discard_granularity is described as, discard_granularity (RO) ------------------------ This shows the size of internal allocation of the device in bytes, if reported by the device. A value of '0' means device does not support the discard functionality. And from Documentation/ABI/testing/sysfs-block, the discard_granularity is described as, What: /sys/block/<disk>/queue/discard_granularity Date: May 2011 Contact: Martin K. Petersen <martin.petersen@oracle.com> Description: Devices that support discard functionality may internally allocate space using units that are bigger than the logical block size. The discard_granularity parameter indicates the size of the internal allocation unit in bytes if reported by the device. Otherwise the discard_granularity will be set to match the device's physical block size. A discard_granularity of 0 means that the device does not support discard functionality. Therefore I took it as a bug when a driver sets its queue discard_granularity as 0 but still announces to support discard operation. But if you don't like the Fixes: tag, it is OK for me to remove it in next version. (CC Martin because he is the origin of the above information) Thanks. Coly Li ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mmc: core: don't set limits.discard_granularity as 0 2020-10-01 4:36 ` Coly Li @ 2020-10-01 6:14 ` Adrian Hunter 2020-10-01 6:29 ` Coly Li 0 siblings, 1 reply; 6+ messages in thread From: Adrian Hunter @ 2020-10-01 6:14 UTC (permalink / raw) To: Coly Li Cc: linux-mmc, linux-kernel, linux-block, Vicente Bergas, Ulf Hansson, Martin K. Petersen On 1/10/20 7:36 am, Coly Li wrote: > On 2020/10/1 01:23, Adrian Hunter wrote: >> On 30/09/20 7:08 pm, Coly Li wrote: >>> In mmc_queue_setup_discard() the mmc driver queue's discard_granularity >>> might be set as 0 (when card->pref_erase > max_discard) while the mmc >>> device still declares to support discard operation. This is buggy and >>> triggered the following kernel warning message, >>> >>> WARNING: CPU: 0 PID: 135 at __blkdev_issue_discard+0x200/0x294 >>> CPU: 0 PID: 135 Comm: f2fs_discard-17 Not tainted 5.9.0-rc6 #1 >>> Hardware name: Google Kevin (DT) >>> pstate: 00000005 (nzcv daif -PAN -UAO BTYPE=--) >>> pc : __blkdev_issue_discard+0x200/0x294 >>> lr : __blkdev_issue_discard+0x54/0x294 >>> sp : ffff800011dd3b10 >>> x29: ffff800011dd3b10 x28: 0000000000000000 x27: ffff800011dd3cc4 x26: ffff800011dd3e18 x25: 000000000004e69b x24: 0000000000000c40 x23: ffff0000f1deaaf0 x22: ffff0000f2849200 x21: 00000000002734d8 x20: 0000000000000008 x19: 0000000000000000 x18: 0000000000000000 x17: 0000000000000000 x16: 0000000000000000 x15: 0000000000000000 x14: 0000000000000394 x13: 0000000000000000 x12: 0000000000000000 x11: 0000000000000000 x10: 00000000000008b0 x9 : ffff800011dd3cb0 x8 : 000000000004e69b x7 : 0000000000000000 x6 : ffff0000f1926400 x5 : ffff0000f1940800 x4 : 0000000000000000 x3 : 0000000000000c40 x2 : 0000000000000008 x1 : 00000000002734d8 x0 : 0000000000000000 Call trace: >>> __blkdev_issue_discard+0x200/0x294 >>> __submit_discard_cmd+0x128/0x374 >>> __issue_discard_cmd_orderly+0x188/0x244 >>> __issue_discard_cmd+0x2e8/0x33c >>> issue_discard_thread+0xe8/0x2f0 >>> kthread+0x11c/0x120 >>> ret_from_fork+0x10/0x1c >>> ---[ end trace e4c8023d33dfe77a ]--- >>> >>> This patch fixes the issue by setting discard_granularity as SECTOR_SIZE >>> instead of 0 when (card->pref_erase > max_discard) is true. Now no more >>> complain from __blkdev_issue_discard() for the improper value of discard >>> granularity. >>> >>> Fixes: commit e056a1b5b67b ("mmc: queue: let host controllers specify maximum discard timeout") >> >> That "Fixes" tag is a bit misleading. For some time, the block layer had >> no problem with discard_granularity of zero, and blk_bio_discard_split() >> still doesn't (see below). >> >> static struct bio *blk_bio_discard_split(struct request_queue *q, >> struct bio *bio, >> struct bio_set *bs, >> unsigned *nsegs) >> { >> unsigned int max_discard_sectors, granularity; >> int alignment; >> sector_t tmp; >> unsigned split_sectors; >> >> *nsegs = 1; >> >> /* Zero-sector (unknown) and one-sector granularities are the same. */ >> granularity = max(q->limits.discard_granularity >> 9, 1U); >> > >>From Documentation/block/queue-sysfs.rst, the discard_granularity is > described as, > > discard_granularity (RO) > ------------------------ > This shows the size of internal allocation of the device in bytes, if > reported by the device. A value of '0' means device does not support > the discard functionality. > > > And from Documentation/ABI/testing/sysfs-block, the discard_granularity > is described as, > > What: /sys/block/<disk>/queue/discard_granularity > Date: May 2011 > Contact: Martin K. Petersen <martin.petersen@oracle.com> > Description: > Devices that support discard functionality may > internally allocate space using units that are bigger > than the logical block size. The discard_granularity > parameter indicates the size of the internal allocation > unit in bytes if reported by the device. Otherwise the > discard_granularity will be set to match the device's > physical block size. A discard_granularity of 0 means > that the device does not support discard functionality. > > > Therefore I took it as a bug when a driver sets its queue > discard_granularity as 0 but still announces to support discard operation. > > But if you don't like the Fixes: tag, it is OK for me to remove it in > next version. Not at all. I just wrote "a bit misleading" because people might also want to know from what patch things stopped working. > > (CC Martin because he is the origin of the above information) > > Thanks. > > Coly Li > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mmc: core: don't set limits.discard_granularity as 0 2020-10-01 6:14 ` Adrian Hunter @ 2020-10-01 6:29 ` Coly Li 2020-10-01 7:02 ` Adrian Hunter 0 siblings, 1 reply; 6+ messages in thread From: Coly Li @ 2020-10-01 6:29 UTC (permalink / raw) To: Adrian Hunter Cc: linux-mmc, linux-kernel, linux-block, Vicente Bergas, Ulf Hansson, Martin K. Petersen On 2020/10/1 14:14, Adrian Hunter wrote: > On 1/10/20 7:36 am, Coly Li wrote: >> On 2020/10/1 01:23, Adrian Hunter wrote: >>> On 30/09/20 7:08 pm, Coly Li wrote: >>>> In mmc_queue_setup_discard() the mmc driver queue's discard_granularity >>>> might be set as 0 (when card->pref_erase > max_discard) while the mmc >>>> device still declares to support discard operation. This is buggy and >>>> triggered the following kernel warning message, >>>> >>>> WARNING: CPU: 0 PID: 135 at __blkdev_issue_discard+0x200/0x294 >>>> CPU: 0 PID: 135 Comm: f2fs_discard-17 Not tainted 5.9.0-rc6 #1 >>>> Hardware name: Google Kevin (DT) >>>> pstate: 00000005 (nzcv daif -PAN -UAO BTYPE=--) >>>> pc : __blkdev_issue_discard+0x200/0x294 >>>> lr : __blkdev_issue_discard+0x54/0x294 >>>> sp : ffff800011dd3b10 >>>> x29: ffff800011dd3b10 x28: 0000000000000000 x27: ffff800011dd3cc4 x26: ffff800011dd3e18 x25: 000000000004e69b x24: 0000000000000c40 x23: ffff0000f1deaaf0 x22: ffff0000f2849200 x21: 00000000002734d8 x20: 0000000000000008 x19: 0000000000000000 x18: 0000000000000000 x17: 0000000000000000 x16: 0000000000000000 x15: 0000000000000000 x14: 0000000000000394 x13: 0000000000000000 x12: 0000000000000000 x11: 0000000000000000 x10: 00000000000008b0 x9 : ffff800011dd3cb0 x8 : 000000000004e69b x7 : 0000000000000000 x6 : ffff0000f1926400 x5 : ffff0000f1940800 x4 : 0000000000000000 x3 : 0000000000000c40 x2 : 0000000000000008 x1 : 00000000002734d8 x0 : 0000000000000000 Call trace: >>>> __blkdev_issue_discard+0x200/0x294 >>>> __submit_discard_cmd+0x128/0x374 >>>> __issue_discard_cmd_orderly+0x188/0x244 >>>> __issue_discard_cmd+0x2e8/0x33c >>>> issue_discard_thread+0xe8/0x2f0 >>>> kthread+0x11c/0x120 >>>> ret_from_fork+0x10/0x1c >>>> ---[ end trace e4c8023d33dfe77a ]--- >>>> >>>> This patch fixes the issue by setting discard_granularity as SECTOR_SIZE >>>> instead of 0 when (card->pref_erase > max_discard) is true. Now no more >>>> complain from __blkdev_issue_discard() for the improper value of discard >>>> granularity. >>>> >>>> Fixes: commit e056a1b5b67b ("mmc: queue: let host controllers specify maximum discard timeout") >>> >>> That "Fixes" tag is a bit misleading. For some time, the block layer had >>> no problem with discard_granularity of zero, and blk_bio_discard_split() >>> still doesn't (see below). >>> >>> static struct bio *blk_bio_discard_split(struct request_queue *q, >>> struct bio *bio, >>> struct bio_set *bs, >>> unsigned *nsegs) >>> { >>> unsigned int max_discard_sectors, granularity; >>> int alignment; >>> sector_t tmp; >>> unsigned split_sectors; >>> >>> *nsegs = 1; >>> >>> /* Zero-sector (unknown) and one-sector granularities are the same. */ >>> granularity = max(q->limits.discard_granularity >> 9, 1U); >>> >> >> >From Documentation/block/queue-sysfs.rst, the discard_granularity is >> described as, >> >> discard_granularity (RO) >> ------------------------ >> This shows the size of internal allocation of the device in bytes, if >> reported by the device. A value of '0' means device does not support >> the discard functionality. >> >> >> And from Documentation/ABI/testing/sysfs-block, the discard_granularity >> is described as, >> >> What: /sys/block/<disk>/queue/discard_granularity >> Date: May 2011 >> Contact: Martin K. Petersen <martin.petersen@oracle.com> >> Description: >> Devices that support discard functionality may >> internally allocate space using units that are bigger >> than the logical block size. The discard_granularity >> parameter indicates the size of the internal allocation >> unit in bytes if reported by the device. Otherwise the >> discard_granularity will be set to match the device's >> physical block size. A discard_granularity of 0 means >> that the device does not support discard functionality. >> >> >> Therefore I took it as a bug when a driver sets its queue >> discard_granularity as 0 but still announces to support discard operation. >> >> But if you don't like the Fixes: tag, it is OK for me to remove it in >> next version. > > Not at all. I just wrote "a bit misleading" because people might also want > to know from what patch things stopped working. Oh maybe I understand you. Yes, although this fixed patch was bug, but the warning was triggered since the new discard alignment changes got merged. Hmm, maybe I should add the Fixes tag to commit b35fd7422c2f ("block: check queue's limits.discard_granularity in __blkdev_issue_discard()"). How do you think of this commit id ? Thanks. Coly Li ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mmc: core: don't set limits.discard_granularity as 0 2020-10-01 6:29 ` Coly Li @ 2020-10-01 7:02 ` Adrian Hunter 0 siblings, 0 replies; 6+ messages in thread From: Adrian Hunter @ 2020-10-01 7:02 UTC (permalink / raw) To: Coly Li Cc: linux-mmc, linux-kernel, linux-block, Vicente Bergas, Ulf Hansson, Martin K. Petersen On 1/10/20 9:29 am, Coly Li wrote: > On 2020/10/1 14:14, Adrian Hunter wrote: >> On 1/10/20 7:36 am, Coly Li wrote: >>> On 2020/10/1 01:23, Adrian Hunter wrote: >>>> On 30/09/20 7:08 pm, Coly Li wrote: >>>>> In mmc_queue_setup_discard() the mmc driver queue's discard_granularity >>>>> might be set as 0 (when card->pref_erase > max_discard) while the mmc >>>>> device still declares to support discard operation. This is buggy and >>>>> triggered the following kernel warning message, >>>>> >>>>> WARNING: CPU: 0 PID: 135 at __blkdev_issue_discard+0x200/0x294 >>>>> CPU: 0 PID: 135 Comm: f2fs_discard-17 Not tainted 5.9.0-rc6 #1 >>>>> Hardware name: Google Kevin (DT) >>>>> pstate: 00000005 (nzcv daif -PAN -UAO BTYPE=--) >>>>> pc : __blkdev_issue_discard+0x200/0x294 >>>>> lr : __blkdev_issue_discard+0x54/0x294 >>>>> sp : ffff800011dd3b10 >>>>> x29: ffff800011dd3b10 x28: 0000000000000000 x27: ffff800011dd3cc4 x26: ffff800011dd3e18 x25: 000000000004e69b x24: 0000000000000c40 x23: ffff0000f1deaaf0 x22: ffff0000f2849200 x21: 00000000002734d8 x20: 0000000000000008 x19: 0000000000000000 x18: 0000000000000000 x17: 0000000000000000 x16: 0000000000000000 x15: 0000000000000000 x14: 0000000000000394 x13: 0000000000000000 x12: 0000000000000000 x11: 0000000000000000 x10: 00000000000008b0 x9 : ffff800011dd3cb0 x8 : 000000000004e69b x7 : 0000000000000000 x6 : ffff0000f1926400 x5 : ffff0000f1940800 x4 : 0000000000000000 x3 : 0000000000000c40 x2 : 0000000000000008 x1 : 00000000002734d8 x0 : 0000000000000000 Call trace: >>>>> __blkdev_issue_discard+0x200/0x294 >>>>> __submit_discard_cmd+0x128/0x374 >>>>> __issue_discard_cmd_orderly+0x188/0x244 >>>>> __issue_discard_cmd+0x2e8/0x33c >>>>> issue_discard_thread+0xe8/0x2f0 >>>>> kthread+0x11c/0x120 >>>>> ret_from_fork+0x10/0x1c >>>>> ---[ end trace e4c8023d33dfe77a ]--- >>>>> >>>>> This patch fixes the issue by setting discard_granularity as SECTOR_SIZE >>>>> instead of 0 when (card->pref_erase > max_discard) is true. Now no more >>>>> complain from __blkdev_issue_discard() for the improper value of discard >>>>> granularity. >>>>> >>>>> Fixes: commit e056a1b5b67b ("mmc: queue: let host controllers specify maximum discard timeout") >>>> >>>> That "Fixes" tag is a bit misleading. For some time, the block layer had >>>> no problem with discard_granularity of zero, and blk_bio_discard_split() >>>> still doesn't (see below). >>>> >>>> static struct bio *blk_bio_discard_split(struct request_queue *q, >>>> struct bio *bio, >>>> struct bio_set *bs, >>>> unsigned *nsegs) >>>> { >>>> unsigned int max_discard_sectors, granularity; >>>> int alignment; >>>> sector_t tmp; >>>> unsigned split_sectors; >>>> >>>> *nsegs = 1; >>>> >>>> /* Zero-sector (unknown) and one-sector granularities are the same. */ >>>> granularity = max(q->limits.discard_granularity >> 9, 1U); >>>> >>> >>> >From Documentation/block/queue-sysfs.rst, the discard_granularity is >>> described as, >>> >>> discard_granularity (RO) >>> ------------------------ >>> This shows the size of internal allocation of the device in bytes, if >>> reported by the device. A value of '0' means device does not support >>> the discard functionality. >>> >>> >>> And from Documentation/ABI/testing/sysfs-block, the discard_granularity >>> is described as, >>> >>> What: /sys/block/<disk>/queue/discard_granularity >>> Date: May 2011 >>> Contact: Martin K. Petersen <martin.petersen@oracle.com> >>> Description: >>> Devices that support discard functionality may >>> internally allocate space using units that are bigger >>> than the logical block size. The discard_granularity >>> parameter indicates the size of the internal allocation >>> unit in bytes if reported by the device. Otherwise the >>> discard_granularity will be set to match the device's >>> physical block size. A discard_granularity of 0 means >>> that the device does not support discard functionality. >>> >>> >>> Therefore I took it as a bug when a driver sets its queue >>> discard_granularity as 0 but still announces to support discard operation. >>> >>> But if you don't like the Fixes: tag, it is OK for me to remove it in >>> next version. >> >> Not at all. I just wrote "a bit misleading" because people might also want >> to know from what patch things stopped working. > > Oh maybe I understand you. Yes, although this fixed patch was bug, but > the warning was triggered since the new discard alignment changes got > merged. > > Hmm, maybe I should add the Fixes tag to commit b35fd7422c2f ("block: > check queue's limits.discard_granularity in __blkdev_issue_discard()"). > > How do you think of this commit id ? Yes that could be mentioned in the commit message or Fixes or both. With that: Acked-by: Adrian Hunter <adrian.hunter@intel.com> ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-10-01 7:03 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-09-30 16:08 [PATCH] mmc: core: don't set limits.discard_granularity as 0 Coly Li 2020-09-30 17:23 ` Adrian Hunter 2020-10-01 4:36 ` Coly Li 2020-10-01 6:14 ` Adrian Hunter 2020-10-01 6:29 ` Coly Li 2020-10-01 7:02 ` Adrian Hunter
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox