[PATCH 001 of 3] md: Don't clear bits in bitmap when writing to one device fails during recovery.

2006-03-29 Thread NeilBrown

Currently a device failure during recovery leaves bits set in the
bitmap.  This normally isn't a problem as the offending device will be
rejected because of errors.  However if device re-adding is being used
with non-persistent bitmaps, this can be a problem.

Signed-off-by: Neil Brown [EMAIL PROTECTED]

### Diffstat output
 ./drivers/md/raid1.c |   13 -
 1 file changed, 12 insertions(+), 1 deletion(-)

diff ./drivers/md/raid1.c~current~ ./drivers/md/raid1.c
--- ./drivers/md/raid1.c~current~   2006-03-30 16:48:29.0 +1100
+++ ./drivers/md/raid1.c2006-03-30 16:48:40.0 +1100
@@ -1135,8 +1135,19 @@ static int end_sync_write(struct bio *bi
mirror = i;
break;
}
-   if (!uptodate)
+   if (!uptodate) {
+   int sync_blocks = 0;
+   sector_t s = r1_bio-sector;
+   long sectors_to_go = r1_bio-sectors;
+   /* make sure these bits doesn't get cleared. */
+   do {
+   bitmap_end_sync(mddev-bitmap, r1_bio-sector,
+   sync_blocks, 1);
+   s += sync_blocks;
+   sectors_to_go -= sync_blocks;
+   } while (sectors_to_go  0);
md_error(mddev, conf-mirrors[mirror].rdev);
+   }
 
update_head_pos(mirror, r1_bio);
 
-
To unsubscribe from this list: send the line unsubscribe linux-raid in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 001 of 3] md: Don't clear bits in bitmap when writing to one device fails during recovery.

2006-03-29 Thread Andrew Morton
NeilBrown [EMAIL PROTECTED] wrote:

 + if (!uptodate) {
  +int sync_blocks = 0;
  +sector_t s = r1_bio-sector;
  +long sectors_to_go = r1_bio-sectors;
  +/* make sure these bits doesn't get cleared. */
  +do {
  +bitmap_end_sync(mddev-bitmap, r1_bio-sector,
  +sync_blocks, 1);
  +s += sync_blocks;
  +sectors_to_go -= sync_blocks;
  +} while (sectors_to_go  0);
   md_error(mddev, conf-mirrors[mirror].rdev);
  +}

Can mddev-bitmap be NULL?

If so, will the above loop do the right thing when this:

void bitmap_end_sync(struct bitmap *bitmap, sector_t offset, int *blocks, int 
aborted)
{
bitmap_counter_t *bmc;
unsigned long flags;
/*
if (offset == 0) printk(bitmap_end_sync 0 (%d)\n, aborted);
*/  if (bitmap == NULL) {
*blocks = 1024;
return;
}

triggers?
-
To unsubscribe from this list: send the line unsubscribe linux-raid in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 001 of 3] md: Don't clear bits in bitmap when writing to one device fails during recovery.

2006-03-29 Thread Neil Brown
On Wednesday March 29, [EMAIL PROTECTED] wrote:
 NeilBrown [EMAIL PROTECTED] wrote:
 
  +   if (!uptodate) {
   +  int sync_blocks = 0;
   +  sector_t s = r1_bio-sector;
   +  long sectors_to_go = r1_bio-sectors;
   +  /* make sure these bits doesn't get cleared. */
   +  do {
   +  bitmap_end_sync(mddev-bitmap, r1_bio-sector,
   +  sync_blocks, 1);
   +  s += sync_blocks;
   +  sectors_to_go -= sync_blocks;
   +  } while (sectors_to_go  0);
  md_error(mddev, conf-mirrors[mirror].rdev);
   +  }
 
 Can mddev-bitmap be NULL?

Yes, normally it is.

 
 If so, will the above loop do the right thing when this:
 
 void bitmap_end_sync(struct bitmap *bitmap, sector_t offset, int *blocks, int 
 aborted)
 {
   bitmap_counter_t *bmc;
   unsigned long flags;
 /*
   if (offset == 0) printk(bitmap_end_sync 0 (%d)\n, aborted);
 */if (bitmap == NULL) {
   *blocks = 1024;
   return;
   }
 
 triggers?

Yes.  sync_blocks will be 1024 (a nice big number) and the loop will
exit quite quickly having done nothing (which is what it needs to do
in that case).
Ofcourse, if someone submits a bio for multiple thousands of sectors
it will loop needlessly a few times, but do we ever generate bios that
are even close to a megabyte?
If so, that 1024 can be safely increased to 120, and possibly higher
but I would need to check.

Thanks for asking
NeilBrown
-
To unsubscribe from this list: send the line unsubscribe linux-raid in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html