| Message ID | 38c6a6e1-c5e0-fd7d-4baf-1f0f09be5094@suse.cz (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Jun 29, 2018 at 05:37:02PM +0200, Vlastimil Babka wrote: > On 06/20/2018 01:23 PM, kbuild test robot wrote: > > Hi Vlastimil, > > > > Thank you for the patch! Yet something to improve: > > > > [auto build test ERROR on mmotm/master] > > [also build test ERROR on v4.18-rc1 next-20180619] > > [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] > > > > url: https://github.com/0day-ci/linux/commits/Vlastimil-Babka/kmalloc-reclaimable-caches/20180618-172912 > > base: git://git.cmpxchg.org/linux-mmotm.git master > > config: x86_64-allmodconfig (attached as .config) > > compiler: gcc-7 (Debian 7.3.0-16) 7.3.0 > > reproduce: > > # save the attached .config to linux build tree > > make ARCH=x86_64 > > > > All errors (new ones prefixed by >>): > > > > drivers/staging//android/ion/ion_page_pool.c: In function 'ion_page_pool_remove': > >>> drivers/staging//android/ion/ion_page_pool.c:56:40: error: 'NR_INDIRECTLY_RECLAIMABLE_BYTES' undeclared (first use in this function) > > mod_node_page_state(page_pgdat(page), NR_INDIRECTLY_RECLAIMABLE_BYTES, > > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > drivers/staging//android/ion/ion_page_pool.c:56:40: note: each undeclared identifier is reported only once for each function it appears in > > > > vim +/NR_INDIRECTLY_RECLAIMABLE_BYTES +56 drivers/staging//android/ion/ion_page_pool.c > > Looks like I missed a hunk, updated patch below. > > ----8<---- > From a0053c64c72d7e094252d0d7462de8569d87c543 Mon Sep 17 00:00:00 2001 > From: Vlastimil Babka <vbabka@suse.cz> > Date: Tue, 22 May 2018 16:10:10 +0200 > Subject: [PATCH v3 5/7] mm: rename and change semantics of > nr_indirectly_reclaimable_bytes > > The vmstat counter NR_INDIRECTLY_RECLAIMABLE_BYTES was introduced by commit > eb59254608bc ("mm: introduce NR_INDIRECTLY_RECLAIMABLE_BYTES") with the goal of > accounting objects that can be reclaimed, but cannot be allocated via a > SLAB_RECLAIM_ACCOUNT cache. This is now possible via kmalloc() with > __GFP_RECLAIMABLE flag, and the dcache external names user is converted. > > The counter is however still useful for accounting direct page allocations > (i.e. not slab) with a shrinker, such as the ION page pool. So keep it, and: Btw, it looks like I've another example of usefulness of this counter: dynamic per-cpu data.
On 06/29/2018 11:12 PM, Roman Gushchin wrote: >> >> The vmstat counter NR_INDIRECTLY_RECLAIMABLE_BYTES was introduced by commit >> eb59254608bc ("mm: introduce NR_INDIRECTLY_RECLAIMABLE_BYTES") with the goal of >> accounting objects that can be reclaimed, but cannot be allocated via a >> SLAB_RECLAIM_ACCOUNT cache. This is now possible via kmalloc() with >> __GFP_RECLAIMABLE flag, and the dcache external names user is converted. >> >> The counter is however still useful for accounting direct page allocations >> (i.e. not slab) with a shrinker, such as the ION page pool. So keep it, and: > > Btw, it looks like I've another example of usefulness of this counter: > dynamic per-cpu data. Hmm, but are those reclaimable? Most likely not in general? Do you have examples that are?
On Sat, Jun 30, 2018 at 12:09:27PM +0200, Vlastimil Babka wrote: > On 06/29/2018 11:12 PM, Roman Gushchin wrote: > >> > >> The vmstat counter NR_INDIRECTLY_RECLAIMABLE_BYTES was introduced by commit > >> eb59254608bc ("mm: introduce NR_INDIRECTLY_RECLAIMABLE_BYTES") with the goal of > >> accounting objects that can be reclaimed, but cannot be allocated via a > >> SLAB_RECLAIM_ACCOUNT cache. This is now possible via kmalloc() with > >> __GFP_RECLAIMABLE flag, and the dcache external names user is converted. > >> > >> The counter is however still useful for accounting direct page allocations > >> (i.e. not slab) with a shrinker, such as the ION page pool. So keep it, and: > > > > Btw, it looks like I've another example of usefulness of this counter: > > dynamic per-cpu data. > > Hmm, but are those reclaimable? Most likely not in general? Do you have > examples that are? If these per-cpu data is something like per-cpu refcounters, which are using to manage reclaimable objects (e.g. cgroup css objects). Of course, they are not always reclaimable, but in certain states. Thanks!
On 07/02/2018 06:52 PM, Roman Gushchin wrote: > On Sat, Jun 30, 2018 at 12:09:27PM +0200, Vlastimil Babka wrote: >> On 06/29/2018 11:12 PM, Roman Gushchin wrote: >>>> >>>> The vmstat counter NR_INDIRECTLY_RECLAIMABLE_BYTES was introduced by commit >>>> eb59254608bc ("mm: introduce NR_INDIRECTLY_RECLAIMABLE_BYTES") with the goal of >>>> accounting objects that can be reclaimed, but cannot be allocated via a >>>> SLAB_RECLAIM_ACCOUNT cache. This is now possible via kmalloc() with >>>> __GFP_RECLAIMABLE flag, and the dcache external names user is converted. >>>> >>>> The counter is however still useful for accounting direct page allocations >>>> (i.e. not slab) with a shrinker, such as the ION page pool. So keep it, and: >>> >>> Btw, it looks like I've another example of usefulness of this counter: >>> dynamic per-cpu data. >> >> Hmm, but are those reclaimable? Most likely not in general? Do you have >> examples that are? > > If these per-cpu data is something like per-cpu refcounters, > which are using to manage reclaimable objects (e.g. cgroup css objects). > Of course, they are not always reclaimable, but in certain states. BTW, seems you seem interested, could you provide some more formal review as well? Others too. We don't need to cover all use cases immediately, when the patchset is apparently stalled due to lack of review. Thanks! > Thanks! >
On Tue, Jul 17, 2018 at 10:44:07AM +0200, Vlastimil Babka wrote: > On 07/02/2018 06:52 PM, Roman Gushchin wrote: > > On Sat, Jun 30, 2018 at 12:09:27PM +0200, Vlastimil Babka wrote: > >> On 06/29/2018 11:12 PM, Roman Gushchin wrote: > >>>> > >>>> The vmstat counter NR_INDIRECTLY_RECLAIMABLE_BYTES was introduced by commit > >>>> eb59254608bc ("mm: introduce NR_INDIRECTLY_RECLAIMABLE_BYTES") with the goal of > >>>> accounting objects that can be reclaimed, but cannot be allocated via a > >>>> SLAB_RECLAIM_ACCOUNT cache. This is now possible via kmalloc() with > >>>> __GFP_RECLAIMABLE flag, and the dcache external names user is converted. > >>>> > >>>> The counter is however still useful for accounting direct page allocations > >>>> (i.e. not slab) with a shrinker, such as the ION page pool. So keep it, and: > >>> > >>> Btw, it looks like I've another example of usefulness of this counter: > >>> dynamic per-cpu data. > >> > >> Hmm, but are those reclaimable? Most likely not in general? Do you have > >> examples that are? > > > > If these per-cpu data is something like per-cpu refcounters, > > which are using to manage reclaimable objects (e.g. cgroup css objects). > > Of course, they are not always reclaimable, but in certain states. > > BTW, seems you seem interested, could you provide some more formal > review as well? Others too. We don't need to cover all use cases > immediately, when the patchset is apparently stalled due to lack of > review. Thanks! Sure! The patchset looks sane at a first glance, but I need some time to dig deeper. Is v2 the final version? Thanks!
On 07/17/2018 08:54 PM, Roman Gushchin wrote: > On Tue, Jul 17, 2018 at 10:44:07AM +0200, Vlastimil Babka wrote: >> On 07/02/2018 06:52 PM, Roman Gushchin wrote: >>> On Sat, Jun 30, 2018 at 12:09:27PM +0200, Vlastimil Babka wrote: >>> >>> If these per-cpu data is something like per-cpu refcounters, >>> which are using to manage reclaimable objects (e.g. cgroup css objects). >>> Of course, they are not always reclaimable, but in certain states. >> >> BTW, seems you seem interested, could you provide some more formal >> review as well? Others too. We don't need to cover all use cases >> immediately, when the patchset is apparently stalled due to lack of >> review. Thanks! > > Sure! Thanks! > The patchset looks sane at a first glance, but I need some time > to dig deeper. Is v2 the final version? There was a fixlet on top and some added changelog text, so I'll do a v3 tomorrow incorporating that to make things easier for everyone. > Thanks! >
diff --git a/drivers/staging/android/ion/ion_page_pool.c b/drivers/staging/android/ion/ion_page_pool.c index 9bc56eb48d2a..0d2a95957ee8 100644 --- a/drivers/staging/android/ion/ion_page_pool.c +++ b/drivers/staging/android/ion/ion_page_pool.c @@ -33,8 +33,8 @@ static void ion_page_pool_add(struct ion_page_pool *pool, struct page *page) pool->low_count++; } - mod_node_page_state(page_pgdat(page), NR_INDIRECTLY_RECLAIMABLE_BYTES, - (1 << (PAGE_SHIFT + pool->order))); + mod_node_page_state(page_pgdat(page), NR_KERNEL_MISC_RECLAIMABLE, + 1 << pool->order); mutex_unlock(&pool->mutex); } @@ -53,8 +53,8 @@ static struct page *ion_page_pool_remove(struct ion_page_pool *pool, bool high) } list_del(&page->lru); - mod_node_page_state(page_pgdat(page), NR_INDIRECTLY_RECLAIMABLE_BYTES, - -(1 << (PAGE_SHIFT + pool->order))); + mod_node_page_state(page_pgdat(page), NR_KERNEL_MISC_RECLAIMABLE, + -(1 << pool->order)); return page; } diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h index 32699b2dc52a..c2f6bc4c9e8a 100644 --- a/include/linux/mmzone.h +++ b/include/linux/mmzone.h @@ -180,7 +180,7 @@ enum node_stat_item { NR_VMSCAN_IMMEDIATE, /* Prioritise for reclaim when writeback ends */ NR_DIRTIED, /* page dirtyings since bootup */ NR_WRITTEN, /* page writings since bootup */ - NR_INDIRECTLY_RECLAIMABLE_BYTES, /* measured in bytes */ + NR_KERNEL_MISC_RECLAIMABLE, /* reclaimable non-slab kernel pages */ NR_VM_NODE_STAT_ITEMS }; diff --git a/mm/page_alloc.c b/mm/page_alloc.c index 1521100f1e63..8ceb45e11b97 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -4704,6 +4704,7 @@ long si_mem_available(void) unsigned long pagecache; unsigned long wmark_low = 0; unsigned long pages[NR_LRU_LISTS]; + unsigned long reclaimable; struct zone *zone; int lru; @@ -4729,19 +4730,13 @@ long si_mem_available(void) available += pagecache; /* - * Part of the reclaimable slab consists of items that are in use, - * and cannot be freed. Cap this estimate at the low watermark. + * Part of the reclaimable slab and other kernel memory consists of + * items that are in use, and cannot be freed. Cap this estimate at the + * low watermark. */ - available += global_node_page_state(NR_SLAB_RECLAIMABLE) - - min(global_node_page_state(NR_SLAB_RECLAIMABLE) / 2, - wmark_low); - - /* - * Part of the kernel memory, which can be released under memory - * pressure. - */ - available += global_node_page_state(NR_INDIRECTLY_RECLAIMABLE_BYTES) >> - PAGE_SHIFT; + reclaimable = global_node_page_state(NR_SLAB_RECLAIMABLE) + + global_node_page_state(NR_KERNEL_MISC_RECLAIMABLE); + available += reclaimable - min(reclaimable / 2, wmark_low); if (available < 0) available = 0; diff --git a/mm/util.c b/mm/util.c index 3351659200e6..891f0654e7b5 100644 --- a/mm/util.c +++ b/mm/util.c @@ -675,8 +675,7 @@ int __vm_enough_memory(struct mm_struct *mm, long pages, int cap_sys_admin) * Part of the kernel memory, which can be released * under memory pressure. */ - free += global_node_page_state( - NR_INDIRECTLY_RECLAIMABLE_BYTES) >> PAGE_SHIFT; + free += global_node_page_state(NR_KERNEL_MISC_RECLAIMABLE); /* * Leave reserved pages. The pages are not for anonymous pages. diff --git a/mm/vmstat.c b/mm/vmstat.c index 75eda9c2b260..7c677d3a61ec 100644 --- a/mm/vmstat.c +++ b/mm/vmstat.c @@ -1161,7 +1161,7 @@ const char * const vmstat_text[] = { "nr_vmscan_immediate_reclaim", "nr_dirtied", "nr_written", - "", /* nr_indirectly_reclaimable */ + "nr_kernel_misc_reclaimable", /* enum writeback_stat_item counters */ "nr_dirty_threshold", @@ -1704,10 +1704,6 @@ static int vmstat_show(struct seq_file *m, void *arg) unsigned long *l = arg; unsigned long off = l - (unsigned long *)m->private; - /* Skip hidden vmstat items. */ - if (*vmstat_text[off] == '\0') - return 0; - seq_puts(m, vmstat_text[off]); seq_put_decimal_ull(m, " ", *l); seq_putc(m, '\n');