| Message ID | 20171109125051.12772-1-js07.lee@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Nov 9, 2017 at 9:50 PM, Jungseung Lee <js07.lee@samsung.com> wrote: > The contents of a write block where the trim function has been applied > shall be '0' or '1' and so some of eMMC devices return zeroes on read to > the trimmed blocks. > > Use the behavior to implement REQ_OP_WRITES_ZEROES. We can only know the > behavior based on individual device modles, so this patch adds a flag to > the mmc quirk list for devices works that way. It also sets the new flag > for one such known device. > > Signed-off-by: Jungseung Lee <js07.lee@samsung.com> Ulf, Could you make time to comment for this patch? Best Regards, Jungseung Lee > --- > drivers/mmc/core/block.c | 8 ++++++-- > drivers/mmc/core/queue.c | 4 ++++ > drivers/mmc/core/quirks.h | 7 +++++++ > include/linux/mmc/card.h | 1 + > 4 files changed, 18 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..c18d2a1 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_TRIM_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); > } > diff --git a/drivers/mmc/core/quirks.h b/drivers/mmc/core/quirks.h > index fb72593..58871ac 100644 > --- a/drivers/mmc/core/quirks.h > +++ b/drivers/mmc/core/quirks.h > @@ -90,6 +90,13 @@ static const struct mmc_fixup mmc_blk_fixups[] = { > MMC_FIXUP("V10016", CID_MANFID_KINGSTON, CID_OEMID_ANY, add_quirk_mmc, > MMC_QUIRK_TRIM_BROKEN), > > + /* > + * Some MMC cards deterministically returns zeroes on reads to > + * trimmed logical blocks. > + */ > + MMC_FIXUP("8GME4R", CID_MANFID_SAMSUNG, CID_OEMID_ANY, add_quirk_mmc, > + MMC_QUIRK_TRIM_ZEROES), > + > END_FIXUP > }; > > diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h > index 279b390..d328d1f 100644 > --- a/include/linux/mmc/card.h > +++ b/include/linux/mmc/card.h > @@ -268,6 +268,7 @@ struct mmc_card { > #define MMC_QUIRK_BROKEN_IRQ_POLLING (1<<11) /* Polling SDIO_CCCR_INTx could create a fake interrupt */ > #define MMC_QUIRK_TRIM_BROKEN (1<<12) /* Skip trim */ > #define MMC_QUIRK_BROKEN_HPI (1<<13) /* Disable broken HPI support */ > +#define MMC_QUIRK_TRIM_ZEROES (1<<14) /* Return 0's on read to trimmed logical blocks */ > > bool reenable_cmdq; /* Re-enable Command Queue */ > > -- > 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
On 9 November 2017 at 13:50, Jungseung Lee <js07.lee@samsung.com> wrote: > The contents of a write block where the trim function has been applied > shall be '0' or '1' and so some of eMMC devices return zeroes on read to > the trimmed blocks. > > Use the behavior to implement REQ_OP_WRITES_ZEROES. We can only know the > behavior based on individual device modles, so this patch adds a flag to > the mmc quirk list for devices works that way. It also sets the new flag > for one such known device. Implementing this for trim make sense to me and is a good start. However, perhaps as a next step, we should also consider implementing this for erase!? For erase we need to make sure that we write '0's to those sectors that was not aligned with the erase group size, but that should be doable. Regarding the card quirk, I don't think it's needed. Because the are registers in the SD/eMMC cards, telling whether the device returns '1' or '0' for erased/trimmed sectors. eMMC: EXT_CSD register -> ERASED_MEM_CONT [181] SD: SCR register -> DATA_STAT_AFTER_ERASE [55]. Some additional comments below. > > Signed-off-by: Jungseung Lee <js07.lee@samsung.com> > --- > drivers/mmc/core/block.c | 8 ++++++-- > drivers/mmc/core/queue.c | 4 ++++ > drivers/mmc/core/quirks.h | 7 +++++++ > include/linux/mmc/card.h | 1 + > 4 files changed, 18 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)) { Please no. This becomes to hard to understand - and folding in changes in mmc_blk_issue_discard_rq() for REQ_OP_WRITE_ZEROES seems prone to also inventing errors. I may be wrong, by still. Then I think it's better to re-factor mmc_blk_issue_discard_rq() such parts needed for both REQ_OP_WRITE_ZEROES and REQ_OP_DISCARD, can be shared via some new helper function(s). > 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: [...] Finally, we just about converting the mmc block to use the blkmq interface and adding support for eMMC CMD. I am expecting that really soon to happen, and so I prefer if you could hold off posting a new version of this until this is taken care of. That also means, you will need to adopt your changes to the new mmc blkmq code, which I think should be rather easy. 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..c18d2a1 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_TRIM_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); } diff --git a/drivers/mmc/core/quirks.h b/drivers/mmc/core/quirks.h index fb72593..58871ac 100644 --- a/drivers/mmc/core/quirks.h +++ b/drivers/mmc/core/quirks.h @@ -90,6 +90,13 @@ static const struct mmc_fixup mmc_blk_fixups[] = { MMC_FIXUP("V10016", CID_MANFID_KINGSTON, CID_OEMID_ANY, add_quirk_mmc, MMC_QUIRK_TRIM_BROKEN), + /* + * Some MMC cards deterministically returns zeroes on reads to + * trimmed logical blocks. + */ + MMC_FIXUP("8GME4R", CID_MANFID_SAMSUNG, CID_OEMID_ANY, add_quirk_mmc, + MMC_QUIRK_TRIM_ZEROES), + END_FIXUP }; diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h index 279b390..d328d1f 100644 --- a/include/linux/mmc/card.h +++ b/include/linux/mmc/card.h @@ -268,6 +268,7 @@ struct mmc_card { #define MMC_QUIRK_BROKEN_IRQ_POLLING (1<<11) /* Polling SDIO_CCCR_INTx could create a fake interrupt */ #define MMC_QUIRK_TRIM_BROKEN (1<<12) /* Skip trim */ #define MMC_QUIRK_BROKEN_HPI (1<<13) /* Disable broken HPI support */ +#define MMC_QUIRK_TRIM_ZEROES (1<<14) /* Return 0's on read to trimmed logical blocks */ bool reenable_cmdq; /* Re-enable Command Queue */
The contents of a write block where the trim function has been applied shall be '0' or '1' and so some of eMMC devices return zeroes on read to the trimmed blocks. Use the behavior to implement REQ_OP_WRITES_ZEROES. We can only know the behavior based on individual device modles, so this patch adds a flag to the mmc quirk list for devices works that way. It also sets the new flag for one such known device. Signed-off-by: Jungseung Lee <js07.lee@samsung.com> --- drivers/mmc/core/block.c | 8 ++++++-- drivers/mmc/core/queue.c | 4 ++++ drivers/mmc/core/quirks.h | 7 +++++++ include/linux/mmc/card.h | 1 + 4 files changed, 18 insertions(+), 2 deletions(-)