Fwd: Re: struct page field arrangement

["Jan Beulich" <jbeulich@novell.com>]

[-- Attachment #1: Type: text/plain, Size: 2649 bytes --]

Having looked a little into the disabled SPLIT_PT_LOCK issue on Xen, I
realized that is shouldn't be too difficult to re-enable it (at least in some
cases).

One option would be to not use the mapping field of struct page for
PageForeign but rather the index one (and to that effect was a mail I
sent to Andrew Morton and Hugh Dickins that I got below answer on,
basically indicating that it should be okay to use index here). This would
allow removing the Xen conditional on SPLIT_PTLOCK_CPUS altogether.

The other option would be along the lines of attached patch (not
checked whether it would apply as-is to -unstable, but it appears to
work okay), allowing the splitting at least in non-debug configs.

Opinions?

Thanks, Jan

>>> Hugh Dickins <hugh@veritas.com> 28.02.07 22:08 >>>
On Wed, 28 Feb 2007, Jan Beulich wrote:

> A change early last year reordered struct page so that ptl overlaps not only
> private, but also mapping. Since spinlock_t can be much larger, I'm wondering
> whether there's a reason to not also overlay the space index and lru take -
> are these used for anything on page table pages?

Overlaying lru is a problem for for those architectures which use
kmem_cache_alloc for their pagetables: arm26, powerpc, sparc64 and
perhaps others (I just grepped quickly through include/asm*, didn't
follow up those who have extern functions): since slab reuses the
lru fields for its own purposes.  Could perhaps be stacked somehow.

Overlaying index is fairly straightforward: the index field is fair
game.  In my original patches I did overlay index, but Andrew was
strongly averse to the way I was doing it, and scaled things back,
to private alone if I remember rightly, then relaxed a little to
include mapping too.  Way back then I made up a patch to overlay
index too (when I saw Fedora going out with CONFIG_DEBUG_SPINLOCK),
but I could never get it into a form where I felt it would satisfy
Andrew; and grew increasingly dissatisfied with that approach myself.

I don't think further overlaying is the right answer really.
But I do think it's a scandal that the size of struct page (in a
DEBUG_SPINLOCK system) is governed by such a minority use of the
struct page.  Lacking a satisfying answer, I've just let it drift
on until someone notices and complains.

kmalloc a separate spinlock structure when it's too big to fit in?
Not such a good idea, since then there will tend to be false sharing
of cachelines between them: simpler just to disable SPLIT_PTLOCK in
that case.

I'm not happy with the status quo, but I don't know the right answer:
perhaps allow pagetable pages to use an undebugged spinlock variant?

Hugh

[-- Attachment #2: xen-split-pt-lock.patch --]
[-- Type: text/plain, Size: 2456 bytes --]

From: jbeulich@novell.com
Subject: allow use of split page table lock in non-debug case
Patch-mainline: obsolete

Index: head-2007-02-27/include/linux/mm.h
===================================================================
--- head-2007-02-27.orig/include/linux/mm.h	2007-02-27 16:25:00.000000000 +0100
+++ head-2007-02-27/include/linux/mm.h	2007-02-28 17:11:56.000000000 +0100
@@ -891,7 +891,11 @@ static inline pmd_t *pmd_alloc(struct mm
 #define pte_lock_init(_page)	do {					\
 	spin_lock_init(__pte_lockptr(_page));				\
 } while (0)
-#define pte_lock_deinit(page)	((page)->mapping = NULL)
+#define pte_lock_deinit(_page)	do {					\
+	if ((void *)&((struct page *)0)->mapping			\
+	    < (void *)(__pte_lockptr((struct page *)0) + 1))		\
+		(_page)->mapping = NULL;				\
+} while (0)
 #define pte_lockptr(mm, pmd)	({(void)(mm); __pte_lockptr(pmd_page(*(pmd)));})
 #else
 /*
Index: head-2007-02-27/include/linux/page-flags.h
===================================================================
--- head-2007-02-27.orig/include/linux/page-flags.h	2007-02-27 16:26:22.000000000 +0100
+++ head-2007-02-27/include/linux/page-flags.h	2007-02-28 16:38:41.000000000 +0100
@@ -254,8 +254,16 @@ static inline void SetPageUptodate(struc
 #define ClearPageUncached(page)	clear_bit(PG_uncached, &(page)->flags)
 
 #define PageForeign(page)	test_bit(PG_foreign, &(page)->flags)
+#if NR_CPUS >= CONFIG_SPLIT_PTLOCK_CPUS
+#define PageForeignCheck()						\
+	BUILD_BUG_ON((void *)&((struct page *)0)->mapping		\
+	             < (void *)(__pte_lockptr((struct page *)0) + 1))
+#else
+#define PageForeignCheck() ((void)0)
+#endif
 #define SetPageForeign(page, dtor) do {		\
 	set_bit(PG_foreign, &(page)->flags);	\
+	PageForeignCheck();			\
 	(page)->mapping = (void *)dtor;		\
 } while (0)
 #define ClearPageForeign(page) do {		\
Index: head-2007-02-27/mm/Kconfig
===================================================================
--- head-2007-02-27.orig/mm/Kconfig	2007-02-27 16:23:14.000000000 +0100
+++ head-2007-02-27/mm/Kconfig	2007-02-28 16:26:48.000000000 +0100
@@ -139,7 +139,8 @@ config SPLIT_PTLOCK_CPUS
 	int
 	default "4096" if ARM && !CPU_CACHE_VIPT
 	default "4096" if PARISC && !PA20
-	default "4096" if X86_XEN || X86_64_XEN
+	default "4096" if X86_XEN && (PREEMPT || DEBUG_SPINLOCK || DEBUG_LOCK_ALLOC)
+	default "4096" if X86_64_XEN && (DEBUG_SPINLOCK || DEBUG_LOCK_ALLOC)
 	default "4"
 
 #

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel