| Message ID | 20171107070259.11664-2-js07.lee@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Please send a long the patch to actually set the flag, else it is just dead code. -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 7 November 2017 at 08:02, Jungseung Lee <js07.lee@samsung.com> wrote: > Some devices return zeroes on read to unmmaped logical blocks. > Use this behavior to implement REQ_OP_WRITES_ZEROES. > > Send TRIM operation for the non-partial erase at a convenient time. This doesn't explain the problem enough. What does unmapped logical blocks means for an eMMC device? Isn't all logical blocks that is accessible being mapped? Please elaborate for my understanding. Also, there is no need to split patch1 and patch2, those can go together. And finally, please follow Christoph's suggestion, so we can see when this new quirk is going to be used. Kind regards Uffe > > Signed-off-by: Jungseung Lee <js07.lee@samsung.com> > --- > drivers/mmc/core/block.c | 8 ++++++-- > drivers/mmc/core/queue.c | 4 ++++ > 2 files changed, 10 insertions(+), 2 deletions(-) > > diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c > index ea80ff4..41c4ec1 100644 > --- a/drivers/mmc/core/block.c > +++ b/drivers/mmc/core/block.c > @@ -1273,9 +1273,10 @@ static void mmc_blk_issue_discard_rq(struct mmc_queue *mq, struct request *req) > struct mmc_card *card = md->queue.card; > unsigned int from, nr, arg; > int err = 0, type = MMC_BLK_DISCARD; > + int force_trim = (req_op(req) == REQ_OP_WRITE_ZEROES); > blk_status_t status = BLK_STS_OK; > > - if (!mmc_can_erase(card)) { > + if (!mmc_can_erase(card) || (!mmc_can_trim(card) && force_trim)) { > status = BLK_STS_NOTSUPP; > goto fail; > } > @@ -1283,7 +1284,9 @@ static void mmc_blk_issue_discard_rq(struct mmc_queue *mq, struct request *req) > from = blk_rq_pos(req); > nr = blk_rq_sectors(req); > > - if (mmc_can_discard(card)) > + if (force_trim) > + arg = MMC_TRIM_ARG; > + else if (mmc_can_discard(card)) > arg = MMC_DISCARD_ARG; > else if (mmc_can_trim(card)) > arg = MMC_TRIM_ARG; > @@ -2032,6 +2035,7 @@ void mmc_blk_issue_rq(struct mmc_queue *mq, struct request *req) > mmc_blk_issue_rw_rq(mq, NULL); > mmc_blk_issue_drv_op(mq, req); > break; > + case REQ_OP_WRITE_ZEROES: > case REQ_OP_DISCARD: > /* > * Complete ongoing async transfer before issuing > diff --git a/drivers/mmc/core/queue.c b/drivers/mmc/core/queue.c > index 4f33d27..dc95b3a 100644 > --- a/drivers/mmc/core/queue.c > +++ b/drivers/mmc/core/queue.c > @@ -144,6 +144,10 @@ static void mmc_queue_setup_discard(struct request_queue *q, > /* granularity must not be greater than max. discard */ > if (card->pref_erase > max_discard) > q->limits.discard_granularity = 0; > + > + if (card->quirks & MMC_QUIRK_UNMAPPED_ZEROES) > + blk_queue_max_write_zeroes_sectors(q, UINT_MAX); > + > if (mmc_can_secure_erase_trim(card)) > queue_flag_set_unlocked(QUEUE_FLAG_SECERASE, q); > } > -- > 2.10.1 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-mmc" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Nov 7, 2017 at 9:54 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote: > On 7 November 2017 at 08:02, Jungseung Lee <js07.lee@samsung.com> wrote: >> Some devices return zeroes on read to unmmaped logical blocks. >> Use this behavior to implement REQ_OP_WRITES_ZEROES. >> >> Send TRIM operation for the non-partial erase at a convenient time. > > This doesn't explain the problem enough. > > What does unmapped logical blocks means for an eMMC device? Isn't all > logical blocks that is accessible being mapped? > Right. There is no unmapped logical block. Only unmapped host address space on eMMC device exist in the spec. "The contents of a write block where the trim function has been applied shall be '0' or '1' depending on different memory technology" I'd like to fix the name as trimmed block. Is it more clear and reasonable? > Please elaborate for my understanding. > > Also, there is no need to split patch1 and patch2, those can go together. > > And finally, please follow Christoph's suggestion, so we can see when > this new quirk is going to be used. > OK, I'll send new patch. thanks for the review. > Kind regards > Uffe > >> >> Signed-off-by: Jungseung Lee <js07.lee@samsung.com> >> --- >> drivers/mmc/core/block.c | 8 ++++++-- >> drivers/mmc/core/queue.c | 4 ++++ >> 2 files changed, 10 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c >> index ea80ff4..41c4ec1 100644 >> --- a/drivers/mmc/core/block.c >> +++ b/drivers/mmc/core/block.c >> @@ -1273,9 +1273,10 @@ static void mmc_blk_issue_discard_rq(struct mmc_queue *mq, struct request *req) >> struct mmc_card *card = md->queue.card; >> unsigned int from, nr, arg; >> int err = 0, type = MMC_BLK_DISCARD; >> + int force_trim = (req_op(req) == REQ_OP_WRITE_ZEROES); >> blk_status_t status = BLK_STS_OK; >> >> - if (!mmc_can_erase(card)) { >> + if (!mmc_can_erase(card) || (!mmc_can_trim(card) && force_trim)) { >> status = BLK_STS_NOTSUPP; >> goto fail; >> } >> @@ -1283,7 +1284,9 @@ static void mmc_blk_issue_discard_rq(struct mmc_queue *mq, struct request *req) >> from = blk_rq_pos(req); >> nr = blk_rq_sectors(req); >> >> - if (mmc_can_discard(card)) >> + if (force_trim) >> + arg = MMC_TRIM_ARG; >> + else if (mmc_can_discard(card)) >> arg = MMC_DISCARD_ARG; >> else if (mmc_can_trim(card)) >> arg = MMC_TRIM_ARG; >> @@ -2032,6 +2035,7 @@ void mmc_blk_issue_rq(struct mmc_queue *mq, struct request *req) >> mmc_blk_issue_rw_rq(mq, NULL); >> mmc_blk_issue_drv_op(mq, req); >> break; >> + case REQ_OP_WRITE_ZEROES: >> case REQ_OP_DISCARD: >> /* >> * Complete ongoing async transfer before issuing >> diff --git a/drivers/mmc/core/queue.c b/drivers/mmc/core/queue.c >> index 4f33d27..dc95b3a 100644 >> --- a/drivers/mmc/core/queue.c >> +++ b/drivers/mmc/core/queue.c >> @@ -144,6 +144,10 @@ static void mmc_queue_setup_discard(struct request_queue *q, >> /* granularity must not be greater than max. discard */ >> if (card->pref_erase > max_discard) >> q->limits.discard_granularity = 0; >> + >> + if (card->quirks & MMC_QUIRK_UNMAPPED_ZEROES) >> + blk_queue_max_write_zeroes_sectors(q, UINT_MAX); >> + >> if (mmc_can_secure_erase_trim(card)) >> queue_flag_set_unlocked(QUEUE_FLAG_SECERASE, q); >> } >> -- >> 2.10.1 >> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 8 November 2017 at 15:25, Jungseung Lee <js07.lee@gmail.com> wrote: > On Tue, Nov 7, 2017 at 9:54 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote: >> On 7 November 2017 at 08:02, Jungseung Lee <js07.lee@samsung.com> wrote: >>> Some devices return zeroes on read to unmmaped logical blocks. >>> Use this behavior to implement REQ_OP_WRITES_ZEROES. >>> >>> Send TRIM operation for the non-partial erase at a convenient time. >> >> This doesn't explain the problem enough. >> >> What does unmapped logical blocks means for an eMMC device? Isn't all >> logical blocks that is accessible being mapped? >> > Right. There is no unmapped logical block. > Only unmapped host address space on eMMC device exist in the spec. > > "The contents of a write block where the trim function has been applied > shall be '0' or '1' depending on different memory technology" > > I'd like to fix the name as trimmed block. Is it more clear and reasonable? Yes, thanks! [...] Kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c index ea80ff4..41c4ec1 100644 --- a/drivers/mmc/core/block.c +++ b/drivers/mmc/core/block.c @@ -1273,9 +1273,10 @@ static void mmc_blk_issue_discard_rq(struct mmc_queue *mq, struct request *req) struct mmc_card *card = md->queue.card; unsigned int from, nr, arg; int err = 0, type = MMC_BLK_DISCARD; + int force_trim = (req_op(req) == REQ_OP_WRITE_ZEROES); blk_status_t status = BLK_STS_OK; - if (!mmc_can_erase(card)) { + if (!mmc_can_erase(card) || (!mmc_can_trim(card) && force_trim)) { status = BLK_STS_NOTSUPP; goto fail; } @@ -1283,7 +1284,9 @@ static void mmc_blk_issue_discard_rq(struct mmc_queue *mq, struct request *req) from = blk_rq_pos(req); nr = blk_rq_sectors(req); - if (mmc_can_discard(card)) + if (force_trim) + arg = MMC_TRIM_ARG; + else if (mmc_can_discard(card)) arg = MMC_DISCARD_ARG; else if (mmc_can_trim(card)) arg = MMC_TRIM_ARG; @@ -2032,6 +2035,7 @@ void mmc_blk_issue_rq(struct mmc_queue *mq, struct request *req) mmc_blk_issue_rw_rq(mq, NULL); mmc_blk_issue_drv_op(mq, req); break; + case REQ_OP_WRITE_ZEROES: case REQ_OP_DISCARD: /* * Complete ongoing async transfer before issuing diff --git a/drivers/mmc/core/queue.c b/drivers/mmc/core/queue.c index 4f33d27..dc95b3a 100644 --- a/drivers/mmc/core/queue.c +++ b/drivers/mmc/core/queue.c @@ -144,6 +144,10 @@ static void mmc_queue_setup_discard(struct request_queue *q, /* granularity must not be greater than max. discard */ if (card->pref_erase > max_discard) q->limits.discard_granularity = 0; + + if (card->quirks & MMC_QUIRK_UNMAPPED_ZEROES) + blk_queue_max_write_zeroes_sectors(q, UINT_MAX); + if (mmc_can_secure_erase_trim(card)) queue_flag_set_unlocked(QUEUE_FLAG_SECERASE, q); }
Some devices return zeroes on read to unmmaped logical blocks. Use this behavior to implement REQ_OP_WRITES_ZEROES. Send TRIM operation for the non-partial erase at a convenient time. Signed-off-by: Jungseung Lee <js07.lee@samsung.com> --- drivers/mmc/core/block.c | 8 ++++++-- drivers/mmc/core/queue.c | 4 ++++ 2 files changed, 10 insertions(+), 2 deletions(-)