[PATCH] [MTD] Fix JFFS2 sync silent failure

[PATCH] [MTD] Fix JFFS2 sync silent failure

[Jörn Engel]

Moin David,

if I read the code correctly, JFFS2 will happily perform a NOP on
sys_sync() again.  And this time it appears to be Jens' fault.

JFFS2 does not appear to set s_bdi anywhere.  And as of 32a88aa1,
__sync_filesystem() will return 0 if s_bdi is not set.  As a result,
sync_fs() is never called for jffs2 and whatever remains in the wbuf
will not make it to the device.

The patch also adds a BUG_ON to catch this problem in any remaining or
future offenders.  I am not sure about network filesystems, but at
least bdev- and mtd-based ones should be caught.

Opinions?

Jörn

-- 
No art, however minor, demands less than total dedication if you want
to excel in it.
-- Leon Battista Alberti

diff --git a/drivers/mtd/mtdsuper.c b/drivers/mtd/mtdsuper.c
index af8b42e..7c00319 100644
--- a/drivers/mtd/mtdsuper.c
+++ b/drivers/mtd/mtdsuper.c
@@ -13,6 +13,7 @@
 #include <linux/mtd/super.h>
 #include <linux/namei.h>
 #include <linux/ctype.h>
+#include <linux/slab.h>
 
 /*
  * compare superblocks to see if they're equivalent
@@ -44,6 +45,7 @@ static int get_sb_mtd_set(struct super_block *sb, void *_mtd)
 
 	sb->s_mtd = mtd;
 	sb->s_dev = MKDEV(MTD_BLOCK_MAJOR, mtd->index);
+	sb->s_bdi = mtd->backing_dev_info;
 	return 0;
 }
 
diff --git a/fs/super.c b/fs/super.c
index f35ac60..e8af253 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -954,10 +954,12 @@ vfs_kern_mount(struct file_system_type *type, int flags, const char *name, void
 	if (error < 0)
 		goto out_free_secdata;
 	BUG_ON(!mnt->mnt_sb);
+	BUG_ON(!mnt->mnt_sb->s_bdi &&
+			(mnt->mnt_sb->s_bdev || mnt->mnt_sb->s_mtd));
 
- 	error = security_sb_kern_mount(mnt->mnt_sb, flags, secdata);
- 	if (error)
- 		goto out_sb;
+	error = security_sb_kern_mount(mnt->mnt_sb, flags, secdata);
+	if (error)
+		goto out_sb;
 
 	/*
 	 * filesystems should never set s_maxbytes larger than MAX_LFS_FILESIZE

Re: [PATCH] [MTD] Fix JFFS2 sync silent failure

[Jens Axboe]

On Sat, Apr 17 2010, Jörn Engel wrote:
> Moin David,
> 
> if I read the code correctly, JFFS2 will happily perform a NOP on
> sys_sync() again.  And this time it appears to be Jens' fault.
> 
> JFFS2 does not appear to set s_bdi anywhere.  And as of 32a88aa1,
> __sync_filesystem() will return 0 if s_bdi is not set.  As a result,
> sync_fs() is never called for jffs2 and whatever remains in the wbuf
> will not make it to the device.
> 
> The patch also adds a BUG_ON to catch this problem in any remaining or
> future offenders.  I am not sure about network filesystems, but at
> least bdev- and mtd-based ones should be caught.
> 
> Opinions?

I think that BUG_ON() would be a lot better as a printk() and fail mount
instead. There's really little point in killing the kernel for something
you could easily warn about and recover nicely.

-- 
Jens Axboe

Re: [PATCH] [MTD] Fix JFFS2 sync silent failure

[Jörn Engel]

On Mon, 19 April 2010 09:38:44 +0200, Jens Axboe wrote:
> On Sat, Apr 17 2010, Jörn Engel wrote:
> > Moin David,
> > 
> > if I read the code correctly, JFFS2 will happily perform a NOP on
> > sys_sync() again.  And this time it appears to be Jens' fault.
> > 
> > JFFS2 does not appear to set s_bdi anywhere.  And as of 32a88aa1,
> > __sync_filesystem() will return 0 if s_bdi is not set.  As a result,
> > sync_fs() is never called for jffs2 and whatever remains in the wbuf
> > will not make it to the device.
> > 
> > The patch also adds a BUG_ON to catch this problem in any remaining or
> > future offenders.  I am not sure about network filesystems, but at
> > least bdev- and mtd-based ones should be caught.
> > 
> > Opinions?
> 
> I think that BUG_ON() would be a lot better as a printk() and fail mount
> instead. There's really little point in killing the kernel for something
> you could easily warn about and recover nicely.

*shrug*
The BUG_ON directly above is not qualitatively different.  In both cases
the only solution is to find and fix the bug in some other file,
recompile and try again.  But ultimately I don't care, as long as we
catch the bug before people lose their data.

Feel free to respin this patch.  You caused the problem in the first
place. ;)

For the record, while I consider the two-liner that causes this mess a
real turd, the rest of your patch was brilliant.  A shame I didn't catch
this earlier.

Jörn

-- 
You ain't got no problem, Jules. I'm on the motherfucker. Go back in
there, chill them niggers out and wait for the Wolf, who should be
coming directly.
-- Marsellus Wallace

Re: [PATCH] [MTD] Fix JFFS2 sync silent failure

[Jens Axboe]

On Mon, Apr 19 2010, Jörn Engel wrote:
> On Mon, 19 April 2010 09:38:44 +0200, Jens Axboe wrote:
> > On Sat, Apr 17 2010, Jörn Engel wrote:
> > > Moin David,
> > > 
> > > if I read the code correctly, JFFS2 will happily perform a NOP on
> > > sys_sync() again.  And this time it appears to be Jens' fault.
> > > 
> > > JFFS2 does not appear to set s_bdi anywhere.  And as of 32a88aa1,
> > > __sync_filesystem() will return 0 if s_bdi is not set.  As a result,
> > > sync_fs() is never called for jffs2 and whatever remains in the wbuf
> > > will not make it to the device.
> > > 
> > > The patch also adds a BUG_ON to catch this problem in any remaining or
> > > future offenders.  I am not sure about network filesystems, but at
> > > least bdev- and mtd-based ones should be caught.
> > > 
> > > Opinions?
> > 
> > I think that BUG_ON() would be a lot better as a printk() and fail mount
> > instead. There's really little point in killing the kernel for something
> > you could easily warn about and recover nicely.
> 
> *shrug*
> The BUG_ON directly above is not qualitatively different.  In both cases
> the only solution is to find and fix the bug in some other file,
> recompile and try again.  But ultimately I don't care, as long as we
> catch the bug before people lose their data.
> 
> Feel free to respin this patch.  You caused the problem in the first
> place. ;)
> 
> For the record, while I consider the two-liner that causes this mess a
> real turd, the rest of your patch was brilliant.  A shame I didn't catch
> this earlier.

Thanks, we definitely should have put a debug statement to catch this in
from day 1, good debugging should be an important part of any new
infrastructure.

Care to send your jffs2 patch separately to David? Then I'll commit a
modified variant for complaining about missing ->s_bdi on mount.

-- 
Jens Axboe

Re: [PATCH] [MTD] Fix JFFS2 sync silent failure

[Jörn Engel]

On Mon, 19 April 2010 12:20:56 +0200, Jens Axboe wrote:
> 
> Care to send your jffs2 patch separately to David? Then I'll commit a
> modified variant for complaining about missing ->s_bdi on mount.

Sure.

David, this patch is untested.  It looks trivially correct and fixes a
nasty bug, but I don't test jffs2 and only noticed the problem in
passing.

Jörn

-- 
Maintenance in other professions and of other articles is concerned with
the return of the item to its original state; in Software, maintenance
is concerned with moving an item away from its original state.
-- Les Belady

diff --git a/drivers/mtd/mtdsuper.c b/drivers/mtd/mtdsuper.c
index af8b42e..7c00319 100644
--- a/drivers/mtd/mtdsuper.c
+++ b/drivers/mtd/mtdsuper.c
@@ -44,6 +45,7 @@ static int get_sb_mtd_set(struct super_block *sb, void *_mtd)
 
 	sb->s_mtd = mtd;
 	sb->s_dev = MKDEV(MTD_BLOCK_MAJOR, mtd->index);
+	sb->s_bdi = mtd->backing_dev_info;
 	return 0;
 }
 

Re: [PATCH] [MTD] Fix JFFS2 sync silent failure

[Jörn Engel]

On Mon, 19 April 2010 12:20:56 +0200, Jens Axboe wrote:
> 
> Thanks, we definitely should have put a debug statement to catch this in
> from day 1, good debugging should be an important part of any new
> infrastructure.

Woke up early and had another look at this.  Looks like a much more
widespread problem.  Based on a quick grep an uncaffeinated brain:

9p	no s_bdi
afs	no s_bdi
ceph	creates its own s_bdi
cifs	no s_bdi
coda	no s_bdi
ecryptfs no s_bdi
exofs	no s_bdi
fuse	creates its own s_bdi?
gfs2	creates its own s_bdi?
jffs2	patch exists
logfs	fixed now
ncpfs	no s_bdi
nfs	creates its own s_bdi
ocfs2	no s_bdi
smbfs	no s_bdi
ubifs	creates its own s_bdi

I excluded all filesystems that appear to be read-only, block device
based or lack any sort of backing store.  So there is a chance I have
missed some as well.

Jörn

-- 
Simplicity is prerequisite for reliability.
-- Edsger W. Dijkstra

Re: [PATCH] [MTD] Fix JFFS2 sync silent failure

[Jörn Engel]

Linus,

I think this is bad enough that you should be involved.  32a88aa1 broke
a number of filesystems in a way that sync() would return 0 without
doing any work.  Even politicians are better at keeping the promises.

This is caused by the two-liner in __sync_filesystem:
	if (!sb->s_bdi)
		return 0;
s_bdi is set implicitly for all filesystems using set_bdev_super(), so
most block device based filesystems are safe.  There are, however, a
number of odd-balls around:

On Thu, 22 April 2010 07:54:48 +0200, Jörn Engel wrote:
> 
> 9p	no s_bdi
> afs	no s_bdi
> ceph	creates its own s_bdi
> cifs	no s_bdi
> coda	no s_bdi
> ecryptfs no s_bdi
> exofs	no s_bdi
> fuse	creates its own s_bdi?
> gfs2	creates its own s_bdi?
> jffs2	patch exists
> logfs	fixed now
> ncpfs	no s_bdi
> nfs	creates its own s_bdi
> ocfs2	no s_bdi
> smbfs	no s_bdi
> ubifs	creates its own s_bdi

Obviously this list should get checked and all affected filesystems get
repaired.  Additionally we should add an assertion and BUG() or refuse
to mount or something.  My original patch to that extend was this:

diff --git a/fs/super.c b/fs/super.c
index f35ac60..e8af253 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -954,6 +954,8 @@ vfs_kern_mount(struct file_system_type *type, int flags, const char *name, void
 	if (error < 0)
 		goto out_free_secdata;
 	BUG_ON(!mnt->mnt_sb);
+	BUG_ON(!mnt->mnt_sb->s_bdi &&
+			(mnt->mnt_sb->s_bdev || mnt->mnt_sb->s_mtd));
 
  	error = security_sb_kern_mount(mnt->mnt_sb, flags, secdata);
  	if (error)
  		goto out_sb;

The real problem is finding a condition that has neither false positives
nor false negatives.  The "(mnt->mnt_sb->s_bdev || mnt->mnt_sb->s_mtd)"
part takes care of false positives like tmpfs, but it would catch none
of the network filesystems.  Should we instead annotate tmpfs and friends
with something like sb->s_dont_need_bdi?  It is the only way I can think
of not to miss something.

Jörn

-- 
People will accept your ideas much more readily if you tell them
that Benjamin Franklin said it first.
-- unknown

Re: [PATCH] [MTD] Fix JFFS2 sync silent failure

[Linus Torvalds]

On Thu, 22 Apr 2010, Jörn Engel wrote:
>
> I think this is bad enough that you should be involved.  32a88aa1 broke
> a number of filesystems in a way that sync() would return 0 without
> doing any work.  Even politicians are better at keeping the promises.
> 
> This is caused by the two-liner in __sync_filesystem:
> 	if (!sb->s_bdi)
> 		return 0;
> s_bdi is set implicitly for all filesystems using set_bdev_super(), so
> most block device based filesystems are safe.  There are, however, a
> number of odd-balls around:

Umm. Why not just remove the two-liner? It was incorrect. The comment says 
"this should be safe", and if it wasn't, then the commit that caused this 
all was total crap to begin with.

Why even discuss any other measures?

		Linus

Re: [PATCH] [MTD] Fix JFFS2 sync silent failure

[Linus Torvalds]

On Thu, 22 Apr 2010, Linus Torvalds wrote:
>
> Umm. Why not just remove the two-liner? It was incorrect. The comment says 
> "this should be safe", and if it wasn't, then the commit that caused this 
> all was total crap to begin with.

Grr. Ok, so we do need it. Because Jens made it not work without it, and 
didn't fix up the filesystems, just added a random comment saying "we 
shouldn't need to".

Double-grr. I hate misleading comments. It makes the patch and the code 
look like people knew what they were doing.

Jens - please help fix this up.

		Linus

Re: [PATCH] [MTD] Fix JFFS2 sync silent failure

[Jens Axboe]

On Thu, Apr 22 2010, Linus Torvalds wrote:
> 
> 
> On Thu, 22 Apr 2010, Linus Torvalds wrote:
> >
> > Umm. Why not just remove the two-liner? It was incorrect. The comment says 
> > "this should be safe", and if it wasn't, then the commit that caused this 
> > all was total crap to begin with.
> 
> Grr. Ok, so we do need it. Because Jens made it not work without it, and 
> didn't fix up the filesystems, just added a random comment saying "we 
> shouldn't need to".
> 
> Double-grr. I hate misleading comments. It makes the patch and the code 
> look like people knew what they were doing.

Yeah sorry, that part was apparently not well thought through. It did go
through review and testing, but at least the latter was not good enough.

> Jens - please help fix this up.

Of course, I already posted a series of patches to fix this up. I want
to test them a bit, and I'll send them in tomorrow.

-- 
Jens Axboe

[Patch] Catch filesystems lacking s_bdi

[Jörn Engel]

On Thu, 22 April 2010 18:27:10 +0200, Jens Axboe wrote:
> 
> > Jens - please help fix this up.
> 
> Of course, I already posted a series of patches to fix this up. I want
> to test them a bit, and I'll send them in tomorrow.

How about something like this to catch future cases?  It compiles and
survived a test boot, so it does seem to work for the common cases like
tmpfs, procfs, etc.

Jens, you know the bdi code 10x better than me, would this work?

Jörn

-- 
ticks = jiffies;
while (ticks == jiffies);
ticks = jiffies;
-- /usr/src/linux/init/main.c

noop_backing_dev_info is used only as a flag to mark filesystems that
don't have any backing store, like tmpfs, procfs, spufs, etc.

Signed-off-by: Joern Engel <joern@logfs.org>

diff --git a/fs/super.c b/fs/super.c
index f35ac60..dc72491 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -693,6 +693,7 @@ int set_anon_super(struct super_block *s, void *data)
 		return -EMFILE;
 	}
 	s->s_dev = MKDEV(0, dev & MINORMASK);
+	s->s_bdi = &noop_backing_dev_info;
 	return 0;
 }
 
@@ -954,10 +955,11 @@ vfs_kern_mount(struct file_system_type *type, int flags, const char *name, void
 	if (error < 0)
 		goto out_free_secdata;
 	BUG_ON(!mnt->mnt_sb);
+	BUG_ON(!mnt->mnt_sb->s_bdi);
 
- 	error = security_sb_kern_mount(mnt->mnt_sb, flags, secdata);
- 	if (error)
- 		goto out_sb;
+	error = security_sb_kern_mount(mnt->mnt_sb, flags, secdata);
+	if (error)
+		goto out_sb;
 
 	/*
 	 * filesystems should never set s_maxbytes larger than MAX_LFS_FILESIZE
diff --git a/fs/sync.c b/fs/sync.c
index fc5c3d7..92b2281 100644
--- a/fs/sync.c
+++ b/fs/sync.c
@@ -14,6 +14,7 @@
 #include <linux/pagemap.h>
 #include <linux/quotaops.h>
 #include <linux/buffer_head.h>
+#include <linux/backing-dev.h>
 #include "internal.h"
 
 #define VALID_FLAGS (SYNC_FILE_RANGE_WAIT_BEFORE|SYNC_FILE_RANGE_WRITE| \
@@ -32,7 +33,7 @@ static int __sync_filesystem(struct super_block *sb, int wait)
 	 * This should be safe, as we require bdi backing to actually
 	 * write out data in the first place
 	 */
