Skip to content

Commit fcc0669

Browse files
shakeelbakpm00
authored andcommitted
memcg: skip cgroup_file_notify if spinning is not allowed
Generally memcg charging is allowed from all the contexts including NMI where even spinning on spinlock can cause locking issues. However one call chain was missed during the addition of memcg charging from any context support. That is try_charge_memcg() -> memcg_memory_event() -> cgroup_file_notify(). The possible function call tree under cgroup_file_notify() can acquire many different spin locks in spinning mode. Some of them are cgroup_file_kn_lock, kernfs_notify_lock, pool_workqeue's lock. So, let's just skip cgroup_file_notify() from memcg charging if the context does not allow spinning. Alternative approach was also explored where instead of skipping cgroup_file_notify(), we defer the memcg event processing to irq_work [1]. However it adds complexity and it was decided to keep things simple until we need more memcg events with !allow_spinning requirement. Link: https://lore.kernel.org/all/5qi2llyzf7gklncflo6gxoozljbm4h3tpnuv4u4ej4ztysvi6f@x44v7nz2wdzd/ [1] Link: https://lkml.kernel.org/r/[email protected] Fixes: 3ac4638 ("memcg: make memcg_rstat_updated nmi safe") Signed-off-by: Shakeel Butt <[email protected]> Acked-by: Michal Hocko <[email protected]> Closes: https://lore.kernel.org/all/[email protected]/ Cc: Alexei Starovoitov <[email protected]> Cc: Johannes Weiner <[email protected]> Cc: Kumar Kartikeya Dwivedi <[email protected]> Cc: Muchun Song <[email protected]> Cc: Peilin Ye <[email protected]> Cc: Roman Gushchin <[email protected]> Cc: Tejun Heo <[email protected]> Cc: <[email protected]> Signed-off-by: Andrew Morton <[email protected]>
1 parent 7a405db commit fcc0669

File tree

2 files changed

+23
-10
lines changed

2 files changed

+23
-10
lines changed

include/linux/memcontrol.h

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1001,22 +1001,28 @@ static inline void count_memcg_event_mm(struct mm_struct *mm,
10011001
count_memcg_events_mm(mm, idx, 1);
10021002
}
10031003

1004-
static inline void memcg_memory_event(struct mem_cgroup *memcg,
1005-
enum memcg_memory_event event)
1004+
static inline void __memcg_memory_event(struct mem_cgroup *memcg,
1005+
enum memcg_memory_event event,
1006+
bool allow_spinning)
10061007
{
10071008
bool swap_event = event == MEMCG_SWAP_HIGH || event == MEMCG_SWAP_MAX ||
10081009
event == MEMCG_SWAP_FAIL;
10091010

1011+
/* For now only MEMCG_MAX can happen with !allow_spinning context. */
1012+
VM_WARN_ON_ONCE(!allow_spinning && event != MEMCG_MAX);
1013+
10101014
atomic_long_inc(&memcg->memory_events_local[event]);
1011-
if (!swap_event)
1015+
if (!swap_event && allow_spinning)
10121016
cgroup_file_notify(&memcg->events_local_file);
10131017

10141018
do {
10151019
atomic_long_inc(&memcg->memory_events[event]);
1016-
if (swap_event)
1017-
cgroup_file_notify(&memcg->swap_events_file);
1018-
else
1019-
cgroup_file_notify(&memcg->events_file);
1020+
if (allow_spinning) {
1021+
if (swap_event)
1022+
cgroup_file_notify(&memcg->swap_events_file);
1023+
else
1024+
cgroup_file_notify(&memcg->events_file);
1025+
}
10201026

10211027
if (!cgroup_subsys_on_dfl(memory_cgrp_subsys))
10221028
break;
@@ -1026,6 +1032,12 @@ static inline void memcg_memory_event(struct mem_cgroup *memcg,
10261032
!mem_cgroup_is_root(memcg));
10271033
}
10281034

1035+
static inline void memcg_memory_event(struct mem_cgroup *memcg,
1036+
enum memcg_memory_event event)
1037+
{
1038+
__memcg_memory_event(memcg, event, true);
1039+
}
1040+
10291041
static inline void memcg_memory_event_mm(struct mm_struct *mm,
10301042
enum memcg_memory_event event)
10311043
{

mm/memcontrol.c

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2307,12 +2307,13 @@ static int try_charge_memcg(struct mem_cgroup *memcg, gfp_t gfp_mask,
23072307
bool drained = false;
23082308
bool raised_max_event = false;
23092309
unsigned long pflags;
2310+
bool allow_spinning = gfpflags_allow_spinning(gfp_mask);
23102311

23112312
retry:
23122313
if (consume_stock(memcg, nr_pages))
23132314
return 0;
23142315

2315-
if (!gfpflags_allow_spinning(gfp_mask))
2316+
if (!allow_spinning)
23162317
/* Avoid the refill and flush of the older stock */
23172318
batch = nr_pages;
23182319

@@ -2348,7 +2349,7 @@ static int try_charge_memcg(struct mem_cgroup *memcg, gfp_t gfp_mask,
23482349
if (!gfpflags_allow_blocking(gfp_mask))
23492350
goto nomem;
23502351

2351-
memcg_memory_event(mem_over_limit, MEMCG_MAX);
2352+
__memcg_memory_event(mem_over_limit, MEMCG_MAX, allow_spinning);
23522353
raised_max_event = true;
23532354

23542355
psi_memstall_enter(&pflags);
@@ -2415,7 +2416,7 @@ static int try_charge_memcg(struct mem_cgroup *memcg, gfp_t gfp_mask,
24152416
* a MEMCG_MAX event.
24162417
*/
24172418
if (!raised_max_event)
2418-
memcg_memory_event(mem_over_limit, MEMCG_MAX);
2419+
__memcg_memory_event(mem_over_limit, MEMCG_MAX, allow_spinning);
24192420

24202421
/*
24212422
* The allocation either can't fail or will lead to more memory

0 commit comments

Comments
 (0)