Re: [ANNOUNCE][PATCH 2.6] md: persistent (file-backed) bitmap and async writes

[Paul Clements <paul.clements@steeleye.com>]

Neil,

responses to your feedback on the kernel code:

Neil Brown wrote:
> On Tuesday June 8, paul.clements@steeleye.com wrote:

> This looks a lot better.  There are a few little issues that I noticed
> on a quick read through:
>      - last_block_device should return "sector_t", not "unsigned
>        long".  It would be worth checking for other placed that
>        sector_t might be needed.

Yes, there were a couple. I've fixed them now.

>      - bitmap_checkpage still isn't quite safe.  If "hijacked" gets
>        set while it is allocating a page (unlikely, but possible), it
>        will exit with both highjacked set and an allocated page, which
>        isn't right.

Yes, that's right. Fixed.

>      - The event comparison
> +		    sb->events_hi >= refsb->bitmap_events_hi &&
> +		    sb->events_lo >= refsb->bitmap_events_lo) {
>        in hot_add_disk is wrong. 

I think it's OK. Basically, I only record bitmap events while the array 
is in sync. As soon as the array goes out of sync, we continue to set 
bits in the bitmap, but never clear them. This means that the bitmap is 
valid for resyncing any disk that was part of the array at the time it 
was last in sync. Does that make sense, or am I missing a corner case?


> I'll try to make time to try the patch out in a week or so to get a
> better feel for it.

OK, that sounds great.

Thanks,
Paul