[PATCH 1/2] mmc : Add unmmapped zero quirk flag

[PATCH 1/2] mmc : Add unmmapped zero quirk flag

[Jungseung Lee]

Some devices return zeroes on read to unmmaped logical blocks.
This patch adds a flag to the mmc quirk list for the devices work that way.

Signed-off-by: Jungseung Lee <js07.lee@samsung.com>
---
 include/linux/mmc/card.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
index 279b390..f032cb0 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_UNMAPPED_ZEROES	(1<<14)	/* Return 0's on read to unmmaped logical blocks */
 
 	bool			reenable_cmdq;	/* Re-enable Command Queue */
 
-- 
2.10.1

[PATCH 2/2] mmc : implement REQ_OP_WRITE_ZEROES

[Jungseung Lee]

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(-)

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

Re: [PATCH 2/2] mmc : implement REQ_OP_WRITE_ZEROES

[Christoph Hellwig]

Please send a long the patch to actually set the flag, else it is just
dead code.

Re: [PATCH 2/2] mmc : implement REQ_OP_WRITE_ZEROES

[Ulf Hansson]

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

Re: [PATCH 2/2] mmc : implement REQ_OP_WRITE_ZEROES

[Jungseung Lee]

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

Re: [PATCH 2/2] mmc : implement REQ_OP_WRITE_ZEROES

[Ulf Hansson]

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