All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] memcg: simple cleanup of stats update functions
@ 2024-04-19 23:39 Shakeel Butt
  2024-04-20 13:23 ` Johannes Weiner
  0 siblings, 1 reply; 3+ messages in thread
From: Shakeel Butt @ 2024-04-19 23:39 UTC (permalink / raw
  To: Andrew Morton, Johannes Weiner, Michal Hocko, Roman Gushchin,
	Muchun Song
  Cc: linux-mm, linux-kernel

mod_memcg_lruvec_state() is never called from outside of memcontrol.c
and with always irq disabled. So, replace it with the irq disabled
version and add an assert that irq is disabled in the caller.

Similarly mod_objcg_state() is not called from outside of memcontrol.c,
so simply make it static.

Signed-off-by: Shakeel Butt <shakeel.butt@linux.dev>
---
 include/linux/memcontrol.h | 17 -----------------
 mm/memcontrol.c            | 15 +++++++--------
 mm/slab.h                  |  2 --
 3 files changed, 7 insertions(+), 27 deletions(-)

diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
index 8f332b4ae84c..9aba0d0462ca 100644
--- a/include/linux/memcontrol.h
+++ b/include/linux/memcontrol.h
@@ -1077,8 +1077,6 @@ static inline unsigned long lruvec_page_state_local(struct lruvec *lruvec,
 void mem_cgroup_flush_stats(struct mem_cgroup *memcg);
 void mem_cgroup_flush_stats_ratelimited(struct mem_cgroup *memcg);
 
-void __mod_memcg_lruvec_state(struct lruvec *lruvec, enum node_stat_item idx,
-			      int val);
 void __mod_lruvec_kmem_state(void *p, enum node_stat_item idx, int val);
 
 static inline void mod_lruvec_kmem_state(void *p, enum node_stat_item idx,
@@ -1091,16 +1089,6 @@ static inline void mod_lruvec_kmem_state(void *p, enum node_stat_item idx,
 	local_irq_restore(flags);
 }
 
-static inline void mod_memcg_lruvec_state(struct lruvec *lruvec,
-					  enum node_stat_item idx, int val)
-{
-	unsigned long flags;
-
-	local_irq_save(flags);
-	__mod_memcg_lruvec_state(lruvec, idx, val);
-	local_irq_restore(flags);
-}
-
 void __count_memcg_events(struct mem_cgroup *memcg, enum vm_event_item idx,
 			  unsigned long count);
 
@@ -1594,11 +1582,6 @@ static inline void mem_cgroup_flush_stats_ratelimited(struct mem_cgroup *memcg)
 {
 }
 
-static inline void __mod_memcg_lruvec_state(struct lruvec *lruvec,
-					    enum node_stat_item idx, int val)
-{
-}
-
 static inline void __mod_lruvec_kmem_state(void *p, enum node_stat_item idx,
 					   int val)
 {
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 7703ced535a3..c6b08e6d7499 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -837,8 +837,9 @@ static unsigned long memcg_page_state_local(struct mem_cgroup *memcg, int idx)
 	return x;
 }
 
-void __mod_memcg_lruvec_state(struct lruvec *lruvec, enum node_stat_item idx,
-			      int val)
+static void __mod_memcg_lruvec_state(struct lruvec *lruvec,
+				     enum node_stat_item idx,
+				     int val)
 {
 	struct mem_cgroup_per_node *pn;
 	struct mem_cgroup *memcg;
@@ -2983,10 +2984,6 @@ void mem_cgroup_commit_charge(struct folio *folio, struct mem_cgroup *memcg)
 
 #ifdef CONFIG_MEMCG_KMEM
 
-/*
- * mod_objcg_mlstate() may be called with irq enabled, so
- * mod_memcg_lruvec_state() should be used.
- */
 static inline void mod_objcg_mlstate(struct obj_cgroup *objcg,
 				     struct pglist_data *pgdat,
 				     enum node_stat_item idx, int nr)
@@ -2994,10 +2991,12 @@ static inline void mod_objcg_mlstate(struct obj_cgroup *objcg,
 	struct mem_cgroup *memcg;
 	struct lruvec *lruvec;
 
+	lockdep_assert_irqs_disabled();
+
 	rcu_read_lock();
 	memcg = obj_cgroup_memcg(objcg);
 	lruvec = mem_cgroup_lruvec(memcg, pgdat);
-	mod_memcg_lruvec_state(lruvec, idx, nr);
+	__mod_memcg_lruvec_state(lruvec, idx, nr);
 	rcu_read_unlock();
 }
 
@@ -3317,7 +3316,7 @@ void __memcg_kmem_uncharge_page(struct page *page, int order)
 	obj_cgroup_put(objcg);
 }
 
-void mod_objcg_state(struct obj_cgroup *objcg, struct pglist_data *pgdat,
+static void mod_objcg_state(struct obj_cgroup *objcg, struct pglist_data *pgdat,
 		     enum node_stat_item idx, int nr)
 {
 	struct memcg_stock_pcp *stock;
diff --git a/mm/slab.h b/mm/slab.h
index e32d9cf1077a..5f8f47c5bee0 100644
--- a/mm/slab.h
+++ b/mm/slab.h
@@ -578,8 +578,6 @@ bool __memcg_slab_post_alloc_hook(struct kmem_cache *s, struct list_lru *lru,
 				  gfp_t flags, size_t size, void **p);
 void __memcg_slab_free_hook(struct kmem_cache *s, struct slab *slab,
 			    void **p, int objects, struct slabobj_ext *obj_exts);
-void mod_objcg_state(struct obj_cgroup *objcg, struct pglist_data *pgdat,
-		     enum node_stat_item idx, int nr);
 #endif
 
 size_t __ksize(const void *objp);
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] memcg: simple cleanup of stats update functions
  2024-04-19 23:39 [PATCH] memcg: simple cleanup of stats update functions Shakeel Butt
@ 2024-04-20 13:23 ` Johannes Weiner
  2024-04-20 17:25   ` shakeel.butt
  0 siblings, 1 reply; 3+ messages in thread
From: Johannes Weiner @ 2024-04-20 13:23 UTC (permalink / raw
  To: Shakeel Butt
  Cc: Andrew Morton, Michal Hocko, Roman Gushchin, Muchun Song,
	linux-mm, linux-kernel

Hi Shakeel,

On Fri, Apr 19, 2024 at 04:39:49PM -0700, Shakeel Butt wrote:
> @@ -2983,10 +2984,6 @@ void mem_cgroup_commit_charge(struct folio *folio, struct mem_cgroup *memcg)
>  
>  #ifdef CONFIG_MEMCG_KMEM
>  
> -/*
> - * mod_objcg_mlstate() may be called with irq enabled, so
> - * mod_memcg_lruvec_state() should be used.
> - */
>  static inline void mod_objcg_mlstate(struct obj_cgroup *objcg,
>  				     struct pglist_data *pgdat,
>  				     enum node_stat_item idx, int nr)
> @@ -2994,10 +2991,12 @@ static inline void mod_objcg_mlstate(struct obj_cgroup *objcg,
>  	struct mem_cgroup *memcg;
>  	struct lruvec *lruvec;
>  
> +	lockdep_assert_irqs_disabled();
> +
>  	rcu_read_lock();
>  	memcg = obj_cgroup_memcg(objcg);
>  	lruvec = mem_cgroup_lruvec(memcg, pgdat);
> -	mod_memcg_lruvec_state(lruvec, idx, nr);
> +	__mod_memcg_lruvec_state(lruvec, idx, nr);
>  	rcu_read_unlock();
>  }

Best to rename it to __mod_objcg_mlstate() as well to follow the
naming pattern for whether caller or callee handles IRQ toggling?

Otherwise, looks great to me!

Acked-by: Johannes Weiner <hannes@cmpxchg.org>


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] memcg: simple cleanup of stats update functions
  2024-04-20 13:23 ` Johannes Weiner
