Linux-mm Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH 1/2] mm: avoid spurious 'bad pmd' warning messages
From: Dave Hansen @ 2017-05-17 17:33 UTC (permalink / raw)
  To: Ross Zwisler, Andrew Morton, linux-kernel
  Cc: Darrick J. Wong, Alexander Viro, Christoph Hellwig, Dan Williams,
	Jan Kara, Matthew Wilcox, linux-fsdevel, linux-mm, linux-nvdimm,
	Kirill A . Shutemov, Pawel Lebioda, Dave Jiang, Xiong Zhou,
	Eryu Guan, stable
In-Reply-To: <20170517171639.14501-1-ross.zwisler@linux.intel.com>

On 05/17/2017 10:16 AM, Ross Zwisler wrote:
> @@ -3061,7 +3061,7 @@ static int pte_alloc_one_map(struct vm_fault *vmf)
>  	 * through an atomic read in C, which is what pmd_trans_unstable()
>  	 * provides.
>  	 */
> -	if (pmd_trans_unstable(vmf->pmd) || pmd_devmap(*vmf->pmd))
> +	if (pmd_devmap(*vmf->pmd) || pmd_trans_unstable(vmf->pmd))
>  		return VM_FAULT_NOPAGE;

I'm worried we are very unlikely to get this right in the future.  It's
totally not obvious what the ordering requirement is here.

Could we move pmd_devmap() and pmd_trans_unstable() into a helper that
gets the ordering right and also spells out the ordering requirement?

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

^ permalink raw reply

* Re: [PATCH] Correct spelling and grammar for notification text
From: mikey d @ 2017-05-17 17:23 UTC (permalink / raw)
  To: Randy Dunlap; +Cc: linux-mm, trivial
In-Reply-To: <d55eeeee-76ed-b171-d1df-643b36bb17b9@infradead.org>

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

V2 incoming soon. My first patch :)

On May 17, 2017 11:35, "Randy Dunlap" <rdunlap@infradead.org> wrote:

> On 05/17/17 06:38, Michael DeGuzis wrote:
> > From: professorkaos64 <mdeguzis@gmail.com>
> >
> > This patch fixes up some grammar and spelling in the information
> > block for huge_memory.c.
>
> Missing Signed-off-by: <real name and email address>
>
> > ---
> >  mm/huge_memory.c | 10 +++++-----
> >  1 file changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> > index a84909cf20d3..af137fc0ca09 100644
> > --- a/mm/huge_memory.c
> > +++ b/mm/huge_memory.c
> > @@ -38,12 +38,12 @@
> >  #include "internal.h"
> >
> >  /*
> > - * By default transparent hugepage support is disabled in order that
> avoid
> > - * to risk increase the memory footprint of applications without a
> guaranteed
> > - * benefit. When transparent hugepage support is enabled, is for all
> mappings,
> > - * and khugepaged scans all mappings.
> > + * By default, transparent hugepage support is disabled in order to
> avoid
> > + * risking an increased memory footprint for applications that are not
> > + * guaranteed to benefit from it. When transparent hugepage support is
> > + * enabled, it is for all mappings, and khugepaged scans all mappings.
> >   * Defrag is invoked by khugepaged hugepage allocations and by page
> faults
> > - * for all hugepage allocations.
> > + * for all hugepage allocations.
>
> Several of the new (+) patch lines end with a space character. Not good.
>
> >   */
> >  unsigned long transparent_hugepage_flags __read_mostly =
> >  #ifdef CONFIG_TRANSPARENT_HUGEPAGE_ALWAYS
> >
>
>
> --
> ~Randy
>

