Re: slub-i386-support.patch

[Hugh Dickins <hugh@veritas.com>]

On Thu, 10 May 2007, William Lee Irwin III wrote:
> On Thu, May 10, 2007 at 09:03:39PM +0100, Hugh Dickins wrote:
> > Though when I look at the patchset (copied below), I do wonder why
> > it puts a quicklist_trim() into i386's cpu_idle() and flush_tlb_mm():
> > neither is where I'd expect us to be secretly freeing pages.  Ah,
> > several arches do it in cpu_idle(): how odd, oh well.
> 
> So now quicklist semantics vs. TLB flushing are the motive behind the
> odd flush_tlb_mm() affair. The real trick with it is that flushing
> must never occur until the TLB flush. Any change to the core quicklist
> code that retires pages back to the page allocator earlier (e.g. based
> on some limit) will break things badly.

I don't think that's right.  It's vital that TLB (of an active mm)
be flushed before freeing its page back to the quicklist, before it's
recycled to another mm (or elsewhere in this mm); but having done that,
it really doesn't matter much when quicklist_trim() (check_pgt_cache)
is called to free surplus pages from quicklist back to page_alloc.c.

tlb_finish_mmu() happens to be the traditional place it's done, and
that's where we expect it.  flush_tlb_mm() avoids flushing TLB unless
it's actually required for the mm in question: so wouldn't be a good
place to rely on flushing TLB for pages freed earlier from other mms
(but we'd already be in trouble to be leaving them that late).

I'm guessing (haven't rechecked source) that the cpu_idle() call comes
about because the top level pgd of a process gets freed very late in
its exit, and after a great flurry of processes have just exited,
perhaps there was nothing to free up the accumulation.  Though
it still strikes me as an odd place to do it.

Hugh