@ 2024-04-20 17:25   ` shakeel.butt
  0 siblings, 0 replies; 3+ messages in thread
From: shakeel.butt @ 2024-04-20 17:25 UTC (permalink / raw
  To: Johannes Weiner
  Cc: Andrew Morton, Michal Hocko, Roman Gushchin, Muchun Song,
	linux-mm, linux-kernel

April 20, 2024 at 6:23 AM, "Johannes Weiner" <hannes@cmpxchg.org> wrote:



> 
> Hi Shakeel,
> 
> On Fri, Apr 19, 2024 at 04:39:49PM -0700, Shakeel Butt wrote:
> 
> > 
> > @@ -2983,10 +2984,6 @@ void mem_cgroup_commit_charge(struct folio *folio, struct mem_cgroup *memcg)
> > 
> >  
> > 
> >  #ifdef CONFIG_MEMCG_KMEM
> > 
> >  
> > 
> >  -/*
> > 
> >  - * mod_objcg_mlstate() may be called with irq enabled, so
> > 
> >  - * mod_memcg_lruvec_state() should be used.
> > 
> >  - */
> > 
> >  static inline void mod_objcg_mlstate(struct obj_cgroup *objcg,
> > 
> >  struct pglist_data *pgdat,
> > 
> >  enum node_stat_item idx, int nr)
> > 
> >  @@ -2994,10 +2991,12 @@ static inline void mod_objcg_mlstate(struct obj_cgroup *objcg,
> > 
> >  struct mem_cgroup *memcg;
> > 
> >  struct lruvec *lruvec;
> > 
> >  
> > 
> >  + lockdep_assert_irqs_disabled();
> > 
> >  +
> > 
> >  rcu_read_lock();
> > 
> >  memcg = obj_cgroup_memcg(objcg);
> > 
> >  lruvec = mem_cgroup_lruvec(memcg, pgdat);
> > 
> >  - mod_memcg_lruvec_state(lruvec, idx, nr);
> > 
> >  + __mod_memcg_lruvec_state(lruvec, idx, nr);
> > 
> >  rcu_read_unlock();
> > 
> >  }
> > 
> 
> Best to rename it to __mod_objcg_mlstate() as well to follow the
> 
> naming pattern for whether caller or callee handles IRQ toggling?

Will do shortly in v2.

> 
> Otherwise, looks great to me!
> 
> Acked-by: Johannes Weiner <hannes@cmpxchg.org>

Thanks a lot.

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2024-04-20 17:25 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-04-19 23:39 [PATCH] memcg: simple cleanup of stats update functions Shakeel Butt
2024-04-20 13:23 ` Johannes Weiner
2024-04-20 17:25   ` shakeel.butt

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.