All of lore.kernel.org
 help / color / mirror / Atom feed
From: Chris Mason <chris.mason@fusionio.com>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Chris Mason <clmason@fusionio.com>,
	Mikulas Patocka <mpatocka@redhat.com>,
	Al Viro <viro@zeniv.linux.org.uk>, Jens Axboe <axboe@kernel.dk>,
	Jeff Chua <jeff.chua.linux@gmail.com>,
	Lai Jiangshan <laijs@cn.fujitsu.com>, Jan Kara <jack@suse.cz>,
	lkml <linux-kernel@vger.kernel.org>,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>
Subject: Re: [PATCH v2] Do a proper locking for mmap and block size change
Date: Thu, 29 Nov 2012 20:16:08 -0500	[thread overview]
Message-ID: <20121130011608.GA11004@shiny.int.fusionio.com> (raw)
In-Reply-To: <CA+55aFwoM3wje+XS60=kE3eVndWQD6szfgYa9MyfSp=itdr9Vg@mail.gmail.com>

On Thu, Nov 29, 2012 at 03:36:38PM -0700, Linus Torvalds wrote:
> On Thu, Nov 29, 2012 at 2:16 PM, Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
> >
> > But you're right. The direct-IO code really *is* violating that, and
> > knows that get_block() ends up being defined in i_blkbits regardless
> > of b_size.
> 
> It turns out fs/ioctl.c does the same - it fills in the buffer head
> with some random bh->b_size too. I think it's not even a power of two
> in that case.
> 
> And I guess it's understandable - they don't actually *use* the
> buffer, they just want the offset. So the b_size field really is just
> random crap to the users of the get_block interfaces, since they've
> never cared before.
> 
> Ugh, this was definitely a dark and disgusting underbelly of the VFS
> layer. We've not had to really touch it for a *looong* time..

I searched through filemap.c for the magic i_size check that would let
us get away with ignoring i_blkbits in get_blocks, but its just not
there.  The whole fallback-to-buffered scheme seems to rely on
get_blocks checking for i_size.  I really hope I'm just missing
something.

If we're going to change this, I'd vote for something non-bh based.  I
didn't check every single FS, but I don't think direct-IO really wants
or needs buffer heads at all.

One less wart in direct-io.c would really be nice, but I'm assuming
it'll take us at least one full release to hammer out a shiny new
get_blocks.  Passing i_blkbits would be more mechanical, since all the
filesystems would just ignore it.

-chris

  reply	other threads:[~2012-11-30  1:16 UTC|newest]

