Linux-mm Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH v1 00/11] mm/kasan: support per-page shadow memory to reduce memory consumption
From: Joonsoo Kim @ 2017-05-19  1:53 UTC (permalink / raw)
  To: Andrey Ryabinin
  Cc: Andrew Morton, Alexander Potapenko, Dmitry Vyukov, kasan-dev,
	linux-mm, linux-kernel, Thomas Gleixner, Ingo Molnar,
	H . Peter Anvin, kernel-team
In-Reply-To: <ebcc02d9-fa2b-30b1-2260-99cdf7434487@virtuozzo.com>

On Wed, May 17, 2017 at 03:17:13PM +0300, Andrey Ryabinin wrote:
> On 05/16/2017 04:16 AM, js1304@gmail.com wrote:
> > From: Joonsoo Kim <iamjoonsoo.kim@lge.com>
> > 
> > Hello, all.
> > 
> > This is an attempt to recude memory consumption of KASAN. Please see
> > following description to get the more information.
> > 
> > 1. What is per-page shadow memory
> > 
> > This patch introduces infrastructure to support per-page shadow memory.
> > Per-page shadow memory is the same with original shadow memory except
> > the granualarity. It's one byte shows the shadow value for the page.
> > The purpose of introducing this new shadow memory is to save memory
> > consumption.
> > 
> > 2. Problem of current approach
> > 
> > Until now, KASAN needs shadow memory for all the range of the memory
> > so the amount of statically allocated memory is so large. It causes
> > the problem that KASAN cannot run on the system with hard memory
> > constraint. Even if KASAN can run, large memory consumption due to
> > KASAN changes behaviour of the workload so we cannot validate
> > the moment that we want to check.
> > 
> > 3. How does this patch fix the problem
> > 
> > This patch tries to fix the problem by reducing memory consumption for
> > the shadow memory. There are two observations.
> > 
> 
> 
> I think that the best way to deal with your problem is to increase shadow scale size.
> 
> You'll need to add tunable to gcc to control shadow size. I expect that gcc has some
> places where 8-shadow scale size is hardcoded, but it should be fixable.
> 
> The kernel also have some small amount of code written with KASAN_SHADOW_SCALE_SIZE == 8 in mind,
> which should be easy to fix.
> 
> Note that bigger shadow scale size requires bigger alignment of allocated memory and variables.
> However, according to comments in gcc/asan.c gcc already aligns stack and global variables and at
> 32-bytes boundary.
> So we could bump shadow scale up to 32 without increasing current stack consumption.
> 
> On a small machine (1Gb) 1/32 of shadow is just 32Mb which is comparable to yours 30Mb, but I expect it to be
> much faster. More importantly, this will require only small amount of simple changes in code, which will be
> a *lot* more easier to maintain.

I agree that it is also a good option to reduce memory consumption.
Nevertheless, there are two reasons that justifies this patchset.

1) With this patchset, memory consumption isn't increased in
proportional to total memory size. Please consider my 4Gb system
example on the below. With increasing shadow scale size to 32, memory
would be consumed by 128M. However, this patchset consumed 50MB. This
difference can be larger if we run KASAN with bigger machine.

2) These two optimization can be applied simulatenously. It is just an
orthogonal feature. If shadow scale size is increased to 32, memory
consumption will be decreased in case of my patchset, too.

Therefore, I think that this patchset is useful in any case.

Note that increasing shadow scale has it's own trade-off. It requires
that the size of slab object is aligned to shadow scale. It will
increase memory consumption due to slab.

Thanks.

--
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

* [mmotm:master 61/139] mm/vmstat.c:1224:16: warning: 'page' may be used uninitialized in this function
From: kbuild test robot @ 2017-05-19  0:07 UTC (permalink / raw)
  To: Michal Hocko
  Cc: kbuild-all, Johannes Weiner, Andrew Morton,
	Linux Memory Management List

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

