On Thu, Mar 23, 2023 at 9:00 AM Johannes Weiner <hannes@xxxxxxxxxxx> wrote: > > On Thu, Mar 23, 2023 at 04:00:36AM +0000, Yosry Ahmed wrote: > > In workingset_refault(), we call mem_cgroup_flush_stats_delayed() to > > flush stats within an RCU read section and with sleeping disallowed. > > Move the call to mem_cgroup_flush_stats_delayed() above the RCU read > > section and allow sleeping to avoid unnecessarily performing a lot of > > work without sleeping. > > > > Signed-off-by: Yosry Ahmed <yosryahmed@xxxxxxxxxx> > > --- > > > > A lot of code paths call into workingset_refault(), so I am not > > generally sure at all whether it's okay to sleep in all contexts or not. > > Feedback here would be very helpful. > > > > --- > > mm/workingset.c | 5 ++--- > > 1 file changed, 2 insertions(+), 3 deletions(-) > > > > diff --git a/mm/workingset.c b/mm/workingset.c > > index 042eabbb43f6..410bc6684ea7 100644 > > --- a/mm/workingset.c > > +++ b/mm/workingset.c > > @@ -406,6 +406,8 @@ void workingset_refault(struct folio *folio, void *shadow) > > unpack_shadow(shadow, &memcgid, &pgdat, &eviction, &workingset); > > eviction <<= bucket_order; > > > > + /* Flush stats (and potentially sleep) before holding RCU read lock */ > > + mem_cgroup_flush_stats_delayed(true); > > Btw, it might be a good time to rename this while you're in the > area. delayed suggests this is using a delayed_work, but this is > actually sometimes flushing directly from the callsite. > > What it's doing is ratelimited calls. A better name would be: > > mem_cgroup_flush_stats_ratelimited() Agreed. Will do in the next version.
