[PATCH 1/3] mm: memcg/slab: remove unused argument by charge_slab_page()

[PATCH 1/3] mm: memcg/slab: remove unused argument by charge_slab_page()

[Roman Gushchin]

charge_slab_page() is not using the gfp argument anymore,
remove it.

Signed-off-by: Roman Gushchin <guro@fb.com>
---
 mm/slab.c | 2 +-
 mm/slab.h | 3 +--
 mm/slub.c | 2 +-
 3 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/mm/slab.c b/mm/slab.c
index 2850fe3c5fb8..fafd46877504 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -1379,7 +1379,7 @@ static struct page *kmem_getpages(struct kmem_cache *cachep, gfp_t flags,
 		return NULL;
 	}
 
-	charge_slab_page(page, flags, cachep->gfporder, cachep);
+	charge_slab_page(page, cachep->gfporder, cachep);
 	__SetPageSlab(page);
 	/* Record if ALLOC_NO_WATERMARKS was set when allocating the slab */
 	if (sk_memalloc_socks() && page_is_pfmemalloc(page))
diff --git a/mm/slab.h b/mm/slab.h
index ab172dca8ce2..704a65713f81 100644
--- a/mm/slab.h
+++ b/mm/slab.h
@@ -440,8 +440,7 @@ static inline struct kmem_cache *virt_to_cache(const void *obj)
 	return page->slab_cache;
 }
 
