Re: [PATCH 001 of 3] md: Don't clear bits in bitmap when writing to one device fails during recovery.
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 1<<20, 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
Re: [PATCH 001 of 3] md: Don't clear bits in bitmap when writing to one device fails during recovery.
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
[PATCH 001 of 3] md: Don't clear bits in bitmap when writing to one device fails during recovery.
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