All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrew Morton <akpm@osdl.org>
To: Peter Zijlstra <a.p.zijlstra@chello.nl>,
	Trond Myklebust <trond.myklebust@fys.uio.no>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	Nick Piggin <nickpiggin@yahoo.com.au>
Subject: Re: [PATCH] nfs: fix NR_FILE_DIRTY underflow
Date: Wed, 13 Dec 2006 17:41:48 -0800	[thread overview]
Message-ID: <20061213174148.6197c91a.akpm@osdl.org> (raw)
In-Reply-To: <20061213172921.0c4a2809.akpm@osdl.org>

On Wed, 13 Dec 2006 17:29:21 -0800
Andrew Morton <akpm@osdl.org> wrote:

> a) we're now calling try_to_release_page() with a potentially-dirty
>    page, whereas it was previously clean.
> 
>    I wouldn't expect ->releasepage() implementations to go looking at
>    PG_Dirty, because that's not what they're suppoed to be interested in. 
>    But they might do, dunno.

Still an issue, probably minor.

> b) If invalidate_complete_page2() failed due to, say, dirty buffer_heads
>    then we now have a clean page with dirty buffers.  That is an illegal
>    state and the page will leak permanently.
>
>    I _think_ that's what the was_dirty logic is in there for: to
>    preserve the correct page-vs-buffers dirtiness coherency.  But I'd need
>    to do some 2.5.x changelog-dumpster-diving to be sure.

no, that's bs.  The patch looks OK from that POV: try_to_release_page()
will be able to clear clean buffers from a dirty page.

And in fact if it did that, it will then clean the page for us (see
test_clear_page_dirty() in try_to_free_buffers()).

But we still need the clear_page_dirty() in invalidate_complete_page2() in
case we didn't call try_to_release_page() at all.

> Trond, please define precisely and completely and without reference to
> the existing implementation: what behaviour does NFS want?

But this would be nice.

  reply	other threads:[~2006-12-14  1:41 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-12-13 12:12 [PATCH] nfs: fix NR_FILE_DIRTY underflow Peter Zijlstra
2006-12-13 12:26 ` Trond Myklebust
2006-12-13 15:01   ` Peter Zijlstra
2006-12-13 17:40     ` Trond Myklebust
2006-12-13 18:48       ` Peter Zijlstra
2006-12-14  1:29         ` Andrew Morton
2006-12-14  1:41           ` Andrew Morton [this message]
2006-12-14 14:52             ` Trond Myklebust
2006-12-13 16:22   ` Peter Zijlstra
2006-12-13 16:22     ` Peter Zijlstra

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=20061213174148.6197c91a.akpm@osdl.org \
    --to=akpm@osdl.org \
    --cc=a.p.zijlstra@chello.nl \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nickpiggin@yahoo.com.au \
    --cc=trond.myklebust@fys.uio.no \
    /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.