Re: [PATCH 7/9] mm: split page table lock

[Hugh Dickins]

On Sun, 23 Oct 2005, Andrew Morton wrote:
> Andrew Morton <akpm@xxxxxxxx> wrote:
> >  Hugh Dickins <hugh@xxxxxxxxxxx> wrote:
> >  >
> >  > In this implementation, the spinlock is tucked inside the struct page of
> >  >  the page table page: with a BUILD_BUG_ON in case it overflows - which it
> >  >  would in the case of 32-bit PA-RISC with spinlock debugging enabled.
> > 
> >  eh?   It's going to overflow an unsigned long on x86 too:
> 
> Ah, I think I see what you've done: assume that .index, .lru and .virtual
> are unused on pagetable pages, so we can just overwrite them.

That's right (so I'm ignoring some of your earlier stabs).
Sorry, it's looking like my comment block was inadequate.

I'm also assuming that it'd be very quickly noticed, by bad_page
or otherwise, if anyone changes these fields, so that what it's
overwriting becomes significant.

Would it be better if pte_lock_init(page) explicitly initialize each
field from _page->index onwards, so that any search for uses of that
page field shows up pte_lock_init?  With the BUILD_BUG_ON adjusted
somehow so _page->virtual is excluded (I tend to erase that one from
my mind, but we most certainly don't want a spinlock overwriting it).

> ick.  I think I prefer the union, although it'll make struct page bigger
> for CONFIG_PREEMPT+CONFIG_SMP+NR_CPUS>=4.    hmm.

Hmm indeed.  Definitely not the tradeoff I chose or would choose.

Hugh
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/