Neil,

patch to fix the issues mentioned below has been tested and is attached. 
Should apply on top of 2.6.9-rc2 + md_bitmap.

--
Paul


Paul Clements wrote:
> Neil Brown wrote:
> 
>>> Paul Clements wrote:
> 
> 
>>> itself. Check out the new patch here:
>>>
>>> http://parisc-linux.org/~jejb/md_bitmap/md_bitmap_2_37_2_6_9_rc2.diff
>>>
> 
>> Further comments.
>>
>> bitmap_events
>>   1/  You have inserted bitmap_event_hi/lo *before* recovery_cp, thus
>>       moving recovery_cp, and thus breaking backwards comparability.
> 
> 
> Yes. I guess when recovery_cp came along I failed to notice that...
> 
>>   2/ The test in hot_add_disk:
>> +        if (refsb && sb && uuid_equal(sb, refsb) &&
>> +            sb->events_hi >= refsb->bitmap_events_hi &&
>> +            sb->events_lo >= refsb->bitmap_events_lo) {
>> +            bitmap_invalidate = 0;
>>    is wrong.  The events count must be compared as a 64bit
>>   number. e.g. it is only meaningful to compare events_lo if both
>>   events_hi are equal.
> 
> 
> Yes, that is broken.
> 
>> pending_bio_list
>>   1/ Do you really need a separate pending_bio_lock, or would
>>      the current device_lock be adequate to the task.
> 
> 
> Probably so...especially with the following change...
> 
>>   2/  I think there can be a race with new requests being added to
>>      this list while bitmap_unplug is running in unplug_slaves.
>>      I think you should "bio_get_list" before calling bitmap_unplug,
>>      So that you only then submit requests that were made definitely
>>      *before* the call the bitmap_unplug.  This would have the added
>>      advantage that you don't need to keep claiming and dropping
>>      pending_bio_lock. 
> 
> 
> Yes, that would make sense.