-static __always_inline void charge_slab_page(struct page *page,
-					     gfp_t gfp, int order,
+static __always_inline void charge_slab_page(struct page *page, int order,
 					     struct kmem_cache *s)
 {
 	mod_node_page_state(page_pgdat(page), cache_vmstat_idx(s),
diff --git a/mm/slub.c b/mm/slub.c
index 6769f5f802e9..d9b33a935e58 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -1621,7 +1621,7 @@ static inline struct page *alloc_slab_page(struct kmem_cache *s,
 		page = __alloc_pages_node(node, flags, order);
 
 	if (page)
-		charge_slab_page(page, flags, order, s);
+		charge_slab_page(page, order, s);
 
 	return page;
 }
-- 
2.26.2

[PATCH 2/3] mm: slab: rename (un)charge_slab_page() to (un)account_slab_page()

[Roman Gushchin]

charge_slab_page() and uncharge_slab_page() are not related anymore
to memcg charging and uncharging. In order to make their names
less confusing, let's rename them to account_slab_page() and
unaccount_slab_page() respectively.

Signed-off-by: Roman Gushchin <guro@fb.com>
---
 mm/slab.c | 4 ++--
 mm/slab.h | 8 ++++----
 mm/slub.c | 4 ++--
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/mm/slab.c b/mm/slab.c
index fafd46877504..300adfb67245 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -1379,7 +1379,7 @@ static struct page *kmem_getpages(struct kmem_cache *cachep, gfp_t flags,
 		return NULL;
 	}
 
-	charge_slab_page(page, cachep->gfporder, cachep);
+	account_slab_page(page, cachep->gfporder, cachep);
 	__SetPageSlab(page);
 	/* Record if ALLOC_NO_WATERMARKS was set when allocating the slab */
 	if (sk_memalloc_socks() && page_is_pfmemalloc(page))
@@ -1403,7 +1403,7 @@ static void kmem_freepages(struct kmem_cache *cachep, struct page *page)
 
 	if (current->reclaim_state)
 		current->reclaim_state->reclaimed_slab += 1 << order;
-	uncharge_slab_page(page, order, cachep);
+	unaccount_slab_page(page, order, cachep);
 	__free_pages(page, order);
 }
 
diff --git a/mm/slab.h b/mm/slab.h
index 704a65713f81..72ff7cd933db 100644
--- a/mm/slab.h
+++ b/mm/slab.h
@@ -440,15 +440,15 @@ static inline struct kmem_cache *virt_to_cache(const void *obj)
 	return page->slab_cache;
 }
 
-static __always_inline void charge_slab_page(struct page *page, int order,
-					     struct kmem_cache *s)
+static __always_inline void account_slab_page(struct page *page, int order,
+					      struct kmem_cache *s)
 {
 	mod_node_page_state(page_pgdat(page), cache_vmstat_idx(s),
 			    PAGE_SIZE << order);
 }
 
-static __always_inline void uncharge_slab_page(struct page *page, int order,
-					       struct kmem_cache *s)
+static __always_inline void unaccount_slab_page(struct page *page, int order,
+						struct kmem_cache *s)
 {
 	if (memcg_kmem_enabled())
 		memcg_free_page_obj_cgroups(page);
diff --git a/mm/slub.c b/mm/slub.c
index d9b33a935e58..5cffd3fd440b 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -1621,7 +1621,7 @@ static inline struct page *alloc_slab_page(struct kmem_cache *s,
 		page = __alloc_pages_node(node, flags, order);
 
 	if (page)
-		charge_slab_page(page, order, s);
+		account_slab_page(page, order, s);
 
 	return page;
 }
@@ -1844,7 +1844,7 @@ static void __free_slab(struct kmem_cache *s, struct page *page)
 	page->mapping = NULL;
 	if (current->reclaim_state)
 		current->reclaim_state->reclaimed_slab += pages;
-	uncharge_slab_page(page, order, s);
+	unaccount_slab_page(page, order, s);
 	__free_pages(page, order);
 }
 
-- 
2.26.2

Re: [PATCH 2/3] mm: slab: rename (un)charge_slab_page() to (un)account_slab_page()

[Shakeel Butt]

On Tue, Jul 7, 2020 at 10:36 AM Roman Gushchin <guro@fb.com> wrote:
>
> charge_slab_page() and uncharge_slab_page() are not related anymore
> to memcg charging and uncharging. In order to make their names
> less confusing, let's rename them to account_slab_page() and
> unaccount_slab_page() respectively.
>
> Signed-off-by: Roman Gushchin <guro@fb.com>

Reviewed-by: Shakeel Butt <shakeelb@google.com>

Re: [PATCH 2/3] mm: slab: rename (un)charge_slab_page() to (un)account_slab_page()

[Vlastimil Babka]

On 7/7/20 7:36 PM, Roman Gushchin wrote:
> charge_slab_page() and uncharge_slab_page() are not related anymore
> to memcg charging and uncharging. In order to make their names
> less confusing, let's rename them to account_slab_page() and
> unaccount_slab_page() respectively.
> 
> Signed-off-by: Roman Gushchin <guro@fb.com>

Acked-by: Vlastimil Babka <vbabka@suse.cz>

> ---
>  mm/slab.c | 4 ++--
>  mm/slab.h | 8 ++++----
>  mm/slub.c | 4 ++--
>  3 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/mm/slab.c b/mm/slab.c
> index fafd46877504..300adfb67245 100644
> --- a/mm/slab.c
> +++ b/mm/slab.c
> @@ -1379,7 +1379,7 @@ static struct page *kmem_getpages(struct kmem_cache *cachep, gfp_t flags,
>  		return NULL;
>  	}
>  
> -	charge_slab_page(page, cachep->gfporder, cachep);
> +	account_slab_page(page, cachep->gfporder, cachep);
>  	__SetPageSlab(page);
>  	/* Record if ALLOC_NO_WATERMARKS was set when allocating the slab */
>  	if (sk_memalloc_socks() && page_is_pfmemalloc(page))
> @@ -1403,7 +1403,7 @@ static void kmem_freepages(struct kmem_cache *cachep, struct page *page)
>  
>  	if (current->reclaim_state)
>  		current->reclaim_state->reclaimed_slab += 1 << order;
> -	uncharge_slab_page(page, order, cachep);
> +	unaccount_slab_page(page, order, cachep);
>  	__free_pages(page, order);
>  }
>  
> diff --git a/mm/slab.h b/mm/slab.h
> index 704a65713f81..72ff7cd933db 100644
> --- a/mm/slab.h
> +++ b/mm/slab.h
> @@ -440,15 +440,15 @@ static inline struct kmem_cache *virt_to_cache(const void *obj)
>  	return page->slab_cache;
>  }
>  
> -static __always_inline void charge_slab_page(struct page *page, int order,
> -					     struct kmem_cache *s)
> +static __always_inline void account_slab_page(struct page *page, int order,
> +					      struct kmem_cache *s)
>  {
>  	mod_node_page_state(page_pgdat(page), cache_vmstat_idx(s),
>  			    PAGE_SIZE << order);
>  }
>  
> -static __always_inline void uncharge_slab_page(struct page *page, int order,
> -					       struct kmem_cache *s)
> +static __always_inline void unaccount_slab_page(struct page *page, int order,
> +						struct kmem_cache *s)
>  {
>  	if (memcg_kmem_enabled())
>  		memcg_free_page_obj_cgroups(page);
> diff --git a/mm/slub.c b/mm/slub.c
> index d9b33a935e58..5cffd3fd440b 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -1621,7 +1621,7 @@ static inline struct page *alloc_slab_page(struct kmem_cache *s,
>  		page = __alloc_pages_node(node, flags, order);
>  
>  	if (page)
> -		charge_slab_page(page, order, s);
> +		account_slab_page(page, order, s);
>  
>  	return page;
>  }
> @@ -1844,7 +1844,7 @@ static void __free_slab(struct kmem_cache *s, struct page *page)
>  	page->mapping = NULL;
>  	if (current->reclaim_state)
>  		current->reclaim_state->reclaimed_slab += pages;
> -	uncharge_slab_page(page, order, s);
> +	unaccount_slab_page(page, order, s);
>  	__free_pages(page, order);
>  }
>  
> 

[PATCH 3/3] mm: kmem: switch to static_branch_likely() in memcg_kmem_enabled()

[Roman Gushchin]

Currently memcg_kmem_enabled() is optimized for the kernel memory
accounting being off. It was so for a long time, and arguably the
reason behind was that the kernel memory accounting was initially an
opt-in feature. However, now it's on by default on both cgroup v1
and cgroup v2, and it's on for all cgroups. So let's switch over
to static_branch_likely() to reflect this fact.

Unlikely there is a significant performance difference, as the cost
of a memory allocation and its accounting significantly exceeds the
cost of a jump. However, the conversion makes the code look more
logically.

Signed-off-by: Roman Gushchin <guro@fb.com>
---
 include/linux/memcontrol.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
index b8f52a3fed90..ab9322215b2e 100644
--- a/include/linux/memcontrol.h
+++ b/include/linux/memcontrol.h
@@ -1456,7 +1456,7 @@ void memcg_put_cache_ids(void);
 
 static inline bool memcg_kmem_enabled(void)
 {
-	return static_branch_unlikely(&memcg_kmem_enabled_key);
+	return static_branch_likely(&memcg_kmem_enabled_key);
 }
 
 static inline bool memcg_kmem_bypass(void)
-- 
2.26.2

Re: [PATCH 3/3] mm: kmem: switch to static_branch_likely() in memcg_kmem_enabled()

[Shakeel Butt]

On Tue, Jul 7, 2020 at 10:36 AM Roman Gushchin <guro@fb.com> wrote:
>
> Currently memcg_kmem_enabled() is optimized for the kernel memory
> accounting being off. It was so for a long time, and arguably the
> reason behind was that the kernel memory accounting was initially an
> opt-in feature. However, now it's on by default on both cgroup v1
> and cgroup v2, and it's on for all cgroups. So let's switch over
> to static_branch_likely() to reflect this fact.
>
> Unlikely there is a significant performance difference, as the cost
> of a memory allocation and its accounting significantly exceeds the
> cost of a jump. However, the conversion makes the code look more
> logically.
>
> Signed-off-by: Roman Gushchin <guro@fb.com>

Reviewed-by: Shakeel Butt <shakeelb@google.com>

Re: [PATCH 3/3] mm: kmem: switch to static_branch_likely() in memcg_kmem_enabled()

[Vlastimil Babka]