tree:   git://git.cmpxchg.org/linux-mmotm.git master
head:   ecfddce33be51ebedf97f1e03d954c14e575afb6
commit: d9f7aec054d6fdbf8f9c4f37c269bf7fff4c04bd [61/139] mm, vmstat: skip reporting offline pages in pagetypeinfo
config: i386-defconfig (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        git checkout d9f7aec054d6fdbf8f9c4f37c269bf7fff4c04bd
        # save the attached .config to linux build tree
        make ARCH=i386 

Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings

All warnings (new ones prefixed by >>):

   mm/vmstat.c: In function 'pagetypeinfo_showblockcount_print':
>> mm/vmstat.c:1224:16: warning: 'page' may be used uninitialized in this function [-Wmaybe-uninitialized]
      struct page *page;
                   ^~~~

vim +/page +1224 mm/vmstat.c

467c996c Mel Gorman     2007-10-16  1208  
b2bd8598 David Rientjes 2017-05-03  1209  	walk_zones_in_node(m, pgdat, true, pagetypeinfo_showfree_print);
467c996c Mel Gorman     2007-10-16  1210  
467c996c Mel Gorman     2007-10-16  1211  	return 0;
467c996c Mel Gorman     2007-10-16  1212  }
467c996c Mel Gorman     2007-10-16  1213  
467c996c Mel Gorman     2007-10-16  1214  static void pagetypeinfo_showblockcount_print(struct seq_file *m,
467c996c Mel Gorman     2007-10-16  1215  					pg_data_t *pgdat, struct zone *zone)
467c996c Mel Gorman     2007-10-16  1216  {
467c996c Mel Gorman     2007-10-16  1217  	int mtype;
467c996c Mel Gorman     2007-10-16  1218  	unsigned long pfn;
467c996c Mel Gorman     2007-10-16  1219  	unsigned long start_pfn = zone->zone_start_pfn;
108bcc96 Cody P Schafer 2013-02-22  1220  	unsigned long end_pfn = zone_end_pfn(zone);
467c996c Mel Gorman     2007-10-16  1221  	unsigned long count[MIGRATE_TYPES] = { 0, };
467c996c Mel Gorman     2007-10-16  1222  
467c996c Mel Gorman     2007-10-16  1223  	for (pfn = start_pfn; pfn < end_pfn; pfn += pageblock_nr_pages) {
467c996c Mel Gorman     2007-10-16 @1224  		struct page *page;
467c996c Mel Gorman     2007-10-16  1225  
d9f7aec0 Michal Hocko   2017-05-18  1226  		if (!pfn_to_online_page(pfn))
467c996c Mel Gorman     2007-10-16  1227  			continue;
467c996c Mel Gorman     2007-10-16  1228  
eb33575c Mel Gorman     2009-05-13  1229  		/* Watch for unexpected holes punched in the memmap */
eb33575c Mel Gorman     2009-05-13  1230  		if (!memmap_valid_within(pfn, page, zone))
e80d6a24 Mel Gorman     2008-08-14  1231  			continue;
eb33575c Mel Gorman     2009-05-13  1232  

:::::: The code at line 1224 was first introduced by commit
:::::: 467c996c1e1910633fa8e7adc9b052aa3ed5f97c Print out statistics in relation to fragmentation avoidance to /proc/pagetypeinfo

:::::: TO: Mel Gorman <mel@csn.ul.ie>
:::::: CC: Linus Torvalds <torvalds@woody.linux-foundation.org>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 26204 bytes --]

^ permalink raw reply

* [mmotm:master 61/139] include/asm-generic/memory_model.h:54:52: warning: 'page' is used uninitialized in this function
From: kbuild test robot @ 2017-05-18 23:48 UTC (permalink / raw)
  To: Michal Hocko
  Cc: kbuild-all, Johannes Weiner, Andrew Morton,
	Linux Memory Management List

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

tree:   git://git.cmpxchg.org/linux-mmotm.git master
head:   ecfddce33be51ebedf97f1e03d954c14e575afb6
commit: d9f7aec054d6fdbf8f9c4f37c269bf7fff4c04bd [61/139] mm, vmstat: skip reporting offline pages in pagetypeinfo
config: x86_64-kexec (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        git checkout d9f7aec054d6fdbf8f9c4f37c269bf7fff4c04bd
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   In file included from arch/x86/include/asm/page.h:75:0,
                    from arch/x86/include/asm/thread_info.h:11,
                    from include/linux/thread_info.h:37,
                    from arch/x86/include/asm/preempt.h:6,
                    from include/linux/preempt.h:80,
                    from include/linux/spinlock.h:50,
                    from include/linux/wait.h:8,
                    from include/linux/fs.h:5,
                    from mm/vmstat.c:12:
   mm/vmstat.c: In function 'pagetypeinfo_showblockcount_print':
>> include/asm-generic/memory_model.h:54:52: warning: 'page' is used uninitialized in this function [-Wuninitialized]
    #define __page_to_pfn(page) (unsigned long)((page) - vmemmap)
                                                       ^
   mm/vmstat.c:1224:16: note: 'page' was declared here
      struct page *page;
                   ^~~~

vim +/page +54 include/asm-generic/memory_model.h

a117e66e KAMEZAWA Hiroyuki  2006-03-27  38  ({	unsigned long __pfn = (pfn);		\
c5d71243 Rafael J. Wysocki  2008-11-08  39  	unsigned long __nid = arch_pfn_to_nid(__pfn);  \
a117e66e KAMEZAWA Hiroyuki  2006-03-27  40  	NODE_DATA(__nid)->node_mem_map + arch_local_page_offset(__pfn, __nid);\
a117e66e KAMEZAWA Hiroyuki  2006-03-27  41  })
a117e66e KAMEZAWA Hiroyuki  2006-03-27  42  
67de6482 Andy Whitcroft     2006-06-23  43  #define __page_to_pfn(pg)						\
aa462abe Ian Campbell       2011-08-17  44  ({	const struct page *__pg = (pg);					\
a0140c1d KAMEZAWA Hiroyuki  2006-03-27  45  	struct pglist_data *__pgdat = NODE_DATA(page_to_nid(__pg));	\
a0140c1d KAMEZAWA Hiroyuki  2006-03-27  46  	(unsigned long)(__pg - __pgdat->node_mem_map) +			\
a0140c1d KAMEZAWA Hiroyuki  2006-03-27  47  	 __pgdat->node_start_pfn;					\
a117e66e KAMEZAWA Hiroyuki  2006-03-27  48  })
a117e66e KAMEZAWA Hiroyuki  2006-03-27  49  
8f6aac41 Christoph Lameter  2007-10-16  50  #elif defined(CONFIG_SPARSEMEM_VMEMMAP)
8f6aac41 Christoph Lameter  2007-10-16  51  
af901ca1 Andre Goddard Rosa 2009-11-14  52  /* memmap is virtually contiguous.  */
8f6aac41 Christoph Lameter  2007-10-16  53  #define __pfn_to_page(pfn)	(vmemmap + (pfn))
32272a26 Martin Schwidefsky 2008-12-25 @54  #define __page_to_pfn(page)	(unsigned long)((page) - vmemmap)
8f6aac41 Christoph Lameter  2007-10-16  55  
a117e66e KAMEZAWA Hiroyuki  2006-03-27  56  #elif defined(CONFIG_SPARSEMEM)
a117e66e KAMEZAWA Hiroyuki  2006-03-27  57  /*
1a49123b Zhang Yanfei       2013-10-03  58   * Note: section's mem_map is encoded to reflect its start_pfn.
a117e66e KAMEZAWA Hiroyuki  2006-03-27  59   * section[i].section_mem_map == mem_map's address - start_pfn;
a117e66e KAMEZAWA Hiroyuki  2006-03-27  60   */
67de6482 Andy Whitcroft     2006-06-23  61  #define __page_to_pfn(pg)					\
aa462abe Ian Campbell       2011-08-17  62  ({	const struct page *__pg = (pg);				\

:::::: The code at line 54 was first introduced by commit
:::::: 32272a26974d2027384fd4010cd1780fca425d94 [S390] __page_to_pfn warnings

:::::: TO: Martin Schwidefsky <schwidefsky@de.ibm.com>
:::::: CC: Martin Schwidefsky <schwidefsky@de.ibm.com>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 25361 bytes --]

^ permalink raw reply

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

On Thu, May 18, 2017 at 09:50:37AM +0200, Jan Kara wrote:
> On Wed 17-05-17 11:16:39, Ross Zwisler wrote:
> > 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.
> 
> So I don't see how this second scenario can happen. dax_iomap_pmd_fault()
> will call grab_mapping_entry(). That will either find PTE entry in the
> radix tree -> EEXIST and we retry the fault. Or we will not find PTE entry
> -> try to insert PMD entry which collides with the PTE entry -> EEXIST and
> we retry the fault. Am I missing something?

Yep, sorry, I guess I needed a few extra steps in my flow (the initial private
mapping read by CPU 0):


CPU 0					CPU 1

(private mapping read)
__handle_mm_fault()
  passes check for pmd_none()
  create_huge_pmd()
    dax_iomap_pmd_fault() inserts PMD

(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.

So what happens is that CPU 0 inserts a DAX PMD into the radix tree that has
real storage backing, and all PTE and PMD faults just use that same PMD radix
tree entry for locking and dirty tracking.

> The first scenario seems to be possible. dax_iomap_pmd_fault() will create
> PMD entry in the radix tree. Then dax_iomap_pte_fault() will come, do
> grab_mapping_entry(), there it sees entry is PMD but we are doing PTE fault
> so I'd think that pmd_downgrade = true... But actually the condition there
> doesn't trigger in this case. And that's a catch that although we asked
> grab_mapping_entry() for PTE, we've got PMD back and that screws us later.

Yep, it was a concious decision when implementing the PMD support to allow one
thread to use PMDs and another to use PTEs in the same range, as long as the
thread faulting in PMDs is the first to insert into the radix tree.  A PMD
radix tree entry will be inserted and used for locking and dirty tracking, and
each thread or process can fault in either PTEs or PMDs into its own address
space as needed.

We can revisit this, if you think it is incorrect.  The option you outline
below would basically mean that if any thread were to fault in a PTE in a
range, all threads and processes would be forced to use PTEs because we would
use PTEs in the radix tree.

This is cleaner...I'm not sure if the use case of having two threads accessing
the same area, one with PTEs and one with PMDs, is actually prevalent.  It's
also maybe a bit weird that the current behavior varies based on which thread
faulted first - if the PTE thread faults first, it'll insert a PTE into the
radix tree and everyone will just use PTEs.

> Actually I'm not convinced your patch quite fixes this because
> dax_load_hole() or dax_insert_mapping_entry() will modify the passed entry
> with the assumption that it's PTE entry and so they will likely corrupt the
> entry in the radix tree.

I don't think we can ever call dax_load_hole() if we have a DAX PMD entry in
the radix tree, because we have a block mapping from the filesystem.

For dax_insert_mapping_entry(), we do the right thing.  From the comments
above the function:

 * If we happen to be trying to insert a PTE and there is a PMD
 * already in the tree, we will skip the insertion and just dirty the PMD as
 * appropriate.  If we happen to be trying to insert a PTE and there is a PMD
 * already in the tree, we will skip the insertion and just dirty the PMD as
 * appropriate.

> So I think to fix the first case we should rather modify
> grab_mapping_entry() to properly go through the pmd_downgrade path once we
> find PMD entry and we do PTE fault.
> 
> What do you think?

That could also work, though I do think the fix as submitted is correct.
I think it comes down to whether we want to keep the behavior where a thread
faulting in a PTEs will use an existing PMD entry in the radix tree, instead
of making all other threads fall back to PTEs.

I think either way solves this issue for the DAX case...but do you understand
how this is solved for other fault handlers?  They don't have any isolation
between faults either in the mm/memory.c code, and are susceptible to the same
races.  How do they deal with the fact that by the time they get to their PTE
fault handler, a racing PMD fault handler in another thread could have
inserted a PMD into their page tables, and vice versa?

--
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

* mmotm 2017-05-18-14-18 uploaded
From: akpm @ 2017-05-18 21:19 UTC (permalink / raw)
  To: mm-commits, linux-kernel, linux-mm, linux-fsdevel, linux-next,
	sfr, mhocko, broonie

The mm-of-the-moment snapshot 2017-05-18-14-18 has been uploaded to

   http://www.ozlabs.org/~akpm/mmotm/

mmotm-readme.txt says

README for mm-of-the-moment:

http://www.ozlabs.org/~akpm/mmotm/

This is a snapshot of my -mm patch queue.  Uploaded at random hopefully
more than once a week.

You will need quilt to apply these patches to the latest Linus release (4.x
or 4.x-rcY).  The series file is in broken-out.tar.gz and is duplicated in
http://ozlabs.org/~akpm/mmotm/series

The file broken-out.tar.gz contains two datestamp files: .DATE and
.DATE-yyyy-mm-dd-hh-mm-ss.  Both contain the string yyyy-mm-dd-hh-mm-ss,
followed by the base kernel version against which this patch series is to
be applied.

This tree is partially included in linux-next.  To see which patches are
included in linux-next, consult the `series' file.  Only the patches
within the #NEXT_PATCHES_START/#NEXT_PATCHES_END markers are included in
linux-next.

A git tree which contains the memory management portion of this tree is
maintained at git://git.kernel.org/pub/scm/linux/kernel/git/mhocko/mm.git
by Michal Hocko.  It contains the patches which are between the
"#NEXT_PATCHES_START mm" and "#NEXT_PATCHES_END" markers, from the series
file, http://www.ozlabs.org/~akpm/mmotm/series.


A full copy of the full kernel tree with the linux-next and mmotm patches
already applied is available through git within an hour of the mmotm
release.  Individual mmotm releases are tagged.  The master branch always
points to the latest release, so it's constantly rebasing.

http://git.cmpxchg.org/cgit.cgi/linux-mmotm.git/

To develop on top of mmotm git:

  $ git remote add mmotm git://git.kernel.org/pub/scm/linux/kernel/git/mhocko/mm.git
  $ git remote update mmotm
  $ git checkout -b topic mmotm/master
  <make changes, commit>
  $ git send-email mmotm/master.. [...]

To rebase a branch with older patches to a new mmotm release:

  $ git remote update mmotm
  $ git rebase --onto mmotm/master <topic base> topic




The directory http://www.ozlabs.org/~akpm/mmots/ (mm-of-the-second)
contains daily snapshots of the -mm tree.  It is updated more frequently
than mmotm, and is untested.

A git copy of this tree is available at

	http://git.cmpxchg.org/cgit.cgi/linux-mmots.git/

and use of this tree is similar to
http://git.cmpxchg.org/cgit.cgi/linux-mmotm.git/, described above.


This mmotm tree contains the following patches against 4.12-rc1:
(patches marked "*" will be included in linux-next)

  origin.patch
  i-need-old-gcc.patch
* mm-skip-hwpoisoned-pages-when-onlining-pages.patch
* ksm-prevent-crash-after-write_protect_page-fails.patch
* maintainers-greybus-dev-list-is-members-only.patch
* include-linux-gfph-fix-___gfp_nolockdep-value.patch
* frv-declare-jiffies-to-be-located-in-the-data-section.patch
* mm-clarify-why-we-want-kmalloc-before-falling-backto-vmallock.patch
* mm-clarify-why-we-want-kmalloc-before-falling-backto-vmallock-checkpatch-fixes.patch
* arm-arch-arm-include-asm-pageh-needs-personalityh.patch
* mn10300-remove-wrapper-header-for-asm-deviceh.patch
* teach-initramfs_root_uid-and-initramfs_root_gid-that-1-means-current-user.patch
* clarify-help-text-that-compression-applies-to-ramfs-as-well-as-legacy-ramdisk.patch
* sh-intc-delete-an-error-message-for-a-failed-memory-allocation-in-add_virq_to_pirq.patch
* ocfs2-old-mle-put-and-release-after-the-function-dlm_add_migration_mle-called.patch
* ocfs2-old-mle-put-and-release-after-the-function-dlm_add_migration_mle-called-fix.patch
* ocfs2-dlm-optimization-of-code-while-free-dead-node-locks.patch
* ocfs2-dlm-optimization-of-code-while-free-dead-node-locks-checkpatch-fixes.patch
* ocfs2-give-an-obvious-tip-for-dismatch-cluster-names.patch
* block-restore-proc-partitions-to-not-display-non-partitionable-removable-devices.patch
  mm.patch
* mm-slub-remove-a-redundant-assignment-in-___slab_alloc.patch
* mm-slub-reset-cpu_slabs-pointer-in-deactivate_slab.patch
* mm-slub-pack-red_left_pad-with-another-int-to-save-a-word.patch
* mm-slub-wrap-cpu_slab-partial-in-config_slub_cpu_partial.patch
* mm-slub-wrap-cpu_slab-partial-in-config_slub_cpu_partial-fix.patch
* mm-slub-wrap-kmem_cache-cpu_partial-in-config-config_slub_cpu_partial.patch
* mm-sparsemem-break-out-of-loops-early.patch
* mark-protection_map-as-__ro_after_init.patch
* mm-vmscan-fix-unsequenced-modification-and-access-warning.patch
* mm-nobootmem-return-0-when-start_pfn-equals-end_pfn.patch
* ksm-introduce-ksm_max_page_sharing-per-page-deduplication-limit.patch
* ksm-fix-use-after-free-with-merge_across_nodes-=-0.patch
* ksm-cleanup-stable_node-chain-collapse-case.patch
* ksm-swap-the-two-output-parameters-of-chain-chain_prune.patch
* ksm-optimize-refile-of-stable_node_dup-at-the-head-of-the-chain.patch
* zram-introduce-zram_entry-to-prepare-dedup-functionality.patch
* zram-implement-deduplication-in-zram.patch
* zram-make-deduplication-feature-optional.patch
* zram-compare-all-the-entries-with-same-checksum-for-deduplication.patch
* zram-count-same-page-write-as-page_stored.patch
* zram-do-not-count-duplicated-pages-as-compressed.patch
* zram-try-harder-to-store-user-data-on-compression-error.patch
* mm-vmstat-standardize-file-operations-variable-names.patch
* mm-thp-swap-delay-splitting-thp-during-swap-out.patch
* mm-thp-swap-delay-splitting-thp-during-swap-out-fix.patch
* mm-thp-swap-unify-swap-slot-free-functions-to-put_swap_page.patch
* mm-thp-swap-move-anonymous-thp-split-logic-to-vmscan.patch
* mm-thp-swap-check-whether-thp-can-be-split-firstly.patch
* mm-thp-swap-enable-thp-swap-optimization-only-if-has-compound-map.patch
* mm-remove-return-value-from-init_currently_empty_zone.patch
* mm-memory_hotplug-use-node-instead-of-zone-in-can_online_high_movable.patch
* mm-drop-page_initialized-check-from-get_nid_for_pfn.patch
* mm-memory_hotplug-get-rid-of-is_zone_device_section.patch
* mm-memory_hotplug-split-up-register_one_node.patch
* mm-memory_hotplug-consider-offline-memblocks-removable.patch
* mm-consider-zone-which-is-not-fully-populated-to-have-holes.patch
* mm-consider-zone-which-is-not-fully-populated-to-have-holes-fix.patch
* mm-compaction-skip-over-holes-in-__reset_isolation_suitable.patch
* mm-__first_valid_page-skip-over-offline-pages.patch
* mm-vmstat-skip-reporting-offline-pages-in-pagetypeinfo.patch
* mm-memory_hotplug-do-not-associate-hotadded-memory-to-zones-until-online.patch
* mm-memory_hotplug-replace-for_device-by-want_memblock-in-arch_add_memory.patch
* mm-memory_hotplug-fix-the-section-mismatch-warning.patch
* mm-memory_hotplug-remove-unused-cruft-after-memory-hotplug-rework.patch
* exit-dont-include-unused-userfaultfd_kh.patch
* userfaultfd-drop-dead-code.patch
* mm-madvise-enable-softhard-offline-of-hugetlb-pages-at-pgd-level.patch
* mm-hugetlb-migration-use-set_huge_pte_at-instead-of-set_pte_at.patch
* mm-follow_page_mask-split-follow_page_mask-to-smaller-functions.patch
* mm-hugetlb-export-hugetlb_entry_migration-helper.patch
* mm-follow_page_mask-add-support-for-hugetlb-pgd-entries.patch
* mm-hugetlb-move-default-definition-of-hugepd_t-earlier-in-the-header.patch
* mm-follow_page_mask-add-support-for-hugepage-directory-entry.patch
* powerpc-hugetlb-add-follow_huge_pd-implementation-for-ppc64.patch
* powerpc-mm-hugetlb-remove-follow_huge_addr-for-powerpc.patch
* powerpc-hugetlb-enable-hugetlb-migration-for-ppc64.patch
* mm-zeroing-hash-tables-in-allocator.patch
* mm-updated-callers-to-use-hash_zero-flag.patch
* mm-adaptive-hash-table-scaling.patch
* mm-adaptive-hash-table-scaling-fix.patch
* mm-hugetlb-cleanup-arch_has_gigantic_page.patch
* powerpc-mm-hugetlb-add-support-for-1g-huge-pages.patch
* mm-page_alloc-mark-bad_range-and-meminit_pfn_in_nid-as-__maybe_unused.patch
* mm-drop-null-return-check-of-pte_offset_map_lock.patch
* mm-page_alloc-return-0-in-case-this-node-has-no-page-within-the-zone.patch
* mm-vmscan-do-not-pass-reclaimed-slab-to-vmpressure.patch
* mm-page_owner-align-with-pageblock_nr-pages.patch
* mm-walk-the-zone-in-pageblock_nr_pages-steps.patch
* mm-kasan-use-kasan_zero_pud-for-p4d-table.patch
* mm-kasan-rename-xxx_is_zero-to-xxx_is_nonzero.patch
* frv-remove-wrapper-header-for-asm-deviceh.patch
* kstrtox-delete-end-of-string-test.patch
* kstrtox-use-unsigned-int-more.patch
* lib-interval_tree_test-allow-the-module-to-be-compiled-in.patch
* lib-interval_tree_test-make-test-options-module-parameters.patch
* lib-interval_tree_test-allow-users-to-limit-scope-of-endpoint.patch
* lib-interval_tree_test-allow-full-tree-search.patch
* fs-epoll-short-circuit-fetching-events-if-thread-has-been-killed.patch
* seq_file-delete-small-value-optimization.patch
* virtually-mapped-stacks-do-not-disable-interrupts.patch
* kexec-move-vmcoreinfo-out-of-the-kernels-bss-section.patch
* powerpc-fadump-use-the-correct-vmcoreinfo_note_size-for-phdr.patch
* powerpc-fadump-use-the-correct-vmcoreinfo_note_size-for-phdr-fix.patch
* kdump-protect-vmcoreinfo-data-under-the-crash-memory.patch
* kexec-kdump-minor-documentation-updates-for-arm64-and-image.patch
* kdump-vmcoreinfo-report-actual-value-of-phys_base.patch
* uapi-fix-linux-sysctlh-userspace-compilation-errors.patch
* bfs-fix-sanity-checks-for-empty-files.patch
* fs-kill-config_percpu_rwsem-some-more.patch
* scripts-gdb-add-lx-fdtdump-command.patch
* procfs-fdinfo-extend-information-about-epoll-target-files.patch
* kcmp-add-kcmp_epoll_tfd-mode-to-compare-epoll-target-files.patch
* kcmp-fs-epoll-wrap-kcmp-code-with-config_checkpoint_restore.patch
* kernel-reboot-add-devm_register_reboot_notifier.patch
* kernel-reboot-add-devm_register_reboot_notifier-fix.patch
* fault-inject-support-systematic-fault-injection.patch
* fault-inject-support-systematic-fault-injection-fix.patch
* fault-inject-automatically-detect-the-number-base-for-fail-nth-write-interface.patch
* fault-inject-parse-as-natural-1-based-value-for-fail-nth-write-interface.patch
* fault-inject-make-fail-nth-read-write-interface-symmetric.patch
* fault-inject-simplify-access-check-for-fail-nth.patch
* fault-inject-simplify-access-check-for-fail-nth-fix.patch
* fault-inject-add-proc-pid-fail-nth.patch
* make-initramfs-honor-config_devtmpfs_mount.patch
* ipc-semc-remove-sem_base-embed-struct-sem.patch
* ipc-merge-ipc_rcu-and-kern_ipc_perm.patch
* include-linux-semh-correctly-document-sem_ctime.patch
  linux-next.patch
* sparc64-ng4-memset-32-bits-overflow.patch
* powerpc-sequoia-fix-nand-partitions-not-to-overlap.patch
* lib-crc-ccitt-add-ccitt-false-crc16-variant.patch
  mm-add-strictlimit-knob-v2.patch
  make-sure-nobodys-leaking-resources.patch
  releasing-resources-with-children.patch
  kernel-forkc-export-kernel_thread-to-modules.patch
  mutex-subsystem-synchro-test-module.patch
  slab-leaks3-default-y.patch
  workaround-for-a-pci-restoring-bug.patch

--
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/3] KSMscale cleanup/optimizations
From: Dan Carpenter @ 2017-05-18 20:35 UTC (permalink / raw)
  To: Andrea Arcangeli
  Cc: Andrew Morton, linux-mm, Evgheni Dereveanchin, Andrey Ryabinin,
	Petr Holasek, Hugh Dickins, Arjan van de Ven, Davidlohr Bueso,
	Gavin Guo, Jay Vosburgh, Mel Gorman
In-Reply-To: <20170518173721.22316-1-aarcange@redhat.com>

On Thu, May 18, 2017 at 07:37:18PM +0200, Andrea Arcangeli wrote:
> 2/3 should fix the false positive from Dan's static checker. Dan could
> you check if it still complains?

That works.  Thanks!

regards,
dan carpenter

--
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 PATCH] mm, oom: cgroup-aware OOM-killer
From: Johannes Weiner @ 2017-05-18 20:15 UTC (permalink / raw)
  To: Balbir Singh
  Cc: Michal Hocko, Roman Gushchin, Tejun Heo, Li Zefan,
	Vladimir Davydov, Tetsuo Handa, kernel-team,
	cgroups@vger.kernel.org, open list:DOCUMENTATION,
	linux-kernel@vger.kernel.org, linux-mm
In-Reply-To: <1495136639.21894.3.camel@gmail.com>

On Fri, May 19, 2017 at 05:43:59AM +1000, Balbir Singh wrote:
> On Thu, 2017-05-18 at 15:22 -0400, Johannes Weiner wrote:
> > On Fri, May 19, 2017 at 04:37:27AM +1000, Balbir Singh wrote:
> > > On Fri, May 19, 2017 at 3:30 AM, Michal Hocko <mhocko@kernel.org> wrote:
> > > > On Thu 18-05-17 17:28:04, Roman Gushchin wrote:
> > > > > Traditionally, the OOM killer is operating on a process level.
> > > > > Under oom conditions, it finds a process with the highest oom score
> > > > > and kills it.
> > > > > 
> > > > > This behavior doesn't suit well the system with many running
> > > > > containers. There are two main issues:
> > > > > 
> > > > > 1) There is no fairness between containers. A small container with
> > > > > a few large processes will be chosen over a large one with huge
> > > > > number of small processes.
> > > > > 
> > > > > 2) Containers often do not expect that some random process inside
> > > > > will be killed. So, in general, a much safer behavior is
> > > > > to kill the whole cgroup. Traditionally, this was implemented
> > > > > in userspace, but doing it in the kernel has some advantages,
> > > > > especially in a case of a system-wide OOM.
> > > > > 
> > > > > To address these issues, cgroup-aware OOM killer is introduced.
> > > > > Under OOM conditions, it looks for a memcg with highest oom score,
> > > > > and kills all processes inside.
> > > > > 
> > > > > Memcg oom score is calculated as a size of active and inactive
> > > > > anon LRU lists, unevictable LRU list and swap size.
> > > > > 
> > > > > For a cgroup-wide OOM, only cgroups belonging to the subtree of
> > > > > the OOMing cgroup are considered.
> > > > 
> > > > While this might make sense for some workloads/setups it is not a
> > > > generally acceptable policy IMHO. We have discussed that different OOM
> > > > policies might be interesting few years back at LSFMM but there was no
> > > > real consensus on how to do that. One possibility was to allow bpf like
> > > > mechanisms. Could you explore that path?
> > > 
> > > I agree, I think it needs more thought. I wonder if the real issue is something
> > > else. For example
> > > 
> > > 1. Did we overcommit a particular container too much?
> > > 2. Do we need something like https://lwn.net/Articles/604212/ to solve
> > > the problem?
> > 
> > The occasional OOM kill is an unavoidable reality on our systems (and
> > I bet on most deployments). If we tried not to overcommit, we'd waste
> > a *lot* of memory.
> > 
> > The problem is when OOM happens, we really want the biggest *job* to
> > get killed. Before cgroups, we assumed jobs were processes. But with
> > cgroups, the user is able to define a group of processes as a job, and
> > then an individual process is no longer a first-class memory consumer.
> > 
> > Without a patch like this, the OOM killer will compare the sizes of
> > the random subparticles that the jobs in the system are composed of
> > and kill the single biggest particle, leaving behind the incoherent
> > remains of one of the jobs. That doesn't make a whole lot of sense.
> 
> I agree, but see my response on oom_notifiers in parallel that I sent
> to Roman.

I don't see how they're related to an abstraction problem in the
victim evaluation.

> > If you want to determine the most expensive car in a parking lot, you
> > can't go off and compare the price of one car's muffler with the door
> > handle of another, then point to a wind shield and yell "This is it!"
> > 
> > You need to compare the cars as a whole with each other.
> > 
> > > 3. We have oom notifiers now, could those be used (assuming you are interested
> > > in non memcg related OOM's affecting a container
> > 
> > Right now, we watch for OOM notifications and then have userspace kill
> > the rest of a job. That works - somewhat. What remains is the problem
> > that I described above, that comparing individual process sizes is not
> > meaningful when the terminal memory consumer is a cgroup.
> 
> Could the cgroup limit be used as the comparison point? stats inside
> of the memory cgroup?

The OOM is a result of physical memory shortage, but the limits don't
tell you how much physical memory you are consuming - only how much
you might if it weren't for a lack of physical memory.

We *do* use the stats inside of the cgroup, namely the amount of
memory they consumed overall, to compare them against each other.

As far as configurable priorities comparable to the oom score on the
system level goes, that's seems like a separate discussion. We could
add memory.oom_score, we could think about subtracting memory.low from
the badness of each cgroup (as that's the portion the group is
supposed to be able to consume in peace, and which we always expect to
be available in physical memory, so we want to kill the group with the
most overage above the memory.low limit) etc.

Either way, it's always possible to add configurability as patch 2/2.
Again, this patch is first and foremost about functionality, not about
interfacing and configurability.

> > > 4. How do we determine limits for these containers? From a fariness
> > > perspective
> > 
> > How do you mean?
> 
> How do we set them up so that the larger job gets more of the limits
> as opposed to the small ones?

I'm afraid I still don't entirely understand.

Is this about comparing groups not just by their physical size, but
also by their *intended* size and the difference between the two?
Meaning that a 10G-limit group with 9G allocated could be considered a
larger consumer than a 20G-limit group with 10G worth of memory?

If yes, I think that's where the fact that you overcommit comes
in. Because clearly you don't have 30G - the sum of the memory.max
limits - to hand out, seeing that you OOMed when these groups have
only 19G combined. So the memory.max settings cannot be considered the
intended distribution of memory in the system.

But that's exactly what memory.low is for.

--
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 v5 17/32] x86/mm: Add support to access boot related data in the clear
From: Matt Fleming @ 2017-05-18 19:50 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Tom Lendacky, linux-arch, linux-efi, kvm, linux-doc, x86, kexec,
	linux-kernel, kasan-dev, linux-mm, iommu, Rik van Riel,
	Radim Krčmář, Toshimitsu Kani, Arnd Bergmann,
	Jonathan Corbet, Michael S. Tsirkin, Joerg Roedel,
	Konrad Rzeszutek Wilk, Paolo Bonzini, Larry Woodman,
	Brijesh Singh, Ingo Molnar, Andy Lutomirski, H. Peter Anvin,
	Andrey Ryabinin, Alexander Potapenko, Dave Young, Thomas Gleixner,
	Dmitry Vyukov, Ard Biesheuvel
In-Reply-To: <20170515183517.mb4k2gp2qobbuvtm@pd.tnic>

On Mon, 15 May, at 08:35:17PM, Borislav Petkov wrote:
> On Tue, Apr 18, 2017 at 04:19:21PM -0500, Tom Lendacky wrote:
>
> > +		paddr = boot_params.efi_info.efi_memmap_hi;
> > +		paddr <<= 32;
> > +		paddr |= boot_params.efi_info.efi_memmap;
> > +		if (phys_addr == paddr)
> > +			return true;
> > +
> > +		paddr = boot_params.efi_info.efi_systab_hi;
> > +		paddr <<= 32;
> > +		paddr |= boot_params.efi_info.efi_systab;
> 
> So those two above look like could be two global vars which are
> initialized somewhere in the EFI init path:
> 
> efi_memmap_phys and efi_systab_phys or so.
> 
> Matt ?
> 
> And then you won't need to create that paddr each time on the fly. I
> mean, it's not a lot of instructions but still...
 
We should already have the physical memmap address available in
'efi.memmap.phys_map'.

And the physical address of the system table should be in
'efi_phys.systab'. See efi_init().

--
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 PATCH] mm, oom: cgroup-aware OOM-killer
From: Balbir Singh @ 2017-05-18 19:43 UTC (permalink / raw)
  To: Johannes Weiner
  Cc: Michal Hocko, Roman Gushchin, Tejun Heo, Li Zefan,
	Vladimir Davydov, Tetsuo Handa, kernel-team,
	cgroups@vger.kernel.org, open list:DOCUMENTATION,
	linux-kernel@vger.kernel.org, linux-mm
In-Reply-To: <20170518192240.GA29914@cmpxchg.org>

On Thu, 2017-05-18 at 15:22 -0400, Johannes Weiner wrote:
> On Fri, May 19, 2017 at 04:37:27AM +1000, Balbir Singh wrote:
> > On Fri, May 19, 2017 at 3:30 AM, Michal Hocko <mhocko@kernel.org> wrote:
> > > On Thu 18-05-17 17:28:04, Roman Gushchin wrote:
> > > > Traditionally, the OOM killer is operating on a process level.
> > > > Under oom conditions, it finds a process with the highest oom score
> > > > and kills it.
> > > > 
> > > > This behavior doesn't suit well the system with many running
> > > > containers. There are two main issues:
> > > > 
> > > > 1) There is no fairness between containers. A small container with
> > > > a few large processes will be chosen over a large one with huge
> > > > number of small processes.
> > > > 
> > > > 2) Containers often do not expect that some random process inside
> > > > will be killed. So, in general, a much safer behavior is
> > > > to kill the whole cgroup. Traditionally, this was implemented
> > > > in userspace, but doing it in the kernel has some advantages,
> > > > especially in a case of a system-wide OOM.
> > > > 
> > > > To address these issues, cgroup-aware OOM killer is introduced.
> > > > Under OOM conditions, it looks for a memcg with highest oom score,
> > > > and kills all processes inside.
> > > > 
> > > > Memcg oom score is calculated as a size of active and inactive
> > > > anon LRU lists, unevictable LRU list and swap size.
> > > > 
> > > > For a cgroup-wide OOM, only cgroups belonging to the subtree of
> > > > the OOMing cgroup are considered.
> > > 
> > > While this might make sense for some workloads/setups it is not a
> > > generally acceptable policy IMHO. We have discussed that different OOM
> > > policies might be interesting few years back at LSFMM but there was no
> > > real consensus on how to do that. One possibility was to allow bpf like
> > > mechanisms. Could you explore that path?
> > 
> > I agree, I think it needs more thought. I wonder if the real issue is something
> > else. For example
> > 
> > 1. Did we overcommit a particular container too much?
> > 2. Do we need something like https://lwn.net/Articles/604212/ to solve
> > the problem?
> 
> The occasional OOM kill is an unavoidable reality on our systems (and
> I bet on most deployments). If we tried not to overcommit, we'd waste
> a *lot* of memory.
> 
> The problem is when OOM happens, we really want the biggest *job* to
> get killed. Before cgroups, we assumed jobs were processes. But with
> cgroups, the user is able to define a group of processes as a job, and
> then an individual process is no longer a first-class memory consumer.
> 
> Without a patch like this, the OOM killer will compare the sizes of
> the random subparticles that the jobs in the system are composed of
> and kill the single biggest particle, leaving behind the incoherent
> remains of one of the jobs. That doesn't make a whole lot of sense.

I agree, but see my response on oom_notifiers in parallel that I sent
to Roman.

> 
> If you want to determine the most expensive car in a parking lot, you
> can't go off and compare the price of one car's muffler with the door
> handle of another, then point to a wind shield and yell "This is it!"
> 
> You need to compare the cars as a whole with each other.
> 
> > 3. We have oom notifiers now, could those be used (assuming you are interested
> > in non memcg related OOM's affecting a container
> 
> Right now, we watch for OOM notifications and then have userspace kill
> the rest of a job. That works - somewhat. What remains is the problem
> that I described above, that comparing individual process sizes is not
> meaningful when the terminal memory consumer is a cgroup.

Could the cgroup limit be used as the comparison point? stats inside
of the memory cgroup?

> 
> > 4. How do we determine limits for these containers? From a fariness
> > perspective
> 
> How do you mean?

How do we set them up so that the larger job gets more of the limits
as opposed to the small ones?

Balbir Singh.


--
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 PATCH] mm, oom: cgroup-aware OOM-killer
From: Balbir Singh @ 2017-05-18 19:41 UTC (permalink / raw)
  To: Roman Gushchin
  Cc: Michal Hocko, Johannes Weiner, Tejun Heo, Li Zefan,
	Vladimir Davydov, Tetsuo Handa, kernel-team,
	cgroups@vger.kernel.org, open list:DOCUMENTATION,
	linux-kernel@vger.kernel.org, linux-mm
In-Reply-To: <20170518192050.GA1648@castle>

On Thu, 2017-05-18 at 20:20 +0100, Roman Gushchin wrote:
> On Fri, May 19, 2017 at 04:37:27AM +1000, Balbir Singh wrote:
> > On Fri, May 19, 2017 at 3:30 AM, Michal Hocko <mhocko@kernel.org> wrote:
> > > On Thu 18-05-17 17:28:04, Roman Gushchin wrote:
> > > > Traditionally, the OOM killer is operating on a process level.
> > > > Under oom conditions, it finds a process with the highest oom score
> > > > and kills it.
> > > > 
> > > > This behavior doesn't suit well the system with many running
> > > > containers. There are two main issues:
> > > > 
> > > > 1) There is no fairness between containers. A small container with
> > > > a few large processes will be chosen over a large one with huge
> > > > number of small processes.
> > > > 
> > > > 2) Containers often do not expect that some random process inside
> > > > will be killed. So, in general, a much safer behavior is
> > > > to kill the whole cgroup. Traditionally, this was implemented
> > > > in userspace, but doing it in the kernel has some advantages,
> > > > especially in a case of a system-wide OOM.
> > > > 
> > > > To address these issues, cgroup-aware OOM killer is introduced.
> > > > Under OOM conditions, it looks for a memcg with highest oom score,
> > > > and kills all processes inside.
> > > > 
> > > > Memcg oom score is calculated as a size of active and inactive
> > > > anon LRU lists, unevictable LRU list and swap size.
> > > > 
> > > > For a cgroup-wide OOM, only cgroups belonging to the subtree of
> > > > the OOMing cgroup are considered.
> > > 
> > > While this might make sense for some workloads/setups it is not a
> > > generally acceptable policy IMHO. We have discussed that different OOM
> > > policies might be interesting few years back at LSFMM but there was no
> > > real consensus on how to do that. One possibility was to allow bpf like
> > > mechanisms. Could you explore that path?
> > 
> > I agree, I think it needs more thought. I wonder if the real issue is something
> > else. For example
> > 
> > 1. Did we overcommit a particular container too much?
> 
> Imagine, you have a machine with multiple containers,
> each with it's own process tree, and the machine is overcommited,
> i.e. sum of container's memory limits is larger the amount available RAM.
> 
> In a case of a system-wide OOM some random container will be affected.
> 

The random container containing the most expensive task, yes!

> Historically, this problem was solving by some user-space daemon,
> which was monitoring OOM events and cleaning up affected containers.
> But this approach can't solve the main problem: non-optimal selection
> of a victim. 

Why do you think the problem is non-optimal selection, is it because
we believe that memory cgroup limits should play a role in decision
making of global OOM?


> 
> > 2. Do we need something like https://urldefense.proofpoint.com/v2/url?u=https-3A__lwn.net_Articles_604212_&d=DwIBaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=jJYgtDM7QT-W-Fz_d29HYQ&m=9jV4id5lmsjFJj1kQjJk0auyQ3bzL27-f6Ur6ZNw36c&s=ElsS25CoZSPba6ke7O-EIsR7lN0psP6tDVyLnGqCMfs&e=  to solve
> > the problem?
>
 
The URL got changed to something non-parsable, probably for security, but
could you email client please not do that.

> I don't think it's related.

I was thinking that if we have virtual memory limits and we could set
some sane ones, we could avoid OOM altogether. OOM is a big hammer and
having allocations fail is far more acceptable than killing processes.
I believe that several applications may have much larger VM than actual
memory usage, but I believe with a good overcommit/virtual memory limiter
the problem can be better tackled.

> 
> > 3. We have oom notifiers now, could those be used (assuming you are interested
> > in non memcg related OOM's affecting a container
> 
> They can be used to inform an userspace daemon about an already happened OOM,
> but they do not affect victim selection.

Yes, the whole point is for the OS to select the victim, the notifiers
provide an opportunity for us to do reclaim to probably prevent OOM

In oom_kill, I see

                blocking_notifier_call_chain(&oom_notify_list, 0, &freed);
                if (freed > 0)
                        /* Got some memory back in the last second. */
                        return true;

Could the notification to user space then decide what to cleanup to free
memory? We also have event notification inside of memcg. I am trying to
understand why these are not sufficient?

We also have soft limits to push containers to a smaller size at the
time of global pressure.

> 
> > 4. How do we determine limits for these containers? From a fariness
> > perspective
> 
> Limits are usually set from some high-level understanding of the nature
> of tasks which are working inside, but overcommiting the machine is
> a common place, I assume.

Agreed overcommit is a given and that is why we wrote the cgroup controllers.
I was wondering if the container limits not being set correctly could cause
these issues. I am also trying to understand with the infrastructure we
have for notification and control, do we need more?

> 
> Thank you!
> 
> Roman

Cheers,
Balbir Singh.

--
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 PATCH] mm, oom: cgroup-aware OOM-killer
From: Johannes Weiner @ 2017-05-18 19:22 UTC (permalink / raw)
  To: Balbir Singh
  Cc: Michal Hocko, Roman Gushchin, Tejun Heo, Li Zefan,
	Vladimir Davydov, Tetsuo Handa, kernel-team,
	cgroups@vger.kernel.org, open list:DOCUMENTATION,
	linux-kernel@vger.kernel.org, linux-mm
In-Reply-To: <CAKTCnzkBNV9bsQSg4kzhxY=i=-y3x78StbbXfV9mvXLsJhGHig@mail.gmail.com>

On Fri, May 19, 2017 at 04:37:27AM +1000, Balbir Singh wrote:
> On Fri, May 19, 2017 at 3:30 AM, Michal Hocko <mhocko@kernel.org> wrote:
> > On Thu 18-05-17 17:28:04, Roman Gushchin wrote:
> >> Traditionally, the OOM killer is operating on a process level.
> >> Under oom conditions, it finds a process with the highest oom score
> >> and kills it.
> >>
> >> This behavior doesn't suit well the system with many running
> >> containers. There are two main issues:
> >>
> >> 1) There is no fairness between containers. A small container with
> >> a few large processes will be chosen over a large one with huge
> >> number of small processes.
> >>
> >> 2) Containers often do not expect that some random process inside
> >> will be killed. So, in general, a much safer behavior is
> >> to kill the whole cgroup. Traditionally, this was implemented
> >> in userspace, but doing it in the kernel has some advantages,
> >> especially in a case of a system-wide OOM.
> >>
> >> To address these issues, cgroup-aware OOM killer is introduced.
> >> Under OOM conditions, it looks for a memcg with highest oom score,
> >> and kills all processes inside.
> >>
> >> Memcg oom score is calculated as a size of active and inactive
> >> anon LRU lists, unevictable LRU list and swap size.
> >>
> >> For a cgroup-wide OOM, only cgroups belonging to the subtree of
> >> the OOMing cgroup are considered.
> >
> > While this might make sense for some workloads/setups it is not a
> > generally acceptable policy IMHO. We have discussed that different OOM
> > policies might be interesting few years back at LSFMM but there was no
> > real consensus on how to do that. One possibility was to allow bpf like
> > mechanisms. Could you explore that path?
> 
> I agree, I think it needs more thought. I wonder if the real issue is something
> else. For example
> 
> 1. Did we overcommit a particular container too much?
> 2. Do we need something like https://lwn.net/Articles/604212/ to solve
> the problem?

The occasional OOM kill is an unavoidable reality on our systems (and
I bet on most deployments). If we tried not to overcommit, we'd waste
a *lot* of memory.

The problem is when OOM happens, we really want the biggest *job* to
get killed. Before cgroups, we assumed jobs were processes. But with
cgroups, the user is able to define a group of processes as a job, and
then an individual process is no longer a first-class memory consumer.

Without a patch like this, the OOM killer will compare the sizes of
the random subparticles that the jobs in the system are composed of
and kill the single biggest particle, leaving behind the incoherent
remains of one of the jobs. That doesn't make a whole lot of sense.

If you want to determine the most expensive car in a parking lot, you
can't go off and compare the price of one car's muffler with the door
handle of another, then point to a wind shield and yell "This is it!"

You need to compare the cars as a whole with each other.

> 3. We have oom notifiers now, could those be used (assuming you are interested
> in non memcg related OOM's affecting a container

Right now, we watch for OOM notifications and then have userspace kill
the rest of a job. That works - somewhat. What remains is the problem
that I described above, that comparing individual process sizes is not
meaningful when the terminal memory consumer is a cgroup.

> 4. How do we determine limits for these containers? From a fariness
> perspective

How do you mean?

--
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 PATCH] mm, oom: cgroup-aware OOM-killer
From: Roman Gushchin @ 2017-05-18 19:20 UTC (permalink / raw)
  To: Balbir Singh
  Cc: Michal Hocko, Johannes Weiner, Tejun Heo, Li Zefan,
	Vladimir Davydov, Tetsuo Handa, kernel-team,
	cgroups@vger.kernel.org, open list:DOCUMENTATION,
	linux-kernel@vger.kernel.org, linux-mm
In-Reply-To: <CAKTCnzkBNV9bsQSg4kzhxY=i=-y3x78StbbXfV9mvXLsJhGHig@mail.gmail.com>

On Fri, May 19, 2017 at 04:37:27AM +1000, Balbir Singh wrote:
> On Fri, May 19, 2017 at 3:30 AM, Michal Hocko <mhocko@kernel.org> wrote:
> > On Thu 18-05-17 17:28:04, Roman Gushchin wrote:
> >> Traditionally, the OOM killer is operating on a process level.
> >> Under oom conditions, it finds a process with the highest oom score
> >> and kills it.
> >>
> >> This behavior doesn't suit well the system with many running
> >> containers. There are two main issues:
> >>
> >> 1) There is no fairness between containers. A small container with
> >> a few large processes will be chosen over a large one with huge
> >> number of small processes.
> >>
> >> 2) Containers often do not expect that some random process inside
> >> will be killed. So, in general, a much safer behavior is
> >> to kill the whole cgroup. Traditionally, this was implemented
> >> in userspace, but doing it in the kernel has some advantages,
> >> especially in a case of a system-wide OOM.
> >>
> >> To address these issues, cgroup-aware OOM killer is introduced.
> >> Under OOM conditions, it looks for a memcg with highest oom score,
> >> and kills all processes inside.
> >>
> >> Memcg oom score is calculated as a size of active and inactive
> >> anon LRU lists, unevictable LRU list and swap size.
> >>
> >> For a cgroup-wide OOM, only cgroups belonging to the subtree of
> >> the OOMing cgroup are considered.
> >
> > While this might make sense for some workloads/setups it is not a
> > generally acceptable policy IMHO. We have discussed that different OOM
> > policies might be interesting few years back at LSFMM but there was no
> > real consensus on how to do that. One possibility was to allow bpf like
> > mechanisms. Could you explore that path?
> 
> I agree, I think it needs more thought. I wonder if the real issue is something
> else. For example
> 
> 1. Did we overcommit a particular container too much?

Imagine, you have a machine with multiple containers,
each with it's own process tree, and the machine is overcommited,
i.e. sum of container's memory limits is larger the amount available RAM.

In a case of a system-wide OOM some random container will be affected.

Historically, this problem was solving by some user-space daemon,
which was monitoring OOM events and cleaning up affected containers.
But this approach can't solve the main problem: non-optimal selection
of a victim. 

> 2. Do we need something like https://urldefense.proofpoint.com/v2/url?u=https-3A__lwn.net_Articles_604212_&d=DwIBaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=jJYgtDM7QT-W-Fz_d29HYQ&m=9jV4id5lmsjFJj1kQjJk0auyQ3bzL27-f6Ur6ZNw36c&s=ElsS25CoZSPba6ke7O-EIsR7lN0psP6tDVyLnGqCMfs&e=  to solve
> the problem?

I don't think it's related.

> 3. We have oom notifiers now, could those be used (assuming you are interested
> in non memcg related OOM's affecting a container

They can be used to inform an userspace daemon about an already happened OOM,
but they do not affect victim selection.

> 4. How do we determine limits for these containers? From a fariness
> perspective

Limits are usually set from some high-level understanding of the nature
of tasks which are working inside, but overcommiting the machine is
a common place, I assume.

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: [RFC 1/6] mm, page_alloc: fix more premature OOM due to race with cpuset update
From: Christoph Lameter @ 2017-05-18 19:07 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: <20170518172424.GB30148@dhcp22.suse.cz>

On Thu, 18 May 2017, Michal Hocko wrote:

> > See above. OOM Kill in a cpuset does not kill an innocent task but a task
> > that does an allocation in that specific context meaning a task in that
> > cpuset that also has a memory policty.
>
> No, the oom killer will chose the largest task in the specific NUMA
> domain. If you just fail such an allocation then a page fault would get
> VM_FAULT_OOM and pagefault_out_of_memory would kill a task regardless of
> the cpusets.

Ok someone screwed up that code. There still is the determination that we
have a constrained alloc:

oom_kill:
	/*
         * Check if there were limitations on the allocation (only relevant for
         * NUMA and memcg) that may require different handling.
         */
        constraint = constrained_alloc(oc);
        if (constraint != CONSTRAINT_MEMORY_POLICY)
                oc->nodemask = NULL;
        check_panic_on_oom(oc, constraint);

-- Ok. A constrained failing alloc used to terminate the allocating
	process here. But it falls through to selecting a "bad process"


        if (!is_memcg_oom(oc) && sysctl_oom_kill_allocating_task &&
            current->mm && !oom_unkillable_task(current, NULL, oc->nodemask) &&
            current->signal->oom_score_adj != OOM_SCORE_ADJ_MIN) {
                get_task_struct(current);
                oc->chosen = current;
                oom_kill_process(oc, "Out of memory (oom_kill_allocating_task)");
                return true;
        }

--  A constrained allocation should not get here but fail the process that
	attempts the alloc.

        select_bad_process(oc);


Can we restore the old behavior? If I just specify the right memory policy
I can cause other processes to just be terminated?


> > Regardless of that the point earlier was that the moving logic can avoid
> > creating temporary situations of empty sets of nodes by analysing the
> > memory policies etc and only performing moves when doing so is safe.
>
> How are you going to do that in a raceless way? Moreover the whole
> discussion is about _failing_ allocations on an empty cpuset and
> mempolicy intersection.

Again this is only working for processes that are well behaved and it
never worked in a different way before. There was always the assumption
that a process does not allocate in the areas that have allocation
constraints and that the process does not change memory policies nor
store them somewhere for late etc etc. HPC apps typically allocate memory
on startup and then go through long times of processing and I/O.

The idea that cpuset node to node migration will work with a running
process that does abitrary activity is a pipe dream that we should give
up. There must be constraints on a process in order to allow this to work
and as far as I can tell this is best done in userspace with a library and
by putting requirements on the applications that desire to be movable that
way.

F.e. an application that does not use memory policies or other allocation
constraints should be fine. That has been working.

--
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] dm ioctl: Restore __GFP_HIGH in copy_params()
From: Junaid Shahid @ 2017-05-18 19:00 UTC (permalink / raw)
  To: akpm; +Cc: linux-mm, andreslc, gthelen, mpatocka, rientjes, mhocko, vbabka
In-Reply-To: <20170518185040.108293-1-junaids@google.com>

(Correcting linux-mm email addr)

d224e9381897 (drivers/md/dm-ioctl.c: use kvmalloc rather than opencoded
variant) left out the __GFP_HIGH flag when converting from __vmalloc to
kvmalloc. This can cause the IOCTL to fail in some low memory situations
where it wouldn't have failed earlier. This patch adds it back to avoid
any potential regression.

Signed-off-by: Junaid Shahid <junaids@google.com>
---
 drivers/md/dm-ioctl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/md/dm-ioctl.c b/drivers/md/dm-ioctl.c
index 0555b4410e05..bacad7637a56 100644
--- a/drivers/md/dm-ioctl.c
+++ b/drivers/md/dm-ioctl.c
@@ -1715,7 +1715,7 @@ static int copy_params(struct dm_ioctl __user *user, struct dm_ioctl *param_kern
 	 */
 	dmi = NULL;
 	noio_flag = memalloc_noio_save();
-	dmi = kvmalloc(param_kernel->data_size, GFP_KERNEL);
+	dmi = kvmalloc(param_kernel->data_size, GFP_KERNEL | __GFP_HIGH);
 	memalloc_noio_restore(noio_flag);
 
 	if (!dmi) {
-- 
2.13.0.303.g4ebf302169-goog

--
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 PATCH] mm, oom: cgroup-aware OOM-killer
From: Balbir Singh @ 2017-05-18 18:37 UTC (permalink / raw)
  To: Michal Hocko
  Cc: Roman Gushchin, Johannes Weiner, Tejun Heo, Li Zefan,
	Vladimir Davydov, Tetsuo Handa, kernel-team,
	cgroups@vger.kernel.org, open list:DOCUMENTATION,
	linux-kernel@vger.kernel.org, linux-mm
In-Reply-To: <20170518173002.GC30148@dhcp22.suse.cz>

On Fri, May 19, 2017 at 3:30 AM, Michal Hocko <mhocko@kernel.org> wrote:
> On Thu 18-05-17 17:28:04, Roman Gushchin wrote:
>> Traditionally, the OOM killer is operating on a process level.
>> Under oom conditions, it finds a process with the highest oom score
>> and kills it.
>>
>> This behavior doesn't suit well the system with many running
>> containers. There are two main issues:
>>
>> 1) There is no fairness between containers. A small container with
>> a few large processes will be chosen over a large one with huge
>> number of small processes.
>>
>> 2) Containers often do not expect that some random process inside
>> will be killed. So, in general, a much safer behavior is
>> to kill the whole cgroup. Traditionally, this was implemented
>> in userspace, but doing it in the kernel has some advantages,
>> especially in a case of a system-wide OOM.
>>
>> To address these issues, cgroup-aware OOM killer is introduced.
>> Under OOM conditions, it looks for a memcg with highest oom score,
>> and kills all processes inside.
>>
>> Memcg oom score is calculated as a size of active and inactive
>> anon LRU lists, unevictable LRU list and swap size.
>>
>> For a cgroup-wide OOM, only cgroups belonging to the subtree of
>> the OOMing cgroup are considered.
>
> While this might make sense for some workloads/setups it is not a
> generally acceptable policy IMHO. We have discussed that different OOM
> policies might be interesting few years back at LSFMM but there was no
> real consensus on how to do that. One possibility was to allow bpf like
> mechanisms. Could you explore that path?

I agree, I think it needs more thought. I wonder if the real issue is something
else. For example

1. Did we overcommit a particular container too much?
2. Do we need something like https://lwn.net/Articles/604212/ to solve
the problem?
3. We have oom notifiers now, could those be used (assuming you are interested
in non memcg related OOM's affecting a container
4. How do we determine limits for these containers? From a fariness
perspective

Just trying to understand what leads to the issues you are seeing

Balbir

--
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, page_alloc: Mark bad_range() and meminit_pfn_in_nid() as __maybe_unused
From: Matthias Kaehlcke @ 2017-05-18 18:20 UTC (permalink / raw)
  To: Andrew Morton, Vlastimil Babka, Mel Gorman
  Cc: linux-mm, linux-kernel, Matthias Kaehlcke

The functions are not used in some configurations. Adding the attribute
fixes the following warnings when building with clang:

mm/page_alloc.c:409:19: error: function 'bad_range' is not needed and
    will not be emitted [-Werror,-Wunneeded-internal-declaration]

mm/page_alloc.c:1106:30: error: unused function 'meminit_pfn_in_nid'
    [-Werror,-Wunused-function]

Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
---
 mm/page_alloc.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index f9e450c6b6e4..30d0dede3cf4 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -500,7 +500,7 @@ static int page_is_consistent(struct zone *zone, struct page *page)
 /*
  * Temporary debugging check for pages not lying within a given zone.
  */
-static int bad_range(struct zone *zone, struct page *page)
+static int __maybe_unused bad_range(struct zone *zone, struct page *page)
 {
 	if (page_outside_zone_boundaries(zone, page))
 		return 1;
@@ -510,7 +510,7 @@ static int bad_range(struct zone *zone, struct page *page)
 	return 0;
 }
 #else
-static inline int bad_range(struct zone *zone, struct page *page)
+static inline int __maybe_unused bad_range(struct zone *zone, struct page *page)
 {
 	return 0;
 }
@@ -1286,8 +1286,9 @@ int __meminit early_pfn_to_nid(unsigned long pfn)
 #endif
 
 #ifdef CONFIG_NODES_SPAN_OTHER_NODES
-static inline bool __meminit meminit_pfn_in_nid(unsigned long pfn, int node,
-					struct mminit_pfnnid_cache *state)
+static inline bool __meminit __maybe_unused
+meminit_pfn_in_nid(unsigned long pfn, int node,
+		   struct mminit_pfnnid_cache *state)
 {
 	int nid;
 
@@ -1309,8 +1310,9 @@ static inline bool __meminit early_pfn_in_nid(unsigned long pfn, int node)
 {
 	return true;
 }
-static inline bool __meminit meminit_pfn_in_nid(unsigned long pfn, int node,
-					struct mminit_pfnnid_cache *state)
+static inline bool __meminit  __maybe_unused
+meminit_pfn_in_nid(unsigned long pfn, int node,
+		   struct mminit_pfnnid_cache *state)
 {
 	return true;
 }
-- 
2.13.0.303.g4ebf302169-goog

--
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 PATCH] mm, oom: cgroup-aware OOM-killer
From: Johannes Weiner @ 2017-05-18 18:11 UTC (permalink / raw)
  To: Michal Hocko
  Cc: Roman Gushchin, Tejun Heo, Li Zefan, Vladimir Davydov,
	Tetsuo Handa, kernel-team, cgroups, linux-doc, linux-kernel,
	linux-mm
In-Reply-To: <20170518173002.GC30148@dhcp22.suse.cz>

On Thu, May 18, 2017 at 07:30:04PM +0200, Michal Hocko wrote:
> On Thu 18-05-17 17:28:04, Roman Gushchin wrote:
> > Traditionally, the OOM killer is operating on a process level.
> > Under oom conditions, it finds a process with the highest oom score
> > and kills it.
> > 
> > This behavior doesn't suit well the system with many running
> > containers. There are two main issues:
> > 
> > 1) There is no fairness between containers. A small container with
> > a few large processes will be chosen over a large one with huge
> > number of small processes.
> > 
> > 2) Containers often do not expect that some random process inside
> > will be killed. So, in general, a much safer behavior is
> > to kill the whole cgroup. Traditionally, this was implemented
> > in userspace, but doing it in the kernel has some advantages,
> > especially in a case of a system-wide OOM.
> > 
> > To address these issues, cgroup-aware OOM killer is introduced.
> > Under OOM conditions, it looks for a memcg with highest oom score,
> > and kills all processes inside.
> > 
> > Memcg oom score is calculated as a size of active and inactive
> > anon LRU lists, unevictable LRU list and swap size.
> > 
> > For a cgroup-wide OOM, only cgroups belonging to the subtree of
> > the OOMing cgroup are considered.
> 
> While this might make sense for some workloads/setups it is not a
> generally acceptable policy IMHO. We have discussed that different OOM
> policies might be interesting few years back at LSFMM but there was no
> real consensus on how to do that. One possibility was to allow bpf like
> mechanisms. Could you explore that path?

OOM policy is an orthogonal discussion, though.

The OOM killer's job is to pick a memory consumer to kill. Per default
the unit of the memory consumer is a process, but cgroups allow
grouping processes into compound consumers. Extending the OOM killer
to respect the new definition of "consumer" is not a new policy.

I don't think it's reasonable to ask the person who's trying to make
the OOM killer support group-consumers to design a dynamic OOM policy
framework instead.

All we want is the OOM policy, whatever it is, applied to cgroups.

--
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: [PATCHv5, REBASED 9/9] x86/mm: Allow to have userspace mappings above 47-bits
From: Michal Hocko @ 2017-05-18 17:51 UTC (permalink / raw)
  To: Kirill A. Shutemov
  Cc: Kirill A. Shutemov, x86, Thomas Gleixner, Ingo Molnar,
	H. Peter Anvin, Andi Kleen, Dave Hansen, Andy Lutomirski,
	Dan Williams, linux-mm, linux-kernel, linux-api
In-Reply-To: <20170518171330.GA30148@dhcp22.suse.cz>

On Thu 18-05-17 19:13:30, Michal Hocko wrote:
> On Thu 18-05-17 19:22:55, Kirill A. Shutemov wrote:
> > On Thu, May 18, 2017 at 05:59:14PM +0200, Michal Hocko wrote:
> [...]
> > > I basically mean something like the following
> > > ---
> > > diff --git a/arch/x86/kernel/sys_x86_64.c b/arch/x86/kernel/sys_x86_64.c
> > > index 74d1587b181d..d6f66ff02d0a 100644
> > > --- a/arch/x86/kernel/sys_x86_64.c
> > > +++ b/arch/x86/kernel/sys_x86_64.c
> > > @@ -195,7 +195,7 @@ arch_get_unmapped_area_topdown(struct file *filp, const unsigned long addr0,
> > >  		goto bottomup;
> > >  
> > >  	/* requesting a specific address */
> > > -	if (addr) {
> > > +	if (addr && addr <= DEFAULT_MAP_WINDOW) {
> > >  		addr = PAGE_ALIGN(addr);
> > >  		vma = find_vma(mm, addr);
> > >  		if (TASK_SIZE - len >= addr &&
> > > @@ -215,7 +215,7 @@ arch_get_unmapped_area_topdown(struct file *filp, const unsigned long addr0,
> > >  	 * !in_compat_syscall() check to avoid high addresses for x32.
> > >  	 */
> > >  	if (addr > DEFAULT_MAP_WINDOW && !in_compat_syscall())
> > > -		info.high_limit += TASK_SIZE_MAX - DEFAULT_MAP_WINDOW;
> > > +		info.high_limit += min(TASK_SIZE_MAX, address) - DEFAULT_MAP_WINDOW;
> > >  
> > >  	info.align_mask = 0;
> > >  	info.align_offset = pgoff << PAGE_SHIFT;
> > 
> > You try to stretch the interface too far. With the patch you propose we
> > have totally different behaviour wrt hint address if it below and above
> > 47-bits:
> > 
> >  * <= 47-bits: allocate VM [addr; addr + len - 1], if free;
> 
> unless I am missing something fundamental here this is not how it works.
> We just map a different range if the requested one is not free (in
> absence of MAP_FIXED). And we do that in top->down direction so this is
> already how it works. And you _do_ rely on the same thing when allowing
> larger than 47b except you start from the top of the supported address
> space. So how exactly is your new behavior any different and more clear?

OK, I take that back because I am clearly wrong. We simply always start
from top. Sorry about my confusion.

Feel free to add
Acked-by: Michal Hocko <mhocko@suse.com>
-- 
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

* [PATCH 1/3] ksm: cleanup stable_node chain collapse case
From: Andrea Arcangeli @ 2017-05-18 17:37 UTC (permalink / raw)
  To: Andrew Morton, linux-mm
  Cc: Evgheni Dereveanchin, Andrey Ryabinin, Petr Holasek, Hugh Dickins,
	Arjan van de Ven, Davidlohr Bueso, Gavin Guo, Jay Vosburgh,
	Mel Gorman, Dan Carpenter
In-Reply-To: <20170518173721.22316-1-aarcange@redhat.com>

When the stable_node chain is collapsed we can as well set the caller
stable_node to match the returned stable_node_dup in chain_prune().

This way the collapse case becomes indistinguishable from the regular
stable_node case and we can remove two branches from the KSM page
migration handling slow paths.

While it was all correct this looks cleaner (and faster) as the caller
has to deal with fewer special cases.

Signed-off-by: Andrea Arcangeli <aarcange@redhat.com>
---
 mm/ksm.c | 50 ++++++++++++++++++++++++++++----------------------
 1 file changed, 28 insertions(+), 22 deletions(-)

diff --git a/mm/ksm.c b/mm/ksm.c
index fc0c73bd6cb3..959036064bb7 100644
--- a/mm/ksm.c
+++ b/mm/ksm.c
@@ -1394,14 +1394,18 @@ static struct stable_node *stable_node_dup(struct stable_node **_stable_node,
 			ksm_stable_node_chains--;
 			ksm_stable_node_dups--;
 			/*
-			 * NOTE: the caller depends on the
-			 * *_stable_node to become NULL if the chain
-			 * was collapsed. Enforce that if anything
-			 * uses a stale (freed) stable_node chain a
-			 * visible crash will materialize (instead of
-			 * an use after free).
+			 * NOTE: the caller depends on the stable_node
+			 * to be equal to stable_node_dup if the chain
+			 * was collapsed.
 			 */
-			*_stable_node = stable_node = NULL;
+			*_stable_node = found;
+			/*
+			 * Just for robustneess as stable_node is
+			 * otherwise left as a stable pointer, the
+			 * compiler shall optimize it away at build
+			 * time.
+			 */
+			stable_node = NULL;
 		} else if (__is_page_sharing_candidate(found, 1)) {
 			/*
 			 * Refile our candidate at the head
@@ -1507,7 +1511,11 @@ static struct page *stable_tree_search(struct page *page)
 		 * not NULL. stable_node_dup may have been inserted in
 		 * the rbtree instead as a regular stable_node (in
 		 * order to collapse the stable_node chain if a single
-		 * stable_node dup was found in it).
+		 * stable_node dup was found in it). In such case the
+		 * stable_node is overwritten by the calleee to point
+		 * to the stable_node_dup that was collapsed in the
+		 * stable rbtree and stable_node will be equal to
+		 * stable_node_dup like if the chain never existed.
 		 */
 		if (!stable_node_dup) {
 			/*
@@ -1625,15 +1633,13 @@ static struct page *stable_tree_search(struct page *page)
 replace:
 	/*
 	 * If stable_node was a chain and chain_prune collapsed it,
-	 * stable_node will be NULL here. In that case the
-	 * stable_node_dup is the regular stable_node that has
-	 * replaced the chain. If stable_node is not NULL and equal to
-	 * stable_node_dup there was no chain and stable_node_dup is
-	 * the regular stable_node in the stable rbtree. Otherwise
-	 * stable_node is the chain and stable_node_dup is the dup to
-	 * replace.
+	 * stable_node has been updated to be the new regular
+	 * stable_node. A collapse of the chain is indistinguishable
+	 * from the case there was no chain in the stable
+	 * rbtree. Otherwise stable_node is the chain and
+	 * stable_node_dup is the dup to replace.
 	 */
-	if (!stable_node || stable_node_dup == stable_node) {
+	if (stable_node_dup == stable_node) {
 		VM_BUG_ON(is_stable_node_chain(stable_node_dup));
 		VM_BUG_ON(is_stable_node_dup(stable_node_dup));
 		/* there is no chain */
@@ -1678,13 +1684,13 @@ static struct page *stable_tree_search(struct page *page)
 		stable_node_dup = stable_node_any;
 	/*
 	 * If stable_node was a chain and chain_prune collapsed it,
-	 * stable_node will be NULL here. In that case the
-	 * stable_node_dup is the regular stable_node that has
-	 * replaced the chain. If stable_node is not NULL and equal to
-	 * stable_node_dup there was no chain and stable_node_dup is
-	 * the regular stable_node in the stable rbtree.
+	 * stable_node has been updated to be the new regular
+	 * stable_node. A collapse of the chain is indistinguishable
+	 * from the case there was no chain in the stable
+	 * rbtree. Otherwise stable_node is the chain and
+	 * stable_node_dup is the dup to replace.
 	 */
-	if (!stable_node || stable_node_dup == stable_node) {
+	if (stable_node_dup == stable_node) {
 		VM_BUG_ON(is_stable_node_chain(stable_node_dup));
 		VM_BUG_ON(is_stable_node_dup(stable_node_dup));
 		/* chain is missing so create it */

--
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/3] ksm: swap the two output parameters of chain/chain_prune
From: Andrea Arcangeli @ 2017-05-18 17:37 UTC (permalink / raw)
  To: Andrew Morton, linux-mm
  Cc: Evgheni Dereveanchin, Andrey Ryabinin, Petr Holasek, Hugh Dickins,
	Arjan van de Ven, Davidlohr Bueso, Gavin Guo, Jay Vosburgh,
	Mel Gorman, Dan Carpenter
In-Reply-To: <20170518173721.22316-1-aarcange@redhat.com>

Some static checker complains if chain/chain_prune returns a
potentially stale pointer.

There are two output parameters to chain/chain_prune, one is tree_page
the other is stable_node_dup. Like in get_ksm_page the caller has to
check tree_page is NULL before touching the stable_node. Similarly in
chain/chain_prune the caller has to check tree_page before touching
the stable_node_dup returned or the original stable_node passed as
parameter.

Because the tree_page is never returned as a stale pointer, it may be
more intuitive to return tree_page and to pass stable_node_dup for
reference instead of the reverse.

This patch purely swaps the two output parameters of chain/chain_prune
as a cleanup for the static checker and to mimic the get_ksm_page
behavior more closely. There's no change to the caller at all except
the swap, it's purely a cleanup and it is a noop from the caller point
of view.

Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Andrea Arcangeli <aarcange@redhat.com>
---
 mm/ksm.c | 78 ++++++++++++++++++++++++++++++++++++++++++----------------------
 1 file changed, 52 insertions(+), 26 deletions(-)

diff --git a/mm/ksm.c b/mm/ksm.c
index 959036064bb7..7b2e26f9cf41 100644
--- a/mm/ksm.c
+++ b/mm/ksm.c
@@ -1315,14 +1315,14 @@ bool is_page_sharing_candidate(struct stable_node *stable_node)
 	return __is_page_sharing_candidate(stable_node, 0);
 }
 
-static struct stable_node *stable_node_dup(struct stable_node **_stable_node,
-					   struct page **tree_page,
-					   struct rb_root *root,
-					   bool prune_stale_stable_nodes)
+struct page *stable_node_dup(struct stable_node **_stable_node_dup,
+			     struct stable_node **_stable_node,
+			     struct rb_root *root,
+			     bool prune_stale_stable_nodes)
 {
 	struct stable_node *dup, *found = NULL, *stable_node = *_stable_node;
 	struct hlist_node *hlist_safe;
-	struct page *_tree_page;
+	struct page *_tree_page, *tree_page = NULL;
 	int nr = 0;
 	int found_rmap_hlist_len;
 
@@ -1355,14 +1355,14 @@ static struct stable_node *stable_node_dup(struct stable_node **_stable_node,
 			if (!found ||
 			    dup->rmap_hlist_len > found_rmap_hlist_len) {
 				if (found)
-					put_page(*tree_page);
+					put_page(tree_page);
 				found = dup;
 				found_rmap_hlist_len = found->rmap_hlist_len;
-				*tree_page = _tree_page;
+				tree_page = _tree_page;
 
+				/* skip put_page for found dup */
 				if (!prune_stale_stable_nodes)
 					break;
-				/* skip put_page */
 				continue;
 			}
 		}
@@ -1419,7 +1419,8 @@ static struct stable_node *stable_node_dup(struct stable_node **_stable_node,
 		}
 	}
 
-	return found;
+	*_stable_node_dup = found;
+	return tree_page;
 }
 
 static struct stable_node *stable_node_dup_any(struct stable_node *stable_node,
@@ -1435,35 +1436,60 @@ static struct stable_node *stable_node_dup_any(struct stable_node *stable_node,
 			   typeof(*stable_node), hlist_dup);
 }
 
-static struct stable_node *__stable_node_chain(struct stable_node **_stable_node,
-					       struct page **tree_page,
-					       struct rb_root *root,
-					       bool prune_stale_stable_nodes)
+/*
+ * Like for get_ksm_page, this function can free the *_stable_node and
+ * *_stable_node_dup if the returned tree_page is NULL.
+ *
+ * It can also free and overwrite *_stable_node with the found
+ * stable_node_dup if the chain is collapsed (in which case
+ * *_stable_node will be equal to *_stable_node_dup like if the chain
+ * never existed). It's up to the caller to verify tree_page is not
+ * NULL before dereferencing *_stable_node or *_stable_node_dup.
+ *
+ * *_stable_node_dup is really a second output parameter of this
+ * function and will be overwritten in all cases, the caller doesn't
+ * need to initialize it.
+ */
+static struct page *__stable_node_chain(struct stable_node **_stable_node_dup,
+					struct stable_node **_stable_node,
+					struct rb_root *root,
+					bool prune_stale_stable_nodes)
 {
 	struct stable_node *stable_node = *_stable_node;
 	if (!is_stable_node_chain(stable_node)) {
 		if (is_page_sharing_candidate(stable_node)) {
-			*tree_page = get_ksm_page(stable_node, false);
-			return stable_node;
+			*_stable_node_dup = stable_node;
+			return get_ksm_page(stable_node, false);
 		}
+		/*
+		 * _stable_node_dup set to NULL means the stable_node
+		 * reached the ksm_max_page_sharing limit.
+		 */
+		*_stable_node_dup = NULL;
 		return NULL;
 	}
-	return stable_node_dup(_stable_node, tree_page, root,
+	return stable_node_dup(_stable_node_dup, _stable_node, root,
 			       prune_stale_stable_nodes);
 }
 
-static __always_inline struct stable_node *chain_prune(struct stable_node **s_n,
-						       struct page **t_p,
-						       struct rb_root *root)
+static __always_inline struct page *chain_prune(struct stable_node **s_n_d,
+						struct stable_node **s_n,
+						struct rb_root *root)
 {
-	return __stable_node_chain(s_n, t_p, root, true);
+	return __stable_node_chain(s_n_d, s_n, root, true);
 }
 
-static __always_inline struct stable_node *chain(struct stable_node *s_n,
-						 struct page **t_p,
-						 struct rb_root *root)
+static __always_inline struct page *chain(struct stable_node **s_n_d,
+					  struct stable_node *s_n,
+					  struct rb_root *root)
 {
-	return __stable_node_chain(&s_n, t_p, root, false);
+	struct stable_node *old_stable_node = s_n;
+	struct page *tree_page;
+
+	tree_page = __stable_node_chain(s_n_d, &s_n, root, false);
+	/* not pruning dups so s_n cannot have changed */
+	VM_BUG_ON(s_n != old_stable_node);
+	return tree_page;
 }
 
 /*
@@ -1504,7 +1530,7 @@ static struct page *stable_tree_search(struct page *page)
 		cond_resched();
 		stable_node = rb_entry(*new, struct stable_node, node);
 		stable_node_any = NULL;
-		stable_node_dup = chain_prune(&stable_node, &tree_page, root);
+		tree_page = chain_prune(&stable_node_dup, &stable_node,	root);
 		/*
 		 * NOTE: stable_node may have been freed by
 		 * chain_prune() if the returned stable_node_dup is
@@ -1745,7 +1771,7 @@ static struct stable_node *stable_tree_insert(struct page *kpage)
 		cond_resched();
 		stable_node = rb_entry(*new, struct stable_node, node);
 		stable_node_any = NULL;
-		stable_node_dup = chain(stable_node, &tree_page, root);
+		tree_page = chain(&stable_node_dup, stable_node, root);
 		if (!stable_node_dup) {
 			/*
 			 * Either all stable_node dups were full in

--
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/3] ksm: optimize refile of stable_node_dup at the head of the chain
From: Andrea Arcangeli @ 2017-05-18 17:37 UTC (permalink / raw)
  To: Andrew Morton, linux-mm
  Cc: Evgheni Dereveanchin, Andrey Ryabinin, Petr Holasek, Hugh Dickins,
	Arjan van de Ven, Davidlohr Bueso, Gavin Guo, Jay Vosburgh,
	Mel Gorman, Dan Carpenter
In-Reply-To: <20170518173721.22316-1-aarcange@redhat.com>

If a candidate stable_node_dup has been found and it can accept
further merges it can be refiled to the head of the list to speedup
next searches without altering which dup is found and how the dups
accumulate in the chain.

We already refiled it back to the head in the prune_stale_stable_nodes
case, but we didn't refile it if not pruning (which is more
common). And we also refiled it when it was already at the head which
is unnecessary (in the prune_stale_stable_nodes case, nr > 1 means
there's more than one dup in the chain, it doesn't mean it's not
already at the head of the chain).

The stable_node_chain list is single threaded and there's no SMP
locking contention so it should be faster to refile it to the head of
the list also if prune_stale_stable_nodes is false.

A profiling shows the refile happens 1.9% of the time when a dup is
found with a max_page_sharing limit setting of 3 (with
max_page_sharing of 2 the refile never happens of course as there's
never space for one more merge) which is reasonably low. At higher
max_page_sharing values it should be much less frequent.

This is just an optimization.

Signed-off-by: Andrea Arcangeli <aarcange@redhat.com>
---
 mm/ksm.c | 35 +++++++++++++++++++++++------------
 1 file changed, 23 insertions(+), 12 deletions(-)

diff --git a/mm/ksm.c b/mm/ksm.c
index 7b2e26f9cf41..e02342f4f6aa 100644
--- a/mm/ksm.c
+++ b/mm/ksm.c
@@ -1369,13 +1369,14 @@ struct page *stable_node_dup(struct stable_node **_stable_node_dup,
 		put_page(_tree_page);
 	}
 
-	/*
-	 * nr is relevant only if prune_stale_stable_nodes is true,
-	 * otherwise we may break the loop at nr == 1 even if there
-	 * are multiple entries.
-	 */
-	if (prune_stale_stable_nodes && found) {
-		if (nr == 1) {
+	if (found) {
+		/*
+		 * nr is counting all dups in the chain only if
+		 * prune_stale_stable_nodes is true, otherwise we may
+		 * break the loop at nr == 1 even if there are
+		 * multiple entries.
+		 */
+		if (prune_stale_stable_nodes && nr == 1) {
 			/*
 			 * If there's not just one entry it would
 			 * corrupt memory, better BUG_ON. In KSM
@@ -1406,12 +1407,22 @@ struct page *stable_node_dup(struct stable_node **_stable_node_dup,
 			 * time.
 			 */
 			stable_node = NULL;
-		} else if (__is_page_sharing_candidate(found, 1)) {
+		} else if (stable_node->hlist.first != &found->hlist_dup &&
+			   __is_page_sharing_candidate(found, 1)) {
 			/*
-			 * Refile our candidate at the head
-			 * after the prune if our candidate
-			 * can accept one more future sharing
-			 * in addition to the one underway.
+			 * If the found stable_node dup can accept one
+			 * more future merge (in addition to the one
+			 * that is underway) and is not at the head of
+			 * the chain, put it there so next search will
+			 * be quicker in the !prune_stale_stable_nodes
+			 * case.
+			 *
+			 * NOTE: it would be inaccurate to use nr > 1
+			 * instead of checking the hlist.first pointer
+			 * directly, because in the
+			 * prune_stale_stable_nodes case "nr" isn't
+			 * the position of the found dup in the chain,
+			 * but the total number of dups in the chain.
 			 */
 			hlist_del(&found->hlist_dup);
 			hlist_add_head(&found->hlist_dup,

--
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 0/3] KSMscale cleanup/optimizations
From: Andrea Arcangeli @ 2017-05-18 17:37 UTC (permalink / raw)
  To: Andrew Morton, linux-mm
  Cc: Evgheni Dereveanchin, Andrey Ryabinin, Petr Holasek, Hugh Dickins,
	Arjan van de Ven, Davidlohr Bueso, Gavin Guo, Jay Vosburgh,
	Mel Gorman, Dan Carpenter

Hello,

This is incremental with the two fixes already in -mm.

There are no fixes here it's just minor cleanups and optimizations.

1/3 removes makes the "fix" for the stale stable_node fall in the
standard case without introducing new cases. Setting stable_node to
NULL was marginally safer, but stale pointer is still wiped from the
caller, this looks cleaner.

2/3 should fix the false positive from Dan's static checker. Dan could
you check if it still complains?

3/3 is a microoptimization to apply the the refile of future merge
candidate dups at the head of the chain in all cases and to skip it in
one case where we did it and but it was a noop (to avoid checking if
it was already at the head but now we've to check it anyway so it got
optimized away).

Andrea Arcangeli (3):
  ksm: cleanup stable_node chain collapse case
  ksm: swap the two output parameters of chain/chain_prune
  ksm: optimize refile of stable_node_dup at the head of the chain

 mm/ksm.c | 163 ++++++++++++++++++++++++++++++++++++++++-----------------------
 1 file changed, 103 insertions(+), 60 deletions(-)

--
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 PATCH] mm, oom: cgroup-aware OOM-killer
From: Michal Hocko @ 2017-05-18 17:30 UTC (permalink / raw)
  To: Roman Gushchin
  Cc: Johannes Weiner, Tejun Heo, Li Zefan, Vladimir Davydov,
	Tetsuo Handa, kernel-team, cgroups, linux-doc, linux-kernel,
	linux-mm
In-Reply-To: <1495124884-28974-1-git-send-email-guro@fb.com>

On Thu 18-05-17 17:28:04, Roman Gushchin wrote:
> Traditionally, the OOM killer is operating on a process level.
> Under oom conditions, it finds a process with the highest oom score
> and kills it.
> 
> This behavior doesn't suit well the system with many running
> containers. There are two main issues:
> 
> 1) There is no fairness between containers. A small container with
> a few large processes will be chosen over a large one with huge
> number of small processes.
> 
> 2) Containers often do not expect that some random process inside
> will be killed. So, in general, a much safer behavior is
> to kill the whole cgroup. Traditionally, this was implemented
> in userspace, but doing it in the kernel has some advantages,
> especially in a case of a system-wide OOM.
> 
> To address these issues, cgroup-aware OOM killer is introduced.
> Under OOM conditions, it looks for a memcg with highest oom score,
> and kills all processes inside.
> 
> Memcg oom score is calculated as a size of active and inactive
> anon LRU lists, unevictable LRU list and swap size.
> 
> For a cgroup-wide OOM, only cgroups belonging to the subtree of
> the OOMing cgroup are considered.

While this might make sense for some workloads/setups it is not a
generally acceptable policy IMHO. We have discussed that different OOM
policies might be interesting few years back at LSFMM but there was no
real consensus on how to do that. One possibility was to allow bpf like
mechanisms. Could you explore that path?
-- 
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: [RFC 1/6] mm, page_alloc: fix more premature OOM due to race with cpuset update
From: Michal Hocko @ 2017-05-18 17:24 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.1705181154450.27641@east.gentwo.org>

On Thu 18-05-17 11:57:55, Cristopher Lameter wrote:
> On Thu, 18 May 2017, Michal Hocko wrote:
> 
> > > 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.
> >
> > Not really. If you can trick the system to _think_ that the intersection
> > between mempolicy and the cpuset is empty then the OOM killer might
> > trigger an innocent task rather than the one which tricked it into that
> > situation.
> 
> See above. OOM Kill in a cpuset does not kill an innocent task but a task
> that does an allocation in that specific context meaning a task in that
> cpuset that also has a memory policty.

No, the oom killer will chose the largest task in the specific NUMA
domain. If you just fail such an allocation then a page fault would get
VM_FAULT_OOM and pagefault_out_of_memory would kill a task regardless of
the cpusets.
 
> Regardless of that the point earlier was that the moving logic can avoid
> creating temporary situations of empty sets of nodes by analysing the
> memory policies etc and only performing moves when doing so is safe.

How are you going to do that in a raceless way? Moreover the whole
discussion is about _failing_ allocations on an empty cpuset and
mempolicy intersection.

-- 
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: [RFC PATCH v2 11/17] cgroup: Implement new thread mode semantics
From: Waiman Long @ 2017-05-18 17:21 UTC (permalink / raw)
  To: Tejun Heo
  Cc: Li Zefan, Johannes Weiner, Peter Zijlstra, Ingo Molnar, cgroups,
	linux-kernel, linux-doc, linux-mm, kernel-team, pjt, luto, efault
In-Reply-To: <20170517214718.GH942@htj.duckdns.org>

On 05/17/2017 05:47 PM, Tejun Heo wrote:
> Hello, Waiman.
>
> On Mon, May 15, 2017 at 09:34:10AM -0400, Waiman Long wrote:
>> The current thread mode semantics aren't sufficient to fully support
>> threaded controllers like cpu. The main problem is that when thread
>> mode is enabled at root (mainly for performance reason), all the
>> non-threaded controllers cannot be supported at all.
>>
>> To alleviate this problem, the roles of thread root and threaded
>> cgroups are now further separated. Now thread mode can only be enabled
>> on a non-root leaf cgroup whose parent will then become the thread
>> root. All the descendants of a threaded cgroup will still need to be
>> threaded. All the non-threaded resource will be accounted for in the
>> thread root. Unlike the previous thread mode, however, a thread root
>> can have non-threaded children where system resources like memory
>> can be further split down the hierarchy.
>>
>> Now we could have something like
>>
>> 	R -- A -- B
>> 	 \
>> 	  T1 -- T2
>>
>> where R is the thread root, A and B are non-threaded cgroups, T1 and
>> T2 are threaded cgroups. The cgroups R, T1, T2 form a threaded subtree
>> where all the non-threaded resources are accounted for in R.  The no
>> internal process constraint does not apply in the threaded subtree.
>> Non-threaded controllers need to properly handle the competition
>> between internal processes and child cgroups at the thread root.
>>
>> This model will be flexible enough to support the need of the threaded
>> controllers.
> I do like the approach and it does address the issue with requiring at
> least one level of nesting for the thread mode to be used with other
> controllers.  I need to think a bit more about it and mull over what
> Peterz was suggesting in the old thread.  I'll get back to you soon
> but I'd really prefer this and the earlier related patches to be in
> its own patchset so that we aren't dealing with different things at
> the same time.
>
> Thanks.
>
I have studied the email exchanges with your original thread mode
patchset. This patchset is aimed to hopefully address all the concerns
that Peterz has. This enhanced thread mode should address a big part of
the concern. However, I am not sure if this patch, by itself, is enough
to address all his concerns. That is why I also include 2 other major
changes in the next 2 patches. My goal is to move forward to allow all
controllers to be enabled for v2 eventually. We are not there yet, but I
hope this patchset can move thing forward meaningfully.

Regards,
Longman


--
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


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