| Message ID | 20230328061638.203420-4-yosryahmed@google.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | memcg: make rstat flushing irq and sleep friendly | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
On Mon, Mar 27, 2023 at 11:16 PM Yosry Ahmed <yosryahmed@google.com> wrote: > > Currently, the only context in which we can invoke an rstat flush from > irq context is through mem_cgroup_usage() on the root memcg when called > from memcg_check_events(). An rstat flush is an expensive operation that > should not be done in irq context, so do not flush stats and use the > stale stats in this case. > > Arguably, usage threshold events are not reliable on the root memcg > anyway since its usage is ill-defined. > > Suggested-by: Johannes Weiner <hannes@cmpxchg.org> > Suggested-by: Shakeel Butt <shakeelb@google.com> > Signed-off-by: Yosry Ahmed <yosryahmed@google.com> Acked-by: Shakeel Butt <shakeelb@google.com>
On Tue, Mar 28, 2023 at 06:16:32AM +0000, Yosry Ahmed wrote: > Currently, the only context in which we can invoke an rstat flush from > irq context is through mem_cgroup_usage() on the root memcg when called > from memcg_check_events(). An rstat flush is an expensive operation that > should not be done in irq context, so do not flush stats and use the > stale stats in this case. > > Arguably, usage threshold events are not reliable on the root memcg > anyway since its usage is ill-defined. > > Suggested-by: Johannes Weiner <hannes@cmpxchg.org> > Suggested-by: Shakeel Butt <shakeelb@google.com> > Signed-off-by: Yosry Ahmed <yosryahmed@google.com> Excellent, thanks! Acked-by: Johannes Weiner <hannes@cmpxchg.org>
diff --git a/mm/memcontrol.c b/mm/memcontrol.c index c3b6aae78901..ff39f78f962e 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -3669,7 +3669,21 @@ static unsigned long mem_cgroup_usage(struct mem_cgroup *memcg, bool swap) unsigned long val; if (mem_cgroup_is_root(memcg)) { - mem_cgroup_flush_stats(); + /* + * We can reach here from irq context through: + * uncharge_batch() + * |--memcg_check_events() + * |--mem_cgroup_threshold() + * |--__mem_cgroup_threshold() + * |--mem_cgroup_usage + * + * rstat flushing is an expensive operation that should not be + * done from irq context; use stale stats in this case. + * Arguably, usage threshold events are not reliable on the root + * memcg anyway since its usage is ill-defined. + */ + if (in_task()) + mem_cgroup_flush_stats(); val = memcg_page_state(memcg, NR_FILE_PAGES) + memcg_page_state(memcg, NR_ANON_MAPPED); if (swap)
Currently, the only context in which we can invoke an rstat flush from irq context is through mem_cgroup_usage() on the root memcg when called from memcg_check_events(). An rstat flush is an expensive operation that should not be done in irq context, so do not flush stats and use the stale stats in this case. Arguably, usage threshold events are not reliable on the root memcg anyway since its usage is ill-defined. Suggested-by: Johannes Weiner <hannes@cmpxchg.org> Suggested-by: Shakeel Butt <shakeelb@google.com> Signed-off-by: Yosry Ahmed <yosryahmed@google.com> --- mm/memcontrol.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-)