mmc: failure of block read wait for long time
PATCH 1/1 mmc: failure of block read wait for long time
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.
--- drivers/mmc/card/block.c | 1 - 1 files changed, 0 insertions(+), 1 deletions(-)
drivers/mmc/card/block.c +0 -1
@@ -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; }
PATCH 1/1 mmc: failure of block read wait for long time
Quoted 5 lines · Sukumar Ghorai
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?
Quoted 23 lines
Signed-off-by Sukumar Ghorai <[email protected]>--- drivers/mmc/card/block.c | 1 - 1 files changed, 0 insertions(+), 1 deletions(-)drivers/mmc/card/block.c +0
-2@@ -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
PATCH 1/1 mmc: failure of block read wait for long time
Quoted 12 lines
-----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 timeQuoted 1 line · Sukumar Ghorai
multi-block read failure retries in single block read one by one. It
continues
Quoted 8 lines
Quoted 1 line
retry of subsequent blocks, even after failure. Application will not be
able
Quoted 2 lines
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
Quoted 1 line
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)
Quoted 16 lines
drivers/mmc/card/block.c static int mmc_blk_issue_rq(struct mmc_queue *mq,
Quoted 12 lines
Signed-off-by Sukumar Ghorai <[email protected]>--- drivers/mmc/card/block.c | 1 - 1 files changed, 0 insertions(+), 1 deletions(-)drivers/mmc/card/block.c +0
-0@@ -419,7 +419,6 @@ static int mmc_blk_issue_rq(struct mmc_queue *mq,struct request *req)
drivers/mmc/card/block.c static int mmc_blk_issue_rq(struct mmc_queue *mq, · line 420
Quoted 2 lines
drivers/mmc/card/block.c +0
-0spin_lock_irq (&md->lock); ret = __blk_end_request(req, -EIO,brq.data.blksz);
Quoted 10 lines
Quoted 10 lines
drivers/mmc/card/block.c +0
-2spin_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
PATCH 1/1 mmc: failure of block read wait for long time
Quoted 16 lines · Sukumar
Quoted 16 lines
-----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 timeQuoted 1 line · Sukumar Ghorai
multi-block read failure retries in single block read one by one. It
continues
Quoted 1 line
retry of subsequent blocks, even after failure. Application will not be
able
Quoted 2 lines
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
Quoted 9 lines
Quoted 2 lines
Quoted 1 line
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.
drivers/mmc/card/block.c +27 -19
@@ -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 ; }
Quoted 28 lines
Quoted 14 lines
drivers/mmc/card/block.c static int mmc_blk_issue_rq(struct mmc_queue *mq,
Quoted 10 lines
Signed-off-by Sukumar Ghorai <[email protected]>--- drivers/mmc/card/block.c | 1 - 1 files changed, 0 insertions(+), 1 deletions(-)drivers/mmc/card/block.c +0
-0@@ -419,7 +419,6 @@ static int mmc_blk_issue_rq(struct mmc_queue *mq,struct request *req)
drivers/mmc/card/block.c static int mmc_blk_issue_rq(struct mmc_queue *mq, · line 420
Quoted 2 lines
drivers/mmc/card/block.c +0
-0spin_lock_irq (&md->lock); ret = __blk_end_request(req, -EIO,brq.data.blksz);
Quoted 10 lines
Quoted 10 lines
drivers/mmc/card/block.c +0
-2spin_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.htmlRegards,
Ghorai
PATCH 1/1 mmc: failure of block read wait for long time
Quoted 162 lines
-----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 timeQuoted 8 lines · Sukumar
Quoted 8 lines
-----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 timeQuoted 4 lines
Quoted 4 lines
Quoted 1 line · Sukumar Ghorai
multi-block read failure retries in single block read one by one. It
continues
Quoted 1 line
retry of subsequent blocks, even after failure. Application will not
be
Quoted 3 lines
Quoted 3 lines
able
Quoted 2 lines
to decode the interleave data (even if few single block read success). This patch fixes this problem by returning at the first failure
instead
Quoted 5 lines
Quoted 3 lines
of
Quoted 1 line
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.
Quoted 2 lines
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.
Quoted 2 lines
3. it is quite obvious if the read fail for sector(x) it will fail for
sector(x+1)
Quoted 1 line
None of that is exactly true. However you could change the retry from sectors to segments e.g.
drivers/mmc/card/block.c +2
-1@@ -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));drivers/mmc/card/block.c +3
-1brq.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
drivers/mmc/card/block.c +9
-4+* 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
drivers/mmc/card/block.c +9
-10-* 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 ; }drivers/mmc/card/block.c +4
-3status =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 );drivers/mmc/card/block.c +0
-0spin_unlock_irq (&md->lock);continue ; }Quoted 15 lines
Quoted 15 lines
drivers/mmc/card/block.c static int mmc_blk_issue_rq(struct mmc_queue *mq,
Quoted 12 lines
Signed-off-by Sukumar Ghorai <[email protected]>--- drivers/mmc/card/block.c | 1 - 1 files changed, 0 insertions(+), 1 deletions(-)drivers/mmc/card/block.c +0
-0@@ -419,7 +419,6 @@ static int mmc_blk_issue_rq(struct mmc_queue *mq,struct request *req)
Quoted 2 lines
drivers/mmc/card/block.c +0
-0spin_lock_irq (&md->lock); ret = __blk_end_request(req, -EIO,
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
brq.data.blksz);
drivers/mmc/card/block.c static int mmc_blk_issue_rq(struct mmc_queue *mq,
Quoted 6 lines
drivers/mmc/card/block.c +0
-2spin_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
Quoted 9 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
To unsubscribe from this list: send the line "unsubscribe linux-mmc"
in
Quoted 7 lines
Quoted 3 lines
Quoted 3 lines
the body of a message to majordomo at vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Regards,
Ghorai
PATCH 1/1 mmc: failure of block read wait for long time
Quoted 166 lines · Sukumar
Quoted 156 lines
-----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 timeQuoted 11 lines · Sukumar
Quoted 11 lines
-----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 timeQuoted 1 line · Sukumar Ghorai
multi-block read failure retries in single block read one by one. It
continues
Quoted 1 line
retry of subsequent blocks, even after failure. Application will not
be
Quoted 3 lines
Quoted 3 lines
able
Quoted 2 lines
to decode the interleave data (even if few single block read success). This patch fixes this problem by returning at the first failure
instead
Quoted 5 lines
Quoted 3 lines
of
Quoted 1 line
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.
Quoted 1 line
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.
Quoted 1 line
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.drivers/mmc/card/block.c +2
-1@@ -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));drivers/mmc/card/block.c +3
-1brq.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
drivers/mmc/card/block.c +9
-4+* 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
drivers/mmc/card/block.c +9
-10-* 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 ; }drivers/mmc/card/block.c +4
-3status =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 );drivers/mmc/card/block.c +0
-0spin_unlock_irq (&md->lock);continue ; }Quoted 15 lines
Quoted 15 lines
drivers/mmc/card/block.c static int mmc_blk_issue_rq(struct mmc_queue *mq,
Quoted 12 lines
Signed-off-by Sukumar Ghorai <[email protected]>--- drivers/mmc/card/block.c | 1 - 1 files changed, 0 insertions(+), 1 deletions(-)drivers/mmc/card/block.c +0
-0@@ -419,7 +419,6 @@ static int mmc_blk_issue_rq(struct mmc_queue *mq,struct request *req)
Quoted 2 lines
drivers/mmc/card/block.c +0
-0spin_lock_irq (&md->lock); ret = __blk_end_request(req, -EIO,Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
brq.data.blksz);
drivers/mmc/card/block.c static int mmc_blk_issue_rq(struct mmc_queue *mq,
Quoted 6 lines
drivers/mmc/card/block.c +0
-2spin_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;
Quoted 4 lines
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?
If you have 1MB that is not readable, it sounds like the card is broken. Why are so many reads failing? Has the card been removed?
You might very rarely see ECC errors in a small number of sectors, but more than that sounds like something else is broken.
Quoted 15 lines
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/2714Quoted 8 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
To unsubscribe from this list: send the line "unsubscribe linux-mmc"
in
Quoted 6 lines
Quoted 3 lines
Quoted 3 lines
the body of a message to majordomo at vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Regards,
Ghorai
PATCH 1/1 mmc: failure of block read wait for long time
Quoted 188 lines
-----Original Message-----
From: Adrian Hunter [mailto:adrian.hunter at nokia.com]
Sent: Friday, September 10, 2010 5:14 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 timeQuoted 16 lines · Sukumar
Quoted 16 lines
-----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 timeQuoted 7 lines · Sukumar
Quoted 7 lines
-----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 timeQuoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 1 line · Sukumar Ghorai
multi-block read failure retries in single block read one by one.
It
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
continues
Quoted 1 line
retry of subsequent blocks, even after failure. Application will
not
Quoted 3 lines
Quoted 3 lines
be
Quoted 2 lines
Quoted 2 lines
able
Quoted 1 line
to decode the interleave data (even if few single block read
success).
Quoted 16 lines
Quoted 16 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
This patch fixes this problem by returning at the first failure
instead
Quoted 5 lines
Quoted 3 lines
of
Quoted 1 line
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.
Quoted 1 line
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.
Quoted 1 line
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.Quoted 5 lines
drivers/mmc/card/block.c static int mmc_blk_issue_rw_rq(struct mmc_queue
Quoted 5 lines
drivers/mmc/card/block.c +0
-0@@ -321,13 +321,14 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue*mq,
Quoted 9 lines
Quoted 9 lines
drivers/mmc/card/block.c +1
-1struct 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);Quoted 8 lines
drivers/mmc/card/block.c static int mmc_blk_issue_rw_rq(struct mmc_queue
Quoted 8 lines
drivers/mmc/card/block.c +1
-0do { struct mmc_command cmd; u32 readcmd, writecmd, status = 0; + unsigned int blocks; memset(&brq, 0, sizeof(struct mmc_blk_request));drivers/mmc/card/block.c +0
-0brq.mrq.cmd = &brq.cmd;@@ -341,23 +342,26 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue*mq,
Quoted 37 lines
Quoted 37 lines
drivers/mmc/card/block.c +3
-1struct 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
drivers/mmc/card/block.c +9
-4+* 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
drivers/mmc/card/block.c +1
-5-* 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;Quoted 3 lines
drivers/mmc/card/block.c static int mmc_blk_issue_rw_rq(struct mmc_queue
Quoted 3 lines
drivers/mmc/card/block.c +0
-0if (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,
Quoted 13 lines
Quoted 13 lines
drivers/mmc/card/block.c +4
-5struct 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 */ Quoted 8 lines
drivers/mmc/card/block.c static int mmc_blk_issue_rw_rq(struct mmc_queue
Quoted 8 lines
drivers/mmc/card/block.c +4
-0+printk (KERN_WARNING "%s: retrying read one " +"segment at a time\n" , + req->rq_disk->disk_name); + retrying =1 ;continue ; }drivers/mmc/card/block.c +0
-0status =get_card_status (card, req);@@ -486,12 +493,13 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue*mq,
Quoted 35 lines
Quoted 35 lines
drivers/mmc/card/block.c +4
-3struct 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 );drivers/mmc/card/block.c +0
-0spin_unlock_irq (&md->lock);continue ; }Quoted 16 lines
Quoted 16 lines
drivers/mmc/card/block.c static int mmc_blk_issue_rq(struct mmc_queue *mq,
Quoted 12 lines
Signed-off-by Sukumar Ghorai <[email protected]>--- drivers/mmc/card/block.c | 1 - 1 files changed, 0 insertions(+), 1 deletions(-)drivers/mmc/card/block.c +0
-0@@ -419,7 +419,6 @@ static int mmc_blk_issue_rq(struct mmc_queue *mq,struct request *req)
drivers/mmc/card/block.c static int mmc_blk_issue_rq(struct mmc_queue *mq, · line 420
Quoted 2 lines
drivers/mmc/card/block.c +0
-0spin_lock_irq (&md->lock); ret = __blk_end_request(req, -EIO,brq.data.blksz);
Quoted 8 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
drivers/mmc/card/block.c static int mmc_blk_issue_rq(struct mmc_queue *mq,
Quoted 6 lines
drivers/mmc/card/block.c +0
-2spin_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 1MBdata read) form the original code;
Quoted 2 lines
Initially it was retrying for each page(512 bytes) after multi-block
read fail; but this solution is retying for each segment(2048 bytes);
Quoted 2 lines
1. Now say block layrer reading 1MB and failed for the 1st segment. So
Quoted 8 lines
it will still retry for 1MB/2048-bytes, i.e. 512 times.
Quoted 1 line
2. So do you think any good reason to retry again and again?
If you have 1MB that is not readable, it sounds like the card is broken. Why are so many reads failing? Has the card been removed?
You might very rarely see ECC errors in a small number of sectors, but more than that sounds like something else is broken.
[Ghorai] yes, one example is we remove the card when reading data,
And moreover we should not give the interleave data to apps, as we don't have option to tell application for the valid data.
Quoted 18 lines
Quoted 2 lines
3. And again I mentioned how to inform the blok-layer that which segment
having the valid data?
Quoted 2 lines
And I was suggesting for not to retry again if 1st retry (single block)
failed.
Quoted 12 lines
http://comments.gmane.org/gmane.linux.kernel.mmc/2714
Quoted 8 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
To unsubscribe from this list: send the line "unsubscribe linux-mmc"
in
Quoted 6 lines
Quoted 3 lines
Quoted 3 lines
the body of a message to majordomo at vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Regards,
Ghorai
PATCH 1/1 mmc: failure of block read wait for long time
Quoted 185 lines · Sukumar
Quoted 185 lines
-----Original Message-----
From: Adrian Hunter [mailto:adrian.hunter at nokia.com]
Sent: Friday, September 10, 2010 5:14 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 timeQuoted 15 lines · Sukumar
Quoted 15 lines
-----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 timeQuoted 7 lines · Sukumar
Quoted 7 lines
-----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 timeQuoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 1 line · Sukumar Ghorai
multi-block read failure retries in single block read one by one.
It
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
continues
Quoted 1 line
retry of subsequent blocks, even after failure. Application will
not
Quoted 3 lines
Quoted 3 lines
be
Quoted 2 lines
Quoted 2 lines
able
Quoted 1 line
to decode the interleave data (even if few single block read
success).
Quoted 16 lines
Quoted 16 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
This patch fixes this problem by returning at the first failure
instead
Quoted 5 lines
Quoted 3 lines
of
Quoted 1 line
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.
Quoted 1 line
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.
Quoted 1 line
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.Quoted 5 lines
drivers/mmc/card/block.c static int mmc_blk_issue_rw_rq(struct mmc_queue
Quoted 5 lines
drivers/mmc/card/block.c +0
-0@@ -321,13 +321,14 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue*mq,
Quoted 9 lines
Quoted 9 lines
drivers/mmc/card/block.c +1
-1struct 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);Quoted 8 lines
drivers/mmc/card/block.c static int mmc_blk_issue_rw_rq(struct mmc_queue
Quoted 8 lines
drivers/mmc/card/block.c +1
-0do { struct mmc_command cmd; u32 readcmd, writecmd, status = 0; + unsigned int blocks; memset(&brq, 0, sizeof(struct mmc_blk_request));drivers/mmc/card/block.c +0
-0brq.mrq.cmd = &brq.cmd;@@ -341,23 +342,26 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue*mq,
Quoted 37 lines
Quoted 37 lines
drivers/mmc/card/block.c +3
-1struct 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
drivers/mmc/card/block.c +9
-4+* 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
drivers/mmc/card/block.c +1
-5-* 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;Quoted 3 lines
drivers/mmc/card/block.c static int mmc_blk_issue_rw_rq(struct mmc_queue
Quoted 3 lines
drivers/mmc/card/block.c +0
-0if (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,
Quoted 13 lines
Quoted 13 lines
drivers/mmc/card/block.c +4
-5struct 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 */ Quoted 8 lines
drivers/mmc/card/block.c static int mmc_blk_issue_rw_rq(struct mmc_queue
Quoted 8 lines
drivers/mmc/card/block.c +4
-0+printk (KERN_WARNING "%s: retrying read one " +"segment at a time\n" , + req->rq_disk->disk_name); + retrying =1 ;continue ; }drivers/mmc/card/block.c +0
-0status =get_card_status (card, req);@@ -486,12 +493,13 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue*mq,
Quoted 35 lines
Quoted 35 lines
drivers/mmc/card/block.c +4
-3struct 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 );drivers/mmc/card/block.c +0
-0spin_unlock_irq (&md->lock);continue ; }Quoted 16 lines
Quoted 16 lines
drivers/mmc/card/block.c static int mmc_blk_issue_rq(struct mmc_queue *mq,
Quoted 12 lines
Signed-off-by Sukumar Ghorai <[email protected]>--- drivers/mmc/card/block.c | 1 - 1 files changed, 0 insertions(+), 1 deletions(-)drivers/mmc/card/block.c +0
-0@@ -419,7 +419,6 @@ static int mmc_blk_issue_rq(struct mmc_queue *mq,struct request *req)
drivers/mmc/card/block.c static int mmc_blk_issue_rq(struct mmc_queue *mq, · line 420
Quoted 2 lines
drivers/mmc/card/block.c +0
-0spin_lock_irq (&md->lock); ret = __blk_end_request(req, -EIO,brq.data.blksz);
Quoted 8 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
drivers/mmc/card/block.c static int mmc_blk_issue_rq(struct mmc_queue *mq,
Quoted 6 lines
drivers/mmc/card/block.c +0
-2spin_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 1MBdata read) form the original code;
Quoted 1 line
Initially it was retrying for each page(512 bytes) after multi-block
read fail; but this solution is retying for each segment(2048 bytes);
Quoted 1 line
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.
Quoted 8 lines
Quoted 6 lines
Quoted 1 line
2. So do you think any good reason to retry again and again?
If you have 1MB that is not readable, it sounds like the card is broken. Why are so many reads failing? Has the card been removed?
You might very rarely see ECC errors in a small number of sectors, but more than that sounds like something else is broken.
[Ghorai] yes, one example is we remove the card when reading data,
Well, that is a different case. Once the card has gone, the block driver can (and will once the remove method is called) error out all I/O requests without sending them to MMC. That doesn't happen until there is a card detect interrupt and a resulting rescan.
A possible solution is to put a flag on mmc_card to indicate card_gone that gets set as soon as the drivers card detect interrupt shows there is no card (hoping that we are not getting any bouncing on card detect) and then have mmc_wait_for_req() simple return -ENODEV immediately if the card_gone flag is set. Finally, if the mmc block driver sees a -ENODEV error, it should also check the card_gone flag (via a new core function) and if the card is gone, do not retry - and perhaps even error out the rest of the I/O request queue as well.
I can suggest a patch if you want but I am on vacation next week so it will have to wait a couple of weeks.
Quoted 19 lines
And moreover we should not give the interleave data to apps, as we don't have option to tell application for the valid data.
Quoted 15 lines
Quoted 1 line
3. And again I mentioned how to inform the blok-layer that which segment
having the valid data?
Quoted 1 line
And I was suggesting for not to retry again if 1st retry (single block)
failed.
Quoted 11 lines
http://comments.gmane.org/gmane.linux.kernel.mmc/2714
Quoted 8 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
To unsubscribe from this list: send the line "unsubscribe linux-mmc"
in
Quoted 6 lines
Quoted 3 lines
Quoted 3 lines
the body of a message to majordomo at vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Regards,
Ghorai
PATCH 1/1 mmc: failure of block read wait for long time
Adrian,
[..snip..]
Quoted 14 lines
Quoted 14 lines
Quoted 13 lines
Quoted 2 lines
[Ghorai] Adrian,
Yes this works and reduced the retry by 1/4 (2048 to 512 times for 1MBdata read) form the original code;
Quoted 1 line
Initially it was retrying for each page(512 bytes) after multi-block
read fail; but this solution is retying for each segment(2048 bytes);
Quoted 1 line
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.
Quoted 1 line
2. So do you think any good reason to retry again and again?
If you have 1MB that is not readable, it sounds like the card is broken. Why are so many reads failing? Has the card been removed?
You might very rarely see ECC errors in a small number of sectors, but more than that sounds like something else is broken.
Quoted 6 lines
Quoted 1 line
[Ghorai] yes, one example is we remove the card when reading data,
Well, that is a different case. Once the card has gone, the block driver can (and will once the remove method is called) error out all I/O requests without sending them to MMC. That doesn't happen until there is a card detect interrupt and a resulting rescan.
[Ghorai] here we are discussing two problem,
1. If IO failed how to stop retry; because of -
a. internal card error
b. issue in Filesystem, driver, or host controller issue c. or cards removed.
2. And 2nd how to sync block-layer IO, if card removed,
a. case 1: when card removed interrupt support by the platform b. case 2: when card removed interrupt does not support by the platform?
Quoted 9 lines
A possible solution is to put a flag on
mmc_cardto indicatecard_gonethat gets set as soon as the drivers card detect interrupt shows there is no card (hoping that we are not getting any bouncing on card detect) and then havemmc_wait_for_req()simple return-ENODEVimmediately if thecard_goneflag is set. Finally, if the mmc block driver sees a-ENODEVerror, it should also check thecard_goneflag (via a new core function) and if the card is gone, do not retry - and perhaps even error out the rest of the I/O request queue as well.
[Ghorai] your idea address the 2.a case, but not 2.b, 1.a, 1.b
And the solution I was proposing to return the status of IO failure as soon as possible to above layer; and handle the card removed interrupt separately or any other issue in h/w or s/w or combination of both. Or just think again when platform don't have the card remove interrupt.
So my patch addresses the 1st part
And for the 2nd part we can submit the patch anytime.
Quoted 7 lines
I can suggest a patch if you want but I am on vacation next week so it will have to wait a couple of weeks.
Quoted 1 line
And moreover we should not give the interleave data to apps, as we don't
have option to tell application for the valid data.
Quoted 1 line
[..snip..]
http://comments.gmane.org/gmane.linux.kernel.mmc/2714
Quoted 1 line
Quoted 1 line
PATCH 1/1 mmc: failure of block read wait for long time
Quoted 44 lines · Sukumar
Adrian,
[..snip..]
Quoted 14 lines
Quoted 14 lines
Quoted 13 lines
Quoted 2 lines
[Ghorai] Adrian,
Yes this works and reduced the retry by 1/4 (2048 to 512 times for 1MBdata read) form the original code;
Quoted 1 line
Initially it was retrying for each page(512 bytes) after multi-block
read fail; but this solution is retying for each segment(2048 bytes);
Quoted 1 line
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.
Quoted 1 line
2. So do you think any good reason to retry again and again?
If you have 1MB that is not readable, it sounds like the card is broken. Why are so many reads failing? Has the card been removed?
You might very rarely see ECC errors in a small number of sectors, but more than that sounds like something else is broken.
Quoted 6 lines
Quoted 1 line
[Ghorai] yes, one example is we remove the card when reading data,
Well, that is a different case. Once the card has gone, the block driver can (and will once the remove method is called) error out all I/O requests without sending them to MMC. That doesn't happen until there is a card detect interrupt and a resulting rescan.
[Ghorai] here we are discussing two problem,
1. If IO failed how to stop retry; because of -
a. internal card error
b. issue in Filesystem, driver, or host controller issue c. or cards removed.2. And 2nd how to sync block-layer IO, if card removed,
a. case 1: when card removed interrupt support by the platform b. case 2: when card removed interrupt does not support by the platform?Quoted 9 lines
A possible solution is to put a flag on
mmc_cardto indicatecard_gonethat gets set as soon as the drivers card detect interrupt shows there is no card (hoping that we are not getting any bouncing on card detect) and then havemmc_wait_for_req()simple return-ENODEVimmediately if thecard_goneflag is set. Finally, if the mmc block driver sees a-ENODEVerror, it should also check thecard_goneflag (via a new core function) and if the card is gone, do not retry - and perhaps even error out the rest of the I/O request queue as well.
Quoted 1 line
[Ghorai] your idea address the 2.a case, but not 2.b, 1.a, 1.b
The card removal case can be extended to use the bus ops detect method when there is no card detect irq. I will send a RFC patch.
With respect to 1.a:
- If the card has an internal error, then it is broken. The user should remove the card and use a better one. I do not see how reducing retry delays really helps the user very much. Arguably if the card becomes unresponsive, the MMC core could provide a facility to reinitialise the card, but that is yet another issue.
With respect to 1.b:
- The file system cannot cause the block driver to have I/O errors.
- If there are errors in the driver they should be fixed.
- If there are hardware problems with the host controller, then it is up to the host controller driver to deal with them e.g. by resetting the controller. I don't see what this has to do with the block driver.
You leave out the important case of ECC errors. I am concerned about this because of the possibility that it happens inside a file system journal e.g. EXT4 journal. Perhaps the journal may be recovered if the error only affects the last transaction, but perhaps not if it destroys other transactions - which could happen if the approach you suggest is taken.
Quoted 4 lines
And the solution I was proposing to return the status of IO failure as soon as possible to above layer; and handle the card removed interrupt separately or any other issue in h/w or s/w or combination of both. Or just think again when platform don't have the card remove interrupt.
So my patch addresses the 1st part
It is absolutely unacceptable to return I/O errors to the upper layers for segments that do not have errors.
Quoted 15 lines
And for the 2nd part we can submit the patch anytime.
Quoted 7 lines
I can suggest a patch if you want but I am on vacation next week so it will have to wait a couple of weeks.
Quoted 1 line
And moreover we should not give the interleave data to apps, as we don't
have option to tell application for the valid data.
Quoted 1 line
[..snip..]
http://comments.gmane.org/gmane.linux.kernel.mmc/2714Quoted 1 line
Quoted 1 line
PATCH 1/1 mmc: failure of block read wait for long time
Adrian,
Quoted 78 lines
-----Original Message-----
From: Adrian Hunter [mailto:adrian.hunter at nokia.com]
Sent: Monday, September 20, 2010 1:24 PM
To: Ghorai, Sukumar
Cc: linux-mmc at vger.kernel.org; linux-arm-kernel at lists.infradead.org; Adrian Hunter
Subject: Re: [PATCH] mmc: failure of block read wait for long timeQuoted 5 lines · Sukumar
Adrian,
[..snip..]
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
[Ghorai] Adrian,
Yes this works and reduced the retry by 1/4 (2048 to 512 times for1MB
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
data read) form the original code;
Quoted 1 line
Initially it was retrying for each page(512 bytes) after multi-block
read fail; but this solution is retying for each segment(2048 bytes);
Quoted 1 line
1. Now say block layrer reading 1MB and failed for the 1st segment.
So
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
it will still retry for 1MB/2048-bytes, i.e. 512 times.
Quoted 1 line
2. So do you think any good reason to retry again and again?
If you have 1MB that is not readable, it sounds like the card is
broken.
Quoted 8 lines
Quoted 8 lines
Quoted 6 lines
Quoted 4 lines
Why are so many reads failing? Has the card been removed?
You might very rarely see ECC errors in a small number of sectors, but more than that sounds like something else is broken.
[Ghorai] yes, one example is we remove the card when reading data,
Well, that is a different case. Once the card has gone, the block
driver
Quoted 10 lines
Quoted 3 lines
can (and will once the remove method is called) error out all I/O requests without sending them to MMC. That doesn't happen until there is a card detect interrupt and a resulting rescan.
[Ghorai] here we are discussing two problem,
1. If IO failed how to stop retry; because of -
a. internal card error
b. issue in Filesystem, driver, or host controller issue c. or cards removed.Quoted 3 lines
2. And 2nd how to sync block-layer IO, if card removed,
a. case 1: when card removed interrupt support by the platform b. case 2: when card removed interrupt does not support by theplatform?
Quoted 11 lines
Quoted 10 lines
A possible solution is to put a flag on
mmc_cardto indicatecard_gonethat gets set as soon as the drivers card detect interrupt shows there is no card (hoping that we are not getting any bouncing on card detect) and then havemmc_wait_for_req()simple return-ENODEVimmediately if thecard_goneflag is set. Finally, if the mmc block driver sees a-ENODEVerror, it should also check thecard_goneflag (via a new core function) and if the card is gone, do not retry - and perhaps even error out the rest of the I/O request queue as well.Quoted 1 line
[Ghorai] your idea address the 2.a case, but not 2.b, 1.a, 1.b
The card removal case can be extended to use the bus ops detect method when there is no card detect irq. I will send a RFC patch.
With respect to 1.a:
- If the card has an internal error, then it is broken. The user should remove the card and use a better one. I do not see how reducing retry delays really helps the user very much. Arguably if the card becomes unresponsive, the MMC core could provide a facility to reinitialise the card, but that is yet another issue.With respect to 1.b:
- The file system cannot cause the block driver to have I/O errors.
- If there are errors in the driver they should be fixed.
- If there are hardware problems with the host controller, then it is up to the host controller driver to deal with them e.g. by resetting the controller. I don't see what this has to do with the block driver.
Quoted 7 lines
You leave out the important case of ECC errors. I am concerned about this because of the possibility that it happens inside a file system journal e.g. EXT4 journal. Perhaps the journal may be recovered if the error only affects the last transaction, but perhaps not if it destroys other transactions - which could happen if the approach you suggest is taken.
[Ghorai] Thanks lot for your descriptive answer.
1. Can you answer this? 2.b. case 2: when card removed interrupt does not support by the platform?
2. Why block layer handling for inter-leave data? Can you give example diver who is returning interleave data? And how to tell application that buffer having interleave data?
Quoted 27 lines
Quoted 2 lines
And the solution I was proposing to return the status of IO failure as
soon as possible to above layer; and handle the card removed interrupt separately or any other issue in h/w or s/w or combination of both. Or just think again when platform don't have the card remove interrupt.
Quoted 2 lines
So my patch addresses the 1st part
It is absolutely unacceptable to return I/O errors to the upper layers for segments that do not have errors.
Quoted 7 lines
And for the 2nd part we can submit the patch anytime.
Quoted 5 lines
I can suggest a patch if you want but I am on vacation next week so it will have to wait a couple of weeks.
Quoted 1 line
And moreover we should not give the interleave data to apps, as we
don't
Quoted 8 lines
Quoted 2 lines
have option to tell application for the valid data.
Quoted 1 line
[..snip..]
http://comments.gmane.org/gmane.linux.kernel.mmc/2714Quoted 1 line
Quoted 1 line
PATCH 1/1 mmc: failure of block read wait for long time
Quoted 87 lines · Sukumar
Adrian,
Quoted 78 lines
-----Original Message-----
From: Adrian Hunter [mailto:adrian.hunter at nokia.com]
Sent: Monday, September 20, 2010 1:24 PM
To: Ghorai, Sukumar
Cc: linux-mmc at vger.kernel.org; linux-arm-kernel at lists.infradead.org; Adrian Hunter
Subject: Re: [PATCH] mmc: failure of block read wait for long timeQuoted 5 lines · Sukumar
Adrian,
[..snip..]
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
[Ghorai] Adrian,
Yes this works and reduced the retry by 1/4 (2048 to 512 times for1MB
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
data read) form the original code;
Quoted 1 line
Initially it was retrying for each page(512 bytes) after multi-block
read fail; but this solution is retying for each segment(2048 bytes);
Quoted 1 line
1. Now say block layrer reading 1MB and failed for the 1st segment.
So
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
it will still retry for 1MB/2048-bytes, i.e. 512 times.
Quoted 1 line
2. So do you think any good reason to retry again and again?
If you have 1MB that is not readable, it sounds like the card is
broken.
Quoted 8 lines
Quoted 8 lines
Quoted 6 lines
Quoted 4 lines
Why are so many reads failing? Has the card been removed?
You might very rarely see ECC errors in a small number of sectors, but more than that sounds like something else is broken.
[Ghorai] yes, one example is we remove the card when reading data,
Well, that is a different case. Once the card has gone, the block
driver
Quoted 10 lines
Quoted 3 lines
can (and will once the remove method is called) error out all I/O requests without sending them to MMC. That doesn't happen until there is a card detect interrupt and a resulting rescan.
[Ghorai] here we are discussing two problem,
1. If IO failed how to stop retry; because of -
a. internal card error
b. issue in Filesystem, driver, or host controller issue c. or cards removed.Quoted 3 lines
2. And 2nd how to sync block-layer IO, if card removed,
a. case 1: when card removed interrupt support by the platform b. case 2: when card removed interrupt does not support by theplatform?
Quoted 11 lines
Quoted 10 lines
A possible solution is to put a flag on
mmc_cardto indicatecard_gonethat gets set as soon as the drivers card detect interrupt shows there is no card (hoping that we are not getting any bouncing on card detect) and then havemmc_wait_for_req()simple return-ENODEVimmediately if thecard_goneflag is set. Finally, if the mmc block driver sees a-ENODEVerror, it should also check thecard_goneflag (via a new core function) and if the card is gone, do not retry - and perhaps even error out the rest of the I/O request queue as well.Quoted 1 line
[Ghorai] your idea address the 2.a case, but not 2.b, 1.a, 1.b
The card removal case can be extended to use the bus ops detect method when there is no card detect irq. I will send a RFC patch.
With respect to 1.a:
- If the card has an internal error, then it is broken. The user should remove the card and use a better one. I do not see how reducing retry delays really helps the user very much. Arguably if the card becomes unresponsive, the MMC core could provide a facility to reinitialise the card, but that is yet another issue.With respect to 1.b:
- The file system cannot cause the block driver to have I/O errors.
- If there are errors in the driver they should be fixed.
- If there are hardware problems with the host controller, then it is up to the host controller driver to deal with them e.g. by resetting the controller. I don't see what this has to do with the block driver.Quoted 7 lines
You leave out the important case of ECC errors. I am concerned about this because of the possibility that it happens inside a file system journal e.g. EXT4 journal. Perhaps the journal may be recovered if the error only affects the last transaction, but perhaps not if it destroys other transactions - which could happen if the approach you suggest is taken.
Quoted 2 lines
[Ghorai] Thanks lot for your descriptive answer.
1. Can you answer this? 2.b. case 2: when card removed interrupt does not support by the platform?
As I wrote above: The card removal case can be extended to use the bus ops detect method when there is no card detect irq. I will send a RFC patch.
Quoted 2 lines
2. Why block layer handling for inter-leave data? Can you give example diver who is returning interleave data? And how to tell application that buffer having interleave data?
I am not sure what you mean by interleave data, but file systems for example are free to map any block to any file, directory or file system object, so a consecutive series of sectors may contain unrelated data. Up to a maximum size, the block layer merges I/O requests when the sectors are consecutive, so an I/O request can also contain unrelated data.
Quoted 34 lines
Quoted 12 lines
Quoted 3 lines
And the solution I was proposing to return the status of IO failure as
soon as possible to above layer; and handle the card removed interrupt separately or any other issue in h/w or s/w or combination of both. Or just think again when platform don't have the card remove interrupt.
Quoted 2 lines
So my patch addresses the 1st part
It is absolutely unacceptable to return I/O errors to the upper layers for segments that do not have errors.
Quoted 16 lines
Quoted 7 lines
And for the 2nd part we can submit the patch anytime.
Quoted 5 lines
I can suggest a patch if you want but I am on vacation next week so it will have to wait a couple of weeks.
Quoted 1 line
And moreover we should not give the interleave data to apps, as we
don't
Quoted 8 lines
Quoted 2 lines
have option to tell application for the valid data.
Quoted 1 line
[..snip..]
http://comments.gmane.org/gmane.linux.kernel.mmc/2714Quoted 1 line
Quoted 1 line
--
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
PATCH 1/1 mmc: failure of block read wait for long time
Quoted 117 lines
-----Original Message-----
From: Adrian Hunter [mailto:adrian.hunter at nokia.com]
Sent: Monday, September 20, 2010 5:20 PM
To: Ghorai, Sukumar
Cc: linux-mmc at vger.kernel.org; linux-arm-kernel at lists.infradead.org; Adrian Hunter
Subject: Re: [PATCH] mmc: failure of block read wait for long timeQuoted 19 lines · Sukumar
Adrian,
Quoted 17 lines
-----Original Message-----
From: Adrian Hunter [mailto:adrian.hunter at nokia.com]
Sent: Monday, September 20, 2010 1:24 PM
To: Ghorai, Sukumar
Cc: linux-mmc at vger.kernel.org; linux-arm-kernel at lists.infradead.org; Adrian Hunter
Subject: Re: [PATCH] mmc: failure of block read wait for long timeQuoted 5 lines · Sukumar
Adrian,
[..snip..]
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
[Ghorai] Adrian,
Yes this works and reduced the retry by 1/4 (2048 to 512 times for1MB
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
data read) form the original code;
Quoted 1 line
Initially it was retrying for each page(512 bytes) after multi-
block
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
read fail; but this solution is retying for each segment(2048
bytes);
Quoted 11 lines
Quoted 11 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
1. Now say block layrer reading 1MB and failed for the 1st segment.
So
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
it will still retry for 1MB/2048-bytes, i.e. 512 times.
Quoted 1 line
2. So do you think any good reason to retry again and again?
If you have 1MB that is not readable, it sounds like the card is
broken.
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 4 lines
Why are so many reads failing? Has the card been removed?
You might very rarely see ECC errors in a small number of sectors, but more than that sounds like something else is broken.
Quoted 6 lines
Quoted 6 lines
Quoted 3 lines
Quoted 3 lines
Quoted 1 line
[Ghorai] yes, one example is we remove the card when reading data,
Well, that is a different case. Once the card has gone, the block
driver
Quoted 2 lines
Quoted 2 lines
can (and will once the remove method is called) error out all I/O requests without sending them to MMC. That doesn't happen until
there
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 1 line
is a card detect interrupt and a resulting rescan.
[Ghorai] here we are discussing two problem,
1. If IO failed how to stop retry; because of -
a. internal card error
b. issue in Filesystem, driver, or host controller issue c. or cards removed.Quoted 7 lines
Quoted 7 lines
Quoted 3 lines
2. And 2nd how to sync block-layer IO, if card removed,
a. case 1: when card removed interrupt support by the platform b. case 2: when card removed interrupt does not support by theplatform?
Quoted 3 lines
Quoted 3 lines
A possible solution is to put a flag on
mmc_cardto indicate
card_goneQuoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
that gets set as soon as the drivers card detect interrupt shows
there
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
is no card (hoping that we are not getting any bouncing on card
detect)
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 5 lines
and then have
mmc_wait_for_req()simple return-ENODEVimmediately if thecard_goneflag is set. Finally, if the mmc block driver sees a-ENODEVerror, it should also check thecard_goneflag (via a new core function) and if the card is gone, do not retry - and perhaps even error out the rest of the I/O request queue as well.Quoted 8 lines
Quoted 8 lines
Quoted 1 line
[Ghorai] your idea address the 2.a case, but not 2.b, 1.a, 1.b
The card removal case can be extended to use the bus ops detect method when there is no card detect irq. I will send a RFC patch.
With respect to 1.a:
- If the card has an internal error, then it is broken. The user should remove the card and use a better one. I do not see howreducing
Quoted 19 lines
Quoted 12 lines
retry delays really helps the user very much. Arguably if the card becomes unresponsive, the MMC core could provide a facility to reinitialise the card, but that is yet another issue.
With respect to 1.b:
- The file system cannot cause the block driver to have I/O errors.
- If there are errors in the driver they should be fixed.
- If there are hardware problems with the host controller, then it is up to the host controller driver to deal with them e.g. by resetting the controller. I don't see what this has to do with the block driver.Quoted 7 lines
You leave out the important case of ECC errors. I am concerned about this because of the possibility that it happens inside a file system journal e.g. EXT4 journal. Perhaps the journal may be recovered if the error only affects the last transaction, but perhaps not if it destroys other transactions - which could happen if the approach you suggest is taken.
Quoted 2 lines
[Ghorai] Thanks lot for your descriptive answer.
1. Can you answer this? 2.b. case 2: when card removed interrupt doesnot support by the platform?
As I wrote above: The card removal case can be extended to use the bus ops detect method when there is no card detect irq. I will send a RFC patch.
Quoted 2 lines
2. Why block layer handling for inter-leave data? Can you give example
diver who is returning interleave data? And how to tell application that buffer having interleave data?
Quoted 7 lines
I am not sure what you mean by interleave data, but file systems for example
are free to map any block to any file, directory or file system object, so a consecutive series of sectors may contain unrelated data. Up to a maximum
size, the block layer merges I/O requests when the sectors are consecutive, so an I/O request can also contain unrelated data.
[Ghorai]
1. I don't think so, FS know where data exists and where is the free space. Except oth cluster.
2. Where its mentioned in block media that for segment-x[i],x[j] data read fail out of all all requested segments form [1..n]. And I never gone through any driver/protocol, that retry the next i+1th segment where ith-segment is failed. And for that my suggestion is preferred.
Quoted 35 lines
Quoted 35 lines
Quoted 13 lines
Quoted 4 lines
And the solution I was proposing to return the status of IO failure as
soon as possible to above layer; and handle the card removed interrupt separately or any other issue in h/w or s/w or combination of both. Or just think again when platform don't have the card remove interrupt.
Quoted 2 lines
So my patch addresses the 1st part
It is absolutely unacceptable to return I/O errors to the upper layers for segments that do not have errors.
Quoted 16 lines
Quoted 7 lines
And for the 2nd part we can submit the patch anytime.
Quoted 5 lines
I can suggest a patch if you want but I am on vacation next week so it will have to wait a couple of weeks.
Quoted 1 line
And moreover we should not give the interleave data to apps, as we
don't
Quoted 8 lines
Quoted 2 lines
have option to tell application for the valid data.
Quoted 1 line
[..snip..]
http://comments.gmane.org/gmane.linux.kernel.mmc/2714Quoted 1 line
Quoted 1 line
--
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
PATCH 1/1 mmc: failure of block read wait for long time
Quoted 125 lines · Sukumar
Quoted 117 lines
-----Original Message-----
From: Adrian Hunter [mailto:adrian.hunter at nokia.com]
Sent: Monday, September 20, 2010 5:20 PM
To: Ghorai, Sukumar
Cc: linux-mmc at vger.kernel.org; linux-arm-kernel at lists.infradead.org; Adrian Hunter
Subject: Re: [PATCH] mmc: failure of block read wait for long timeQuoted 19 lines · Sukumar
Adrian,
Quoted 17 lines
-----Original Message-----
From: Adrian Hunter [mailto:adrian.hunter at nokia.com]
Sent: Monday, September 20, 2010 1:24 PM
To: Ghorai, Sukumar
Cc: linux-mmc at vger.kernel.org; linux-arm-kernel at lists.infradead.org; Adrian Hunter
Subject: Re: [PATCH] mmc: failure of block read wait for long timeQuoted 5 lines · Sukumar
Adrian,
[..snip..]
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
[Ghorai] Adrian,
Yes this works and reduced the retry by 1/4 (2048 to 512 times for1MB
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
data read) form the original code;
Quoted 1 line
Initially it was retrying for each page(512 bytes) after multi-
block
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
read fail; but this solution is retying for each segment(2048
bytes);
Quoted 11 lines
Quoted 11 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
1. Now say block layrer reading 1MB and failed for the 1st segment.
So
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
it will still retry for 1MB/2048-bytes, i.e. 512 times.
Quoted 1 line
2. So do you think any good reason to retry again and again?
If you have 1MB that is not readable, it sounds like the card is
broken.
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 4 lines
Why are so many reads failing? Has the card been removed?
You might very rarely see ECC errors in a small number of sectors, but more than that sounds like something else is broken.
Quoted 6 lines
Quoted 6 lines
Quoted 3 lines
Quoted 3 lines
Quoted 1 line
[Ghorai] yes, one example is we remove the card when reading data,
Well, that is a different case. Once the card has gone, the block
driver
Quoted 2 lines
Quoted 2 lines
can (and will once the remove method is called) error out all I/O requests without sending them to MMC. That doesn't happen until
there
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 1 line
is a card detect interrupt and a resulting rescan.
[Ghorai] here we are discussing two problem,
1. If IO failed how to stop retry; because of -
a. internal card error
b. issue in Filesystem, driver, or host controller issue c. or cards removed.Quoted 7 lines
Quoted 7 lines
Quoted 3 lines
2. And 2nd how to sync block-layer IO, if card removed,
a. case 1: when card removed interrupt support by the platform b. case 2: when card removed interrupt does not support by theplatform?
Quoted 3 lines
Quoted 3 lines
A possible solution is to put a flag on
mmc_cardto indicate
card_goneQuoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
that gets set as soon as the drivers card detect interrupt shows
there
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
is no card (hoping that we are not getting any bouncing on card
detect)
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 5 lines
and then have
mmc_wait_for_req()simple return-ENODEVimmediately if thecard_goneflag is set. Finally, if the mmc block driver sees a-ENODEVerror, it should also check thecard_goneflag (via a new core function) and if the card is gone, do not retry - and perhaps even error out the rest of the I/O request queue as well.Quoted 8 lines
Quoted 8 lines
Quoted 1 line
[Ghorai] your idea address the 2.a case, but not 2.b, 1.a, 1.b
The card removal case can be extended to use the bus ops detect method when there is no card detect irq. I will send a RFC patch.
With respect to 1.a:
- If the card has an internal error, then it is broken. The user should remove the card and use a better one. I do not see howreducing
Quoted 19 lines
Quoted 12 lines
retry delays really helps the user very much. Arguably if the card becomes unresponsive, the MMC core could provide a facility to reinitialise the card, but that is yet another issue.
With respect to 1.b:
- The file system cannot cause the block driver to have I/O errors.
- If there are errors in the driver they should be fixed.
- If there are hardware problems with the host controller, then it is up to the host controller driver to deal with them e.g. by resetting the controller. I don't see what this has to do with the block driver.Quoted 7 lines
You leave out the important case of ECC errors. I am concerned about this because of the possibility that it happens inside a file system journal e.g. EXT4 journal. Perhaps the journal may be recovered if the error only affects the last transaction, but perhaps not if it destroys other transactions - which could happen if the approach you suggest is taken.
Quoted 2 lines
[Ghorai] Thanks lot for your descriptive answer.
1. Can you answer this? 2.b. case 2: when card removed interrupt doesnot support by the platform?
As I wrote above: The card removal case can be extended to use the bus ops detect method when there is no card detect irq. I will send a RFC patch.
Quoted 2 lines
2. Why block layer handling for inter-leave data? Can you give example
diver who is returning interleave data? And how to tell application that buffer having interleave data?
Quoted 7 lines
I am not sure what you mean by interleave data, but file systems for example
are free to map any block to any file, directory or file system object, so a consecutive series of sectors may contain unrelated data. Up to a maximum
size, the block layer merges I/O requests when the sectors are consecutive, so an I/O request can also contain unrelated data.
Quoted 2 lines
[Ghorai]
1. I don't think so, FS know where data exists and where is the free space. Except oth cluster.
I was not talking about free space. I was giving an example of why it is not possible to assume anything about what is in an I/O request.
Quoted 3 lines
2. Where its mentioned in block media that for segment-x[i],x[j] data read fail out of all all requested segments form [1..n]. And I never gone through any driver/protocol, that retry the next i+1th segment where ith-segment is failed. And for that my suggestion is preferred.
The SD/MMC protocol does not indicate which sector has the error. There is no possibility of trying the ith+1-segment because i is unknown.
Quoted 38 lines
Quoted 36 lines
Quoted 36 lines
Quoted 14 lines
Quoted 5 lines
And the solution I was proposing to return the status of IO failure as
soon as possible to above layer; and handle the card removed interrupt separately or any other issue in h/w or s/w or combination of both. Or just think again when platform don't have the card remove interrupt.
Quoted 2 lines
So my patch addresses the 1st part
It is absolutely unacceptable to return I/O errors to the upper layers for segments that do not have errors.
Quoted 16 lines
Quoted 7 lines
And for the 2nd part we can submit the patch anytime.
Quoted 5 lines
I can suggest a patch if you want but I am on vacation next week so it will have to wait a couple of weeks.
Quoted 1 line
And moreover we should not give the interleave data to apps, as we
don't
Quoted 8 lines
Quoted 2 lines
have option to tell application for the valid data.
Quoted 1 line
[..snip..]
http://comments.gmane.org/gmane.linux.kernel.mmc/2714Quoted 1 line
Quoted 1 line
--
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
PATCH 1/1 mmc: failure of block read wait for long time
Quoted 159 lines
-----Original Message-----
From: Adrian Hunter [mailto:adrian.hunter at nokia.com]
Sent: Monday, September 20, 2010 6:39 PM
To: Ghorai, Sukumar
Cc: linux-mmc at vger.kernel.org; linux-arm-kernel at lists.infradead.org; Adrian Hunter
Subject: Re: [PATCH] mmc: failure of block read wait for long timeQuoted 21 lines · Sukumar
Quoted 21 lines
-----Original Message-----
From: Adrian Hunter [mailto:adrian.hunter at nokia.com]
Sent: Monday, September 20, 2010 5:20 PM
To: Ghorai, Sukumar
Cc: linux-mmc at vger.kernel.org; linux-arm-kernel at lists.infradead.org; Adrian Hunter
Subject: Re: [PATCH] mmc: failure of block read wait for long timeQuoted 10 lines · Sukumar
Adrian,
Quoted 8 lines
-----Original Message-----
From: Adrian Hunter [mailto:adrian.hunter at nokia.com]
Sent: Monday, September 20, 2010 1:24 PM
To: Ghorai, Sukumar
Cc: linux-mmc at vger.kernel.org; linux-arm-kernel at lists.infradead.org; Adrian Hunter
Subject: Re: [PATCH] mmc: failure of block read wait for long timeQuoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines · Sukumar
Quoted 5 lines
Adrian,
[..snip..]
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
[Ghorai] Adrian,
Yes this works and reduced the retry by 1/4 (2048 to 512 timesfor
Quoted 7 lines
Quoted 7 lines
Quoted 3 lines
Quoted 3 lines
1MB
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
data read) form the original code;
Quoted 1 line
Initially it was retrying for each page(512 bytes) after multi-
block
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
read fail; but this solution is retying for each segment(2048
bytes);
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
1. Now say block layrer reading 1MB and failed for the 1st
segment.
Quoted 31 lines
Quoted 31 lines
Quoted 10 lines
Quoted 10 lines
So
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
it will still retry for 1MB/2048-bytes, i.e. 512 times.
Quoted 1 line
2. So do you think any good reason to retry again and again?
If you have 1MB that is not readable, it sounds like the card is
broken.
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 4 lines
Why are so many reads failing? Has the card been removed?
You might very rarely see ECC errors in a small number of sectors, but more than that sounds like something else is broken.
Quoted 6 lines
Quoted 6 lines
Quoted 3 lines
Quoted 3 lines
Quoted 1 line
[Ghorai] yes, one example is we remove the card when reading data,
Well, that is a different case. Once the card has gone, the block
driver
Quoted 2 lines
Quoted 2 lines
can (and will once the remove method is called) error out all I/O requests without sending them to MMC. That doesn't happen until
there
Quoted 14 lines
Quoted 14 lines
Quoted 11 lines
Quoted 1 line
is a card detect interrupt and a resulting rescan.
[Ghorai] here we are discussing two problem,
1. If IO failed how to stop retry; because of -
a. internal card error
b. issue in Filesystem, driver, or host controller issue c. or cards removed.2. And 2nd how to sync block-layer IO, if card removed,
a. case 1: when card removed interrupt support by the platform b. case 2: when card removed interrupt does not support by theplatform?
Quoted 2 lines
Quoted 2 lines
Quoted 7 lines
Quoted 7 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
A possible solution is to put a flag on
mmc_cardto indicate
card_goneQuoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
that gets set as soon as the drivers card detect interrupt shows
there
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
is no card (hoping that we are not getting any bouncing on card
detect)
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
and then have
mmc_wait_for_req()simple return-ENODEVimmediatelyif
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 6 lines
Quoted 4 lines
the
card_goneflag is set. Finally, if the mmc block driver sees a-ENODEVerror, it should also check thecard_goneflag (via a new core function) and if the card is gone, do not retry - and perhaps even error out the rest of the I/O request queue as well.[Ghorai] your idea address the 2.a case, but not 2.b, 1.a, 1.b
The card removal case can be extended to use the bus ops detect
method
Quoted 7 lines
Quoted 7 lines
Quoted 5 lines
Quoted 5 lines
when there is no card detect irq. I will send a RFC patch.
With respect to 1.a:
- If the card has an internal error, then it is broken. The user should remove the card and use a better one. I do not see howreducing
Quoted 1 line
Quoted 1 line
retry delays really helps the user very much. Arguably if the
card
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
becomes unresponsive, the MMC core could provide a facility to reinitialise the card, but that is yet another issue.
With respect to 1.b:
- The file system cannot cause the block driver to have I/Oerrors.
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
- If there are errors in the driver they should be fixed.
- If there are hardware problems with the host controller, then it is up to the host controller driver to deal with them e.g. by resetting the controller. I don't see what this has to dowith
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
the block driver.
You leave out the important case of ECC errors. I am concerned about this because of the possibility that it happens inside a file system journal e.g. EXT4 journal. Perhaps the journal may be recovered if
the
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
error only affects the last transaction, but perhaps not if it
destroys
Quoted 8 lines
Quoted 8 lines
Quoted 5 lines
Quoted 3 lines
other transactions - which could happen if the approach you suggest is taken.
[Ghorai] Thanks lot for your descriptive answer.
1. Can you answer this? 2.b. case 2: when card removed interrupt doesnot support by the platform?
As I wrote above: The card removal case can be extended to use the bus
ops
Quoted 1 line
Quoted 1 line
detect method when there is no card detect irq. I will send a RFC
patch.
Quoted 4 lines
Quoted 4 lines
Quoted 3 lines
2. Why block layer handling for inter-leave data? Can you give example
diver who is returning interleave data? And how to tell application
that
Quoted 8 lines
Quoted 8 lines
buffer having interleave data?
I am not sure what you mean by interleave data, but file systems for example
are free to map any block to any file, directory or file system object, so a consecutive series of sectors may contain unrelated data. Up to a maximum
size, the block layer merges I/O requests when the sectors areconsecutive,
Quoted 4 lines
Quoted 1 line
so an I/O request can also contain unrelated data.
[Ghorai]
1. I don't think so, FS know where data exists and where is the freespace. Except oth cluster.
I was not talking about free space. I was giving an example of why it is not possible to assume anything about what is in an I/O request.
Quoted 2 lines
2. Where its mentioned in block media that for segment-x[i],x[j] data
Quoted 8 lines
read fail out of all all requested segments form [1..n].
Quoted 1 line
And I never gone through any driver/protocol, that retry the next i+1th
segment where ith-segment is failed. And for that my suggestion is preferred.
The SD/MMC protocol does not indicate which sector has the error. There is no possibility of trying the ith+1-segment because i is unknown.
[Ghorai] I understand, but I think still you are fever of retrying each and every sector(i, i+1, i+2, ..) for the all request segments sequentially, when single block read for the i-th sector failed.
Quoted 44 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
And the solution I was proposing to return the status of IO failure
as
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
soon as possible to above layer; and handle the card removed
interrupt
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
separately or any other issue in h/w or s/w or combination of both.
Or
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
just think again when platform don't have the card remove interrupt.
Quoted 2 lines
So my patch addresses the 1st part
It is absolutely unacceptable to return I/O errors to the upper
layers
Quoted 19 lines
Quoted 19 lines
Quoted 19 lines
Quoted 18 lines
for segments that do not have errors.
Quoted 7 lines
And for the 2nd part we can submit the patch anytime.
Quoted 5 lines
I can suggest a patch if you want but I am on vacation next week so it will have to wait a couple of weeks.
Quoted 1 line
And moreover we should not give the interleave data to apps, as we
don't
Quoted 8 lines
Quoted 2 lines
have option to tell application for the valid data.
Quoted 1 line
[..snip..]
http://comments.gmane.org/gmane.linux.kernel.mmc/2714Quoted 1 line
Quoted 1 line
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc"in
Quoted 5 lines
Quoted 3 lines
Quoted 3 lines
the body of a message to majordomo at vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH 1/1 mmc: failure of block read wait for long time
In reply to Adrian HunterQuoted 3 lines · Adrian Hunter
The SD/MMC protocol does not indicate which sector has the error. There is no possibility of trying the ith+1-segment because i is unknown.
Not entirely true.
I seem to remember that the MMC protocol (and probably SD protocol) provides an ack at the end of each data block to say whether the data block was correct or not.
Some hosts tell you when they receive this ack, and you can track how many blocks have been successfully read without error. The ARM MMCI primecell does this.
However, some hosts don't give you this information, in which case you don't know where the error occurred in the read - so when a read fails, you can only safely report that no bytes were read successfully. This means that even if the error was on the last block, there's no way to know this.
PATCH 1/1 mmc: failure of block read wait for long time
Chris,
Quoted 23 lines
-----Original Message-----
From: Russell King - ARM Linux [mailto:linux at arm.linux.org.uk]
Sent: Monday, September 20, 2010 7:07 PM
To: Adrian Hunter
Cc: Ghorai, Sukumar; linux-mmc at vger.kernel.org; linux-arm- kernel at lists.infradead.org
Subject: Re: [PATCH] mmc: failure of block read wait for long timeQuoted 3 lines · Adrian Hunter
The SD/MMC protocol does not indicate which sector has the error. There is no possibility of trying the ith+1-segment because i is unknown.
Not entirely true.
I seem to remember that the MMC protocol (and probably SD protocol) provides an ack at the end of each data block to say whether the data block was correct or not.
Some hosts tell you when they receive this ack, and you can track how many blocks have been successfully read without error. The ARM MMCI primecell does this.
Quoted 5 lines
However, some hosts don't give you this information, in which case you don't know where the error occurred in the read - so when a read fails, you can only safely report that no bytes were read successfully. This means that even if the error was on the last block, there's no way to know this.
[Ghorai]
Would you please review and merge this patch [1] (attached too)?
[1] http://comments.gmane.org/gmane.linux.kernel.mmc/2714
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-mmc-failure-of-block-read-wait-for-long-time.patch
Type: application/octet-stream
Size: 1114 bytes
Desc: 0001-mmc-failure-of-block-read-wait-for-long-time.patch
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20100922/ee9f688f/attachment.obj>
PATCH 1/1 mmc: failure of block read wait for long time
Quoted 2 lines · Sukumar
Would you please review and merge this patch [1] (attached too)?
[1] http://comments.gmane.org/gmane.linux.kernel.mmc/2714
I've been following the thread. I believe Adrian has NACKed this patch, by saying "It is absolutely unacceptable to return I/O errors to the upper layers for segments that do not have errors."
I think it's possible to merge patches to improve the situation (such as the idea of noticing a card disappearing earlier), but your initial patch is not the patch to do that. You should continue to work with Adrian -- when he's happy that a patch does not break the semantics above, we can consider merging it.
Thanks,
Signature · 2 lines
Chris Ball <[email protected]> <http://printf.net/> One Laptop Per Child
PATCH 1/1 mmc: failure of block read wait for long time
Chris,
Quoted 8 lines
-----Original Message-----
From: Chris Ball [mailto:cjb at laptop.org]
Sent: Wednesday, September 22, 2010 6:13 PM
To: Ghorai, Sukumar
Cc: Adrian Hunter; linux-mmc at vger.kernel.org; linux-arm- kernel at lists.infradead.org; Russell King - ARM Linux
Subject: Re: [PATCH] mmc: failure of block read wait for long time
Quoted 7 lines
Quoted 2 lines · Sukumar
Would you please review and merge this patch [1] (attached too)?
[1] http://comments.gmane.org/gmane.linux.kernel.mmc/2714I've been following the thread. I believe Adrian has NACKed this patch, by saying "It is absolutely unacceptable to return I/O errors to the upper layers for segments that do not have errors."
[Ghorai]
I think Russell also mentioned his opinion. Would you please add your idea too?
1. I would prefer Adrian to explain again what this statement means, in the context - data read fail and how we make it success?
2. if data read fail for sector(x) why we have to try for sector(x+1, ..x+n)?
3. how to inform reader function which sector having the valid data out of (1...n) sectors.
4. do we have any driver/code in Linux or any other os, which give inter-leave data and return as success?
Quoted 12 lines
I think it's possible to merge patches to improve the situation (such as the idea of noticing a card disappearing earlier), but your initial patch is not the patch to do that. You should continue to work with Adrian -- when he's happy that a patch does not break the semantics above, we can consider merging it.
Thanks,
--
Chris Ball <[email protected]> <http://printf.net/>
One Laptop Per Child
PATCH 1/1 mmc: failure of block read wait for long time
In reply to Chris BallChris and Adrian,
[..snip..]
Quoted 24 lines
Quoted 9 lines
-----Original Message-----
From: Chris Ball [mailto:cjb at laptop.org]
Sent: Wednesday, September 22, 2010 6:13 PM
To: Ghorai, Sukumar
Cc: Adrian Hunter; linux-mmc at vger.kernel.org; linux-arm- kernel at lists.infradead.org; Russell King - ARM Linux
Subject: Re: [PATCH] mmc: failure of block read wait for long timeQuoted 7 lines
Quoted 2 lines · Sukumar
Would you please review and merge this patch [1] (attached too)?
[1] http://comments.gmane.org/gmane.linux.kernel.mmc/2714I've been following the thread. I believe Adrian has NACKed this patch, by saying "It is absolutely unacceptable to return I/O errors to the upper layers for segments that do not have errors."
[Ghorai]
I think Russell also mentioned his opinion. Would you please add your idea too?1. I would prefer Adrian to explain again what this statement means, in the context - data read fail and how we make it success?
Quoted 9 lines
2. if data read fail for sector(x) why we have to try for
sector(x+1, ..x+n)?3. how to inform reader function which sector having the valid data out of (1...n) sectors.
4. do we have any driver/code in Linux or any other os, which give inter- leave data and return as success?
[Ghorai] please reply with your input on my/ Russell's suggestion?
Quoted 12 lines
Quoted 12 lines
I think it's possible to merge patches to improve the situation (such as the idea of noticing a card disappearing earlier), but your initial patch is not the patch to do that. You should continue to work with Adrian -- when he's happy that a patch does not break the semantics above, we can consider merging it.
Thanks,
--
Chris Ball <[email protected]> <http://printf.net/>
One Laptop Per Child
PATCH 1/1 mmc: failure of block read wait for long time
In reply to Chris BallChris and Adrian,
[..snip..]
Quoted 6 lines
Chris and Adrian,
[..snip..]
Quoted 2 lines
Quoted 2 lines
-----Original Message-----
[..snip..]
Quoted 23 lines
Quoted 7 lines
Quoted 7 lines
Subject: Re: [PATCH] mmc: failure of block read wait for long time
Quoted 2 lines · Sukumar
Would you please review and merge this patch [1] (attached too)?
[1] http://comments.gmane.org/gmane.linux.kernel.mmc/2714I've been following the thread. I believe Adrian has NACKed this
patch,
Quoted 5 lines
Quoted 2 lines
by saying "It is absolutely unacceptable to return I/O errors to the upper layers for segments that do not have errors."
[Ghorai]
I think Russell also mentioned his opinion. Would you please add youridea
Quoted 9 lines
too?
1. I would prefer Adrian to explain again what this statement means, in the context - data read fail and how we make it success?
2. if data read fail for sector(x) why we have to try for
sector(x+1, ..x+n)?3. how to inform reader function which sector having the valid data out
Quoted 8 lines
of
Quoted 3 lines
(1...n) sectors.
4. do we have any driver/code in Linux or any other os, which give
inter-
Quoted 2 lines
leave data and return as success?
[Ghorai] please reply with your input on my/ Russell's suggestion?
[Ghorai] any input?
Quoted 13 lines
Quoted 13 lines
Quoted 13 lines
I think it's possible to merge patches to improve the situation (such as the idea of noticing a card disappearing earlier), but your initial patch is not the patch to do that. You should continue to work with Adrian -- when he's happy that a patch does not break the semantics above, we can consider merging it.
Thanks,
--
Chris Ball <[email protected]> <http://printf.net/>
One Laptop Per Child
PATCH 1/1 mmc: failure of block read wait for long time
Quoted 28 lines · Sukumar
Chris and Adrian,
[..snip..]
Quoted 6 lines
Chris and Adrian,
[..snip..]
Quoted 2 lines
Quoted 2 lines
-----Original Message-----
[..snip..]
Quoted 18 lines
Quoted 7 lines
Quoted 7 lines
Subject: Re: [PATCH] mmc: failure of block read wait for long time
Quoted 2 lines · Sukumar
Would you please review and merge this patch [1] (attached too)?
[1] http://comments.gmane.org/gmane.linux.kernel.mmc/2714I've been following the thread. I believe Adrian has NACKed this
patch,
Quoted 5 lines
Quoted 2 lines
by saying "It is absolutely unacceptable to return I/O errors to the upper layers for segments that do not have errors."
[Ghorai]
I think Russell also mentioned his opinion. Would you please add youridea
Quoted 4 lines
too?
1. I would prefer Adrian to explain again what this statement means, in the context - data read fail and how we make it success?
Because I/O requests are made up of segments and every segment can be a success or failure.
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
2. if data read fail for sector(x) why we have to try for
sector(x+1, ..x+n)?
See answer to q. 1
Quoted 4 lines
Quoted 4 lines
Quoted 2 lines
3. how to inform reader function which sector having the valid data out
of
Quoted 1 line
(1...n) sectors.
__blk_end_request() does that
Quoted 4 lines
Quoted 4 lines
Quoted 2 lines
4. do we have any driver/code in Linux or any other os, which give
inter-
Quoted 1 line
leave data and return as success?
Here is the problem with that question. The *same* I/O request can have data for *different*sources.
Quoted 3 lines
Quoted 2 lines
Quoted 1 line
[Ghorai] please reply with your input on my/ Russell's suggestion?
[Ghorai] any input?
I have a question for you. What use cases do you want to address
- other than card removal?
Quoted 14 lines
Quoted 13 lines
Quoted 13 lines
Quoted 13 lines
I think it's possible to merge patches to improve the situation (such as the idea of noticing a card disappearing earlier), but your initial patch is not the patch to do that. You should continue to work with Adrian -- when he's happy that a patch does not break the semantics above, we can consider merging it.
Thanks,
--
Chris Ball<[email protected]> <http://printf.net/>
One Laptop Per Child
PATCH 1/1 mmc: failure of block read wait for long time
Adrian,
Quoted 33 lines
-----Original Message-----
From: Adrian Hunter [mailto:adrian.hunter at nokia.com]
Sent: Wednesday, September 29, 2010 12:03 AM
To: Ghorai, Sukumar
Cc: Chris Ball; linux-mmc at vger.kernel.org; linux-arm- kernel at lists.infradead.org; Russell King - ARM Linux
Subject: Re: [PATCH] mmc: failure of block read wait for long timeQuoted 24 lines · Sukumar
Chris and Adrian,
[..snip..]
Quoted 6 lines
Chris and Adrian,
[..snip..]
Quoted 2 lines
Quoted 2 lines
-----Original Message-----
[..snip..]
Quoted 14 lines
Quoted 7 lines
Quoted 7 lines
Subject: Re: [PATCH] mmc: failure of block read wait for long time
Quoted 2 lines · Sukumar
Would you please review and merge this patch [1] (attached too)?
[1] http://comments.gmane.org/gmane.linux.kernel.mmc/2714I've been following the thread. I believe Adrian has NACKed this
patch,
Quoted 5 lines
Quoted 2 lines
by saying "It is absolutely unacceptable to return I/O errors to the upper layers for segments that do not have errors."
[Ghorai]
I think Russell also mentioned his opinion. Would you please add youridea
Quoted 8 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
too?
1. I would prefer Adrian to explain again what this statement means,
in
Quoted 1 line
Quoted 1 line
Quoted 1 line
the context - data read fail and how we make it success?
Because I/O requests are made up of segments and every segment can be a success or failure.
[Ghorai] don't you conflict your self for the comments you provide for following patch - [PATCH] MMC: Refine block layer waiting for card state
[Adrian].. then why wait for lots of errors before doing it.
Quoted 8 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
2. if data read fail for sector(x) why we have to try for
sector(x+1, ..x+n)?See answer to q. 1
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 6 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
3. how to inform reader function which sector having the valid data
out
Quoted 2 lines
Quoted 2 lines
of
Quoted 1 line
(1...n) sectors.
__blk_end_request()does that
[Ghorai] not true. Please check the code again.
Quoted 8 lines
Quoted 5 lines
Quoted 5 lines
Quoted 3 lines
4. do we have any driver/code in Linux or any other os, which give
inter-
Quoted 1 line
leave data and return as success?
Here is the problem with that question. The *same* I/O request can have data for *different*sources.
[Ghorai] File system does not do that and can you test that once how data comes from difference soure? Also conflicting your-self for the input you gave for the patch and as -
[PATCH] MMC: Refine block layer waiting for card state
[Adrian].. then why wait for lots of errors before doing it.
Quoted 7 lines
Quoted 4 lines
Quoted 3 lines
Quoted 2 lines
[Ghorai] please reply with your input on my/ Russell's suggestion?
[Ghorai] any input?
I have a question for you. What use cases do you want to address
- other than card removal?
[Ghorai]
1. can you reply to original input form Russell's on the same thread?
2. can you check if you return the interleave data to FS how it can behave?
3. still you don't have any reference driver which provide the interleave data.
Quoted 16 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
I think it's possible to merge patches to improve the situation (such as the idea of noticing a card disappearing earlier), but your
initial
Quoted 10 lines
Quoted 9 lines
Quoted 9 lines
Quoted 9 lines
patch is not the patch to do that. You should continue to work with Adrian -- when he's happy that a patch does not break the semantics above, we can consider merging it.
Thanks,
--
Chris Ball<[email protected]> <http://printf.net/>
One Laptop Per Child
PATCH 1/1 mmc: failure of block read wait for long time
Quoted 38 lines · Sukumar
Adrian,
Quoted 36 lines
-----Original Message-----
From: Adrian Hunter [mailto:adrian.hunter at nokia.com]
Sent: Wednesday, September 29, 2010 12:03 AM
To: Ghorai, Sukumar
Cc: Chris Ball; linux-mmc at vger.kernel.org; linux-arm- kernel at lists.infradead.org; Russell King - ARM Linux
Subject: Re: [PATCH] mmc: failure of block read wait for long timeQuoted 27 lines · Sukumar
Chris and Adrian,
[..snip..]
Quoted 6 lines
Chris and Adrian,
[..snip..]
Quoted 2 lines
Quoted 2 lines
-----Original Message-----
[..snip..]
Quoted 17 lines
Quoted 7 lines
Quoted 7 lines
Subject: Re: [PATCH] mmc: failure of block read wait for long time
Quoted 2 lines · Sukumar
Would you please review and merge this patch [1] (attached too)?
[1] http://comments.gmane.org/gmane.linux.kernel.mmc/2714I've been following the thread. I believe Adrian has NACKed this
patch,
Quoted 5 lines
Quoted 2 lines
by saying "It is absolutely unacceptable to return I/O errors to the upper layers for segments that do not have errors."
[Ghorai]
I think Russell also mentioned his opinion. Would you please add youridea
Quoted 3 lines
too?
1. I would prefer Adrian to explain again what this statement means,
Quoted 8 lines
Quoted 5 lines
in
Quoted 1 line
Quoted 1 line
Quoted 1 line
the context - data read fail and how we make it success?
Because I/O requests are made up of segments and every segment can be a success or failure.
[Ghorai] don't you conflict your self for the comments you provide for following patch - [PATCH] MMC: Refine block layer waiting for card state
[Adrian].. then why wait for lots of errors before doing it.
That patch needs a lot more work. Please do not base your understanding on it.
Quoted 9 lines
Quoted 9 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
2. if data read fail for sector(x) why we have to try for
sector(x+1, ..x+n)?See answer to q. 1
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 7 lines
Quoted 6 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
3. how to inform reader function which sector having the valid data
out
Quoted 2 lines
Quoted 2 lines
of
Quoted 1 line
(1...n) sectors.
__blk_end_request()does that[Ghorai] not true. Please check the code again.
Every time you call __blk_end_request() you specify success or failure for the specified numbers of bytes starting from the last position.
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 4 lines
4. do we have any driver/code in Linux or any other os, which give
inter-
Quoted 16 lines
Quoted 4 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
leave data and return as success?
Here is the problem with that question. The *same* I/O request can have data for *different*sources.
[Ghorai] File system does not do that and can you test that once how data comes from difference soure? Also conflicting your-self for the input you gave for the patch and as -
[PATCH] MMC: Refine block layer waiting for card state
[Adrian].. then why wait for lots of errors before doing it.Quoted 7 lines
Quoted 4 lines
Quoted 3 lines
Quoted 2 lines
[Ghorai] please reply with your input on my/ Russell's suggestion?
[Ghorai] any input?
I have a question for you. What use cases do you want to address
- other than card removal?
Please answer this question.
Quoted 2 lines
[Ghorai]
1. can you reply to original input form Russell's on the same thread?
Russell did not make any suggestions. He pointed out that some drivers, but not all (and not omap_hsmmc), indicate how many bytes were transferred. However it is difficult for me to explain how this will or will not help if you won't give more information about your use cases.
For example, in the case of ECC errors, there are usually only a few blocks in error, so only a few of the retries timeout, so retrying is not slow. That is very different in the case the card has been removed, or has become unresponsive - in which case every retry fails and has to timeout.
I still plan to address the card removal issue, but I am very busy, so don't hold your breath.
Quoted 2 lines
2. can you check if you return the interleave data to FS how it can behave?
3. still you don't have any reference driver which provide the interleave data.
A single I/O request could have resulted from merging I/O requests from two *different* file systems on two *different* partitions. I provide as reference every single linux file system.
Quoted 20 lines
Quoted 18 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
I think it's possible to merge patches to improve the situation (such as the idea of noticing a card disappearing earlier), but your
initial
Quoted 10 lines
Quoted 9 lines
Quoted 9 lines
Quoted 9 lines
patch is not the patch to do that. You should continue to work with Adrian -- when he's happy that a patch does not break the semantics above, we can consider merging it.
Thanks,
--
Chris Ball<[email protected]> <http://printf.net/>
One Laptop Per Child
PATCH 1/1 mmc: failure of block read wait for long time
Quoted 56 lines
-----Original Message-----
From: Adrian Hunter [mailto:adrian.hunter at nokia.com]
Sent: Wednesday, September 29, 2010 1:35 AM
To: Ghorai, Sukumar
Cc: Chris Ball; linux-mmc at vger.kernel.org; linux-arm- kernel at lists.infradead.org; Russell King - ARM Linux
Subject: Re: [PATCH] mmc: failure of block read wait for long timeQuoted 30 lines · Sukumar
Adrian,
Quoted 28 lines
-----Original Message-----
From: Adrian Hunter [mailto:adrian.hunter at nokia.com]
Sent: Wednesday, September 29, 2010 12:03 AM
To: Ghorai, Sukumar
Cc: Chris Ball; linux-mmc at vger.kernel.org; linux-arm- kernel at lists.infradead.org; Russell King - ARM Linux
Subject: Re: [PATCH] mmc: failure of block read wait for long timeQuoted 19 lines · Sukumar
Chris and Adrian,
[..snip..]
Quoted 6 lines
Chris and Adrian,
[..snip..]
Quoted 2 lines
Quoted 2 lines
-----Original Message-----
[..snip..]
Quoted 9 lines
Quoted 7 lines
Quoted 7 lines
Subject: Re: [PATCH] mmc: failure of block read wait for long time
Quoted 2 lines · Sukumar
Would you please review and merge this patch [1] (attached too)?
[1] http://comments.gmane.org/gmane.linux.kernel.mmc/2714I've been following the thread. I believe Adrian has NACKed this
patch,
Quoted 1 line
Quoted 1 line
by saying "It is absolutely unacceptable to return I/O errors to
the
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 1 line
upper layers for segments that do not have errors."
[Ghorai]
I think Russell also mentioned his opinion. Would you please addyour
Quoted 9 lines
Quoted 9 lines
Quoted 4 lines
Quoted 4 lines
idea
Quoted 3 lines
too?
1. I would prefer Adrian to explain again what this statement means,
in
Quoted 1 line
Quoted 1 line
Quoted 1 line
the context - data read fail and how we make it success?
Because I/O requests are made up of segments and every segment can be a success or failure.
Quoted 7 lines
Quoted 1 line
[Ghorai] don't you conflict your self for the comments you provide for
following patch -
Quoted 2 lines
[PATCH] MMC: Refine block layer waiting for card state [Adrian].. then why wait for lots of errors before doing it.
That patch needs a lot more work. Please do not base your understanding on it.
[Ghorai] it's the similar problem when read fails and you also suggest how to break early.
Quoted 39 lines
Quoted 10 lines
Quoted 10 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
2. if data read fail for sector(x) why we have to try for
sector(x+1, ..x+n)?See answer to q. 1
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 7 lines
Quoted 6 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
3. how to inform reader function which sector having the valid data
out
Quoted 2 lines
Quoted 2 lines
of
Quoted 1 line
(1...n) sectors.
__blk_end_request()does that[Ghorai] not true. Please check the code again.
Every time you call
__blk_end_request()you specify success or failure for the specified numbers of bytes starting from the last position.Quoted 10 lines
Quoted 9 lines
Quoted 6 lines
Quoted 6 lines
Quoted 4 lines
4. do we have any driver/code in Linux or any other os, which give
inter-
Quoted 1 line
leave data and return as success?
Here is the problem with that question. The *same* I/O request can have data for *different*sources.
[Ghorai] File system does not do that and can you test that once how
data comes from difference soure?
Quoted 6 lines
Also conflicting your-self for the input you gave for the patch and as - [PATCH] MMC: Refine block layer waiting for card state [Adrian].. then why wait for lots of errors before doing it.
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 7 lines
Quoted 5 lines
Quoted 5 lines
Quoted 2 lines
Quoted 1 line
[Ghorai] please reply with your input on my/ Russell's suggestion?
[Ghorai] any input?
I have a question for you. What use cases do you want to address
- other than card removal?Please answer this question.
[Ghorai] say data error (including timeout), ECC error ..
Quoted 9 lines
Quoted 3 lines
[Ghorai]
1. can you reply to original input form Russell's on the same thread?Russell did not make any suggestions. He pointed out that some drivers, but not all (and not
omap_hsmmc), indicate how many bytes were transferred. However it is difficult for me to explain how this will or will not help if
you won't give more information about your use cases.
[Ghorai] any driver give the interleave data to apps? Can you test if you give the interleave data to FS how it behave? Even it can cause the system fault. And will spend day and night to find out the issue in which module - memory, host, card?
Quoted 11 lines
For example, in the case of ECC errors, there are usually only a few blocks
in error, so only a few of the retries timeout, so retrying is not slow. That is very different in the case the card has been removed, or has become
unresponsive - in which case every retry fails and has to timeout.I still plan to address the card removal issue, but I am very busy, so don't
hold your breath.
[Ghorai] so I will wait for your patch forever.
Quoted 3 lines
Quoted 2 lines
2. can you check if you return the interleave data to FS how it can
behave?
[Ghorai] any time?
Quoted 6 lines
Quoted 1 line
3. still you don't have any reference driver which provide the
interleave data.
A single I/O request could have resulted from merging I/O requests from two *different* file systems on two *different* partitions. I provide as reference every single linux file system.
[Ghorai] Filesystem never marge two IO in a single request, and if say cache did not support (sync mode)?
[Ghorai] thanks that you want to provide the solution in different way and in different patch. I will wait for your patch forever. In the mean time will take this fix locally for system integration.
And in the mean time I will simulate a case when IO failed in between (say any request to sector number 100 to 200) will return the Data timeout. And will check how system behaves.
Quoted 23 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
I think it's possible to merge patches to improve the situation
(such
Quoted 3 lines
Quoted 3 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
as the idea of noticing a card disappearing earlier), but your
initial
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
patch is not the patch to do that. You should continue to work
with
Quoted 11 lines
Quoted 9 lines
Quoted 9 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Adrian -- when he's happy that a patch does not break the semantics above, we can consider merging it.
Thanks,
--
Chris Ball<[email protected]> <http://printf.net/>
One Laptop Per Child
PATCH 1/1 mmc: failure of block read wait for long time
In reply to Sukumar GhoraiHi,
Quoted 16 lines · Sukumar Ghorai
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.
Signed-off-by Sukumar Ghorai <[email protected]>--- drivers/mmc/card/block.c | 1 - 1 files changed, 0 insertions(+), 1 deletions(-)drivers/mmc/card/block.c +0
-0
drivers/mmc/card/block.c static int mmc_blk_issue_rq(struct mmc_queue *mq, struct request *req)
Quoted 8 lines
drivers/mmc/card/block.c +0
-2@@ -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; } --
Sukumar/Adrian, any thoughts on a new version of this patch containing Adrian's retry-segments idea?
Signature · 2 lines
Chris Ball <[email protected]> <http://printf.net/> One Laptop Per Child
PATCH 1/1 mmc: failure of block read wait for long time
Quoted 36 lines
-----Original Message-----
From: Chris Ball [mailto:cjb at laptop.org]
Sent: Saturday, August 28, 2010 2:30 AM
To: Ghorai, Sukumar
Cc: linux-mmc at vger.kernel.org; linux-arm-kernel at lists.infradead.org; Adrian Hunter
Subject: Re: [PATCH] mmc: failure of block read wait for long timeHi,
Quoted 1 line · Sukumar Ghorai
multi-block read failure retries in single block read one by one. It
continues
Quoted 1 line
retry of subsequent blocks, even after failure. Application will not be
able
Quoted 2 lines
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
drivers/mmc/card/block.c static int mmc_blk_issue_rq(struct mmc_queue *mq,
Quoted 12 lines
waiting for long duration.
Signed-off-by Sukumar Ghorai <[email protected]>--- drivers/mmc/card/block.c | 1 - 1 files changed, 0 insertions(+), 1 deletions(-)drivers/mmc/card/block.c +0
-0@@ -419,7 +419,6 @@ static int mmc_blk_issue_rq(struct mmc_queue *mq,struct request *req)
drivers/mmc/card/block.c static int mmc_blk_issue_rq(struct mmc_queue *mq, · line 420
Quoted 2 lines
drivers/mmc/card/block.c +0
-0spin_lock_irq (&md->lock); ret = __blk_end_request(req, -EIO,brq.data.blksz);
Quoted 9 lines
drivers/mmc/card/block.c static int mmc_blk_issue_rq(struct mmc_queue *mq, struct request *req)
Quoted 6 lines
drivers/mmc/card/block.c +0
-2spin_unlock_irq (&md->lock); -continue ; } goto cmd_err; } --Sukumar/Adrian, any thoughts on a new version of this patch containing Adrian's retry-segments idea?
[Ghorai] I am checked and tested the suggestion from Adrian. But was thinking it's just reducing the retry by 1/4 time; mean -
1. Each segment is size of 4K. and multi-block-write failed move to next segment(4k);
2. i.e. original code it was retrying for each 512 bytes. Now it's retrying for 8*512 bytes. So retry reduce by 1/8 times.
So I will post the 2nd version as suggested by Adrian; but let's see others input.
Quoted 4 lines
--
Chris Ball <[email protected]> <http://printf.net/>
One Laptop Per Child