All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] mm: memcontrol: switch to rstat fix
@ 2021-03-15 23:41 Johannes Weiner
  2021-03-15 23:58 ` Shakeel Butt
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Johannes Weiner @ 2021-03-15 23:41 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Michal Hocko, Roman Gushchin, Shakeel Butt, Tejun Heo,
	Michal Koutný,
	cgroups, linux-mm, linux-kernel, kernel-team

Fix a sleep in atomic section problem: wb_writeback() takes a spinlock
and calls wb_over_bg_thresh() -> mem_cgroup_wb_stats, but the regular
rstat flushing function called from in there does lockbreaking and may
sleep. Switch to the atomic variant, cgroup_rstat_irqsafe().

To be consistent with other memcg flush calls, but without adding
another memcg wrapper, inline and drop memcg_flush_vmstats() instead.

Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
---
 mm/memcontrol.c | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index f7fb12d3c2fc..9091913ec877 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -757,11 +757,6 @@ mem_cgroup_largest_soft_limit_node(struct mem_cgroup_tree_per_node *mctz)
 	return mz;
 }
 
-static void memcg_flush_vmstats(struct mem_cgroup *memcg)
-{
-	cgroup_rstat_flush(memcg->css.cgroup);
-}
-
 /**
  * __mod_memcg_state - update cgroup memory statistics
  * @memcg: the memory cgroup
@@ -1572,7 +1567,7 @@ static char *memory_stat_format(struct mem_cgroup *memcg)
 	 *
 	 * Current memory state:
 	 */
-	memcg_flush_vmstats(memcg);
+	cgroup_rstat_flush(memcg->css.cgroup);
 
 	for (i = 0; i < ARRAY_SIZE(memory_stats); i++) {
 		u64 size;
@@ -3523,7 +3518,7 @@ static unsigned long mem_cgroup_usage(struct mem_cgroup *memcg, bool swap)
 	unsigned long val;
 
 	if (mem_cgroup_is_root(memcg)) {
-		memcg_flush_vmstats(memcg);
+		cgroup_rstat_flush(memcg->css.cgroup);
 		val = memcg_page_state(memcg, NR_FILE_PAGES) +
 			memcg_page_state(memcg, NR_ANON_MAPPED);
 		if (swap)
@@ -3925,7 +3920,7 @@ static int memcg_numa_stat_show(struct seq_file *m, void *v)
 	int nid;
 	struct mem_cgroup *memcg = mem_cgroup_from_seq(m);
 
-	memcg_flush_vmstats(memcg);
+	cgroup_rstat_flush(memcg->css.cgroup);
 
 	for (stat = stats; stat < stats + ARRAY_SIZE(stats); stat++) {
 		seq_printf(m, "%s=%lu", stat->name,
@@ -3997,7 +3992,7 @@ static int memcg_stat_show(struct seq_file *m, void *v)
 
 	BUILD_BUG_ON(ARRAY_SIZE(memcg1_stat_names) != ARRAY_SIZE(memcg1_stats));
 
-	memcg_flush_vmstats(memcg);
+	cgroup_rstat_flush(memcg->css.cgroup);
 
 	for (i = 0; i < ARRAY_SIZE(memcg1_stats); i++) {
 		unsigned long nr;
@@ -4500,7 +4495,7 @@ void mem_cgroup_wb_stats(struct bdi_writeback *wb, unsigned long *pfilepages,
 	struct mem_cgroup *memcg = mem_cgroup_from_css(wb->memcg_css);
 	struct mem_cgroup *parent;
 
-	memcg_flush_vmstats(memcg);
+	cgroup_rstat_flush_irqsafe(memcg->css.cgroup);
 
 	*pdirty = memcg_page_state(memcg, NR_FILE_DIRTY);
 	*pwriteback = memcg_page_state(memcg, NR_WRITEBACK);
-- 
2.30.1


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

* Re: [PATCH] mm: memcontrol: switch to rstat fix
  2021-03-15 23:41 [PATCH] mm: memcontrol: switch to rstat fix Johannes Weiner
@ 2021-03-15 23:58 ` Shakeel Butt
  2021-03-16  0:52 ` Singh, Balbir
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Shakeel Butt @ 2021-03-15 23:58 UTC (permalink / raw)
  To: Johannes Weiner
  Cc: Andrew Morton, Michal Hocko, Roman Gushchin, Tejun Heo,
	Michal Koutný,
	Cgroups, Linux MM, LKML, Kernel Team

On Mon, Mar 15, 2021 at 4:41 PM Johannes Weiner <hannes@cmpxchg.org> wrote:
>
> Fix a sleep in atomic section problem: wb_writeback() takes a spinlock
> and calls wb_over_bg_thresh() -> mem_cgroup_wb_stats, but the regular
> rstat flushing function called from in there does lockbreaking and may
> sleep. Switch to the atomic variant, cgroup_rstat_irqsafe().
>
> To be consistent with other memcg flush calls, but without adding
> another memcg wrapper, inline and drop memcg_flush_vmstats() instead.
>
> Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>

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

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

* Re: [PATCH] mm: memcontrol: switch to rstat fix
  2021-03-15 23:41 [PATCH] mm: memcontrol: switch to rstat fix Johannes Weiner
  2021-03-15 23:58 ` Shakeel Butt
