2022-05-02 23:11:39

by Vincent Whitchurch

[permalink] [raw]
Subject: [PATCH] mmc: core: support zeroout using TRIM

If the device supports TRIM and indicates that it erases to zeros, we
can use it to support hardware offloading of REQ_OP_WRITE_ZEROES.

Signed-off-by: Vincent Whitchurch <[email protected]>
---

Notes:
https://lore.kernel.org/lkml/[email protected]/ seems to agree
that BLKZEROOUT can use TRIM on eMMC.

BLKDISCARD uses DISCARD when available so it can't be used to send TRIM.

If TRIM should not be used for BLKZEROOUT for some reason I guess the only way
is to use MMC_IOC_MULTI_CMD like in this commit in mmc-utils but that's a
rather low-level interface:

https://git.kernel.org/pub/scm/utils/mmc/mmc-utils.git/commit/?id=43282e80e174cc73b09b81a4d17cb3a7b4dc5cfc

drivers/mmc/core/block.c | 26 ++++++++++++++++++++++----
drivers/mmc/core/queue.c | 2 ++
2 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
index 506dc900f5c7..0398b205a285 100644
--- a/drivers/mmc/core/block.c
+++ b/drivers/mmc/core/block.c
@@ -126,6 +126,7 @@ struct mmc_blk_data {
#define MMC_BLK_DISCARD BIT(2)
#define MMC_BLK_SECDISCARD BIT(3)
#define MMC_BLK_CQE_RECOVERY BIT(4)
+#define MMC_BLK_TRIM BIT(5)

/*
* Only set in main mmc_blk_data associated
@@ -1090,12 +1091,13 @@ static void mmc_blk_issue_drv_op(struct mmc_queue *mq, struct request *req)
blk_mq_end_request(req, ret ? BLK_STS_IOERR : BLK_STS_OK);
}

-static void mmc_blk_issue_discard_rq(struct mmc_queue *mq, struct request *req)
+static void mmc_blk_issue_erase_rq(struct mmc_queue *mq, struct request *req,
+ int type, unsigned int erase_arg)
{
struct mmc_blk_data *md = mq->blkdata;
struct mmc_card *card = md->queue.card;
unsigned int from, nr;
- int err = 0, type = MMC_BLK_DISCARD;
+ int err = 0;
blk_status_t status = BLK_STS_OK;

if (!mmc_can_erase(card)) {
@@ -1111,13 +1113,13 @@ static void mmc_blk_issue_discard_rq(struct mmc_queue *mq, struct request *req)
if (card->quirks & MMC_QUIRK_INAND_CMD38) {
err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
INAND_CMD38_ARG_EXT_CSD,
- card->erase_arg == MMC_TRIM_ARG ?
+ erase_arg == MMC_TRIM_ARG ?
INAND_CMD38_ARG_TRIM :
INAND_CMD38_ARG_ERASE,
card->ext_csd.generic_cmd6_time);
}
if (!err)
- err = mmc_erase(card, from, nr, card->erase_arg);
+ err = mmc_erase(card, from, nr, erase_arg);
} while (err == -EIO && !mmc_blk_reset(md, card->host, type));
if (err)
status = BLK_STS_IOERR;
@@ -1127,6 +1129,19 @@ static void mmc_blk_issue_discard_rq(struct mmc_queue *mq, struct request *req)
blk_mq_end_request(req, status);
}

+static void mmc_blk_issue_trim_rq(struct mmc_queue *mq, struct request *req)
+{
+ mmc_blk_issue_erase_rq(mq, req, MMC_BLK_TRIM, MMC_TRIM_ARG);
+}
+
+static void mmc_blk_issue_discard_rq(struct mmc_queue *mq, struct request *req)
+{
+ struct mmc_blk_data *md = mq->blkdata;
+ struct mmc_card *card = md->queue.card;
+
+ mmc_blk_issue_erase_rq(mq, req, MMC_BLK_DISCARD, card->erase_arg);
+}
+
static void mmc_blk_issue_secdiscard_rq(struct mmc_queue *mq,
struct request *req)
{
@@ -2327,6 +2342,9 @@ enum mmc_issued mmc_blk_mq_issue_rq(struct mmc_queue *mq, struct request *req)
case REQ_OP_SECURE_ERASE:
mmc_blk_issue_secdiscard_rq(mq, req);
break;
+ case REQ_OP_WRITE_ZEROES:
+ mmc_blk_issue_trim_rq(mq, req);
+ break;
case REQ_OP_FLUSH:
mmc_blk_issue_flush(mq, req);
break;
diff --git a/drivers/mmc/core/queue.c b/drivers/mmc/core/queue.c
index c69b2d9df6f1..bbe2ea829ea7 100644
--- a/drivers/mmc/core/queue.c
+++ b/drivers/mmc/core/queue.c
@@ -191,6 +191,8 @@ static void mmc_queue_setup_discard(struct request_queue *q,
q->limits.discard_granularity = SECTOR_SIZE;
if (mmc_can_secure_erase_trim(card))
blk_queue_flag_set(QUEUE_FLAG_SECERASE, q);
+ if (mmc_can_trim(card) && card->erased_byte == 0)
+ blk_queue_max_write_zeroes_sectors(q, max_discard);
}

static unsigned short mmc_get_max_segments(struct mmc_host *host)
--
2.34.1


2022-05-03 00:53:58

by Avri Altman

[permalink] [raw]
Subject: RE: [PATCH] mmc: core: support zeroout using TRIM

> If the device supports TRIM and indicates that it erases to zeros, we can use
> it to support hardware offloading of REQ_OP_WRITE_ZEROES.
>
> Signed-off-by: Vincent Whitchurch <[email protected]>
I agree that zero out is indeed a privet case of trim, as indicated by ERASED_MEM_CONT.

Looks good to me.

Thanks,
Avri

> ---
>
> Notes:
> https://lore.kernel.org/lkml/[email protected]/ seems
> to agree
> that BLKZEROOUT can use TRIM on eMMC.
>
> BLKDISCARD uses DISCARD when available so it can't be used to send
> TRIM.
>
> If TRIM should not be used for BLKZEROOUT for some reason I guess the
> only way
> is to use MMC_IOC_MULTI_CMD like in this commit in mmc-utils but
> that's a
> rather low-level interface:
>
> https://git.kernel.org/pub/scm/utils/mmc/mmc-
> utils.git/commit/?id=43282e80e174cc73b09b81a4d17cb3a7b4dc5cfc
>
> drivers/mmc/core/block.c | 26 ++++++++++++++++++++++----
> drivers/mmc/core/queue.c | 2 ++
> 2 files changed, 24 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c index
> 506dc900f5c7..0398b205a285 100644
> --- a/drivers/mmc/core/block.c
> +++ b/drivers/mmc/core/block.c
> @@ -126,6 +126,7 @@ struct mmc_blk_data {
> #define MMC_BLK_DISCARD BIT(2)
> #define MMC_BLK_SECDISCARD BIT(3)
> #define MMC_BLK_CQE_RECOVERY BIT(4)
> +#define MMC_BLK_TRIM BIT(5)
>
> /*
> * Only set in main mmc_blk_data associated @@ -1090,12 +1091,13
> @@ static void mmc_blk_issue_drv_op(struct mmc_queue *mq, struct
> request *req)
> blk_mq_end_request(req, ret ? BLK_STS_IOERR : BLK_STS_OK); }
>
> -static void mmc_blk_issue_discard_rq(struct mmc_queue *mq, struct
> request *req)
> +static void mmc_blk_issue_erase_rq(struct mmc_queue *mq, struct
> request *req,
> + int type, unsigned int erase_arg)
> {
> struct mmc_blk_data *md = mq->blkdata;
> struct mmc_card *card = md->queue.card;
> unsigned int from, nr;
> - int err = 0, type = MMC_BLK_DISCARD;
> + int err = 0;
> blk_status_t status = BLK_STS_OK;
>
> if (!mmc_can_erase(card)) {
> @@ -1111,13 +1113,13 @@ static void mmc_blk_issue_discard_rq(struct
> mmc_queue *mq, struct request *req)
> if (card->quirks & MMC_QUIRK_INAND_CMD38) {
> err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
> INAND_CMD38_ARG_EXT_CSD,
> - card->erase_arg == MMC_TRIM_ARG ?
> + erase_arg == MMC_TRIM_ARG ?
> INAND_CMD38_ARG_TRIM :
> INAND_CMD38_ARG_ERASE,
> card->ext_csd.generic_cmd6_time);
> }
> if (!err)
> - err = mmc_erase(card, from, nr, card->erase_arg);
> + err = mmc_erase(card, from, nr, erase_arg);
> } while (err == -EIO && !mmc_blk_reset(md, card->host, type));
> if (err)
> status = BLK_STS_IOERR;
> @@ -1127,6 +1129,19 @@ static void mmc_blk_issue_discard_rq(struct
> mmc_queue *mq, struct request *req)
> blk_mq_end_request(req, status); }
>
> +static void mmc_blk_issue_trim_rq(struct mmc_queue *mq, struct request
> +*req) {
> + mmc_blk_issue_erase_rq(mq, req, MMC_BLK_TRIM, MMC_TRIM_ARG);
> }
> +
> +static void mmc_blk_issue_discard_rq(struct mmc_queue *mq, struct
> +request *req) {
> + struct mmc_blk_data *md = mq->blkdata;
> + struct mmc_card *card = md->queue.card;
> +
> + mmc_blk_issue_erase_rq(mq, req, MMC_BLK_DISCARD,
> +card->erase_arg); }
> +
> static void mmc_blk_issue_secdiscard_rq(struct mmc_queue *mq,
> struct request *req) { @@ -2327,6 +2342,9 @@ enum
> mmc_issued mmc_blk_mq_issue_rq(struct mmc_queue *mq, struct request
> *req)
> case REQ_OP_SECURE_ERASE:
> mmc_blk_issue_secdiscard_rq(mq, req);
> break;
> + case REQ_OP_WRITE_ZEROES:
> + mmc_blk_issue_trim_rq(mq, req);
> + break;
> case REQ_OP_FLUSH:
> mmc_blk_issue_flush(mq, req);
> break;
> diff --git a/drivers/mmc/core/queue.c b/drivers/mmc/core/queue.c index
> c69b2d9df6f1..bbe2ea829ea7 100644
> --- a/drivers/mmc/core/queue.c
> +++ b/drivers/mmc/core/queue.c
> @@ -191,6 +191,8 @@ static void mmc_queue_setup_discard(struct
> request_queue *q,
> q->limits.discard_granularity = SECTOR_SIZE;
> if (mmc_can_secure_erase_trim(card))
> blk_queue_flag_set(QUEUE_FLAG_SECERASE, q);
> + if (mmc_can_trim(card) && card->erased_byte == 0)
> + blk_queue_max_write_zeroes_sectors(q, max_discard);
> }
>
> static unsigned short mmc_get_max_segments(struct mmc_host *host)
> --
> 2.34.1

2022-05-10 15:10:28

by Ulf Hansson

[permalink] [raw]
Subject: Re: [PATCH] mmc: core: support zeroout using TRIM

+ Avri

On Fri, 29 Apr 2022 at 17:22, Vincent Whitchurch
<[email protected]> wrote:
>
> If the device supports TRIM and indicates that it erases to zeros, we
> can use it to support hardware offloading of REQ_OP_WRITE_ZEROES.
>
> Signed-off-by: Vincent Whitchurch <[email protected]>

Thanks for the patch, this certainly makes sense to me too.

So, applied for next (with a slightly updated commit message) and by
adding Avri's reviewed-by tag, thanks!

Kind regards
Uffe


> ---
>
> Notes:
> https://lore.kernel.org/lkml/[email protected]/ seems to agree
> that BLKZEROOUT can use TRIM on eMMC.
>
> BLKDISCARD uses DISCARD when available so it can't be used to send TRIM.
>
> If TRIM should not be used for BLKZEROOUT for some reason I guess the only way
> is to use MMC_IOC_MULTI_CMD like in this commit in mmc-utils but that's a
> rather low-level interface:
>
> https://git.kernel.org/pub/scm/utils/mmc/mmc-utils.git/commit/?id=43282e80e174cc73b09b81a4d17cb3a7b4dc5cfc
>
> drivers/mmc/core/block.c | 26 ++++++++++++++++++++++----
> drivers/mmc/core/queue.c | 2 ++
> 2 files changed, 24 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
> index 506dc900f5c7..0398b205a285 100644
> --- a/drivers/mmc/core/block.c
> +++ b/drivers/mmc/core/block.c
> @@ -126,6 +126,7 @@ struct mmc_blk_data {
> #define MMC_BLK_DISCARD BIT(2)
> #define MMC_BLK_SECDISCARD BIT(3)
> #define MMC_BLK_CQE_RECOVERY BIT(4)
> +#define MMC_BLK_TRIM BIT(5)
>
> /*
> * Only set in main mmc_blk_data associated
> @@ -1090,12 +1091,13 @@ static void mmc_blk_issue_drv_op(struct mmc_queue *mq, struct request *req)
> blk_mq_end_request(req, ret ? BLK_STS_IOERR : BLK_STS_OK);
> }
>
> -static void mmc_blk_issue_discard_rq(struct mmc_queue *mq, struct request *req)
> +static void mmc_blk_issue_erase_rq(struct mmc_queue *mq, struct request *req,
> + int type, unsigned int erase_arg)
> {
> struct mmc_blk_data *md = mq->blkdata;
> struct mmc_card *card = md->queue.card;
> unsigned int from, nr;
> - int err = 0, type = MMC_BLK_DISCARD;
> + int err = 0;
> blk_status_t status = BLK_STS_OK;
>
> if (!mmc_can_erase(card)) {
> @@ -1111,13 +1113,13 @@ static void mmc_blk_issue_discard_rq(struct mmc_queue *mq, struct request *req)
> if (card->quirks & MMC_QUIRK_INAND_CMD38) {
> err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
> INAND_CMD38_ARG_EXT_CSD,
> - card->erase_arg == MMC_TRIM_ARG ?
> + erase_arg == MMC_TRIM_ARG ?
> INAND_CMD38_ARG_TRIM :
> INAND_CMD38_ARG_ERASE,
> card->ext_csd.generic_cmd6_time);
> }
> if (!err)
> - err = mmc_erase(card, from, nr, card->erase_arg);
> + err = mmc_erase(card, from, nr, erase_arg);
> } while (err == -EIO && !mmc_blk_reset(md, card->host, type));
> if (err)
> status = BLK_STS_IOERR;
> @@ -1127,6 +1129,19 @@ static void mmc_blk_issue_discard_rq(struct mmc_queue *mq, struct request *req)
> blk_mq_end_request(req, status);
> }
>
> +static void mmc_blk_issue_trim_rq(struct mmc_queue *mq, struct request *req)
> +{
> + mmc_blk_issue_erase_rq(mq, req, MMC_BLK_TRIM, MMC_TRIM_ARG);
> +}
> +
> +static void mmc_blk_issue_discard_rq(struct mmc_queue *mq, struct request *req)
> +{
> + struct mmc_blk_data *md = mq->blkdata;
> + struct mmc_card *card = md->queue.card;
> +
> + mmc_blk_issue_erase_rq(mq, req, MMC_BLK_DISCARD, card->erase_arg);
> +}
> +
> static void mmc_blk_issue_secdiscard_rq(struct mmc_queue *mq,
> struct request *req)
> {
> @@ -2327,6 +2342,9 @@ enum mmc_issued mmc_blk_mq_issue_rq(struct mmc_queue *mq, struct request *req)
> case REQ_OP_SECURE_ERASE:
> mmc_blk_issue_secdiscard_rq(mq, req);
> break;
> + case REQ_OP_WRITE_ZEROES:
> + mmc_blk_issue_trim_rq(mq, req);
> + break;
> case REQ_OP_FLUSH:
> mmc_blk_issue_flush(mq, req);
> break;
> diff --git a/drivers/mmc/core/queue.c b/drivers/mmc/core/queue.c
> index c69b2d9df6f1..bbe2ea829ea7 100644
> --- a/drivers/mmc/core/queue.c
> +++ b/drivers/mmc/core/queue.c
> @@ -191,6 +191,8 @@ static void mmc_queue_setup_discard(struct request_queue *q,
> q->limits.discard_granularity = SECTOR_SIZE;
> if (mmc_can_secure_erase_trim(card))
> blk_queue_flag_set(QUEUE_FLAG_SECERASE, q);
> + if (mmc_can_trim(card) && card->erased_byte == 0)
> + blk_queue_max_write_zeroes_sectors(q, max_discard);
> }
>
> static unsigned short mmc_get_max_segments(struct mmc_host *host)
> --
> 2.34.1
>

2022-10-19 15:08:53

by Jon Hunter

[permalink] [raw]
Subject: Re: [PATCH] mmc: core: support zeroout using TRIM

Hi Vincent,

On 29/04/2022 16:21, Vincent Whitchurch wrote:
> If the device supports TRIM and indicates that it erases to zeros, we
> can use it to support hardware offloading of REQ_OP_WRITE_ZEROES.
>
> Signed-off-by: Vincent Whitchurch <[email protected]>
> ---
>
> Notes:
> https://lore.kernel.org/lkml/[email protected]/ seems to agree
> that BLKZEROOUT can use TRIM on eMMC.
>
> BLKDISCARD uses DISCARD when available so it can't be used to send TRIM.
>
> If TRIM should not be used for BLKZEROOUT for some reason I guess the only way
> is to use MMC_IOC_MULTI_CMD like in this commit in mmc-utils but that's a
> rather low-level interface:
>
> https://git.kernel.org/pub/scm/utils/mmc/mmc-utils.git/commit/?id=43282e80e174cc73b09b81a4d17cb3a7b4dc5cfc
>
> drivers/mmc/core/block.c | 26 ++++++++++++++++++++++----
> drivers/mmc/core/queue.c | 2 ++
> 2 files changed, 24 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
> index 506dc900f5c7..0398b205a285 100644
> --- a/drivers/mmc/core/block.c
> +++ b/drivers/mmc/core/block.c
> @@ -126,6 +126,7 @@ struct mmc_blk_data {
> #define MMC_BLK_DISCARD BIT(2)
> #define MMC_BLK_SECDISCARD BIT(3)
> #define MMC_BLK_CQE_RECOVERY BIT(4)
> +#define MMC_BLK_TRIM BIT(5)
>
> /*
> * Only set in main mmc_blk_data associated
> @@ -1090,12 +1091,13 @@ static void mmc_blk_issue_drv_op(struct mmc_queue *mq, struct request *req)
> blk_mq_end_request(req, ret ? BLK_STS_IOERR : BLK_STS_OK);
> }
>
> -static void mmc_blk_issue_discard_rq(struct mmc_queue *mq, struct request *req)
> +static void mmc_blk_issue_erase_rq(struct mmc_queue *mq, struct request *req,
> + int type, unsigned int erase_arg)
> {
> struct mmc_blk_data *md = mq->blkdata;
> struct mmc_card *card = md->queue.card;
> unsigned int from, nr;
> - int err = 0, type = MMC_BLK_DISCARD;
> + int err = 0;
> blk_status_t status = BLK_STS_OK;
>
> if (!mmc_can_erase(card)) {
> @@ -1111,13 +1113,13 @@ static void mmc_blk_issue_discard_rq(struct mmc_queue *mq, struct request *req)
> if (card->quirks & MMC_QUIRK_INAND_CMD38) {
> err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
> INAND_CMD38_ARG_EXT_CSD,
> - card->erase_arg == MMC_TRIM_ARG ?
> + erase_arg == MMC_TRIM_ARG ?
> INAND_CMD38_ARG_TRIM :
> INAND_CMD38_ARG_ERASE,
> card->ext_csd.generic_cmd6_time);
> }
> if (!err)
> - err = mmc_erase(card, from, nr, card->erase_arg);
> + err = mmc_erase(card, from, nr, erase_arg);
> } while (err == -EIO && !mmc_blk_reset(md, card->host, type));
> if (err)
> status = BLK_STS_IOERR;
> @@ -1127,6 +1129,19 @@ static void mmc_blk_issue_discard_rq(struct mmc_queue *mq, struct request *req)
> blk_mq_end_request(req, status);
> }
>
> +static void mmc_blk_issue_trim_rq(struct mmc_queue *mq, struct request *req)
> +{
> + mmc_blk_issue_erase_rq(mq, req, MMC_BLK_TRIM, MMC_TRIM_ARG);
> +}
> +
> +static void mmc_blk_issue_discard_rq(struct mmc_queue *mq, struct request *req)
> +{
> + struct mmc_blk_data *md = mq->blkdata;
> + struct mmc_card *card = md->queue.card;
> +
> + mmc_blk_issue_erase_rq(mq, req, MMC_BLK_DISCARD, card->erase_arg);
> +}
> +
> static void mmc_blk_issue_secdiscard_rq(struct mmc_queue *mq,
> struct request *req)
> {
> @@ -2327,6 +2342,9 @@ enum mmc_issued mmc_blk_mq_issue_rq(struct mmc_queue *mq, struct request *req)
> case REQ_OP_SECURE_ERASE:
> mmc_blk_issue_secdiscard_rq(mq, req);
> break;
> + case REQ_OP_WRITE_ZEROES:
> + mmc_blk_issue_trim_rq(mq, req);
> + break;
> case REQ_OP_FLUSH:
> mmc_blk_issue_flush(mq, req);
> break;
> diff --git a/drivers/mmc/core/queue.c b/drivers/mmc/core/queue.c
> index c69b2d9df6f1..bbe2ea829ea7 100644
> --- a/drivers/mmc/core/queue.c
> +++ b/drivers/mmc/core/queue.c
> @@ -191,6 +191,8 @@ static void mmc_queue_setup_discard(struct request_queue *q,
> q->limits.discard_granularity = SECTOR_SIZE;
> if (mmc_can_secure_erase_trim(card))
> blk_queue_flag_set(QUEUE_FLAG_SECERASE, q);
> + if (mmc_can_trim(card) && card->erased_byte == 0)
> + blk_queue_max_write_zeroes_sectors(q, max_discard);
> }
>
> static unsigned short mmc_get_max_segments(struct mmc_host *host)


On some of our Tegra boards we have noticed the following warnings and it appears to be related to this change ...

[ 4.168317] mmc0: Command Queue Engine enabled
[ 4.176723] mmc0: new HS400 Enhanced strobe MMC card at address 0001
[ 4.189609] mmcblk0: mmc0:0001 HBG4a2 29.1 GiB
[ 4.207660] mmc0: running CQE recovery
[ 4.215332] mmcblk0: p1 p2 p3 p4 p5 p6 p7 p8 p9 p10 p11 p12 p13 p14 p15 p16 p17 p18 p19 p20 p21 p22 p23 p24 p25 p26 p27 p28 p29 p30 p31 p32 p33 p34 p35 p36 p37 p38 p39 p40 p41 p42
[ 4.249403] mmcblk0boot0: mmc0:0001 HBG4a2 8.00 MiB
[ 4.255457] mmcblk0boot1: mmc0:0001 HBG4a2 8.00 MiB
[ 4.262063] mmcblk0rpmb: mmc0:0001 HBG4a2 4.00 MiB, chardev (511:0)
...
[ 9.034384] ------------[ cut here ]------------
[ 9.038985] WARNING: CPU: 4 PID: 199 at /mlt/kernel/drivers/mmc/core/block.c:2379 mmc_blk_mq_issue_rq+0x370/0x820
[ 9.049100] Modules linked in: ip_tables x_tables ipv6
[ 9.054180] CPU: 4 PID: 199 Comm: kworker/4:1H Not tainted 6.1.0-rc1-00025-gaae703b02f92 #1
[ 9.062399] Hardware name: NVIDIA Jetson AGX Xavier Developer Kit (DT)
[ 9.068821] Workqueue: kblockd blk_mq_run_work_fn
[ 9.073464] pstate: 20400009 (nzCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[ 9.080318] pc : mmc_blk_mq_issue_rq+0x370/0x820
[ 9.084872] lr : mmc_blk_mq_issue_rq+0x70/0x820
[ 9.089382] sp : ffff80000b5b3ad0
[ 9.092643] x29: ffff80000b5b3ad0 x28: ffff00008467a288 x27: 0000000000000014
[ 9.099733] x26: ffff000082d49000 x25: ffff00008467a240 x24: ffff0000802acad8
[ 9.106758] x23: ffff0000802aca10 x22: ffff0000802aca00 x21: 0000000000000000
[ 9.113810] x20: ffff000080fa0800 x19: ffff800009824000 x18: 0000000000000080
[ 9.120858] x17: 0000000000000000 x16: f8ffffffffffffff x15: 000000000000029a
[ 9.127918] x14: 0000000000000000 x13: 0000000000002000 x12: 0000000000000000
[ 9.134980] x11: 0000000106d66000 x10: 0000000000000001 x9 : 0000000000000002
[ 9.142066] x8 : 0000000000000009 x7 : ffff000084650118 x6 : 00000000000000ff
[ 9.149171] x5 : ffff000084650000 x4 : 0000000000403082 x3 : ffffffffffffca4a
[ 9.156240] x2 : 0000000000000000 x1 : 00000000ffffffe7 x0 : 0000000000000009
[ 9.163276] Call trace:
[ 9.165737] mmc_blk_mq_issue_rq+0x370/0x820
[ 9.169993] mmc_mq_queue_rq+0x134/0x270
[ 9.173900] blk_mq_dispatch_rq_list+0x14c/0x8d8
[ 9.178500] blk_mq_do_dispatch_sched+0x330/0x348
[ 9.183169] __blk_mq_sched_dispatch_requests+0xd4/0x170
[ 9.188440] blk_mq_sched_dispatch_requests+0x34/0x70
[ 9.193456] __blk_mq_run_hw_queue+0x58/0xb0
[ 9.197666] blk_mq_run_work_fn+0x20/0x28
[ 9.201673] process_one_work+0x1e0/0x348
[ 9.205672] worker_thread+0x48/0x410
[ 9.209326] kthread+0xf4/0x110
[ 9.212462] ret_from_fork+0x10/0x20
[ 9.216025] ---[ end trace 0000000000000000 ]---
[ 9.220899] I/O error, dev mmcblk0, sector 12624 op 0x9:(WRITE_ZEROES) flags 0x800 phys_seg 0 prio class 2
[ 11.500035] I/O error, dev mmcblk0, sector 16720 op 0x9:(WRITE_ZEROES) flags 0x800 phys_seg 0 prio class 2
[ 13.804317] I/O error, dev mmcblk0, sector 20816 op 0x9:(WRITE_ZEROES) flags 0x800 phys_seg 0 prio class 2
[ 16.104063] I/O error, dev mmcblk0, sector 24912 op 0x9:(WRITE_ZEROES) flags 0x800 phys_seg 0 prio class 2


Reverting this makes the issue go away. Please let me know if you have any thoughts on this.

Thanks
Jon

--
nvpublic

2022-10-19 15:43:28

by Vincent Whitchurch

[permalink] [raw]
Subject: Re: [PATCH] mmc: core: support zeroout using TRIM

On Wed, Oct 19, 2022 at 04:54:12PM +0200, Jon Hunter wrote:
> [ 4.168317] mmc0: Command Queue Engine enabled
> [ 4.176723] mmc0: new HS400 Enhanced strobe MMC card at address 0001
> [ 4.189609] mmcblk0: mmc0:0001 HBG4a2 29.1 GiB
> [ 4.207660] mmc0: running CQE recovery
> [ 4.215332] mmcblk0: p1 p2 p3 p4 p5 p6 p7 p8 p9 p10 p11 p12 p13 p14 p15 p16 p17 p18 p19 p20 p21 p22 p23 p24 p25 p26 p27 p28 p29 p30 p31 p32 p33 p34 p35 p36 p37 p38 p39 p40 p41 p42
> [ 4.249403] mmcblk0boot0: mmc0:0001 HBG4a2 8.00 MiB
> [ 4.255457] mmcblk0boot1: mmc0:0001 HBG4a2 8.00 MiB
> [ 4.262063] mmcblk0rpmb: mmc0:0001 HBG4a2 4.00 MiB, chardev (511:0)
> ...
> [ 9.034384] ------------[ cut here ]------------
> [ 9.038985] WARNING: CPU: 4 PID: 199 at /mlt/kernel/drivers/mmc/core/block.c:2379 mmc_blk_mq_issue_rq+0x370/0x820
> [ 9.049100] Modules linked in: ip_tables x_tables ipv6
> [ 9.054180] CPU: 4 PID: 199 Comm: kworker/4:1H Not tainted 6.1.0-rc1-00025-gaae703b02f92 #1
> [ 9.062399] Hardware name: NVIDIA Jetson AGX Xavier Developer Kit (DT)
> [ 9.068821] Workqueue: kblockd blk_mq_run_work_fn
> [ 9.073464] pstate: 20400009 (nzCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> [ 9.080318] pc : mmc_blk_mq_issue_rq+0x370/0x820
> [ 9.084872] lr : mmc_blk_mq_issue_rq+0x70/0x820
> [ 9.089382] sp : ffff80000b5b3ad0
> [ 9.092643] x29: ffff80000b5b3ad0 x28: ffff00008467a288 x27: 0000000000000014
> [ 9.099733] x26: ffff000082d49000 x25: ffff00008467a240 x24: ffff0000802acad8
> [ 9.106758] x23: ffff0000802aca10 x22: ffff0000802aca00 x21: 0000000000000000
> [ 9.113810] x20: ffff000080fa0800 x19: ffff800009824000 x18: 0000000000000080
> [ 9.120858] x17: 0000000000000000 x16: f8ffffffffffffff x15: 000000000000029a
> [ 9.127918] x14: 0000000000000000 x13: 0000000000002000 x12: 0000000000000000
> [ 9.134980] x11: 0000000106d66000 x10: 0000000000000001 x9 : 0000000000000002
> [ 9.142066] x8 : 0000000000000009 x7 : ffff000084650118 x6 : 00000000000000ff
> [ 9.149171] x5 : ffff000084650000 x4 : 0000000000403082 x3 : ffffffffffffca4a
> [ 9.156240] x2 : 0000000000000000 x1 : 00000000ffffffe7 x0 : 0000000000000009
> [ 9.163276] Call trace:
> [ 9.165737] mmc_blk_mq_issue_rq+0x370/0x820
> [ 9.169993] mmc_mq_queue_rq+0x134/0x270
> [ 9.173900] blk_mq_dispatch_rq_list+0x14c/0x8d8
> [ 9.178500] blk_mq_do_dispatch_sched+0x330/0x348
> [ 9.183169] __blk_mq_sched_dispatch_requests+0xd4/0x170
> [ 9.188440] blk_mq_sched_dispatch_requests+0x34/0x70
> [ 9.193456] __blk_mq_run_hw_queue+0x58/0xb0
> [ 9.197666] blk_mq_run_work_fn+0x20/0x28
> [ 9.201673] process_one_work+0x1e0/0x348
> [ 9.205672] worker_thread+0x48/0x410
> [ 9.209326] kthread+0xf4/0x110
> [ 9.212462] ret_from_fork+0x10/0x20
> [ 9.216025] ---[ end trace 0000000000000000 ]---
> [ 9.220899] I/O error, dev mmcblk0, sector 12624 op 0x9:(WRITE_ZEROES) flags 0x800 phys_seg 0 prio class 2
> [ 11.500035] I/O error, dev mmcblk0, sector 16720 op 0x9:(WRITE_ZEROES) flags 0x800 phys_seg 0 prio class 2
> [ 13.804317] I/O error, dev mmcblk0, sector 20816 op 0x9:(WRITE_ZEROES) flags 0x800 phys_seg 0 prio class 2
> [ 16.104063] I/O error, dev mmcblk0, sector 24912 op 0x9:(WRITE_ZEROES) flags 0x800 phys_seg 0 prio class 2
>
>
> Reverting this makes the issue go away. Please let me know if you have any thoughts on this.

Could you please test if the below change fixes it? Thank you.

diff --git a/drivers/mmc/core/queue.c b/drivers/mmc/core/queue.c
index fefaa901b50f..3661ba0bbc87 100644
--- a/drivers/mmc/core/queue.c
+++ b/drivers/mmc/core/queue.c
@@ -48,6 +48,7 @@ static enum mmc_issue_type mmc_cqe_issue_type(struct mmc_host *host,
case REQ_OP_DRV_OUT:
case REQ_OP_DISCARD:
case REQ_OP_SECURE_ERASE:
+ case REQ_OP_WRITE_ZEROES:
return MMC_ISSUE_SYNC;
case REQ_OP_FLUSH:
return mmc_cqe_can_dcmd(host) ? MMC_ISSUE_DCMD : MMC_ISSUE_SYNC;

2022-10-19 17:55:49

by Jon Hunter

[permalink] [raw]
Subject: Re: [PATCH] mmc: core: support zeroout using TRIM


On 19/10/2022 16:07, Vincent Whitchurch wrote:
> On Wed, Oct 19, 2022 at 04:54:12PM +0200, Jon Hunter wrote:
>> [ 4.168317] mmc0: Command Queue Engine enabled
>> [ 4.176723] mmc0: new HS400 Enhanced strobe MMC card at address 0001
>> [ 4.189609] mmcblk0: mmc0:0001 HBG4a2 29.1 GiB
>> [ 4.207660] mmc0: running CQE recovery
>> [ 4.215332] mmcblk0: p1 p2 p3 p4 p5 p6 p7 p8 p9 p10 p11 p12 p13 p14 p15 p16 p17 p18 p19 p20 p21 p22 p23 p24 p25 p26 p27 p28 p29 p30 p31 p32 p33 p34 p35 p36 p37 p38 p39 p40 p41 p42
>> [ 4.249403] mmcblk0boot0: mmc0:0001 HBG4a2 8.00 MiB
>> [ 4.255457] mmcblk0boot1: mmc0:0001 HBG4a2 8.00 MiB
>> [ 4.262063] mmcblk0rpmb: mmc0:0001 HBG4a2 4.00 MiB, chardev (511:0)
>> ...
>> [ 9.034384] ------------[ cut here ]------------
>> [ 9.038985] WARNING: CPU: 4 PID: 199 at /mlt/kernel/drivers/mmc/core/block.c:2379 mmc_blk_mq_issue_rq+0x370/0x820
>> [ 9.049100] Modules linked in: ip_tables x_tables ipv6
>> [ 9.054180] CPU: 4 PID: 199 Comm: kworker/4:1H Not tainted 6.1.0-rc1-00025-gaae703b02f92 #1
>> [ 9.062399] Hardware name: NVIDIA Jetson AGX Xavier Developer Kit (DT)
>> [ 9.068821] Workqueue: kblockd blk_mq_run_work_fn
>> [ 9.073464] pstate: 20400009 (nzCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
>> [ 9.080318] pc : mmc_blk_mq_issue_rq+0x370/0x820
>> [ 9.084872] lr : mmc_blk_mq_issue_rq+0x70/0x820
>> [ 9.089382] sp : ffff80000b5b3ad0
>> [ 9.092643] x29: ffff80000b5b3ad0 x28: ffff00008467a288 x27: 0000000000000014
>> [ 9.099733] x26: ffff000082d49000 x25: ffff00008467a240 x24: ffff0000802acad8
>> [ 9.106758] x23: ffff0000802aca10 x22: ffff0000802aca00 x21: 0000000000000000
>> [ 9.113810] x20: ffff000080fa0800 x19: ffff800009824000 x18: 0000000000000080
>> [ 9.120858] x17: 0000000000000000 x16: f8ffffffffffffff x15: 000000000000029a
>> [ 9.127918] x14: 0000000000000000 x13: 0000000000002000 x12: 0000000000000000
>> [ 9.134980] x11: 0000000106d66000 x10: 0000000000000001 x9 : 0000000000000002
>> [ 9.142066] x8 : 0000000000000009 x7 : ffff000084650118 x6 : 00000000000000ff
>> [ 9.149171] x5 : ffff000084650000 x4 : 0000000000403082 x3 : ffffffffffffca4a
>> [ 9.156240] x2 : 0000000000000000 x1 : 00000000ffffffe7 x0 : 0000000000000009
>> [ 9.163276] Call trace:
>> [ 9.165737] mmc_blk_mq_issue_rq+0x370/0x820
>> [ 9.169993] mmc_mq_queue_rq+0x134/0x270
>> [ 9.173900] blk_mq_dispatch_rq_list+0x14c/0x8d8
>> [ 9.178500] blk_mq_do_dispatch_sched+0x330/0x348
>> [ 9.183169] __blk_mq_sched_dispatch_requests+0xd4/0x170
>> [ 9.188440] blk_mq_sched_dispatch_requests+0x34/0x70
>> [ 9.193456] __blk_mq_run_hw_queue+0x58/0xb0
>> [ 9.197666] blk_mq_run_work_fn+0x20/0x28
>> [ 9.201673] process_one_work+0x1e0/0x348
>> [ 9.205672] worker_thread+0x48/0x410
>> [ 9.209326] kthread+0xf4/0x110
>> [ 9.212462] ret_from_fork+0x10/0x20
>> [ 9.216025] ---[ end trace 0000000000000000 ]---
>> [ 9.220899] I/O error, dev mmcblk0, sector 12624 op 0x9:(WRITE_ZEROES) flags 0x800 phys_seg 0 prio class 2
>> [ 11.500035] I/O error, dev mmcblk0, sector 16720 op 0x9:(WRITE_ZEROES) flags 0x800 phys_seg 0 prio class 2
>> [ 13.804317] I/O error, dev mmcblk0, sector 20816 op 0x9:(WRITE_ZEROES) flags 0x800 phys_seg 0 prio class 2
>> [ 16.104063] I/O error, dev mmcblk0, sector 24912 op 0x9:(WRITE_ZEROES) flags 0x800 phys_seg 0 prio class 2
>>
>>
>> Reverting this makes the issue go away. Please let me know if you have any thoughts on this.
>
> Could you please test if the below change fixes it? Thank you.
>
> diff --git a/drivers/mmc/core/queue.c b/drivers/mmc/core/queue.c
> index fefaa901b50f..3661ba0bbc87 100644
> --- a/drivers/mmc/core/queue.c
> +++ b/drivers/mmc/core/queue.c
> @@ -48,6 +48,7 @@ static enum mmc_issue_type mmc_cqe_issue_type(struct mmc_host *host,
> case REQ_OP_DRV_OUT:
> case REQ_OP_DISCARD:
> case REQ_OP_SECURE_ERASE:
> + case REQ_OP_WRITE_ZEROES:
> return MMC_ISSUE_SYNC;
> case REQ_OP_FLUSH:
> return mmc_cqe_can_dcmd(host) ? MMC_ISSUE_DCMD : MMC_ISSUE_SYNC;


Thanks, that does appear to fix the problem.

Tested-by: Jon Hunter <[email protected]>

Cheers
Jon

--
nvpublic