vfs.git bits and pieces

[Apologies for delay; I'd spent the last day hunting down something that turned out to be a VM leak completely unrelated to this stuff - it's present in mainline, for starters. Unreliable reproducers make for fun bisects ;-/ Anyway, by now I'm absolutely sure that this is a VM bug and not something I had somehow managed to break, so...]

Assorted bits that got missed in the first pull request + fixes for a couple of coredump regressions. Please, pull from the usual place -
git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs.git for-linus

Shortlog:
Al Viro (7):
dump_align(): fix the dumb braino dump_emit(): use __kernel_write(), not vfs_write()
gfs2: endianness misannotations
consolidate simple ->d_delete() instances take read_seqbegin_or_lock() and friends to seqlock.h dcache.c: get rid of pointless macros
fold try_to_ascend() into the sole remaining caller

Diffstat:

 arch/ia64/kernel/perfmon.c |    8 +----
 fs/9p/vfs_dentry.c         |   19 +---------
 fs/configfs/dir.c          |   12 +------
 fs/coredump.c              |    6 ++--
 fs/dcache.c                |   84 ++++++++++---------------------------------
 fs/efivarfs/super.c        |   11 +-----
 fs/gfs2/lock_dlm.c         |    8 ++--
 fs/gfs2/quota.c            |   23 +++++-------
 fs/gfs2/rgrp.c             |    4 +-
 fs/hostfs/hostfs_kern.c    |   11 +-----
 fs/libfs.c                 |   12 ++++---
 fs/proc/generic.c          |   18 +---------
 fs/proc/namespaces.c       |    8 +----
 include/linux/fs.h         |    2 +
 include/linux/seqlock.h    |   29 +++++++++++++++
 kernel/cgroup.c            |    7 +---
 net/sunrpc/rpc_pipe.c      |   11 +-----
 17 files changed, 86 insertions(+), 187 deletions(-)

BTW, something odd happened to mm/memory.c - either a mangled patch or a lost followup. Take a look at the last commit in there:
commit ea1e7ed33708c7a760419ff9ded0a6cb90586a50

Author: Kirill A. Shutemov <[email protected]>
Date:   Thu Nov 14 14:31:53 2013 -0800

mm: create a separate slab for page->ptl allocation

If DEBUG_SPINLOCK and DEBUG_LOCK_ALLOC are enabled spinlock_t on x86_64 is 72 bytes. For page->ptl they will be allocated from kmalloc-96 slab, so we loose 24 on each. An average system can easily allocate few tens thousands of page->ptl and overhead is significant.

Let's create a separate slab for page->ptl allocation to solve this.

Fair enough, and yes, it does create that separate slab. The problem is, it's still using kmalloc/kfree for those beasts - page_ptl_cachep isn't used at all...

While digging in the same area:

Wrong page freed on preallocate_pmds() failure exit

Note that pmds[i] is simply uninitialized at that point... Granted, it's very hard to hit (you need split page locks *and* kmalloc(sizeof(spinlock_t), GFP_KERNEL) failing), but the code is obviously bogus.

---

index a7cccb6d..36aa999 100644
@@ -209,7 +209,7 @@ static int preallocate_pmds(pmd_t *pmds[])
 		if (!pmd)
 			failed = true;
 		if (pmd && !pgtable_pmd_page_ctor(virt_to_page(pmd))) {
-			free_page((unsigned long)pmds[i]);
+			free_page((unsigned long)pmd);
 			pmd = NULL;
 			failed = true;
 		}

Ok, it looks straightforward enough to just replace the kmalloc/kfree with using a slab allocation using the page_ptl_cachep pointer. I'd do it myself, but I would like to know how it got lost? Also, much testing to make sure the cachep is initialized early enough.

Or should we just revert the commit that added the pointless/unused slab pointer?

Andrew, Kirill, comments?

Also note the other issue Al found: see commit 2a46eed54a28 ("Wrong page freed on preallocate_pmds() failure exit") that I just pushed out.

Linus

agh, I went through hell keeping that patch alive and it appears I lost some of it.

Let's just kill it please. We can try again for 3.14.

lgtm, thanks.

Actually, I've lost it while adding BLOATED_SPINLOCKS :(

I'm okay with that.
Only side note: it's useful not only for debug case, but also for PREEMPT_RT where spinlock_t is always bloated.

Fixed patch:

From: "Kirill A. Shutemov" <[email protected]>
Date: Thu, 14 Nov 2013 14:31:53 -0800
Subject: [PATCH] mm: create a separate slab for page->ptl allocation, try two

If DEBUG_SPINLOCK and DEBUG_LOCK_ALLOC are enabled spinlock_t on x86_64 is 72 bytes. For page->ptl they will be allocated from kmalloc-96 slab, so we loose 24 on each. An average system can easily allocate few tens thousands of page->ptl and overhead is significant.

Let's create a separate slab for page->ptl allocation to solve this.

To make sure that it really works this time, some numbers from my test machine (just booted, no load):

Before:

  # grep '^\(kmalloc-96\|page->ptl\)' /proc/slabinfo
  kmalloc-96         31987  32190    128   30    1 : tunables  120   60    8 : slabdata   1073   1073     92
After:
  # grep '^\(kmalloc-96\|page->ptl\)' /proc/slabinfo
  page->ptl          27516  28143     72   53    1 : tunables  120   60    8 : slabdata    531    531      9
  kmalloc-96          3853   5280    128   30    1 : tunables  120   60    8 : slabdata    176    176      0

Note that the patch is useful not only for debug case, but also for PREEMPT_RT, where spinlock_t is always bloated.

---
 include/linux/mm.h |  9 +++++++++
 init/main.c        |  2 +-
 mm/memory.c        | 11 +++++++++--
 3 files changed, 19 insertions(+), 3 deletions(-)

index 1cedd000cf29..0548eb201e05 100644
@@ -1318,6 +1318,7 @@ static inline pmd_t *pmd_alloc(struct mm_struct *mm, pud_t *pud, unsigned long a
 
 #if USE_SPLIT_PTE_PTLOCKS
 #if BLOATED_SPINLOCKS
+void __init ptlock_cache_init(void);
 extern bool ptlock_alloc(struct page *page);
 extern void ptlock_free(struct page *page);
 

@@ -1326,6 +1327,7 @@ static inline spinlock_t *ptlock_ptr(struct page *page)
 	return page->ptl;
 }
 #else /* BLOATED_SPINLOCKS */
+static inline void ptlock_cache_init(void) {}
 static inline bool ptlock_alloc(struct page *page)
 {
 	return true;

@@ -1378,10 +1380,17 @@ static inline spinlock_t *pte_lockptr(struct mm_struct *mm, pmd_t *pmd)
 {
 	return &mm->page_table_lock;
 }
+static inline void ptlock_cache_init(void) {}
 static inline bool ptlock_init(struct page *page) { return true; }
 static inline void pte_lock_deinit(struct page *page) {}
 #endif /* USE_SPLIT_PTE_PTLOCKS */
 
+static inline void pgtable_init(void)
+{
+	ptlock_cache_init();
+	pgtable_cache_init();
+}
+
 static inline bool pgtable_page_ctor(struct page *page)
 {
 	inc_zone_page_state(page, NR_PAGETABLE);

index febc511e078a..01573fdfa186 100644
@@ -476,7 +476,7 @@ static void __init mm_init(void)
 	mem_init();
 	kmem_cache_init();
 	percpu_init_late();
-	pgtable_cache_init();
+	pgtable_init();
 	vmalloc_init();
 }
 

index 5d9025f3b3e1..cf6098c10084 100644
@@ -4272,11 +4272,18 @@ void copy_user_huge_page(struct page *dst, struct page *src,
 #endif /* CONFIG_TRANSPARENT_HUGEPAGE || CONFIG_HUGETLBFS */
 
 #if USE_SPLIT_PTE_PTLOCKS && BLOATED_SPINLOCKS
+static struct kmem_cache *page_ptl_cachep;
+void __init ptlock_cache_init(void)
+{
+	page_ptl_cachep = kmem_cache_create("page->ptl", sizeof(spinlock_t), 0,
+			SLAB_PANIC, NULL);
+}
+
 bool ptlock_alloc(struct page *page)
 {
 	spinlock_t *ptl;
 
-	ptl = kmalloc(sizeof(spinlock_t), GFP_KERNEL);
+	ptl = kmem_cache_alloc(page_ptl_cachep, GFP_KERNEL);
 	if (!ptl)
 		return false;
 	page->ptl = ptl;

@@ -4285,6 +4292,6 @@ bool ptlock_alloc(struct page *page)
 
 void ptlock_free(struct page *page)
 {
-	kfree(page->ptl);
+	kmem_cache_free(page_ptl_cachep, page->ptl);
 }
 #endif

 Kirill A. Shutemov

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

The initial sending had the proper hunks at the end, so something really got lost afterwards...
https://lkml.org/lkml/2013/10/22/129

Damien Wyart