[-- Attachment #2: Type: text/html, Size: 2235 bytes --]

^ permalink raw reply

* [PATCH] generic: add regression test for DAX PTE/PMD races
From: Ross Zwisler @ 2017-05-17 17:17 UTC (permalink / raw)
  To: fstests, Xiong Zhou, Eryu Guan
  Cc: Ross Zwisler, Darrick J. Wong, Alexander Viro, Christoph Hellwig,
	Dan Williams, Dave Hansen, Jan Kara, Matthew Wilcox,
	linux-fsdevel, linux-mm, linux-nvdimm, Andrew Morton,
	linux-kernel, Kirill A . Shutemov, Pawel Lebioda, Dave Jiang
In-Reply-To: <20170517171639.14501-2-ross.zwisler@linux.intel.com>

This adds a regression test for the following kernel patches:

  mm: avoid spurious 'bad pmd' warning messages
  dax: Fix race between colliding PMD & PTE entries

The above patches fix two related PMD vs PTE races in the DAX code.  These
can both be easily triggered by having two threads reading and writing
simultaneously to the same private mapping, with the key being that private
mapping reads can be handled with PMDs but private mapping writes are
always handled with PTEs so that we can COW.

Without this 2-patch kernel series, the newly added test will result in the
following errors:

  run fstests generic/435 at 2017-05-16 16:53:43
  mm/pgtable-generic.c:39: bad pmd ffff8808daa49b88(84000001006000a5)
  	... a bunch of the bad pmd messages ...
  BUG: Bad rss-counter state mm:ffff8800a8c1b700 idx:1 val:1
  BUG: non-zero nr_ptes on freeing mm: 38
  XFS (pmem0p1): Unmounting Filesystem

Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
---
 .gitignore            |   1 +
 src/Makefile          |   3 +-
 src/t_mmap_cow_race.c | 106 ++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/generic/435     |  61 +++++++++++++++++++++++++++++
 tests/generic/435.out |   2 +
 tests/generic/group   |   1 +
 6 files changed, 173 insertions(+), 1 deletion(-)
 create mode 100644 src/t_mmap_cow_race.c
 create mode 100755 tests/generic/435
 create mode 100644 tests/generic/435.out

diff --git a/.gitignore b/.gitignore
index 38f3a00..a6ac25e 100644
--- a/.gitignore
+++ b/.gitignore
@@ -149,6 +149,7 @@
 /src/t_rename_overwrite
 /src/t_mmap_dio
 /src/t_mmap_stale_pmd
+/src/t_mmap_cow_race
 
 # dmapi/ binaries
 /dmapi/src/common/cmd/read_invis
diff --git a/src/Makefile b/src/Makefile
index e5042c9..b505b42 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -12,7 +12,8 @@ TARGETS = dirstress fill fill2 getpagesize holes lstat64 \
 	godown resvtest writemod makeextents itrash rename \
 	multi_open_unlink dmiperf unwritten_sync genhashnames t_holes \
 	t_mmap_writev t_truncate_cmtime dirhash_collide t_rename_overwrite \
-	holetest t_truncate_self t_mmap_dio af_unix t_mmap_stale_pmd
+	holetest t_truncate_self t_mmap_dio af_unix t_mmap_stale_pmd \
+	t_mmap_cow_race
 
 LINUX_TARGETS = xfsctl bstat t_mtab getdevicesize preallo_rw_pattern_reader \
 	preallo_rw_pattern_writer ftrunc trunc fs_perms testx looptest \
diff --git a/src/t_mmap_cow_race.c b/src/t_mmap_cow_race.c
new file mode 100644
index 0000000..207ba42
--- /dev/null
+++ b/src/t_mmap_cow_race.c
@@ -0,0 +1,106 @@
+#include <errno.h>
+#include <fcntl.h>
+#include <libgen.h>
+#include <pthread.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/mman.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+#include <unistd.h>
+
+#define MiB(a) ((a)*1024*1024)
+#define NUM_THREADS 2
+
+void err_exit(char *op)
+{
+	fprintf(stderr, "%s: %s\n", op, strerror(errno));
+	exit(1);
+}
+
+void worker_fn(void *ptr)
+{
+	char *data = (char *)ptr;
+	volatile int a;
+	int i, err;
+
+	for (i = 0; i < 10; i++) {
+		a = data[0];
+		data[0] = a;
+
+		err = madvise(data, MiB(2), MADV_DONTNEED);
+		if (err < 0)
+			err_exit("madvise");
+
+		/* Mix up the thread timings to encourage the race. */
+		err = usleep(rand() % 100);
+		if (err < 0)
+			err_exit("usleep");
+	}
+}
+
+int main(int argc, char *argv[])
+{
+	pthread_t thread[NUM_THREADS];
+	int i, j, fd, err;
+	char *data;
+
+	if (argc < 2) {
+		printf("Usage: %s <file>\n", basename(argv[0]));
+		exit(0);
+	}
+
+	fd = open(argv[1], O_RDWR|O_CREAT, S_IRUSR|S_IWUSR);
+	if (fd < 0)
+		err_exit("fd");
+
+	/* This allows us to map a huge page. */
+	ftruncate(fd, 0);
+	ftruncate(fd, MiB(2));
+
+	/*
+	 * First we set up a shared mapping.  Our write will (hopefully) get
+	 * the filesystem to give us a 2MiB huge page DAX mapping.  We will
+	 * then use this 2MiB page for our private mapping race.
+	 */
+	data = mmap(NULL, MiB(2), PROT_READ|PROT_WRITE, MAP_SHARED, fd, 0);
+	if (data == MAP_FAILED)
+		err_exit("shared mmap");
+
+	data[0] = 1;
+
+	err = munmap(data, MiB(2));
+	if (err < 0)
+		err_exit("shared munmap");
+
+	for (i = 0; i < 500; i++) {
+		data = mmap(NULL, MiB(2), PROT_READ|PROT_WRITE, MAP_PRIVATE,
+				fd, 0);
+		if (data == MAP_FAILED)
+			err_exit("private mmap");
+
+		for (j = 0; j < NUM_THREADS; j++) {
+			err = pthread_create(&thread[j], NULL,
+					(void*)&worker_fn, data);
+			if (err)
+				err_exit("pthread_create");
+		}
+
+		for (j = 0; j < NUM_THREADS; j++) {
+			err = pthread_join(thread[j], NULL);
+			if (err)
+				err_exit("pthread_join");
+		}
+
+		err = munmap(data, MiB(2));
+		if (err < 0)
+			err_exit("private munmap");
+	}
+
+	err = close(fd);
+	if (err < 0)
+		err_exit("close");
+
+	return 0;
+}
diff --git a/tests/generic/435 b/tests/generic/435
new file mode 100755
index 0000000..f1413f1
--- /dev/null
+++ b/tests/generic/435
@@ -0,0 +1,61 @@
+#! /bin/bash
+# FS QA Test 435
+#
+# This is a regression test for kernel patches:
+#   mm: avoid spurious 'bad pmd' warning messages
+#   dax: Fix race between colliding PMD & PTE entries
+# created by Ross Zwisler <ross.zwisler@linux.intel.com>
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2017 Intel Corporation.  All Rights Reserved.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write the Free Software Foundation,
+# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+#-----------------------------------------------------------------------
+#
+
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1	# failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	cd /
+	rm -f $tmp.*
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+
+# remove previous $seqres.full before test
+rm -f $seqres.full
+
+# Modify as appropriate.
+_supported_fs generic
+_supported_os Linux
+_require_test
+_require_test_program "t_mmap_cow_race"
+
+# real QA test starts here
+src/t_mmap_cow_race $TEST_DIR/testfile
+
+# success, all done
+echo "Silence is golden"
+status=0
+exit
diff --git a/tests/generic/435.out b/tests/generic/435.out
new file mode 100644
index 0000000..6a175d3
--- /dev/null
+++ b/tests/generic/435.out
@@ -0,0 +1,2 @@
+QA output created by 435
+Silence is golden
diff --git a/tests/generic/group b/tests/generic/group
index c4911b8..ac43f42 100644
--- a/tests/generic/group
+++ b/tests/generic/group
@@ -437,3 +437,4 @@
 432 auto quick copy
 433 auto quick copy
 434 auto quick copy
+435 auto quick
-- 
2.9.4

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

^ permalink raw reply related

* [PATCH 2/2] dax: Fix race between colliding PMD & PTE entries
From: Ross Zwisler @ 2017-05-17 17:16 UTC (permalink / raw)
  To: Andrew Morton, linux-kernel
  Cc: Ross Zwisler, Darrick J. Wong, Alexander Viro, Christoph Hellwig,
	Dan Williams, Dave Hansen, Jan Kara, Matthew Wilcox,
	linux-fsdevel, linux-mm, linux-nvdimm, Kirill A . Shutemov,
	Pawel Lebioda, Dave Jiang, Xiong Zhou, Eryu Guan, stable
In-Reply-To: <20170517171639.14501-1-ross.zwisler@linux.intel.com>

We currently have two related PMD vs PTE races in the DAX code.  These can
both be easily triggered by having two threads reading and writing
simultaneously to the same private mapping, with the key being that private
mapping reads can be handled with PMDs but private mapping writes are
always handled with PTEs so that we can COW.

Here is the first race:

CPU 0					CPU 1

(private mapping write)
__handle_mm_fault()
  create_huge_pmd() - FALLBACK
  handle_pte_fault()
    passes check for pmd_devmap()

					(private mapping read)
					__handle_mm_fault()
					  create_huge_pmd()
					    dax_iomap_pmd_fault() inserts PMD

    dax_iomap_pte_fault() does a PTE fault, but we already have a DAX PMD
    			  installed in our page tables at this spot.

Here's the second race:

CPU 0					CPU 1

(private mapping write)
__handle_mm_fault()
  create_huge_pmd() - FALLBACK
					(private mapping read)
					__handle_mm_fault()
					  passes check for pmd_none()
					  create_huge_pmd()

  handle_pte_fault()
    dax_iomap_pte_fault() inserts PTE
					    dax_iomap_pmd_fault() inserts PMD,
					       but we already have a PTE at
					       this spot.

The core of the issue is that while there is isolation between faults to
the same range in the DAX fault handlers via our DAX entry locking, there
is no isolation between faults in the code in mm/memory.c.  This means for
instance that this code in __handle_mm_fault() can run:

	if (pmd_none(*vmf.pmd) && transparent_hugepage_enabled(vma)) {
		ret = create_huge_pmd(&vmf);

But by the time we actually get to run the fault handler called by
create_huge_pmd(), the PMD is no longer pmd_none() because a racing PTE
fault has installed a normal PMD here as a parent.  This is the cause of
the 2nd race.  The first race is similar - there is the following check in
handle_pte_fault():

	} else {
		/* See comment in pte_alloc_one_map() */
		if (pmd_devmap(*vmf->pmd) || pmd_trans_unstable(vmf->pmd))
			return 0;

So if a pmd_devmap() PMD (a DAX PMD) has been installed at vmf->pmd, we
will bail and retry the fault.  This is correct, but there is nothing
preventing the PMD from being installed after this check but before we
actually get to the DAX PTE fault handlers.

In my testing these races result in the following types of errors:

 BUG: Bad rss-counter state mm:ffff8800a817d280 idx:1 val:1
 BUG: non-zero nr_ptes on freeing mm: 15

Fix this issue by having the DAX fault handlers verify that it is safe to
continue their fault after they have taken an entry lock to block other
racing faults.

Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
Reported-by: Pawel Lebioda <pawel.lebioda@intel.com>
Cc: stable@vger.kernel.org

---

I've written a new xfstest for this race, which I will send in response to
this patch series.  This series has also survived an xfstest run without
any new issues.

---
 fs/dax.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/fs/dax.c b/fs/dax.c
index c22eaf1..3cc02d1 100644
--- a/fs/dax.c
+++ b/fs/dax.c
@@ -1155,6 +1155,15 @@ static int dax_iomap_pte_fault(struct vm_fault *vmf,
 	}
 
 	/*
+	 * It is possible, particularly with mixed reads & writes to private
+	 * mappings, that we have raced with a PMD fault that overlaps with
+	 * the PTE we need to set up.  Now that we have a locked mapping entry
+	 * we can safely unmap the huge PMD so that we can install our PTE in
+	 * our page tables.
+	 */
+	split_huge_pmd(vmf->vma, vmf->pmd, vmf->address);
+
+	/*
 	 * Note that we don't bother to use iomap_apply here: DAX required
 	 * the file system block size to be equal the page size, which means
 	 * that we never have to deal with more than a single extent here.
@@ -1398,6 +1407,15 @@ static int dax_iomap_pmd_fault(struct vm_fault *vmf,
 		goto fallback;
 
 	/*
+	 * It is possible, particularly with mixed reads & writes to private
+	 * mappings, that we have raced with a PTE fault that overlaps with
+	 * the PMD we need to set up.  If so we just fall back to a PTE fault
+	 * ourselves.
+	 */
+	if (!pmd_none(*vmf->pmd))
+		goto unlock_entry;
+
+	/*
 	 * Note that we don't use iomap_apply here.  We aren't doing I/O, only
 	 * setting up a mapping, so really we're using iomap_begin() as a way
 	 * to look up our filesystem block.
-- 
2.9.4

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

^ permalink raw reply related

* [PATCH 1/2] mm: avoid spurious 'bad pmd' warning messages
From: Ross Zwisler @ 2017-05-17 17:16 UTC (permalink / raw)
  To: Andrew Morton, linux-kernel
  Cc: Ross Zwisler, Darrick J. Wong, Alexander Viro, Christoph Hellwig,
	Dan Williams, Dave Hansen, Jan Kara, Matthew Wilcox,
	linux-fsdevel, linux-mm, linux-nvdimm, Kirill A . Shutemov,
	Pawel Lebioda, Dave Jiang, Xiong Zhou, Eryu Guan, stable

When the pmd_devmap() checks were added by:

commit 5c7fb56e5e3f ("mm, dax: dax-pmd vs thp-pmd vs hugetlbfs-pmd")

to add better support for DAX huge pages, they were all added to the end of
if() statements after existing pmd_trans_huge() checks.  So, things like:

-       if (pmd_trans_huge(*pmd))
+       if (pmd_trans_huge(*pmd) || pmd_devmap(*pmd))

When further checks were added after pmd_trans_unstable() checks by:

commit 7267ec008b5c ("mm: postpone page table allocation until we have page
to map")

they were also added at the end of the conditional:

+       if (pmd_trans_unstable(fe->pmd) || pmd_devmap(*fe->pmd))

This ordering is fine for pmd_trans_huge(), but doesn't work for
pmd_trans_unstable().  This is because DAX huge pages trip the bad_pmd()
check inside of pmd_none_or_trans_huge_or_clear_bad() (called by
pmd_trans_unstable()), which prints out a warning and returns 1.  So, we do
end up doing the right thing, but only after spamming dmesg with suspicious
looking messages:

mm/pgtable-generic.c:39: bad pmd ffff8808daa49b88(84000001006000a5)

Reorder these checks so that pmd_devmap() is checked first, avoiding the
error messages.

Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
Fixes: commit 7267ec008b5c ("mm: postpone page table allocation until we have page to map")
Cc: stable@vger.kernel.org
---
 mm/memory.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/mm/memory.c b/mm/memory.c
index 6ff5d72..1ee269d 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -3061,7 +3061,7 @@ static int pte_alloc_one_map(struct vm_fault *vmf)
 	 * through an atomic read in C, which is what pmd_trans_unstable()
 	 * provides.
 	 */
-	if (pmd_trans_unstable(vmf->pmd) || pmd_devmap(*vmf->pmd))
+	if (pmd_devmap(*vmf->pmd) || pmd_trans_unstable(vmf->pmd))
 		return VM_FAULT_NOPAGE;
 
 	vmf->pte = pte_offset_map_lock(vma->vm_mm, vmf->pmd, vmf->address,
@@ -3690,7 +3690,7 @@ static int handle_pte_fault(struct vm_fault *vmf)
 		vmf->pte = NULL;
 	} else {
 		/* See comment in pte_alloc_one_map() */
-		if (pmd_trans_unstable(vmf->pmd) || pmd_devmap(*vmf->pmd))
+		if (pmd_devmap(*vmf->pmd) || pmd_trans_unstable(vmf->pmd))
 			return 0;
 		/*
 		 * A regular pmd is established and it can't morph into a huge
-- 
2.9.4

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

^ permalink raw reply related

* Re: [PATCH v2 0/6] cpuset/mempolicies related fixes and cleanups
From: Michal Hocko @ 2017-05-17 16:24 UTC (permalink / raw)
  To: Vlastimil Babka
  Cc: Andrew Morton, linux-mm, linux-api, linux-kernel, cgroups,
	Li Zefan, Mel Gorman, David Rientjes, Christoph Lameter,
	Hugh Dickins, Andrea Arcangeli, Anshuman Khandual,
	Kirill A. Shutemov
In-Reply-To: <20170517081140.30654-1-vbabka@suse.cz>

On Wed 17-05-17 10:11:34, Vlastimil Babka wrote:
> Changes since RFC v1 [3]:
> 
> - Reworked patch 2 after discussion with Christoph Lameter.
> - Fix bug in patch 5 spotted by Hillf Danton.
> - Rebased to mmotm-2017-05-12-15-53
> 
> I would like to stress that this patchset aims to fix issues and cleanup the
> code *within the existing documented semantics*, i.e. patch 1 ignores mempolicy
> restrictions if the set of allowed nodes has no intersection with set of nodes
> allowed by cpuset. I believe discussing potential changes of the semantics can
> be better done once we have a baseline with no known bugs of the current
> semantics.
> 
> ===
> 
> I've recently summarized the cpuset/mempolicy issues in a LSF/MM proposal [1]
> and the discussion itself [2]. I've been trying to rewrite the handling as
> proposed, with the idea that changing semantics to make all mempolicies static
> wrt cpuset updates (and discarding the relative and default modes) can be tried
> on top, as there's a high risk of being rejected/reverted because somebody
> might still care about the removed modes.
> 
> However I haven't yet figured out how to properly:
> 
> 1) make mempolicies swappable instead of rebinding in place. I thought mbind()
> already works that way and uses refcounting to avoid use-after-free of the old
> policy by a parallel allocation, but turns out true refcounting is only done
> for shared (shmem) mempolicies, and the actual protection for mbind() comes
> from mmap_sem. Extending the refcounting means more overhead in allocator hot
> path.

But that overhead would be there only if any policy is in place, right?
Do you think it would be possible to use per cpu ref counting and thus
reduce the overhead? We use this trick in the css ref. counting in the
memcg and the overhead is not measurable. It would be certainly better
to use the same mechanism for shared and anon mappings whenever
possible.

> Also swapping whole mempolicies means that we have to allocate the new
> ones, which can fail, and reverting of the partially done work also means
> allocating (note that mbind() doesn't care and will just leave part of the
> range updated and part not updated when returning -ENOMEM...).

This is nasty but we already have to deal with half-the-way
initialization already so why cannot we use the same approach? I am
sorry if I am asking something trivial but I have already flushed all
details from memory so have to re-read it again.

> 2) make cpuset's task->mems_allowed also swappable (after converting it from
> nodemask to zonelist, which is the easy part) for mostly the same reasons.
> 
> The good news is that while trying to do the above, I've at least figured out
> how to hopefully close the remaining premature OOM's, and do a buch of cleanups
> on top, removing quite some of the code that was also supposed to prevent the
> cpuset update races, but doesn't work anymore nowadays. This should fix the
> most pressing concerns with this topic and give us a better baseline before
> either proceeding with the original proposal, or pushing a change of semantics
> that removes the problem 1) above. I'd be then fine with trying to change the
> semantic first and rewrite later.

the diffstat definitely looks promissing. I will have a look tomorrow
(unless something unexpected jums in again).

> Patchset is based on next-20170411 and has been tested with the LTP cpuset01
> stress test.
> 
> [1] https://lkml.kernel.org/r/4c44a589-5fd8-08d0-892c-e893bb525b71@suse.cz
> [2] https://lwn.net/Articles/717797/
> [3] https://marc.info/?l=linux-mm&m=149191957922828&w=2
> 
> Vlastimil Babka (6):
>   mm, page_alloc: fix more premature OOM due to race with cpuset update
>   mm, mempolicy: stop adjusting current->il_next in
>     mpol_rebind_nodemask()
>   mm, page_alloc: pass preferred nid instead of zonelist to allocator
>   mm, mempolicy: simplify rebinding mempolicies when updating cpusets
>   mm, cpuset: always use seqlock when changing task's nodemask
>   mm, mempolicy: don't check cpuset seqlock where it doesn't matter
> 
>  include/linux/gfp.h            |  11 ++-
>  include/linux/mempolicy.h      |  12 ++-
>  include/linux/sched.h          |   2 +-
>  include/uapi/linux/mempolicy.h |   8 --
>  kernel/cgroup/cpuset.c         |  33 ++------
>  mm/hugetlb.c                   |  15 ++--
>  mm/memory_hotplug.c            |   6 +-
>  mm/mempolicy.c                 | 181 ++++++++++-------------------------------
>  mm/page_alloc.c                |  61 ++++++++++----
>  9 files changed, 118 insertions(+), 211 deletions(-)
> 
> -- 
> 2.12.2

-- 
Michal Hocko
SUSE Labs

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

^ permalink raw reply

* Re: [PATCH] mm,oom: fix oom invocation issues
From: Michal Hocko @ 2017-05-17 16:14 UTC (permalink / raw)
  To: Roman Gushchin
  Cc: Tetsuo Handa, Johannes Weiner, Vladimir Davydov, kernel-team,
	linux-mm, linux-kernel
In-Reply-To: <1495034780-9520-1-git-send-email-guro@fb.com>

On Wed 17-05-17 16:26:20, Roman Gushchin wrote:
[...]
> [   25.781882] Out of memory: Kill process 492 (allocate) score 899 or sacrifice child
> [   25.783874] Killed process 492 (allocate) total-vm:2052368kB, anon-rss:1894576kB, file-rss:4kB, shmem-rss:0kB

Are there any oom_reaper messages? Could you provide the full kernel log
please?

> <cut>
> [   25.817589] allocate invoked oom-killer: gfp_mask=0x0(), nodemask=(null),  order=0, oom_score_adj=0
> [   25.818821] allocate cpuset=/ mems_allowed=0
> [   25.819259] CPU: 1 PID: 492 Comm: allocate Not tainted 4.12.0-rc1-mm1+ #181
> [   25.819847] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Ubuntu-1.8.2-1ubuntu1 04/01/2014
> [   25.820549] Call Trace:
> [   25.820733]  dump_stack+0x63/0x82
> [   25.820961]  dump_header+0x97/0x21a
> [   25.820961]  ? security_capable_noaudit+0x45/0x60
> [   25.820961]  oom_kill_process+0x219/0x3e0
> [   25.820961]  out_of_memory+0x11d/0x480

This is interesting. OOM usually happens from the page allocator path.
Hitting it from here means that somebody has returned VM_FAULT_OOM. Who
was that and is there any preceeding OOM before?

> [   25.820961]  pagefault_out_of_memory+0x68/0x80
> [   25.820961]  mm_fault_error+0x8f/0x190
> [   25.820961]  ? handle_mm_fault+0xf3/0x210
> [   25.820961]  __do_page_fault+0x4b2/0x4e0
> [   25.820961]  trace_do_page_fault+0x37/0xe0
> [   25.820961]  do_async_page_fault+0x19/0x70
> [   25.820961]  async_page_fault+0x28/0x30
> <cut>
> [   25.863078] Out of memory: Kill process 233 (firewalld) score 10 or sacrifice child
> [   25.863634] Killed process 233 (firewalld) total-vm:246076kB, anon-rss:20956kB, file-rss:0kB, shmem-rss:0kB
> 
> After some investigations I've found some issues:
> 
> 1) Prior to commit 1af8bb432695 ("mm, oom: fortify task_will_free_mem()"),
>    if a process with a pending SIGKILL was calling out_of_memory(),
>    it was always immediately selected as a victim.

Yes but this had its own issues. Mainly picking the same victim again
without making a further progress.

>    But now, after some changes, it's not always a case.
>    If a process has been reaped at the moment, MMF_SKIP_FLAG is set,
>    task_will_free_mem() will return false, and a new
>    victim selection logic will be started.

right. The point is that it doesn't make any sense to consider such a
task because it either cannot be reaped or it has been reaped and there
is not much left to consider. It would be interesting to see what
happened in your case.

>    This actually happens if a userspace pagefault causing an OOM.
>    pagefault_out_of_memory() is called in a context of a faulting
>    process after it has been selected as OOM victim (assuming, it
>    has), and killed. With some probability (there is a race with
>    oom_reaper thread) this process will be passed to the oom reaper
>    again, or an innocent victim will be selected and killed.
> 
> 2) We clear up the task->oom_reaper_list before setting
>    the MMF_OOM_SKIP flag, so there is a race.

I am not sure what you mean here. Why would a race matter?

> 
> 3) We skip the MMF_OOM_SKIP flag check in case of
>    an sysrq-triggered OOM.

yes because we we always want to pick a new victim when sysrq is
invoked.

> To address these issues, the following is proposed:
> 1) If task is already an oom victim, skip out_of_memory() call
>    from the pagefault_out_of_memory().

Hmm, this alone doesn't look all that bad. It would be better to simply
let the task die than go over the oom handling. But I am still not sure
what is going on in your case so I do not see how could this help.
 
> 2) Set the MMF_OOM_SKIP bit in wake_oom_reaper() before adding a
>    process to the oom_reaper list. If it's already set, do nothing.
>    Do not rely on tsk->oom_reaper_list value.

This is wrong. The sole purpose of MMF_OOM_SKIP is to let the oom
selection logic know that this task is not interesting anymore. Setting
it in wake_oom_reaper means it would be set _before_ the oom_reaper had
any chance to free any memory from the task. So we would

> 3) Check the MMF_OOM_SKIP even if OOM is triggered by a sysrq.

The code is a bit messy here but we do check MMF_OOM_SKIP in that case.
We just do it in oom_badness(). So this is not needed, strictly
speaking.

That being said I would like to here more about the cause of the OOM and
the full dmesg would be interesting. The proposed setting of
MMF_OOM_SKIP before the task is reaped is a nogo, though. 1) would be
acceptable I think but I would have to think about it some more.

> Signed-off-by: Roman Gushchin <guro@fb.com>
> Cc: Michal Hocko <mhocko@suse.com>
> Cc: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> Cc: Johannes Weiner <hannes@cmpxchg.org>
> Cc: Vladimir Davydov <vdavydov.dev@gmail.com>
> Cc: kernel-team@fb.com
> Cc: linux-mm@kvack.org
> Cc: linux-kernel@vger.kernel.org
> ---
>  mm/oom_kill.c | 33 +++++++++++++--------------------
>  1 file changed, 13 insertions(+), 20 deletions(-)
> 
> diff --git a/mm/oom_kill.c b/mm/oom_kill.c
> index 04c9143..c630c76 100644
> --- a/mm/oom_kill.c
> +++ b/mm/oom_kill.c
> @@ -302,10 +302,11 @@ static int oom_evaluate_task(struct task_struct *task, void *arg)
>  	 * the task has MMF_OOM_SKIP because chances that it would release
>  	 * any memory is quite low.
>  	 */
> -	if (!is_sysrq_oom(oc) && tsk_is_oom_victim(task)) {
> +	if (tsk_is_oom_victim(task)) {
>  		if (test_bit(MMF_OOM_SKIP, &task->signal->oom_mm->flags))
>  			goto next;
> -		goto abort;
> +		if (!is_sysrq_oom(oc))
> +			goto abort;
>  	}
>  
>  	/*
> @@ -559,22 +560,11 @@ static void oom_reap_task(struct task_struct *tsk)
>  	while (attempts++ < MAX_OOM_REAP_RETRIES && !__oom_reap_task_mm(tsk, mm))
>  		schedule_timeout_idle(HZ/10);
>  
> -	if (attempts <= MAX_OOM_REAP_RETRIES)
> -		goto done;
> -
> -
> -	pr_info("oom_reaper: unable to reap pid:%d (%s)\n",
> -		task_pid_nr(tsk), tsk->comm);
> -	debug_show_all_locks();
> -
> -done:
> -	tsk->oom_reaper_list = NULL;
> -
> -	/*
> -	 * Hide this mm from OOM killer because it has been either reaped or
> -	 * somebody can't call up_write(mmap_sem).
> -	 */
> -	set_bit(MMF_OOM_SKIP, &mm->flags);
> +	if (attempts > MAX_OOM_REAP_RETRIES) {
> +		pr_info("oom_reaper: unable to reap pid:%d (%s)\n",
> +			task_pid_nr(tsk), tsk->comm);
> +		debug_show_all_locks();
> +	}
>  
>  	/* Drop a reference taken by wake_oom_reaper */
>  	put_task_struct(tsk);
> @@ -590,6 +580,7 @@ static int oom_reaper(void *unused)
>  		if (oom_reaper_list != NULL) {
>  			tsk = oom_reaper_list;
>  			oom_reaper_list = tsk->oom_reaper_list;
> +			tsk->oom_reaper_list = NULL;
>  		}
>  		spin_unlock(&oom_reaper_lock);
>  
> @@ -605,8 +596,7 @@ static void wake_oom_reaper(struct task_struct *tsk)
>  	if (!oom_reaper_th)
>  		return;
>  
> -	/* tsk is already queued? */
> -	if (tsk == oom_reaper_list || tsk->oom_reaper_list)
> +	if (test_and_set_bit(MMF_OOM_SKIP, &tsk->signal->oom_mm->flags))
>  		return;
>  
>  	get_task_struct(tsk);
> @@ -1068,6 +1058,9 @@ void pagefault_out_of_memory(void)
>  	if (mem_cgroup_oom_synchronize(true))
>  		return;
>  
> +	if (tsk_is_oom_victim(current))
> +		return;
> +
>  	if (!mutex_trylock(&oom_lock))
>  		return;
>  	out_of_memory(&oc);
> -- 
> 2.7.4
> 
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

-- 
Michal Hocko
SUSE Labs

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

^ permalink raw reply

* Re: [PATCH] mm: per-cgroup memory reclaim stats
From: Roman Gushchin @ 2017-05-17 15:50 UTC (permalink / raw)
  To: Balbir Singh
  Cc: Johannes Weiner, Tejun Heo, Li Zefan, Michal Hocko,
	Vladimir Davydov, cgroups@vger.kernel.org,
	open list:DOCUMENTATION, linux-kernel@vger.kernel.org, linux-mm
In-Reply-To: <CAKTCnz=vj9-C2-XPcijB=fZOVVdxvqZvLEA93xXtRZmF+y3-Lg@mail.gmail.com>

On Wed, May 17, 2017 at 08:03:03AM +1000, Balbir Singh wrote:
> On Sat, May 13, 2017 at 2:42 AM, Johannes Weiner <hannes@cmpxchg.org> wrote:
> > On Fri, May 12, 2017 at 12:25:22PM +1000, Balbir Singh wrote:
> >>
> >> It sounds like memcg accumlates both global and memcg reclaim driver
> >> counts -- is this what we want?
> >
> > Yes.
> >
> > Consider a fully containerized system that is using only memory.low
> > and thus exclusively global reclaim to enforce the partitioning, NOT
> > artificial limits and limit reclaim. In this case, we still want to
> > know how much reclaim activity each group is experiencing.
> 
> But its also confusing to see memcg.stat's value being greater
> than the global value? At-least for me. For example PGSTEAL_DIRECT
> inside a memcg > global value of PGSTEAL_DIRECT. Do we make
> memcg.stat values sum of all impact on memcg or local to memcg?

Yes, I think that global counters should include both results of
global and per-cgroup reclaim. I will prepare a separate patch for this.

Thank you!

Roman

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

^ permalink raw reply

* Re: [PATCH v3 4/4] mm: Adaptive hash table scaling
From: Pasha Tatashin @ 2017-05-17 15:51 UTC (permalink / raw)
  To: Andrew Morton, Michal Hocko; +Cc: linux-mm, sparclinux, linux-fsdevel, Al Viro
In-Reply-To: <20170303153247.f16a31c95404c02a8f3e2c5f@linux-foundation.org>



On 03/03/2017 06:32 PM, Andrew Morton wrote:
> On Thu,  2 Mar 2017 00:33:45 -0500 Pavel Tatashin <pasha.tatashin@oracle.com> wrote:
> 
>> Allow hash tables to scale with memory but at slower pace, when HASH_ADAPT
>> is provided every time memory quadruples the sizes of hash tables will only
>> double instead of quadrupling as well. This algorithm starts working only
>> when memory size reaches a certain point, currently set to 64G.
>>
>> This is example of dentry hash table size, before and after four various
>> memory configurations:
>>
>> MEMORY	   SCALE	 HASH_SIZE
>> 	old	new	old	new
>>      8G	 13	 13      8M      8M
>>     16G	 13	 13     16M     16M
>>     32G	 13	 13     32M     32M
>>     64G	 13	 13     64M     64M
>>    128G	 13	 14    128M     64M
>>    256G	 13	 14    256M    128M
>>    512G	 13	 15    512M    128M
>>   1024G	 13	 15   1024M    256M
>>   2048G	 13	 16   2048M    256M
>>   4096G	 13	 16   4096M    512M
>>   8192G	 13	 17   8192M    512M
>> 16384G	 13	 17  16384M   1024M
>> 32768G	 13	 18  32768M   1024M
>> 65536G	 13	 18  65536M   2048M
> 
> OK, but what are the runtime effects?  Presumably some workloads will
> slow down a bit.  How much? How do we know that this is a worthwhile
> tradeoff?
> 
> If the effect of this change is "undetectable" then those hash tables
> are simply too large, and additional tuning is needed, yes?
> 
Hi Andrew,

The effect of this change on runtime is undetectable as filesystem 
growth is not proportional to machine memory size as what is currently 
assumed. The change effects only large memory machine. Additional tuning 
might be needed, but that can be done by the clients of the 
kmem_cache_create interface, not the generic cache allocator itself.

Thank you,
Pasha

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

^ permalink raw reply

* Re: [PATCH] Correct spelling and grammar for notification text
From: Randy Dunlap @ 2017-05-17 15:35 UTC (permalink / raw)
  To: Michael DeGuzis, linux-mm; +Cc: trivial
In-Reply-To: <20170517133842.5733-1-mdeguzis@gmail.com>

On 05/17/17 06:38, Michael DeGuzis wrote:
> From: professorkaos64 <mdeguzis@gmail.com>
> 
> This patch fixes up some grammar and spelling in the information
> block for huge_memory.c.

Missing Signed-off-by: <real name and email address>

> ---
>  mm/huge_memory.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index a84909cf20d3..af137fc0ca09 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -38,12 +38,12 @@
>  #include "internal.h"
>  
>  /*
> - * By default transparent hugepage support is disabled in order that avoid
> - * to risk increase the memory footprint of applications without a guaranteed
> - * benefit. When transparent hugepage support is enabled, is for all mappings,
> - * and khugepaged scans all mappings.
> + * By default, transparent hugepage support is disabled in order to avoid
> + * risking an increased memory footprint for applications that are not 
> + * guaranteed to benefit from it. When transparent hugepage support is 
> + * enabled, it is for all mappings, and khugepaged scans all mappings.
>   * Defrag is invoked by khugepaged hugepage allocations and by page faults
> - * for all hugepage allocations.
> + * for all hugepage allocations. 

Several of the new (+) patch lines end with a space character. Not good.

>   */
>  unsigned long transparent_hugepage_flags __read_mostly =
>  #ifdef CONFIG_TRANSPARENT_HUGEPAGE_ALWAYS
> 


-- 
~Randy

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

^ permalink raw reply

* Re: [RFC 1/6] mm, page_alloc: fix more premature OOM due to race with cpuset update
From: Christoph Lameter @ 2017-05-17 15:27 UTC (permalink / raw)
  To: Michal Hocko
  Cc: Vlastimil Babka, linux-mm, linux-kernel, cgroups, Li Zefan,
	Mel Gorman, David Rientjes, Hugh Dickins, Andrea Arcangeli,
	Anshuman Khandual, Kirill A. Shutemov, linux-api
In-Reply-To: <20170517145645.GO18247@dhcp22.suse.cz>

On Wed, 17 May 2017, Michal Hocko wrote:

> > The race is where? If you expand the node set during the move of the
> > application then you are safe in terms of the legacy apps that did not
> > include static bindings.
>
> I am pretty sure it is describe in those changelogs and I won't repeat
> it here.

I cannot figure out what you are referring to. There are numerous
patches and discussions about OOM scenarios in this context.

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

^ permalink raw reply

* [PATCH] mm,oom: fix oom invocation issues
From: Roman Gushchin @ 2017-05-17 15:26 UTC (permalink / raw)
  To: Michal Hocko
  Cc: Roman Gushchin, Tetsuo Handa, Johannes Weiner, Vladimir Davydov,
	kernel-team, linux-mm, linux-kernel

During the debugging of some OOM-related stuff, I've noticed
that sometimes OOM kills two processes instead of one.

The problem can be easily reproduced on a vanilla kernel
(allocate is a simple process which calls malloc() and faults
each page in a infinite loop):
[   25.721494] allocate invoked oom-killer: gfp_mask=0x14280ca(GFP_HIGHUSER_MOVABLE|__GFP_ZERO), nodemask=(null),  order=0, oom_score_adj=0
[   25.725658] allocate cpuset=/ mems_allowed=0
[   25.727033] CPU: 1 PID: 492 Comm: allocate Not tainted 4.12.0-rc1-mm1+ #181
<cut>
[   25.768293] [ pid ]   uid  tgid total_vm      rss nr_ptes nr_pmds swapents oom_score_adj name
[   25.768860] [  121]     0   121    25672      133      50       3        0             0 systemd-journal
[   25.769530] [  156]     0   156    11157      197      22       3        0         -1000 systemd-udevd
[   25.770206] [  206]     0   206    13896       99      29       3        0         -1000 auditd
[   25.770822] [  227]     0   227    11874      124      27       3        0             0 systemd-logind
[   25.771494] [  229]    81   229    11577      146      28       3        0          -900 dbus-daemon
[   25.772126] [  231]   997   231    27502      102      25       3        0             0 chronyd
[   25.772731] [  233]     0   233    61519     5239      85       3        0             0 firewalld
[   25.773345] [  238]     0   238   123495      529      74       4        0             0 NetworkManager
[   25.773988] [  265]     0   265    25117      231      52       3        0         -1000 sshd
[   25.774569] [  271]     0   271     6092      154      17       3        0             0 crond
[   25.775137] [  277]     0   277    11297       93      26       3        0             0 systemd-hostnam
[   25.775766] [  284]     0   284     1716       29       9       3        0             0 agetty
[   25.776342] [  285]     0   285     2030       34       9       4        0             0 agetty
[   25.776919] [  302]   998   302   133102     2578      58       3        0             0 polkitd
[   25.777505] [  394]     0   394    21785     3076      45       3        0             0 dhclient
[   25.778092] [  444]     0   444    36717      312      74       3        0             0 sshd
[   25.778744] [  446]     0   446    15966      223      36       3        0             0 systemd
[   25.779304] [  447]     0   447    23459      384      47       3        0             0 (sd-pam)
[   25.779877] [  451]     0   451    36717      316      72       3        0             0 sshd
[   25.780450] [  452]     0   452     3611      315      11       3        0             0 bash
[   25.781107] [  492]     0   492   513092   473645     934       5        0             0 allocate
[   25.781882] Out of memory: Kill process 492 (allocate) score 899 or sacrifice child
[   25.783874] Killed process 492 (allocate) total-vm:2052368kB, anon-rss:1894576kB, file-rss:4kB, shmem-rss:0kB
<cut>
[   25.817589] allocate invoked oom-killer: gfp_mask=0x0(), nodemask=(null),  order=0, oom_score_adj=0
[   25.818821] allocate cpuset=/ mems_allowed=0
[   25.819259] CPU: 1 PID: 492 Comm: allocate Not tainted 4.12.0-rc1-mm1+ #181
[   25.819847] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Ubuntu-1.8.2-1ubuntu1 04/01/2014
[   25.820549] Call Trace:
[   25.820733]  dump_stack+0x63/0x82
[   25.820961]  dump_header+0x97/0x21a
[   25.820961]  ? security_capable_noaudit+0x45/0x60
[   25.820961]  oom_kill_process+0x219/0x3e0
[   25.820961]  out_of_memory+0x11d/0x480
[   25.820961]  pagefault_out_of_memory+0x68/0x80
[   25.820961]  mm_fault_error+0x8f/0x190
[   25.820961]  ? handle_mm_fault+0xf3/0x210
[   25.820961]  __do_page_fault+0x4b2/0x4e0
[   25.820961]  trace_do_page_fault+0x37/0xe0
[   25.820961]  do_async_page_fault+0x19/0x70
[   25.820961]  async_page_fault+0x28/0x30
<cut>
[   25.863078] Out of memory: Kill process 233 (firewalld) score 10 or sacrifice child
[   25.863634] Killed process 233 (firewalld) total-vm:246076kB, anon-rss:20956kB, file-rss:0kB, shmem-rss:0kB

After some investigations I've found some issues:

1) Prior to commit 1af8bb432695 ("mm, oom: fortify task_will_free_mem()"),
   if a process with a pending SIGKILL was calling out_of_memory(),
   it was always immediately selected as a victim.
   But now, after some changes, it's not always a case.
   If a process has been reaped at the moment, MMF_SKIP_FLAG is set,
   task_will_free_mem() will return false, and a new
   victim selection logic will be started.

   This actually happens if a userspace pagefault causing an OOM.
   pagefault_out_of_memory() is called in a context of a faulting
   process after it has been selected as OOM victim (assuming, it
   has), and killed. With some probability (there is a race with
   oom_reaper thread) this process will be passed to the oom reaper
   again, or an innocent victim will be selected and killed.

2) We clear up the task->oom_reaper_list before setting
   the MMF_OOM_SKIP flag, so there is a race.

3) We skip the MMF_OOM_SKIP flag check in case of
   an sysrq-triggered OOM.

To address these issues, the following is proposed:
1) If task is already an oom victim, skip out_of_memory() call
   from the pagefault_out_of_memory().

