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
prev parent 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.