jffs2_flash_writev(): Non-contiguous write to 00825300 with mtd_dataflash

jffs2_flash_writev(): Non-contiguous write to 00825300 with mtd_dataflash

[Haavard Skinnemoen]

Hi everyone,

I've seen a couple of other people reporting this bug before, so I'm
just going to offer some observations I've made when trying to debug
this (I'm not a filesystem developer, so I don't fully understand the
code).

I can successfully mount a jffs2 filesystem on a freshly erased
AT45DB642x DataFlash chip through the mtd_dataflash driver. However,
when trying to write something to a file on that filesystem, I get a
BUG and the messages

jffs2_flash_writev(): Non-contiguous write to 00825300
wbuf was previously 00825300-008253b8

Just creating a file without trying to write anything to it works fine.

I've inserted a call to dump_stack() and a few debugging printks in
jffs2_flash_writev() to see what's going on, and I see the following:

pagesize=0x420 sector_size=0x2100

jffs2_flash_writev: count=1 to=0x825300
called from jffs2_write_dnode()
writebuffer: ofs=0x825300 len=0x0
jffs2_flash_writev: count=2 to=0x825344
called from jffs2_write_dirent()
writebuffer: ofs=0x825300 len=0x44
jffs2_flash_writev: count=2 to=0x825370
called from jffs2_write_dnode()
writebuffer: ofs=0x825300 len=0x6f
jffs2_flash_writev: count=1 to=0x825300
called from jffs2_mark_node_obsolete()
writebuffer: ofs=0x825300 len=0xb8
jffs2_flash_writev(): Non-contiguous write to 00825300
wbuf was previously 00825300-008253b8

So what appears to be going on is that a new file is created, then
something is written to it and the metadata has to be updated. This is
done by first writing a new dnode and then marking the old one as
obsolete. Since both nodes share the same sector, obsoleting the old
one means updating something that's already in the write buffer and
thus violating the assumption that the write buffer will only be
appended to.

Like I said, my understanding of the code is very limited, so I'd
really appreciate if someone could tell me if I understand this
correctly. I'd be happy to debug this further if you could just point
me in the right direction.

Btw, I'm using a new SPI driver for the Atmel SPI controller present on
most AT91 and AT32 chips. I'm planning to send it to akpm in a day or
two for possible inclusion in the -mm kernel, but I can of course send
it to you if you think something funny is going on in there.

Haavard

Re: jffs2_flash_writev(): Non-contiguous write to 00825300 with mtd_dataflash

[Artem Bityutskiy]

Hello Haavard,

On Wed, 2006-11-01 at 11:52 +0100, Haavard Skinnemoen wrote:
> I've seen a couple of other people reporting this bug before, so I'm
> just going to offer some observations I've made when trying to debug
> this (I'm not a filesystem developer, so I don't fully understand the
> code).
> 
> I can successfully mount a jffs2 filesystem on a freshly erased
> AT45DB642x DataFlash chip through the mtd_dataflash driver. However,
> when trying to write something to a file on that filesystem, I get a
> BUG and the messages
> 
> jffs2_flash_writev(): Non-contiguous write to 00825300
> wbuf was previously 00825300-008253b8
> 
> Just creating a file without trying to write anything to it works fine.
> 
> I've inserted a call to dump_stack() and a few debugging printks in
> jffs2_flash_writev() to see what's going on, and I see the following:

So all it seems that you need to to make jffs2_can_mark_obsokete()
return 0 in case of dataflash.

-- 
Best regards,
Artem Bityutskiy (Битюцкий Артём)

Re: jffs2_flash_writev(): Non-contiguous write to 00825300 with mtd_dataflash

[Haavard Skinnemoen]

On Mon, 13 Nov 2006 09:46:49 +0200
Artem Bityutskiy <dedekind@infradead.org> wrote:

> On Wed, 2006-11-01 at 11:52 +0100, Haavard Skinnemoen wrote:
> > jffs2_flash_writev(): Non-contiguous write to 00825300
> > wbuf was previously 00825300-008253b8
>
> So all it seems that you need to to make jffs2_can_mark_obsokete()
> return 0 in case of dataflash.

That seems to work. Thanks, and sorry for taking so long to respond to
this.
---
From: Haavard Skinnemoen <hskinnemoen@atmel.com>
Subject: [PATCH] MTD: jffs2_can_mark_obsolete() should return 0 for dataflash

Attempting to write something to a file on a jffs2 filesystem using
mtd_dataflash often results in the following BUG:

jffs2_flash_writev(): Non-contiguous write to 00825720
wbuf was previously 00825300-0082530c
kernel BUG at /home/hskinnemoen/git/linux-devel/fs/jffs2/wbuf.c:786!

Fix this by returning 0 from jffs2_can_mark_obsolete() when running
on top of dataflash.

Signed-off-by: Haavard Skinnemoen <hskinnemoen@atmel.com>
---
 fs/jffs2/os-linux.h |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/fs/jffs2/os-linux.h b/fs/jffs2/os-linux.h
index 9f41fc0..a037fa4 100644
--- a/fs/jffs2/os-linux.h
+++ b/fs/jffs2/os-linux.h
@@ -106,7 +106,8 @@ static inline void jffs2_init_inode_info
 #ifdef CONFIG_JFFS2_SUMMARY
 #define jffs2_can_mark_obsolete(c) (0)
 #else
-#define jffs2_can_mark_obsolete(c) (c->mtd->flags & (MTD_BIT_WRITEABLE))
+#define jffs2_can_mark_obsolete(c) (c->mtd->flags & (MTD_BIT_WRITEABLE)	\
+				    && !jffs2_dataflash(c))
 #endif
 
 #define jffs2_cleanmarker_oob(c) (c->mtd->type == MTD_NANDFLASH)
