From: adrian.hunter@nokia•com (Adrian Hunter)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH] mmc: failure of block read wait for long time
Date: Wed, 28 Jul 2010 11:41:26 +0300 [thread overview]
Message-ID: <4C4FED36.3010607@nokia.com> (raw)
In-Reply-To: <2A3DCF3DA181AD40BDE86A3150B27B6B030E417D80@dbde02.ent.ti.com>
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@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@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;
>>> }
>>> --
>>> 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
>
next prev parent reply other threads:[~2010-07-28 8:41 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-07-27 11:27 [PATCH] mmc: failure of block read wait for long time Sukumar Ghorai
2010-07-27 13:22 ` Adrian Hunter
2010-07-27 13:32 ` Ghorai, Sukumar
2010-07-28 8:41 ` Adrian Hunter [this message]
2010-09-10 11:30 ` Ghorai, Sukumar
2010-09-10 11:43 ` Adrian Hunter
2010-09-10 11:48 ` Ghorai, Sukumar
2010-09-10 13:02 ` Adrian Hunter
2010-09-14 5:15 ` Ghorai, Sukumar
2010-09-20 7:54 ` Adrian Hunter
2010-09-20 8:57 ` Ghorai, Sukumar
2010-09-20 11:49 ` Adrian Hunter
2010-09-20 12:37 ` Ghorai, Sukumar
2010-09-20 13:09 ` Adrian Hunter
2010-09-20 13:25 ` Ghorai, Sukumar
2010-09-20 13:37 ` Russell King - ARM Linux
2010-09-22 5:32 ` Ghorai, Sukumar
2010-09-22 12:43 ` Chris Ball
2010-09-22 12:51 ` Ghorai, Sukumar
2010-09-24 14:35 ` Ghorai, Sukumar
2010-09-28 15:03 ` Ghorai, Sukumar
2010-09-28 18:32 ` Adrian Hunter
2010-09-28 18:59 ` Ghorai, Sukumar
2010-09-28 20:05 ` Adrian Hunter
2010-09-29 5:59 ` Ghorai, Sukumar
2010-08-27 20:59 ` Chris Ball
2010-08-30 19:09 ` Ghorai, Sukumar
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4C4FED36.3010607@nokia.com \
--to=adrian.hunter@nokia$(echo .)com \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox