All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Jan Beulich" <jbeulich@novell.com>
To: "Jeremy Fitzhardinge" <jeremy@goop.org>
Cc: "Ingo Molnar" <mingo@elte.hu>, <linux-kernel@vger.kernel.org>
Subject: operation ordering during pgd_alloc/pgd_free
Date: Thu, 05 Jun 2008 09:51:55 +0100	[thread overview]
Message-ID: <4847C54B.76E4.0078.0@novell.com> (raw)

At present, pgd_ctor() adds a new pgd to pgd_list solely based on
!SHARED_KERNEL_PMD. For PAE && !SHARED_KERNEL_PMD (i.e. Xen)
this doesn't seem correct, as the pgd is still empty, which will confuse
vmalloc_sync_all(). So in this case, list insertion should only happen at
the end of pgd_prepopulate_pmd().

Likewise, pgd_free() calls pgd_mop_up_pmds() *before* pgd_dtor(),
with the former zeroing pgd entries as it goes and only the latter
removing the pgd from the list. Just as above this can confuse
vmalloc_sync_all(), so here I would think that the two calls should just
be swapped. However, if they get swapped, careful inspection of the
interaction with save/restore will be needed - XenSource's Linux tree
has a comment specifically to that effect:

/*
 * After this the pgd should not be pinned for the duration of this
 * function's execution. We should never sleep and thus never race:
 *  1. User pmds will not become write-protected under our feet due
 *     to a concurrent mm_pin_all().
 *  2. The machine addresses in PGD entries will not become invalid
 *     due to a concurrent save/restore.
 */

Since that tree doesn't support preemption, this is perhaps fine, but
likely going to cause problems in the (preemptable) pv-ops code.

The issue with vmalloc_sync_all() would even go unnoticed, since the
patch to unify the pgd_list mechanism with x86-64 removed the
BUG_ON() that was meant to trigger on issues like this.

But maybe I'm missing something?

Jan


             reply	other threads:[~2008-06-05  8:51 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-06-05  8:51 Jan Beulich [this message]
2008-06-05  9:27 ` operation ordering during pgd_alloc/pgd_free Jeremy Fitzhardinge
2008-06-05  9:58   ` Jan Beulich

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=4847C54B.76E4.0078.0@novell.com \
    --to=jbeulich@novell.com \
    --cc=jeremy@goop.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    /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.