linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: Hugh Dickins <hugh@veritas.com>
Cc: Nick Piggin <nickpiggin@yahoo.com.au>, linux-mm@kvack.org
Subject: Re: [rfc][patch] mm: dirty page accounting hole
Date: Tue, 12 Aug 2008 13:30:56 +0200	[thread overview]
Message-ID: <1218540656.10800.188.camel@twins> (raw)
In-Reply-To: <Pine.LNX.4.64.0808121210250.31744@blonde.site>

On Tue, 2008-08-12 at 12:15 +0100, Hugh Dickins wrote:
> On Tue, 12 Aug 2008, Nick Piggin wrote:
> > 
> > I think I'm running into a hole in dirty page accounting...
> > 
> > What seems to be happening is that a page gets written to via a
> > VM_SHARED vma. We then set the pte dirty, then mark the page dirty.
> > Next, mprotect changes the vma so it is no longer writeable so it
> > is no longer VM_SHARED. The pte is still dirty.
> 
> I don't think you've got that right yet.
> 
> mprotect can of course change vma->vm_flags to take VM_WRITE off,
> making vma no longer writeable; but it shouldn't be touching
> VM_SHARED.  And a quick check with debugger confirms that.
> 
> It's precisely because of mprotect that page_mkclean_one tests
> VM_SHARED not VM_WRITE.  Changing that to VM_MAYSHARE, as in your
> patch below, should make no difference to correctness; but would
> potentially make its loop less efficient (it would also go off to
> check MAP_SHARED, PROT_READ, fd readonly mappings unnecessarily).
> 
> Perhaps there's somewhere else that clears VM_SHARED by mistake?
> Or another path through mprotect which does so?  I haven't checked
> further, hoping this will jolt you into a different realization.

You are right, I cannot find a path through mprotect that unsets
VM_SHARED either.

Something which I failed to validate this morning.

--
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>

  reply	other threads:[~2008-08-12 11:30 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-08-12  5:58 Nick Piggin
2008-08-12  6:50 ` Peter Zijlstra
2008-08-12  7:06   ` Nick Piggin
2008-08-12 11:15 ` Hugh Dickins
2008-08-12 11:30   ` Peter Zijlstra [this message]
2008-08-12 11:53   ` Nick Piggin
2008-08-12 13:17     ` 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=1218540656.10800.188.camel@twins \
    --to=peterz@infradead.org \
    --cc=hugh@veritas.com \
    --cc=linux-mm@kvack.org \
    --cc=nickpiggin@yahoo.com.au \
    /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