lkml.org 
[lkml]   [2004]   [Feb]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: dm core patches
James Bottomley wrote:
>>The mechanism is in place, but the SCSI stack still needs a few changes
>>to pass down the correct errors. The easiest would be to pass down
>>pseudo-sense keys (I'd rather just call them something else as not to
>>confuse things, io error hints or something) to
>>end_that_request_first(), changing uptodate from a bool to a hint.
>
>
> Yes, I'm ready to do this in SCSI. I think the uptodate field should
> include at least two (and possibly three) failure type indications:
>
> - fatal: error cannot be retried
> - retryable: error may be retried
>
> and possibly
>
> - informational: This is dangerous, since it's giving information about
> a transaction that actually succeeded (i.e. we'd need to fix drivers to
> recognise it as being uptodate but with info, like sector remapped)
>
> Then, we also have a error origin indication:
>
> - device: The device is actually reporting the problem
> - transport: the error is a transport error
> - driver: the error comes from the device driver.
>
> So dm would know that fatal transport or driver errors could be
> repathed, but fatal device errors probably couldn't.
>

I apologize for not starting a new thread, but I just wanted some
feedback as to whether or not the attached patch is headed in the right
direction or even acceptable. block-err.patch adds new errornos to
include/linux/errno.h (it does not touch the asm values), so useful IO
error info can passed from callers of end_that_request_first to
bio_endio and eventually to the DM/MD endio functions.

I have an alternative patch that defines BLK_ERR_xxx values instead of
touching errno.h, but becuase the error values get passed through the
request code, bio code and DM/MD code the callers of bio_endio that are
already using -Exxx values could present a problem. It would be nice to
change them to the BLK_ERR_xxx, so the bio layer could have a single
error value namespace. It's a more invasive change as there are several
callers passing at least -EIO, -EWOULDBLOCK and -EPERM, so I am not sure
if that is going to be OK since we are already in 2.6.3?

Thanks,

Mike Christie
mikenc@us.ibm.com
diff -aurp linux-2.6.3-orig/drivers/block/ll_rw_blk.c linux-2.6.3-ec/drivers/block/ll_rw_blk.c
--- linux-2.6.3-orig/drivers/block/ll_rw_blk.c 2004-02-17 19:57:16.000000000 -0800
+++ linux-2.6.3-ec/drivers/block/ll_rw_blk.c 2004-02-18 12:33:50.000000000 -0800
@@ -2456,8 +2456,13 @@ static int __end_that_request_first(stru
if (!blk_pc_request(req))
req->errors = 0;

- if (!uptodate) {
- error = -EIO;
+ /*
+ * Most drivers set uptodate to 0 for error and 1 for success.
+ * MD/DM ready drivers will set 1 for success and a -Exxx
+ * value to indicate a specific error.
+ */
+ if (uptodate < 1) {
+ error = (uptodate == 0 ? -EIO : uptodate);
if (blk_fs_request(req) && !(req->flags & REQ_QUIET))
printk("end_request: I/O error, dev %s, sector %llu\n",
req->rq_disk ? req->rq_disk->disk_name : "?",
@@ -2540,7 +2545,7 @@ static int __end_that_request_first(stru
/**
* end_that_request_first - end I/O on a request
* @req: the request being processed
- * @uptodate: 0 for I/O error
+ * @@uptodate: <= 0 to indicate an I/O error.
* @nr_sectors: number of sectors to end I/O on
*
* Description:
@@ -2561,7 +2566,7 @@ EXPORT_SYMBOL(end_that_request_first);
/**
* end_that_request_chunk - end I/O on a request
* @req: the request being processed
- * @uptodate: 0 for I/O error
+ * @uptodate: <= 0 to indicate an I/O error.
* @nr_bytes: number of bytes to complete
*
* Description:
diff -aurp linux-2.6.3-orig/include/linux/errno.h linux-2.6.3-ec/include/linux/errno.h
--- linux-2.6.3-orig/include/linux/errno.h 2004-02-17 19:59:12.000000000 -0800
+++ linux-2.6.3-ec/include/linux/errno.h 2004-02-18 12:45:42.000000000 -0800
@@ -23,6 +23,14 @@
#define EJUKEBOX 528 /* Request initiated, but will not complete before timeout */
#define EIOCBQUEUED 529 /* iocb queued, will get completion event */

+/* Block device error codes */
+#define EFATALDEV 540 /* Fatal device error */
+#define EFATALTRNSPT 541 /* Fatal transport error */
+#define EFATALDRV 542 /* Fatal driver error */
+#define ERETRYDEV 543 /* Device error occured, I/O may be retried */
+#define ERETRYTRNSPT 544 /* Transport error occured, I/O may be retried */
+#define ERETRYDRV 545 /* Driver error occured, I/O may be retried */
+
#endif

#endif
\
 
 \ /
  Last update: 2005-03-22 14:01    [from the cache]
©2003-2014 Jasper Spaans. Advertise on this site