All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Jörn Engel" <joern@logfs.org>
To: David Woodhouse <dwmw2@infradead.org>
Cc: Christoph Hellwig <hch@infradead.org>,
	linux-mtd@lists.infradead.org, linux-kernel@vger.kernel.org,
	Jens Axboe <jens.axboe@oracle.com>
Subject: [PATCH] [MTD] Fix JFFS2 sync silent failure
Date: Sat, 17 Apr 2010 20:40:16 +0200	[thread overview]
Message-ID: <20100417184016.GA17345@logfs.org> (raw)

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

WARNING: multiple messages have this Message-ID (diff)
From: "Jörn Engel" <joern@logfs.org>
To: David Woodhouse <dwmw2@infradead.org>
Cc: linux-mtd@lists.infradead.org, linux-kernel@vger.kernel.org,
	Jens Axboe <jens.axboe@oracle.com>,
	Christoph Hellwig <hch@infradead.org>
Subject: [PATCH] [MTD] Fix JFFS2 sync silent failure
Date: Sat, 17 Apr 2010 20:40:16 +0200	[thread overview]
Message-ID: <20100417184016.GA17345@logfs.org> (raw)

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

             reply	other threads:[~2010-04-17 18:40 UTC|newest]

Thread overview: 96+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-04-17 18:40 Jörn Engel [this message]
2010-04-17 18:40 ` [PATCH] [MTD] Fix JFFS2 sync silent failure Jörn Engel
2010-04-19  7:38 ` Jens Axboe
2010-04-19  7:38   ` Jens Axboe
2010-04-19 10:15   ` Jörn Engel
2010-04-19 10:15     ` Jörn Engel
2010-04-19 10:20     ` Jens Axboe
2010-04-19 10:20       ` Jens Axboe
2010-04-19 11:39       ` Jörn Engel
2010-04-19 11:39         ` Jörn Engel
2010-04-22  5:54       ` Jörn Engel
2010-04-22  5:54         ` Jörn Engel
2010-04-22  6:26         ` Jörn Engel
2010-04-22  6:26           ` Jörn Engel
2010-04-22 14:25           ` Linus Torvalds
2010-04-22 14:25             ` Linus Torvalds
2010-04-22 14:33             ` Linus Torvalds
2010-04-22 14:33               ` Linus Torvalds
2010-04-22 16:27               ` Jens Axboe
2010-04-22 16:27                 ` Jens Axboe
2010-04-22 20:33                 ` [Patch] Catch filesystems lacking s_bdi Jörn Engel
2010-04-22 20:33                   ` Jörn Engel
2010-04-23 10:05                   ` Jens Axboe
2010-04-23 10:05                     ` Jens Axboe
2010-04-23 20:55                     ` Jörn Engel
2010-04-23 20:55                       ` Jörn Engel
2010-04-26  9:48                       ` Jens Axboe
2010-04-26  9:48                         ` Jens Axboe
2010-04-26 14:32                         ` Jörn Engel
2010-04-26 14:32                           ` Jörn Engel
2010-04-26 14:38                           ` Jens Axboe
2010-04-26 14:38                             ` Jens Axboe
2010-04-26 14:45                             ` Jens Axboe
2010-04-26 14:45                               ` Jens Axboe
2010-04-26 16:30                               ` [PATCH 1/2] [MTD] Move mtd_bdi_*mappable to mtdcore.c Jörn Engel
2010-04-26 16:30                                 ` Jörn Engel
2010-04-26 16:31                                 ` [PATCH 2/2] [MTD] Call bdi_init() and bdi_register() Jörn Engel
2010-04-26 16:31                                   ` Jörn Engel
2010-04-26 17:02                                   ` Jens Axboe
2010-04-26 17:02                                     ` Jens Axboe
2010-04-26 17:12                                     ` Jörn Engel
2010-04-26 17:12                                       ` Jörn Engel
2010-04-27  7:52                                       ` Jens Axboe
2010-04-27  7:52                                         ` Jens Axboe
2010-04-27  8:11                                         ` Paolo Minazzi
2010-04-27  8:11                                           ` Paolo Minazzi
2010-04-27  8:16                                           ` Jens Axboe
2010-04-27  8:16                                             ` Jens Axboe
2010-04-27 11:29                                         ` Jörn Engel
2010-04-27 11:29                                           ` Jörn Engel
2010-04-27 11:33                                           ` Jens Axboe
2010-04-27 11:33                                             ` Jens Axboe
2010-04-27  9:01                                   ` Paolo Minazzi
2010-04-27  9:01                                     ` Paolo Minazzi
2010-04-27  9:16                                     ` Jens Axboe
2010-04-27  9:16                                       ` Jens Axboe
2010-04-27  9:26                                       ` Paolo Minazzi
2010-04-27  9:26                                         ` Paolo Minazzi
2010-04-27  9:29                                         ` Jens Axboe
2010-04-27  9:29                                           ` Jens Axboe
2010-04-27  9:36                                           ` Paolo Minazzi
2010-04-27  9:36                                             ` Paolo Minazzi
2010-04-27 11:17                                     ` Jörn Engel
2010-04-27 11:17                                       ` Jörn Engel
2010-04-27 11:31                                       ` Paolo Minazzi
2010-04-27 11:31                                         ` Paolo Minazzi
2010-04-27 11:40                                         ` Jörn Engel
2010-04-27 11:40                                           ` Jörn Engel
2010-04-27 11:48                                           ` [PATCH] [LogFS] Return -EINVAL if filesystem image doesn't match Jörn Engel
2010-04-27 11:48                                             ` Jörn Engel
2010-04-27 12:53                                             ` Paolo Minazzi
2010-04-27 12:53                                               ` Paolo Minazzi
2010-04-27 11:54                                           ` [PATCH 2/2] [MTD] Call bdi_init() and bdi_register() Paolo Minazzi
2010-04-27 11:54                                             ` Paolo Minazzi
2010-04-27 12:05                                             ` Jörn Engel
2010-04-27 12:05                                               ` Jörn Engel
2010-04-26 17:01                                 ` [PATCH 1/2] [MTD] Move mtd_bdi_*mappable to mtdcore.c Jens Axboe
2010-04-26 17:01                                   ` Jens Axboe
2010-04-26 17:08                                   ` Jörn Engel
2010-04-26 17:08                                     ` Jörn Engel
2010-04-26 17:10                                     ` Jens Axboe
2010-04-26 17:10                                       ` Jens Axboe
2010-04-22  9:03         ` [PATCH] [MTD] Fix JFFS2 sync silent failure Jens Axboe
2010-04-22  9:03           ` Jens Axboe
2010-04-22 10:39           ` Jens Axboe
2010-04-22 10:39             ` Jens Axboe
2010-04-22 10:58             ` David Woodhouse
2010-04-22 10:58               ` David Woodhouse
2010-04-22 11:05               ` Jens Axboe
2010-04-22 11:05                 ` Jens Axboe
2010-04-22 11:55             ` Jörn Engel
2010-04-22 11:55               ` Jörn Engel
2010-04-22 12:08               ` Jens Axboe
2010-04-22 12:08                 ` Jens Axboe
2010-04-22 12:17                 ` Jörn Engel
2010-04-22 12:17                   ` Jörn Engel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20100417184016.GA17345@logfs.org \
    --to=joern@logfs.org \
    --cc=dwmw2@infradead.org \
    --cc=hch@infradead.org \
    --cc=jens.axboe@oracle.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mtd@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.