-- 
1.4.3.3

Re: jffs2_flash_writev(): Non-contiguous write to 00825300 with mtd_dataflash

[Artem Bityutskiy]

On Tue, 2006-11-28 at 12:58 +0100, Haavard Skinnemoen wrote:
> +#define jffs2_can_mark_obsolete(c) (c->mtd->flags & (MTD_BIT_WRITEABLE)	\
> +				    && !jffs2_dataflash(c))

Please, make sure the DataFlash driver does has MTD_BIT_WRITEABLE bit
set at c->mtd->flags instead

-- 
Best regards,
Artem Bityutskiy (Битюцкий Артём)

Re: jffs2_flash_writev(): Non-contiguous write to 00825300 with mtd_dataflash

[Haavard Skinnemoen]

On Tue, 28 Nov 2006 14:36:17 +0200
Artem Bityutskiy <dedekind@infradead.org> wrote:

> On Tue, 2006-11-28 at 12:58 +0100, Haavard Skinnemoen wrote:
> > +#define jffs2_can_mark_obsolete(c) (c->mtd->flags &
> > (MTD_BIT_WRITEABLE)	\
> > +				    && !jffs2_dataflash(c))
> 
> Please, make sure the DataFlash driver does has MTD_BIT_WRITEABLE bit
> set at c->mtd->flags instead

Ok, how about this instead? It seems to work equally well on my board.

Haavard
---
From: Haavard Skinnemoen <hskinnemoen@atmel.com>
Subject: [PATCH] [MTD] Don't set MTD_BIT_WRITEABLE for DataFlash

Attempting to write something to a file on a jffs2 filesystem using
mtd_dataflash often results in the following BUG:

jffs2_flash_writev(): Non-contiguous write to 00825720
wbuf was previously 00825300-0082530c
kernel BUG at /home/hskinnemoen/git/linux-devel/fs/jffs2/wbuf.c:786!

Fix this by not setting MTD_BIT_WRITEABLE for DataFlash devices, thus
causing jffs2_can_mark_obsolete() to return 0.

Signed-off-by: Haavard Skinnemoen <hskinnemoen@atmel.com>
---
 drivers/mtd/devices/mtd_dataflash.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/mtd/devices/mtd_dataflash.c b/drivers/mtd/devices/mtd_dataflash.c
index 5db7160..10a4f4e 100644
--- a/drivers/mtd/devices/mtd_dataflash.c
+++ b/drivers/mtd/devices/mtd_dataflash.c
@@ -480,7 +480,7 @@ add_dataflash(struct spi_device *spi, ch
 	device->writesize = pagesize;
 	device->owner = THIS_MODULE;
 	device->type = MTD_DATAFLASH;
-	device->flags = MTD_CAP_NORFLASH;
+	device->flags = MTD_WRITEABLE;
 	device->erase = dataflash_erase;
 	device->read = dataflash_read;
 	device->write = dataflash_write;
-- 
1.4.3.3

Re: jffs2_flash_writev(): Non-contiguous write to 00825300 with mtd_dataflash

[David Brownell]

On Wednesday 29 November 2006 5:26 am, Haavard Skinnemoen wrote:

> -	device->flags = MTD_CAP_NORFLASH;
> +	device->flags = MTD_WRITEABLE;

That "NORFLASH" flag always seemed bogus to me, but it was at the
time the only way to make the MTD core behave.

Re: jffs2_flash_writev(): Non-contiguous write to 00825300 with mtd_dataflash

[Andrew Victor]

hi,

> > -	device->flags = MTD_CAP_NORFLASH;
> > +	device->flags = MTD_WRITEABLE;
> 
> That "NORFLASH" flag always seemed bogus to me, but it was at the
> time the only way to make the MTD core behave.

Wouldn't it be better to introduce a MTD_CAP_DATAFLASH.
That way if/when more capability flags are added to MTD core, the
dataflash support will be considered and not forgotten about.


Regards,
  Andrew Victor

Re: [Bulk] Re: jffs2_flash_writev(): Non-contiguous write to 00825300 with mtd_dataflash

[David Brownell]

On Wednesday 29 November 2006 10:29 pm, Andrew Victor wrote:
> hi,
> 
> > > -	device->flags = MTD_CAP_NORFLASH;
> > > +	device->flags = MTD_WRITEABLE;
> > 
> > That "NORFLASH" flag always seemed bogus to me, but it was at the
> > time the only way to make the MTD core behave.
> 
> Wouldn't it be better to introduce a MTD_CAP_DATAFLASH.
> That way if/when more capability flags are added to MTD core, the
> dataflash support will be considered and not forgotten about.

I have no strong opinion one way or another.  Structurally it's
maybe a bit more like NAND, and CAP_NANDFLASH == WRITEABLE, so
there might not be a _need_ for such a capability now that some
of that strangeness is gone ...

- Dave

Re: jffs2_flash_writev(): Non-contiguous write to 00825300 with mtd_dataflash

[Artem Bityutskiy]

On Wed, 2006-11-29 at 09:21 -0800, David Brownell wrote:
> > -	device->flags = MTD_CAP_NORFLASH;
> > +	device->flags = MTD_WRITEABLE;
> 
> That "NORFLASH" flag always seemed bogus to me, but it was at the
> time the only way to make the MTD core behave.

I personally not fond of adding capabilities bits to the mtd_info
structure at all...

-- 
Best regards,
Artem Bityutskiy (Битюцкий Артём)