2007-10-19 06:09:25

by Erez Zadok

[permalink] [raw]
Subject: BUG at mm/filemap.c:1749 (2.6.24, jffs2, unionfs)

David,

I'm testing unionfs on top of jffs2, using 2.6.24 as of linus's commit
4fa4d23fa20de67df919030c1216295664866ad7. All of my unionfs tests pass when
unionfs is stacked on top of jffs2, other than my truncate test -- whic
tries to truncate files up/down (through the union, which then is passed
through to the lower jffs2 f/s). The same truncate test passes on all other
file systems I've tried unionfs/2.6.24 with, as well as all of the earlier
kernels that unionfs runs on (2.6.9--2.6.23). So I tend to think this bug
is more probably due to something else going on in 2.6.24, possibly wrt
jffs2/mtd. (Of course, it's still possible that unionfs isn't doing
something right -- any pointers?)

The oops trace is included below. Is this a known issue and if so, any
fixes? If this is the first you hear of this problem, let me know and I'll
try to narrow it down further.

Thanks,
Erez.

------------[ cut here ]------------
kernel BUG at mm/filemap.c:1749!
invalid opcode: 0000 [#1] DEBUG_PAGEALLOC
Modules linked in: block2mtd mtdblock jffs2 mtd_blkdevs mtd zlib_deflate
zlib_inflate nfsd exportfs auth_rpcgss nfs lockd nfs_acl sunrpc pcnet32
CPU: 0
EIP: 0060:[<c012f03d>] Not tainted VLI
EFLAGS: 00010287 (2.6.23-unionfs2-2.6.24-rc0-pre #9)
EIP is at iov_iter_advance+0x13/0x5d
eax: c538fdec ebx: 00001000 ecx: c538fdec edx: 00001000
esi: 00000fa0 edi: 00000000 ebp: c538fd7c esp: c538fd6c
ds: 007b es: 007b fs: 0000 gs: 0033 ss: 0068
Process dd (pid: 11484, ti=c538e000 task=c4cb05b0 task.ti=c538e000)
Stack: 00001000 00001000 00000fa0 00000000 c538fe10 c01307b7 00000fa0 00000000
00000060 00000060 c16fb0c0 c690edc8 c690eed4 00000000 c538fed4 6238f40
c690eed4 f890a4c0 c690edc8 00000000 00000060 00000fa0 f890a4c0 c16fb0a0
Call Trace:
[<c0102bc2>] show_trace_log_lvl+0x1a/0x2f
[<c0102c72>] show_stack_log_lvl+0x9b/0xa3
[<c0102e2e>] show_registers+0x1b4/0x285
[<c0102fff>] die+0x100/0x21d
[<c01031a5>] do_trap+0x89/0xa2
[<c010347d>] do_invalid_op+0x88/0x92
[<c026a412>] error_code+0x6a/0x70
[<c01307b7>] generic_file_buffered_write+0x163/0x51d
[<c0130f60>] __generic_file_aio_write_nolock+0x3ef/0x43f
[<c0131008>] generic_file_aio_write+0x58/0xb6
[<c01498f8>] do_sync_write+0xc4/0x101
[<c014a04b>] vfs_write+0x90/0x119
[<c014a500>] sys_write+0x3d/0x61
[<c0102586>] sysenter_past_esp+0x5f/0x91
=======================
Code: ff 39 c2 74 0c 89 da 8a 41 ff 88 45 f7 89 d0 eb 02 31 c0 5a 5b 5e 5d
c3 55 89 c1 89 e5 57 56 53 83 ec 04 89 55 f0 39 50 0c 73 04 <0f> 0b eb fe 83
78 04 01 75 08 8b 45 f0 01 41 08 eb 2c 8b 38 8b
EIP: [<c012f03d>] iov_iter_advance+0x13/0x5d SS:ESP 0068:c538fd6c


2007-10-19 07:03:48

by Nick Piggin

[permalink] [raw]
Subject: Re: BUG at mm/filemap.c:1749 (2.6.24, jffs2, unionfs)

On Friday 19 October 2007 16:05, Erez Zadok wrote:
> David,
>
> I'm testing unionfs on top of jffs2, using 2.6.24 as of linus's commit
> 4fa4d23fa20de67df919030c1216295664866ad7. All of my unionfs tests pass
> when unionfs is stacked on top of jffs2, other than my truncate test --
> whic tries to truncate files up/down (through the union, which then is
> passed through to the lower jffs2 f/s). The same truncate test passes on
> all other file systems I've tried unionfs/2.6.24 with, as well as all of
> the earlier kernels that unionfs runs on (2.6.9--2.6.23). So I tend to
> think this bug is more probably due to something else going on in 2.6.24,
> possibly wrt jffs2/mtd. (Of course, it's still possible that unionfs isn't
> doing something right -- any pointers?)
>
> The oops trace is included below. Is this a known issue and if so, any
> fixes? If this is the first you hear of this problem, let me know and I'll
> try to narrow it down further.

It's had quite a lot of recent changes in that area -- the "new aops"
patches.

They've been getting quite a bit of testing in -mm and no such problems,
but I doubt anyone was doing much unionfs over jffs2, or even much jffs2
testing with -mm.

The bug smells like jffs2 is actually passing back a "written" length
greater than the length we passed into it.

The following might show what's happening.


Attachments:
(No filename) (1.33 kB)
mm-debug.patch (738.00 B)
Download all attachments

2007-10-19 07:17:33

by Nick Piggin

[permalink] [raw]
Subject: Re: BUG at mm/filemap.c:1749 (2.6.24, jffs2, unionfs)

On Friday 19 October 2007 17:03, Nick Piggin wrote:
> On Friday 19 October 2007 16:05, Erez Zadok wrote:
> > David,
> >
> > I'm testing unionfs on top of jffs2, using 2.6.24 as of linus's commit
> > 4fa4d23fa20de67df919030c1216295664866ad7. All of my unionfs tests pass
> > when unionfs is stacked on top of jffs2, other than my truncate test --
> > whic tries to truncate files up/down (through the union, which then is
> > passed through to the lower jffs2 f/s). The same truncate test passes on
> > all other file systems I've tried unionfs/2.6.24 with, as well as all of
> > the earlier kernels that unionfs runs on (2.6.9--2.6.23). So I tend to
> > think this bug is more probably due to something else going on in 2.6.24,
> > possibly wrt jffs2/mtd. (Of course, it's still possible that unionfs
> > isn't doing something right -- any pointers?)
> >
> > The oops trace is included below. Is this a known issue and if so, any
> > fixes? If this is the first you hear of this problem, let me know and
> > I'll try to narrow it down further.
>
> It's had quite a lot of recent changes in that area -- the "new aops"
> patches.
>
> They've been getting quite a bit of testing in -mm and no such problems,
> but I doubt anyone was doing much unionfs over jffs2, or even much jffs2
> testing with -mm.
>
> The bug smells like jffs2 is actually passing back a "written" length
> greater than the length we passed into it.
>
> The following might show what's happening.

Hmm, looks like jffs2_write_end is writing more than we actually ask it
to, and returns that back.

unsigned aligned_start = start & ~3;

and

if (end == PAGE_CACHE_SIZE) {
/* When writing out the end of a page, write out the
_whole_ page. This helps to reduce the number of
nodes in files which have many short writes, like
syslog files. */
start = aligned_start = 0;
}

These "longer" writes are fine, but they shouldn't get propagated back
to the vm/vfs. Something like the following patch might fix it.


Attachments:
(No filename) (2.05 kB)
jffs2-writtenlen-fix.patch (1.11 kB)
Download all attachments

2007-10-19 17:42:17

by Erez Zadok

[permalink] [raw]
Subject: Re: BUG at mm/filemap.c:1749 (2.6.24, jffs2, unionfs)

In message <[email protected]>, Nick Piggin writes:
[...]
> Hmm, looks like jffs2_write_end is writing more than we actually ask it
> to, and returns that back.
>
> unsigned aligned_start = start & ~3;
>
> and
>
> if (end == PAGE_CACHE_SIZE) {
> /* When writing out the end of a page, write out the
> _whole_ page. This helps to reduce the number of
> nodes in files which have many short writes, like
> syslog files. */
> start = aligned_start = 0;
> }
>
> These "longer" writes are fine, but they shouldn't get propagated back
> to the vm/vfs. Something like the following patch might fix it.
>
>
> --Boundary-00=_lnFGHwOggSRGKPd
> Content-Type: text/x-diff;
> charset="utf-8";
> name="jffs2-writtenlen-fix.patch"
> Content-Transfer-Encoding: 7bit
> Content-Disposition: attachment;
> filename="jffs2-writtenlen-fix.patch"

Nick, the patch worked. All of my unionfs-over-jffs2 tests passed.

Thanks,
Erez.

2007-10-20 13:16:49

by David Woodhouse

[permalink] [raw]
Subject: Re: BUG at mm/filemap.c:1749 (2.6.24, jffs2, unionfs)

On Fri, 2007-10-19 at 13:38 -0400, Erez Zadok wrote:
> Nick, the patch worked. All of my unionfs-over-jffs2 tests passed.

Can I have a Signed-off-by: for it please?

--
dwmw2

2007-10-21 08:55:55

by David Woodhouse

[permalink] [raw]
Subject: Re: BUG at mm/filemap.c:1749 (2.6.24, jffs2, unionfs)

On Fri, 2007-10-19 at 17:16 +1000, Nick Piggin wrote:
>
> if (writtenlen) {
> - if (inode->i_size < (pg->index << PAGE_CACHE_SHIFT) + start + writtenlen) {
> - inode->i_size = (pg->index << PAGE_CACHE_SHIFT) + start + writtenlen;
> + if (inode->i_size < pos + start + writtenlen) {
> + inode->i_size = pos + start + writtenlen;

This part seems wrong. Shouldn't it just be pos + writtenlen -- which is
basically what it was already: pos==(pg->index<<PAGE_CACHE_SHIFT)+start

--
dwmw2

2007-10-21 10:02:30

by Nick Piggin

[permalink] [raw]
Subject: Re: BUG at mm/filemap.c:1749 (2.6.24, jffs2, unionfs)

On Sunday 21 October 2007 18:55, David Woodhouse wrote:
> On Fri, 2007-10-19 at 17:16 +1000, Nick Piggin wrote:
> > if (writtenlen) {
> > - if (inode->i_size < (pg->index << PAGE_CACHE_SHIFT) +
> > start + writtenlen) { - inode->i_size = (pg->index
> > << PAGE_CACHE_SHIFT) + start + writtenlen; + if
> > (inode->i_size < pos + start + writtenlen) {
> > + inode->i_size = pos + start + writtenlen;
>
> This part seems wrong. Shouldn't it just be pos + writtenlen -- which is
> basically what it was already: pos==(pg->index<<PAGE_CACHE_SHIFT)+start

Yeah you're right, thanks.