Skip to contentSkip to conversation

corruption

DISCUSSION [email protected] 12 participants 28 messages Nov 29, 2000 – Dec 4, 2000 VFS

lore
Share & tools
lore
#1 lore raw
Quoted 3 lines · On Wed, 29 Nov 2000 [email protected] wrote:
Quoted 1 line

ISTR bug reports looking like that and IIRC they were never resolved.

Have you looked at the report by Daniel Phillips?

Yes. The problem is real, but the fix... I'm doing a cleanup there and I'll post the patch when I'll give it some testing.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

#2 lore raw
In reply to [email protected]
Quoted 10 lines · Lawrence Walton

my system has been acting slightly odd on all the pre 12 kernels with the fs going read only with out any messages until now. no opps or anything like that, but I did get this just now.

EXT2-fs error (device sd(8,2)): ext2_readdir:
bad entry in directory #458430: directory entry
across blocks - offset=152, inode=3393794200,
rec_len=12440, name_len=73

3393794200 == 0xca493098. A kernel address. And 152 is 0x98, which is equal to N * 0x20 + 0x18. Read on...

I am somewhat reluctant to report this problem because I always run kernels with the lowish latency patch, but having reviewed the effects of that patch on fs/*.c I don't think it's to blame. Plus it's been 100% stable for months.

I believe that the problem I've observed is caused by or exposed by the O_SYNC changes. Or maybe not.

Running test11-ac4 on *very* vanilla machines. x86 UP, IDE, 3c905 and really nothing else. No APM, fat, vfat, isofs, USB, audio, etc. It has happened on two different machines which have been 100% reliable for a year.

The problem is corruption of in-core files. It has only started happening in the past few days. It happened after two days uptime. In the most recent case my /bin/ls went bad. I took a copy and rebooted. After reboot /bin/ls had a correct MD5 sum. Here's the diff:

ls.bad renamed from ls.good +3 -3
@@ -1589,7 +1589,7 @@
 006340: C7 85 F8 BF FF FF 00 00 00 00 E9 EA 02 00 00 90      >@@@@@@@@@@@@@@@@<
 006350: 8B BD FC BF FF FF 8D B5 00 E0 FF FF 57 68 00 20      >@@@@@@@@@@@@Wh@ <
 006360: 00 00 56 E8 3C B2 FF FF 83 C4 0C 85 C0 0F 84 DD      >@@V@<@@@@@@@@@@@<
-006370: 02 00 00 6A 0A 56 E8 49 B0 FF FF 83 C4 08 85 C0      >@@@j@V@I@@@@@@@@<
+006370: 02 00 00 6A 0A 56 E8 49 78 73 62 C6 78 73 62 C6      >@@@j@V@Ixsb@xsb@<
 006380: 75 2E 8D 9D 00 C0 FF FF 8B BD FC BF FF FF 57 68      >u.@@@@@@@@@@@@Wh<
 006390: 00 20 00 00 53 E8 0A B2 FF FF 83 C4 0C 85 C0 74      >@ @@S@@@@@@@@@@t<
 0063A0: 0F 6A 0A 53 E8 1B B0 FF FF 83 C4 08 85 C0 74 D8      >@j@S@@@@@@@@@@t@<
@@ -1709,7 +1709,7 @@
 006AC0: 00 00 00 FF 75 DF 83 E8 03 40 40 2B 44 24 58 83      >@@@@u@@@@@@+D$X@<
 006AD0: C0 02 89 44 24 14 EB 08 C7 44 24 14 01 00 00 00      >@@@D$@@@@D$@@@@@<
 006AE0: 8B 4C 24 3C F6 C1 01 74 5B 8B 44 24 5C 8B 74 24      >@L$<@@@t[@D$\@t$<
-006AF0: 14 89 C2 83 E0 03 74 16 7A 0F 83 F8 02 74 05 38      >@@@@@@t@z@@@@t@8<
+006AF0: 14 89 C2 83 E0 03 74 16 F8 7A 62 C6 F8 7A 62 C6      >@@@@@@t@@zb@@zb@<
 006B00: 22 74 2F 42 38 22 74 2A 42 38 22 74 25 42 8B 02      >"t/B8"t*B8"t%B@@<
 006B10: 84 E0 75 08 84 C0 74 1A 84 E4 74 15 A9 00 00 FF      >@@u@@@t@@@t@@@@@<
 006B20: 00 74 0D 83 C2 04 A9 00 00 00 FF 75 E1 83 EA 03      >@t@@@@@@@@@u@@@@<
@@ -1733,7 +1733,7 @@
 006C40: 4C 24 54 40 51 50 E8 C9 A7 FF FF 83 C4 08 83 7C      >L$T@QP@@@@@@@@@|<
 006C50: 24 1C 00 74 38 C6 00 2C 8B 5C 24 3C 40 8B 4C 24      >$@@t8@@,@\$<@@L$<
 006C60: 3C 83 E3 01 F6 C1 02 74 0E 8B 74 24 58 56 50 E8      ><@@@@@@t@@t$XVP@<
-006C70: A0 A7 FF FF 83 C4 08 85 DB 74 12 C6 00 5F 8B 4C      >@@@@@@@@@t@@@_@L<
+006C70: A0 A7 FF FF 83 C4 08 85 78 7C 62 C6 78 7C 62 C6      >@@@@@@@@x|b@x|b@<
 006C80: 24 5C 40 51 50 E8 8A A7 FF FF 83 C4 08 C6 00 2F      >$\@QP@@@@@@@@@@/<
 006C90: 31 FF 8B 74 24 60 40 56 50 E8 76 A7 FF FF 83 C4      >1@@t$`@VP@v@@@@@<
 006CA0: 08 8B 4C 24 30 8B 29 85 ED 74 31 90 8D 74 26 00      >@@L$0@)@@t1@@t&@<

Note that in both my cases (and, apparently, Lawrence's) the corrupted data consists of two identical kernel addresses which have the value

	N * 0x20 + 0x18

and they are always equal. And they occur at a file offset of

	N * 0x20 + 0x18

Which leads one to believe that someone somewhere is doing an init_list_head() on a wild pointer.

Or, more likely, someone is doing a list_del() on a list_head which points at recycled memory, and that list_head resides within a structure at offset 0x18.

And that description perfectly matches the new i_dirty_buffers field in struct inode.

Which would perhaps indicate that one of the following statements:

- the list_del in buffer_insert_inode_queue() or

- the list_del in __remove_inode_queue()

- the list_del in fsync_inode_buffers()

has gotten itself a wild pointer.

Other possible candidates apart from i_dirty_buffers which have a list_head at offset 0x18 and whose list_dels should be reviewed are:

- request_queue.elevator.queue
- dentry.d_hash
- anything which has a timer_list at offset 0x18
- anything which has a waitqueue at offset 0x14

There may be others which have list_heads at 0x38, 0x58, ...

This doesn't just happen a single time. The first time it happened during a CVS commit at least eight files on the server ended up with this corruption, as did /usr/lib/netscape/netscape-communicator, so we had a whole bunch of corruptions happening in a short period of time.

It takes a very bad kernel bug to be able to crash netscape.

Anyway, something to be thinking about. I've written the canonical list_head debugging code. I'll run that overnight and finish it off tomorrow.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

#3 lore raw
In reply to [email protected]

In article <[email protected]>,
Andrew Morton <[email protected]> writes:

Quoted 10 lines · Lawrence Walton
Quoted 10 lines

my system has been acting slightly odd on all the pre 12 kernels with the fs going read only with out any messages until now. no opps or anything like that, but I did get this just now.

EXT2-fs error (device sd(8,2)): ext2_readdir:
bad entry in directory #458430: directory entry
across blocks - offset=152, inode=3393794200,
rec_len=12440, name_len=73

AM>
AM> 3393794200 == 0xca493098. A kernel address. And 152 is 0x98, AM> which is equal to N * 0x20 + 0x18. Read on...
AM>

Don't know what these do for your analysis, observed on 2.4.0test12pre2, compiling mozilla.

EXT2-fs error (device ide0(3,11)):
ext2_readdir: bad entry in directory #409870: directory entry across blocks
- offset=88, inode=3284439128, rec_len=36952, name_len=196

EXT2-fs error (device ide0(3,11)):
ext2_add_entry: bad entry in directory #344273: rec_len % 4 != 0 - offset=0, inode=1769234798, rec_len=28271, name_len=85

Recompiling it with 2.4.0test12pre3 last night did not cause any fs problems, at least that I've noticed.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

#4 lore raw
Quoted 19 lines · Jonathan Hudson

In article <[email protected]>,
Andrew Morton <[email protected]> writes:

Quoted 10 lines · Lawrence Walton
Quoted 10 lines

my system has been acting slightly odd on all the pre 12 kernels with the fs going read only with out any messages until now. no opps or anything like that, but I did get this just now.

EXT2-fs error (device sd(8,2)): ext2_readdir:
bad entry in directory #458430: directory entry
across blocks - offset=152, inode=3393794200,
rec_len=12440, name_len=73

AM>
AM> 3393794200 == 0xca493098. A kernel address. And 152 is 0x98, AM> which is equal to N * 0x20 + 0x18. Read on...
AM>

Quoted 6 lines

Don't know what these do for your analysis, observed on 2.4.0test12pre2, compiling mozilla.

EXT2-fs error (device ide0(3,11)):
ext2_readdir: bad entry in directory #409870: directory entry across blocks
- offset=88, inode=3284439128, rec_len=36952, name_len=196

offset 0x58, data: 58 90 c4 c3 58 90 c4

Confirms. That's definitely an empty list_head at address 0xc3c49058 and -pre2 has O_SYNC patches.

Quoted 3 lines

EXT2-fs error (device ide0(3,11)):

   ext2_add_entry: bad entry in directory #344273: rec_len % 4 != 0 - offset=0, 
   inode=1769234798, rec_len=28271, name_len=85

data: 6e 61 74 69 6f 6e 55, i.e. "nationU"

... and that one looks like a duplicated blocks effect, but there may be a lot of other reasons for that.

Quoted 2 lines

Recompiling it with 2.4.0test12pre3 last night did not cause any fs problems, at least that I've noticed.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

#5 lore raw
In reply to Jonathan Hudson
Quoted 3 lines · Alexander Viro

Confirms. That's definitely an empty list_head at address 0xc3c49058 and -pre2 has O_SYNC patches.

foo. The overnight run wedged tight in mmap002. No progress.

I bet this'll catch it:

linux/list.h renamed from linux/list.h.orig +1 -0
@@ -90,6 +90,7 @@
 static __inline__ void list_del(struct list_head *entry)
 {
 	__list_del(entry->prev, entry->next);
+	entry->next = entry->prev = 0;
 }
 
 /**

First person to send a ksymoops trace gets a cookie.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

#6 lore raw
Quoted 12 lines · Andrew Morton
Quoted 3 lines · Alexander Viro

Confirms. That's definitely an empty list_head at address 0xc3c49058 and -pre2 has O_SYNC patches.

foo. The overnight run wedged tight in mmap002. No progress.

I bet this'll catch it:

linux/list.h line 95

Quoted 6 lines
linux/list.h renamed from linux/list.h.orig +1 -0
@@ -90,6 +90,7 @@
 static __inline__ void list_del(struct list_head *entry)
 {
 	__list_del(entry->prev, entry->next);
+	entry->next = entry->prev = 0;
 }
 

Or just call list_del_init instead...

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

#7 lore raw
In reply to Jonathan Hudson
Quoted 14 lines · Andrew Morton

I bet this'll catch it:

linux/list.h Fri Dec 1 08:33:55 2000 renamed from linux/list.h.orig Fri Dec 1 08:33:36 2000 +1 -0
@@ -90,6 +90,7 @@
 static __inline__ void list_del(struct list_head *entry)
 {
        __list_del(entry->prev, entry->next);
+       entry->next = entry->prev = 0;
 }
 
 /**
Quoted 1 line

First person to send a ksymoops trace gets a cookie.

mmmm... choc-chip.

With the above patch applied the machine crashed after an hour. Crashed a second time during the e2fsck. gdb backtrace:

(gdb) l *0xc01755d4
0xc01755d4 is in __make_request (ll_rw_blk.c:744).
739 * skip first entry, for devices with active queue head
740 */
741 if (q->head_active && !q->plugged)
742 head = head->next;
743
744 if (list_empty(head)) {
745 q->plug_device_fn(q, bh->b_rdev); /* is atomic */
746 goto get_rq;
747 }
748
(gdb) l *0xc0175c3d
0xc0175c3d is in generic_make_request (ll_rw_blk.c:882).
877 buffer_IO_error(bh);
878 break;
879 }
880
881 }
882 while (q->make_request_fn(q, rw, bh));
883 }
884
885 /* This function can be used to request a number of buffers from a block
886 device. Currently the only restriction is that all buffers must belong to
(gdb) l *0xc0175da1
0xc0175da1 is in ll_rw_block (ll_rw_blk.c:924).
919 printk(KERN_NOTICE "Can't write to read-only device %s\n",
920 kdevname(bhs[0]->b_dev));
921 goto sorry;
922 }
923
924 for (i = 0; i < nr; i++) {
925 bh = bhs[i];
926
927 /* Only one thread can actually submit the I/O. */
928 if (test_and_set_bit(BH_Lock, &bh->b_state))
(gdb) l *0xc01309e9
0xc01309e9 is in sync_buffers (/uhome/morton/akpm/linux/include/asm/atomic.h:65).
60 :"=m" (v->counter)
61 :"m" (v->counter));
62 }
63
64 static __inline__ void atomic_dec(atomic_t *v)
65 {
66 __asm__ __volatile__(
67 LOCK "decl %0"
68 :"=m" (v->counter)
69 :"m" (v->counter));
(gdb) l *0xc0130af7
0xc0130af7 is in fsync_dev (/uhome/morton/akpm/linux/include/asm/current.h:9).
4 struct task_struct;
5
6 static inline struct task_struct * get_current(void)
7 {
8 struct task_struct *current;
9 __asm__("andl %%esp,%0; ":"=r" (current) : "0" (~8191UL));
10 return current;
11 }
12
13 #define current get_current()
(gdb) l *0xc0137339
0xc0137339 is in block_fsync (block_dev.c:353).
348 * since the vma has no handle.
349 */
350
351 static int block_fsync(struct file *filp, struct dentry *dentry, int datasync)
352 {
353 return fsync_dev(dentry->d_inode->i_rdev);
354 }
355
356 /*
357 * bdev cache handling - shamelessly stolen from inode.c
(gdb) l *0xc0130c76
0xc0130c76 is in sys_fsync (buffer.c:373).
368 if (!file->f_op || !file->f_op->fsync)
369 goto out_putf;
370
371 /* We need to protect against concurrent writers.. */
372 down(&inode->i_sem);
373 err = file->f_op->fsync(file, dentry, 0);
374 up(&inode->i_sem);
375
376 out_putf:
377 fput(file);
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

#8 lore raw
In reply to Jonathan Hudson
Quoted 13 lines · Andrew Morton
Quoted 12 lines · Andrew Morton

I bet this'll catch it:

linux/list.h Fri Dec 1 08:33:55 2000 renamed from linux/list.h.orig Fri Dec 1 08:33:36 2000 +1 -0
@@ -90,6 +90,7 @@
 static __inline__ void list_del(struct list_head *entry)
 {
        __list_del(entry->prev, entry->next);
+       entry->next = entry->prev = 0;
 }
 
Quoted 8 lines
Quoted 3 lines
linux/list.h Fri Dec 1 08:33:55 2000 renamed from linux/list.h.orig Fri Dec 1 08:33:36 2000 +0 -0
 /**

First person to send a ksymoops trace gets a cookie.

mmmm... choc-chip.

With the above patch applied the machine crashed after an hour. Crashed a second time during the e2fsck. gdb backtrace:

Very interesting. IDE / SCSI?

Signature · 6 lines
* Jens Axboe <[email protected]>
* SuSE Labs
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/
#9 lore raw
In reply to Jonathan Hudson
Quoted 14 lines · Andrew Morton
Quoted 12 lines · Andrew Morton

I bet this'll catch it:

linux/list.h Fri Dec 1 08:33:55 2000 renamed from linux/list.h.orig Fri Dec 1 08:33:36 2000 +1 -0
@@ -90,6 +90,7 @@
 static __inline__ void list_del(struct list_head *entry)
 {
        __list_del(entry->prev, entry->next);
+       entry->next = entry->prev = 0;
 }
 
Quoted 9 lines
Quoted 3 lines
linux/list.h Fri Dec 1 08:33:55 2000 renamed from linux/list.h.orig Fri Dec 1 08:33:36 2000 +0 -0
 /**

First person to send a ksymoops trace gets a cookie.

mmmm... choc-chip.

With the above patch applied the machine crashed after an hour. Crashed a second time during the e2fsck. gdb backtrace:

This sync_buffers corruption is fixed by Jens' patch, which is present in test12-pre3.

However it is possible that the original list_head-based corruption which I reported will not be fixed by this patch. This is because none of the structure offsets match up with what I observed, and because Lawrence's sytem is "SCSI-only" - no SCSI drivers are headactive.

Lawrence, did you see this problem with test12-pre3?
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

#10 lore raw
In reply to Jonathan Hudson
Quoted 17 lines · Jens Axboe
Quoted 15 lines · Andrew Morton
Quoted 14 lines · Andrew Morton

I bet this'll catch it:

linux/list.h Fri Dec 1 08:33:55 2000 renamed from linux/list.h.orig Fri Dec 1 08:33:36 2000 +1 -0
@@ -90,6 +90,7 @@
 static __inline__ void list_del(struct list_head *entry)
 {
        __list_del(entry->prev, entry->next);
+       entry->next = entry->prev = 0;
 }
 
 /**
Quoted 8 lines
Quoted 6 lines
Quoted 1 line

First person to send a ksymoops trace gets a cookie.

mmmm... choc-chip.

With the above patch applied the machine crashed after an hour. Crashed a second time during the e2fsck. gdb backtrace:

Very interesting. IDE / SCSI?

hmm.. Overlapping emails.

The crash with e2fsck was easily repeatable with the above patch. Just dirty a few buffers and run /sbin/sync. It's due to the __make_request queue_head thing which you fixed in test12-pre3. Yes, this was IDE.

However the original problem of a list_del being performed on a wild pointer is being seen on SCSI systems. I expect the above patch will catch it if it's still happening.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

#11 lore raw
In reply to Jonathan Hudson
Quoted 5 lines · Andrew Morton
Quoted 5 lines
Quoted 4 lines

mmmm... choc-chip.

With the above patch applied the machine crashed after an hour. Crashed a second time during the e2fsck. gdb backtrace:

Quoted 7 lines
Quoted 1 line

Very interesting. IDE / SCSI?

hmm.. Overlapping emails.

The crash with e2fsck was easily repeatable with the above patch. Just dirty a few buffers and run /sbin/sync. It's due to the __make_request queue_head thing which you fixed in test12-pre3. Yes, this was IDE.

Ah ok, I thought this was on test12-pre3.

Quoted 3 lines

However the original problem of a list_del being performed on a wild pointer is being seen on SCSI systems. I expect the above patch will catch it if it's still happening.

Indeed, and I don't think it's request queue_head related anymore. I will look forward to seeing a trace, though :-)

Signature · 6 lines
* Jens Axboe <[email protected]>
* SuSE Labs
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/
#12 lore raw
In reply to Jonathan Hudson

Hi,

Review

Quoted 8 lines · Andrew Morton

I bet this'll catch it:

static __inline__ void list_del(struct list_head *entry) {

(quoted diff) +1 -0
 	__list_del(entry->prev, entry->next);
+	entry->next = entry->prev = 0;
 }

No, because the buffer hash list is never referenced unless buffer->b_inode is non-null, so we can't ever do a double-list_del on the buffer.

The patch below should fix it. It has been sent to Linus. The important part is the first hunk of the inode.c diff.

Cheers,
Stephen

fsync-fix2.diff:

buffer.c renamed from buffer.c.~1~ +4 -2
@@ -871,10 +871,11 @@
 		else {
 			bh->b_inode = &tmp;
 			list_add(&bh->b_inode_buffers, &tmp.i_dirty_buffers);
-			atomic_inc(&bh->b_count);
 			if (buffer_dirty(bh)) {
+				atomic_inc(&bh->b_count);
 				spin_unlock(&lru_list_lock);
 				ll_rw_block(WRITE, 1, &bh);
+				brelse(bh);
 				spin_lock(&lru_list_lock);
 			}
 		}
@@ -883,6 +884,7 @@
 	while (!list_empty(&tmp.i_dirty_buffers)) {
 		bh = BH_ENTRY(tmp.i_dirty_buffers.prev);
 		remove_inode_queue(bh);
+		atomic_inc(&bh->b_count);
 		spin_unlock(&lru_list_lock);
 		wait_on_buffer(bh);
 		if (!buffer_uptodate(bh))
@@ -929,9 +931,9 @@
 			atomic_inc(&bh->b_count);
 			spin_unlock(&lru_list_lock);
 			wait_on_buffer(bh);
-			brelse(bh);
 			if (!buffer_uptodate(bh))
 				err = -EIO;
+			brelse(bh);
 			spin_lock(&lru_list_lock);
 			goto repeat;
 		}
inode.c renamed from inode.c.~1~ +14 -1
@@ -77,7 +77,13 @@
 
 #define alloc_inode() \
 	 ((struct inode *) kmem_cache_alloc(inode_cachep, SLAB_KERNEL))
-#define destroy_inode(inode) kmem_cache_free(inode_cachep, (inode))
+static void destroy_inode(struct inode *inode) 
+{
+	if (!list_empty(&inode->i_dirty_buffers))
+		BUG();
+	kmem_cache_free(inode_cachep, (inode));
+}
+
 
 /*
  * These are initializations that only need to be done
@@ -348,6 +354,12 @@
  
 void clear_inode(struct inode *inode)
 {
+	if (!list_empty(&inode->i_dirty_buffers)) {
+		if (inode->i_nlink)
+			BUG();
+		invalidate_inode_buffers(inode);
+	}
+       
 	if (inode->i_data.nrpages)
 		BUG();
 	if (!(inode->i_state & I_FREEING))
@@ -407,6 +419,7 @@
 		inode = list_entry(tmp, struct inode, i_list);
 		if (inode->i_sb != sb)
 			continue;
+		invalidate_inode_buffers(inode);
 		if (!atomic_read(&inode->i_count)) {
 			list_del(&inode->i_hash);
 			INIT_LIST_HEAD(&inode->i_hash);

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

#13 lore raw
In reply to Jonathan Hudson
Quoted 29 lines · Andrew Morton [[email protected]] wrote:
Quoted 14 lines · Andrew Morton
Quoted 12 lines · Andrew Morton

I bet this'll catch it:

linux/list.h Fri Dec 1 08:33:55 2000 renamed from linux/list.h.orig Fri Dec 1 08:33:36 2000 +1 -0
@@ -90,6 +90,7 @@
 static __inline__ void list_del(struct list_head *entry)
 {
        __list_del(entry->prev, entry->next);
+       entry->next = entry->prev = 0;
 }
 
Quoted 9 lines
Quoted 3 lines
linux/list.h Fri Dec 1 08:33:55 2000 renamed from linux/list.h.orig Fri Dec 1 08:33:36 2000 +0 -0
 /**

First person to send a ksymoops trace gets a cookie.

mmmm... choc-chip.

With the above patch applied the machine crashed after an hour. Crashed a second time during the e2fsck. gdb backtrace:

This sync_buffers corruption is fixed by Jens' patch, which is present in test12-pre3.

Quoted 6 lines

However it is possible that the original list_head-based corruption which I reported will not be fixed by this patch. This is because none of the structure offsets match up with what I observed, and because Lawrence's system is "SCSI-only" - no SCSI drivers are headactive.

Lawrence, did you see this problem with test12-pre3?

Yes all of my current problems are with pre3;
Is there anything you would like me to test
specific?
The best way to cause problems so far is
compiling mozilla.

Signature · 12 lines
*--* Mail: [email protected]
*--* Voice: 425.739.4247
*--* Fax: 425.827.9577
*--* HTTP://www.otak-k.com/~lawrence/
--------------------------------------
- - - - - - O t a k  i n c . - - - - - 


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/
#14 lore raw
In reply to Jonathan Hudson
Quoted 13 lines · Stephen C. Tweedie

Hi,

Quoted 8 lines · Andrew Morton

I bet this'll catch it:

static __inline__ void list_del(struct list_head *entry)

 {
      __list_del(entry->prev, entry->next);

+ entry->next = entry->prev = 0;
}

Quoted 3 lines

No, because the buffer hash list is never referenced unless buffer->b_inode is non-null, so we can't ever do a double-list_del on the buffer.

You are right. An overnight diskthrash with the above patch didn't oops, but it turned up three instances of the EXT2 directory warnings.

Incidentally, there's something wrong in blockdev/VM land. The overnight run consisted of:

- one looping instance of `dbench 4 ; sleep 120'
- one looping instance of 'lmbench ; sleep 120'
- one looping instance of `bonnie++ ; sleep 120'
- one looping instance of `mmap001;mmap002;misc001;sleep 120'

Things which went wrong (after ten hours):

- the first dbench run never completed.

- the first bonnie++ run never completed.

- I then killed everything with ALT-SYSRQ-T. It's been 20 minutes now and the disk LED is *still* hard on. This machine has 256 megs and the hdparm disk bandwidth is 20 megs/sec.

You can observe the latter problem pretty easily by running `misc001' on its own. Kill it after 20 seconds and the disk remains active for *ages*. Usually ninety seconds. Long enough to write all physical memory out ten times...

Quoted 2 lines

The patch below should fix it. It has been sent to Linus. The important part is the first hunk of the inode.c diff.

Okay, will test. (25 minutes now. It's fsck time...)
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

#15 lore raw

Hi, Andrew,

Quoted 8 lines · Andrew Morton

- I then killed everything with ALT-SYSRQ-T. It's been 20 minutes now and the disk LED is *still* hard on. This machine has 256 megs and the hdparm disk bandwidth is 20 megs/sec.

You can observe the latter problem pretty easily by running `misc001' on its own. Kill it after 20 seconds and the disk remains active for *ages*. Usually ninety seconds. Long enough to write all physical memory out ten times...

If the benchmark writes blocks completely random, the random writing performance should be used instead of the bandwidth. It is tipically 30blk/s to 200blk/s depends on the seek speed.

90 second writing corresponds to 10MB to 72MB dirty buffer (4K fs block), it is not so crazy. But 30 minutes is still too long on 256MB physical memory.

I think they are different, aren't they?

--
Computer Systems Laboratory, Fujitsu Labs. [email protected]
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

#16 lore raw
In reply to Jonathan Hudson
Quoted 13 lines · Stephen C. Tweedie

Hi,

Quoted 8 lines · Andrew Morton

I bet this'll catch it:

static __inline__ void list_del(struct list_head *entry)

 {
      __list_del(entry->prev, entry->next);

+ entry->next = entry->prev = 0;
}

Quoted 6 lines

No, because the buffer hash list is never referenced unless buffer->b_inode is non-null, so we can't ever do a double-list_del on the buffer.

The patch below should fix it. It has been sent to Linus. The important part is the first hunk of the inode.c diff.

Testing test11-pre3 with your inode.c patch and the above list_del patch. x86 dual processor, IDE. Same workload as before, except I cut out misc001 (and the machine recovered almost immediately when I killed everything! Need more testing to characterise this).

kernel BUG at inode.c:83! The trace is below. Now, this was probably triggered by my list_del change. If so it means that we're doing a list_empty() test on a list_head which has actually been deleted from a list. So it's possibly the actual assertion in destroy_inode() which is a little bogus.

But the wierd thing is that this BUG only hit a single time, after three hours of intensive testing. If my theory is
right, the BUG should hit every time. Will investigate further...

ksymoops 0.7c on i686 2.4.0-test12-pre3. Options used
-V (default)
-k /proc/ksyms (default)
-l /proc/modules (default)
-o /lib/modules/2.4.0-test12-pre3/ (default) -m /usr/src/linux/System.map (default)

Warning: You did not tell me where to find symbol information. I will assume that the log matches the kernel and modules that are running right now and I'll use the default options above for symbol resolution. If the current kernel and/or modules do not match the log, you can get more accurate output by telling me the kernel version and where to find map, modules, ksyms etc. ksymoops -h explains the options.

Reading Oops report from the terminal

Dec  2 13:15:45 mnm kernel: invalid operand: 0000
Dec  2 13:15:45 mnm kernel: CPU:    0
Dec  2 13:15:45 mnm kernel: EIP:    0010:[<c014570e>]
Using defaults from ksymoops -t elf32-i386 -a i386
Dec  2 13:15:45 mnm kernel: EFLAGS: 00010282
Dec  2 13:15:45 mnm kernel: eax: 0000001a   ebx: c78686e0   ecx: 00000000   edx: 0000002f
Dec  2 13:15:45 mnm kernel: esi: c025b800   edi: cd950960   ebp: c7869160   esp: ce611f3c
Dec  2 13:15:45 mnm kernel: ds: 0018   es: 0018   ss: 0018
Dec  2 13:15:45 mnm kernel: Process dbench (pid: 13094, stackpage=ce611000)
Dec  2 13:15:45 mnm kernel: Stack: c021b7e5 c021b8a5 00000053 c78686e0 c0146916 c78686e0 c7869160 c78686e0
Dec  2 13:15:45 mnm kernel:        c0145096 c78686e0 00000000 ce610000 c013de4d c7869160 c7869160 c9b1e000
Dec  2 13:15:45 mnm kernel:        ce611fa4 c7869160 cd9509d0 c013df25 cd950960 c7869160 ce610000 bffff5ca
Dec  2 13:15:45 mnm kernel: Call Trace: [<c021b7e5>] [<c021b8a5>] [<c0146916>] [<c0145096>] [<c013de4d>] [<c013df25>
Dec  2 13:15:45 mnm kernel:        [<c010002b>]
Dec  2 13:15:45 mnm kernel: Code: 0f 0b 83 c4 0c 53 a1 10 d1 2a c0 50 e8 cd 3d fe ff 83 c4 08
Quoted 1 line
Quoted 1 line

EIP; c014570e <destroy_inode+1e/34> <=====

Trace; c021b7e5 <tvecs+5a3d/1a3d8>
Trace; c021b8a5 <tvecs+5afd/1a3d8>
Trace; c0146916 <iput+18e/194>
Trace; c0145096 <d_delete+66/ac>
Trace; c013de4d <vfs_unlink+18d/1c0>
Trace; c013df25 <sys_unlink+a5/118>
Trace; c010002b <startup_32+2b/cb>
Code; c014570e <destroy_inode+1e/34>
00000000 <_EIP>:
Code; c014570e <destroy_inode+1e/34> <=====
0: 0f 0b ud2a <=====
Code; c0145710 <destroy_inode+20/34>
2: 83 c4 0c add $0xc,%esp
Code; c0145713 <destroy_inode+23/34>
5: 53 push %ebx
Code; c0145714 <destroy_inode+24/34>
6: a1 10 d1 2a c0 mov 0xc02ad110,%eax
Code; c0145719 <destroy_inode+29/34>
b: 50 push %eax
Code; c014571a <destroy_inode+2a/34>
c: e8 cd 3d fe ff call fffe3dde <_EIP+0xfffe3dde> c01294ec <kmem_cache_free+0/7c>
Code; c014571f <destroy_inode+2f/34>
11: 83 c4 08 add $0x8,%esp

1 warning issued. Results may not be reliable.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

#17 lore raw
In reply to Jonathan Hudson
Quoted 33 lines · Andrew Morton
Quoted 13 lines · Stephen C. Tweedie

Hi,

Quoted 8 lines · Andrew Morton

I bet this'll catch it:

static __inline__ void list_del(struct list_head *entry)

 {
      __list_del(entry->prev, entry->next);

+ entry->next = entry->prev = 0;
}

Quoted 6 lines

No, because the buffer hash list is never referenced unless buffer->b_inode is non-null, so we can't ever do a double-list_del on the buffer.

The patch below should fix it. It has been sent to Linus. The important part is the first hunk of the inode.c diff.

Testing test11-pre3 with your inode.c patch and the above list_del patch. x86 dual processor, IDE. Same workload as before, except I cut out misc001 (and the machine recovered almost immediately when I killed everything! Need more testing to characterise this).

kernel BUG at inode.c:83! The trace is below. Now, this was probably triggered by my list_del change. If so it means that we're doing a list_empty() test on a list_head which has actually been deleted from a list. So it's possibly the actual assertion in destroy_inode() which is a little bogus.

Quoted 4 lines

But the wierd thing is that this BUG only hit a single time, after three hours of intensive testing. If my theory is
right, the BUG should hit every time. Will investigate further...

It appears that this problem is not fixed.

My destroy_inode() is now

static void destroy_inode(struct inode *inode)

{
        if (!list_empty(&inode->i_dirty_buffers)) {
                printk("&inode->i_dirty_buffers=0x%p\n", &inode->i_dirty_buffers);
                printk("next=0x%p\n", inode->i_dirty_buffers.next);
                printk("prev=0x%p\n", inode->i_dirty_buffers.prev);
                BUG();
        }
        kmem_cache_free(inode_cachep, (inode));
}

After 45 minutes of running the previously described tests:

Dec  2 16:14:30 mnm kernel: &inode->i_dirty_buffers=0xcfe16878
Dec  2 16:14:30 mnm kernel: next=0xc16f3678
Dec  2 16:14:30 mnm kernel: prev=0xc16f3678
Dec  2 16:14:30 mnm kernel: kernel BUG at inode.c:86!

We're throwing away an inode which has live data on its dirty buffers list.

This is 2.4.0-test11-pre3 + list_del patch + sct's inode patch (buffer.c, inode.c). x86 dual processor. gcc 2.91.66. I rediffed my tree. No rogue patches.

It's not an SMP thing - the address patterns match up with the previously reported uniprocessor corruption.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

#18 lore raw
Quoted 1 line · Andrew Morton

It appears that this problem is not fixed.

Sure, it isn't. Place where the shit hits the fan: fs/buffer.c::unmap_buffer(). Add the call of remove_inode_queue(bh) there and see if it helps. I.e.

ed fs/buffer.c <<EOF
/unmap_buffer/
/}/i

		remove_inode_queue(bh);

.
wq
EOF

Linus, could you apply that? We are leaving the unmapped buffers on the inode queue. I.e. every truncate_inode_pages() leaves a lot of junk around. Now, guess what happens when we destroy the last link to inode that nobody keeps open...

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

#19 lore raw
In reply to Andrew Morton
Quoted 11 lines · Alexander Viro
Quoted 1 line · Andrew Morton

It appears that this problem is not fixed.

Sure, it isn't. Place where the shit hits the fan: fs/buffer.c::unmap_buffer(). Add the call of remove_inode_queue(bh) there and see if it helps. I.e.

ed fs/buffer.c <<EOF
/unmap_buffer/
/}/i

		remove_inode_queue(bh);
Quoted 8 lines

.
wq
EOF

Linus, could you apply that? We are leaving the unmapped buffers on the inode queue. I.e. every truncate_inode_pages() leaves a lot of junk around. Now, guess what happens when we destroy the last link to inode that nobody keeps open...

Nothing new (was it meant to run remove_inode_queue() conditionaly inside buffer_mapped() branch? ed did it that way). First is list of buffers at time of destroy_inode, then process. If you want full oops trace, it is available at http://platan.vc.cvut.cz/oops3.txt, but last part is always iput. For now I'm back on test9, as I lost inetd.conf again :-( Someone should shoot sendmail Debian maintainer... Running update-inetd at startup is really bad idea, as fsck then usually removes both old and new inetd.conf, so I'm back on inetd.conf from 25 Aug 1999 :-(((

Fields printed from buffer_head are b_next, b_blocknr, b_size, b_list, b_count, b_state and b_inode. (oops now I see that I left remove_inode_queue(bh) in printing loop (I copied it from invalidate_inode_buffers()), but it should not hurt, I believe. Dirty buffers should find its way to disk anyway, or not?)
Best regards,
Petr Vandrovec [email protected]

BTW, running 'ksymoops < oops > oops.txt' is great source of errors below, as it (probably) uses couple of unlinked temorary files...

next=00000000, nr=2457654, size=4096, list=2
count=0
state=0000001B inode=c7bf3ce0

Process mount (pid: 30, stackpage=c7df3000)

next=00000000, nr=2457654, size=4096, list=2
count=0
state=0000001B inode=c7bf3ce0

Process mount (pid: 31, stackpage=c7df3000)

next=00000000, nr=2457654, size=4096, list=2
count=0
state=0000001B inode=c7c1a860

Process mount (pid: 68, stackpage=c7997000)

next=00000000, nr=2457654, size=4096, list=2
count=0
state=0000001B inode=c7b54c80

Process mount (pid: 70, stackpage=c7997000)

next=00000000, nr=2457654, size=4096, list=2
count=0
state=0000001B inode=c7b54c80

Process mount (pid: 70, stackpage=c7997000)

next=00000000, nr=3375109, size=4096, list=2
count=0
state=0000001B inode=c77e2260

Process rm (pid: 82, stackpage=c7b35000)

next=00000000, nr=3506180, size=4096, list=2
count=0
state=0000001B inode=c77eb9c0

Process rm (pid: 121, stackpage=c776d000)

next=00000000, nr=3506180, size=4096, list=2
count=0
state=0000001B inode=c77ebb40
next=00000000, nr=3507147, size=4096, list=2
count=0
state=0000001B inode=c77ebb40

Process rm (pid: 122, stackpage=c776d000)

next=00000000, nr=1179657, size=4096, list=2
count=0
state=0000001B inode=c77ebb40

Process rm (pid: 123, stackpage=c776d000)

next=00000000, nr=294919, size=4096, list=2
count=0
state=0000001B inode=c77eb240

Process rm (pid: 129, stackpage=c775d000)

next=00000000, nr=294919, size=4096, list=2
count=0
state=0000001B inode=c77eb0c0

Process rm (pid: 130, stackpage=c775d000)

next=00000000, nr=2457654, size=4096, list=2
count=0
state=0000001B inode=c7889c60

Process mv (pid: 138, stackpage=c7985000)

next=c796fce0, nr=294916, size=4096, list=2
count=0
state=0000001B inode=c77eb6c0

Process rm (pid: 142, stackpage=c7721000)

next=00000000, nr=2457654, size=4096, list=2
count=0
state=0000001B inode=c75e83a0

Process update-inetd (pid: 273, stackpage=c715d000)

next=00000000, nr=2457654, size=4096, list=2
count=0
state=0000001B inode=c7b359a0

Process rm (pid: 305, stackpage=c6b33000)

next=00000000, nr=2457654, size=4096, list=2
count=0
state=0000001B inode=c7b356a0

Process rm (pid: 305, stackpage=c6b33000)

next=c6ed8980, nr=753674, size=4096, list=2
count=0
state=0000001B inode=c655a3a0

Process mc (pid: 330, stackpage=c6703000)

next=00000000, nr=196617, size=4096, list=2
count=0
state=0000001B inode=c656c4e0

Process mc (pid: 330, stackpage=c6703000)
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

#20 lore raw
Quoted 3 lines · Petr Vandrovec

Nothing new (was it meant to run remove_inode_queue() conditionaly inside buffer_mapped() branch? ed did it that way). First is list of buffers at time of destroy_inode, then process. If you want full oops trace, it is

what list of buffers? ->i_dirty_buffers one?

Quoted 11 lines

available at http://platan.vc.cvut.cz/oops3.txt, but last part is always iput. For now I'm back on test9, as I lost inetd.conf again :-( Someone should shoot sendmail Debian maintainer... Running update-inetd at startup is really bad idea, as fsck then usually removes both old and new inetd.conf, so I'm back on inetd.conf from 25 Aug 1999 :-(((

Fields printed from buffer_head are b_next, b_blocknr, b_size, b_list, b_count, b_state and b_inode. (oops now I see that I left remove_inode_queue(bh) in printing loop (I copied it from invalidate_inode_buffers()), but it should not hurt, I believe. Dirty buffers should find its way to disk anyway, or not?)

When you delete the inode? Why would they? Petr, could you send me the diff between the variant you've run and pristine 12-pre3? I'ld really like to see what exactly was doing the printks...

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

#21 lore raw
In reply to Petr Vandrovec

On Sat, 2 Dec 2000, Petr Vandrovec wrote:

[I wrote:]

Quoted 3 lines
Quoted 3 lines

ed fs/buffer.c <<EOF
/unmap_buffer/
/}/i

		spin_lock(&lru_list_lock);
Quoted 1 line
Quoted 1 line
		remove_inode_queue(bh);
		spin_unlock(&lru_list_lock);
Quoted 3 lines
Quoted 3 lines

.
wq
EOF

Damn. I claim the sudden idiocy attack - didn't look at the locking rules for the ->b_inode_buffers. My apologies.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

#22 lore raw
In reply to Petr Vandrovec

In article <[email protected]>,
Alexander Viro <[email protected]> writes:
AV>

Quoted 3 lines
Quoted 3 lines
Quoted 3 lines

ed fs/buffer.c <<EOF
/unmap_buffer/
/}/i

AV> spin_lock(&lru_list_lock);

Quoted 1 line
Quoted 1 line
Quoted 1 line
		remove_inode_queue(bh);

AV> spin_unlock(&lru_list_lock);

Quoted 3 lines
Quoted 3 lines
Quoted 3 lines

.
wq
EOF

AV>

I applied this on top the the previous SCT patch, and have thrashed the system harder than I would have dared previously. It's still running. I feel very comfortable with this, much more so than any prior 2.4.0t*.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

#23 lore raw
In reply to Andrew Morton
Quoted 8 lines · Alexander Viro
Quoted 1 line · Andrew Morton

It appears that this problem is not fixed.

Sure, it isn't. Place where the shit hits the fan: fs/buffer.c::unmap_buffer(). Add the call of remove_inode_queue(bh) there and see if it helps. I.e.

Quoted 8 lines

ed fs/buffer.c <<EOF
/unmap_buffer/
/}/i

                remove_inode_queue(bh);

.
wq
EOF

Sorry, it's still failing. It took three hours.

&inode->i_dirty_buffers=0xca9e63f8
next=0xc30a2598
prev=0xc30a2598
kernel BUG at inode.c:86!

The ksymoops output is here, as is my current diff wrt test12-pre3.

ksymoops 0.7c on i686 2.4.0-test12-pre3. Options used
-V (default)
-k /proc/ksyms (default)
-l /proc/modules (default)
-o /lib/modules/2.4.0-test12-pre3/ (default) -m /usr/src/linux/System.map (default)

Warning: You did not tell me where to find symbol information. I will assume that the log matches the kernel and modules that are running right now and I'll use the default options above for symbol resolution. If the current kernel and/or modules do not match the log, you can get more accurate output by telling me the kernel version and where to find map, modules, ksyms etc. ksymoops -h explains the options.

Reading Oops report from the terminal

Dec  4 01:56:02 mnm kernel: EIP:    0010:[<c0145758>]
Using defaults from ksymoops -t elf32-i386 -a i386
Dec  4 01:56:02 mnm kernel: EFLAGS: 00010282
Dec  4 01:56:02 mnm kernel: eax: 0000001a   ebx: ca9e63e0   ecx: 00000001   edx: 00000000
Dec  4 01:56:02 mnm kernel: esi: c025b8a0   edi: cfbcb040   ebp: ce2b0260   esp: ced4df3c
Dec  4 01:56:02 mnm kernel: ds: 0018   es: 0018   ss: 0018
Dec  4 01:56:02 mnm kernel: Process lat_fs (pid: 17559, stackpage=ced4d000)                                         Dec  4 01:56:02 mnm kernel: Stack: c021b845 c021b939 00000056 ca9e63e0 c0146966 ca9e63e0 ce2b0260 ca9e63e0
Dec  4 01:56:02 mnm kernel: Call Trace: [<c021b845>] [<c021b939>] [<c0146966>] [<c01450b6>] [<c013de6d>] [<c013df45>] [<c0108fdf>]
Dec  4 01:56:02 mnm kernel: Code: 0f 0b 83 c4 0c 8d 76 00 53 a1 10 d1 2a c0 50 e8 80 3d fe ff
Quoted 1 line
Quoted 1 line

EIP; c0145758 <destroy_inode+48/64> <=====

Trace; c021b845 <tvecs+5a3d/1a418>
Trace; c021b939 <tvecs+5b31/1a418>
Trace; c0146966 <iput+18e/194>
Trace; c01450b6 <d_delete+66/ac>
Trace; c013de6d <vfs_unlink+18d/1c0>
Trace; c013df45 <sys_unlink+a5/118>
Trace; c0108fdf <system_call+33/38>
Code; c0145758 <destroy_inode+48/64>
00000000 <_EIP>:
Code; c0145758 <destroy_inode+48/64> <=====
0: 0f 0b ud2a <=====
Code; c014575a <destroy_inode+4a/64>
2: 83 c4 0c add $0xc,%esp
Code; c014575d <destroy_inode+4d/64>
5: 8d 76 00 lea 0x0(%esi),%esi
Code; c0145760 <destroy_inode+50/64>
8: 53 push %ebx
Code; c0145761 <destroy_inode+51/64>
9: a1 10 d1 2a c0 mov 0xc02ad110,%eax
Code; c0145766 <destroy_inode+56/64>
e: 50 push %eax
Code; c0145767 <destroy_inode+57/64>
f: e8 80 3d fe ff call fffe3d94 <_EIP+0xfffe3d94> c01294ec <kmem_cache_free+0/7c>

1 warning issued. Results may not be reliable.

include/linux/list.h +1 -0
@@ -90,6 +90,7 @@
 static __inline__ void list_del(struct list_head *entry)
 {
 	__list_del(entry->prev, entry->next);
+	entry->next = entry->prev = 0;
 }
 
 /**
fs/buffer.c +7 -2
@@ -871,10 +871,11 @@
 		else {
 			bh->b_inode = &tmp;
 			list_add(&bh->b_inode_buffers, &tmp.i_dirty_buffers);
-			atomic_inc(&bh->b_count);
 			if (buffer_dirty(bh)) {
+				atomic_inc(&bh->b_count);
 				spin_unlock(&lru_list_lock);
 				ll_rw_block(WRITE, 1, &bh);
+				brelse(bh);
 				spin_lock(&lru_list_lock);
 			}
 		}
@@ -883,6 +884,7 @@
 	while (!list_empty(&tmp.i_dirty_buffers)) {
 		bh = BH_ENTRY(tmp.i_dirty_buffers.prev);
 		remove_inode_queue(bh);
+		atomic_inc(&bh->b_count);
 		spin_unlock(&lru_list_lock);
 		wait_on_buffer(bh);
 		if (!buffer_uptodate(bh))
@@ -929,9 +931,9 @@
 			atomic_inc(&bh->b_count);
 			spin_unlock(&lru_list_lock);
 			wait_on_buffer(bh);
-			brelse(bh);
 			if (!buffer_uptodate(bh))
 				err = -EIO;
+			brelse(bh);
 			spin_lock(&lru_list_lock);
 			goto repeat;
 		}
@@ -1459,6 +1461,9 @@
 		clear_bit(BH_Mapped, &bh->b_state);
 		clear_bit(BH_Req, &bh->b_state);
 		clear_bit(BH_New, &bh->b_state);
+		spin_lock(&lru_list_lock);
+		remove_inode_queue(bh);
+		spin_unlock(&lru_list_lock);
 	}
 }
 
fs/inode.c +18 -1
@@ -77,7 +77,17 @@
 
 #define alloc_inode() \
 	 ((struct inode *) kmem_cache_alloc(inode_cachep, SLAB_KERNEL))
-#define destroy_inode(inode) kmem_cache_free(inode_cachep, (inode))
+static void destroy_inode(struct inode *inode) 
+{
+	if (!list_empty(&inode->i_dirty_buffers)) {
+		printk("&inode->i_dirty_buffers=0x%p\n", &inode->i_dirty_buffers);
+		printk("next=0x%p\n", inode->i_dirty_buffers.next);
+		printk("prev=0x%p\n", inode->i_dirty_buffers.prev);
+		BUG();
+	}
+	kmem_cache_free(inode_cachep, (inode));
+}
+
 
 /*
  * These are initializations that only need to be done
@@ -348,6 +358,12 @@
  
 void clear_inode(struct inode *inode)
 {
+	if (!list_empty(&inode->i_dirty_buffers)) {
+		if (inode->i_nlink)
+			BUG();
+		invalidate_inode_buffers(inode);
+	}
+       
 	if (inode->i_data.nrpages)
 		BUG();
 	if (!(inode->i_state & I_FREEING))
@@ -407,6 +423,7 @@
 		inode = list_entry(tmp, struct inode, i_list);
 		if (inode->i_sb != sb)
 			continue;
+		invalidate_inode_buffers(inode);
 		if (!atomic_read(&inode->i_count)) {
 			list_del(&inode->i_hash);
 			INIT_LIST_HEAD(&inode->i_hash);

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

#24 lore raw
Quoted 1 line · Andrew Morton

Sorry, it's still failing. It took three hours.

Yes. For one thing, original was plain wrong wrt locking (lru_list_lock should be held). For another, it does not take care of metadata. And that's way more serious. What really happens:

ext2_truncate() got a buffer_head of indirect block that is going to die. Fine, we release the blocks refered from it and... do bforget() on our block. Notice that we are not guaranteed that bh will actually die here. buffer.c code might bump its ->b_count for a while, it might be written out right now, etc. As the result, bforget() leaves the sucker alive. It's not a big deal, since we will do unmap_underlying_metadata() before we write anything there (if it will be reused for data) or we'll just pick the bh and zero the buffer out (if it will be reused for metadata).

Unfortunately, we also leave it on the per-inode dirty blocks list. Guess what happens if inode is destroyed, page that used to hold it gets reused and bh gets finally written? Exactly.

Suggested fix: void bforget_inode(struct buffer_head *bh) that would be a copy of __bforget(), except that it would call remove_inode_queue(bh) unconditionally. And replace bforget() with bforget_inode() in those places of ext2/inode.c that are followed by freeing the block.

Comments? I'll do a patch, but I'ld really like to know what had already gone into the main tree. Linus, could you put the 12-pre4-dont-use on ftp.kernel.org?

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

#25 lore raw
In reply to Andrew Morton
Quoted 23 lines · Alexander Viro
Quoted 1 line · Andrew Morton

Sorry, it's still failing. It took three hours.

Yes. For one thing, original was plain wrong wrt locking (lru_list_lock should be held). For another, it does not take care of metadata. And that's way more serious. What really happens:

ext2_truncate() got a buffer_head of indirect block that is going to die. Fine, we release the blocks refered from it and... do bforget() on our block. Notice that we are not guaranteed that bh will actually die here. buffer.c code might bump its ->b_count for a while, it might be written out right now, etc. As the result, bforget() leaves the sucker alive. It's not a big deal, since we will do unmap_underlying_metadata() before we write anything there (if it will be reused for data) or we'll just pick the bh and zero the buffer out (if it will be reused for metadata).

Unfortunately, we also leave it on the per-inode dirty blocks list. Guess what happens if inode is destroyed, page that used to hold it gets reused and bh gets finally written? Exactly.

Quoted 8 lines

Suggested fix: void bforget_inode(struct buffer_head *bh) that would be a copy of __bforget(), except that it would call remove_inode_queue(bh) unconditionally. And replace bforget() with bforget_inode() in those places of ext2/inode.c that are followed by freeing the block.

Comments? I'll do a patch, but I'ld really like to know what had already gone into the main tree. Linus, could you put the 12-pre4-dont-use on ftp.kernel.org?

Al,

I am always amazed at how rapidly you seem to be able to run down some
of these file system corruption problems. You seem to understand the
interaction of this layer extremely well. :-)

Jeff

Quoted 5 lines

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

#26 lore raw
In reply to Andrew Morton

Hi,

Quoted 6 lines · Alexander Viro
Quoted 1 line · Andrew Morton

It appears that this problem is not fixed.

Sure, it isn't. Place where the shit hits the fan: fs/buffer.c::unmap_buffer(). Add the call of remove_inode_queue(bh) there and see if it helps. I.e.

unmap_buffer() calls mark_buffer_clean() calls refile_buffer() calls remove_inode_queue(), which is why we don't see this all the time.

However, refile_buffer() is only calling the remove_inode_queue() if the buffer disposition changes. I'm looking to see where we may be going wrong here --- the refile_buffer() is not atomic wrt. the bh->b_inode structures.

--Stephen
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

#27 lore raw
Quoted 2 lines · Stephen C. Tweedie

unmap_buffer() calls mark_buffer_clean() calls refile_buffer() calls remove_inode_queue(), which is why we don't see this all the time.

Not enough, since you can hit the window between the request completion (bh is marked clean) and getting it picked by flush_dirty_buffers() et.al. If you get destroy_inode() before that window will close...

Quoted 4 lines

However, refile_buffer() is only calling the remove_inode_queue() if the buffer disposition changes. I'm looking to see where we may be going wrong here --- the refile_buffer() is not atomic wrt. the bh->b_inode structures.

See above. Point about the metadata (bforget() is not enough) also stands, ditto for ext2_update_inode() one.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/