headers
Erez Zadok | 19 Oct 2007 08:05
Picon
Favicon

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
(Continue reading)

Permalink | Reply | 1 comment |
headers
Nick Piggin | 19 Oct 2007 09:03
Picon

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.
(Continue reading)

Permalink | Reply | 1 comment |
headers
Nick Piggin | 19 Oct 2007 09:16
Picon

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.
(Continue reading)

Permalink | Reply | 2 comments |
headers
Erez Zadok | 19 Oct 2007 19:38
Picon
Favicon

Re: BUG at mm/filemap.c:1749 (2.6.24, jffs2, unionfs)

In message <200710191716.53470.nickpiggin <at> yahoo.com.au>, 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.
(Continue reading)

Permalink | Reply | 1 comment |
headers
David Woodhouse | 20 Oct 2007 15:16
Favicon

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
Permalink | Reply | 0 comments |
headers
David Woodhouse | 21 Oct 2007 10:55
Favicon

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

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
Permalink | Reply | 1 comment |
headers
Nick Piggin | 21 Oct 2007 11:57
Picon

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.
Permalink | Reply | 0 comments |
headers
Adrian Bunk | 24 Oct 2007 18:27

[2.6 patch] make jffs2_get_acl() static

jffs2_get_acl() can now become static again.

Signed-off-by: Adrian Bunk <bunk <at> kernel.org>

---

 fs/jffs2/acl.c |    2 +-
 fs/jffs2/acl.h |    2 --
 2 files changed, 1 insertion(+), 3 deletions(-)

add2b887d64536f3fe978e62f0774292456f1ddb 
diff --git a/fs/jffs2/acl.c b/fs/jffs2/acl.c
index 9728614..b14e805 100644
--- a/fs/jffs2/acl.c
+++ b/fs/jffs2/acl.c
 <at>  <at>  -176,7 +176,7  <at>  <at>  static void jffs2_iset_acl(struct inode *inode, struct posix_acl **i_acl, struct
 	spin_unlock(&inode->i_lock);
 }

-struct posix_acl *jffs2_get_acl(struct inode *inode, int type)
+static struct posix_acl *jffs2_get_acl(struct inode *inode, int type)
 {
 	struct jffs2_inode_info *f = JFFS2_INODE_INFO(inode);
 	struct posix_acl *acl;
diff --git a/fs/jffs2/acl.h b/fs/jffs2/acl.h
index 76c6ebd..0bb7f00 100644
--- a/fs/jffs2/acl.h
+++ b/fs/jffs2/acl.h
 <at>  <at>  -28,7 +28,6  <at>  <at>  struct jffs2_acl_header {
(Continue reading)

Permalink | Reply | 2 comments |
headers
KaiGai Kohei | 25 Oct 2007 04:19
Picon

Re: [2.6 patch] make jffs2_get_acl() static

Adrian Bunk wrote:
> jffs2_get_acl() can now become static again.
> 
> Signed-off-by: Adrian Bunk <bunk <at> kernel.org>

Acked-by: KaiGai Kohei <kaigai <at> ak.jp.nec.com>

> ---
> 
>  fs/jffs2/acl.c |    2 +-
>  fs/jffs2/acl.h |    2 --
>  2 files changed, 1 insertion(+), 3 deletions(-)
> 
> add2b887d64536f3fe978e62f0774292456f1ddb 
> diff --git a/fs/jffs2/acl.c b/fs/jffs2/acl.c
> index 9728614..b14e805 100644
> --- a/fs/jffs2/acl.c
> +++ b/fs/jffs2/acl.c
>  <at>  <at>  -176,7 +176,7  <at>  <at>  static void jffs2_iset_acl(struct inode *inode, struct posix_acl **i_acl, struct
>  	spin_unlock(&inode->i_lock);
>  }
>  
> -struct posix_acl *jffs2_get_acl(struct inode *inode, int type)
> +static struct posix_acl *jffs2_get_acl(struct inode *inode, int type)
>  {
>  	struct jffs2_inode_info *f = JFFS2_INODE_INFO(inode);
>  	struct posix_acl *acl;
> diff --git a/fs/jffs2/acl.h b/fs/jffs2/acl.h
> index 76c6ebd..0bb7f00 100644
> --- a/fs/jffs2/acl.h
(Continue reading)

Permalink | Reply | 0 comments |
headers
Adrian Bunk | 27 Oct 2007 16:18

jffs2_init_acl_post() can return uninitialized variable

Commit cfc8dc6f6f69ede939e09c2af06a01adee577285 added the following 
function that can return the value of an uninitialized variable:

<--  snip  -->

...
int jffs2_init_acl_post(struct inode *inode)
{
        struct jffs2_inode_info *f = JFFS2_INODE_INFO(inode);
        int rc;

        if (f->i_acl_default) {
                rc = __jffs2_set_acl(inode, JFFS2_XPREFIX_ACL_DEFAULT, f->i_acl_default);
                if (rc)
                        return rc;
        }

        if (f->i_acl_access) {
                rc = __jffs2_set_acl(inode, JFFS2_XPREFIX_ACL_ACCESS, f->i_acl_access);
                if (rc)
                        return rc;
        }

        return rc;
}
...

<--  snip  -->

Spotted by the Coverity checker.
(Continue reading)

Permalink | Reply | 3 comments |

Gmane