From: Hugh Dickins <hugh@veritas.com>
To: Andrew Morton <akpm@osdl.org>
Cc: linux-kernel@vger.kernel.org
Subject: [PATCH 01/15] mm: poison struct page for ptlock
Date: Thu, 10 Nov 2005 01:43:14 +0000 (GMT) [thread overview]
Message-ID: <Pine.LNX.4.61.0511100142160.5814@goblin.wat.veritas.com> (raw)
In-Reply-To: <Pine.LNX.4.61.0511100139550.5814@goblin.wat.veritas.com>
The split ptlock patch enlarged the default SMP PREEMPT struct page from
32 to 36 bytes on most 32-bit platforms, from 32 to 44 bytes on PA-RISC
7xxx (without PREEMPT). That was not my intention, and I don't believe
that split ptlock deserves any such slice of the user's memory.
Could we please try this patch, or something like it? Again to overlay
the spinlock_t from &page->private onwards, with corrected BUILD_BUG_ON
that we don't go too far; with poisoning of the fields overlaid, and
unsplit SMP config verifying that the split config is safe to use them.
The previous attempt at this patch broke ppc64, which uses slab for its
page tables - and slab saves vital info in page->lru: but no config of
spinlock_t needs to overwrite lru on 64-bit anyway; and the only 32-bit
user of slab for page tables is arm26, which is never SMP i.e. all the
problems came from the "safety" checks, not from what's actually needed.
So previous checks refined with #ifdefs, and a BUG_ON(PageSlab) added.
This overlaying is unlikely to be portable forever: but the added checks
should warn developers when it's going to break, long before any users.
Signed-off-by: Hugh Dickins <hugh@veritas.com>
---
include/linux/mm.h | 78 +++++++++++++++++++++++++++++++++++++++++++----------
1 files changed, 64 insertions(+), 14 deletions(-)
--- 2.6.14-mm1/include/linux/mm.h 2005-11-07 07:39:57.000000000 +0000
+++ mm01/include/linux/mm.h 2005-11-09 14:37:47.000000000 +0000
@@ -224,18 +224,19 @@ struct page {
* to show when page is mapped
* & limit reverse map searches.
*/
- union {
- unsigned long private; /* Mapping-private opaque data:
+ unsigned long private; /* Mapping-private opaque data:
* usually used for buffer_heads
* if PagePrivate set; used for
* swp_entry_t if PageSwapCache
* When page is free, this indicates
* order in the buddy system.
*/
-#if NR_CPUS >= CONFIG_SPLIT_PTLOCK_CPUS
- spinlock_t ptl;
-#endif
- } u;
+ /*
+ * Depending on config, along with private, subsequent fields of
+ * a page table page's struct page may be overlaid by a spinlock
+ * for pte locking: see comment on "split ptlock" below. Please
+ * do not rearrange these fields without considering that usage.
+ */
struct address_space *mapping; /* If low bit clear, points to
* inode address_space, or NULL.
* If page mapped as anonymous
@@ -268,8 +269,8 @@ struct page {
#endif
};
-#define page_private(page) ((page)->u.private)
-#define set_page_private(page, v) ((page)->u.private = (v))
+#define page_private(page) ((page)->private)
+#define set_page_private(page, v) ((page)->private = (v))
/*
* FIXME: take this include out, include page-flags.h in
@@ -827,25 +828,74 @@ static inline pmd_t *pmd_alloc(struct mm
}
#endif /* CONFIG_MMU && !__ARCH_HAS_4LEVEL_HACK */
+/*
+ * In the split ptlock case, we shall be overlaying the struct page
+ * of a page table page with a spinlock starting at &page->private:
+ * ending dependent on architecture and config, but never beyond lru.
+ *
+ * So poison page table struct page in all SMP cases (in part to assert
+ * our territory: that pte locking owns these fields of a page table's
+ * struct page); and verify it when freeing in the unsplit ptlock case,
+ * when none of these fields should have been touched. So that now the
+ * common config checks also for the larger PREEMPT DEBUG_SPINLOCK lock.
+ *
+ * Poison lru only in the 32-bit configs, since no 64-bit spinlock_t
+ * extends that far - and ppc64 allocates from slab, which saves info in
+ * these lru fields (arm26 also allocates from slab, but is never SMP).
+ * Poison lru back-to-front, to make sure that list_del was not used.
+ */
+static inline void poison_struct_page(struct page *page)
+{
+#ifdef CONFIG_SMP
+ page->private = (unsigned long) page;
+ page->mapping = (struct address_space *) page;
+ page->index = (pgoff_t) page;
+#if BITS_PER_LONG == 32
+ BUG_ON(PageSlab(page));
+ page->lru.next = LIST_POISON2;
+ page->lru.prev = LIST_POISON1;
+#endif
+#endif
+}
+
+static inline void verify_struct_page(struct page *page)
+{
+#ifdef CONFIG_SMP
+ BUG_ON(page->private != (unsigned long) page);
+ BUG_ON(page->mapping != (struct address_space *) page);
+ BUG_ON(page->index != (pgoff_t) page);
+ page->mapping = NULL;
+#if BITS_PER_LONG == 32
+ BUG_ON(page->lru.next != LIST_POISON2);
+ BUG_ON(page->lru.prev != LIST_POISON1);
+#endif
+#endif
+}
+
#if NR_CPUS >= CONFIG_SPLIT_PTLOCK_CPUS
/*
* We tuck a spinlock to guard each pagetable page into its struct page,
* at page->private, with BUILD_BUG_ON to make sure that this will not
- * overflow into the next struct page (as it might with DEBUG_SPINLOCK).
- * When freeing, reset page->mapping so free_pages_check won't complain.
+ * extend further than expected.
*/
-#define __pte_lockptr(page) &((page)->u.ptl)
+#define __pte_lockptr(page) ((spinlock_t *)&((page)->private))
#define pte_lock_init(_page) do { \
+ BUILD_BUG_ON(__pte_lockptr((struct page *)0) + 1 > (spinlock_t*)\
+ (&((struct page *)0)->lru + (BITS_PER_LONG == 32))); \
+ poison_struct_page(_page); \
spin_lock_init(__pte_lockptr(_page)); \
} while (0)
+/*
+ * When freeing, reset page->mapping so free_pages_check won't complain.
+ */
#define pte_lock_deinit(page) ((page)->mapping = NULL)
#define pte_lockptr(mm, pmd) ({(void)(mm); __pte_lockptr(pmd_page(*(pmd)));})
-#else
+#else /* NR_CPUS < CONFIG_SPLIT_PTLOCK_CPUS */
/*
* We use mm->page_table_lock to guard all pagetable pages of the mm.
*/
-#define pte_lock_init(page) do {} while (0)
-#define pte_lock_deinit(page) do {} while (0)
+#define pte_lock_init(page) poison_struct_page(page)
+#define pte_lock_deinit(page) verify_struct_page(page)
#define pte_lockptr(mm, pmd) ({(void)(pmd); &(mm)->page_table_lock;})
#endif /* NR_CPUS < CONFIG_SPLIT_PTLOCK_CPUS */
next prev parent reply other threads:[~2005-11-10 1:44 UTC|newest]
Thread overview: 52+ messages / expand[flat|nested] mbox.gz Atom feed top
2005-11-10 1:42 [PATCH 00/15] mm: struct page lock and counts Hugh Dickins
2005-11-10 1:43 ` Hugh Dickins [this message]
2005-11-10 2:10 ` [PATCH 01/15] mm: poison struct page for ptlock Andrew Morton
2005-11-10 2:22 ` Hugh Dickins
2005-11-10 2:56 ` Andrew Morton
2005-11-10 2:58 ` Andrew Morton
2005-11-10 11:28 ` Ingo Molnar
2005-11-10 12:06 ` Ingo Molnar
2005-11-10 12:26 ` Andrew Morton
2005-11-10 21:37 ` Christoph Lameter
2005-11-10 21:52 ` Christoph Hellwig
2005-11-11 10:46 ` Ingo Molnar
2005-11-12 23:48 ` Adrian Bunk
2005-11-10 12:35 ` Hugh Dickins
2005-11-10 12:51 ` Andrew Morton
2005-11-10 13:29 ` Hugh Dickins
2005-11-10 15:00 ` Ingo Molnar
2005-11-10 15:38 ` Hugh Dickins
2005-11-10 19:49 ` Andrew Morton
2005-11-10 19:56 ` Linus Torvalds
2005-11-11 0:10 ` Russell King
2005-11-12 6:27 ` Benjamin Herrenschmidt
2005-11-11 15:02 ` Hugh Dickins
2005-11-15 18:49 ` Andrew Morton
2005-11-15 19:51 ` Hugh Dickins
2005-11-15 20:05 ` Andrew Morton
2005-11-10 1:44 ` [PATCH 02/15] mm: revert page_private Hugh Dickins
2005-11-10 1:46 ` [PATCH 03/15] mm reiser4: " Hugh Dickins
2005-11-10 1:47 ` [PATCH 04/15] mm: update split ptlock Kconfig Hugh Dickins
2005-11-10 1:48 ` [PATCH 05/15] mm: unbloat get_futex_key Hugh Dickins
2005-11-10 1:50 ` [PATCH 06/15] mm: remove ppc highpte Hugh Dickins
2005-11-10 1:52 ` Benjamin Herrenschmidt
2005-11-10 1:55 ` Paul Mackerras
2005-11-10 2:46 ` Hugh Dickins
2005-11-10 1:51 ` [PATCH 07/15] mm: powerpc ptlock comments Hugh Dickins
2005-11-10 1:53 ` [PATCH 08/15] mm: powerpc init_mm without ptlock Hugh Dickins
2005-11-10 1:56 ` [PATCH 09/15] mm: fill arch atomic64 gaps Hugh Dickins
2005-11-10 13:38 ` Andi Kleen
2005-11-10 15:19 ` Hugh Dickins
2005-11-10 1:57 ` [PATCH 10/15] mm: atomic64 page counts Hugh Dickins
2005-11-10 2:16 ` Andrew Morton
2005-11-10 2:33 ` Hugh Dickins
2005-11-10 3:01 ` Andrew Morton
2005-11-10 21:43 ` Christoph Lameter
2005-11-10 21:53 ` Andrew Morton
2005-11-11 15:25 ` Hugh Dickins
2005-11-11 18:03 ` Christoph Lameter
2005-11-10 2:00 ` [PATCH 11/15] mm: long " Hugh Dickins
2005-11-10 2:01 ` [PATCH 12/15] mm reiser4: " Hugh Dickins
2005-11-10 2:03 ` [PATCH 13/15] mm: get_user_pages check count Hugh Dickins
2005-11-10 2:08 ` [PATCH 14/15] mm: inc_page_table_pages check max Hugh Dickins
2005-11-10 2:09 ` [PATCH 15/15] mm: remove install_page limit 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=Pine.LNX.4.61.0511100142160.5814@goblin.wat.veritas.com \
--to=hugh@veritas.com \
--cc=akpm@osdl.org \
--cc=linux-kernel@vger.kernel.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
all inboxes | Powered by JetHome®