2022-07-01 12:56:10

by Christian Loehle

[permalink] [raw]
Subject: [PATCHv4] mmc: block: Add single read for 4k sector cards

Cards with 4k native sector size may only be read 4k-aligned,
accommodate for this in the single read recovery and use it.

Fixes: 81196976ed946 (mmc: block: Add blk-mq support)
Signed-off-by: Christian Loehle <[email protected]>
---
drivers/mmc/core/block.c | 28 ++++++++++++++--------------
1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
index f4a1281658db..912a398a9a76 100644
--- a/drivers/mmc/core/block.c
+++ b/drivers/mmc/core/block.c
@@ -176,7 +176,7 @@ static inline int mmc_blk_part_switch(struct mmc_card *card,
unsigned int part_type);
static void mmc_blk_rw_rq_prep(struct mmc_queue_req *mqrq,
struct mmc_card *card,
- int disable_multi,
+ int recovery_mode,
struct mmc_queue *mq);
static void mmc_blk_hsq_req_done(struct mmc_request *mrq);

@@ -1302,7 +1302,7 @@ static void mmc_blk_eval_resp_error(struct mmc_blk_request *brq)
}

static void mmc_blk_data_prep(struct mmc_queue *mq, struct mmc_queue_req *mqrq,
- int disable_multi, bool *do_rel_wr_p,
+ int recovery_mode, bool *do_rel_wr_p,
bool *do_data_tag_p)
{
struct mmc_blk_data *md = mq->blkdata;
@@ -1368,12 +1368,12 @@ static void mmc_blk_data_prep(struct mmc_queue *mq, struct mmc_queue_req *mqrq,
brq->data.blocks--;

/*
- * After a read error, we redo the request one sector
+ * After a read error, we redo the request one (native) sector
* at a time in order to accurately determine which
* sectors can be read successfully.
*/
- if (disable_multi)
- brq->data.blocks = 1;
+ if (recovery_mode)
+ brq->data.blocks = queue_physical_block_size(mq->queue) >> 9;

/*
* Some controllers have HW issues while operating
@@ -1590,7 +1590,7 @@ static int mmc_blk_cqe_issue_rw_rq(struct mmc_queue *mq, struct request *req)

static void mmc_blk_rw_rq_prep(struct mmc_queue_req *mqrq,
struct mmc_card *card,
- int disable_multi,
+ int recovery_mode,
struct mmc_queue *mq)
{
u32 readcmd, writecmd;
@@ -1599,7 +1599,7 @@ static void mmc_blk_rw_rq_prep(struct mmc_queue_req *mqrq,
struct mmc_blk_data *md = mq->blkdata;
bool do_rel_wr, do_data_tag;

- mmc_blk_data_prep(mq, mqrq, disable_multi, &do_rel_wr, &do_data_tag);
+ mmc_blk_data_prep(mq, mqrq, recovery_mode, &do_rel_wr, &do_data_tag);

brq->mrq.cmd = &brq->cmd;

@@ -1690,7 +1690,7 @@ static int mmc_blk_fix_state(struct mmc_card *card, struct request *req)

#define MMC_READ_SINGLE_RETRIES 2

-/* Single sector read during recovery */
+/* Single (native) sector read during recovery */
static void mmc_blk_read_single(struct mmc_queue *mq, struct request *req)
{
struct mmc_queue_req *mqrq = req_to_mmc_queue_req(req);
@@ -1698,6 +1698,7 @@ static void mmc_blk_read_single(struct mmc_queue *mq, struct request *req)
struct mmc_card *card = mq->card;
struct mmc_host *host = card->host;
blk_status_t error = BLK_STS_OK;
+ size_t bytes_per_read = queue_physical_block_size(mq->queue);

do {
u32 status;
@@ -1732,13 +1733,13 @@ static void mmc_blk_read_single(struct mmc_queue *mq, struct request *req)
else
error = BLK_STS_OK;

- } while (blk_update_request(req, error, 512));
+ } while (blk_update_request(req, error, bytes_per_read));

return;

error_exit:
mrq->data->bytes_xfered = 0;
- blk_update_request(req, BLK_STS_IOERR, 512);
+ blk_update_request(req, BLK_STS_IOERR, bytes_per_read);
/* Let it try the remaining request again */
if (mqrq->retries > MMC_MAX_RETRIES - 1)
mqrq->retries = MMC_MAX_RETRIES - 1;
@@ -1879,10 +1880,9 @@ static void mmc_blk_mq_rw_recovery(struct mmc_queue *mq, struct request *req)
return;
}

- /* FIXME: Missing single sector read for large sector size */
- if (!mmc_large_sector(card) && rq_data_dir(req) == READ &&
- brq->data.blocks > 1) {
- /* Read one sector at a time */
+ if (rq_data_dir(req) == READ && brq->data.blocks >
+ queue_physical_block_size(mq->queue) >> 9) {
+ /* Read one (native) sector at a time */
mmc_blk_read_single(mq, req);
return;
}
--
2.36.1

Hyperstone GmbH | Reichenaustr. 39a | 78467 Konstanz
Managing Director: Dr. Jan Peter Berns.
Commercial register of local courts: Freiburg HRB381782


2022-07-01 14:10:48

by Adrian Hunter

[permalink] [raw]
Subject: Re: [PATCHv4] mmc: block: Add single read for 4k sector cards

On 1/07/22 15:43, Christian Loehle wrote:
> Cards with 4k native sector size may only be read 4k-aligned,
> accommodate for this in the single read recovery and use it.
>
> Fixes: 81196976ed946 (mmc: block: Add blk-mq support)
> Signed-off-by: Christian Loehle <[email protected]>

Acked-by: Adrian Hunter <[email protected]>

> ---
> drivers/mmc/core/block.c | 28 ++++++++++++++--------------
> 1 file changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
> index f4a1281658db..912a398a9a76 100644
> --- a/drivers/mmc/core/block.c
> +++ b/drivers/mmc/core/block.c
> @@ -176,7 +176,7 @@ static inline int mmc_blk_part_switch(struct mmc_card *card,
> unsigned int part_type);
> static void mmc_blk_rw_rq_prep(struct mmc_queue_req *mqrq,
> struct mmc_card *card,
> - int disable_multi,
> + int recovery_mode,
> struct mmc_queue *mq);
> static void mmc_blk_hsq_req_done(struct mmc_request *mrq);
>
> @@ -1302,7 +1302,7 @@ static void mmc_blk_eval_resp_error(struct mmc_blk_request *brq)
> }
>
> static void mmc_blk_data_prep(struct mmc_queue *mq, struct mmc_queue_req *mqrq,
> - int disable_multi, bool *do_rel_wr_p,
> + int recovery_mode, bool *do_rel_wr_p,
> bool *do_data_tag_p)
> {
> struct mmc_blk_data *md = mq->blkdata;
> @@ -1368,12 +1368,12 @@ static void mmc_blk_data_prep(struct mmc_queue *mq, struct mmc_queue_req *mqrq,
> brq->data.blocks--;
>
> /*
> - * After a read error, we redo the request one sector
> + * After a read error, we redo the request one (native) sector
> * at a time in order to accurately determine which
> * sectors can be read successfully.
> */
> - if (disable_multi)
> - brq->data.blocks = 1;
> + if (recovery_mode)
> + brq->data.blocks = queue_physical_block_size(mq->queue) >> 9;
>
> /*
> * Some controllers have HW issues while operating
> @@ -1590,7 +1590,7 @@ static int mmc_blk_cqe_issue_rw_rq(struct mmc_queue *mq, struct request *req)
>
> static void mmc_blk_rw_rq_prep(struct mmc_queue_req *mqrq,
> struct mmc_card *card,
> - int disable_multi,
> + int recovery_mode,
> struct mmc_queue *mq)
> {
> u32 readcmd, writecmd;
> @@ -1599,7 +1599,7 @@ static void mmc_blk_rw_rq_prep(struct mmc_queue_req *mqrq,
> struct mmc_blk_data *md = mq->blkdata;
> bool do_rel_wr, do_data_tag;
>
> - mmc_blk_data_prep(mq, mqrq, disable_multi, &do_rel_wr, &do_data_tag);
> + mmc_blk_data_prep(mq, mqrq, recovery_mode, &do_rel_wr, &do_data_tag);
>
> brq->mrq.cmd = &brq->cmd;
>
> @@ -1690,7 +1690,7 @@ static int mmc_blk_fix_state(struct mmc_card *card, struct request *req)
>
> #define MMC_READ_SINGLE_RETRIES 2
>
> -/* Single sector read during recovery */
> +/* Single (native) sector read during recovery */
> static void mmc_blk_read_single(struct mmc_queue *mq, struct request *req)
> {
> struct mmc_queue_req *mqrq = req_to_mmc_queue_req(req);
> @@ -1698,6 +1698,7 @@ static void mmc_blk_read_single(struct mmc_queue *mq, struct request *req)
> struct mmc_card *card = mq->card;
> struct mmc_host *host = card->host;
> blk_status_t error = BLK_STS_OK;
> + size_t bytes_per_read = queue_physical_block_size(mq->queue);
>
> do {
> u32 status;
> @@ -1732,13 +1733,13 @@ static void mmc_blk_read_single(struct mmc_queue *mq, struct request *req)
> else
> error = BLK_STS_OK;
>
> - } while (blk_update_request(req, error, 512));
> + } while (blk_update_request(req, error, bytes_per_read));
>
> return;
>
> error_exit:
> mrq->data->bytes_xfered = 0;
> - blk_update_request(req, BLK_STS_IOERR, 512);
> + blk_update_request(req, BLK_STS_IOERR, bytes_per_read);
> /* Let it try the remaining request again */
> if (mqrq->retries > MMC_MAX_RETRIES - 1)
> mqrq->retries = MMC_MAX_RETRIES - 1;
> @@ -1879,10 +1880,9 @@ static void mmc_blk_mq_rw_recovery(struct mmc_queue *mq, struct request *req)
> return;
> }
>
> - /* FIXME: Missing single sector read for large sector size */
> - if (!mmc_large_sector(card) && rq_data_dir(req) == READ &&
> - brq->data.blocks > 1) {
> - /* Read one sector at a time */
> + if (rq_data_dir(req) == READ && brq->data.blocks >
> + queue_physical_block_size(mq->queue) >> 9) {
> + /* Read one (native) sector at a time */
> mmc_blk_read_single(mq, req);
> return;
> }

2022-07-01 14:31:26

by Avri Altman

[permalink] [raw]
Subject: RE: [PATCHv4] mmc: block: Add single read for 4k sector cards

> On 1/07/22 15:43, Christian Loehle wrote:
> > Cards with 4k native sector size may only be read 4k-aligned,
> > accommodate for this in the single read recovery and use it.
> >
> > Fixes: 81196976ed946 (mmc: block: Add blk-mq support)
> > Signed-off-by: Christian Loehle <[email protected]>
>
> Acked-by: Adrian Hunter <[email protected]>
Reviewed-by: Avri Altman <[email protected]>

>
> > ---
> > drivers/mmc/core/block.c | 28 ++++++++++++++--------------
> > 1 file changed, 14 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c index
> > f4a1281658db..912a398a9a76 100644
> > --- a/drivers/mmc/core/block.c
> > +++ b/drivers/mmc/core/block.c
> > @@ -176,7 +176,7 @@ static inline int mmc_blk_part_switch(struct
> mmc_card *card,
> > unsigned int part_type); static
> > void mmc_blk_rw_rq_prep(struct mmc_queue_req *mqrq,
> > struct mmc_card *card,
> > - int disable_multi,
> > + int recovery_mode,
> > struct mmc_queue *mq); static void
> > mmc_blk_hsq_req_done(struct mmc_request *mrq);
> >
> > @@ -1302,7 +1302,7 @@ static void mmc_blk_eval_resp_error(struct
> > mmc_blk_request *brq) }
> >
> > static void mmc_blk_data_prep(struct mmc_queue *mq, struct
> mmc_queue_req *mqrq,
> > - int disable_multi, bool *do_rel_wr_p,
> > + int recovery_mode, bool *do_rel_wr_p,
> > bool *do_data_tag_p) {
> > struct mmc_blk_data *md = mq->blkdata; @@ -1368,12 +1368,12 @@
> > static void mmc_blk_data_prep(struct mmc_queue *mq, struct
> mmc_queue_req *mqrq,
> > brq->data.blocks--;
> >
> > /*
> > - * After a read error, we redo the request one sector
> > + * After a read error, we redo the request one (native)
> > + sector
> > * at a time in order to accurately determine which
> > * sectors can be read successfully.
> > */
> > - if (disable_multi)
> > - brq->data.blocks = 1;
> > + if (recovery_mode)
> > + brq->data.blocks =
> > + queue_physical_block_size(mq->queue) >> 9;
> >
> > /*
> > * Some controllers have HW issues while operating @@
> > -1590,7 +1590,7 @@ static int mmc_blk_cqe_issue_rw_rq(struct mmc_queue
> > *mq, struct request *req)
> >
> > static void mmc_blk_rw_rq_prep(struct mmc_queue_req *mqrq,
> > struct mmc_card *card,
> > - int disable_multi,
> > + int recovery_mode,
> > struct mmc_queue *mq) {
> > u32 readcmd, writecmd;
> > @@ -1599,7 +1599,7 @@ static void mmc_blk_rw_rq_prep(struct
> mmc_queue_req *mqrq,
> > struct mmc_blk_data *md = mq->blkdata;
> > bool do_rel_wr, do_data_tag;
> >
> > - mmc_blk_data_prep(mq, mqrq, disable_multi, &do_rel_wr,
> &do_data_tag);
> > + mmc_blk_data_prep(mq, mqrq, recovery_mode, &do_rel_wr,
> > + &do_data_tag);
> >
> > brq->mrq.cmd = &brq->cmd;
> >
> > @@ -1690,7 +1690,7 @@ static int mmc_blk_fix_state(struct mmc_card
> > *card, struct request *req)
> >
> > #define MMC_READ_SINGLE_RETRIES 2
> >
> > -/* Single sector read during recovery */
> > +/* Single (native) sector read during recovery */
> > static void mmc_blk_read_single(struct mmc_queue *mq, struct request
> > *req) {
> > struct mmc_queue_req *mqrq = req_to_mmc_queue_req(req); @@
> > -1698,6 +1698,7 @@ static void mmc_blk_read_single(struct mmc_queue
> *mq, struct request *req)
> > struct mmc_card *card = mq->card;
> > struct mmc_host *host = card->host;
> > blk_status_t error = BLK_STS_OK;
> > + size_t bytes_per_read = queue_physical_block_size(mq->queue);
> >
> > do {
> > u32 status;
> > @@ -1732,13 +1733,13 @@ static void mmc_blk_read_single(struct
> mmc_queue *mq, struct request *req)
> > else
> > error = BLK_STS_OK;
> >
> > - } while (blk_update_request(req, error, 512));
> > + } while (blk_update_request(req, error, bytes_per_read));
> >
> > return;
> >
> > error_exit:
> > mrq->data->bytes_xfered = 0;
> > - blk_update_request(req, BLK_STS_IOERR, 512);
> > + blk_update_request(req, BLK_STS_IOERR, bytes_per_read);
> > /* Let it try the remaining request again */
> > if (mqrq->retries > MMC_MAX_RETRIES - 1)
> > mqrq->retries = MMC_MAX_RETRIES - 1; @@ -1879,10 +1880,9
> > @@ static void mmc_blk_mq_rw_recovery(struct mmc_queue *mq, struct
> request *req)
> > return;
> > }
> >
> > - /* FIXME: Missing single sector read for large sector size */
> > - if (!mmc_large_sector(card) && rq_data_dir(req) == READ &&
> > - brq->data.blocks > 1) {
> > - /* Read one sector at a time */
> > + if (rq_data_dir(req) == READ && brq->data.blocks >
> > + queue_physical_block_size(mq->queue) >> 9) {
> > + /* Read one (native) sector at a time */
> > mmc_blk_read_single(mq, req);
> > return;
> > }

2022-07-13 11:32:33

by Ulf Hansson

[permalink] [raw]
Subject: Re: [PATCHv4] mmc: block: Add single read for 4k sector cards

On Fri, 1 Jul 2022 at 14:43, Christian Loehle <[email protected]> wrote:
>
> Cards with 4k native sector size may only be read 4k-aligned,
> accommodate for this in the single read recovery and use it.
>
> Fixes: 81196976ed946 (mmc: block: Add blk-mq support)
> Signed-off-by: Christian Loehle <[email protected]>

Applied for next (please tell if you prefer this to be queued for
v5.19rc and/or stable), thanks!

Kind regards
Uffe

> ---
> drivers/mmc/core/block.c | 28 ++++++++++++++--------------
> 1 file changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
> index f4a1281658db..912a398a9a76 100644
> --- a/drivers/mmc/core/block.c
> +++ b/drivers/mmc/core/block.c
> @@ -176,7 +176,7 @@ static inline int mmc_blk_part_switch(struct mmc_card *card,
> unsigned int part_type);
> static void mmc_blk_rw_rq_prep(struct mmc_queue_req *mqrq,
> struct mmc_card *card,
> - int disable_multi,
> + int recovery_mode,
> struct mmc_queue *mq);
> static void mmc_blk_hsq_req_done(struct mmc_request *mrq);
>
> @@ -1302,7 +1302,7 @@ static void mmc_blk_eval_resp_error(struct mmc_blk_request *brq)
> }
>
> static void mmc_blk_data_prep(struct mmc_queue *mq, struct mmc_queue_req *mqrq,
> - int disable_multi, bool *do_rel_wr_p,
> + int recovery_mode, bool *do_rel_wr_p,
> bool *do_data_tag_p)
> {
> struct mmc_blk_data *md = mq->blkdata;
> @@ -1368,12 +1368,12 @@ static void mmc_blk_data_prep(struct mmc_queue *mq, struct mmc_queue_req *mqrq,
> brq->data.blocks--;
>
> /*
> - * After a read error, we redo the request one sector
> + * After a read error, we redo the request one (native) sector
> * at a time in order to accurately determine which
> * sectors can be read successfully.
> */
> - if (disable_multi)
> - brq->data.blocks = 1;
> + if (recovery_mode)
> + brq->data.blocks = queue_physical_block_size(mq->queue) >> 9;
>
> /*
> * Some controllers have HW issues while operating
> @@ -1590,7 +1590,7 @@ static int mmc_blk_cqe_issue_rw_rq(struct mmc_queue *mq, struct request *req)
>
> static void mmc_blk_rw_rq_prep(struct mmc_queue_req *mqrq,
> struct mmc_card *card,
> - int disable_multi,
> + int recovery_mode,
> struct mmc_queue *mq)
> {
> u32 readcmd, writecmd;
> @@ -1599,7 +1599,7 @@ static void mmc_blk_rw_rq_prep(struct mmc_queue_req *mqrq,
> struct mmc_blk_data *md = mq->blkdata;
> bool do_rel_wr, do_data_tag;
>
> - mmc_blk_data_prep(mq, mqrq, disable_multi, &do_rel_wr, &do_data_tag);
> + mmc_blk_data_prep(mq, mqrq, recovery_mode, &do_rel_wr, &do_data_tag);
>
> brq->mrq.cmd = &brq->cmd;
>
> @@ -1690,7 +1690,7 @@ static int mmc_blk_fix_state(struct mmc_card *card, struct request *req)
>
> #define MMC_READ_SINGLE_RETRIES 2
>
> -/* Single sector read during recovery */
> +/* Single (native) sector read during recovery */
> static void mmc_blk_read_single(struct mmc_queue *mq, struct request *req)
> {
> struct mmc_queue_req *mqrq = req_to_mmc_queue_req(req);
> @@ -1698,6 +1698,7 @@ static void mmc_blk_read_single(struct mmc_queue *mq, struct request *req)
> struct mmc_card *card = mq->card;
> struct mmc_host *host = card->host;
> blk_status_t error = BLK_STS_OK;
> + size_t bytes_per_read = queue_physical_block_size(mq->queue);
>
> do {
> u32 status;
> @@ -1732,13 +1733,13 @@ static void mmc_blk_read_single(struct mmc_queue *mq, struct request *req)
> else
> error = BLK_STS_OK;
>
> - } while (blk_update_request(req, error, 512));
> + } while (blk_update_request(req, error, bytes_per_read));
>
> return;
>
> error_exit:
> mrq->data->bytes_xfered = 0;
> - blk_update_request(req, BLK_STS_IOERR, 512);
> + blk_update_request(req, BLK_STS_IOERR, bytes_per_read);
> /* Let it try the remaining request again */
> if (mqrq->retries > MMC_MAX_RETRIES - 1)
> mqrq->retries = MMC_MAX_RETRIES - 1;
> @@ -1879,10 +1880,9 @@ static void mmc_blk_mq_rw_recovery(struct mmc_queue *mq, struct request *req)
> return;
> }
>
> - /* FIXME: Missing single sector read for large sector size */
> - if (!mmc_large_sector(card) && rq_data_dir(req) == READ &&
> - brq->data.blocks > 1) {
> - /* Read one sector at a time */
> + if (rq_data_dir(req) == READ && brq->data.blocks >
> + queue_physical_block_size(mq->queue) >> 9) {
> + /* Read one (native) sector at a time */
> mmc_blk_read_single(mq, req);
> return;
> }
> --
> 2.36.1
>
> Hyperstone GmbH | Reichenaustr. 39a | 78467 Konstanz
> Managing Director: Dr. Jan Peter Berns.
> Commercial register of local courts: Freiburg HRB381782
>