@ 2021-03-16  0:52 ` Singh, Balbir
  2021-03-16  9:30 ` Michal Hocko
  2021-03-17 19:19 ` Michal Koutný
  3 siblings, 0 replies; 5+ messages in thread
From: Singh, Balbir @ 2021-03-16  0:52 UTC (permalink / raw)
  To: Johannes Weiner, Andrew Morton
  Cc: Michal Hocko, Roman Gushchin, Shakeel Butt, Tejun Heo,
	Michal Koutný,
	cgroups, linux-mm, linux-kernel, kernel-team

On 16/3/21 10:41 am, Johannes Weiner wrote:
> Fix a sleep in atomic section problem: wb_writeback() takes a spinlock
> and calls wb_over_bg_thresh() -> mem_cgroup_wb_stats, but the regular
> rstat flushing function called from in there does lockbreaking and may
> sleep. Switch to the atomic variant, cgroup_rstat_irqsafe().
> 
> To be consistent with other memcg flush calls, but without adding
> another memcg wrapper, inline and drop memcg_flush_vmstats() instead.
> 
> Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
> ---

The patch make sense, but it does break any notion of abstraction we had
about controllers have some independence in their strategy to maintain
their own counters and stats. It now couples writeback with rstat instead
of just memcg.

Acked-by: Balbir Singh <bsingharora@gmail.com>

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

* Re: [PATCH] mm: memcontrol: switch to rstat fix
  2021-03-15 23:41 [PATCH] mm: memcontrol: switch to rstat fix Johannes Weiner
  2021-03-15 23:58 ` Shakeel Butt
  2021-03-16  0:52 ` Singh, Balbir
@ 2021-03-16  9:30 ` Michal Hocko
  2021-03-17 19:19 ` Michal Koutný
  3 siblings, 0 replies; 5+ messages in thread
From: Michal Hocko @ 2021-03-16  9:30 UTC (permalink / raw)
  To: Johannes Weiner
  Cc: Andrew Morton, Roman Gushchin, Shakeel Butt, Tejun Heo,
	Michal Koutný,
	cgroups, linux-mm, linux-kernel, kernel-team

On Mon 15-03-21 19:41:00, Johannes Weiner wrote:
> Fix a sleep in atomic section problem: wb_writeback() takes a spinlock
> and calls wb_over_bg_thresh() -> mem_cgroup_wb_stats, but the regular
> rstat flushing function called from in there does lockbreaking and may
> sleep. Switch to the atomic variant, cgroup_rstat_irqsafe().
> 
> To be consistent with other memcg flush calls, but without adding
> another memcg wrapper, inline and drop memcg_flush_vmstats() instead.
> 
> Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>

Acked-by: Michal Hocko <mhocko@suse.com>

> ---
>  mm/memcontrol.c | 15 +++++----------
>  1 file changed, 5 insertions(+), 10 deletions(-)
> 
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index f7fb12d3c2fc..9091913ec877 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -757,11 +757,6 @@ mem_cgroup_largest_soft_limit_node(struct mem_cgroup_tree_per_node *mctz)
>  	return mz;
>  }
>  
> -static void memcg_flush_vmstats(struct mem_cgroup *memcg)
> -{
> -	cgroup_rstat_flush(memcg->css.cgroup);
> -}
> -
>  /**
>   * __mod_memcg_state - update cgroup memory statistics
>   * @memcg: the memory cgroup
> @@ -1572,7 +1567,7 @@ static char *memory_stat_format(struct mem_cgroup *memcg)
>  	 *
>  	 * Current memory state:
>  	 */
> -	memcg_flush_vmstats(memcg);
> +	cgroup_rstat_flush(memcg->css.cgroup);
>  
>  	for (i = 0; i < ARRAY_SIZE(memory_stats); i++) {
>  		u64 size;
> @@ -3523,7 +3518,7 @@ static unsigned long mem_cgroup_usage(struct mem_cgroup *memcg, bool swap)
>  	unsigned long val;
>  
>  	if (mem_cgroup_is_root(memcg)) {
> -		memcg_flush_vmstats(memcg);
> +		cgroup_rstat_flush(memcg->css.cgroup);
>  		val = memcg_page_state(memcg, NR_FILE_PAGES) +
>  			memcg_page_state(memcg, NR_ANON_MAPPED);
>  		if (swap)
> @@ -3925,7 +3920,7 @@ static int memcg_numa_stat_show(struct seq_file *m, void *v)
>  	int nid;
>  	struct mem_cgroup *memcg = mem_cgroup_from_seq(m);
>  
> -	memcg_flush_vmstats(memcg);
> +	cgroup_rstat_flush(memcg->css.cgroup);
>  
>  	for (stat = stats; stat < stats + ARRAY_SIZE(stats); stat++) {
>  		seq_printf(m, "%s=%lu", stat->name,
> @@ -3997,7 +3992,7 @@ static int memcg_stat_show(struct seq_file *m, void *v)
>  
>  	BUILD_BUG_ON(ARRAY_SIZE(memcg1_stat_names) != ARRAY_SIZE(memcg1_stats));
>  
> -	memcg_flush_vmstats(memcg);
> +	cgroup_rstat_flush(memcg->css.cgroup);
>  
>  	for (i = 0; i < ARRAY_SIZE(memcg1_stats); i++) {
>  		unsigned long nr;
> @@ -4500,7 +4495,7 @@ void mem_cgroup_wb_stats(struct bdi_writeback *wb, unsigned long *pfilepages,
>  	struct mem_cgroup *memcg = mem_cgroup_from_css(wb->memcg_css);
>  	struct mem_cgroup *parent;
>  
> -	memcg_flush_vmstats(memcg);
> +	cgroup_rstat_flush_irqsafe(memcg->css.cgroup);
>  
>  	*pdirty = memcg_page_state(memcg, NR_FILE_DIRTY);
>  	*pwriteback = memcg_page_state(memcg, NR_WRITEBACK);
> -- 
> 2.30.1

-- 
Michal Hocko
SUSE Labs

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

* Re: [PATCH] mm: memcontrol: switch to rstat fix
  2021-03-15 23:41 [PATCH] mm: memcontrol: switch to rstat fix Johannes Weiner
                   ` (2 preceding siblings ...)
  2021-03-16  9:30 ` Michal Hocko
@ 2021-03-17 19:19 ` Michal Koutný
  3 siblings, 0 replies; 5+ messages in thread
From: Michal Koutný @ 2021-03-17 19:19 UTC (permalink / raw)
  To: Johannes Weiner
  Cc: Andrew Morton, Michal Hocko, Roman Gushchin, Shakeel Butt,
	Tejun Heo, cgroups, linux-mm, linux-kernel, kernel-team

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

On Mon, Mar 15, 2021 at 07:41:00PM -0400, Johannes Weiner <hannes@cmpxchg.org> wrote:
> Switch to the atomic variant, cgroup_rstat_irqsafe().
Congratulations(?), the first use of cgroup_rstat_irqsafe().

Reviewed-by: Michal Koutný <mkoutny@suse.com>

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

end of thread, other threads:[~2021-03-17 19:20 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-15 23:41 [PATCH] mm: memcontrol: switch to rstat fix Johannes Weiner
2021-03-15 23:58 ` Shakeel Butt
2021-03-16  0:52 ` Singh, Balbir
2021-03-16  9:30 ` Michal Hocko
2021-03-17 19:19 ` Michal Koutný

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.