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
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
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-indexPAGE_CACHE_SHIFT)+start
--
dwmw2
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
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-indexPAGE_CACHE_SHIFT)+start
Yeah you're right, thanks.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
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 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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 - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
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.
Index: linux-2.6/fs/jffs2/file.c
===
--- linux-2.6.orig/fs/jffs2/file.c
+++ linux-2.6/fs/jffs2/file.c
@@ -255,7 +255,7 @@ static int jffs2_write_end(struct file *
_whole_ page. This helps to reduce the number of
nodes in files which have many short writes, like
syslog files. */
- start = aligned_start = 0;
+ aligned_start = 0;
}
ri = jffs2_alloc_raw_inode();
@@ -291,14 +291,11 @@ static int jffs2_write_end(struct file *
}
/* Adjust writtenlen for the padding we did, so we don't confuse our caller */
- if (writtenlen < (start&3))
- writtenlen = 0;
- else
- writtenlen -= (start&3);
+ writtenlen -= min(writtenlen, (start - aligned_start));
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;
inode->i_blocks = (inode->i_size + 511) >> 9;
inode->i_ctime = inode->i_mtime = ITIME(je32_to_cpu(ri->ctime));
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.
Index: linux-2.6/mm/filemap.c
===
--- linux-2.6.orig/mm/filemap.c
+++ linux-2.6/mm/filemap.c
@@ -2184,6 +2184,7 @@ fs_write_aop_error:
return written ? written : status;
}
+#include
static ssize_t generic_perform_write(struct file *file,
struct iov_iter *i, loff_t pos)
{
@@ -2243,6 +2244,13 @@ again:
page, fsdata);
if (unlikely(status < 0))
break;
+ if (status > copied) {
+ print_symbol("%s returned more than it should!\n", (unsigned long)a_ops->write_end);
+ printk("status = %ld, copied = %lu\n", status, copied);
+ dump_stack();
+ status = copied; /* try to fix */
+ }
+
copied = status;
cond_resched();
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.
Index: linux-2.6/fs/jffs2/file.c
===
--- linux-2.6.orig/fs/jffs2/file.c
+++ linux-2.6/fs/jffs2/file.c
@@ -255,7 +255,7 @@ static int jffs2_write_end(struct file *
_whole_ page. This helps to reduce the number of
nodes in files which have many short writes, like
syslog files. */
- start = aligned_start = 0;
+ aligned_start = 0;
}
ri = jffs2_alloc_raw_inode();
@@ -291,14 +291,11 @@ static int jffs2_write_end(struct file *
}
/* Adjust writtenlen for the padding we did, so we don't confuse our caller */
- if (writtenlen (start3))
- writtenlen = 0;
- else
- writtenlen -= (start3);
+ writtenlen -= min(writtenlen, (start - aligned_start));
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;
inode-i_blocks = (inode-i_size + 511) 9;
inode-i_ctime = inode-i_mtime = ITIME(je32_to_cpu(ri-ctime));
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.
Index: linux-2.6/mm/filemap.c
===
--- linux-2.6.orig/mm/filemap.c
+++ linux-2.6/mm/filemap.c
@@ -2184,6 +2184,7 @@ fs_write_aop_error:
return written ? written : status;
}
+#include linux/kallsyms.h
static ssize_t generic_perform_write(struct file *file,
struct iov_iter *i, loff_t pos)
{
@@ -2243,6 +2244,13 @@ again:
page, fsdata);
if (unlikely(status 0))
break;
+ if (status copied) {
+ print_symbol(%s returned more than it should!\n, (unsigned long)a_ops-write_end);
+ printk(status = %ld, copied = %lu\n, status, copied);
+ dump_stack();
+ status = copied; /* try to fix */
+ }
+
copied = status;
cond_resched();
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.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