2) Set the MMF_OOM_SKIP bit in wake_oom_reaper() before adding a
   process to the oom_reaper list. If it's already set, do nothing.
   Do not rely on tsk->oom_reaper_list value.

3) Check the MMF_OOM_SKIP even if OOM is triggered by a sysrq.

Signed-off-by: Roman Gushchin <guro@fb.com>
Cc: Michal Hocko <mhocko@suse.com>
Cc: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Cc: Johannes Weiner <hannes@cmpxchg.org>
Cc: Vladimir Davydov <vdavydov.dev@gmail.com>
Cc: kernel-team@fb.com
Cc: linux-mm@kvack.org
Cc: linux-kernel@vger.kernel.org
---
 mm/oom_kill.c | 33 +++++++++++++--------------------
 1 file changed, 13 insertions(+), 20 deletions(-)

diff --git a/mm/oom_kill.c b/mm/oom_kill.c
index 04c9143..c630c76 100644
--- a/mm/oom_kill.c
+++ b/mm/oom_kill.c
@@ -302,10 +302,11 @@ static int oom_evaluate_task(struct task_struct *task, void *arg)
 	 * the task has MMF_OOM_SKIP because chances that it would release
 	 * any memory is quite low.
 	 */
-	if (!is_sysrq_oom(oc) && tsk_is_oom_victim(task)) {
+	if (tsk_is_oom_victim(task)) {
 		if (test_bit(MMF_OOM_SKIP, &task->signal->oom_mm->flags))
 			goto next;
-		goto abort;
+		if (!is_sysrq_oom(oc))
+			goto abort;
 	}
 
 	/*
@@ -559,22 +560,11 @@ static void oom_reap_task(struct task_struct *tsk)
 	while (attempts++ < MAX_OOM_REAP_RETRIES && !__oom_reap_task_mm(tsk, mm))
 		schedule_timeout_idle(HZ/10);
 
-	if (attempts <= MAX_OOM_REAP_RETRIES)
-		goto done;
-
-
-	pr_info("oom_reaper: unable to reap pid:%d (%s)\n",
-		task_pid_nr(tsk), tsk->comm);
-	debug_show_all_locks();
-
-done:
-	tsk->oom_reaper_list = NULL;
-
-	/*
-	 * Hide this mm from OOM killer because it has been either reaped or
-	 * somebody can't call up_write(mmap_sem).
-	 */
-	set_bit(MMF_OOM_SKIP, &mm->flags);
+	if (attempts > MAX_OOM_REAP_RETRIES) {
+		pr_info("oom_reaper: unable to reap pid:%d (%s)\n",
+			task_pid_nr(tsk), tsk->comm);
+		debug_show_all_locks();
+	}
 
 	/* Drop a reference taken by wake_oom_reaper */
 	put_task_struct(tsk);
@@ -590,6 +580,7 @@ static int oom_reaper(void *unused)
 		if (oom_reaper_list != NULL) {
 			tsk = oom_reaper_list;
 			oom_reaper_list = tsk->oom_reaper_list;
+			tsk->oom_reaper_list = NULL;
 		}
 		spin_unlock(&oom_reaper_lock);
 
@@ -605,8 +596,7 @@ static void wake_oom_reaper(struct task_struct *tsk)
 	if (!oom_reaper_th)
 		return;
 
-	/* tsk is already queued? */
-	if (tsk == oom_reaper_list || tsk->oom_reaper_list)
+	if (test_and_set_bit(MMF_OOM_SKIP, &tsk->signal->oom_mm->flags))
 		return;
 
 	get_task_struct(tsk);
@@ -1068,6 +1058,9 @@ void pagefault_out_of_memory(void)
 	if (mem_cgroup_oom_synchronize(true))
 		return;
 
+	if (tsk_is_oom_victim(current))
+		return;
+
 	if (!mutex_trylock(&oom_lock))
 		return;
 	out_of_memory(&oc);
-- 
2.7.4

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

^ permalink raw reply related

* Re: [RFC 1/6] mm, page_alloc: fix more premature OOM due to race with cpuset update
From: Christoph Lameter @ 2017-05-17 15:25 UTC (permalink / raw)
  To: Michal Hocko
  Cc: Vlastimil Babka, linux-mm, linux-kernel, cgroups, Li Zefan,
	Mel Gorman, David Rientjes, Hugh Dickins, Andrea Arcangeli,
	Anshuman Khandual, Kirill A. Shutemov, linux-api
In-Reply-To: <20170517145645.GO18247@dhcp22.suse.cz>

On Wed, 17 May 2017, Michal Hocko wrote:

> > If you have screwy things like static mbinds in there then you are
> > hopelessly lost anyways. You may have moved the process to another set
> > of nodes but the static bindings may refer to a node no longer
> > available. Thus the OOM is legitimate.
>
> The point is that you do _not_ want such a process to trigger the OOM
> because it can cause other processes being killed.

Nope. The OOM in a cpuset gets the process doing the alloc killed. Or what
that changed?

At this point you have messed up royally and nothing is going to rescue
you anyways. OOM or not does not matter anymore. The app will fail.

> > At least a user space app could inspect
> > the situation and come up with custom ways of dealing with the mess.
>
> I do not really see how would this help to prevent a malicious user from
> playing tricks.

How did a malicious user come into this? Of course you can mess up in
significant ways if you can overflow nodes and cause an app that has
restrictions to fail but nothing is going to change that.

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

^ permalink raw reply

* Re: [PATCH v2 3/6] mm, page_alloc: pass preferred nid instead of zonelist to allocator
From: Christoph Lameter @ 2017-05-17 15:19 UTC (permalink / raw)
  To: Vlastimil Babka
  Cc: Andrew Morton, linux-mm, linux-api, linux-kernel, cgroups,
	Li Zefan, Michal Hocko, Mel Gorman, David Rientjes, Hugh Dickins,
	Andrea Arcangeli, Anshuman Khandual, Kirill A. Shutemov,
	Dimitri Sivanich
In-Reply-To: <20170517081140.30654-4-vbabka@suse.cz>

On Wed, 17 May 2017, Vlastimil Babka wrote:

>  struct page *
> -__alloc_pages_nodemask(gfp_t gfp_mask, unsigned int order,
> -		       struct zonelist *zonelist, nodemask_t *nodemask);
> +__alloc_pages_nodemask(gfp_t gfp_mask, unsigned int order, int preferred_nid,
> +							nodemask_t *nodemask);
>
>  static inline struct page *
> -__alloc_pages(gfp_t gfp_mask, unsigned int order,
> -		struct zonelist *zonelist)
> +__alloc_pages(gfp_t gfp_mask, unsigned int order, int preferred_nid)
>  {
> -	return __alloc_pages_nodemask(gfp_mask, order, zonelist, NULL);
> +	return __alloc_pages_nodemask(gfp_mask, order, preferred_nid, NULL);
>  }

Maybe use nid instead of preferred_nid like in __alloc_pages? Otherwise
there may be confusion with the MPOL_PREFER policy.

> @@ -1963,8 +1960,8 @@ alloc_pages_vma(gfp_t gfp, int order, struct vm_area_struct *vma,
>  {
>  	struct mempolicy *pol;
>  	struct page *page;
> +	int preferred_nid;
>  	unsigned int cpuset_mems_cookie;
> -	struct zonelist *zl;
>  	nodemask_t *nmask;

Same here.

> @@ -4012,8 +4012,8 @@ static inline void finalise_ac(gfp_t gfp_mask,
>   * This is the 'heart' of the zoned buddy allocator.
>   */
>  struct page *
> -__alloc_pages_nodemask(gfp_t gfp_mask, unsigned int order,
> -			struct zonelist *zonelist, nodemask_t *nodemask)
> +__alloc_pages_nodemask(gfp_t gfp_mask, unsigned int order, int preferred_nid,
> +							nodemask_t *nodemask)
>  {

and here

This looks clean to me. Still feel a bit uneasy about this since I do
remember that we had a reason to use zonelists instead of nodes back then
but cannot remember what that reason was....

CCing Dimitri at SGI. This may break a lot of legacy SGIapps. If you read
this Dimitri then please review this patchset and the discussions around
it.

Reviewed-by: Christoph Lameter <cl@linux.com>

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

^ permalink raw reply

* Re: [PATCH v2 2/6] mm, mempolicy: stop adjusting current->il_next in mpol_rebind_nodemask()
From: Christoph Lameter @ 2017-05-17 15:07 UTC (permalink / raw)
  To: Vlastimil Babka
  Cc: Andrew Morton, linux-mm, linux-api, linux-kernel, cgroups,
	Li Zefan, Michal Hocko, Mel Gorman, David Rientjes, Hugh Dickins,
	Andrea Arcangeli, Anshuman Khandual, Kirill A. Shutemov
In-Reply-To: <20170517081140.30654-3-vbabka@suse.cz>

On Wed, 17 May 2017, Vlastimil Babka wrote:

> The task->il_next variable stores the next allocation node id for task's
> MPOL_INTERLEAVE policy. mpol_rebind_nodemask() updates interleave and
> bind mempolicies due to changing cpuset mems. Currently it also tries to
> make sure that current->il_next is valid within the updated nodemask. This is
> bogus, because 1) we are updating potentially any task's mempolicy, not just
> current, and 2) we might be updating a per-vma mempolicy, not task one.

Reviewed-by: Christoph Lameter <cl@linux.com>

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

^ permalink raw reply

* Re: [PATCH 0/6] refine and rename slub sysfs
From: Christoph Lameter @ 2017-05-17 14:57 UTC (permalink / raw)
  To: Wei Yang; +Cc: penberg, rientjes, akpm, linux-mm, linux-kernel
In-Reply-To: <20170517141146.11063-1-richard.weiyang@gmail.com>

On Wed, 17 May 2017, Wei Yang wrote:

> This patch serial could be divided into two parts.
>
> First three patches refine and adds slab sysfs.
> Second three patches rename slab sysfs.

These changes will break the slabinfo tool in linux/tools/vm/slabinfo.c.
Please update it as well.

> 1. Refine slab sysfs
>
> There are four level slabs:

levels? Maybe types of slabs?

>     CPU
>     CPU_PARTIAL
>     PARTIAL
>     FULL
>
> And in sysfs, it use show_slab_objects() and cpu_partial_slabs_show() to
> reflect the statistics.
>
> In patch 2, it splits some function in show_slab_objects() which makes sure
> only cpu_partial_slabs_show() covers statistics for CPU_PARTIAL slabs.
>
> After doing so, it would be more clear that show_slab_objects() has totally 9
> statistic combinations for three level of slabs. Each slab has three cases
> statistic.
>
>     slabs
>     objects
>     total_objects

That sounds good.

> which is a little bit hard for users to understand. The second three patches
> rename sysfs file in this pattern.
>
>     xxx_slabs[[_total]_objects]
>
> Finally it looks Like
>
>     slabs
>     slabs_objects
>     slabs_total_objects
>     cpu_slabs
>     cpu_slabs_objects
>     cpu_slabs_total_objects
>     partial_slabs
>     partial_slabs_objects
>     partial_slabs_total_objects
>     cpu_partial_slabs

Arent we missing:

cpu_partial_slabs_objects
cpu_partial_slabs_total_objects

And the partial slabs exclude the cpu slabs as well as the cpu_partial
slabs?

Could you add some documentation as well to explain the exact semantics?

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

^ permalink raw reply

* Re: [RFC 1/6] mm, page_alloc: fix more premature OOM due to race with cpuset update
From: Michal Hocko @ 2017-05-17 14:56 UTC (permalink / raw)
  To: Christoph Lameter
  Cc: Vlastimil Babka, linux-mm, linux-kernel, cgroups, Li Zefan,
	Mel Gorman, David Rientjes, Hugh Dickins, Andrea Arcangeli,
	Anshuman Khandual, Kirill A. Shutemov, linux-api
In-Reply-To: <alpine.DEB.2.20.1705170943090.8714@east.gentwo.org>

On Wed 17-05-17 09:48:25, Cristopher Lameter wrote:
> On Wed, 17 May 2017, Michal Hocko wrote:
> 
> > > > So how are you going to distinguish VM_FAULT_OOM from an empty mempolicy
> > > > case in a raceless way?
> > >
> > > You dont have to do that if you do not create an empty mempolicy in the
> > > first place. The current kernel code avoids that by first allowing access
> > > to the new set of nodes and removing the old ones from the set when done.
> >
> > which is racy and as Vlastimil pointed out. If we simply fail such an
> > allocation the failure will go up the call chain until we hit the OOM
> > killer due to VM_FAULT_OOM. How would you want to handle that?
> 
> The race is where? If you expand the node set during the move of the
> application then you are safe in terms of the legacy apps that did not
> include static bindings.

I am pretty sure it is describe in those changelogs and I won't repeat
it here.

> If you have screwy things like static mbinds in there then you are
> hopelessly lost anyways. You may have moved the process to another set
> of nodes but the static bindings may refer to a node no longer
> available. Thus the OOM is legitimate.

The point is that you do _not_ want such a process to trigger the OOM
because it can cause other processes being killed.

> At least a user space app could inspect
> the situation and come up with custom ways of dealing with the mess.

I do not really see how would this help to prevent a malicious user from
playing tricks.

-- 
Michal Hocko
SUSE Labs

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

^ permalink raw reply

* Re: [PATCH 1/6] mm/slub: add total_objects_partial sysfs
From: Christoph Lameter @ 2017-05-17 14:50 UTC (permalink / raw)
  To: Wei Yang; +Cc: penberg, rientjes, akpm, linux-mm, linux-kernel
In-Reply-To: <20170517141146.11063-2-richard.weiyang@gmail.com>

On Wed, 17 May 2017, Wei Yang wrote:

> For partial slabs, show_slab_objects could display its total objects.
>
> This patch just adds an entry to display it.

Acked-by: Christoph Lameter <cl@linux.com>

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

^ permalink raw reply

* Re: [RFC 1/6] mm, page_alloc: fix more premature OOM due to race with cpuset update
From: Christoph Lameter @ 2017-05-17 14:48 UTC (permalink / raw)
  To: Michal Hocko
  Cc: Vlastimil Babka, linux-mm, linux-kernel, cgroups, Li Zefan,
	Mel Gorman, David Rientjes, Hugh Dickins, Andrea Arcangeli,
	Anshuman Khandual, Kirill A. Shutemov, linux-api
In-Reply-To: <20170517140501.GM18247@dhcp22.suse.cz>

On Wed, 17 May 2017, Michal Hocko wrote:

> > > So how are you going to distinguish VM_FAULT_OOM from an empty mempolicy
> > > case in a raceless way?
> >
> > You dont have to do that if you do not create an empty mempolicy in the
> > first place. The current kernel code avoids that by first allowing access
> > to the new set of nodes and removing the old ones from the set when done.
>
> which is racy and as Vlastimil pointed out. If we simply fail such an
> allocation the failure will go up the call chain until we hit the OOM
> killer due to VM_FAULT_OOM. How would you want to handle that?

The race is where? If you expand the node set during the move of the
application then you are safe in terms of the legacy apps that did not
include static bindings.

If you have screwy things like static mbinds in there then you are
hopelessly lost anyways. You may have moved the process to another set
of nodes but the static bindings may refer to a node no longer
available. Thus the OOM is legitimate.

At least a user space app could inspect
the situation and come up with custom ways of dealing with the mess.

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

^ permalink raw reply

* Re: [Qustion] vmalloc area overlap with another allocated vmalloc area
From: zhong jiang @ 2017-05-17 14:23 UTC (permalink / raw)
  To: Michal Hocko
  Cc: Vlastimil Babka, Joonsoo Kim, Linux Memory Management List, LKML
In-Reply-To: <20170517134412.GL18247@dhcp22.suse.cz>

On 2017/5/17 21:44, Michal Hocko wrote:
> On Wed 17-05-17 20:53:57, zhong jiang wrote:
>> +to linux-mm maintainer for any suggestions
>>
>> Thanks
>> zhongjiang
>> On 2017/5/16 13:03, zhong jiang wrote:
>>> Hi
>>>
>>> I  hit the following issue by runing /proc/vmallocinfo.  The kernel is 4.1 stable and
>>> 32 bit to be used.  after I expand the vamlloc area,  the issue is not occur again.
>>> it is related to the overflow. but I do not see any problem so far.
> Is this a clean 4.1 stable kernel without any additional patches on top?
> Are you able to reproduce this? How? Does the same problem happen with
> the current Linus tree?
  It is hard to reproduce.  just for special case and only once. we can not structure the case.
  I do not test it in Linus tree so far.  because I know it is hard to reprodeuce.

  Just by reading the code, I do not find the same issue. so I have no idea.

 Thanks
 zhongjiang 
>>> cat /proc/vmallocinfo
>>> 0xf1580000-0xf1600000  524288 raw_dump_mem_write+0x10c/0x188 phys=8b901000 ioremap
>>> 0xf1638000-0xf163a000    8192 mcss_pou_queue_init+0xa0/0x13c [mcss] phys=fc614000 ioremap
>>> 0xf528e000-0xf5292000   16384 n_tty_open+0x10/0xd0 pages=3 vmalloc
>>> 0xf5000000-0xf9001000 67112960 devm_ioremap+0x38/0x70 phys=40000000 ioremap
>>> 0xfe001000-0xfe002000    4096 iotable_init+0x0/0xc phys=20001000 ioremap
>>> 0xfe200000-0xfe201000    4096 iotable_init+0x0/0xc phys=1a000000 ioremap
>>> 0xff100000-0xff101000    4096 iotable_init+0x0/0xc phys=2000a000 ioremap
>>>
>>> n_tty_open allocate the vmap area is surrounded by the devm_ioremap ioremap by above info.
>>> I do not see also the race in the condition.
>>>
>>> I have no idea to the issue.  Anyone has any suggestions will be appreicated.
>>> The related config is attatched.
>>>
>>> Thanks
>>> zhongjiang


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

^ permalink raw reply

* [PATCH 6/6] mm/slub: rename cpu_partial_slab sysfs
From: Wei Yang @ 2017-05-17 14:11 UTC (permalink / raw)
  To: cl, penberg, rientjes, akpm; +Cc: linux-mm, linux-kernel, Wei Yang
In-Reply-To: <20170517141146.11063-1-richard.weiyang@gmail.com>

Apply the sysfs pattern

    xxx_slabs

to CPU_PARTIAL slabs.

Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
---
 mm/slub.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/mm/slub.c b/mm/slub.c
index eb0eaa0239fd..93ff334b725e 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -4988,7 +4988,7 @@ static ssize_t partial_slabs_total_objects_show(struct kmem_cache *s, char *buf)
 }
 SLAB_ATTR_RO(partial_slabs_total_objects);
 
-static ssize_t slabs_cpu_partial_show(struct kmem_cache *s, char *buf)
+static ssize_t cpu_partial_slabs_show(struct kmem_cache *s, char *buf)
 {
 	int objects = 0;
 	int pages = 0;
@@ -5019,7 +5019,7 @@ static ssize_t slabs_cpu_partial_show(struct kmem_cache *s, char *buf)
 #endif
 	return len + sprintf(buf + len, "\n");
 }
-SLAB_ATTR_RO(slabs_cpu_partial);
+SLAB_ATTR_RO(cpu_partial_slabs);
 
 static ssize_t reclaim_account_show(struct kmem_cache *s, char *buf)
 {
@@ -5377,7 +5377,7 @@ static struct attribute *slab_attrs[] = {
 	&destroy_by_rcu_attr.attr,
 	&shrink_attr.attr,
 	&reserved_attr.attr,
-	&slabs_cpu_partial_attr.attr,
+	&cpu_partial_slabs_attr.attr,
 #ifdef CONFIG_SLUB_DEBUG
 	&slabs_total_objects_attr.attr,
 	&slabs_attr.attr,
-- 
2.11.0

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

^ permalink raw reply related

* [PATCH 5/6] mm/slub: rename partial_slabs sysfs
From: Wei Yang @ 2017-05-17 14:11 UTC (permalink / raw)
  To: cl, penberg, rientjes, akpm; +Cc: linux-mm, linux-kernel, Wei Yang
In-Reply-To: <20170517141146.11063-1-richard.weiyang@gmail.com>

Apply the sysfs pattern

    xxx_slabs[[_total]_objects]

to PARTIAL slabs.

Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
---
 mm/slub.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/mm/slub.c b/mm/slub.c
index 443dacbf214e..eb0eaa0239fd 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -4946,11 +4946,11 @@ static ssize_t aliases_show(struct kmem_cache *s, char *buf)
 }
 SLAB_ATTR_RO(aliases);
 
-static ssize_t partial_show(struct kmem_cache *s, char *buf)
+static ssize_t partial_slabs_show(struct kmem_cache *s, char *buf)
 {
 	return show_slab_objects(s, buf, SO_PARTIAL);
 }
-SLAB_ATTR_RO(partial);
+SLAB_ATTR_RO(partial_slabs);
 
 static ssize_t cpu_slabs_show(struct kmem_cache *s, char *buf)
 {
@@ -4976,17 +4976,17 @@ static ssize_t slabs_objects_show(struct kmem_cache *s, char *buf)
 }
 SLAB_ATTR_RO(slabs_objects);
 
-static ssize_t objects_partial_show(struct kmem_cache *s, char *buf)
+static ssize_t partial_slabs_objects_show(struct kmem_cache *s, char *buf)
 {
 	return show_slab_objects(s, buf, SO_PARTIAL|SO_OBJECTS);
 }
-SLAB_ATTR_RO(objects_partial);
+SLAB_ATTR_RO(partial_slabs_objects);
 
-static ssize_t total_objects_partial_show(struct kmem_cache *s, char *buf)
+static ssize_t partial_slabs_total_objects_show(struct kmem_cache *s, char *buf)
 {
 	return show_slab_objects(s, buf, SO_PARTIAL|SO_TOTAL);
 }
-SLAB_ATTR_RO(total_objects_partial);
+SLAB_ATTR_RO(partial_slabs_total_objects);
 
 static ssize_t slabs_cpu_partial_show(struct kmem_cache *s, char *buf)
 {
@@ -5363,9 +5363,9 @@ static struct attribute *slab_attrs[] = {
 	&min_partial_attr.attr,
 	&cpu_partial_attr.attr,
 	&slabs_objects_attr.attr,
-	&objects_partial_attr.attr,
-	&total_objects_partial_attr.attr,
-	&partial_attr.attr,
+	&partial_slabs_objects_attr.attr,
+	&partial_slabs_total_objects_attr.attr,
+	&partial_slabs_attr.attr,
 	&cpu_slabs_objects_attr.attr,
 	&cpu_slabs_total_objects_attr.attr,
 	&cpu_slabs_attr.attr,
-- 
2.11.0

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

^ permalink raw reply related

* [PATCH 4/6] mm/slub: rename ALL slabs sysfs
From: Wei Yang @ 2017-05-17 14:11 UTC (permalink / raw)
  To: cl, penberg, rientjes, akpm; +Cc: linux-mm, linux-kernel, Wei Yang
In-Reply-To: <20170517141146.11063-1-richard.weiyang@gmail.com>

Apply the sysfs pattern

    xxx_slabs[[_total]_objects]

to ALL slabs.

Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
---
 mm/slub.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/mm/slub.c b/mm/slub.c
index f2f751e6cb96..443dacbf214e 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -4970,11 +4970,11 @@ static ssize_t cpu_slabs_total_objects_show(struct kmem_cache *s, char *buf)
 }
 SLAB_ATTR_RO(cpu_slabs_total_objects);
 
-static ssize_t objects_show(struct kmem_cache *s, char *buf)
+static ssize_t slabs_objects_show(struct kmem_cache *s, char *buf)
 {
 	return show_slab_objects(s, buf, SO_ALL|SO_OBJECTS);
 }
-SLAB_ATTR_RO(objects);
+SLAB_ATTR_RO(slabs_objects);
 
 static ssize_t objects_partial_show(struct kmem_cache *s, char *buf)
 {
@@ -5069,11 +5069,11 @@ static ssize_t slabs_show(struct kmem_cache *s, char *buf)
 }
 SLAB_ATTR_RO(slabs);
 
-static ssize_t total_objects_show(struct kmem_cache *s, char *buf)
+static ssize_t slabs_total_objects_show(struct kmem_cache *s, char *buf)
 {
 	return show_slab_objects(s, buf, SO_ALL|SO_TOTAL);
 }
-SLAB_ATTR_RO(total_objects);
+SLAB_ATTR_RO(slabs_total_objects);
 
 static ssize_t sanity_checks_show(struct kmem_cache *s, char *buf)
 {
@@ -5362,7 +5362,7 @@ static struct attribute *slab_attrs[] = {
 	&order_attr.attr,
 	&min_partial_attr.attr,
 	&cpu_partial_attr.attr,
-	&objects_attr.attr,
+	&slabs_objects_attr.attr,
 	&objects_partial_attr.attr,
 	&total_objects_partial_attr.attr,
 	&partial_attr.attr,
@@ -5379,7 +5379,7 @@ static struct attribute *slab_attrs[] = {
 	&reserved_attr.attr,
 	&slabs_cpu_partial_attr.attr,
 #ifdef CONFIG_SLUB_DEBUG
-	&total_objects_attr.attr,
+	&slabs_total_objects_attr.attr,
 	&slabs_attr.attr,
 	&sanity_checks_attr.attr,
 	&trace_attr.attr,
-- 
2.11.0

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

^ permalink raw reply related

* [PATCH 3/6] mm/slub: add cpu_slabs_[total_]objects sysfs
From: Wei Yang @ 2017-05-17 14:11 UTC (permalink / raw)
  To: cl, penberg, rientjes, akpm; +Cc: linux-mm, linux-kernel, Wei Yang
In-Reply-To: <20170517141146.11063-1-richard.weiyang@gmail.com>

For cpu slabs, show_slab_objects could display statistics for objects.

This patch just adds an entry to reflect it.

Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
---
 mm/slub.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/mm/slub.c b/mm/slub.c
index c7dddf22829d..f2f751e6cb96 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -4958,6 +4958,18 @@ static ssize_t cpu_slabs_show(struct kmem_cache *s, char *buf)
 }
 SLAB_ATTR_RO(cpu_slabs);
 
+static ssize_t cpu_slabs_objects_show(struct kmem_cache *s, char *buf)
+{
+	return show_slab_objects(s, buf, SO_CPU|SO_OBJECTS);
+}
+SLAB_ATTR_RO(cpu_slabs_objects);
+
+static ssize_t cpu_slabs_total_objects_show(struct kmem_cache *s, char *buf)
+{
+	return show_slab_objects(s, buf, SO_CPU|SO_TOTAL);
+}
+SLAB_ATTR_RO(cpu_slabs_total_objects);
+
 static ssize_t objects_show(struct kmem_cache *s, char *buf)
 {
 	return show_slab_objects(s, buf, SO_ALL|SO_OBJECTS);
@@ -5354,6 +5366,8 @@ static struct attribute *slab_attrs[] = {
 	&objects_partial_attr.attr,
 	&total_objects_partial_attr.attr,
 	&partial_attr.attr,
+	&cpu_slabs_objects_attr.attr,
+	&cpu_slabs_total_objects_attr.attr,
 	&cpu_slabs_attr.attr,
 	&ctor_attr.attr,
 	&aliases_attr.attr,
-- 
2.11.0

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

^ permalink raw reply related

* [PATCH 2/6] mm/slub: not include cpu_partial data in cpu_slabs sysfs
From: Wei Yang @ 2017-05-17 14:11 UTC (permalink / raw)
  To: cl, penberg, rientjes, akpm; +Cc: linux-mm, linux-kernel, Wei Yang
In-Reply-To: <20170517141146.11063-1-richard.weiyang@gmail.com>

There are four level slabs:

    CPU
    CPU_PARTIAL
    PARTIAL
    FULL

In current implementation, cpu_slabs sysfs would give statistics including
the first two levels. While there is another sysfs entry cpu_partial_slabs
gives details on the second level slab statistics. Since each cpu has one
slab for the first level, the current cpu_slabs output is easy to be
calculated from cpu_partial_slabs.

This patch removes the cpu_partial data in cpu_slabs for more specific slab
statistics and leave room to retrieve objects and total objects on CPU
level in the future.

Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
---
 mm/slub.c | 13 -------------
 1 file changed, 13 deletions(-)

diff --git a/mm/slub.c b/mm/slub.c
index 1100d2e75870..c7dddf22829d 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -4762,19 +4762,6 @@ static ssize_t show_slab_objects(struct kmem_cache *s,
 
 			total += x;
 			nodes[node] += x;
-
-			page = slub_percpu_partial_read_once(c);
-			if (page) {
-				node = page_to_nid(page);
-				if (flags & SO_TOTAL)
-					WARN_ON_ONCE(1);
-				else if (flags & SO_OBJECTS)
-					WARN_ON_ONCE(1);
-				else
-					x = page->pages;
-				total += x;
-				nodes[node] += x;
-			}
 		}
 	}
 
-- 
2.11.0

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

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox