Re: [PATCH] memcg: localize memcg_kmem_enabled() check

[kbuild test robot]

Hi Shakeel,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v4.20 next-20190102]
[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/Shakeel-Butt/memcg-localize-memcg_kmem_enabled-check/20190103-120255
config: x86_64-randconfig-x013-201900 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 

All errors (new ones prefixed by >>):

   mm/page_alloc.c: In function 'free_pages_prepare':
>> mm/page_alloc.c:1059:3: error: implicit declaration of function 
>> '__memcg_kmem_uncharge'; did you mean 'memcg_kmem_uncharge'? 
>> [-Werror=implicit-function-declaration]
  __memcg_kmem_uncharge(page, order);
  ^
  memcg_kmem_uncharge
   In file included from include/asm-generic/bug.h:5:0,
from arch/x86/include/asm/bug.h:83,
from include/linux/bug.h:5,
from include/linux/mmdebug.h:5,
from include/linux/mm.h:9,
from mm/page_alloc.c:18:
   mm/page_alloc.c: In function '__alloc_pages_nodemask':
>> mm/page_alloc.c:4553:15: error: implicit declaration of function 
>> '__memcg_kmem_charge'; did you mean 'memcg_kmem_charge'? 
>> [-Werror=implicit-function-declaration]
 unlikely(__memcg_kmem_charge(page, gfp_mask, order) != 0)) {
  ^
   include/linux/compiler.h:77:42: note: in definition of macro 'unlikely'
# define unlikely(x) __builtin_expect(!!(x), 0)
 ^
   cc1: some warnings being treated as errors

vim +1059 mm/page_alloc.c

  1024  
  1025  static __always_inline bool free_pages_prepare(struct page *page,
  1026  unsigned int order, bool 
check_free)
  1027  {
  1028  int bad = 0;
  1029  
  1030  VM_BUG_ON_PAGE(PageTail(page), page);
  1031  
  1032  trace_mm_page_free(page, order);
  1033  
  1034  /*
  1035   * Check tail pages before head page information is cleared to
  1036   * avoid checking PageCompound for order-0 pages.
  1037   */
  1038  if (unlikely(order)) {
  1039  bool compound = PageCompound(page);
  1040  int i;
  1041  
  1042  VM_BUG_ON_PAGE(compound && compound_order(page) != 
order, page);
  1043  
  1044  if (compound)
  1045  ClearPageDoubleMap(page);
  1046  for (i = 1; i < (1 << order); i++) {
  1047  if (compound)
  1048  bad += free_tail_pages_check(page, page 
+ i);
  1049  if (unlikely(free_pages_check(page + i))) {
  1050  bad++;
  1051  continue;
  1052  }
  1053  (page + i)->flags &= ~PAGE_FLAGS_CHECK_AT_PREP;
  1054  }
  1055  }
  1056  if (PageMappingFlags(page))
  1057  page->mapping = NULL;
  1058  if (memcg_kmem_enabled() && PageKmemcg(page))
> 1059  __memcg_kmem_uncharge(page, order);
  1060  if (check_free)
  1061  bad += free_pages_check(page);
  1062  if (bad)
  1063  return false;
  1064  
  1065  page_cpupid_reset_last(page);
  1066  page->flags &= ~PAGE_FLAGS_CHECK_AT_PREP;
  1067  reset_page_owner(page, order);
  1068  
  1069  if (!PageHighMem(page)) {
  1070  debug_check_no_locks_freed(page_address(page),
  1071 PAGE_SIZE << order);
  1072  debug_check_no_obj_freed(page_address(page),
  1073 PAGE_SIZE << order);
  1074  }
  1075  arch_free_page(page, order);
  1076  kernel_poison_pages(page, 1 << order, 0);
  1077  kernel_map_pages(page, 1 << order, 0);
  1078  kasan_free_nondeferred_pages(page, order);
  1079  
  1080  return true;
  1081  }
  1082  

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


.config.gz
Description: application/gzip

[PATCH] memcg: localize memcg_kmem_enabled() check

[Shakeel Butt]

Move the memcg_kmem_enabled() checks into memcg kmem charge/uncharge
functions, so, the users don't have to explicitly check that condition.
This is purely code cleanup patch without any functional change. Only
the order of checks in memcg_charge_slab() can potentially be changed
but the functionally it will be same. This should not matter as
memcg_charge_slab() is not in the hot path.

Signed-off-by: Shakeel Butt 
---
 fs/pipe.c  |  3 +--
 include/linux/memcontrol.h | 28 
 mm/memcontrol.c| 16 
 mm/page_alloc.c|  4 ++--
 mm/slab.h  |  4 
 5 files changed, 35 insertions(+), 20 deletions(-)

diff --git a/fs/pipe.c b/fs/pipe.c
index bdc5d3c0977d..51d5fd8840ab 100644
--- a/fs/pipe.c
+++ b/fs/pipe.c
@@ -140,8 +140,7 @@ static int anon_pipe_buf_steal(struct pipe_inode_info *pipe,
struct page *page = buf->page;
 
if (page_count(page) == 1) {
-   if (memcg_kmem_enabled())
-   memcg_kmem_uncharge(page, 0);
+   memcg_kmem_uncharge(page, 0);
__SetPageLocked(page);
return 0;
}
diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
index 83ae11cbd12c..e264d5c28781 100644
--- a/include/linux/memcontrol.h
+++ b/include/linux/memcontrol.h
@@ -1273,12 +1273,12 @@ static inline bool 
mem_cgroup_under_socket_pressure(struct mem_cgroup *memcg)
 
 struct kmem_cache *memcg_kmem_get_cache(struct kmem_cache *cachep);
 void memcg_kmem_put_cache(struct kmem_cache *cachep);
-int memcg_kmem_charge_memcg(struct page *page, gfp_t gfp, int order,
-   struct mem_cgroup *memcg);
 
 #ifdef CONFIG_MEMCG_KMEM
-int memcg_kmem_charge(struct page *page, gfp_t gfp, int order);
-void memcg_kmem_uncharge(struct page *page, int order);
+int __memcg_kmem_charge(struct page *page, gfp_t gfp, int order);
+void __memcg_kmem_uncharge(struct page *page, int order);
+int __memcg_kmem_charge_memcg(struct page *page, gfp_t gfp, int order,
+ struct mem_cgroup *memcg);
 
 extern struct static_key_false memcg_kmem_enabled_key;
 extern struct workqueue_struct *memcg_kmem_cache_wq;
@@ -1300,6 +1300,26 @@ static inline bool memcg_kmem_enabled(void)
return static_branch_unlikely(_kmem_enabled_key);
 }
 
+static inline int memcg_kmem_charge(struct page *page, gfp_t gfp, int order)
+{
+   if (memcg_kmem_enabled())
+   return __memcg_kmem_charge(page, gfp, order);
+   return 0;
+}
+
+static inline void memcg_kmem_uncharge(struct page *page, int order)
+{
+   if (memcg_kmem_enabled())
+   __memcg_kmem_uncharge(page, order);
+}
+
+static inline int memcg_kmem_charge_memcg(struct page *page, gfp_t gfp,
+ int order, struct mem_cgroup *memcg)
+{
+   if (memcg_kmem_enabled())
+   return __memcg_kmem_charge_memcg(page, gfp, order, memcg);
+   return 0;
+}
 /*
  * helper for accessing a memcg's index. It will be used as an index in the
  * child cache array in kmem_cache, and also to derive its name. This function
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 4afd5971f2d4..e8ca09920d71 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -2557,7 +2557,7 @@ void memcg_kmem_put_cache(struct kmem_cache *cachep)
 }
 
 /**
- * memcg_kmem_charge_memcg: charge a kmem page
+ * __memcg_kmem_charge_memcg: charge a kmem page
  * @page: page to charge
  * @gfp: reclaim mode
  * @order: allocation order
@@ -2565,7 +2565,7 @@ void memcg_kmem_put_cache(struct kmem_cache *cachep)
  *
  * Returns 0 on success, an error code on failure.
  */
-int memcg_kmem_charge_memcg(struct page *page, gfp_t gfp, int order,
+int __memcg_kmem_charge_memcg(struct page *page, gfp_t gfp, int order,
struct mem_cgroup *memcg)
 {
unsigned int nr_pages = 1 << order;
@@ -2588,24 +2588,24 @@ int memcg_kmem_charge_memcg(struct page *page, gfp_t 
gfp, int order,
 }
 
 /**
- * memcg_kmem_charge: charge a kmem page to the current memory cgroup
+ * __memcg_kmem_charge: charge a kmem page to the current memory cgroup
  * @page: page to charge
  * @gfp: reclaim mode
  * @order: allocation order
  *
  * Returns 0 on success, an error code on failure.
  */
-int memcg_kmem_charge(struct page *page, gfp_t gfp, int order)
+int __memcg_kmem_charge(struct page *page, gfp_t gfp, int order)
 {
struct mem_cgroup *memcg;
int ret = 0;
 
-   if (mem_cgroup_disabled() || memcg_kmem_bypass())
+   if (memcg_kmem_bypass())
return 0;
 
memcg = get_mem_cgroup_from_current();
if (!mem_cgroup_is_root(memcg)) {
-   ret = memcg_kmem_charge_memcg(page, gfp, order, memcg);
+   ret = __memcg_kmem_charge_memcg(page, gfp, order, memcg);
if (!ret)
__SetPageKmemcg(page);
}
@@ -2613,11 +2613,11 @@ int