-	if (!sb->s_bdi)
+	if (!sb->s_bdi || sb->s_bdi == &noop_backing_dev_info)
 		return 0;
 
 	if (sb->s_qcop && sb->s_qcop->quota_sync)
diff --git a/include/linux/backing-dev.h b/include/linux/backing-dev.h
index fcbc26a..f4a1436 100644
--- a/include/linux/backing-dev.h
+++ b/include/linux/backing-dev.h
@@ -246,6 +246,7 @@ int bdi_set_max_ratio(struct backing_dev_info *bdi, unsigned int max_ratio);
 #endif
 
 extern struct backing_dev_info default_backing_dev_info;
+extern struct backing_dev_info noop_backing_dev_info;
 void default_unplug_io_fn(struct backing_dev_info *bdi, struct page *page);
 
 int writeback_in_progress(struct backing_dev_info *bdi);
diff --git a/mm/backing-dev.c b/mm/backing-dev.c
index f13e067..4aba836 100644
--- a/mm/backing-dev.c
+++ b/mm/backing-dev.c
@@ -25,6 +25,11 @@ struct backing_dev_info default_backing_dev_info = {
 };
 EXPORT_SYMBOL_GPL(default_backing_dev_info);
 
+struct backing_dev_info noop_backing_dev_info = {
+	.name		= "noop",
+};
+EXPORT_SYMBOL_GPL(noop_backing_dev_info);
+
 static struct class *bdi_class;
 
 /*

Re: [Patch] Catch filesystems lacking s_bdi

[Jens Axboe]

On Thu, Apr 22 2010, Jörn Engel wrote:
> On Thu, 22 April 2010 18:27:10 +0200, Jens Axboe wrote:
> > 
> > > Jens - please help fix this up.
> > 
> > Of course, I already posted a series of patches to fix this up. I want
> > to test them a bit, and I'll send them in tomorrow.
> 
> How about something like this to catch future cases?  It compiles and
> survived a test boot, so it does seem to work for the common cases like
> tmpfs, procfs, etc.
> 
> Jens, you know the bdi code 10x better than me, would this work?

Looks sane, it's a good start. I think we should augment that with a
check to ensure that we don't ever add dirty inodes to this bdi, since
it's not going to be flushed.

Something like a:

        WARN_ON(bdi == &noop_backing_dev_info);

to __mark_inode_dirty(). Looking at the code it should already trigger a
warning, since it'll check for BDI_CAP_NO_WRITEBACK (which isn't set
for noop_backing_dev_info) and the fact that noop-bdi isn't registered
to begin with.

So it's probably safe and good enough as-is, I'll add it. Thanks!

-- 
Jens Axboe

Re: [Patch] Catch filesystems lacking s_bdi

[Jörn Engel]

On Fri, 23 April 2010 12:05:32 +0200, Jens Axboe wrote:
> 
> So it's probably safe and good enough as-is, I'll add it. Thanks!

I cannot see this patch in your tree yet.  Could be the weekend or a
deliberate decision not to send this for 2.6.34-rc anymore.

In case it was a deliberate decision, can we please make it explicit?  I
don't like the idea of adding a BUG_ON() that potentially triggers for
thousands of people this late in the stabilization process - but it is
better than having people lose data.  Even if we already ran two stable
kernels that way.

Damned if you do, damned if you don't. :(

Jörn

-- 
Time? What's that? Time is only worth what you do with it.
-- Theo de Raadt

Re: [Patch] Catch filesystems lacking s_bdi

[Jens Axboe]

On Fri, Apr 23 2010, Jörn Engel wrote:
> On Fri, 23 April 2010 12:05:32 +0200, Jens Axboe wrote:
> > 
> > So it's probably safe and good enough as-is, I'll add it. Thanks!
> 
> I cannot see this patch in your tree yet.  Could be the weekend or a
> deliberate decision not to send this for 2.6.34-rc anymore.

It's there, I put it in yesterday. It's definitely 2.6.34-rc material, I
hope to submit it tonight.

> In case it was a deliberate decision, can we please make it explicit?  I
> don't like the idea of adding a BUG_ON() that potentially triggers for
> thousands of people this late in the stabilization process - but it is
> better than having people lose data.  Even if we already ran two stable
> kernels that way.
> 
> Damned if you do, damned if you don't. :(

Yeah, it's a bad situation to be in. I changed that BUG_ON() to a
WARN_ON(). I'm not too worried about that part, I'm more worried about
the file system changed. OTOH, they do lack proper flushing now, so it's
likely not a huge risk from that perspective.

-- 
Jens Axboe

Re: [Patch] Catch filesystems lacking s_bdi

[Jörn Engel]

On Mon, 26 April 2010 11:48:11 +0200, Jens Axboe wrote:
> On Fri, Apr 23 2010, Jörn Engel wrote:
> > On Fri, 23 April 2010 12:05:32 +0200, Jens Axboe wrote:
> > > 
> > > So it's probably safe and good enough as-is, I'll add it. Thanks!
> > 
> > I cannot see this patch in your tree yet.  Could be the weekend or a
> > deliberate decision not to send this for 2.6.34-rc anymore.
> 
> It's there, I put it in yesterday. It's definitely 2.6.34-rc material, I
> hope to submit it tonight.

Ok, thanks.

> > In case it was a deliberate decision, can we please make it explicit?  I
> > don't like the idea of adding a BUG_ON() that potentially triggers for
> > thousands of people this late in the stabilization process - but it is
> > better than having people lose data.  Even if we already ran two stable
> > kernels that way.
> > 
> > Damned if you do, damned if you don't. :(
> 
> Yeah, it's a bad situation to be in. I changed that BUG_ON() to a
> WARN_ON(). I'm not too worried about that part, I'm more worried about
> the file system changed. OTOH, they do lack proper flushing now, so it's
> likely not a huge risk from that perspective.

It is worse still.  Using mtd->backing_dev_info results in 
kernel BUG at fs/fs-writeback.c:157

which is BUG_ON(!work->seen); in bdi_queue_work().  Logfs is affected
and I bet jffs2 is as well.  So much for dwmw2 or me actually testing
the fix. :(

I did a hexdump to see what sb->s_bdi actually contained and the result
was this:
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
04 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
00 00 00 00 00 00 00 00                          ........

Which should be mtd_bdi_unmappable.  And at this point I have to admit
being clueless.  What exactly should a struct backing_dev_info contain
and for what purposes?  And where is this documented?

Jörn

-- 
He who knows others is wise.
He who knows himself is enlightened.
-- Lao Tsu

Re: [Patch] Catch filesystems lacking s_bdi

[Jens Axboe]

On Mon, Apr 26 2010, Jörn Engel wrote:
> > Yeah, it's a bad situation to be in. I changed that BUG_ON() to a
> > WARN_ON(). I'm not too worried about that part, I'm more worried about
> > the file system changed. OTOH, they do lack proper flushing now, so it's
> > likely not a huge risk from that perspective.
> 
> It is worse still.  Using mtd->backing_dev_info results in 
> kernel BUG at fs/fs-writeback.c:157
> 
> which is BUG_ON(!work->seen); in bdi_queue_work().  Logfs is affected
> and I bet jffs2 is as well.  So much for dwmw2 or me actually testing
> the fix. :(
> 
> I did a hexdump to see what sb->s_bdi actually contained and the result
> was this:
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> 04 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> 00 00 00 00 00 00 00 00                          ........
> 
> Which should be mtd_bdi_unmappable.  And at this point I have to admit
> being clueless.  What exactly should a struct backing_dev_info contain
> and for what purposes?  And where is this documented?

The important bit is that each bdi must be initialized and registered if
it's going to be handling dirty data, it can't just be a static
placeholder. See the bdi_setup_and_register() helper I added.

-- 
Jens Axboe

Re: [Patch] Catch filesystems lacking s_bdi

[Jens Axboe]

On Mon, Apr 26 2010, Jens Axboe wrote:
> On Mon, Apr 26 2010, Jörn Engel wrote:
> > > Yeah, it's a bad situation to be in. I changed that BUG_ON() to a
> > > WARN_ON(). I'm not too worried about that part, I'm more worried about
> > > the file system changed. OTOH, they do lack proper flushing now, so it's
> > > likely not a huge risk from that perspective.
> > 
> > It is worse still.  Using mtd->backing_dev_info results in 
> > kernel BUG at fs/fs-writeback.c:157
> > 
> > which is BUG_ON(!work->seen); in bdi_queue_work().  Logfs is affected
> > and I bet jffs2 is as well.  So much for dwmw2 or me actually testing
> > the fix. :(
> > 
> > I did a hexdump to see what sb->s_bdi actually contained and the result
> > was this:
> > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> > 04 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> > 00 00 00 00 00 00 00 00                          ........
> > 
> > Which should be mtd_bdi_unmappable.  And at this point I have to admit
> > being clueless.  What exactly should a struct backing_dev_info contain
> > and for what purposes?  And where is this documented?
> 
> The important bit is that each bdi must be initialized and registered if
> it's going to be handling dirty data, it can't just be a static
> placeholder. See the bdi_setup_and_register() helper I added.

Took a quick look, and you want bdi_setup_and_register() for the three
bdis listed in mtdbdi.c in mtdcore.c:init_mtd(). Or manual bdi_init()
and bdi_register(). I'll take a look post-dinner. Either is workable,
but since the flags are already setup, the latter may be cleaner.

-- 
Jens Axboe

[PATCH 1/2] [MTD] Move mtd_bdi_*mappable to mtdcore.c

[Jörn Engel]

On Mon, 26 April 2010 16:45:32 +0200, Jens Axboe wrote:
> 
> Took a quick look, and you want bdi_setup_and_register() for the three
> bdis listed in mtdbdi.c in mtdcore.c:init_mtd(). Or manual bdi_init()
> and bdi_register(). I'll take a look post-dinner. Either is workable,
> but since the flags are already setup, the latter may be cleaner.

Ok, here are two patches that appear to solve the problem for me.

Jörn

-- 
Courage is not the absence of fear, but rather the judgement that
something else is more important than fear.
-- Ambrose Redmoon

Removes one .h and one .c file that are never used outside of
mtdcore.c.

Signed-off-by: Joern Engel <joern@logfs.org>
---
 drivers/mtd/Makefile        |    2 +-
 drivers/mtd/devices/drais.c |    2 +-
 drivers/mtd/internal.h      |   17 -----------------
 drivers/mtd/mtdbdi.c        |   43 -------------------------------------------
 drivers/mtd/mtdcore.c       |   33 ++++++++++++++++++++++++++++++++-
 5 files changed, 34 insertions(+), 63 deletions(-)

diff --git a/drivers/mtd/Makefile b/drivers/mtd/Makefile
index 82d1e4d..4521b1e 100644
--- a/drivers/mtd/Makefile
+++ b/drivers/mtd/Makefile
@@ -4,7 +4,7 @@
 
 # Core functionality.
 obj-$(CONFIG_MTD)		+= mtd.o
-mtd-y				:= mtdcore.o mtdsuper.o mtdbdi.o
+mtd-y				:= mtdcore.o mtdsuper.o
 mtd-$(CONFIG_MTD_PARTITIONS)	+= mtdpart.o
 
 obj-$(CONFIG_MTD_CONCAT)	+= mtdconcat.o
diff --git a/drivers/mtd/devices/drais.c b/drivers/mtd/devices/drais.c
index b93ff3f..6f81b28 100644
--- a/drivers/mtd/devices/drais.c
+++ b/drivers/mtd/devices/drais.c
@@ -33,7 +33,7 @@
 #include <linux/sched.h>
 #include "drais.h"
 
-#undef DEBUG_ERASE
+#define DEBUG_ERASE
 
 #define DRAIS_BB_MAGIC		0x3911b26ba5a931d8ull
 
diff --git a/drivers/mtd/internal.h b/drivers/mtd/internal.h
index c658fe7..e69de29 100644
--- a/drivers/mtd/internal.h
+++ b/drivers/mtd/internal.h
@@ -1,17 +0,0 @@
-/* Internal MTD definitions
- *
- * Copyright © 2006 Red Hat, Inc. All Rights Reserved.
- * Written by David Howells (dhowells@redhat.com)
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License
- * as published by the Free Software Foundation; either version
- * 2 of the License, or (at your option) any later version.
- */
-
-/*
- * mtdbdi.c
- */
-extern struct backing_dev_info mtd_bdi_unmappable;
-extern struct backing_dev_info mtd_bdi_ro_mappable;
-extern struct backing_dev_info mtd_bdi_rw_mappable;
diff --git a/drivers/mtd/mtdbdi.c b/drivers/mtd/mtdbdi.c
index 5ca5aed..e69de29 100644
--- a/drivers/mtd/mtdbdi.c
+++ b/drivers/mtd/mtdbdi.c
@@ -1,43 +0,0 @@
-/* MTD backing device capabilities
- *
- * Copyright © 2006 Red Hat, Inc. All Rights Reserved.
- * Written by David Howells (dhowells@redhat.com)
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License
- * as published by the Free Software Foundation; either version
- * 2 of the License, or (at your option) any later version.
- */
-
-#include <linux/backing-dev.h>
-#include <linux/mtd/mtd.h>
-#include "internal.h"
-
-/*
- * backing device capabilities for non-mappable devices (such as NAND flash)
- * - permits private mappings, copies are taken of the data
- */
-struct backing_dev_info mtd_bdi_unmappable = {
-	.capabilities	= BDI_CAP_MAP_COPY,
-};
-
-/*
- * backing device capabilities for R/O mappable devices (such as ROM)
- * - permits private mappings, copies are taken of the data
- * - permits non-writable shared mappings
- */
-struct backing_dev_info mtd_bdi_ro_mappable = {
-	.capabilities	= (BDI_CAP_MAP_COPY | BDI_CAP_MAP_DIRECT |
-			   BDI_CAP_EXEC_MAP | BDI_CAP_READ_MAP),
-};
-
-/*
- * backing device capabilities for writable mappable devices (such as RAM)
- * - permits private mappings, copies are taken of the data
- * - permits non-writable shared mappings
- */
-struct backing_dev_info mtd_bdi_rw_mappable = {
-	.capabilities	= (BDI_CAP_MAP_COPY | BDI_CAP_MAP_DIRECT |
-			   BDI_CAP_EXEC_MAP | BDI_CAP_READ_MAP |
-			   BDI_CAP_WRITE_MAP),
-};
diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index 05b9577..cb4858b 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -2,6 +2,9 @@
  * Core registration and callback routines for MTD
  * drivers and users.
  *
