All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jeremy Fitzhardinge <jeremy@goop.org>
To: Jan Beulich <jbeulich@novell.com>
Cc: xen-devel@lists.xensource.com
Subject: Re: [RFC] linux: add	p[mug]d_clear_full()	accessors
Date: Wed, 21 May 2008 10:02:25 +0100	[thread overview]
Message-ID: <4833E521.2050607@goop.org> (raw)
In-Reply-To: <4833E1A1.76E4.0078.0@novell.com>

Jan Beulich wrote:
>>> One question though - in our 2.6.23 merge (where pv-ops-Xen's
>>> PG_pinned appeared as an alias of PG_owner_priv_1, and where
>>> PG_arch_1 got assigned a meaning for native x86, so PG_pinned
>>> for the traditional patches needed to change anyway) I intentionally
>>> didn't follow pv-ops for our patches, since PG_pinned is among the
>>> flags bad_page() checks for (in the XenSource tree, and I think this
>>> should really also be done in upstream Linux), and hence re-using
>>> the bit here would change behavior for other parts of the kernel.
>>>       
>> I don't think so.  So long as its clear by the time you free the page, 
>> it doesn't matter how it gets used in the meantime (after all, you 
>> should never free a pinned pagetable page).
>>     
>
> But isn't that one of the purposes of those page flags checks - checking
> whether e.g. a page may validly get freed (free_pages_check())? All
> of bad_page(), free_pages_check(), and prep_new_page() currently
> add PG_pinned into the set of flags cleared/checked, and I continue to
> think that that is the right thing to do.

Well, yes, it would be helpful for free_pages_check to test whether 
PG_owner_priv_1 is set on a page you're attempting to free, since that 
would definitely be a bug.  But its also a bug which would turn up 
fairly quickly anyway, since Xen would complain about any subsequent 
users of that page.  Regardless, its not actually a bug that has turned 
up, since the lifespan of pagetable pages is pretty well controlled, and 
there's only a couple of places where they get allocated and freed.

So, not a big issue either way, I think.

    J

      reply	other threads:[~2008-05-21  9:02 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-05-20 14:42 [RFC] linux: add p[mug]d_clear_full() accessors Jan Beulich
2008-05-20 14:46 ` Keir Fraser
2008-05-20 15:17 ` Jeremy Fitzhardinge
2008-05-20 15:54   ` Jan Beulich
2008-05-20 20:03     ` Jeremy Fitzhardinge
2008-05-21  6:47       ` Jan Beulich
2008-05-21  9:02         ` Jeremy Fitzhardinge [this message]

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=4833E521.2050607@goop.org \
    --to=jeremy@goop.org \
    --cc=jbeulich@novell.com \
    --cc=xen-devel@lists.xensource.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 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.