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

[Neil Brown]

On Wednesday March 29, akpm@xxxxxxxx wrote:
> NeilBrown <neilb@xxxxxxx> 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-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/