+ * bdi bits are:
+ * Copyright © 2006 Red Hat, Inc. All Rights Reserved.
+ * Written by David Howells (dhowells@redhat.com)
  */
 
 #include <linux/module.h>
@@ -16,11 +19,39 @@
 #include <linux/init.h>
 #include <linux/mtd/compatmac.h>
 #include <linux/proc_fs.h>
+#include <linux/backing-dev.h>
 
 #include <linux/mtd/mtd.h>
-#include "internal.h"
 
 #include "mtdcore.h"
+/*
+ * backing device capabilities for non-mappable devices (such as NAND flash)
+ * - permits private mappings, copies are taken of the data
+ */
+struct backing_dev_info mtd_bdi_unmappable = {
+	.capabilities	= BDI_CAP_MAP_COPY,
+};
+
+/*
+ * backing device capabilities for R/O mappable devices (such as ROM)
+ * - permits private mappings, copies are taken of the data
+ * - permits non-writable shared mappings
+ */
+struct backing_dev_info mtd_bdi_ro_mappable = {
+	.capabilities	= (BDI_CAP_MAP_COPY | BDI_CAP_MAP_DIRECT |
+			   BDI_CAP_EXEC_MAP | BDI_CAP_READ_MAP),
+};
+
+/*
+ * backing device capabilities for writable mappable devices (such as RAM)
+ * - permits private mappings, copies are taken of the data
+ * - permits non-writable shared mappings
+ */
+struct backing_dev_info mtd_bdi_rw_mappable = {
+	.capabilities	= (BDI_CAP_MAP_COPY | BDI_CAP_MAP_DIRECT |
+			   BDI_CAP_EXEC_MAP | BDI_CAP_READ_MAP |
+			   BDI_CAP_WRITE_MAP),
+};
 
 static int mtd_cls_suspend(struct device *dev, pm_message_t state);
 static int mtd_cls_resume(struct device *dev);
-- 
1.6.2.1

[PATCH 2/2] [MTD] Call bdi_init() and bdi_register()

[Jörn Engel]

Otherwise we hit a BUG_ON in bdi_queue_work().

Signed-off-by: Joern Engel <joern@logfs.org>
---
 drivers/mtd/mtdcore.c |   17 ++++++++++++++++-
 1 files changed, 16 insertions(+), 1 deletions(-)

diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index cb4858b..8dd3e46 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -299,7 +299,7 @@ static struct device_type mtd_devtype = {
 
 int add_mtd_device(struct mtd_info *mtd)
 {
-	int i;
+	int i, err;
 
 	if (!mtd->backing_dev_info) {
 		switch (mtd->type) {
@@ -322,6 +322,12 @@ int add_mtd_device(struct mtd_info *mtd)
 		if (!mtd_table[i]) {
 			struct mtd_notifier *not;
 
+			err = bdi_register(mtd->backing_dev_info, NULL, "mtd%d",
+					i);
+			if (err) {
+				/* We lose the errno information :( */
+				break;
+			}
 			mtd_table[i] = mtd;
 			mtd->index = i;
 			mtd->usecount = 0;
@@ -692,6 +698,15 @@ static int __init init_mtd(void)
 	int ret;
 	ret = class_register(&mtd_class);
 
+	ret = bdi_init(&mtd_bdi_unmappable);
+	if (ret)
+		return ret;
+	ret = bdi_init(&mtd_bdi_ro_mappable);
+	if (ret)
+		return ret;
+	ret = bdi_init(&mtd_bdi_rw_mappable);
+	if (ret)
+		return ret;
 	if (ret) {
 		pr_err("Error registering mtd class: %d\n", ret);
 		return ret;
-- 
1.6.2.1

Re: [PATCH 2/2] [MTD] Call bdi_init() and bdi_register()

[Jens Axboe]

On Mon, Apr 26 2010, Jörn Engel wrote:
> Otherwise we hit a BUG_ON in bdi_queue_work().
> 
> Signed-off-by: Joern Engel <joern@logfs.org>
> ---
>  drivers/mtd/mtdcore.c |   17 ++++++++++++++++-
>  1 files changed, 16 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
> index cb4858b..8dd3e46 100644
> --- a/drivers/mtd/mtdcore.c
> +++ b/drivers/mtd/mtdcore.c
> @@ -322,6 +322,12 @@ int add_mtd_device(struct mtd_info *mtd)
>  		if (!mtd_table[i]) {
>  			struct mtd_notifier *not;
>  
> +			err = bdi_register(mtd->backing_dev_info, NULL, "mtd%d",
> +					i);
> +			if (err) {
> +				/* We lose the errno information :( */
> +				break;
> +			}

This is not a good idea, even if it'll currently work.

> @@ -692,6 +698,15 @@ static int __init init_mtd(void)
>  	int ret;
>  	ret = class_register(&mtd_class);
>  
> +	ret = bdi_init(&mtd_bdi_unmappable);
> +	if (ret)
> +		return ret;
> +	ret = bdi_init(&mtd_bdi_ro_mappable);
> +	if (ret)
> +		return ret;
> +	ret = bdi_init(&mtd_bdi_rw_mappable);
> +	if (ret)
> +		return ret;

Do the bdi_register() here as well.

-- 
Jens Axboe

Re: [PATCH 2/2] [MTD] Call bdi_init() and bdi_register()

[Jörn Engel]

On Mon, 26 April 2010 19:02:48 +0200, Jens Axboe wrote:
> On Mon, Apr 26 2010, Jörn Engel wrote:
> > Otherwise we hit a BUG_ON in bdi_queue_work().
> > 
> > Signed-off-by: Joern Engel <joern@logfs.org>
> > ---
> >  drivers/mtd/mtdcore.c |   17 ++++++++++++++++-
> >  1 files changed, 16 insertions(+), 1 deletions(-)
> > 
> > diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
> > index cb4858b..8dd3e46 100644
> > --- a/drivers/mtd/mtdcore.c
> > +++ b/drivers/mtd/mtdcore.c
> > @@ -322,6 +322,12 @@ int add_mtd_device(struct mtd_info *mtd)
> >  		if (!mtd_table[i]) {
> >  			struct mtd_notifier *not;
> >  
> > +			err = bdi_register(mtd->backing_dev_info, NULL, "mtd%d",
> > +					i);
> > +			if (err) {
> > +				/* We lose the errno information :( */
> > +				break;
> > +			}
> 
> This is not a good idea, even if it'll currently work.
> 
> > @@ -692,6 +698,15 @@ static int __init init_mtd(void)
> >  	int ret;
> >  	ret = class_register(&mtd_class);
> >  
> > +	ret = bdi_init(&mtd_bdi_unmappable);
> > +	if (ret)
> > +		return ret;
> > +	ret = bdi_init(&mtd_bdi_ro_mappable);
> > +	if (ret)
> > +		return ret;
> > +	ret = bdi_init(&mtd_bdi_rw_mappable);
> > +	if (ret)
> > +		return ret;
> 
> Do the bdi_register() here as well.

-ENOBRAIN

Initially I wanted to do just than.  Then I looked at the block layer
and thought we could create one backing_dev_info per mtd as well.  Not
necessarily a bad if I would actually _create_ them and not just reuse
the same ones over and over again.

Jörn

-- 
Linux [...] existed just for discussion between people who wanted
to show off how geeky they were.
-- Rob Enderle

Re: [PATCH 2/2] [MTD] Call bdi_init() and bdi_register()

[Jens Axboe]

On Mon, Apr 26 2010, Jörn Engel wrote:
> On Mon, 26 April 2010 19:02:48 +0200, Jens Axboe wrote:
> > On Mon, Apr 26 2010, Jörn Engel wrote:
> > > Otherwise we hit a BUG_ON in bdi_queue_work().
> > > 
> > > Signed-off-by: Joern Engel <joern@logfs.org>
> > > ---
> > >  drivers/mtd/mtdcore.c |   17 ++++++++++++++++-
> > >  1 files changed, 16 insertions(+), 1 deletions(-)
> > > 
> > > diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
> > > index cb4858b..8dd3e46 100644
> > > --- a/drivers/mtd/mtdcore.c
> > > +++ b/drivers/mtd/mtdcore.c
> > > @@ -322,6 +322,12 @@ int add_mtd_device(struct mtd_info *mtd)
> > >  		if (!mtd_table[i]) {
> > >  			struct mtd_notifier *not;
> > >  
> > > +			err = bdi_register(mtd->backing_dev_info, NULL, "mtd%d",
> > > +					i);
> > > +			if (err) {
> > > +				/* We lose the errno information :( */
> > > +				break;
> > > +			}
> > 
> > This is not a good idea, even if it'll currently work.
> > 
> > > @@ -692,6 +698,15 @@ static int __init init_mtd(void)
> > >  	int ret;
> > >  	ret = class_register(&mtd_class);
> > >  
> > > +	ret = bdi_init(&mtd_bdi_unmappable);
> > > +	if (ret)
> > > +		return ret;
> > > +	ret = bdi_init(&mtd_bdi_ro_mappable);
> > > +	if (ret)
> > > +		return ret;
> > > +	ret = bdi_init(&mtd_bdi_rw_mappable);
> > > +	if (ret)
> > > +		return ret;
> > 
> > Do the bdi_register() here as well.
> 
> -ENOBRAIN
> 
> Initially I wanted to do just than.  Then I looked at the block layer
> and thought we could create one backing_dev_info per mtd as well.  Not
> necessarily a bad if I would actually _create_ them and not just reuse
> the same ones over and over again.

I cooked up that patch myself, here:

http://git.kernel.dk/?p=linux-2.6-block.git;a=commit;h=0661b1ac5d48eb47c8a5948c0554fea25e0895ab

Care to give it a quick spin?

-- 
Jens Axboe

Re: [PATCH 2/2] [MTD] Call bdi_init() and bdi_register()

[Paolo Minazzi]

I Jens,
I'm Paolo Minazzi and I have a problem with logfs and 2.6.34rc5.
Can I apply the patch to rc5 and make a test on my ARM board ?
Paolo



On Tue, Apr 27, 2010 at 9:52 AM, Jens Axboe <jens.axboe@oracle.com> wrote:
> On Mon, Apr 26 2010, Jörn Engel wrote:
>> On Mon, 26 April 2010 19:02:48 +0200, Jens Axboe wrote:
>> > On Mon, Apr 26 2010, Jörn Engel wrote:
>> > > Otherwise we hit a BUG_ON in bdi_queue_work().
>> > >
>> > > Signed-off-by: Joern Engel <joern@logfs.org>
>> > > ---
>> > >  drivers/mtd/mtdcore.c |   17 ++++++++++++++++-
>> > >  1 files changed, 16 insertions(+), 1 deletions(-)
>> > >
>> > > diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
>> > > index cb4858b..8dd3e46 100644
>> > > --- a/drivers/mtd/mtdcore.c
>> > > +++ b/drivers/mtd/mtdcore.c
>> > > @@ -322,6 +322,12 @@ int add_mtd_device(struct mtd_info *mtd)
>> > >           if (!mtd_table[i]) {
>> > >                   struct mtd_notifier *not;
>> > >
>> > > +                 err = bdi_register(mtd->backing_dev_info, NULL, "mtd%d",
>> > > +                                 i);
>> > > +                 if (err) {
>> > > +                         /* We lose the errno information :( */
>> > > +                         break;
>> > > +                 }
>> >
>> > This is not a good idea, even if it'll currently work.
>> >
>> > > @@ -692,6 +698,15 @@ static int __init init_mtd(void)
>> > >   int ret;
>> > >   ret = class_register(&mtd_class);
>> > >
>> > > + ret = bdi_init(&mtd_bdi_unmappable);
>> > > + if (ret)
>> > > +         return ret;
>> > > + ret = bdi_init(&mtd_bdi_ro_mappable);
>> > > + if (ret)
>> > > +         return ret;
>> > > + ret = bdi_init(&mtd_bdi_rw_mappable);
>> > > + if (ret)
>> > > +         return ret;
>> >
>> > Do the bdi_register() here as well.
>>
>> -ENOBRAIN
>>
>> Initially I wanted to do just than.  Then I looked at the block layer
>> and thought we could create one backing_dev_info per mtd as well.  Not
>> necessarily a bad if I would actually _create_ them and not just reuse
>> the same ones over and over again.
>
> I cooked up that patch myself, here:
>
> http://git.kernel.dk/?p=linux-2.6-block.git;a=commit;h=0661b1ac5d48eb47c8a5948c0554fea25e0895ab
>
> Care to give it a quick spin?
>
> --
> Jens Axboe
>
>

Re: [PATCH 2/2] [MTD] Call bdi_init() and bdi_register()

[Jens Axboe]

On Tue, Apr 27 2010, Paolo Minazzi wrote:
> I Jens,
> I'm Paolo Minazzi and I have a problem with logfs and 2.6.34rc5.
> Can I apply the patch to rc5 and make a test on my ARM board ?
> Paolo

Yes please do, below are the collected fixes for mtd in this area. It
should apply to recent -git just fine.

diff --git a/drivers/mtd/Makefile b/drivers/mtd/Makefile
index 82d1e4d..4521b1e 100644
--- a/drivers/mtd/Makefile
+++ b/drivers/mtd/Makefile
@@ -4,7 +4,7 @@
 
 # Core functionality.
 obj-$(CONFIG_MTD)		+= mtd.o
-mtd-y				:= mtdcore.o mtdsuper.o mtdbdi.o
+mtd-y				:= mtdcore.o mtdsuper.o
 mtd-$(CONFIG_MTD_PARTITIONS)	+= mtdpart.o
 
 obj-$(CONFIG_MTD_CONCAT)	+= mtdconcat.o
diff --git a/drivers/mtd/internal.h b/drivers/mtd/internal.h
index c658fe7..e69de29 100644
--- a/drivers/mtd/internal.h
+++ b/drivers/mtd/internal.h
@@ -1,17 +0,0 @@
-/* Internal MTD definitions
- *
- * Copyright © 2006 Red Hat, Inc. All Rights Reserved.
- * Written by David Howells (dhowells@redhat.com)
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License
- * as published by the Free Software Foundation; either version
- * 2 of the License, or (at your option) any later version.
- */
-
-/*
- * mtdbdi.c
- */
-extern struct backing_dev_info mtd_bdi_unmappable;
-extern struct backing_dev_info mtd_bdi_ro_mappable;
-extern struct backing_dev_info mtd_bdi_rw_mappable;
diff --git a/drivers/mtd/mtdbdi.c b/drivers/mtd/mtdbdi.c
index 5ca5aed..e69de29 100644
--- a/drivers/mtd/mtdbdi.c
+++ b/drivers/mtd/mtdbdi.c
@@ -1,43 +0,0 @@
-/* MTD backing device capabilities
- *
- * Copyright © 2006 Red Hat, Inc. All Rights Reserved.
- * Written by David Howells (dhowells@redhat.com)
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License
- * as published by the Free Software Foundation; either version
- * 2 of the License, or (at your option) any later version.
- */
-
-#include <linux/backing-dev.h>
-#include <linux/mtd/mtd.h>
-#include "internal.h"
-
-/*
- * backing device capabilities for non-mappable devices (such as NAND flash)
- * - permits private mappings, copies are taken of the data
- */
-struct backing_dev_info mtd_bdi_unmappable = {
-	.capabilities	= BDI_CAP_MAP_COPY,
-};
-
-/*
- * backing device capabilities for R/O mappable devices (such as ROM)
- * - permits private mappings, copies are taken of the data
- * - permits non-writable shared mappings
- */
-struct backing_dev_info mtd_bdi_ro_mappable = {
-	.capabilities	= (BDI_CAP_MAP_COPY | BDI_CAP_MAP_DIRECT |
-			   BDI_CAP_EXEC_MAP | BDI_CAP_READ_MAP),
-};
-
-/*
- * backing device capabilities for writable mappable devices (such as RAM)
- * - permits private mappings, copies are taken of the data
- * - permits non-writable shared mappings
- */
-struct backing_dev_info mtd_bdi_rw_mappable = {
-	.capabilities	= (BDI_CAP_MAP_COPY | BDI_CAP_MAP_DIRECT |
-			   BDI_CAP_EXEC_MAP | BDI_CAP_READ_MAP |
-			   BDI_CAP_WRITE_MAP),
-};
diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index 5b38b17..b177e75 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -2,6 +2,9 @@
  * Core registration and callback routines for MTD
  * drivers and users.
  *
+ * bdi bits are:
+ * Copyright © 2006 Red Hat, Inc. All Rights Reserved.
+ * Written by David Howells (dhowells@redhat.com)
  */
 
 #include <linux/module.h>
@@ -16,11 +19,39 @@
 #include <linux/init.h>
 #include <linux/mtd/compatmac.h>
 #include <linux/proc_fs.h>
+#include <linux/backing-dev.h>
 
 #include <linux/mtd/mtd.h>
-#include "internal.h"
 
 #include "mtdcore.h"
+/*
+ * backing device capabilities for non-mappable devices (such as NAND flash)
+ * - permits private mappings, copies are taken of the data
+ */
+struct backing_dev_info mtd_bdi_unmappable = {
+	.capabilities	= BDI_CAP_MAP_COPY,
+};
+
+/*
+ * backing device capabilities for R/O mappable devices (such as ROM)
+ * - permits private mappings, copies are taken of the data
+ * - permits non-writable shared mappings
+ */
+struct backing_dev_info mtd_bdi_ro_mappable = {
+	.capabilities	= (BDI_CAP_MAP_COPY | BDI_CAP_MAP_DIRECT |
+			   BDI_CAP_EXEC_MAP | BDI_CAP_READ_MAP),
+};
+
+/*
+ * backing device capabilities for writable mappable devices (such as RAM)
+ * - permits private mappings, copies are taken of the data
+ * - permits non-writable shared mappings
+ */
+struct backing_dev_info mtd_bdi_rw_mappable = {
+	.capabilities	= (BDI_CAP_MAP_COPY | BDI_CAP_MAP_DIRECT |
+			   BDI_CAP_EXEC_MAP | BDI_CAP_READ_MAP |
+			   BDI_CAP_WRITE_MAP),
+};
 
 static int mtd_cls_suspend(struct device *dev, pm_message_t state);
 static int mtd_cls_resume(struct device *dev);
@@ -628,20 +659,55 @@ done:
 /*====================================================================*/
 /* Init code */
 
+static int __init mtd_bdi_init(struct backing_dev_info *bdi, const char *name)
+{
+	int ret;
+
+	ret = bdi_init(bdi);
+	if (!ret)
+		ret = bdi_register(bdi, NULL, name);
+
+	if (ret)
+		bdi_destroy(bdi);
+
+	return ret;
+}
+
 static int __init init_mtd(void)
 {
 	int ret;
+
 	ret = class_register(&mtd_class);
+	if (ret)
+		goto err_reg;
+
+	ret = mtd_bdi_init(&mtd_bdi_unmappable, "mtd-unmap");
+	if (ret)
+		goto err_bdi1;
+
+	ret = mtd_bdi_init(&mtd_bdi_ro_mappable, "mtd-romap");
+	if (ret)
+		goto err_bdi2;
+
+	ret = mtd_bdi_init(&mtd_bdi_rw_mappable, "mtd-rwmap");
+	if (ret)
+		goto err_bdi3;
 
-	if (ret) {
-		pr_err("Error registering mtd class: %d\n", ret);
-		return ret;
-	}
 #ifdef CONFIG_PROC_FS
 	if ((proc_mtd = create_proc_entry( "mtd", 0, NULL )))
 		proc_mtd->read_proc = mtd_read_proc;
 #endif /* CONFIG_PROC_FS */
 	return 0;
+
+err_bdi3:
+	bdi_destroy(&mtd_bdi_ro_mappable);
+err_bdi2:
+	bdi_destroy(&mtd_bdi_unmappable);
+err_bdi1:
+	class_unregister(&mtd_class);
+err_reg:
+	pr_err("Error registering mtd class or bdi: %d\n", ret);
+	return ret;
 }
 
 static void __exit cleanup_mtd(void)
@@ -651,6 +717,9 @@ static void __exit cleanup_mtd(void)
 		remove_proc_entry( "mtd", NULL);
 #endif /* CONFIG_PROC_FS */
 	class_unregister(&mtd_class);
+	bdi_destroy(&mtd_bdi_unmappable);
+	bdi_destroy(&mtd_bdi_ro_mappable);
+	bdi_destroy(&mtd_bdi_rw_mappable);
 }
 
 module_init(init_mtd);
diff --git a/drivers/mtd/mtdsuper.c b/drivers/mtd/mtdsuper.c
index af8b42e..7c00319 100644
--- a/drivers/mtd/mtdsuper.c
+++ b/drivers/mtd/mtdsuper.c
@@ -13,6 +13,7 @@
 #include <linux/mtd/super.h>
 #include <linux/namei.h>
 #include <linux/ctype.h>
+#include <linux/slab.h>
 
 /*
  * compare superblocks to see if they're equivalent
@@ -44,6 +45,7 @@ static int get_sb_mtd_set(struct super_block *sb, void *_mtd)
 
 	sb->s_mtd = mtd;
 	sb->s_dev = MKDEV(MTD_BLOCK_MAJOR, mtd->index);
+	sb->s_bdi = mtd->backing_dev_info;
 	return 0;
 }
 


-- 
Jens Axboe

Re: [PATCH 2/2] [MTD] Call bdi_init() and bdi_register()

[Jörn Engel]

On Tue, 27 April 2010 09:52:29 +0200, Jens Axboe wrote:
> 
> I cooked up that patch myself, here:
> 
> http://git.kernel.dk/?p=linux-2.6-block.git;a=commit;h=0661b1ac5d48eb47c8a5948c0554fea25e0895ab
> 
> Care to give it a quick spin?

Works for me.

Acked-and-tested-by: Joern Engel <joern@logfs.org>

Jörn

-- 
If you're willing to restrict the flexibility of your approach,
you can almost always do something better.
-- John Carmack

Re: [PATCH 2/2] [MTD] Call bdi_init() and bdi_register()

[Jens Axboe]

On Tue, Apr 27 2010, Jörn Engel wrote:
> On Tue, 27 April 2010 09:52:29 +0200, Jens Axboe wrote:
> > 
> > I cooked up that patch myself, here:
> > 
> > http://git.kernel.dk/?p=linux-2.6-block.git;a=commit;h=0661b1ac5d48eb47c8a5948c0554fea25e0895ab
> > 
> > Care to give it a quick spin?
> 
> Works for me.

Goodness, thanks for testing. Now we are pretty close to have something
we can send out.

-- 
Jens Axboe

Re: [PATCH 2/2] [MTD] Call bdi_init() and bdi_register()

[Paolo Minazzi]

I have tried this patch.
I have enabled LOGFS, but not mounted partition with it.
/dev/mtdblock1 is my romfs root partition and it is OK.

The problem is that init cannot mount my /dev/mtdblock1 romfs root.

This is the fault :
Platform: Cirrus Logic EDB9315A Board (ARM920T) Rev A
Copyright (C) 2000, 2001, 2002, Red Hat, Inc.
       |----------------------------------------------
Raw file loaded 0x00080000-0x001dce6b, assumed entry at
0x00080000-----------------------------------------------------------
RedBoot> exec -s 0x00b00000 -r 0x00a00000 -c "root=/dev/mtdblock1
console=ttyAM console=tty1"--------------------------------
ENTRY=0xc0008000-------------------------------------------------------------------------------------------------------------
LENGTH=0x00300000------------------------------------------------------------------------------------------------------------
BASE_ADDR=0x00080000---------------------------------------------------------------------------------------------------------
Using base address 0x00080000 and length
0x00300000--------------------------------------------------------------------------
Uncompressing Linux... done, booting the kernel.
Linux version 2.6.34-rc5 (root@darkstar) (gcc version 3.4.3) #18 Tue
Apr 27 10:55:55 CEST 2010
CPU: ARM920T [41129200] revision 0 (ARMv4T), cr=c0007177
CPU: VIVT data cache, VIVT instruction cache
Machine: Cirrus Logic EDB9315A Evaluation Board
Memory policy: ECC disabled, Data cache writeback
Built 1 zonelists in Zone order, mobility grouping on.  Total pages: 32512
Kernel command line: root=/dev/mtdblock1 console=ttyAM console=tty1
PID hash table entries: 512 (order: -1, 2048 bytes)
Dentry cache hash table entries: 16384 (order: 4, 65536 bytes)
Inode-cache hash table entries: 8192 (order: 3, 32768 bytes)
Memory: 128MB = 128MB total
Memory: 126928k/126928k available, 4144k reserved, 0K highmem
Virtual kernel memory layout:
    vector  : 0xffff0000 - 0xffff1000   (   4 kB)
    fixmap  : 0xfff00000 - 0xfffe0000   ( 896 kB)
    DMA     : 0xffc00000 - 0xffe00000   (   2 MB)
    vmalloc : 0xc8800000 - 0xfe800000   ( 864 MB)
    lowmem  : 0xc0000000 - 0xc8000000   ( 128 MB)
    modules : 0xbf000000 - 0xc0000000   (  16 MB)
      .init : 0xc0008000 - 0xc0036000   ( 184 kB)
      .text : 0xc0036000 - 0xc02ab000   (2516 kB)
      .data : 0xc02ac000 - 0xc02c4e00   ( 100 kB)
SLUB: Genslabs=11, HWalign=32, Order=0-3, MinObjects=0, CPUs=1, Nodes=1
Hierarchical RCU implementation.
NR_IRQS:120
VIC @fefb0000: id 0x00041190, vendor 0x41
VIC @fefc0000: id 0x00041190, vendor 0x41
Console: colour dummy device 80x30
console [tty1] enabled
Calibrating delay loop... 99.73 BogoMIPS (lpj=498688)
Mount-cache hash table entries: 512
CPU: Testing write buffer coherency: ok
NET: Registered protocol family 16
ep93xx clock: PLL1 running at 200 MHz, PLL2 at 48 MHz
ep93xx clock: FCLK 200 MHz, HCLK 100 MHz, PCLK 50 MHz
ep93xx dma_m2p: M2P DMA subsystem initialized
bio: create slab <bio-0> at 0
SCSI subsystem initialized
usbcore: registered new interface driver usbfs
usbcore: registered new interface driver hub
usbcore: registered new device driver usb
NET: Registered protocol family 2
IP route cache hash table entries: 1024 (order: 0, 4096 bytes)
TCP established hash table entries: 4096 (order: 3, 32768 bytes)
TCP bind hash table entries: 4096 (order: 2, 16384 bytes)
TCP: Hash tables configured (established 4096 bind 4096)
TCP reno registered
UDP hash table entries: 256 (order: 0, 4096 bytes)
UDP-Lite hash table entries: 256 (order: 0, 4096 bytes)
NET: Registered protocol family 1
JFFS2 version 2.2. (NAND) © 2001-2006 Red Hat, Inc.
***REGISTER LOGFS
***REGISTER LOGFS DONE ret=0
ROMFS MTD (C) 2007 Red Hat, Inc.
msgmni has been set to 247
io scheduler noop registered
io scheduler deadline registered
io scheduler cfq registered (default)
Console: switching to colour frame buffer device 100x37
graphics fb0: registered. Mode = 800x600-16
Serial: AMBA driver
apb:uart1: ttyAM0 at MMIO 0x808c0000 (irq = 52) is a AMBA
console [ttyAM0] enabled
apb:uart2: ttyAM1 at MMIO 0x808d0000 (irq = 54) is a AMBA
apb:uart3: ttyAM2 at MMIO 0x808e0000 (irq = 55) is a AMBA
dev->num_resources=1
physmap platform flash device: 02000000 at 60000000
physmap-flash.0: Found 1 x16 devices at 0x0 in 16-bit bank
physmap-flash.0: Found 1 x16 devices at 0x1000000 in 16-bit bank
 Intel/Sharp Extended Query Table at 0x0031
 Intel/Sharp Extended Query Table at 0x0031
Using buffer write method
cfi_cmdset_0001: Erase suspend on write enabled
cmdlinepart partition parsing not available
Searching for RedBoot partition table in physmap-flash.0 at offset 0x1fe0000
6 RedBoot partitions found on MTD device physmap-flash.0
Creating 6 MTD partitions on "physmap-flash.0":
0x000000000000-0x000000040000 : "RedBoot"
0x000000040000-0x000000b40000 : "ramdisk"
0x000000b40000-0x000000e40000 : "zImage"
0x000000e40000-0x000001fc0000 : "jffs2"
0x000001fc0000-0x000001fe0000 : "RedBoot config"
0x000001fe0000-0x000002000000 : "FIS directory"
PPP generic driver version 2.4.2
PPP Deflate Compression module registered
PPP BSD Compression module registered
ep93xx-eth version 0.1 loading
eth0: ep93xx on-chip ethernet, IRQ 39, 00:00:11:22:33:36
ohci_hcd: USB 1.1 'Open' Host Controller (OHCI) Driver
ep93xx-ohci ep93xx-ohci: EP93xx OHCI
ep93xx-ohci ep93xx-ohci: new USB bus registered, assigned bus number 1
ep93xx-ohci ep93xx-ohci: irq 56, io mem 0x80020000
hub 1-0:1.0: USB hub found
hub 1-0:1.0: 3 ports detected
usbcore: registered new interface driver cdc_acm
cdc_acm: v0.26:USB Abstract Control Model driver for USB modems and
ISDN adapters
Initializing USB Mass Storage driver...
usbcore: registered new interface driver usb-storage
USB Mass Storage support registered.
ep93xx-rtc ep93xx-rtc: rtc core: registered ep93xx-rtc as rtc0
usbcore: registered new interface driver usbhid
usbhid: USB HID core driver
TCP cubic registered
NET: Registered protocol family 17
Last power-down at UTC 08:47:16 on 27/04/2010
ep93xx-rtc ep93xx-rtc: setting system clock to 2010-04-27 08:47:16 UTC
(1272358036)
VFS: Cannot open root device "mtdblock1" or unknown-block(31,1)
Please append a correct "root=" boot option; here are the available partitions:
1f00             256 mtdblock0 (driver?)
1f01           11264 mtdblock1 (driver?)
1f02            3072 mtdblock2 (driver?)
1f03           17920 mtdblock3 (driver?)
1f04             128 mtdblock4 (driver?)
1f05             128 mtdblock5 (driver?)
Kernel panic - not syncing: VFS: Unable to mount root fs on unknown-block(31,1)
Backtrace:
[<c003a3dc>] (dump_backtrace+0x0/0x12c) from [<c003a520>] (dump_stack+0x18/0x1c)
 r7:c7c11000 r6:fffffffb r5:c7c11000 r4:00000000
[<c003a508>] (dump_stack+0x0/0x1c) from [<c0046628>] (panic+0x3c/0xc4)
[<c00465ec>] (panic+0x0/0xc4) from [<c0008d58>] (mount_block_root+0x110/0x2e8)
 r3:00000000 r2:20000013 r1:c7c27f58 r0:c0276b44
[<c0008c48>] (mount_block_root+0x0/0x2e8) from [<c0008f84>]
(mount_root+0x54/0x6c)
[<c0008f30>] (mount_root+0x0/0x6c) from [<c0009094>]
(prepare_namespace+0xf8/0x194)
 r5:c001a56c r4:c001a570
[<c0008f9c>] (prepare_namespace+0x0/0x194) from [<c0008aa4>]
(kernel_init+0x114/0x15c)
 r6:c0019b0c r5:c0019d5c r4:c02c4e18
[<c0008990>] (kernel_init+0x0/0x15c) from [<c0048e98>] (do_exit+0x0/0x628)
 r6:00000000 r5:00000000 r4:00000000



If I disable logfs, my root /dev/mtdblock1 is mounted correclty.

Any ideas ?

Paolo









On Mon, Apr 26, 2010 at 6:31 PM, Jörn Engel <joern@logfs.org> wrote:
> Otherwise we hit a BUG_ON in bdi_queue_work().
>
> Signed-off-by: Joern Engel <joern@logfs.org>
> ---
>  drivers/mtd/mtdcore.c |   17 ++++++++++++++++-
>  1 files changed, 16 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
> index cb4858b..8dd3e46 100644
> --- a/drivers/mtd/mtdcore.c
> +++ b/drivers/mtd/mtdcore.c
> @@ -299,7 +299,7 @@ static struct device_type mtd_devtype = {
>
>  int add_mtd_device(struct mtd_info *mtd)
>  {
> -       int i;
> +       int i, err;
>
>        if (!mtd->backing_dev_info) {
>                switch (mtd->type) {
> @@ -322,6 +322,12 @@ int add_mtd_device(struct mtd_info *mtd)
>                if (!mtd_table[i]) {
>                        struct mtd_notifier *not;
>
> +                       err = bdi_register(mtd->backing_dev_info, NULL, "mtd%d",
> +                                       i);
> +                       if (err) {
> +                               /* We lose the errno information :( */
> +                               break;
> +                       }
>                        mtd_table[i] = mtd;
>                        mtd->index = i;
>                        mtd->usecount = 0;
> @@ -692,6 +698,15 @@ static int __init init_mtd(void)
>        int ret;
>        ret = class_register(&mtd_class);
>
> +       ret = bdi_init(&mtd_bdi_unmappable);
> +       if (ret)
> +               return ret;
> +       ret = bdi_init(&mtd_bdi_ro_mappable);
> +       if (ret)
> +               return ret;
> +       ret = bdi_init(&mtd_bdi_rw_mappable);
> +       if (ret)
> +               return ret;
>        if (ret) {
>                pr_err("Error registering mtd class: %d\n", ret);
>                return ret;
> --
> 1.6.2.1
>
>

Re: [PATCH 2/2] [MTD] Call bdi_init() and bdi_register()

[Jens Axboe]

On Tue, Apr 27 2010, Paolo Minazzi wrote:
> I have tried this patch.
> I have enabled LOGFS, but not mounted partition with it.
> /dev/mtdblock1 is my romfs root partition and it is OK.
> 
> The problem is that init cannot mount my /dev/mtdblock1 romfs root.
> 
> This is the fault :
> Platform: Cirrus Logic EDB9315A Board (ARM920T) Rev A
> Copyright (C) 2000, 2001, 2002, Red Hat, Inc.
>        |----------------------------------------------
> Raw file loaded 0x00080000-0x001dce6b, assumed entry at
> 0x00080000-----------------------------------------------------------
> RedBoot> exec -s 0x00b00000 -r 0x00a00000 -c "root=/dev/mtdblock1
> console=ttyAM console=tty1"--------------------------------
> ENTRY=0xc0008000-------------------------------------------------------------------------------------------------------------
> LENGTH=0x00300000------------------------------------------------------------------------------------------------------------
> BASE_ADDR=0x00080000---------------------------------------------------------------------------------------------------------
> Using base address 0x00080000 and length
> 0x00300000--------------------------------------------------------------------------
> Uncompressing Linux... done, booting the kernel.
> Linux version 2.6.34-rc5 (root@darkstar) (gcc version 3.4.3) #18 Tue
> Apr 27 10:55:55 CEST 2010
> CPU: ARM920T [41129200] revision 0 (ARMv4T), cr=c0007177
> CPU: VIVT data cache, VIVT instruction cache
> Machine: Cirrus Logic EDB9315A Evaluation Board
> Memory policy: ECC disabled, Data cache writeback
> Built 1 zonelists in Zone order, mobility grouping on.  Total pages: 32512
> Kernel command line: root=/dev/mtdblock1 console=ttyAM console=tty1
> PID hash table entries: 512 (order: -1, 2048 bytes)
> Dentry cache hash table entries: 16384 (order: 4, 65536 bytes)
> Inode-cache hash table entries: 8192 (order: 3, 32768 bytes)
> Memory: 128MB = 128MB total
> Memory: 126928k/126928k available, 4144k reserved, 0K highmem
> Virtual kernel memory layout:
>     vector  : 0xffff0000 - 0xffff1000   (   4 kB)
>     fixmap  : 0xfff00000 - 0xfffe0000   ( 896 kB)
>     DMA     : 0xffc00000 - 0xffe00000   (   2 MB)
>     vmalloc : 0xc8800000 - 0xfe800000   ( 864 MB)
>     lowmem  : 0xc0000000 - 0xc8000000   ( 128 MB)
>     modules : 0xbf000000 - 0xc0000000   (  16 MB)
>       .init : 0xc0008000 - 0xc0036000   ( 184 kB)
>       .text : 0xc0036000 - 0xc02ab000   (2516 kB)
>       .data : 0xc02ac000 - 0xc02c4e00   ( 100 kB)
> SLUB: Genslabs=11, HWalign=32, Order=0-3, MinObjects=0, CPUs=1, Nodes=1
> Hierarchical RCU implementation.
> NR_IRQS:120
> VIC @fefb0000: id 0x00041190, vendor 0x41
> VIC @fefc0000: id 0x00041190, vendor 0x41
> Console: colour dummy device 80x30
> console [tty1] enabled
> Calibrating delay loop... 99.73 BogoMIPS (lpj=498688)
> Mount-cache hash table entries: 512
> CPU: Testing write buffer coherency: ok
> NET: Registered protocol family 16
> ep93xx clock: PLL1 running at 200 MHz, PLL2 at 48 MHz
> ep93xx clock: FCLK 200 MHz, HCLK 100 MHz, PCLK 50 MHz
> ep93xx dma_m2p: M2P DMA subsystem initialized
> bio: create slab <bio-0> at 0
> SCSI subsystem initialized
> usbcore: registered new interface driver usbfs
> usbcore: registered new interface driver hub
> usbcore: registered new device driver usb
> NET: Registered protocol family 2
> IP route cache hash table entries: 1024 (order: 0, 4096 bytes)
> TCP established hash table entries: 4096 (order: 3, 32768 bytes)
> TCP bind hash table entries: 4096 (order: 2, 16384 bytes)
> TCP: Hash tables configured (established 4096 bind 4096)
> TCP reno registered
> UDP hash table entries: 256 (order: 0, 4096 bytes)
> UDP-Lite hash table entries: 256 (order: 0, 4096 bytes)
> NET: Registered protocol family 1
> JFFS2 version 2.2. (NAND) © 2001-2006 Red Hat, Inc.
> ***REGISTER LOGFS
> ***REGISTER LOGFS DONE ret=0
> ROMFS MTD (C) 2007 Red Hat, Inc.
> msgmni has been set to 247
> io scheduler noop registered
> io scheduler deadline registered
> io scheduler cfq registered (default)
> Console: switching to colour frame buffer device 100x37
> graphics fb0: registered. Mode = 800x600-16
> Serial: AMBA driver
> apb:uart1: ttyAM0 at MMIO 0x808c0000 (irq = 52) is a AMBA
> console [ttyAM0] enabled
> apb:uart2: ttyAM1 at MMIO 0x808d0000 (irq = 54) is a AMBA
> apb:uart3: ttyAM2 at MMIO 0x808e0000 (irq = 55) is a AMBA
> dev->num_resources=1
> physmap platform flash device: 02000000 at 60000000
> physmap-flash.0: Found 1 x16 devices at 0x0 in 16-bit bank
> physmap-flash.0: Found 1 x16 devices at 0x1000000 in 16-bit bank
>  Intel/Sharp Extended Query Table at 0x0031
>  Intel/Sharp Extended Query Table at 0x0031
> Using buffer write method
> cfi_cmdset_0001: Erase suspend on write enabled
> cmdlinepart partition parsing not available
> Searching for RedBoot partition table in physmap-flash.0 at offset 0x1fe0000
> 6 RedBoot partitions found on MTD device physmap-flash.0
> Creating 6 MTD partitions on "physmap-flash.0":
> 0x000000000000-0x000000040000 : "RedBoot"
> 0x000000040000-0x000000b40000 : "ramdisk"
> 0x000000b40000-0x000000e40000 : "zImage"
> 0x000000e40000-0x000001fc0000 : "jffs2"
> 0x000001fc0000-0x000001fe0000 : "RedBoot config"
> 0x000001fe0000-0x000002000000 : "FIS directory"
> PPP generic driver version 2.4.2
> PPP Deflate Compression module registered
> PPP BSD Compression module registered
> ep93xx-eth version 0.1 loading
> eth0: ep93xx on-chip ethernet, IRQ 39, 00:00:11:22:33:36
> ohci_hcd: USB 1.1 'Open' Host Controller (OHCI) Driver
> ep93xx-ohci ep93xx-ohci: EP93xx OHCI
> ep93xx-ohci ep93xx-ohci: new USB bus registered, assigned bus number 1
> ep93xx-ohci ep93xx-ohci: irq 56, io mem 0x80020000
> hub 1-0:1.0: USB hub found
> hub 1-0:1.0: 3 ports detected
> usbcore: registered new interface driver cdc_acm
> cdc_acm: v0.26:USB Abstract Control Model driver for USB modems and
> ISDN adapters
> Initializing USB Mass Storage driver...
> usbcore: registered new interface driver usb-storage
> USB Mass Storage support registered.
> ep93xx-rtc ep93xx-rtc: rtc core: registered ep93xx-rtc as rtc0
> usbcore: registered new interface driver usbhid
> usbhid: USB HID core driver
> TCP cubic registered
> NET: Registered protocol family 17
> Last power-down at UTC 08:47:16 on 27/04/2010
> ep93xx-rtc ep93xx-rtc: setting system clock to 2010-04-27 08:47:16 UTC
> (1272358036)
> VFS: Cannot open root device "mtdblock1" or unknown-block(31,1)
> Please append a correct "root=" boot option; here are the available partitions:
> 1f00             256 mtdblock0 (driver?)
> 1f01           11264 mtdblock1 (driver?)
> 1f02            3072 mtdblock2 (driver?)
> 1f03           17920 mtdblock3 (driver?)
> 1f04             128 mtdblock4 (driver?)
> 1f05             128 mtdblock5 (driver?)
> Kernel panic - not syncing: VFS: Unable to mount root fs on unknown-block(31,1)
> Backtrace:
> [<c003a3dc>] (dump_backtrace+0x0/0x12c) from [<c003a520>] (dump_stack+0x18/0x1c)
>  r7:c7c11000 r6:fffffffb r5:c7c11000 r4:00000000
> [<c003a508>] (dump_stack+0x0/0x1c) from [<c0046628>] (panic+0x3c/0xc4)
> [<c00465ec>] (panic+0x0/0xc4) from [<c0008d58>] (mount_block_root+0x110/0x2e8)
>  r3:00000000 r2:20000013 r1:c7c27f58 r0:c0276b44
> [<c0008c48>] (mount_block_root+0x0/0x2e8) from [<c0008f84>]
> (mount_root+0x54/0x6c)
> [<c0008f30>] (mount_root+0x0/0x6c) from [<c0009094>]
> (prepare_namespace+0xf8/0x194)
>  r5:c001a56c r4:c001a570
> [<c0008f9c>] (prepare_namespace+0x0/0x194) from [<c0008aa4>]
> (kernel_init+0x114/0x15c)
>  r6:c0019b0c r5:c0019d5c r4:c02c4e18
> [<c0008990>] (kernel_init+0x0/0x15c) from [<c0048e98>] (do_exit+0x0/0x628)
>  r6:00000000 r5:00000000 r4:00000000
> 
> 
> 
> If I disable logfs, my root /dev/mtdblock1 is mounted correclty.

So this issue looks unrelated, but a bug none the less (if just enabling
logfs breaks the mount).

-- 
Jens Axboe

Re: [PATCH 2/2] [MTD] Call bdi_init() and bdi_register()

[Paolo Minazzi]

To be precise, I say :

- downloaded 2.6.34rc5
- apply the jens patch
- the logfs code is the code in the 2.6.34rc5 (I have no applied other
jorn patch)

Paolo

On Tue, Apr 27, 2010 at 11:16 AM, Jens Axboe <jens.axboe@oracle.com> wrote:
> On Tue, Apr 27 2010, Paolo Minazzi wrote:
>> I have tried this patch.
>> I have enabled LOGFS, but not mounted partition with it.
>> /dev/mtdblock1 is my romfs root partition and it is OK.
>>
>> The problem is that init cannot mount my /dev/mtdblock1 romfs root.
>>
>> This is the fault :
>> Platform: Cirrus Logic EDB9315A Board (ARM920T) Rev A
>> Copyright (C) 2000, 2001, 2002, Red Hat, Inc.
>>        |----------------------------------------------
>> Raw file loaded 0x00080000-0x001dce6b, assumed entry at
>> 0x00080000-----------------------------------------------------------
>> RedBoot> exec -s 0x00b00000 -r 0x00a00000 -c "root=/dev/mtdblock1
>> console=ttyAM console=tty1"--------------------------------
>> ENTRY=0xc0008000-------------------------------------------------------------------------------------------------------------
>> LENGTH=0x00300000------------------------------------------------------------------------------------------------------------
>> BASE_ADDR=0x00080000---------------------------------------------------------------------------------------------------------
>> Using base address 0x00080000 and length
>> 0x00300000--------------------------------------------------------------------------
>> Uncompressing Linux... done, booting the kernel.
>> Linux version 2.6.34-rc5 (root@darkstar) (gcc version 3.4.3) #18 Tue
>> Apr 27 10:55:55 CEST 2010
>> CPU: ARM920T [41129200] revision 0 (ARMv4T), cr=c0007177
>> CPU: VIVT data cache, VIVT instruction cache
>> Machine: Cirrus Logic EDB9315A Evaluation Board
>> Memory policy: ECC disabled, Data cache writeback
>> Built 1 zonelists in Zone order, mobility grouping on.  Total pages: 32512
>> Kernel command line: root=/dev/mtdblock1 console=ttyAM console=tty1
>> PID hash table entries: 512 (order: -1, 2048 bytes)
>> Dentry cache hash table entries: 16384 (order: 4, 65536 bytes)
>> Inode-cache hash table entries: 8192 (order: 3, 32768 bytes)
>> Memory: 128MB = 128MB total
>> Memory: 126928k/126928k available, 4144k reserved, 0K highmem
>> Virtual kernel memory layout:
>>     vector  : 0xffff0000 - 0xffff1000   (   4 kB)
>>     fixmap  : 0xfff00000 - 0xfffe0000   ( 896 kB)
>>     DMA     : 0xffc00000 - 0xffe00000   (   2 MB)
>>     vmalloc : 0xc8800000 - 0xfe800000   ( 864 MB)
>>     lowmem  : 0xc0000000 - 0xc8000000   ( 128 MB)
>>     modules : 0xbf000000 - 0xc0000000   (  16 MB)
>>       .init : 0xc0008000 - 0xc0036000   ( 184 kB)
>>       .text : 0xc0036000 - 0xc02ab000   (2516 kB)
>>       .data : 0xc02ac000 - 0xc02c4e00   ( 100 kB)
>> SLUB: Genslabs=11, HWalign=32, Order=0-3, MinObjects=0, CPUs=1, Nodes=1
>> Hierarchical RCU implementation.
>> NR_IRQS:120
>> VIC @fefb0000: id 0x00041190, vendor 0x41
>> VIC @fefc0000: id 0x00041190, vendor 0x41
>> Console: colour dummy device 80x30
>> console [tty1] enabled
>> Calibrating delay loop... 99.73 BogoMIPS (lpj=498688)
>> Mount-cache hash table entries: 512
>> CPU: Testing write buffer coherency: ok
>> NET: Registered protocol family 16
>> ep93xx clock: PLL1 running at 200 MHz, PLL2 at 48 MHz
>> ep93xx clock: FCLK 200 MHz, HCLK 100 MHz, PCLK 50 MHz
>> ep93xx dma_m2p: M2P DMA subsystem initialized
>> bio: create slab <bio-0> at 0
>> SCSI subsystem initialized
>> usbcore: registered new interface driver usbfs
>> usbcore: registered new interface driver hub
>> usbcore: registered new device driver usb
>> NET: Registered protocol family 2
>> IP route cache hash table entries: 1024 (order: 0, 4096 bytes)
>> TCP established hash table entries: 4096 (order: 3, 32768 bytes)
>> TCP bind hash table entries: 4096 (order: 2, 16384 bytes)
>> TCP: Hash tables configured (established 4096 bind 4096)
>> TCP reno registered
>> UDP hash table entries: 256 (order: 0, 4096 bytes)
>> UDP-Lite hash table entries: 256 (order: 0, 4096 bytes)
>> NET: Registered protocol family 1
>> JFFS2 version 2.2. (NAND) © 2001-2006 Red Hat, Inc.
>> ***REGISTER LOGFS
>> ***REGISTER LOGFS DONE ret=0
>> ROMFS MTD (C) 2007 Red Hat, Inc.
>> msgmni has been set to 247
>> io scheduler noop registered
>> io scheduler deadline registered
>> io scheduler cfq registered (default)
>> Console: switching to colour frame buffer device 100x37
>> graphics fb0: registered. Mode = 800x600-16
>> Serial: AMBA driver
>> apb:uart1: ttyAM0 at MMIO 0x808c0000 (irq = 52) is a AMBA
>> console [ttyAM0] enabled
>> apb:uart2: ttyAM1 at MMIO 0x808d0000 (irq = 54) is a AMBA
>> apb:uart3: ttyAM2 at MMIO 0x808e0000 (irq = 55) is a AMBA
>> dev->num_resources=1
>> physmap platform flash device: 02000000 at 60000000
>> physmap-flash.0: Found 1 x16 devices at 0x0 in 16-bit bank
>> physmap-flash.0: Found 1 x16 devices at 0x1000000 in 16-bit bank
>>  Intel/Sharp Extended Query Table at 0x0031
>>  Intel/Sharp Extended Query Table at 0x0031
>> Using buffer write method
>> cfi_cmdset_0001: Erase suspend on write enabled
>> cmdlinepart partition parsing not available
>> Searching for RedBoot partition table in physmap-flash.0 at offset 0x1fe0000
>> 6 RedBoot partitions found on MTD device physmap-flash.0
>> Creating 6 MTD partitions on "physmap-flash.0":
>> 0x000000000000-0x000000040000 : "RedBoot"
>> 0x000000040000-0x000000b40000 : "ramdisk"
>> 0x000000b40000-0x000000e40000 : "zImage"
>> 0x000000e40000-0x000001fc0000 : "jffs2"
>> 0x000001fc0000-0x000001fe0000 : "RedBoot config"
>> 0x000001fe0000-0x000002000000 : "FIS directory"
>> PPP generic driver version 2.4.2
>> PPP Deflate Compression module registered
>> PPP BSD Compression module registered
>> ep93xx-eth version 0.1 loading
>> eth0: ep93xx on-chip ethernet, IRQ 39, 00:00:11:22:33:36
>> ohci_hcd: USB 1.1 'Open' Host Controller (OHCI) Driver
>> ep93xx-ohci ep93xx-ohci: EP93xx OHCI
>> ep93xx-ohci ep93xx-ohci: new USB bus registered, assigned bus number 1
>> ep93xx-ohci ep93xx-ohci: irq 56, io mem 0x80020000
>> hub 1-0:1.0: USB hub found
>> hub 1-0:1.0: 3 ports detected
>> usbcore: registered new interface driver cdc_acm
>> cdc_acm: v0.26:USB Abstract Control Model driver for USB modems and
>> ISDN adapters
>> Initializing USB Mass Storage driver...
>> usbcore: registered new interface driver usb-storage
>> USB Mass Storage support registered.
>> ep93xx-rtc ep93xx-rtc: rtc core: registered ep93xx-rtc as rtc0
>> usbcore: registered new interface driver usbhid
>> usbhid: USB HID core driver
>> TCP cubic registered
>> NET: Registered protocol family 17
>> Last power-down at UTC 08:47:16 on 27/04/2010
>> ep93xx-rtc ep93xx-rtc: setting system clock to 2010-04-27 08:47:16 UTC
>> (1272358036)
>> VFS: Cannot open root device "mtdblock1" or unknown-block(31,1)
>> Please append a correct "root=" boot option; here are the available partitions:
>> 1f00             256 mtdblock0 (driver?)
>> 1f01           11264 mtdblock1 (driver?)
>> 1f02            3072 mtdblock2 (driver?)
>> 1f03           17920 mtdblock3 (driver?)
>> 1f04             128 mtdblock4 (driver?)
>> 1f05             128 mtdblock5 (driver?)
>> Kernel panic - not syncing: VFS: Unable to mount root fs on unknown-block(31,1)
>> Backtrace:
>> [<c003a3dc>] (dump_backtrace+0x0/0x12c) from [<c003a520>] (dump_stack+0x18/0x1c)
>>  r7:c7c11000 r6:fffffffb r5:c7c11000 r4:00000000
>> [<c003a508>] (dump_stack+0x0/0x1c) from [<c0046628>] (panic+0x3c/0xc4)
>> [<c00465ec>] (panic+0x0/0xc4) from [<c0008d58>] (mount_block_root+0x110/0x2e8)
>>  r3:00000000 r2:20000013 r1:c7c27f58 r0:c0276b44
>> [<c0008c48>] (mount_block_root+0x0/0x2e8) from [<c0008f84>]
>> (mount_root+0x54/0x6c)
>> [<c0008f30>] (mount_root+0x0/0x6c) from [<c0009094>]
>> (prepare_namespace+0xf8/0x194)
>>  r5:c001a56c r4:c001a570
>> [<c0008f9c>] (prepare_namespace+0x0/0x194) from [<c0008aa4>]
>> (kernel_init+0x114/0x15c)
>>  r6:c0019b0c r5:c0019d5c r4:c02c4e18
>> [<c0008990>] (kernel_init+0x0/0x15c) from [<c0048e98>] (do_exit+0x0/0x628)
>>  r6:00000000 r5:00000000 r4:00000000
>>
>>
>>
>> If I disable logfs, my root /dev/mtdblock1 is mounted correclty.
>
> So this issue looks unrelated, but a bug none the less (if just enabling
> logfs breaks the mount).
>
> --
> Jens Axboe
>
>

Re: [PATCH 2/2] [MTD] Call bdi_init() and bdi_register()

[Jens Axboe]

On Tue, Apr 27 2010, Paolo Minazzi wrote:
> On Tue, Apr 27, 2010 at 11:16 AM, Jens Axboe <jens.axboe@oracle.com> wrote:
> > On Tue, Apr 27 2010, Paolo Minazzi wrote:
> >> I have tried this patch.
> >> I have enabled LOGFS, but not mounted partition with it.
> >> /dev/mtdblock1 is my romfs root partition and it is OK.
> >>
> >> The problem is that init cannot mount my /dev/mtdblock1 romfs root.
> >>
> >> This is the fault :
> >> Platform: Cirrus Logic EDB9315A Board (ARM920T) Rev A
> >> Copyright (C) 2000, 2001, 2002, Red Hat, Inc.
> >>        |----------------------------------------------
> >> Raw file loaded 0x00080000-0x001dce6b, assumed entry at
> >> 0x00080000-----------------------------------------------------------
> >> RedBoot> exec -s 0x00b00000 -r 0x00a00000 -c "root=/dev/mtdblock1
> >> console=ttyAM console=tty1"--------------------------------
> >> ENTRY=0xc0008000-------------------------------------------------------------------------------------------------------------
> >> LENGTH=0x00300000------------------------------------------------------------------------------------------------------------
> >> BASE_ADDR=0x00080000---------------------------------------------------------------------------------------------------------
> >> Using base address 0x00080000 and length
> >> 0x00300000--------------------------------------------------------------------------
> >> Uncompressing Linux... done, booting the kernel.
> >> Linux version 2.6.34-rc5 (root@darkstar) (gcc version 3.4.3) #18 Tue
> >> Apr 27 10:55:55 CEST 2010
> >> CPU: ARM920T [41129200] revision 0 (ARMv4T), cr=c0007177
> >> CPU: VIVT data cache, VIVT instruction cache
> >> Machine: Cirrus Logic EDB9315A Evaluation Board
> >> Memory policy: ECC disabled, Data cache writeback
> >> Built 1 zonelists in Zone order, mobility grouping on.  Total pages: 32512
> >> Kernel command line: root=/dev/mtdblock1 console=ttyAM console=tty1
> >> PID hash table entries: 512 (order: -1, 2048 bytes)
> >> Dentry cache hash table entries: 16384 (order: 4, 65536 bytes)
> >> Inode-cache hash table entries: 8192 (order: 3, 32768 bytes)
> >> Memory: 128MB = 128MB total
> >> Memory: 126928k/126928k available, 4144k reserved, 0K highmem
> >> Virtual kernel memory layout:
> >>     vector  : 0xffff0000 - 0xffff1000   (   4 kB)
> >>     fixmap  : 0xfff00000 - 0xfffe0000   ( 896 kB)
> >>     DMA     : 0xffc00000 - 0xffe00000   (   2 MB)
> >>     vmalloc : 0xc8800000 - 0xfe800000   ( 864 MB)
> >>     lowmem  : 0xc0000000 - 0xc8000000   ( 128 MB)
> >>     modules : 0xbf000000 - 0xc0000000   (  16 MB)
> >>       .init : 0xc0008000 - 0xc0036000   ( 184 kB)
> >>       .text : 0xc0036000 - 0xc02ab000   (2516 kB)
> >>       .data : 0xc02ac000 - 0xc02c4e00   ( 100 kB)
> >> SLUB: Genslabs=11, HWalign=32, Order=0-3, MinObjects=0, CPUs=1, Nodes=1
> >> Hierarchical RCU implementation.
> >> NR_IRQS:120
> >> VIC @fefb0000: id 0x00041190, vendor 0x41
> >> VIC @fefc0000: id 0x00041190, vendor 0x41
> >> Console: colour dummy device 80x30
> >> console [tty1] enabled
> >> Calibrating delay loop... 99.73 BogoMIPS (lpj=498688)
> >> Mount-cache hash table entries: 512
> >> CPU: Testing write buffer coherency: ok
> >> NET: Registered protocol family 16
> >> ep93xx clock: PLL1 running at 200 MHz, PLL2 at 48 MHz
> >> ep93xx clock: FCLK 200 MHz, HCLK 100 MHz, PCLK 50 MHz
> >> ep93xx dma_m2p: M2P DMA subsystem initialized
> >> bio: create slab <bio-0> at 0
> >> SCSI subsystem initialized
> >> usbcore: registered new interface driver usbfs
> >> usbcore: registered new interface driver hub
> >> usbcore: registered new device driver usb
> >> NET: Registered protocol family 2
> >> IP route cache hash table entries: 1024 (order: 0, 4096 bytes)
> >> TCP established hash table entries: 4096 (order: 3, 32768 bytes)
> >> TCP bind hash table entries: 4096 (order: 2, 16384 bytes)
> >> TCP: Hash tables configured (established 4096 bind 4096)
> >> TCP reno registered
> >> UDP hash table entries: 256 (order: 0, 4096 bytes)
> >> UDP-Lite hash table entries: 256 (order: 0, 4096 bytes)
> >> NET: Registered protocol family 1
> >> JFFS2 version 2.2. (NAND) © 2001-2006 Red Hat, Inc.
> >> ***REGISTER LOGFS
> >> ***REGISTER LOGFS DONE ret=0
> >> ROMFS MTD (C) 2007 Red Hat, Inc.
> >> msgmni has been set to 247
> >> io scheduler noop registered
> >> io scheduler deadline registered
> >> io scheduler cfq registered (default)
> >> Console: switching to colour frame buffer device 100x37
> >> graphics fb0: registered. Mode = 800x600-16
> >> Serial: AMBA driver
> >> apb:uart1: ttyAM0 at MMIO 0x808c0000 (irq = 52) is a AMBA
> >> console [ttyAM0] enabled
> >> apb:uart2: ttyAM1 at MMIO 0x808d0000 (irq = 54) is a AMBA
> >> apb:uart3: ttyAM2 at MMIO 0x808e0000 (irq = 55) is a AMBA
> >> dev->num_resources=1
> >> physmap platform flash device: 02000000 at 60000000
> >> physmap-flash.0: Found 1 x16 devices at 0x0 in 16-bit bank
> >> physmap-flash.0: Found 1 x16 devices at 0x1000000 in 16-bit bank
> >>  Intel/Sharp Extended Query Table at 0x0031
> >>  Intel/Sharp Extended Query Table at 0x0031
> >> Using buffer write method
> >> cfi_cmdset_0001: Erase suspend on write enabled
> >> cmdlinepart partition parsing not available
> >> Searching for RedBoot partition table in physmap-flash.0 at offset 0x1fe0000
> >> 6 RedBoot partitions found on MTD device physmap-flash.0
> >> Creating 6 MTD partitions on "physmap-flash.0":
> >> 0x000000000000-0x000000040000 : "RedBoot"
> >> 0x000000040000-0x000000b40000 : "ramdisk"
> >> 0x000000b40000-0x000000e40000 : "zImage"
> >> 0x000000e40000-0x000001fc0000 : "jffs2"
> >> 0x000001fc0000-0x000001fe0000 : "RedBoot config"
> >> 0x000001fe0000-0x000002000000 : "FIS directory"
> >> PPP generic driver version 2.4.2
> >> PPP Deflate Compression module registered
> >> PPP BSD Compression module registered
> >> ep93xx-eth version 0.1 loading
> >> eth0: ep93xx on-chip ethernet, IRQ 39, 00:00:11:22:33:36
> >> ohci_hcd: USB 1.1 'Open' Host Controller (OHCI) Driver
> >> ep93xx-ohci ep93xx-ohci: EP93xx OHCI
> >> ep93xx-ohci ep93xx-ohci: new USB bus registered, assigned bus number 1
> >> ep93xx-ohci ep93xx-ohci: irq 56, io mem 0x80020000
> >> hub 1-0:1.0: USB hub found
> >> hub 1-0:1.0: 3 ports detected
> >> usbcore: registered new interface driver cdc_acm
> >> cdc_acm: v0.26:USB Abstract Control Model driver for USB modems and
> >> ISDN adapters
> >> Initializing USB Mass Storage driver...
> >> usbcore: registered new interface driver usb-storage
> >> USB Mass Storage support registered.
> >> ep93xx-rtc ep93xx-rtc: rtc core: registered ep93xx-rtc as rtc0
> >> usbcore: registered new interface driver usbhid
> >> usbhid: USB HID core driver
> >> TCP cubic registered
> >> NET: Registered protocol family 17
> >> Last power-down at UTC 08:47:16 on 27/04/2010
> >> ep93xx-rtc ep93xx-rtc: setting system clock to 2010-04-27 08:47:16 UTC
> >> (1272358036)
> >> VFS: Cannot open root device "mtdblock1" or unknown-block(31,1)
> >> Please append a correct "root=" boot option; here are the available partitions:
> >> 1f00             256 mtdblock0 (driver?)
> >> 1f01           11264 mtdblock1 (driver?)
> >> 1f02            3072 mtdblock2 (driver?)
> >> 1f03           17920 mtdblock3 (driver?)
> >> 1f04             128 mtdblock4 (driver?)
> >> 1f05             128 mtdblock5 (driver?)
> >> Kernel panic - not syncing: VFS: Unable to mount root fs on unknown-block(31,1)
> >> Backtrace:
> >> [<c003a3dc>] (dump_backtrace+0x0/0x12c) from [<c003a520>] (dump_stack+0x18/0x1c)
> >>  r7:c7c11000 r6:fffffffb r5:c7c11000 r4:00000000
> >> [<c003a508>] (dump_stack+0x0/0x1c) from [<c0046628>] (panic+0x3c/0xc4)
> >> [<c00465ec>] (panic+0x0/0xc4) from [<c0008d58>] (mount_block_root+0x110/0x2e8)
> >>  r3:00000000 r2:20000013 r1:c7c27f58 r0:c0276b44
> >> [<c0008c48>] (mount_block_root+0x0/0x2e8) from [<c0008f84>]
> >> (mount_root+0x54/0x6c)
> >> [<c0008f30>] (mount_root+0x0/0x6c) from [<c0009094>]
> >> (prepare_namespace+0xf8/0x194)
> >>  r5:c001a56c r4:c001a570
> >> [<c0008f9c>] (prepare_namespace+0x0/0x194) from [<c0008aa4>]
> >> (kernel_init+0x114/0x15c)
> >>  r6:c0019b0c r5:c0019d5c r4:c02c4e18
> >> [<c0008990>] (kernel_init+0x0/0x15c) from [<c0048e98>] (do_exit+0x0/0x628)
> >>  r6:00000000 r5:00000000 r4:00000000
> >>
> >>
> >>
> >> If I disable logfs, my root /dev/mtdblock1 is mounted correclty.
> >
> > So this issue looks unrelated, but a bug none the less (if just enabling
> > logfs breaks the mount).
>
> To be precise, I say :
> 
> - downloaded 2.6.34rc5
> - apply the jens patch
> - the logfs code is the code in the 2.6.34rc5 (I have no applied other
> jorn patch)

(please stop top posting, I fixed this one up for you).

Just to be on the safe side - without the patch, does it work or not?

-- 
Jens Axboe

Re: [PATCH 2/2] [MTD] Call bdi_init() and bdi_register()

[Paolo Minazzi]

> Just to be on the safe side - without the patch, does it work or not?
>
> --
> Jens Axboe

Without the patch for me means the standard 2.6.34rc5.
In this case :
- enabling logfs I have an oop in fs/write-back.c:157 before kernel calls init.
- disabling logfs I can do normal mount my root /dev/mtdblock1

This is my experience.

Paolo

Re: [PATCH 2/2] [MTD] Call bdi_init() and bdi_register()

[Jörn Engel]

On Tue, 27 April 2010 11:01:04 +0200, Paolo Minazzi wrote:
> 
> I have tried this patch.
> I have enabled LOGFS, but not mounted partition with it.
> /dev/mtdblock1 is my romfs root partition and it is OK.
> 
> The problem is that init cannot mount my /dev/mtdblock1 romfs root.
> 
> This is the fault :
> Platform: Cirrus Logic EDB9315A Board (ARM920T) Rev A
> Copyright (C) 2000, 2001, 2002, Red Hat, Inc.
>        |----------------------------------------------
> Raw file loaded 0x00080000-0x001dce6b, assumed entry at
> 0x00080000-----------------------------------------------------------
> RedBoot> exec -s 0x00b00000 -r 0x00a00000 -c "root=/dev/mtdblock1
> console=ttyAM console=tty1"--------------------------------
> ENTRY=0xc0008000-------------------------------------------------------------------------------------------------------------
> LENGTH=0x00300000------------------------------------------------------------------------------------------------------------
> BASE_ADDR=0x00080000---------------------------------------------------------------------------------------------------------
> Using base address 0x00080000 and length
> 0x00300000--------------------------------------------------------------------------
> Uncompressing Linux... done, booting the kernel.
> Linux version 2.6.34-rc5 (root@darkstar) (gcc version 3.4.3) #18 Tue
> Apr 27 10:55:55 CEST 2010
> CPU: ARM920T [41129200] revision 0 (ARMv4T), cr=c0007177
> CPU: VIVT data cache, VIVT instruction cache
> Machine: Cirrus Logic EDB9315A Evaluation Board
> Memory policy: ECC disabled, Data cache writeback
> Built 1 zonelists in Zone order, mobility grouping on.  Total pages: 32512
> Kernel command line: root=/dev/mtdblock1 console=ttyAM console=tty1

If you add "rootfstype=romfs" to the command line, does the problem
still exist?

Jörn

-- 
And spam is a useful source of entropy for /dev/random too!
-- Jasmine Strong

Re: [PATCH 2/2] [MTD] Call bdi_init() and bdi_register()

[Paolo Minazzi]

> If you add "rootfstype=romfs" to the command line, does the problem
> still exist?
>
> Jörn

Jorn , you are right.
It seems work....
please wait....

Re: [PATCH 2/2] [MTD] Call bdi_init() and bdi_register()

[Jörn Engel]

On Tue, 27 April 2010 13:31:11 +0200, Paolo Minazzi wrote:
> 
> > If you add "rootfstype=romfs" to the command line, does the problem
> > still exist?
> 
> Jorn , you are right.
> It seems work....
> please wait....

Ok, I'm pretty sure that logfs returns -EIO where it should return
-EINVAL.  If filesystems are tried in alphabetical order, logfs comes
first and -EIO tells the kernel to stop trying and panic, essentially.

Will cook up a patch...

Jörn

-- 
A defeated army first battles and then seeks victory.
-- Sun Tzu

[PATCH] [LogFS] Return -EINVAL if filesystem image doesn't match

[Jörn Engel]

On Tue, 27 April 2010 13:40:04 +0200, Jörn Engel wrote:
> On Tue, 27 April 2010 13:31:11 +0200, Paolo Minazzi wrote:
> > 
> > > If you add "rootfstype=romfs" to the command line, does the problem
> > > still exist?
> > 
> > Jorn , you are right.
> > It seems work....
> > please wait....
> 
> Ok, I'm pretty sure that logfs returns -EIO where it should return
> -EINVAL.  If filesystems are tried in alphabetical order, logfs comes
> first and -EIO tells the kernel to stop trying and panic, essentially.
> 
> Will cook up a patch...

Does the patch below solve the problem for you (without the explicit
"rootfstype=romfs")?

Jörn

-- 
One of my most productive days was throwing away 1000 lines of code.
-- Ken Thompson.


Signed-off-by: Joern Engel <joern@logfs.org>
---
 fs/logfs/super.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/fs/logfs/super.c b/fs/logfs/super.c
index 5866ee6..f649038 100644
--- a/fs/logfs/super.c
+++ b/fs/logfs/super.c
@@ -413,7 +413,7 @@ static int __logfs_read_sb(struct super_block *sb)
 
 	page = find_super_block(sb);
 	if (!page)
-		return -EIO;
+		return -EINVAL;
 
 	ds = page_address(page);
 	super->s_size = be64_to_cpu(ds->ds_filesystem_size);
-- 
1.6.2.1

Re: [PATCH] [LogFS] Return -EINVAL if filesystem image doesn't match

[Paolo Minazzi]

> Does the patch below solve the problem for you (without the explicit
> "rootfstype=romfs")?
>
> Jörn
>
> --
> One of my most productive days was throwing away 1000 lines of code.
> -- Ken Thompson.
>
>
> Signed-off-by: Joern Engel <joern@logfs.org>
> ---
>  fs/logfs/super.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/fs/logfs/super.c b/fs/logfs/super.c
> index 5866ee6..f649038 100644
> --- a/fs/logfs/super.c
> +++ b/fs/logfs/super.c
> @@ -413,7 +413,7 @@ static int __logfs_read_sb(struct super_block *sb)
>
>        page = find_super_block(sb);
>        if (!page)
> -               return -EIO;
> +               return -EINVAL;
>
>        ds = page_address(page);
>        super->s_size = be64_to_cpu(ds->ds_filesystem_size);

The above patch solve the problem for me.
Now I can mount root without rootfstype=romfs

Bye,
Paolo

Re: [PATCH 2/2] [MTD] Call bdi_init() and bdi_register()

[Paolo Minazzi]

Ok, now it is very better.
My problem now is that logfs does not save files in the following case:
1) power-off, then power-on (without umount)
2) sync, then power-off, then power-on (without umount)

Using umount sometimes seems works, sometimes I have the following oop :

~ # mount -t logfs /dev/mtdblock7 /mitrolbackup
~ # cd /mitrolbackup
/mitrolbackup # echo ciao > ciao
/mitrolbackup # echo pino > pino
/mitrolbackup # ls
ciao  pino
/mitrolbackup # cd ..
~ # umount /mitrolbackup/
kernel BUG at fs/logfs/readwrite.c:1976!
Unable to handle kernel NULL pointer dereference at virtual address 00000000
pgd = c7db0000
[00000000] *pgd=c7dac031, *pte=00000000, *ppte=00000000
Internal error: Oops: 817 [#1]
last sysfs file:
Modules linked in:
CPU: 0    Not tainted  (2.6.34-rc5 #19)
PC is at __bug+0x20/0x2c
LR is at release_console_sem+0x1b8/0x1ec
pc : [<c003a1ac>]    lr : [<c004760c>]    psr: 60000013
sp : c7d5fe60  ip : c7d5fd98  fp : c7d5fe6c
r10: 00000003  r9 : 00000000  r8 : c7d5feb4
r7 : 00000003  r6 : c7d5feb4  r5 : c7819878  r4 : c7819870
r3 : 00000000  r2 : 60000013  r1 : 000019c2  r0 : 0000002f
Flags: nZCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment user
Control: c000717f  Table: c7db0000  DAC: 00000015
Process umount (pid: 307, stack limit = 0xc7d5e260)
Stack: (0xc7d5fe60 to 0xc7d60000)
fe60: c7d5fe7c c7d5fe70 c00fb2f0 c003a19c c7d5fe90 c7d5fe80 c00ad358 c00fb274
fe80: c7819870 c7d5feb0 c7d5fe94 c00ad3e4 c00ad2bc c7d46a64 c7d46a54 c7819880
fea0: c7d46a64 c7d5fee4 c7d5feb4 c00ad5a8 c00ad3d0 c7d5feb4 c7d5feb4 c7d46a00
fec0: c7d5e000 c022a324 00000000 c7d5ff54 c7d2b880 00000000 c7d5ff00 c7d5fee8
fee0: c009c8ec c00ad4c0 c7d46a00 c7c14000 c7d2b880 c7d5ff1c c7d5ff04 c00feb38
ff00: c009c84c c7d46a00 c02b7b84 c7d2b880 c7d5ff34 c7d5ff20 c009c740 c00feb00
ff20: c7d2b880 c7d46a00 c7d5ff4c c7d5ff38 c00b0bd0 c009c708 00000000 c7d2b898
ff40: c7d5ff94 c7d5ff50 c00b1e90 c00b0b7c 00000000 c7d5ff54 c7d5ff54 c7d5ff5c
ff60: c7d5ff5c c7d2b880 c7817500 be8e3e0c 00000000 be8e5fb4 00000016 c00370a4
ff80: c7d5e000 be8e5eec c7d5ffa4 c7d5ff98 c00b2164 c00b1e34 00000000 c7d5ffa8
ffa0: c0036f00 c00b2160 be8e3e0c 00000000 be8e5fb4 be8e3e0c be8e3e1a 00000000
ffc0: be8e3e0c 00000000 be8e5fb4 00000000 00000000 00000000 be8e5eec 00000000
ffe0: 40099450 be8e3de4 0006779c 40099458 20000010 be8e5fb4 e0830007 e0833006
Backtrace:
[<c003a18c>] (__bug+0x0/0x2c) from [<c00fb2f0>] (logfs_clear_inode+0x8c/0xb0)
[<c00fb264>] (logfs_clear_inode+0x0/0xb0) from [<c00ad358>]
(clear_inode+0xac/0x114)
[<c00ad2ac>] (clear_inode+0x0/0x114) from [<c00ad3e4>] (dispose_list+0x24/0xf0)
 r4:c7819870
[<c00ad3c0>] (dispose_list+0x0/0xf0) from [<c00ad5a8>]
(invalidate_inodes+0xf8/0x12c)
 r7:c7d46a64 r6:c7819880 r5:c7d46a54 r4:c7d46a64
[<c00ad4b0>] (invalidate_inodes+0x0/0x12c) from [<c009c8ec>]
(generic_shutdown_super+0xb0/0x11c)
[<c009c83c>] (generic_shutdown_super+0x0/0x11c) from [<c00feb38>]
(logfs_kill_sb+0x48/0xfc)
 r6:c7d2b880 r5:c7c14000 r4:c7d46a00
[<c00feaf0>] (logfs_kill_sb+0x0/0xfc) from [<c009c740>]
(deactivate_super+0x48/0x60)
 r6:c7d2b880 r5:c02b7b84 r4:c7d46a00
[<c009c6f8>] (deactivate_super+0x0/0x60) from [<c00b0bd0>]
(mntput_no_expire+0x64/0xbc)
 r5:c7d46a00 r4:c7d2b880
[<c00b0b6c>] (mntput_no_expire+0x0/0xbc) from [<c00b1e90>]
(sys_umount+0x6c/0x32c)
 r5:c7d2b898 r4:00000000
[<c00b1e24>] (sys_umount+0x0/0x32c) from [<c00b2164>] (sys_oldumount+0x14/0x18)
[<c00b2150>] (sys_oldumount+0x0/0x18) from [<c0036f00>]
(ret_fast_syscall+0x0/0x2c)
Code: e1a01000 e59f000c eb003605 e3a03000 (e5833000)
---[ end trace 979e02c61e928049 ]---
------------[ cut here ]------------
WARNING: at kernel/exit.c:907 do_exit+0x50c/0x628()
Modules linked in:
Backtrace:
[<c003a3dc>] (dump_backtrace+0x0/0x12c) from [<c003a520>] (dump_stack+0x18/0x1c)
 r7:00000000 r6:c00493a4 r5:c02794cc r4:0000038b
[<c003a508>] (dump_stack+0x0/0x1c) from [<c0046a38>]
(warn_slowpath_common+0x4c/0x64)
[<c00469ec>] (warn_slowpath_common+0x0/0x64) from [<c0046a68>]
(warn_slowpath_null+0x18/0x1c)
 r7:c7d5fcdf r6:c7c8cc00 r5:00000000 r4:0000000b
[<c0046a50>] (warn_slowpath_null+0x0/0x1c) from [<c00493a4>]
(do_exit+0x50c/0x628)
[<c0048e98>] (do_exit+0x0/0x628) from [<c003a7bc>] (die+0x27c/0x2b4)
[<c003a540>] (die+0x0/0x2b4) from [<c003be7c>] (__do_kernel_fault+0x6c/0x8c)
[<c003be10>] (__do_kernel_fault+0x0/0x8c) from [<c003c108>]
(do_page_fault+0x198/0x1d8)
 r8:00000000 r7:00010000 r6:c7d341b4 r5:c7d34180 r4:c7d33d74
[<c003bf70>] (do_page_fault+0x0/0x1d8) from [<c0036210>]
(do_DataAbort+0x40/0xa8)
[<c00361d0>] (do_DataAbort+0x0/0xa8) from [<c0036a80>] (__dabt_svc+0x40/0x60)
Exception stack(0xc7d5fe18 to 0xc7d5fe60)
fe00:                                                       0000002f 000019c2
fe20: 60000013 00000000 c7819870 c7819878 c7d5feb4 00000003 c7d5feb4 00000000
fe40: 00000003 c7d5fe6c c7d5fd98 c7d5fe60 c004760c c003a1ac 60000013 ffffffff
 r8:c7d5feb4 r7:00000003 r6:c7d5feb4 r5:c7d5fe4c r4:ffffffff
[<c003a18c>] (__bug+0x0/0x2c) from [<c00fb2f0>] (logfs_clear_inode+0x8c/0xb0)
[<c00fb264>] (logfs_clear_inode+0x0/0xb0) from [<c00ad358>]
(clear_inode+0xac/0x114)
[<c00ad2ac>] (clear_inode+0x0/0x114) from [<c00ad3e4>] (dispose_list+0x24/0xf0)
 r4:c7819870
[<c00ad3c0>] (dispose_list+0x0/0xf0) from [<c00ad5a8>]
(invalidate_inodes+0xf8/0x12c)
 r7:c7d46a64 r6:c7819880 r5:c7d46a54 r4:c7d46a64
[<c00ad4b0>] (invalidate_inodes+0x0/0x12c) from [<c009c8ec>]
(generic_shutdown_super+0xb0/0x11c)
[<c009c83c>] (generic_shutdown_super+0x0/0x11c) from [<c00feb38>]
(logfs_kill_sb+0x48/0xfc)
 r6:c7d2b880 r5:c7c14000 r4:c7d46a00
[<c00feaf0>] (logfs_kill_sb+0x0/0xfc) from [<c009c740>]
(deactivate_super+0x48/0x60)
 r6:c7d2b880 r5:c02b7b84 r4:c7d46a00
[<c009c6f8>] (deactivate_super+0x0/0x60) from [<c00b0bd0>]
(mntput_no_expire+0x64/0xbc)
 r5:c7d46a00 r4:c7d2b880
[<c00b0b6c>] (mntput_no_expire+0x0/0xbc) from [<c00b1e90>]
(sys_umount+0x6c/0x32c)
 r5:c7d2b898 r4:00000000
[<c00b1e24>] (sys_umount+0x0/0x32c) from [<c00b2164>] (sys_oldumount+0x14/0x18)
[<c00b2150>] (sys_oldumount+0x0/0x18) from [<c0036f00>]
(ret_fast_syscall+0x0/0x2c)
---[ end trace 979e02c61e92804a ]---
Segmentation fault

Today I have to work for money....probably I have no much time to test ....
As soon as I can I'm happy to give my contribute to test.

Paolo

Re: [PATCH 2/2] [MTD] Call bdi_init() and bdi_register()

[Jörn Engel]

On Tue, 27 April 2010 13:54:09 +0200, Paolo Minazzi wrote:
> 
> Ok, now it is very better.
> My problem now is that logfs does not save files in the following case:
> 1) power-off, then power-on (without umount)
> 2) sync, then power-off, then power-on (without umount)

Ok, I'll have a look.

Can we move this and any other logfs problems to a seperate thread?  I'd
like to leave this one for the sync issues and not spam everyone on Cc:
with unrelated bugs. :)

Jörn

-- 
Don't patch bad code, rewrite it.
-- Kernigham and Pike, according to Rusty

Re: [PATCH 1/2] [MTD] Move mtd_bdi_*mappable to mtdcore.c

[Jens Axboe]

On Mon, Apr 26 2010, Jörn Engel wrote:
> On Mon, 26 April 2010 16:45:32 +0200, Jens Axboe wrote:
> > 
> > Took a quick look, and you want bdi_setup_and_register() for the three
> > bdis listed in mtdbdi.c in mtdcore.c:init_mtd(). Or manual bdi_init()
> > and bdi_register(). I'll take a look post-dinner. Either is workable,
> > but since the flags are already setup, the latter may be cleaner.
> 
> Ok, here are two patches that appear to solve the problem for me.
> 
> diff --git a/drivers/mtd/devices/drais.c b/drivers/mtd/devices/drais.c
> index b93ff3f..6f81b28 100644
> --- a/drivers/mtd/devices/drais.c
> +++ b/drivers/mtd/devices/drais.c
> @@ -33,7 +33,7 @@
>  #include <linux/sched.h>
>  #include "drais.h"
>  
> -#undef DEBUG_ERASE
> +#define DEBUG_ERASE

Leftover debugging?

-- 
Jens Axboe

Re: [PATCH 1/2] [MTD] Move mtd_bdi_*mappable to mtdcore.c

[Jörn Engel]

On Mon, 26 April 2010 19:01:28 +0200, Jens Axboe wrote:
> On Mon, Apr 26 2010, Jörn Engel wrote:
> > 
> > diff --git a/drivers/mtd/devices/drais.c b/drivers/mtd/devices/drais.c
> > index b93ff3f..6f81b28 100644
> > --- a/drivers/mtd/devices/drais.c
> > +++ b/drivers/mtd/devices/drais.c
> > @@ -33,7 +33,7 @@
> >  #include <linux/sched.h>
> >  #include "drais.h"
> >  
> > -#undef DEBUG_ERASE
> > +#define DEBUG_ERASE
> 
> Leftover debugging?

Yes.  Can you just remove that hunk or should I resend?

Jörn

-- 
"Error protection by error detection and correction."
-- from a university class

Re: [PATCH 1/2] [MTD] Move mtd_bdi_*mappable to mtdcore.c

[Jens Axboe]

On Mon, Apr 26 2010, Jörn Engel wrote:
> On Mon, 26 April 2010 19:01:28 +0200, Jens Axboe wrote:
> > On Mon, Apr 26 2010, Jörn Engel wrote:
> > > 
> > > diff --git a/drivers/mtd/devices/drais.c b/drivers/mtd/devices/drais.c
> > > index b93ff3f..6f81b28 100644
> > > --- a/drivers/mtd/devices/drais.c
> > > +++ b/drivers/mtd/devices/drais.c
> > > @@ -33,7 +33,7 @@
> > >  #include <linux/sched.h>
> > >  #include "drais.h"
> > >  
> > > -#undef DEBUG_ERASE
> > > +#define DEBUG_ERASE
> > 
> > Leftover debugging?
> 
> Yes.  Can you just remove that hunk or should I resend?

I can kill that hunk, no problem.

-- 
Jens Axboe

Re: [PATCH] [MTD] Fix JFFS2 sync silent failure

[Jens Axboe]

On Thu, Apr 22 2010, Jörn Engel wrote:
> On Mon, 19 April 2010 12:20:56 +0200, Jens Axboe wrote:
> > 
> > Thanks, we definitely should have put a debug statement to catch this in
> > from day 1, good debugging should be an important part of any new
> > infrastructure.
> 
> Woke up early and had another look at this.  Looks like a much more
> widespread problem.  Based on a quick grep an uncaffeinated brain:
> 
> 9p	no s_bdi
> afs	no s_bdi
> ceph	creates its own s_bdi
> cifs	no s_bdi
> coda	no s_bdi
> ecryptfs no s_bdi
> exofs	no s_bdi
> fuse	creates its own s_bdi?
> gfs2	creates its own s_bdi?
> jffs2	patch exists
> logfs	fixed now
> ncpfs	no s_bdi
> nfs	creates its own s_bdi
> ocfs2	no s_bdi
> smbfs	no s_bdi
> ubifs	creates its own s_bdi
> 
> I excluded all filesystems that appear to be read-only, block device
> based or lack any sort of backing store.  So there is a chance I have
> missed some as well.

It's funky, I was pretty sure there was/is code to set a default bdi for
non-bdev file systems. It appears to be missing, that's not good. So
options include:

- Add the appropriate per-sb bdi for these file systems (right fix), or
- Pre-fill default_backing_dev_info as a fallback ->s_bdi to at least
  ensure that data gets flushed (quick fix)

I'll slap together a set of fixes for this.

-- 
Jens Axboe

Re: [PATCH] [MTD] Fix JFFS2 sync silent failure

[Jens Axboe]

On Thu, Apr 22 2010, Jens Axboe wrote:
> On Thu, Apr 22 2010, Jörn Engel wrote:
> > On Mon, 19 April 2010 12:20:56 +0200, Jens Axboe wrote:
> > > 
> > > Thanks, we definitely should have put a debug statement to catch this in
> > > from day 1, good debugging should be an important part of any new
> > > infrastructure.
> > 
> > Woke up early and had another look at this.  Looks like a much more
> > widespread problem.  Based on a quick grep an uncaffeinated brain:
> > 
> > 9p	no s_bdi
> > afs	no s_bdi
> > ceph	creates its own s_bdi
> > cifs	no s_bdi
> > coda	no s_bdi
> > ecryptfs no s_bdi
> > exofs	no s_bdi
> > fuse	creates its own s_bdi?
> > gfs2	creates its own s_bdi?
> > jffs2	patch exists
> > logfs	fixed now
> > ncpfs	no s_bdi
> > nfs	creates its own s_bdi
> > ocfs2	no s_bdi
> > smbfs	no s_bdi
> > ubifs	creates its own s_bdi
> > 
> > I excluded all filesystems that appear to be read-only, block device
> > based or lack any sort of backing store.  So there is a chance I have
> > missed some as well.
> 
> It's funky, I was pretty sure there was/is code to set a default bdi for
> non-bdev file systems. It appears to be missing, that's not good. So
> options include:
> 
> - Add the appropriate per-sb bdi for these file systems (right fix), or
> - Pre-fill default_backing_dev_info as a fallback ->s_bdi to at least
>   ensure that data gets flushed (quick fix)
> 
> I'll slap together a set of fixes for this.

Here's a series for fixing these. At this point they are totally
untested except that I did compile them. Note that your analysis
appeared correct for all cases but ocfs2, which does use get_sb_bdev()
and hence gets ->s_bdi assigned.

You can see them here, I'll post the series soon:

http://git.kernel.dk/?p=linux-2.6-block.git;a=shortlog;h=refs/heads/for-linus

The first patch is a helper addition, the rest are per-fs fixups.

-- 
Jens Axboe

Re: [PATCH] [MTD] Fix JFFS2 sync silent failure

[David Woodhouse]

On Thu, 2010-04-22 at 12:39 +0200, Jens Axboe wrote:
> 
> Here's a series for fixing these. At this point they are totally
> untested except that I did compile them. Note that your analysis
> appeared correct for all cases but ocfs2, which does use get_sb_bdev()
> and hence gets ->s_bdi assigned.
> 
> You can see them here, I'll post the series soon:
> 
> http://git.kernel.dk/?p=linux-2.6-block.git;a=shortlog;h=refs/heads/for-linus
> 
> The first patch is a helper addition, the rest are per-fs fixups.

Do you want to include Jörn's addition of same to get_sb_mtd_set(), with
my Acked-By: David Woodhouse <David.Woodhouse@intel.com> ? 

-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation

Re: [PATCH] [MTD] Fix JFFS2 sync silent failure

[Jens Axboe]

On Thu, Apr 22 2010, David Woodhouse wrote:
> On Thu, 2010-04-22 at 12:39 +0200, Jens Axboe wrote:
> > 
> > Here's a series for fixing these. At this point they are totally
> > untested except that I did compile them. Note that your analysis
> > appeared correct for all cases but ocfs2, which does use get_sb_bdev()
> > and hence gets ->s_bdi assigned.
> > 
> > You can see them here, I'll post the series soon:
> > 
> > http://git.kernel.dk/?p=linux-2.6-block.git;a=shortlog;h=refs/heads/for-linus
> > 
> > The first patch is a helper addition, the rest are per-fs fixups.
> 
> Do you want to include Jörn's addition of same to get_sb_mtd_set(), with
> my Acked-By: David Woodhouse <David.Woodhouse@intel.com> ? 

Yeah will do. I also really wanted to provide a WARN and mount fail if
we get it wrong, but I don't see an easy way to do that. Basically I'd
want to check whether the storage backing is volatile or not.

-- 
Jens Axboe

Re: [PATCH] [MTD] Fix JFFS2 sync silent failure

[Jörn Engel]

On Thu, 22 April 2010 12:39:53 +0200, Jens Axboe wrote:
> 
> Here's a series for fixing these. At this point they are totally
> untested except that I did compile them. Note that your analysis
> appeared correct for all cases but ocfs2, which does use get_sb_bdev()
> and hence gets ->s_bdi assigned.
> 
> You can see them here, I'll post the series soon:
> 
> http://git.kernel.dk/?p=linux-2.6-block.git;a=shortlog;h=refs/heads/for-linus
> 
> The first patch is a helper addition, the rest are per-fs fixups.

Looks good at a cursory glance.  What's still missing is some sort of
assertion.  You are a smart person and missed this problem, twice even.
Even if you hadn't, a not so smart person can add a new filesystem and
miss s_bdi, like I did.  We want some automatism to catch this.

Jörn

-- 
"Security vulnerabilities are here to stay."
-- Scott Culp, Manager of the Microsoft Security Response Center, 2001

Re: [PATCH] [MTD] Fix JFFS2 sync silent failure

[Jens Axboe]

On Thu, Apr 22 2010, Jörn Engel wrote:
> On Thu, 22 April 2010 12:39:53 +0200, Jens Axboe wrote:
> > 
> > Here's a series for fixing these. At this point they are totally
> > untested except that I did compile them. Note that your analysis
> > appeared correct for all cases but ocfs2, which does use get_sb_bdev()
> > and hence gets ->s_bdi assigned.
> > 
> > You can see them here, I'll post the series soon:
> > 
> > http://git.kernel.dk/?p=linux-2.6-block.git;a=shortlog;h=refs/heads/for-linus
> > 
> > The first patch is a helper addition, the rest are per-fs fixups.
> 
> Looks good at a cursory glance.  What's still missing is some sort of
> assertion.  You are a smart person and missed this problem, twice even.
> Even if you hadn't, a not so smart person can add a new filesystem and
> miss s_bdi, like I did.  We want some automatism to catch this.

I totally agree, we want some way to catch this problem in the future.
Really the check needs to be something ala:

        if (!sb->s_bdi && mnt_has_storage_backing() && rw_mount)
                yell_and_fail;

but I'm not sure how best to accomplish that. We can check for ->s_bdev
and mtd like you did, but that does not catch network file systems for
instance.

-- 
Jens Axboe

Re: [PATCH] [MTD] Fix JFFS2 sync silent failure

[Jörn Engel]

On Thu, 22 April 2010 14:08:37 +0200, Jens Axboe wrote:
> 
> I totally agree, we want some way to catch this problem in the future.
> Really the check needs to be something ala:
> 
>         if (!sb->s_bdi && mnt_has_storage_backing() && rw_mount)
>                 yell_and_fail;
> 
> but I'm not sure how best to accomplish that. We can check for ->s_bdev
> and mtd like you did, but that does not catch network file systems for
> instance.

One way would be to either add a flag to all safe filesystems or create
a noop_bdi for them.  It adds a line of code and some bytes[*] per
instance to most filesystems, but that's the only catchall I can think
of.

I guess if noone comes up with a better plan I'll look into that.

[*] Maybe we can steal a bit somewhere to make it less expensive.

Jörn

-- 
You can't tell where a program is going to spend its time. Bottlenecks
occur in surprising places, so don't try to second guess and put in a
speed hack until you've proven that's where the bottleneck is.
-- Rob Pike