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.
collection. I would really appreciate it if any further changes could be sent as incremental patches, as reading through a large patch like that takes a fair bit of effort.
Yes, I certainly understand that. I appreciate all the effort you've put into reviewing and giving suggestions. Further patches will be on top of the current bitmap patch.
I will send out an incremental patch to fix the issues above, probably in a couple of days.
Thanks, Paul
- To unsubscribe from this list: send the line "unsubscribe linux-raid" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
