public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: William Lee Irwin III <wli@holomorphy.com>
To: Hugh Dickins <hugh@veritas.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Andi Kleen <ak@suse.de>, Christoph Lameter <clameter@sgi.com>,
	linux-kernel@vger.kernel.org
Subject: Re: slub-i386-support.patch
Date: Thu, 10 May 2007 18:08:42 -0700	[thread overview]
Message-ID: <20070511010842.GJ31925@holomorphy.com> (raw)
In-Reply-To: <20070511000702.GI31925@holomorphy.com>

On Thu, May 10, 2007 at 05:07:02PM -0700, William Lee Irwin III wrote:
> quicklist_free() with unflushed TLB entries admits speculation through
> the pagetable entries corresponding to the list links. So tlb_finish_mmu()
> is the place to call quicklist_free() on pagetables. This requires
> distinguishing preconstructed pagetables from freed user pages, which
> is not done in include/asm-generic/tlb.h (and core callers may need
> to be adjusted, pending the results of audits).
> To clarify, upper levels of pagetables are indeed cached by x86 TLB's.
> The same kind of deferral of freeing until the TLB is flushed required
> for leaf pagetables is required for the upper levels as well.

Looking more closely at it, the entire attempt to avoid struct page
pointers is far beyond pointless. The freeing functions unconditionally
require struct page pointers to either be passed or computed and the
allocation function's virtual address it returns as a result is not
directly usable. The callers all have to do arithmetic on the result.
One might as well stash precomputed pfn's (if not paddrs) and vaddrs in
page->private and page->mapping, chain them with ->lru (use only .next
if you care to stay singly-linked), and handle struct page pointers
throughout. At that point quicklists not only become directly callable
for pagetable freeing (including upper levels) instead of needing calls
to quicklist freeing staged to occur at the time of tlb_finish_mmu(),
but also become usable for the highpte case.

The computations this is trying to save on are computing the virtual
and physical addresses (pfn's modulo a cheap shift; besides, all the
API's work on pfn's) of a page from the pointer to the struct page.
Chaining through the memory for the page incurs the cost of having to
stage freeing through tlb_finish_mmu() instead of using the quicklist
as a staging arena directly. So the translation from a struct page
pointer is not saving work. It's not saving cache, either. The page's
memory is no more likely to be hot than its struct page.

In the course of freeing the pointer to the struct page is computed
whether by the caller or the API function. So the translation to a
struct page pointer is done during freeing regardless.

A better solution would be to precompute those results and store
them in various fields of the struct page. i386 can move to using
generation numbers (->_mapcount and ->index are still available
for 64 bits there even after quicklists use ->lru, ->mapping, and
->private, and quicklists really only need half of ->lru) to handle
change_page_attr() and vmalloc_sync().


-- wli

  reply	other threads:[~2007-05-11  1:08 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-05-10 20:03 slub-i386-support.patch Hugh Dickins
2007-05-10 20:17 ` slub-i386-support.patch Andrew Morton
2007-05-10 20:31   ` slub-i386-support.patch Christoph Lameter
2007-05-10 20:50     ` slub-i386-support.patch David Miller
2007-05-10 20:31 ` slub-i386-support.patch William Lee Irwin III
2007-05-10 20:35   ` slub-i386-support.patch Christoph Lameter
2007-05-10 21:09     ` slub-i386-support.patch William Lee Irwin III
2007-05-10 21:28       ` slub-i386-support.patch Jeremy Fitzhardinge
2007-05-10 23:35         ` slub-i386-support.patch William Lee Irwin III
2007-05-10 21:22     ` slub-i386-support.patch Jeremy Fitzhardinge
2007-05-10 23:14   ` slub-i386-support.patch Hugh Dickins
2007-05-11  0:07     ` slub-i386-support.patch William Lee Irwin III
2007-05-11  1:08       ` William Lee Irwin III [this message]
2007-05-11  5:09         ` slub-i386-support.patch Christoph Lameter
2007-05-11  7:43           ` slub-i386-support.patch William Lee Irwin III
2007-05-11  1:42       ` slub-i386-support.patch William Lee Irwin III
2007-05-11  8:29     ` slub-i386-support.patch Andi Kleen
2007-05-11  7:42       ` slub-i386-support.patch Andrew Morton
2007-05-11  7:54         ` slub-i386-support.patch William Lee Irwin III
2007-05-11 16:15           ` slub-i386-support.patch Christoph Lameter
2007-05-11 20:47             ` slub-i386-support.patch William Lee Irwin III
2007-05-11  9:27         ` slub-i386-support.patch Hugh Dickins

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=20070511010842.GJ31925@holomorphy.com \
    --to=wli@holomorphy.com \
    --cc=ak@suse.de \
    --cc=akpm@linux-foundation.org \
    --cc=clameter@sgi.com \
    --cc=hugh@veritas.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=torvalds@linux-foundation.org \
    /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