Thread overview: 82+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-11-19  0:33 Recent kernel "mount" slow Jeff Chua
2012-11-20 18:09 ` Jan Kara
2012-11-21 15:46   ` Jeff Chua
2012-11-22 14:30     ` Jeff Chua
2012-11-22 19:21       ` Linus Torvalds
2012-11-23 13:24         ` Jens Axboe
2012-11-23 22:21           ` Jeff Chua
2012-11-23 23:31             ` Jeff Chua
2012-11-23 23:48               ` Jeff Chua
2012-11-24 21:09             ` Mikulas Patocka
2012-11-24 23:23               ` Jeff Chua
2012-11-27  5:57                 ` Jeff Chua
2012-11-27  7:38                   ` Jens Axboe
2012-11-27  7:44                     ` Jens Axboe
2012-11-27  8:45                       ` Jeff Chua
2012-11-27 10:06                     ` Jeff Chua
2012-11-27 12:33                       ` Jens Axboe
2012-11-28  3:57                         ` Mikulas Patocka
2012-11-28  8:33                           ` Jens Axboe
2012-11-28 13:05                             ` Jeff Chua
2012-11-28 17:25                             ` [PATCH] Introduce a method to catch mmap_region (was: Recent kernel "mount" slow) Mikulas Patocka
2012-11-28 19:15                               ` Linus Torvalds
2012-11-28 19:43                                 ` Al Viro
2012-11-28 19:53                                   ` Linus Torvalds
2012-11-28 22:01                                   ` [PATCH v2] Do a proper locking for mmap and block size change Mikulas Patocka
2012-11-29 17:19                                     ` Linus Torvalds
2012-11-29 18:23                                       ` Mikulas Patocka
2012-11-29 18:46                                         ` Linus Torvalds
2012-11-29 19:02                                       ` Linus Torvalds
2012-11-29 19:15                                         ` Chris Mason
2012-11-29 19:26                                           ` Linus Torvalds
2012-11-29 19:48                                             ` Chris Mason
2012-11-29 19:55                                               ` Linus Torvalds
2012-11-29 20:10                                                 ` Linus Torvalds
2012-11-29 20:52                                               ` Linus Torvalds
2012-11-29 21:29                                                 ` Chris Mason
2012-11-29 22:16                                                   ` Linus Torvalds
2012-11-29 22:36                                                     ` Linus Torvalds
2012-11-30  1:16                                                       ` Chris Mason [this message]
2012-11-30  2:13                                                         ` Linus Torvalds
2012-11-30  2:27                                                           ` Chris Mason
2012-11-30  2:49                                                     ` Dave Chinner
2012-11-30 14:31                                                       ` Chris Mason
2012-11-30 16:42                                                         ` Linus Torvalds
2012-11-30 16:36                                                       ` Christoph Hellwig
2012-11-30 22:40                                                         ` Dave Chinner
2012-11-30 23:09                                                           ` Christoph Hellwig
2012-11-29 19:50                                             ` Linus Torvalds
2012-11-28 19:50                                 ` [PATCH] Introduce a method to catch mmap_region (was: Recent kernel "mount" slow) Mikulas Patocka
2012-11-28 20:03                                   ` Linus Torvalds
2012-11-28 20:13                                     ` Linus Torvalds
2012-11-28 20:32                                       ` Linus Torvalds
2012-11-28 20:47                                         ` Linus Torvalds
2012-11-28 22:10                                           ` Mikulas Patocka
2012-11-28 21:29                                       ` Mikulas Patocka
2012-11-28 22:52                                         ` Linus Torvalds
2012-11-28 23:13                                           ` Linus Torvalds
2012-11-29  1:20                                             ` Mikulas Patocka
2012-11-29  0:38                                           ` Mikulas Patocka
2012-11-29  2:04                                             ` Linus Torvalds
2012-11-29  2:58                                               ` Linus Torvalds
2012-11-29  6:16                                                 ` Linus Torvalds
2012-11-29  6:25                                                   ` Al Viro
2012-11-29  6:30                                                     ` Al Viro
2012-11-29  6:37                                                       ` Linus Torvalds
2012-11-29  6:45                                                         ` Al Viro
2012-11-29 10:57                                                           ` Jeff Chua
2012-11-29  6:33                                                     ` Linus Torvalds
2012-11-29 14:12                                                   ` Chris Mason
2012-11-29 17:26                                                     ` Chris Mason
2012-11-29 17:26                                                     ` Linus Torvalds
2012-11-29 17:51                                                       ` Chris Mason
2012-11-29 18:12                                                         ` Linus Torvalds
2012-11-28  3:59                       ` [PATCH 1/2] percpu-rwsem: use synchronize_sched_expedited Mikulas Patocka
2012-11-28  4:01                         ` [PATCH 2/2] block_dev: don't take the write lock if block size doesn't change Mikulas Patocka
2012-11-28 14:24                           ` Jeff Chua
2012-11-28 22:03                             ` Mikulas Patocka
2012-11-28 14:19                         ` [PATCH 1/2] percpu-rwsem: use synchronize_sched_expedited Jeff Chua
2012-11-30  0:06                         ` Andrew Morton
2012-11-30  3:00                           ` Mikulas Patocka
2012-11-30 13:42                             ` Paul E. McKenney
2012-11-30 18:57                           ` Oleg Nesterov

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=20121130011608.GA11004@shiny.int.fusionio.com \
    --to=chris.mason@fusionio.com \
    --cc=axboe@kernel.dk \
    --cc=clmason@fusionio.com \
    --cc=jack@suse.cz \
    --cc=jeff.chua.linux@gmail.com \
    --cc=laijs@cn.fujitsu.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mpatocka@redhat.com \
    --cc=torvalds@linux-foundation.org \
    --cc=viro@zeniv.linux.org.uk \
    /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.