linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Andrew Morton <akpm@osdl.org>
To: Trond Myklebust <Trond.Myklebust@netapp.com>
Cc: Nick Piggin <nickpiggin@yahoo.com.au>,
	Chuck Lever <chucklever@gmail.com>,
	linux-mm@kvack.org, steved@redhat.com
Subject: Re: [PATCH] Make invalidate_inode_pages2() work again
Date: Mon, 25 Sep 2006 18:42:13 -0700	[thread overview]
Message-ID: <20060925184213.11c7387c.akpm@osdl.org> (raw)
In-Reply-To: <1159233613.5442.61.camel@lade.trondhjem.org>

On Mon, 25 Sep 2006 21:20:13 -0400
Trond Myklebust <Trond.Myklebust@netapp.com> wrote:

> On Tue, 2006-09-26 at 09:59 +1000, Nick Piggin wrote:
> > Chuck Lever wrote:
> > 
> > >A recent change to fix a problem with invalidate_inode_pages() has weakened
> > >the behavior of invalidate_inode_pages2() inadvertently.  Add a flag to
> > >tell the helper routines when stronger invalidation semantics are desired.
> > >
> > 
> > Question: if invalidate_inode_pages2 cares about not invalidating dirty
> > pages, how can one avoid the page_count check and it still be correct
> > (ie. not randomly lose dirty bits in some situations)?
> 
> Tests of page_count _suck_ 'cos they are 100% non-specific. Is there no
> way to set a page flag or something to indicate that the page may have
> been remapped while we were sleeping?

Its a question of "what are these functions supposed to do"?

I'd suggest:

invalidate_inode_pages() -> best-effort, remove-it-if-it-isn't-busy

truncate_inode_pages() -> guaranteed, data-destroying takedown.

invalidate_inode_pages2() -> Somewhere in between.  Any takers?

I'd suggest "guaranteed, non-data-destroying takedown".  Maybe.  So it
doesn't remove dirty pages, but it does remove otherwise-busy pages.

As definitions go, that really sucks.

I think testing page_count() makes sense for invalidate_inode_pages(),
because that page is clearly in use by someone for something and we
shouldn't go and whip it out of pagecache under that someone's feet.  It
is, after all, "pinned".

For invalidate_inode_pages2(), proper behaviour would be to block until
whoever is busying that page stops being busy on it.

I perhaps we could do a wake_up(page_waitqueue(page)) in vmscan when it
drops the ref on a page.  But that would mean that
invalidate_inode_pages2() would get permanently stuck on a
permanently-pinned page.

It's a bit of a pickle.  Perhaps we just add the
invalidate_complete_page2().  That re-adds the direct-io race, which is
fixable by locking the page in the pagefault handler.  argh.

--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  parent reply	other threads:[~2006-09-26  1:42 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-09-25 23:15 Chuck Lever
2006-09-25 23:59 ` Nick Piggin
2006-09-26  1:20   ` Trond Myklebust
2006-09-26  1:39     ` Nick Piggin
2006-09-26  6:24       ` Nick Piggin
2006-09-26  1:42     ` Andrew Morton [this message]
2006-09-26  1:54       ` Nick Piggin

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=20060925184213.11c7387c.akpm@osdl.org \
    --to=akpm@osdl.org \
    --cc=Trond.Myklebust@netapp.com \
    --cc=chucklever@gmail.com \
    --cc=linux-mm@kvack.org \
    --cc=nickpiggin@yahoo.com.au \
    --cc=steved@redhat.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