All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jens Axboe <jens.axboe@oracle.com>
To: "Jörn Engel" <joern@logfs.org>
Cc: Christoph Hellwig <hch@infradead.org>,
	linux-mtd@lists.infradead.org,
	David Woodhouse <dwmw2@infradead.org>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] [MTD] Fix JFFS2 sync silent failure
Date: Thu, 22 Apr 2010 14:08:37 +0200	[thread overview]
Message-ID: <20100422120837.GE27497@kernel.dk> (raw)
In-Reply-To: <20100422115539.GE27309@logfs.org>

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

WARNING: multiple messages have this Message-ID (diff)
From: Jens Axboe <jens.axboe@oracle.com>
To: "Jörn Engel" <joern@logfs.org>
Cc: David Woodhouse <dwmw2@infradead.org>,
	linux-mtd@lists.infradead.org, linux-kernel@vger.kernel.org,
	Christoph Hellwig <hch@infradead.org>
Subject: Re: [PATCH] [MTD] Fix JFFS2 sync silent failure
Date: Thu, 22 Apr 2010 14:08:37 +0200	[thread overview]
Message-ID: <20100422120837.GE27497@kernel.dk> (raw)
In-Reply-To: <20100422115539.GE27309@logfs.org>

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


  reply	other threads:[~2010-04-22 12:08 UTC|newest]

Thread overview: 96+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-04-17 18:40 [PATCH] [MTD] Fix JFFS2 sync silent failure Jörn Engel
2010-04-17 18:40 ` 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 [this message]
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=20100422120837.GE27497@kernel.dk \
    --to=jens.axboe@oracle.com \
    --cc=dwmw2@infradead.org \
    --cc=hch@infradead.org \
    --cc=joern@logfs.org \
    --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.