[RFC][PATCH] ataraid_end_request hides errors (all? 2.4 kernels)

[Martins Krikis <mkrikis@yahoo.com>]

[-- Attachment #1: Type: text/plain, Size: 1592 bytes --]

I know that interest in 2.4 kernels and ataraid at this point
is probably minimal, and I myself don't use ataraid_end_request.

However, I would appreciate if somebody could tell me whether a
patch like the one attached is acceptable or whether the use of
the BH_PrivateStart flag (style issues aside) can introduce any
new problems. In particular, I've noticed that ext3 and xfs also
use this flag.

The bug that the patch attempts to solve is the following.
Ataraid_end_request() uses the success/failure of the last
one of component I/Os as the success/failure of the complete
I/O (to a RAID volume). Thus, even if all the first components
fail, as long as the last one succeeds, it will report the
complete I/O as a success, and the user will not even get
an indication of any errors. The code is used by all ataraid
subdrivers except iswraid, as far as I know.

If using the BH_PrivateStart is not appropriate in this module,
a different free bit from b_state can be chosen. Protection
against component-I/O failure can also be achieved by introducing
a new field in ataraid_bh_private. Ideally the subdrivers would
also clear the flag/field, although for a truly unused flag
in b_state this can be skipped, so a flag-based solution can
be both quicker and need no extra space, I think.

Anyway, I'm very interested to hear whether anybody cares
and whether it is OK to use BH_PrivateStart in block device
drivers.

Thanks,

  Martins Krikis



		
__________________________________
Do you Yahoo!?
Take Yahoo! Mail with you! Get it on your mobile phone.
http://mobile.yahoo.com/maildemo 

[-- Attachment #2: ataraid.patch --]
[-- Type: application/octet-stream, Size: 949 bytes --]

--- linux/drivers/ide/raid/ataraid.c.orig	2004-07-13 23:45:21.000000000 -0400
+++ linux/drivers/ide/raid/ataraid.c	2004-07-13 23:45:32.000000000 -0400
@@ -153,7 +153,14 @@ void ataraid_end_request(struct buffer_h
 	if (private==NULL)
 		BUG();
 
+	if (!uptodate) /* record failures of components of the original I/O */
+		set_bit(BH_PrivateStart, &private->parent->b_state);
+	
 	if (atomic_dec_and_test(&private->count)) {
+		if (test_bit(BH_PrivateStart, &private->parent->b_state)) {
+			uptodate = 0; /* fail the completed original I/O */
+			clear_bit(BH_PrivateStart, &private->parent->b_state);
+		}
 		private->parent->b_end_io(private->parent,uptodate);
 		private->parent = NULL;
 		kfree(private);
@@ -194,6 +201,8 @@ static void ataraid_split_request(reques
 
 	bh2->b_data +=  bh->b_size/2;
 
+	clear_bit(BH_PrivateStart, &bh->b_state); /* this bit tracks success */
+
 	generic_make_request(rw,bh1);
 	generic_make_request(rw,bh2);
 }