On 7/7/20 7:36 PM, Roman Gushchin wrote:
> Currently memcg_kmem_enabled() is optimized for the kernel memory
> accounting being off. It was so for a long time, and arguably the
> reason behind was that the kernel memory accounting was initially an
> opt-in feature. However, now it's on by default on both cgroup v1
> and cgroup v2, and it's on for all cgroups. So let's switch over
> to static_branch_likely() to reflect this fact.
> 
> Unlikely there is a significant performance difference, as the cost
> of a memory allocation and its accounting significantly exceeds the
> cost of a jump. However, the conversion makes the code look more
> logically.
> 
> Signed-off-by: Roman Gushchin <guro@fb.com>

Acked-by: Vlastimil Babka <vbabka@suse.cz>

> ---
>  include/linux/memcontrol.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
> index b8f52a3fed90..ab9322215b2e 100644
> --- a/include/linux/memcontrol.h
> +++ b/include/linux/memcontrol.h
> @@ -1456,7 +1456,7 @@ void memcg_put_cache_ids(void);
>  
>  static inline bool memcg_kmem_enabled(void)
>  {
> -	return static_branch_unlikely(&memcg_kmem_enabled_key);
> +	return static_branch_likely(&memcg_kmem_enabled_key);
>  }
>  
>  static inline bool memcg_kmem_bypass(void)
> 

Re: [PATCH 1/3] mm: memcg/slab: remove unused argument by charge_slab_page()

[Shakeel Butt]

On Tue, Jul 7, 2020 at 10:36 AM Roman Gushchin <guro@fb.com> wrote:
>
> charge_slab_page() is not using the gfp argument anymore,
> remove it.
>
> Signed-off-by: Roman Gushchin <guro@fb.com>

Reviewed-by: Shakeel Butt <shakeelb@google.com>

Re: [PATCH 1/3] mm: memcg/slab: remove unused argument by charge_slab_page()

[Vlastimil Babka]

On 7/7/20 7:36 PM, Roman Gushchin wrote:
> charge_slab_page() is not using the gfp argument anymore,
> remove it.
> 
> Signed-off-by: Roman Gushchin <guro@fb.com>

Acked-by: Vlastimil Babka <vbabka@suse.cz>

> ---
>  mm/slab.c | 2 +-
>  mm/slab.h | 3 +--
>  mm/slub.c | 2 +-
>  3 files changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/mm/slab.c b/mm/slab.c
> index 2850fe3c5fb8..fafd46877504 100644
> --- a/mm/slab.c
> +++ b/mm/slab.c
> @@ -1379,7 +1379,7 @@ static struct page *kmem_getpages(struct kmem_cache *cachep, gfp_t flags,
>  		return NULL;
>  	}
>  
> -	charge_slab_page(page, flags, cachep->gfporder, cachep);
> +	charge_slab_page(page, cachep->gfporder, cachep);
>  	__SetPageSlab(page);
>  	/* Record if ALLOC_NO_WATERMARKS was set when allocating the slab */
>  	if (sk_memalloc_socks() && page_is_pfmemalloc(page))
> diff --git a/mm/slab.h b/mm/slab.h
> index ab172dca8ce2..704a65713f81 100644
> --- a/mm/slab.h
> +++ b/mm/slab.h
> @@ -440,8 +440,7 @@ static inline struct kmem_cache *virt_to_cache(const void *obj)
>  	return page->slab_cache;
>  }
>  
> -static __always_inline void charge_slab_page(struct page *page,
> -					     gfp_t gfp, int order,
> +static __always_inline void charge_slab_page(struct page *page, int order,
>  					     struct kmem_cache *s)
>  {
>  	mod_node_page_state(page_pgdat(page), cache_vmstat_idx(s),
> diff --git a/mm/slub.c b/mm/slub.c
> index 6769f5f802e9..d9b33a935e58 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -1621,7 +1621,7 @@ static inline struct page *alloc_slab_page(struct kmem_cache *s,
>  		page = __alloc_pages_node(node, flags, order);
>  
>  	if (page)
> -		charge_slab_page(page, flags, order, s);
> +		charge_slab_page(page, order, s);
>  
>  	return page;
>  }
>