Introduction

PATCH In review NeilBrown 3 participants 16 messages Mar 23, 2005 MD RAID

Share & tools

NeilBrown

author

2005-03-23 05:47 UTC #0 lore raw

PATCH 1/1 Introduction

Here are 12 patches for the bitmap write-intent logging in md in 2.6.12-rc1-mm1 With this, it is getting quite close to being really usable (though there are a couple of issues that I haven't resolved yet.

Andrew: Are you happy to keep collecting these as a list of patches (bugs followed by bug-fixes :-), or would it be easier if I merged all the bug fixes into earlier patches and just resent a small number of "add-functionality" patches??

NeilBrown

[PATCH md 1 of 12] Check return value of write_page, rather than ignore it [PATCH md 2 of 12] Enable the bitmap write-back daemon and wait for it. [PATCH md 3 of 12] Improve debug-printing of bitmap superblock.
[PATCH md 4 of 12] Minor code rearrangement in bitmap_init_from_disk [PATCH md 5 of 12] Print correct pid for newly created bitmap-writeback-daemon. [PATCH md 6 of 12] Call bitmap_daemon_work regularly
[PATCH md 7 of 12] Don't skip bitmap pages due to lack of bit that we just cleared. [PATCH md 8 of 12] A couple of tidyups relating to the bitmap file. [PATCH md 9 of 12] Make sure md bitmap is cleared on a clean start. [PATCH md 10 of 12] Fix bug when raid1 attempts a partial reconstruct. [PATCH md 11 of 12] Allow md to update multiple superblocks in parallel. [PATCH md 12 of 12] Allow md intent bitmap to be stored near the superblock.

NeilBrown

author

2005-03-23 05:47 UTC #1 lore raw

PATCH 1/1 Check return value of write_page, rather than ignore it

Signed-off-by Neil Brown <[email protected]>

### Diffstat output

 ./drivers/md/bitmap.c |   13 +++++++------
 1 files changed, 7 insertions(+), 6 deletions(-)

drivers/md/bitmap.c renamed from drivers/md/bitmap.c~current~ +7 -6

diff ./drivers/md/bitmap.c~current~ ./drivers/md/bitmap.c
@@ -350,8 +350,7 @@ int bitmap_update_sb(struct bitmap *bitm
 	if (!bitmap->mddev->degraded)
 		sb->events_cleared = cpu_to_le64(bitmap->mddev->events);
 	kunmap(bitmap->sb_page);
-	write_page(bitmap->sb_page, 0);
-	return 0;
+	return write_page(bitmap->sb_page, 0);
 }
 
 /* print out the bitmap file superblock */

@@ -735,7 +734,8 @@ int bitmap_unplug(struct bitmap *bitmap)
 		spin_unlock_irqrestore(&bitmap->lock, flags);
 
 		if (attr & (BITMAP_PAGE_DIRTY | BITMAP_PAGE_NEEDWRITE))
-			write_page(page, 0);
+			if (write_page(page, 0))
+				return 1;
 	}
 	if (wait) { /* if any writes were performed, we need to wait on them */
 		spin_lock_irq(&bitmap->write_lock);

@@ -961,7 +961,7 @@ int bitmap_daemon_work(struct bitmap *bi
 				if (get_page_attr(bitmap, lastpage) & BITMAP_PAGE_NEEDWRITE) {
 					clear_page_attr(bitmap, lastpage, BITMAP_PAGE_NEEDWRITE);
 					spin_unlock_irqrestore(&bitmap->lock, flags);
-					write_page(lastpage, 0);
+					err = write_page(lastpage, 0);
 				} else {
 					set_page_attr(bitmap, lastpage, BITMAP_PAGE_NEEDWRITE);
 					spin_unlock_irqrestore(&bitmap->lock, flags);

@@ -1009,7 +1009,7 @@ int bitmap_daemon_work(struct bitmap *bi
 		if (get_page_attr(bitmap, lastpage) &BITMAP_PAGE_NEEDWRITE) {
 			clear_page_attr(bitmap, lastpage, BITMAP_PAGE_NEEDWRITE);
 			spin_unlock_irqrestore(&bitmap->lock, flags);
-			write_page(lastpage, 0);
+			err = write_page(lastpage, 0);
 		} else {
 			set_page_attr(bitmap, lastpage, BITMAP_PAGE_NEEDWRITE);
 			spin_unlock_irqrestore(&bitmap->lock, flags);

@@ -1384,7 +1384,8 @@ int bitmap_setallbits(struct bitmap *bit
 		spin_unlock_irqrestore(&bitmap->lock, flags);
 		memset(kmap(page), 0xff, PAGE_SIZE);
 		kunmap(page);
-		write_page(page, 0);
+		if (write_page(page, 0))
+			return 1;
 	}
 
 	return 0;

NeilBrown

author

2005-03-23 05:47 UTC #2 lore raw

PATCH 1/1 Enable the bitmap write-back daemon and wait for it.

In reply to NeilBrown

Currently we don't wait for updates to the bitmap to be flushed to disk properly. The infrastructure all there, but it isn't being used....

A separate kernel thread (bitmap_writeback_daemon) is needed to wait for each page as we cannot get callbacks when a page write completes.

Signed-off-by Neil Brown <[email protected]>

### Diffstat output

 ./drivers/md/bitmap.c         |  119 ++++++++++++++++++------------------------
 ./include/linux/raid/bitmap.h |   13 ----
 2 files changed, 55 insertions(+), 77 deletions(-)

drivers/md/bitmap.c renamed from drivers/md/bitmap.c~current~ +53 ~~-66~~

diff ./drivers/md/bitmap.c~current~ ./drivers/md/bitmap.c
@@ -261,30 +261,33 @@ char *file_path(struct file *file, char 
 /*
  * write out a page
  */
-static int write_page(struct page *page, int wait)
+static int write_page(struct bitmap *bitmap, struct page *page, int wait)
 {
 	int ret = -ENOMEM;
 
 	lock_page(page);
 
-	if (page->mapping == NULL)
-		goto unlock_out;
-	else if (i_size_read(page->mapping->host) < page->index << PAGE_SHIFT) {
-		ret = -ENOENT;
-		goto unlock_out;
-	}
-
 	ret = page->mapping->a_ops->prepare_write(NULL, page, 0, PAGE_SIZE);
 	if (!ret)
 		ret = page->mapping->a_ops->commit_write(NULL, page, 0,
 			PAGE_SIZE);
 	if (ret) {
-unlock_out:
 		unlock_page(page);
 		return ret;
 	}
 
 	set_page_dirty(page); /* force it to be written out */
+
+	if (!wait) {
+		/* add to list to be waited for by daemon */
+		struct page_list *item = mempool_alloc(bitmap->write_pool, GFP_NOIO);
+		item->page = page;
+		page_cache_get(page);
+		spin_lock(&bitmap->write_lock);
+		list_add(&item->list, &bitmap->complete_pages);
+		spin_unlock(&bitmap->write_lock);
+		md_wakeup_thread(bitmap->writeback_daemon);
+	}
 	return write_one_page(page, wait);
 }

@@ -343,14 +346,13 @@ int bitmap_update_sb(struct bitmap *bitm
 		spin_unlock_irqrestore(&bitmap->lock, flags);
 		return 0;
 	}
-	page_cache_get(bitmap->sb_page);
 	spin_unlock_irqrestore(&bitmap->lock, flags);
 	sb = (bitmap_super_t *)kmap(bitmap->sb_page);
 	sb->events = cpu_to_le64(bitmap->mddev->events);
 	if (!bitmap->mddev->degraded)
 		sb->events_cleared = cpu_to_le64(bitmap->mddev->events);
 	kunmap(bitmap->sb_page);
-	return write_page(bitmap->sb_page, 0);
+	return write_page(bitmap, bitmap->sb_page, 0);
 }
 
 /* print out the bitmap file superblock */

@@ -556,10 +558,10 @@ static void bitmap_file_unmap(struct bit
 static void bitmap_stop_daemons(struct bitmap *bitmap);
 
 /* dequeue the next item in a page list -- don't call from irq context */
-static struct page_list *dequeue_page(struct bitmap *bitmap,
-					struct list_head *head)
+static struct page_list *dequeue_page(struct bitmap *bitmap)
 {
 	struct page_list *item = NULL;
+	struct list_head *head = &bitmap->complete_pages;
 
 	spin_lock(&bitmap->write_lock);
 	if (list_empty(head))

@@ -573,23 +575,15 @@ out:
 
 static void drain_write_queues(struct bitmap *bitmap)
 {
-	struct list_head *queues[] = { 	&bitmap->complete_pages, NULL };
-	struct list_head *head;
 	struct page_list *item;
-	int i;
 
-	for (i = 0; queues[i]; i++) {
-		head = queues[i];
-		while ((item = dequeue_page(bitmap, head))) {
-			page_cache_release(item->page);
-			mempool_free(item, bitmap->write_pool);
-		}
+	while ((item = dequeue_page(bitmap))) {
+		/* don't bother to wait */
+		page_cache_release(item->page);
+		mempool_free(item, bitmap->write_pool);
 	}
 
-	spin_lock(&bitmap->write_lock);
-	bitmap->writes_pending = 0; /* make sure waiters continue */
 	wake_up(&bitmap->write_wait);
-	spin_unlock(&bitmap->write_lock);
 }
 
 static void bitmap_file_put(struct bitmap *bitmap)

@@ -734,13 +728,13 @@ int bitmap_unplug(struct bitmap *bitmap)
 		spin_unlock_irqrestore(&bitmap->lock, flags);
 
 		if (attr & (BITMAP_PAGE_DIRTY | BITMAP_PAGE_NEEDWRITE))
-			if (write_page(page, 0))
+			if (write_page(bitmap, page, 0))
 				return 1;
 	}
 	if (wait) { /* if any writes were performed, we need to wait on them */
 		spin_lock_irq(&bitmap->write_lock);
 		wait_event_lock_irq(bitmap->write_wait,
-			bitmap->writes_pending == 0, bitmap->write_lock,
+			list_empty(&bitmap->complete_pages), bitmap->write_lock,
 			wake_up_process(bitmap->writeback_daemon->tsk));
 		spin_unlock_irq(&bitmap->write_lock);
 	}

@@ -847,7 +841,7 @@ static int bitmap_init_from_disk(struct 
 				 */
 				memset(page_address(page) + offset, 0xff,
 					PAGE_SIZE - offset);
-				ret = write_page(page, 1);
+				ret = write_page(bitmap, page, 1);
 				if (ret) {
 					kunmap(page);
 					/* release, page not in filemap yet */

@@ -945,7 +939,7 @@ int bitmap_daemon_work(struct bitmap *bi
 			}
 			spin_unlock_irqrestore(&bitmap->lock, flags);
 			if (attr & BITMAP_PAGE_NEEDWRITE) {
-				if (write_page(page, 0))
+				if (write_page(bitmap, page, 0))
 					bitmap_file_kick(bitmap);
 				page_cache_release(page);
 			}

@@ -961,7 +955,7 @@ int bitmap_daemon_work(struct bitmap *bi
 				if (get_page_attr(bitmap, lastpage) & BITMAP_PAGE_NEEDWRITE) {
 					clear_page_attr(bitmap, lastpage, BITMAP_PAGE_NEEDWRITE);
 					spin_unlock_irqrestore(&bitmap->lock, flags);
-					err = write_page(lastpage, 0);
+					err = write_page(bitmap, lastpage, 0);
 				} else {
 					set_page_attr(bitmap, lastpage, BITMAP_PAGE_NEEDWRITE);
 					spin_unlock_irqrestore(&bitmap->lock, flags);

@@ -1009,7 +1003,7 @@ int bitmap_daemon_work(struct bitmap *bi
 		if (get_page_attr(bitmap, lastpage) &BITMAP_PAGE_NEEDWRITE) {
 			clear_page_attr(bitmap, lastpage, BITMAP_PAGE_NEEDWRITE);
 			spin_unlock_irqrestore(&bitmap->lock, flags);
-			err = write_page(lastpage, 0);
+			err = write_page(bitmap, lastpage, 0);
 		} else {
 			set_page_attr(bitmap, lastpage, BITMAP_PAGE_NEEDWRITE);
 			spin_unlock_irqrestore(&bitmap->lock, flags);

@@ -1045,46 +1039,40 @@ static void bitmap_writeback_daemon(mdde
 	struct page_list *item;
 	int err = 0;
 
-	while (1) {
-		PRINTK("%s: bitmap writeback daemon waiting...\n", bmname(bitmap));
-		down_interruptible(&bitmap->write_done);
-		if (signal_pending(current)) {
-			printk(KERN_INFO
-			    "%s: bitmap writeback daemon got signal, exiting...\n",
-			    bmname(bitmap));
-			break;
-		}
+	if (signal_pending(current)) {
+		printk(KERN_INFO
+		       "%s: bitmap writeback daemon got signal, exiting...\n",
+		       bmname(bitmap));
+		err = -EINTR;
+		goto out;
+	}
 
-		PRINTK("%s: bitmap writeback daemon woke up...\n", bmname(bitmap));
-		/* wait on bitmap page writebacks */
-		while ((item = dequeue_page(bitmap, &bitmap->complete_pages))) {
-			page = item->page;
-			mempool_free(item, bitmap->write_pool);
-			PRINTK("wait on page writeback: %p %lu\n", page, bitmap->writes_pending);
-			wait_on_page_writeback(page);
-			PRINTK("finished page writeback: %p %lu\n", page, bitmap->writes_pending);
-			spin_lock(&bitmap->write_lock);
-			if (!--bitmap->writes_pending)
-				wake_up(&bitmap->write_wait);
-			spin_unlock(&bitmap->write_lock);
-			err = PageError(page);
-			page_cache_release(page);
-			if (err) {
-				printk(KERN_WARNING "%s: bitmap file writeback "
-					"failed (page %lu): %d\n",
-					bmname(bitmap), page->index, err);
-				bitmap_file_kick(bitmap);
-				goto out;
-			}
+	PRINTK("%s: bitmap writeback daemon woke up...\n", bmname(bitmap));
+	/* wait on bitmap page writebacks */
+	while ((item = dequeue_page(bitmap))) {
+		page = item->page;
+		mempool_free(item, bitmap->write_pool);
+		PRINTK("wait on page writeback: %p\n", page);
+		wait_on_page_writeback(page);
+		PRINTK("finished page writeback: %p\n", page);
+
+		err = PageError(page);
+		page_cache_release(page);
+		if (err) {
+			printk(KERN_WARNING "%s: bitmap file writeback "
+			       "failed (page %lu): %d\n",
+			       bmname(bitmap), page->index, err);
+			bitmap_file_kick(bitmap);
+			goto out;
 		}
 	}
-out:
+ out:
+	wake_up(&bitmap->write_wait);
 	if (err) {
 		printk(KERN_INFO "%s: bitmap writeback daemon exiting (%d)\n",
-			bmname(bitmap), err);
+		       bmname(bitmap), err);
 		daemon_exit(bitmap, &bitmap->writeback_daemon);
 	}
-	return;
 }
 
 static int bitmap_start_daemon(struct bitmap *bitmap, mdk_thread_t **ptr,

@@ -1384,7 +1372,7 @@ int bitmap_setallbits(struct bitmap *bit
 		spin_unlock_irqrestore(&bitmap->lock, flags);
 		memset(kmap(page), 0xff, PAGE_SIZE);
 		kunmap(page);
-		if (write_page(page, 0))
+		if (write_page(bitmap, page, 0))
 			return 1;
 	}

@@ -1452,7 +1440,6 @@ int bitmap_create(mddev_t *mddev)
 	mddev->bitmap = bitmap;
 
 	spin_lock_init(&bitmap->write_lock);
-	init_MUTEX_LOCKED(&bitmap->write_done);
 	INIT_LIST_HEAD(&bitmap->complete_pages);
 	init_waitqueue_head(&bitmap->write_wait);
 	bitmap->write_pool = mempool_create(WRITE_POOL_SIZE, write_pool_alloc,

include/linux/raid/bitmap.h renamed from include/linux/raid/bitmap.h~current~ +2 ~~-11~~

diff ./include/linux/raid/bitmap.h~current~ ./include/linux/raid/bitmap.h
@@ -233,21 +233,12 @@ struct bitmap {
 	unsigned long daemon_sleep; /* how many seconds between updates? */
 
 	/*
-	 * bitmap write daemon - this daemon performs writes to the bitmap file
-	 * this thread is only needed because of a limitation in ext3 (jbd)
-	 * that does not allow a task to have two journal transactions ongoing
-	 * simultaneously (even if the transactions are for two different
-	 * filesystems) -- in the case of bitmap, that would be the filesystem
-	 * that the bitmap file resides on and the filesystem that is mounted
-	 * on the md device -- see current->journal_info in jbd/transaction.c
+	 * bitmap_writeback_daemon waits for file-pages that have been written,
+	 * as there is no way to get a call-back when a page write completes.
 	 */
 	mdk_thread_t *writeback_daemon;
 	spinlock_t write_lock;
-	struct semaphore write_ready;
-	struct semaphore write_done;
-	unsigned long writes_pending;
 	wait_queue_head_t write_wait;
-	struct list_head write_pages;
 	struct list_head complete_pages;
 	mempool_t *write_pool;
 };

NeilBrown

author

2005-03-23 05:47 UTC #3 lore raw

PATCH 1/1 Improve debug-printing of bitmap superblock.

In reply to NeilBrown

- report sync_size properly - need /2 to convert sectors to KB
- move everything over 2 spaces to allow proper spelling of "events cleared".

Signed-off-by Neil Brown <[email protected]>

### Diffstat output

 ./drivers/md/bitmap.c |   20 ++++++++++----------
 1 files changed, 10 insertions(+), 10 deletions(-)

drivers/md/bitmap.c renamed from drivers/md/bitmap.c~current~ +10 ~~-10~~

diff ./drivers/md/bitmap.c~current~ ./drivers/md/bitmap.c
@@ -364,22 +364,22 @@ void bitmap_print_sb(struct bitmap *bitm
 		return;
 	sb = (bitmap_super_t *)kmap(bitmap->sb_page);
 	printk(KERN_DEBUG "%s: bitmap file superblock:\n", bmname(bitmap));
-	printk(KERN_DEBUG "       magic: %08x\n", le32_to_cpu(sb->magic));
-	printk(KERN_DEBUG "     version: %d\n", le32_to_cpu(sb->version));
-	printk(KERN_DEBUG "        uuid: %08x.%08x.%08x.%08x\n",
+	printk(KERN_DEBUG "         magic: %08x\n", le32_to_cpu(sb->magic));
+	printk(KERN_DEBUG "       version: %d\n", le32_to_cpu(sb->version));
+	printk(KERN_DEBUG "          uuid: %08x.%08x.%08x.%08x\n",
 					*(__u32 *)(sb->uuid+0),
 					*(__u32 *)(sb->uuid+4),
 					*(__u32 *)(sb->uuid+8),
 					*(__u32 *)(sb->uuid+12));
-	printk(KERN_DEBUG "      events: %llu\n",
+	printk(KERN_DEBUG "        events: %llu\n",
 			(unsigned long long) le64_to_cpu(sb->events));
-	printk(KERN_DEBUG "events_clred: %llu\n",
+	printk(KERN_DEBUG "events cleared: %llu\n",
 			(unsigned long long) le64_to_cpu(sb->events_cleared));
-	printk(KERN_DEBUG "       state: %08x\n", le32_to_cpu(sb->state));
-	printk(KERN_DEBUG "   chunksize: %d B\n", le32_to_cpu(sb->chunksize));
-	printk(KERN_DEBUG "daemon sleep: %ds\n", le32_to_cpu(sb->daemon_sleep));
-	printk(KERN_DEBUG "   sync size: %llu KB\n",
-			(unsigned long long)le64_to_cpu(sb->sync_size));
+	printk(KERN_DEBUG "         state: %08x\n", le32_to_cpu(sb->state));
+	printk(KERN_DEBUG "     chunksize: %d B\n", le32_to_cpu(sb->chunksize));
+	printk(KERN_DEBUG "  daemon sleep: %ds\n", le32_to_cpu(sb->daemon_sleep));
+	printk(KERN_DEBUG "     sync size: %llu KB\n",
+			(unsigned long long)le64_to_cpu(sb->sync_size)/2);
 	kunmap(bitmap->sb_page);
 }

NeilBrown

author

2005-03-23 05:47 UTC #4 lore raw

PATCH 1/1 Minor code rearrangement in bitmap_init_from_disk

In reply to NeilBrown

Signed-off-by Neil Brown <[email protected]>

### Diffstat output

 ./drivers/md/bitmap.c |   16 ++++++++--------
 1 files changed, 8 insertions(+), 8 deletions(-)

drivers/md/bitmap.c renamed from drivers/md/bitmap.c~current~ +8 -8

diff ./drivers/md/bitmap.c~current~ ./drivers/md/bitmap.c
@@ -782,7 +782,9 @@ static int bitmap_init_from_disk(struct 
 			"recovery\n", bmname(bitmap));
 
 	bytes = (chunks + 7) / 8;
-	num_pages = (bytes + PAGE_SIZE - 1) / PAGE_SIZE;
+
+	num_pages = (bytes + sizeof(bitmap_super_t) + PAGE_SIZE - 1) / PAGE_SIZE + 1;
+
 	if (i_size_read(file->f_mapping->host) < bytes + sizeof(bitmap_super_t)) {
 		printk(KERN_INFO "%s: bitmap file too short %lu < %lu\n",
 			bmname(bitmap),

@@ -790,18 +792,16 @@ static int bitmap_init_from_disk(struct 
 			bytes + sizeof(bitmap_super_t));
 		goto out;
 	}
-	num_pages++;
+
+	ret = -ENOMEM;
+
 	bitmap->filemap = kmalloc(sizeof(struct page *) * num_pages, GFP_KERNEL);
-	if (!bitmap->filemap) {
-		ret = -ENOMEM;
+	if (!bitmap->filemap)
 		goto out;
-	}
 
 	bitmap->filemap_attr = kmalloc(sizeof(long) * num_pages, GFP_KERNEL);
-	if (!bitmap->filemap_attr) {
-		ret = -ENOMEM;
+	if (!bitmap->filemap_attr)
 		goto out;
-	}
 
 	memset(bitmap->filemap_attr, 0, sizeof(long) * num_pages);

NeilBrown

author

2005-03-23 05:47 UTC #5 lore raw

PATCH 1/1 Print correct pid for newly created bitmap-writeback-daemon.

In reply to NeilBrown

The debugging message printed the wrong pid, which didn't help remove bugs....

Signed-off-by Neil Brown <[email protected]>

### Diffstat output

 ./drivers/md/bitmap.c |    2 +-
 1 files changed, 1 insertion(+), 1 deletion(-)

drivers/md/bitmap.c renamed from drivers/md/bitmap.c~current~ +1 -1

diff ./drivers/md/bitmap.c~current~ ./drivers/md/bitmap.c
@@ -1107,7 +1107,7 @@ static int bitmap_start_daemon(struct bi
 	md_wakeup_thread(daemon); /* start it running */
 
 	PRINTK("%s: %s daemon (pid %d) started...\n",
-		bmname(bitmap), name, bitmap->daemon->tsk->pid);
+		bmname(bitmap), name, daemon->tsk->pid);
 out_unlock:
 	spin_unlock_irqrestore(&bitmap->lock, flags);
 	return 0;

NeilBrown

author

2005-03-23 05:47 UTC #6 lore raw

PATCH 1/1 Call bitmap_daemon_work regularly

In reply to NeilBrown

bitmap_daemon_work clears bits in the bitmap for blocks that haven't been written to for a while.
It needs to be called regularly to make sure the bitmap doesn't endup full of ones .... but it wasn't.

So call it from the increasingly-inaptly-named md_check_recovery

Signed-off-by Neil Brown <[email protected]>

### Diffstat output

 ./drivers/md/md.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletion(-)

drivers/md/md.c renamed from drivers/md/md.c~current~ +2 -1

diff ./drivers/md/md.c~current~ ./drivers/md/md.c
@@ -3682,7 +3682,8 @@ void md_check_recovery(mddev_t *mddev)
 	struct list_head *rtmp;
 
 
-	dprintk(KERN_INFO "md: recovery thread got woken up ...\n");
+	if (mddev->bitmap)
+		bitmap_daemon_work(mddev->bitmap);
 
 	if (mddev->ro)
 		return;

NeilBrown

author

2005-03-23 05:47 UTC #7 lore raw

PATCH 1/1 Don't skip bitmap pages due to lack of bit that we just cleared.

In reply to NeilBrown

When looking for pages that need cleaning we skip pages that don't have BITMAP_PAGE_CLEAN set. But if it is the 'current' page we will have cleared that bit ourselves, so skipping it is wrong.
So: move the 'skip this page' inside 'if page != lastpage'.

Also fold call of file_page_offset into the one place where the value (bit) is used.

Signed-off-by Neil Brown <[email protected]>

### Diffstat output

 ./drivers/md/bitmap.c |   35 +++++++++++++++++------------------
 1 files changed, 17 insertions(+), 18 deletions(-)

drivers/md/bitmap.c renamed from drivers/md/bitmap.c~current~ +17 ~~-18~~

diff ./drivers/md/bitmap.c~current~ ./drivers/md/bitmap.c
@@ -908,7 +908,7 @@ static bitmap_counter_t *bitmap_get_coun
 
 int bitmap_daemon_work(struct bitmap *bitmap)
 {
-	unsigned long bit, j;
+	unsigned long j;
 	unsigned long flags;
 	struct page *page = NULL, *lastpage = NULL;
 	int err = 0;

@@ -931,24 +931,23 @@ int bitmap_daemon_work(struct bitmap *bi
 		}
 
 		page = filemap_get_page(bitmap, j);
-		/* skip this page unless it's marked as needing cleaning */
-		if (!((attr=get_page_attr(bitmap, page)) & BITMAP_PAGE_CLEAN)) {
-			if (attr & BITMAP_PAGE_NEEDWRITE) {
-				page_cache_get(page);
-				clear_page_attr(bitmap, page, BITMAP_PAGE_NEEDWRITE);
-			}
-			spin_unlock_irqrestore(&bitmap->lock, flags);
-			if (attr & BITMAP_PAGE_NEEDWRITE) {
-				if (write_page(bitmap, page, 0))
-					bitmap_file_kick(bitmap);
-				page_cache_release(page);
-			}
-			continue;
-		}
-
-		bit = file_page_offset(j);
 
 		if (page != lastpage) {
+			/* skip this page unless it's marked as needing cleaning */
+			if (!((attr=get_page_attr(bitmap, page)) & BITMAP_PAGE_CLEAN)) {
+				if (attr & BITMAP_PAGE_NEEDWRITE) {
+					page_cache_get(page);
+					clear_page_attr(bitmap, page, BITMAP_PAGE_NEEDWRITE);
+				}
+				spin_unlock_irqrestore(&bitmap->lock, flags);
+				if (attr & BITMAP_PAGE_NEEDWRITE) {
+					if (write_page(bitmap, page, 0))
+						bitmap_file_kick(bitmap);
+					page_cache_release(page);
+				}
+				continue;
+			}
+
 			/* grab the new page, sync and release the old */
 			page_cache_get(page);
 			if (lastpage != NULL) {

@@ -990,7 +989,7 @@ int bitmap_daemon_work(struct bitmap *bi
 						  -1);
 
 				/* clear the bit */
-				clear_bit(bit, page_address(page));
+				clear_bit(file_page_offset(j), page_address(page));
 			}
 		}
 		spin_unlock_irqrestore(&bitmap->lock, flags);

NeilBrown

author

2005-03-23 05:47 UTC #8 lore raw

PATCH 1/1 A couple of tidyups relating to the bitmap file.

In reply to NeilBrown

1/ When init from disk, it is a BUG if there is nowhere
to init from,
2/ use seq_path to print path in /proc/mdstat

Signed-off-by Neil Brown <[email protected]>

### Diffstat output

 ./drivers/md/bitmap.c |    8 +-------
 ./drivers/md/md.c     |   11 +++++------
 2 files changed, 6 insertions(+), 13 deletions(-)

drivers/md/bitmap.c renamed from drivers/md/bitmap.c~current~ +1 -7

diff ./drivers/md/bitmap.c~current~ ./drivers/md/bitmap.c
@@ -764,13 +764,7 @@ static int bitmap_init_from_disk(struct 
 	chunks = bitmap->chunks;
 	file = bitmap->file;
 
-	if (!file) { /* no file, dirty all the in-memory bits */
-		printk(KERN_INFO "%s: no bitmap file, doing full recovery\n",
-			bmname(bitmap));
-		bitmap_set_memory_bits(bitmap, 0,
-				       chunks << CHUNK_BLOCK_SHIFT(bitmap), 1);
-		return 0;
-	}
+	BUG_ON(!file);
 
 #if INJECT_FAULTS_3
 	outofdate = 1;

drivers/md/md.c renamed from drivers/md/md.c~current~ +5 -6

diff ./drivers/md/md.c~current~ ./drivers/md/md.c
@@ -3259,10 +3259,8 @@ static int md_seq_show(struct seq_file *
 			seq_printf(seq, "\n       ");
 
 		if ((bitmap = mddev->bitmap)) {
-			char *buf, *path;
 			unsigned long chunk_kb;
 			unsigned long flags;
-			buf = kmalloc(PAGE_SIZE, GFP_KERNEL);
 			spin_lock_irqsave(&bitmap->lock, flags);
 			chunk_kb = bitmap->chunksize >> 10;
 			seq_printf(seq, "bitmap: %lu/%lu pages [%luKB], "

@@ -3273,13 +3271,14 @@ static int md_seq_show(struct seq_file *
 					<< (PAGE_SHIFT - 10),
 				chunk_kb ? chunk_kb : bitmap->chunksize,
 				chunk_kb ? "KB" : "B");
-			if (bitmap->file && buf) {
-				path = file_path(bitmap->file, buf, PAGE_SIZE);
-				seq_printf(seq, ", file: %s", path ? path : "");
+			if (bitmap->file) {
+				seq_printf(seq, ", file: ");
+				seq_path(seq, bitmap->file->f_vfsmnt, 
+					 bitmap->file->f_dentry," \t\n");
 			}
+
 			seq_printf(seq, "\n");
 			spin_unlock_irqrestore(&bitmap->lock, flags);
-			kfree(buf);
 		}
 
 		seq_printf(seq, "\n");

NeilBrown

author

2005-03-23 05:47 UTC #9 lore raw

PATCH 1/1 Make sure md bitmap is cleared on a clean start.

In reply to NeilBrown

As the array-wide clean bit (in the superblock) is set more agressively than the bits in the bitmap are cleared, it is possible to have an array which is clean despite there being bits set in the bitmap.

These bits will currently never get cleared, as they can only be cleared by a resync pass, which never happens.

No, when reading bits from disk, be aware of whether the whole array is known to be in sync, and act accordingly.

Signed-off-by Neil Brown <[email protected]>

### Diffstat output

 ./drivers/md/bitmap.c |   27 ++++++++++++---------------
 1 files changed, 12 insertions(+), 15 deletions(-)

drivers/md/bitmap.c renamed from drivers/md/bitmap.c~current~ +12 ~~-15~~

diff ./drivers/md/bitmap.c~current~ ./drivers/md/bitmap.c
@@ -742,7 +742,7 @@ int bitmap_unplug(struct bitmap *bitmap)
 }
 
 static void bitmap_set_memory_bits(struct bitmap *bitmap, sector_t offset,
-	unsigned long sectors, int set);
+	unsigned long sectors, int in_sync);
 /* * bitmap_init_from_disk -- called at bitmap_create time to initialize
  * the in-memory bitmap from the on-disk bitmap -- also, sets up the
  * memory mapping of the bitmap file

@@ -751,7 +751,7 @@ static void bitmap_set_memory_bits(struc
  *   previously kicked from the array, we mark all the bits as
  *   1's in order to cause a full resync.
  */
-static int bitmap_init_from_disk(struct bitmap *bitmap)
+static int bitmap_init_from_disk(struct bitmap *bitmap, int in_sync)
 {
 	unsigned long i, chunks, index, oldindex, bit;
 	struct page *page = NULL, *oldpage = NULL;

@@ -777,7 +777,7 @@ static int bitmap_init_from_disk(struct 
 
 	bytes = (chunks + 7) / 8;
 
-	num_pages = (bytes + sizeof(bitmap_super_t) + PAGE_SIZE - 1) / PAGE_SIZE + 1;
+	num_pages = (bytes + sizeof(bitmap_super_t) + PAGE_SIZE - 1) / PAGE_SIZE;
 
 	if (i_size_read(file->f_mapping->host) < bytes + sizeof(bitmap_super_t)) {
 		printk(KERN_INFO "%s: bitmap file too short %lu < %lu\n",

@@ -849,14 +849,9 @@ static int bitmap_init_from_disk(struct 
 		if (test_bit(bit, page_address(page))) {
 			/* if the disk bit is set, set the memory bit */
 			bitmap_set_memory_bits(bitmap,
-					i << CHUNK_BLOCK_SHIFT(bitmap), 1, 1);
+					i << CHUNK_BLOCK_SHIFT(bitmap), 1, in_sync);
 			bit_cnt++;
 		}
-#if 0
-		else
-			bitmap_set_memory_bits(bitmap,
-				       i << CHUNK_BLOCK_SHIFT(bitmap), 1, 0);
-#endif
 	}
 
  	/* everything went OK */

@@ -1319,10 +1314,10 @@ void bitmap_close_sync(struct bitmap *bi
 }
 
 static void bitmap_set_memory_bits(struct bitmap *bitmap, sector_t offset,
-				   unsigned long sectors, int set)
+				   unsigned long sectors, int in_sync)
 {
 	/* For each chunk covered by any of these sectors, set the
-	 * resync needed bit, and the counter to 1.  They should all
+	 * counter to 1 and set resync_needed unless in_sync.  They should all
 	 * be 0 at this point
 	 */
 	while (sectors) {

@@ -1334,10 +1329,12 @@ static void bitmap_set_memory_bits(struc
 			spin_unlock_irq(&bitmap->lock);
 			return;
 		}
-		if (set && !NEEDED(*bmc)) {
-			BUG_ON(*bmc);
-			*bmc = NEEDED_MASK | 1;
+		if (! *bmc) {
+			struct page *page;
+			*bmc = 1 | (in_sync? 0 : NEEDED_MASK);
 			bitmap_count_page(bitmap, offset, 1);
+			page = filemap_get_page(bitmap, offset >> CHUNK_BLOCK_SHIFT(bitmap));
+			set_page_attr(bitmap, page, BITMAP_PAGE_CLEAN);
 		}
 		spin_unlock_irq(&bitmap->lock);
 		if (sectors > secs)

@@ -1477,7 +1474,7 @@ int bitmap_create(mddev_t *mddev)
 
 	/* now that we have some pages available, initialize the in-memory
 	 * bitmap from the on-disk bitmap */
-	err = bitmap_init_from_disk(bitmap);
+	err = bitmap_init_from_disk(bitmap, mddev->recovery_cp == MaxSector);
 	if (err)
 		return err;

NeilBrown

author

2005-03-23 05:47 UTC #10 lore raw

PATCH 1/1 Fix bug when raid1 attempts a partial reconstruct.

In reply to NeilBrown

The logic here is wrong. if fullsync is 0, it WILL BUG.

Signed-off-by Neil Brown <[email protected]>

### Diffstat output

 ./drivers/md/raid1.c |   17 ++++++++++-------
 1 files changed, 10 insertions(+), 7 deletions(-)

drivers/md/raid1.c renamed from drivers/md/raid1.c~current~ +10 -7

diff ./drivers/md/raid1.c~current~ ./drivers/md/raid1.c
@@ -1243,13 +1243,16 @@ static sector_t sync_request(mddev_t *md
 			len = (max_sector - sector_nr) << 9;
 		if (len == 0)
 			break;
-		if (!conf->fullsync && sync_blocks == 0)
-			if (!bitmap_start_sync(mddev->bitmap,
-					       sector_nr, &sync_blocks))
-				break;
-		if (sync_blocks < (PAGE_SIZE>>9))
-			BUG();
-		if (len > (sync_blocks<<9)) len = sync_blocks<<9;
+		if (!conf->fullsync) {
+			if (sync_blocks == 0) {
+				if (!bitmap_start_sync(mddev->bitmap,
+						       sector_nr, &sync_blocks))
+					break;
+				if (sync_blocks < (PAGE_SIZE>>9))
+					BUG();
+				if (len > (sync_blocks<<9)) len = sync_blocks<<9;
+			}
+		}
 
 		for (i=0 ; i < conf->raid_disks; i++) {
 			bio = r1_bio->bios[i];

NeilBrown

author

2005-03-23 05:47 UTC #11 lore raw

PATCH 1/1 Allow md to update multiple superblocks in parallel.

In reply to NeilBrown

currently, md updates all superblocks (one on each device) in series. It waits for one write to complete before starting the next. This isn't a big problem as superblock updates don't happen that often.
However it is neater to do it in parallel, and if the drives in the array have gone to "sleep" after a period of idleness, then waking them is parallel is faster (and someone else should be worrying about power drain).

Futher, we will need parallel superblock updates for a future patch which keeps the intent-logging bitmap near the superblock.

Also remove the silly code that retired superblock updates 100 times. This simply never made sense.

Signed-off-by Neil Brown <[email protected]>

### Diffstat output

 ./drivers/md/md.c           |   85 ++++++++++++++++++++++++--------------------
 ./include/linux/raid/md_k.h |    1 
 2 files changed, 49 insertions(+), 37 deletions(-)

drivers/md/md.c renamed from drivers/md/md.c~current~ +48 ~~-37~~

diff ./drivers/md/md.c~current~ ./drivers/md/md.c
@@ -328,6 +328,40 @@ static void free_disk_sb(mdk_rdev_t * rd
 }
 
 
+static int super_written(struct bio *bio, unsigned int bytes_done, int error)
+{
+	mdk_rdev_t *rdev = bio->bi_private;
+	if (bio->bi_size)
+		return 1;
+
+	if (error || !test_bit(BIO_UPTODATE, &bio->bi_flags))
+		md_error(rdev->mddev, rdev);
+
+	if (atomic_dec_and_test(&rdev->mddev->pending_writes))
+		wake_up(&rdev->mddev->sb_wait);
+	return 0;
+}
+
+void md_super_write(mddev_t *mddev, mdk_rdev_t *rdev,
+		   sector_t sector, int size, struct page *page)
+{
+	/* write first size bytes of page to sector of rdev
+	 * Increment mddev->pending_writes before returning
+	 * and decrement it on completion, waking up sb_wait
+	 * if zero is reached.
+	 * If an error occurred, call md_error
+	 */
+	struct bio *bio = bio_alloc(GFP_KERNEL, 1);
+
+	bio->bi_bdev = rdev->bdev;
+	bio->bi_sector = sector;
+	bio_add_page(bio, page, size, 0);
+	bio->bi_private = rdev;
+	bio->bi_end_io = super_written;
+	atomic_inc(&mddev->pending_writes);
+	submit_bio((1<<BIO_RW)|(1<<BIO_RW_SYNC), bio);
+}
+
 static int bi_complete(struct bio *bio, unsigned int bytes_done, int error)
 {
 	if (bio->bi_size)

@@ -1240,30 +1274,6 @@ void md_print_devices(void)
 }
 
 
-static int write_disk_sb(mdk_rdev_t * rdev)
-{
-	char b[BDEVNAME_SIZE];
-	if (!rdev->sb_loaded) {
-		MD_BUG();
-		return 1;
-	}
-	if (rdev->faulty) {
-		MD_BUG();
-		return 1;
-	}
-
-	dprintk(KERN_INFO "(write) %s's sb offset: %llu\n",
-		bdevname(rdev->bdev,b),
-	       (unsigned long long)rdev->sb_offset);
-  
-	if (sync_page_io(rdev->bdev, rdev->sb_offset<<1, MD_SB_BYTES, rdev->sb_page, WRITE))
-		return 0;
-
-	printk("md: write_disk_sb failed for device %s\n", 
-		bdevname(rdev->bdev,b));
-	return 1;
-}
-
 static void sync_sbs(mddev_t * mddev)
 {
 	mdk_rdev_t *rdev;

@@ -1278,7 +1288,7 @@ static void sync_sbs(mddev_t * mddev)
 
 static void md_update_sb(mddev_t * mddev)
 {
-	int err, count = 100;
+	int err;
 	struct list_head *tmp;
 	mdk_rdev_t *rdev;
 	int sync_req;

@@ -1298,6 +1308,7 @@ repeat:
 		MD_BUG();
 		mddev->events --;
 	}
+	mddev->sb_dirty = 2;
 	sync_sbs(mddev);
 
 	/*

@@ -1325,24 +1336,24 @@ repeat:
 
 		dprintk("%s ", bdevname(rdev->bdev,b));
 		if (!rdev->faulty) {
-			err += write_disk_sb(rdev);
+			md_super_write(mddev,rdev,
+				       rdev->sb_offset<<1, MD_SB_BYTES,
+				       rdev->sb_page);
+			dprintk(KERN_INFO "(write) %s's sb offset: %llu\n",
+				bdevname(rdev->bdev,b),
+				(unsigned long long)rdev->sb_offset);
+
 		} else
 			dprintk(")\n");
-		if (!err && mddev->level == LEVEL_MULTIPATH)
+		if (mddev->level == LEVEL_MULTIPATH)
 			/* only need to write one superblock... */
 			break;
 	}
-	if (err) {
-		if (--count) {
-			printk(KERN_ERR "md: errors occurred during superblock"
-				" update, repeating\n");
-			goto repeat;
-		}
-		printk(KERN_ERR \
-			"md: excessive errors occurred during superblock update, exiting\n");
-	}
+	wait_event(mddev->sb_wait, atomic_read(&mddev->pending_writes)==0);
+	/* if there was a failure, sb_dirty was set to 1, and we re-write super */
+
 	spin_lock(&mddev->write_lock);
-	if (mddev->in_sync != sync_req) {
+	if (mddev->in_sync != sync_req|| mddev->sb_dirty == 1) {
 		/* have to write it out again */
 		spin_unlock(&mddev->write_lock);
 		goto repeat;

include/linux/raid/md_k.h renamed from include/linux/raid/md_k.h~current~ +1 -0

diff ./include/linux/raid/md_k.h~current~ ./include/linux/raid/md_k.h
@@ -262,6 +262,7 @@ struct mddev_s
 
 	spinlock_t			write_lock;
 	wait_queue_head_t		sb_wait;	/* for waiting on superblock updates */
+	atomic_t			pending_writes;	/* number of active superblock writes */
 
 	unsigned int			safemode;	/* if set, update "clean" superblock
 							 * when no writes pending.

NeilBrown

author

2005-03-23 05:47 UTC #12 lore raw

PATCH 1/1 Allow md intent bitmap to be stored near the superblock.

In reply to NeilBrown

This provides an alternate to storing the bitmap in a separate file. The bitmap can be stored at a given offset from the superblock. Obviously the creator of the array must make sure this doesn't intersect with data....
After is good for version-0.90 superblocks.

Signed-off-by Neil Brown <[email protected]>

Only write bitmap to devices which are in-sync

Signed-off-by Neil Brown <[email protected]>

### Diffstat output

 ./drivers/md/bitmap.c         |  132 +++++++++++++++++++++++++++++++++---------
 ./drivers/md/md.c             |   40 ++++++++++++
 ./include/linux/raid/bitmap.h |    2 
 ./include/linux/raid/md.h     |   15 ++++
 ./include/linux/raid/md_k.h   |    4 +
 ./include/linux/raid/md_p.h   |    7 +-
 6 files changed, 170 insertions(+), 30 deletions(-)

drivers/md/bitmap.c renamed from drivers/md/bitmap.c~current~ +106 ~~-26~~

diff ./drivers/md/bitmap.c~current~ ./drivers/md/bitmap.c
@@ -116,7 +116,7 @@ static unsigned char *bitmap_alloc_page(
 	if (!page)
 		printk("%s: bitmap_alloc_page FAILED\n", bmname(bitmap));
 	else
-		printk("%s: bitmap_alloc_page: allocated page at %p\n",
+		PRINTK("%s: bitmap_alloc_page: allocated page at %p\n",
 			bmname(bitmap), page);
 	return page;
 }

@@ -258,13 +258,61 @@ char *file_path(struct file *file, char 
  * basic page I/O operations
  */
 
+/* IO operations when bitmap is stored near all superblocks */
+static struct page *read_sb_page(mddev_t *mddev, long offset, unsigned long index)
+{
+	/* choose a good rdev and read the page from there */
+
+	mdk_rdev_t *rdev;
+	struct list_head *tmp;
+	struct page *page = alloc_page(GFP_KERNEL);
+	sector_t target;
+
+	if (!page)
+		return ERR_PTR(-ENOMEM);
+	do {
+		ITERATE_RDEV(mddev, rdev, tmp)
+			if (rdev->in_sync && !rdev->faulty)
+				goto found;
+		return ERR_PTR(-EIO);
+
+	found:
+		target = (rdev->sb_offset << 1) + offset + index * (PAGE_SIZE/512);
+
+	} while (!sync_page_io(rdev->bdev, target, PAGE_SIZE, page, READ));
+
+	page->index = index;
+	return page;
+}
+
+static int write_sb_page(mddev_t *mddev, long offset, struct page *page, int wait)
+{
+	mdk_rdev_t *rdev;
+	struct list_head *tmp;
+
+	ITERATE_RDEV(mddev, rdev, tmp) 
+		if (rdev->in_sync && !rdev->faulty)
+			md_super_write(mddev, rdev, 
+				       (rdev->sb_offset<<1) + offset
+				       + page->index * (PAGE_SIZE/512),
+				       PAGE_SIZE,
+				       page);
+
+	if (wait)
+		wait_event(mddev->sb_wait, atomic_read(&mddev->pending_writes)==0);
+	return 0;
+}
+
 /*
- * write out a page
+ * write out a page to a file
  */
 static int write_page(struct bitmap *bitmap, struct page *page, int wait)
 {
 	int ret = -ENOMEM;
 
+	if (bitmap->file == NULL)
+		return write_sb_page(bitmap->mddev, bitmap->offset, page, wait);
+
 	lock_page(page);
 
 	ret = page->mapping->a_ops->prepare_write(NULL, page, 0, PAGE_SIZE);

@@ -394,7 +442,12 @@ static int bitmap_read_sb(struct bitmap 
 	int err = -EINVAL;
 
 	/* page 0 is the superblock, read it... */
-	bitmap->sb_page = read_page(bitmap->file, 0, &bytes_read);
+	if (bitmap->file)
+		bitmap->sb_page = read_page(bitmap->file, 0, &bytes_read);
+	else {
+		bitmap->sb_page = read_sb_page(bitmap->mddev, bitmap->offset, 0);
+		bytes_read = PAGE_SIZE;
+	}
 	if (IS_ERR(bitmap->sb_page)) {
 		err = PTR_ERR(bitmap->sb_page);
 		bitmap->sb_page = NULL;

@@ -625,14 +678,16 @@ static void bitmap_file_kick(struct bitm
 	bitmap_mask_state(bitmap, BITMAP_STALE, MASK_SET);
 	bitmap_update_sb(bitmap);
 
-	path = kmalloc(PAGE_SIZE, GFP_KERNEL);
-	if (path)
-		ptr = file_path(bitmap->file, path, PAGE_SIZE);
+	if (bitmap->file) {
+		path = kmalloc(PAGE_SIZE, GFP_KERNEL);
+		if (path)
+			ptr = file_path(bitmap->file, path, PAGE_SIZE);
 
-	printk(KERN_ALERT "%s: kicking failed bitmap file %s from array!\n",
-		bmname(bitmap), ptr ? ptr : "");
+		printk(KERN_ALERT "%s: kicking failed bitmap file %s from array!\n",
+		       bmname(bitmap), ptr ? ptr : "");
 
-	kfree(path);
+		kfree(path);
+	}
 
 	bitmap_file_put(bitmap);

@@ -676,7 +731,7 @@ static void bitmap_file_set_bit(struct b
 	void *kaddr;
 	unsigned long chunk = block >> CHUNK_BLOCK_SHIFT(bitmap);
 
-	if (!bitmap->file || !bitmap->filemap) {
+	if (!bitmap->filemap) {
 		return;
 	}

@@ -715,7 +770,7 @@ int bitmap_unplug(struct bitmap *bitmap)
 	 * flushed out to disk */
 	for (i = 0; i < bitmap->file_pages; i++) {
 		spin_lock_irqsave(&bitmap->lock, flags);
-		if (!bitmap->file || !bitmap->filemap) {
+		if (!bitmap->filemap) {
 			spin_unlock_irqrestore(&bitmap->lock, flags);
 			return 0;
 		}

@@ -732,11 +787,15 @@ int bitmap_unplug(struct bitmap *bitmap)
 				return 1;
 	}
 	if (wait) { /* if any writes were performed, we need to wait on them */
-		spin_lock_irq(&bitmap->write_lock);
-		wait_event_lock_irq(bitmap->write_wait,
-			list_empty(&bitmap->complete_pages), bitmap->write_lock,
-			wake_up_process(bitmap->writeback_daemon->tsk));
-		spin_unlock_irq(&bitmap->write_lock);
+		if (bitmap->file) {
+			spin_lock_irq(&bitmap->write_lock);
+			wait_event_lock_irq(bitmap->write_wait,
+					    list_empty(&bitmap->complete_pages), bitmap->write_lock,
+					    wake_up_process(bitmap->writeback_daemon->tsk));
+			spin_unlock_irq(&bitmap->write_lock);
+		} else
+			wait_event(bitmap->mddev->sb_wait,
+				   atomic_read(&bitmap->mddev->pending_writes)==0);
 	}
 	return 0;
 }

@@ -764,7 +823,7 @@ static int bitmap_init_from_disk(struct 
 	chunks = bitmap->chunks;
 	file = bitmap->file;
 
-	BUG_ON(!file);
+	BUG_ON(!file && !bitmap->offset);
 
 #if INJECT_FAULTS_3
 	outofdate = 1;

@@ -779,7 +838,7 @@ static int bitmap_init_from_disk(struct 
 
 	num_pages = (bytes + sizeof(bitmap_super_t) + PAGE_SIZE - 1) / PAGE_SIZE;
 
-	if (i_size_read(file->f_mapping->host) < bytes + sizeof(bitmap_super_t)) {
+	if (file && i_size_read(file->f_mapping->host) < bytes + sizeof(bitmap_super_t)) {
 		printk(KERN_INFO "%s: bitmap file too short %lu < %lu\n",
 			bmname(bitmap),
 			(unsigned long) i_size_read(file->f_mapping->host),

@@ -816,14 +875,18 @@ static int bitmap_init_from_disk(struct 
 				 */
 				page = bitmap->sb_page;
 				offset = sizeof(bitmap_super_t);
-			} else {
+			} else if (file) {
 				page = read_page(file, index, &dummy);
-				if (IS_ERR(page)) { /* read error */
-					ret = PTR_ERR(page);
-					goto out;
-				}
 				offset = 0;
+			} else {
+				page = read_sb_page(bitmap->mddev, bitmap->offset, index);
+				offset = 0;
+			}
+			if (IS_ERR(page)) { /* read error */
+				ret = PTR_ERR(page);
+				goto out;
 			}
+
 			oldindex = index;
 			oldpage = page;
 			kmap(page);

@@ -874,6 +937,19 @@ out:
 	return ret;
 }
 
+void bitmap_write_all(struct bitmap *bitmap)
+{
+	/* We don't actually write all bitmap blocks here,
+	 * just flag them as needing to be written
+	 */
+
+	unsigned long chunks = bitmap->chunks;
+	unsigned long bytes = (chunks+7)/8 + sizeof(bitmap_super_t);
+	unsigned long num_pages = (bytes + PAGE_SIZE-1) / PAGE_SIZE;
+	while (num_pages--)
+		bitmap->filemap_attr[num_pages] |= BITMAP_PAGE_NEEDWRITE;
+}
+
 
 static void bitmap_count_page(struct bitmap *bitmap, sector_t offset, int inc)
 {

@@ -913,7 +989,7 @@ int bitmap_daemon_work(struct bitmap *bi
 	for (j = 0; j < bitmap->chunks; j++) {
 		bitmap_counter_t *bmc;
 		spin_lock_irqsave(&bitmap->lock, flags);
-		if (!bitmap->file || !bitmap->filemap) {
+		if (!bitmap->filemap) {
 			/* error or shutdown */
 			spin_unlock_irqrestore(&bitmap->lock, flags);
 			break;

@@ -1072,6 +1148,7 @@ static int bitmap_start_daemon(struct bi
 
 	spin_lock_irqsave(&bitmap->lock, flags);
 	*ptr = NULL;
+
 	if (!bitmap->file) /* no need for daemon if there's no backing file */
 		goto out_unlock;

@@ -1416,9 +1493,11 @@ int bitmap_create(mddev_t *mddev)
 
 	BUG_ON(sizeof(bitmap_super_t) != 256);
 
-	if (!file) /* bitmap disabled, nothing to do */
+	if (!file && !mddev->bitmap_offset) /* bitmap disabled, nothing to do */
 		return 0;
 
+	BUG_ON(file && mddev->bitmap_offset);
+
 	bitmap = kmalloc(sizeof(*bitmap), GFP_KERNEL);
 	if (!bitmap)
 		return -ENOMEM;

@@ -1438,7 +1517,8 @@ int bitmap_create(mddev_t *mddev)
 		return -ENOMEM;
 
 	bitmap->file = file;
-	get_file(file);
+	bitmap->offset = mddev->bitmap_offset;
+	if (file) get_file(file);
 	/* read superblock from bitmap file (this sets bitmap->chunksize) */
 	err = bitmap_read_sb(bitmap);
 	if (err)

drivers/md/md.c renamed from drivers/md/md.c~current~ +38 -2

diff ./drivers/md/md.c~current~ ./drivers/md/md.c
@@ -371,7 +371,7 @@ static int bi_complete(struct bio *bio, 
 	return 0;
 }
 
-static int sync_page_io(struct block_device *bdev, sector_t sector, int size,
+int sync_page_io(struct block_device *bdev, sector_t sector, int size,
 		   struct page *page, int rw)
 {
 	struct bio *bio = bio_alloc(GFP_KERNEL, 1);

@@ -643,6 +643,17 @@ static int super_90_validate(mddev_t *md
 		memcpy(mddev->uuid+12,&sb->set_uuid3, 4);
 
 		mddev->max_disks = MD_SB_DISKS;
+
+		if (sb->state & (1<<MD_SB_BITMAP_PRESENT) && 
+		    mddev->bitmap_file == NULL) {
+			if (mddev->level != 1) {
+				/* FIXME use a better test */
+				printk(KERN_WARNING "md: bitmaps only support for raid1\n");
+				return -EINVAL;
+			}
+			mddev->bitmap_offset = (MD_SB_BYTES >> 9);
+		}
+
 	} else if (mddev->pers == NULL) {
 		/* Insist on good event counter while assembling */
 		__u64 ev1 = md_event(sb);

@@ -736,6 +747,9 @@ static void super_90_sync(mddev_t *mddev
 	sb->layout = mddev->layout;
 	sb->chunk_size = mddev->chunk_size;
 
+	if (mddev->bitmap && mddev->bitmap_file == NULL)
+		sb->state |= (1<<MD_SB_BITMAP_PRESENT);
+
 	sb->disks[0].state = (1<<MD_DISK_REMOVED);
 	ITERATE_RDEV(mddev,rdev2,tmp) {
 		mdp_disk_t *d;

@@ -932,6 +946,15 @@ static int super_1_validate(mddev_t *mdd
 		memcpy(mddev->uuid, sb->set_uuid, 16);
 
 		mddev->max_disks =  (4096-256)/2;
+
+		if ((le32_to_cpu(sb->feature_map) & 1) &&
+		    mddev->bitmap_file == NULL ) {
+			if (mddev->level != 1) {
+				printk(KERN_WARNING "md: bitmaps only supported for raid1\n");
+				return -EINVAL;
+			}
+			mddev->bitmap_offset = (__s32)le32_to_cpu(sb->bitmap_offset);
+		}
 	} else if (mddev->pers == NULL) {
 		/* Insist of good event counter while assembling */
 		__u64 ev1 = le64_to_cpu(sb->events);

@@ -994,6 +1017,11 @@ static void super_1_sync(mddev_t *mddev,
 	else
 		sb->resync_offset = cpu_to_le64(0);
 
+	if (mddev->bitmap && mddev->bitmap_file == NULL) {
+		sb->bitmap_offset = cpu_to_le32((__u32)mddev->bitmap_offset);
+		sb->feature_map = cpu_to_le32(1);
+	}
+
 	max_dev = 0;
 	ITERATE_RDEV(mddev,rdev2,tmp)
 		if (rdev2->desc_nr+1 > max_dev)

@@ -2426,7 +2454,8 @@ static int set_bitmap_file(mddev_t *mdde
 			mdname(mddev));
 		fput(mddev->bitmap_file);
 		mddev->bitmap_file = NULL;
-	}
+	} else
+		mddev->bitmap_offset = 0; /* file overrides offset */
 	return err;
 }

@@ -3801,6 +3830,13 @@ void md_check_recovery(mddev_t *mddev)
 			set_bit(MD_RECOVERY_RUNNING, &mddev->recovery);
 			if (!spares)
 				set_bit(MD_RECOVERY_SYNC, &mddev->recovery);
+			if (spares && mddev->bitmap && ! mddev->bitmap->file) {
+				/* We are adding a device or devices to an array 
+				 * which has the bitmap stored on all devices.
+				 * So make sure all bitmap pages get written
+				 */
+				bitmap_write_all(mddev->bitmap);
+			}
 			mddev->sync_thread = md_register_thread(md_do_sync,
 								mddev,
 								"%s_resync");

include/linux/raid/bitmap.h renamed from include/linux/raid/bitmap.h~current~ +2 -0

diff ./include/linux/raid/bitmap.h~current~ ./include/linux/raid/bitmap.h
@@ -217,6 +217,7 @@ struct bitmap {
 	/* bitmap spinlock */
 	spinlock_t lock;
 
+	long offset; /* offset from superblock if file is NULL */
 	struct file *file; /* backing disk file */
 	struct page *sb_page; /* cached copy of the bitmap file superblock */
 	struct page **filemap; /* list of cache pages for the file */

@@ -255,6 +256,7 @@ void bitmap_print_sb(struct bitmap *bitm
 int bitmap_update_sb(struct bitmap *bitmap);
 
 int  bitmap_setallbits(struct bitmap *bitmap);
+void bitmap_write_all(struct bitmap *bitmap);
 
 /* these are exported */
 int bitmap_startwrite(struct bitmap *bitmap, sector_t offset, unsigned long sectors);

include/linux/raid/md.h renamed from include/linux/raid/md.h~current~ +14 -1

diff ./include/linux/raid/md.h~current~ ./include/linux/raid/md.h
@@ -60,7 +60,14 @@
  */
 #define MD_MAJOR_VERSION                0
 #define MD_MINOR_VERSION                90
-#define MD_PATCHLEVEL_VERSION           1
+/*
+ * MD_PATCHLEVEL_VERSION indicates kernel functionality.
+ * >=1 means different superblock formats are selectable using SET_ARRAY_INFO
+ *     and major_version/minor_version accordingly
+ * >=2 means that Internal bitmaps are supported by setting MD_SB_BITMAP_PRESENT
+ *     in the super status byte
+ */
+#define MD_PATCHLEVEL_VERSION           2
 
 extern int register_md_personality (int p_num, mdk_personality_t *p);
 extern int unregister_md_personality (int p_num);

@@ -78,6 +85,12 @@ extern void md_unplug_mddev(mddev_t *mdd
 
 extern void md_print_devices (void);
 
+extern void md_super_write(mddev_t *mddev, mdk_rdev_t *rdev,
+			   sector_t sector, int size, struct page *page);
+extern int sync_page_io(struct block_device *bdev, sector_t sector, int size,
+			struct page *page, int rw);
+
+
 #define MD_BUG(x...) { printk("md: bug in file %s, line %d\n", __FILE__, __LINE__); md_print_devices(); }
 
 #endif

include/linux/raid/md_k.h renamed from include/linux/raid/md_k.h~current~ +4 -0

diff ./include/linux/raid/md_k.h~current~ ./include/linux/raid/md_k.h
@@ -274,6 +274,10 @@ struct mddev_s
 
 	struct bitmap                   *bitmap; /* the bitmap for the device */
 	struct file			*bitmap_file; /* the bitmap file */
+	long				bitmap_offset; /* offset from superblock of
+							* start of bitmap. May be 
+							* negative, but not '0'
+							*/
 
 	struct list_head		all_mddevs;
 };

include/linux/raid/md_p.h renamed from include/linux/raid/md_p.h~current~ +6 -1

diff ./include/linux/raid/md_p.h~current~ ./include/linux/raid/md_p.h
@@ -96,6 +96,7 @@ typedef struct mdp_device_descriptor_s {
 #define MD_SB_CLEAN		0
 #define MD_SB_ERRORS		1
 
+#define	MD_SB_BITMAP_PRESENT	8 /* bitmap may be present nearby */
 typedef struct mdp_superblock_s {
 	/*
 	 * Constant generic information

@@ -184,7 +185,7 @@ struct mdp_superblock_1 {
 	/* constant array information - 128 bytes */
 	__u32	magic;		/* MD_SB_MAGIC: 0xa92b4efc - little endian */
 	__u32	major_version;	/* 1 */
-	__u32	feature_map;	/* 0 for now */
+	__u32	feature_map;	/* bit 0 set if 'bitmap_offset' is meaningful */
 	__u32	pad0;		/* always set to 0 when writing */
 
 	__u8	set_uuid[16];	/* user-space generated. */

@@ -197,6 +198,10 @@ struct mdp_superblock_1 {
 
 	__u32	chunksize;	/* in 512byte sectors */
 	__u32	raid_disks;
+	__u32	bitmap_offset;	/* sectors after start of superblock that bitmap starts
+				 * NOTE: signed, so bitmap can be before superblock
+				 * only meaningful of feature_map[0] is set.
+				 */
 	__u8	pad1[128-96];	/* set to 0 when written */
 
 	/* constant this-device information - 64 bytes */

Andrew Morton

2005-03-23 09:52 UTC #13 lore raw

In reply to NeilBrown

Quoted 6 lines · NeilBrown

Andrew: Are you happy to keep collecting these as a list of patches (bugs followed by bug-fixes :-), or would it be easier if I merged all the bug fixes into earlier patches and just resent a small number of "add-functionality" patches??

What I'll generally do is to topologically sort the patches. Work out what patch each of the new patches fix and then rename the patches to name-of-the-patch-whcih-is-being-fixed-fix.patch.

So if you had

foo.patch
bar.patch

and then sent be "[patch] frob the nozzle"

I'd work out that this new patch is fixing foo.patch and I'd name it "foo-fix.patch" and the sequence would become:

foo.patch
foo-fix.patch
bar.patch

and then at some time in the future I'll collapse foo-fix.patch, foo-fix-fix.patch, etc into foo.patch.

Of course it doesn't always work out that simply ;)

In this case it's not clear that a tsort will work out right. I'll take a look.

Andrew Morton

2005-03-23 10:12 UTC #14 lore raw

Quoted 5 lines · Andrew Morton

Of course it doesn't always work out that simply ;)
In this case it's not clear that a tsort will work out right. I'll take a look.

Well it wasn't completely clean, and I forgot to rename the patches, but this is how the tsort came out:

md-merge-md_enter_safemode-into-md_check_recovery.patch md-improve-locking-on-safemode-and-move-superblock-writes.patch md-improve-the-interface-to-sync_request.patch md-optimised-resync-using-bitmap-based-intent-logging.patch
+md-a-couple-of-tidyups-relating-to-the-bitmap-file.patch +md-call-bitmap_daemon_work-regularly.patch +md-print-correct-pid-for-newly-created-bitmap-writeback-daemon.patch +md-minor-code-rearrangement-in-bitmap_init_from_disk.patch +md-make-sure-md-bitmap-is-cleared-on-a-clean-start.patch
md-printk-fix.patch
+md-improve-debug-printing-of-bitmap-superblock.patch +md-check-return-value-of-write_page-rather-than-ignore-it.patch +md-enable-the-bitmap-write-back-daemon-and-wait-for-it.patch +md-dont-skip-bitmap-pages-due-to-lack-of-bit-that-we-just-cleared.patch
md-optimised-resync-using-bitmap-based-intent-logging-fix.patch md-raid1-support-for-bitmap-intent-logging.patch
+md-fix-bug-when-raid1-attempts-a-partial-reconstruct.patch
md-raid1-support-for-bitmap-intent-logging-fix.patch md-optimise-reconstruction-when-re-adding-a-recently-failed-drive.patch md-fix-deadlock-due-to-md-thread-processing-delayed-requests.patch
+md-allow-md-intent-bitmap-to-be-stored-near-the-superblock.patch +md-allow-md-to-update-multiple-superblocks-in-parallel.patch

So we can later work out which of these to fold into which other ones.

Lars Marowsky-Bree

2005-03-23 17:53 UTC #15 lore raw

In reply to NeilBrown

Quoted 4 lines · NeilBrown

Andrew: Are you happy to keep collecting these as a list of patches (bugs followed by bug-fixes :-), or would it be easier if I merged all the bug fixes into earlier patches and just resent a small number of "add-functionality" patches??

I'm not Andrew, but as we're pulling out fixes for the existing code from -mm too, having it split into bugfixes + new features (w/fixes) makes it a bit easier for us...

Sincerely,
Lars Marowsky-Brée <[email protected]>

Signature · 8 lines

High Availability & Clustering
SUSE Labs, Research and Development
SUSE LINUX Products GmbH - A Novell Business

-
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html