[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#443373: linux-image-2.6.18-4-ixp4xx: kernel null paging request, crash from bitmapped md devices



hi martin,

On Wednesday 26 September 2007 11:15:12 am Martin Michlmayr wrote:
> * sean finney <seanius@debian.org> [2007-09-21 09:10]:
> > i don't really have the resources to try compilingpatched arm kernels, 
> > but if you do and want me to test something just let me know.
>
> I put two test kernels at http://merkel.debian.org/~tbm/nslu2/
>
> If the "a" variant works, can you please also try the "b" variant (if
> the "a" doesn't work, there's no need to try "b").

like i mentioned on irc, "a" doesn't work, unfortunately.  with "a" it doesn't 
oops/crash, and instead will hang when mounting an md device with a bitmap.

the system is still useable while it hangs, though it will require a hard 
reboot due to the hung mount.  i'm pretty sur ethe patch is still relevant, 
but i think there's more that needs to be done.  so i've taken the difference 
between 2.6.18 and 2.6.21 from upstream git, and cherry picked the commits 
that looked like they might be relevant.  below i've attached the commit logs 
incl description and commit id's, as well as a patch of all of these 
cherry-picked into the 2.6.18 version.

shall we give it another try with this? 


	sean



diff --git a/drivers/md/bitmap.c b/drivers/md/bitmap.c
index ecc5676..1814ac0 100644
--- a/drivers/md/bitmap.c
+++ b/drivers/md/bitmap.c
@@ -479,9 +479,12 @@ static int bitmap_read_sb(struct bitmap *bitmap)
 	int err = -EINVAL;
 
 	/* page 0 is the superblock, read it... */
-	if (bitmap->file)
-		bitmap->sb_page = read_page(bitmap->file, 0, bitmap, PAGE_SIZE);
-	else {
+	if (bitmap->file) {
+		loff_t isize = i_size_read(bitmap->file->f_mapping->host);
+		int bytes = isize > PAGE_SIZE ? PAGE_SIZE : isize;
+
+		bitmap->sb_page = read_page(bitmap->file, 0, bitmap, bytes);
+	} else {
 		bitmap->sb_page = read_sb_page(bitmap->mddev, bitmap->offset, 0);
 	}
 	if (IS_ERR(bitmap->sb_page)) {
@@ -536,7 +539,7 @@ static int bitmap_read_sb(struct bitmap *bitmap)
 		printk(KERN_INFO "%s: bitmap file is out of date (%llu < %llu) "
 			"-- forcing full recovery\n", bmname(bitmap), events,
 			(unsigned long long) bitmap->mddev->events);
-		sb->state |= BITMAP_STALE;
+		sb->state |= cpu_to_le32(BITMAP_STALE);
 	}
 success:
 	/* assign fields using values from superblock */
@@ -544,11 +547,11 @@ success:
 	bitmap->daemon_sleep = daemon_sleep;
 	bitmap->daemon_lastrun = jiffies;
 	bitmap->max_write_behind = write_behind;
-	bitmap->flags |= sb->state;
+	bitmap->flags |= le32_to_cpu(sb->state);
 	if (le32_to_cpu(sb->version) == BITMAP_MAJOR_HOSTENDIAN)
 		bitmap->flags |= BITMAP_HOSTENDIAN;
 	bitmap->events_cleared = le64_to_cpu(sb->events_cleared);
-	if (sb->state & BITMAP_STALE)
+	if (sb->state & cpu_to_le32(BITMAP_STALE))
 		bitmap->events_cleared = bitmap->mddev->events;
 	err = 0;
 out:
@@ -578,9 +581,9 @@ static void bitmap_mask_state(struct bitmap *bitmap, enum bitmap_state bits,
 	spin_unlock_irqrestore(&bitmap->lock, flags);
 	sb = (bitmap_super_t *)kmap_atomic(bitmap->sb_page, KM_USER0);
 	switch (op) {
-		case MASK_SET: sb->state |= bits;
+		case MASK_SET: sb->state |= cpu_to_le32(bits);
 				break;
-		case MASK_UNSET: sb->state &= ~bits;
+		case MASK_UNSET: sb->state &= cpu_to_le32(~bits);
 				break;
 		default: BUG();
 	}
@@ -858,9 +861,7 @@ static int bitmap_init_from_disk(struct bitmap *bitmap, sector_t start)
 
 	/* We need 4 bits per page, rounded up to a multiple of sizeof(unsigned long) */
 	bitmap->filemap_attr = kzalloc(
-		(((num_pages*4/8)+sizeof(unsigned long)-1)
-		 /sizeof(unsigned long))
-		*sizeof(unsigned long),
+		roundup( DIV_ROUND_UP(num_pages*4, 8), sizeof(unsigned long)),
 		GFP_KERNEL);
 	if (!bitmap->filemap_attr)
 		goto out;
@@ -875,7 +876,8 @@ static int bitmap_init_from_disk(struct bitmap *bitmap, sector_t start)
 			int count;
 			/* unmap the old page, we're done with it */
 			if (index == num_pages-1)
-				count = bytes - index * PAGE_SIZE;
+				count = bytes + sizeof(bitmap_super_t)
+					- index * PAGE_SIZE;
 			else
 				count = PAGE_SIZE;
 			if (index == 0) {
@@ -1154,6 +1156,22 @@ int bitmap_startwrite(struct bitmap *bitmap, sector_t offset, unsigned long sect
 			return 0;
 		}
 
+		if (unlikely((*bmc & COUNTER_MAX) == COUNTER_MAX)) {
+			DEFINE_WAIT(__wait);
+			/* note that it is safe to do the prepare_to_wait
+			 * after the test as long as we do it before dropping
+			 * the spinlock.
+			 */
+			prepare_to_wait(&bitmap->overflow_wait, &__wait,
+					TASK_UNINTERRUPTIBLE);
+			spin_unlock_irq(&bitmap->lock);
+			bitmap->mddev->queue
+				->unplug_fn(bitmap->mddev->queue);
+			schedule();
+			finish_wait(&bitmap->overflow_wait, &__wait);
+			continue;
+		}
+
 		switch(*bmc) {
 		case 0:
 			bitmap_file_set_bit(bitmap, offset);
@@ -1163,7 +1181,7 @@ int bitmap_startwrite(struct bitmap *bitmap, sector_t offset, unsigned long sect
 		case 1:
 			*bmc = 2;
 		}
-		BUG_ON((*bmc & COUNTER_MAX) == COUNTER_MAX);
+
 		(*bmc)++;
 
 		spin_unlock_irq(&bitmap->lock);
@@ -1201,6 +1219,9 @@ void bitmap_endwrite(struct bitmap *bitmap, sector_t offset, unsigned long secto
 		if (!success && ! (*bmc & NEEDED_MASK))
 			*bmc |= NEEDED_MASK;
 
+		if ((*bmc & COUNTER_MAX) == COUNTER_MAX)
+			wake_up(&bitmap->overflow_wait);
+
 		(*bmc)--;
 		if (*bmc <= 2) {
 			set_page_attr(bitmap,
@@ -1399,7 +1420,7 @@ int bitmap_create(mddev_t *mddev)
 	int err;
 	sector_t start;
 
-	BUG_ON(sizeof(bitmap_super_t) != 256);
+	BUILD_BUG_ON(sizeof(bitmap_super_t) != 256);
 
 	if (!file && !mddev->bitmap_offset) /* bitmap disabled, nothing to do */
 		return 0;
@@ -1413,6 +1434,7 @@ int bitmap_create(mddev_t *mddev)
 	spin_lock_init(&bitmap->lock);
 	atomic_set(&bitmap->pending_writes, 0);
 	init_waitqueue_head(&bitmap->write_wait);
+	init_waitqueue_head(&bitmap->overflow_wait);
 
 	bitmap->mddev = mddev;
 
diff --git a/include/linux/raid/bitmap.h b/include/linux/raid/bitmap.h
index 63df898..f21a77d 100644
--- a/include/linux/raid/bitmap.h
+++ b/include/linux/raid/bitmap.h
@@ -146,16 +146,16 @@ enum bitmap_state {
 
 /* the superblock at the front of the bitmap file -- little endian */
 typedef struct bitmap_super_s {
-	__u32 magic;        /*  0  BITMAP_MAGIC */
-	__u32 version;      /*  4  the bitmap major for now, could change... */
-	__u8  uuid[16];     /*  8  128 bit uuid - must match md device uuid */
-	__u64 events;       /* 24  event counter for the bitmap (1)*/
-	__u64 events_cleared;/*32  event counter when last bit cleared (2) */
-	__u64 sync_size;    /* 40  the size of the md device's sync range(3) */
-	__u32 state;        /* 48  bitmap state information */
-	__u32 chunksize;    /* 52  the bitmap chunk size in bytes */
-	__u32 daemon_sleep; /* 56  seconds between disk flushes */
-	__u32 write_behind; /* 60  number of outstanding write-behind writes */
+	__le32 magic;        /*  0  BITMAP_MAGIC */
+	__le32 version;      /*  4  the bitmap major for now, could change... */
+	__u8  uuid[16];      /*  8  128 bit uuid - must match md device uuid */
+	__le64 events;       /* 24  event counter for the bitmap (1)*/
+	__le64 events_cleared;/*32  event counter when last bit cleared (2) */
+	__le64 sync_size;    /* 40  the size of the md device's sync range(3) */
+	__le32 state;        /* 48  bitmap state information */
+	__le32 chunksize;    /* 52  the bitmap chunk size in bytes */
+	__le32 daemon_sleep; /* 56  seconds between disk flushes */
+	__le32 write_behind; /* 60  number of outstanding write-behind writes */
 
 	__u8  pad[256 - 64]; /* set to zero */
 } bitmap_super_t;
@@ -247,6 +247,7 @@ struct bitmap {
 
 	atomic_t pending_writes; /* pending writes to the bitmap file */
 	wait_queue_head_t write_wait;
+	wait_queue_head_t overflow_wait;
 
 };
 
commit c3e2386daf9d8a86f6fefe666abc3e496bec8396
Author: Neil Brown <neilb@suse.de>
Date:   Wed Apr 11 23:28:44 2007 -0700

    [PATCH] md: fix calculation for size of filemap_attr array in md/bitmap
    
    If 'num_pages' were ever 1 more than a multiple of 8 (32bit platforms)
    or of 16 (64 bit platforms).  filemap_attr would be allocated one
    'unsigned long' shorter than required.  We need a round-up in there.
    
    Signed-off-by: Neil Brown <neilb@suse.de>
    Cc: <stable@kernel.org>
    Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
    Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

commit 9cc70949ab61fd00dde2e1a99d4d5b6c8f7d1608
Author: Neil Brown <neilb@suse.de>
Date:   Thu Feb 8 14:20:37 2007 -0800

    [PATCH] md: avoid possible BUG_ON in md bitmap handling
    
    md/bitmap tracks how many active write requests are pending on blocks
    associated with each bit in the bitmap, so that it knows when it can clear
    the bit (when count hits zero).
    
    The counter has 14 bits of space, so if there are ever more than 16383, we
    cannot cope.
    
    Currently the code just calles BUG_ON as "all" drivers have request queue
    limits much smaller than this.
    
    However is seems that some don't.  Apparently some multipath configurations
    can allow more than 16383 concurrent write requests.
    
    So, in this unlikely situation, instead of calling BUG_ON we now wait
    for the count to drop down a bit.  This requires a new wait_queue_head,
    some waiting code, and a wakeup call.
    
    Tested by limiting the counter to 20 instead of 16383 (writes go a lot slower
    in that case...).
    
    Signed-off-by: Neil Brown <neilb@suse.de>
    Cc: <stable@kernel.org>
    Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
    Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

commit e9262a4c04b40fb3f94bac9385ee5369fe4317ed
Author: NeilBrown <neilb@suse.de>
Date:   Fri Jan 26 00:57:03 2007 -0800

    [PATCH] md: avoid reading past the end of a bitmap file
    
    In most cases we check the size of the bitmap file before reading data from
    it.  However when reading the superblock, we always read the first PAGE_SIZE
    bytes, which might not always be appropriate.  So limit that read to the size
    of the file if appropriate.
    
    Also, we get the count of available bytes wrong in one place, so that too can
    read past the end of the file.
    
    Cc: "yang yin" <yinyang801120@gmail.com>
    Signed-off-by: Neil Brown <neilb@suse.de>
    Signed-off-by: Andrew Morton <akpm@osdl.org>
    Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

commit 5c93776128122e633db9bc4b1afacf43f7e2924f
Author: NeilBrown <neilb@suse.de>
Date:   Sat Oct 21 10:24:09 2006 -0700

    [PATCH] md: endian annotations for the bitmap superblock
    
    And a couple of bug fixes found by sparse.
    
    Signed-off-by: Neil Brown <neilb@suse.de>
    Signed-off-by: Andrew Morton <akpm@osdl.org>
    Signed-off-by: Linus Torvalds <torvalds@osdl.org>

commit 63754634c067ae3c924cb4cca82572bb41b90f93
Author: Alexey Dobriyan <adobriyan@gmail.com>
Date:   Wed Oct 11 01:22:26 2006 -0700

    [PATCH] md: use BUILD_BUG_ON
    
    Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
    Acked-by: Neil Brown <neilb@suse.de>
    Signed-off-by: Andrew Morton <akpm@osdl.org>
    Signed-off-by: Linus Torvalds <torvalds@osdl.org>

Attachment: signature.asc
Description: This is a digitally signed message part.


Reply to: