[PATCH] mmc: failure of block read wait for long time
Ghorai, Sukumar
s-ghorai at ti.com
Fri Sep 10 07:30:01 EDT 2010
> -----Original Message-----
> From: Adrian Hunter [mailto:adrian.hunter at nokia.com]
> Sent: Wednesday, July 28, 2010 2:11 PM
> To: Ghorai, Sukumar
> Cc: linux-mmc at vger.kernel.org; linux-arm-kernel at lists.infradead.org
> Subject: Re: [PATCH] mmc: failure of block read wait for long time
>
> ext Ghorai, Sukumar wrote:
> >
> >> -----Original Message-----
> >> From: Adrian Hunter [mailto:adrian.hunter at nokia.com]
> >> Sent: Tuesday, July 27, 2010 6:52 PM
> >> To: Ghorai, Sukumar
> >> Cc: linux-mmc at vger.kernel.org; linux-arm-kernel at lists.infradead.org
> >> Subject: Re: [PATCH] mmc: failure of block read wait for long time
> >>
> >> Sukumar Ghorai wrote:
> >>> multi-block read failure retries in single block read one by one. It
> >> continues
> >>> retry of subsequent blocks, even after failure. Application will not
> be
> >> able
> >>> to decode the interleave data (even if few single block read success).
> >>> This patch fixes this problem by returning at the first failure
> instead
> >> of
> >>> waiting for long duration.
> >> How can you know what sectors are important to the upper layers?
> > [Ghorai]
> > 1. This is obvious either give the complete data to above layer or not.
> And every protocol follows that.
> >
> > 2. And other scenario, say apps request for to read the 128MB data and
> it will take quite long time before returning in failure case.
> >
> > 3. it is quite obvious if the read fail for sector(x) it will fail for
> sector(x+1)
> >
>
> None of that is exactly true. However you could change the retry
> from sectors to segments e.g.
>
>
> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
> index d545f79..5ed15f6 100644
> --- a/drivers/mmc/card/block.c
> +++ b/drivers/mmc/card/block.c
> @@ -321,13 +321,14 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq,
> struct request *req)
> struct mmc_blk_data *md = mq->data;
> struct mmc_card *card = md->queue.card;
> struct mmc_blk_request brq;
> - int ret = 1, disable_multi = 0;
> + int ret = 1, retrying = 0;
>
> mmc_claim_host(card->host);
>
> do {
> struct mmc_command cmd;
> u32 readcmd, writecmd, status = 0;
> + unsigned int blocks;
>
> memset(&brq, 0, sizeof(struct mmc_blk_request));
> brq.mrq.cmd = &brq.cmd;
> @@ -341,23 +342,26 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq,
> struct request *req)
> brq.stop.opcode = MMC_STOP_TRANSMISSION;
> brq.stop.arg = 0;
> brq.stop.flags = MMC_RSP_SPI_R1B | MMC_RSP_R1B | MMC_CMD_AC;
> - brq.data.blocks = blk_rq_sectors(req);
> +
> + /*
> + * After a read error, we redo the request one segment at a
> time
> + * in order to accurately determine which segments can be read
> + * successfully.
> + */
> + if (retrying)
> + blocks = blk_rq_cur_sectors(req);
> + else
> + blocks = blk_rq_sectors(req);
>
> /*
> * The block layer doesn't support all sector count
> * restrictions, so we need to be prepared for too big
> * requests.
> */
> - if (brq.data.blocks > card->host->max_blk_count)
> - brq.data.blocks = card->host->max_blk_count;
> + if (blocks > card->host->max_blk_count)
> + blocks = card->host->max_blk_count;
>
> - /*
> - * After a read error, we redo the request one sector at a
> time
> - * in order to accurately determine which sectors can be read
> - * successfully.
> - */
> - if (disable_multi && brq.data.blocks > 1)
> - brq.data.blocks = 1;
> + brq.data.blocks = blocks;
>
> if (brq.data.blocks > 1) {
> /* SPI multiblock writes terminate using a special
> @@ -418,11 +422,14 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq,
> struct request *req)
> * programming mode even when things go wrong.
> */
> if (brq.cmd.error || brq.data.error || brq.stop.error) {
> - if (brq.data.blocks > 1 && rq_data_dir(req) == READ) {
> - /* Redo read one sector at a time */
> - printk(KERN_WARNING "%s: retrying using single "
> - "block read\n", req->rq_disk->disk_name);
> - disable_multi = 1;
> + if (!retrying && rq_data_dir(req) == READ &&
> + blk_rq_cur_sectors(req) &&
> + blk_rq_cur_sectors(req) < blocks) {
> + /* Redo read one segment at a time */
> + printk(KERN_WARNING "%s: retrying read one "
> + "segment at a time\n",
> + req->rq_disk->disk_name);
> + retrying = 1;
> continue;
> }
> status = get_card_status(card, req);
> @@ -486,12 +493,13 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq,
> struct request *req)
> if (brq.cmd.error || brq.stop.error || brq.data.error) {
> if (rq_data_dir(req) == READ) {
> /*
> - * After an error, we redo I/O one sector at a
> + * After an error, we redo I/O one segment at a
> * time, so we only reach here after trying to
> - * read a single sector.
> + * read a single segment. Error out the whole
> + * segment and continue to the next.
> */
> spin_lock_irq(&md->lock);
> - ret = __blk_end_request(req, -EIO,
> brq.data.blksz);
> + ret = __blk_end_request(req, -EIO,
> blk_rq_cur_sectors(req) << 9);
> spin_unlock_irq(&md->lock);
> continue;
> }
>
>
> >>> Signed-off-by: Sukumar Ghorai <s-ghorai at ti.com>
> >>> ---
> >>> drivers/mmc/card/block.c | 1 -
> >>> 1 files changed, 0 insertions(+), 1 deletions(-)
> >>>
> >>> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
> >>> index cb9fbc8..cfb0827 100644
> >>> --- a/drivers/mmc/card/block.c
> >>> +++ b/drivers/mmc/card/block.c
> >>> @@ -419,7 +419,6 @@ static int mmc_blk_issue_rq(struct mmc_queue *mq,
> >> struct request *req)
> >>> spin_lock_irq(&md->lock);
> >>> ret = __blk_end_request(req, -EIO,
> >> brq.data.blksz);
> >>> spin_unlock_irq(&md->lock);
> >>> - continue;
> >>> }
> >>> goto cmd_err;
> >>> }
> >>> --
[Ghorai] Adrian,
Yes this works and reduced the retry by 1/4 (2048 to 512 times for 1MB data read) form the original code;
Initially it was retrying for each page(512 bytes) after multi-block read fail; but this solution is retying for each segment(2048 bytes);
1. Now say block layrer reading 1MB and failed for the 1st segment. So it will still retry for 1MB/2048-bytes, i.e. 512 times.
2. So do you think any good reason to retry again and again?
3. And again I mentioned how to inform the blok-layer that which segment having the valid data?
And I was suggesting for not to retry again if 1st retry (single block) failed.
http://comments.gmane.org/gmane.linux.kernel.mmc/2714
> >>> To unsubscribe from this list: send the line "unsubscribe linux-mmc"
> in
> >>> the body of a message to majordomo at vger.kernel.org
> >>> More majordomo info at http://vger.kernel.org/majordomo-info.html
> >>>
> >
> > Regards,
> > Ghorai
> >
More information about the linux-arm-kernel
mailing list