Re: [PATCH] Introduce a method to catch mmap_region (was: Recent kernel "mount" slow)

Mikulas Patocka
2012-11-28T19:51:50Z

On Wed, 28 Nov 2012, Linus Torvalds wrote:

> No, this is crap.
>
> We don't introduce random hooks like this just because the block layer
> has shit-for-brains and cannot be bothered to do things right.
>
> The fact is, the whole locking in the block layer open routine is
> total and utter crap. It doesn't lock the right thing, even with your
> change *anyway* (or with the change Jens had). Absolutely nothing in
> "mmap_region()" cares at all about the block-size anywhere - it's
> generic, after all - so locking around it is f*cking pointless. There
> is no way in hell that the caller of ->mmap can *ever* care about the
> block size, since it never even looks at it.
>
> Don't do random crap like this.
>
> Why does the code think that mmap matters so much anyway? As you say,
> the mmap itself does *nothing*. It has no impact for the block size.
>
> Linus

mmap_region() doesn't care about the block size. But a lot of
page-in/page-out code does.

The problem is that once the block device is mapped, page faults or page
writeback can happen anytime - so the simplest solution is to not allow
the block device being mapped while we change block size.

The function set_blocksize takes bd_block_size_semaphore for write (that
blocks read/write/mmap), then it calls sync_blockdev (now we are sure that
there is no more writeback), then it changes the block size, then it calls
kill_bdev (now we are sure that there are no more any pages with buffers
with the old blocksize).

If you want to allow to change block size while a block device is mapped,
you'd have to add explicit locks around all mm callbacks (so that the
block size can't change while the callback is in progress) - and even
then, there are some unsolvable cases - i.e. what are you going to do if
the user mlocks a mapped block device and you change block size of that
device? - you can't drop the pages (that would violate mlock semantics)
and you can leave them there (because they have buffers with wrong size).

If you don't like what I sent, propose a different solution.

Mikulas