From 961902c0f8240175729274cd14198872f42072b7 Mon Sep 17 00:00:00 2001 From: NeilBrown Date: Fri, 23 Dec 2011 09:57:48 +1100 Subject: md/bitmap: It is OK to clear bits during recovery. commit d0a4bb492772ce5c4bdfba3744a99ed6f6fb238f introduced a regression which is annoying but fairly harmless. When writing to an array that is undergoing recovery (a spare in being integrated into the array), writing to the array will set bits in the bitmap, but they will not be cleared when the write completes. For bits covering areas that have not been recovered yet this is not a problem as the recovery will clear the bits. However bits set in already-recovered region will stay set and never be cleared. This doesn't risk data integrity. The only negatives are: - next time there is a crash, more resyncing than necessary will be done. - the bitmap doesn't look clean, which is confusing. While an array is recovering we don't want to update the 'events_cleared' setting in the bitmap but we do still want to clear bits that have very recently been set - providing they were written to the recovering device. So split those two needs - which previously both depended on 'success' and always clear the bit of the write went to all devices. Signed-off-by: NeilBrown --- drivers/md/bitmap.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) (limited to 'drivers/md/bitmap.c') diff --git a/drivers/md/bitmap.c b/drivers/md/bitmap.c index b6907118283..6d03774b176 100644 --- a/drivers/md/bitmap.c +++ b/drivers/md/bitmap.c @@ -1393,9 +1393,6 @@ void bitmap_endwrite(struct bitmap *bitmap, sector_t offset, unsigned long secto atomic_read(&bitmap->behind_writes), bitmap->mddev->bitmap_info.max_write_behind); } - if (bitmap->mddev->degraded) - /* Never clear bits or update events_cleared when degraded */ - success = 0; while (sectors) { sector_t blocks; @@ -1409,7 +1406,7 @@ void bitmap_endwrite(struct bitmap *bitmap, sector_t offset, unsigned long secto return; } - if (success && + if (success && !bitmap->mddev->degraded && bitmap->events_cleared < bitmap->mddev->events) { bitmap->events_cleared = bitmap->mddev->events; bitmap->need_sync = 1; -- cgit v1.2.3-70-g09d2 From 2e61ebbcc45438899235d7a39f17291cb24e746c Mon Sep 17 00:00:00 2001 From: NeilBrown Date: Fri, 23 Dec 2011 10:17:50 +1100 Subject: md/bitmap: daemon_work cleanup. We have a variable 'mddev' in this function, but repeatedly get the same value by dereferencing bitmap->mddev. There is room for simplification here... Signed-off-by: NeilBrown --- drivers/md/bitmap.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'drivers/md/bitmap.c') diff --git a/drivers/md/bitmap.c b/drivers/md/bitmap.c index 6d03774b176..048eec75147 100644 --- a/drivers/md/bitmap.c +++ b/drivers/md/bitmap.c @@ -1149,12 +1149,12 @@ void bitmap_daemon_work(struct mddev *mddev) return; } if (time_before(jiffies, bitmap->daemon_lastrun - + bitmap->mddev->bitmap_info.daemon_sleep)) + + mddev->bitmap_info.daemon_sleep)) goto done; bitmap->daemon_lastrun = jiffies; if (bitmap->allclean) { - bitmap->mddev->thread->timeout = MAX_SCHEDULE_TIMEOUT; + mddev->thread->timeout = MAX_SCHEDULE_TIMEOUT; goto done; } bitmap->allclean = 1; @@ -1206,7 +1206,7 @@ void bitmap_daemon_work(struct mddev *mddev) * sure that events_cleared is up-to-date. */ if (bitmap->need_sync && - bitmap->mddev->bitmap_info.external == 0) { + mddev->bitmap_info.external == 0) { bitmap_super_t *sb; bitmap->need_sync = 0; sb = kmap_atomic(bitmap->sb_page, KM_USER0); @@ -1270,8 +1270,8 @@ void bitmap_daemon_work(struct mddev *mddev) done: if (bitmap->allclean == 0) - bitmap->mddev->thread->timeout = - bitmap->mddev->bitmap_info.daemon_sleep; + mddev->thread->timeout = + mddev->bitmap_info.daemon_sleep; mutex_unlock(&mddev->bitmap_info.mutex); } -- cgit v1.2.3-70-g09d2 From 915c420ddfa3eb22a0dbdb7a8e0ecf020c31961f Mon Sep 17 00:00:00 2001 From: NeilBrown Date: Fri, 23 Dec 2011 10:17:51 +1100 Subject: md/bitmap: be more consistent when setting new bits in memory bitmap. For each active region corresponding to a bit in the bitmap with have a 14bit counter (and some flags). This counts number of active writes + bit in the on-disk bitmap + delay-needed. The "delay-needed" is because we always want a delay before clearing a bit. So the number here is normally number of active writes plus 2. If there have been no writes for a while, we drop to 1. If still no writes we clear the bit and drop to 0. So for consistency, when setting bit from the on-disk bitmap or by request from user-space it is best to set the counter to '2' to start with. In particular we might also set the NEEDED_MASK flag at this time, and in all other cases NEEDED_MASK is only set when the counter is 2 or more. Signed-off-by: NeilBrown --- drivers/md/bitmap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/md/bitmap.c') diff --git a/drivers/md/bitmap.c b/drivers/md/bitmap.c index 048eec75147..cdf36b1e9aa 100644 --- a/drivers/md/bitmap.c +++ b/drivers/md/bitmap.c @@ -1587,7 +1587,7 @@ static void bitmap_set_memory_bits(struct bitmap *bitmap, sector_t offset, int n } if (!*bmc) { struct page *page; - *bmc = 1 | (needed ? NEEDED_MASK : 0); + *bmc = 2 | (needed ? NEEDED_MASK : 0); bitmap_count_page(bitmap, offset, 1); page = filemap_get_page(bitmap, offset >> CHUNK_BLOCK_SHIFT(bitmap)); set_page_attr(bitmap, page, BITMAP_PAGE_PENDING); -- cgit v1.2.3-70-g09d2