linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Dave Chinner <david@fromorbit.com>
To: Michel Lespinasse <walken@google.com>
Cc: Peter Zijlstra <peterz@infradead.org>,
	Nick Piggin <npiggin@kernel.dk>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	Andrew Morton <akpm@linux-foundation.org>,
	Hugh Dickins <hughd@google.com>, Rik van Riel <riel@redhat.com>,
	Kosaki Motohiro <kosaki.motohiro@jp.fujitsu.com>,
	Theodore Tso <tytso@google.com>,
	Michael Rubin <mrubin@google.com>,
	Suleiman Souhlal <suleiman@google.com>
Subject: Re: [PATCH 3/3] mlock: avoid dirtying pages and triggering writeback
Date: Thu, 18 Nov 2010 10:11:43 +1100	[thread overview]
Message-ID: <20101117231143.GQ22876@dastard> (raw)
In-Reply-To: <AANLkTim4tO_aKzXLXJm-N-iEQ9rNSa0=HGJVDAz33kY6@mail.gmail.com>

On Wed, Nov 17, 2010 at 02:05:30PM -0800, Michel Lespinasse wrote:
> On Wed, Nov 17, 2010 at 7:28 AM, Peter Zijlstra <peterz@infradead.org> wrote:
> > On Wed, 2010-11-17 at 23:57 +1100, Nick Piggin wrote:
> >> On Wed, Nov 17, 2010 at 04:23:58AM -0800, Michel Lespinasse wrote:
> >> > When faulting in pages for mlock(), we want to break COW for anonymous
> >> > or file pages within VM_WRITABLE, non-VM_SHARED vmas. However, there is
> >> > no need to write-fault into VM_SHARED vmas since shared file pages can
> >> > be mlocked first and dirtied later, when/if they actually get written to.
> >> > Skipping the write fault is desirable, as we don't want to unnecessarily
> >> > cause these pages to be dirtied and queued for writeback.
> >>
> >> It's not just to break COW, but to do block allocation and such
> >> (filesystem's page_mkwrite op). That needs to at least be explained
> >> in the changelog.
> >
> > Agreed, the 0/3 description actually does mention this.
> >
> >> Filesystem doesn't have a good way to fully pin required things
> >> according to mlock, but page_mkwrite provides some reasonable things
> >> (like block allocation / reservation).
> >
> > Right, but marking all pages dirty isn't really sane. I can imagine
> > making the reservation but not marking things dirty solution, although
> > it might be lots harder to implement, esp since some filesystems don't
> > actually have a page_mkwrite() implementation.
> 
> Really, my understanding is that not pre-allocating filesystem blocks
> is just fine. This is, after all, what happens with ext3 and it's
> never been reported as a bug (that I know of).

It's not ext3 you have to worry about - it's the filesystems that
need special state set up on their pages/buffers for ->writepage to
work correctly that are the problem. You need to call
->write_begin/->write_end to get the state set up properly.

If this state is not set up properly, silent data loss will occur
during mmap writes either by ENOSPC or failing to set up writes into
unwritten extents correctly (i.e. we'll be back to where we were in
2.6.15).

I don't think ->page_mkwrite can be worked around - we need that to
be called on the first write fault of any mmap()d page to ensure it
is set up correctly for writeback.  If we don't get write faults
after the page is mlock()d, then we need the ->page_mkwrite() call
during the mlock() call.

> If filesystem people's feedback is that they really want mlock() to
> continue pre-allocating blocks, maybe we can just do it using
> fallocate() rather than page_mkwrite() callbacks ?

Fallocate is not good enough to avoid ->page_mkwrite callbacks.
Indeed, XFS (at least) requires the ->page_mkwrite() callout even
on preallocated space to correctly mark the buffers as unwritten so
extent conversion in ->writepage is triggered correctly (see test
#166 in xfstests).

Hence I think that avoiding ->page_mkwrite callouts is likely to
break some filesystems in subtle, undetected ways.  IMO, regardless
of what is done, it would be really good to start by writing a new
regression test to exercise and encode the expected the mlock
behaviour so we can detect regressions later on....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom policy in Canada: sign http://dissolvethecrtc.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  parent reply	other threads:[~2010-11-17 23:12 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-11-17 12:23 [PATCH 0/3] Avoid dirtying pages during mlock Michel Lespinasse
2010-11-17 12:23 ` [PATCH 1/3] do_wp_page: remove the 'reuse' flag Michel Lespinasse
2010-11-17 12:23 ` [PATCH 2/3] do_wp_page: clarify dirty_page handling Michel Lespinasse
2010-11-17 12:23 ` [PATCH 3/3] mlock: avoid dirtying pages and triggering writeback Michel Lespinasse
2010-11-17 12:57   ` Nick Piggin
2010-11-17 15:28     ` Peter Zijlstra
2010-11-17 22:05       ` Michel Lespinasse
2010-11-17 22:18         ` Peter Zijlstra
2010-11-17 23:11         ` Dave Chinner [this message]
2010-11-17 23:31           ` Michel Lespinasse
2010-11-19  1:46             ` Dave Chinner
2010-11-17 23:52           ` Ted Ts'o
2010-11-18  0:53             ` Andrew Morton
2010-11-18 11:03               ` Michel Lespinasse
2010-11-18 13:37           ` Christoph Hellwig
2010-11-18 17:41             ` Hugh Dickins
2010-11-19  7:23               ` Michel Lespinasse
2010-11-19 13:38                 ` Theodore Tso
2010-11-19 13:42                 ` Theodore Tso
2010-11-19 15:06                   ` Christoph Hellwig
2010-11-19 22:54                 ` Andrew Morton
2010-11-19 23:22                   ` Ted Ts'o
2010-11-20  0:29                     ` Dustin Kirkland
2010-11-19 23:31                   ` Michel Lespinasse
2010-11-19 23:54                 ` Dave Chinner
2010-11-18  5:46       ` Nick Piggin
2010-11-18 10:43         ` Theodore Tso
2010-11-18 13:39           ` Christoph Hellwig
2010-11-18 18:00             ` Hugh Dickins

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=20101117231143.GQ22876@dastard \
    --to=david@fromorbit.com \
    --cc=akpm@linux-foundation.org \
    --cc=hughd@google.com \
    --cc=kosaki.motohiro@jp.fujitsu.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mrubin@google.com \
    --cc=npiggin@kernel.dk \
    --cc=peterz@infradead.org \
    --cc=riel@redhat.com \
    --cc=suleiman@google.com \
    --cc=tytso@google.com \
    --cc=walken@google.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox