On Thu 30-03-23 00:13:16, Yosry Ahmed wrote: > On Thu, Mar 30, 2023 at 12:06 AM Michal Hocko <mhocko@xxxxxxxx> wrote: [...] > > So the real question is. Do we really care so deeply? After all somebody > > might be calling this from within a spin lock or irq disabled section > > resulting in a similar situation without noticing. > > There are discussions in [1] about making atomic rstat flush not > disable irqs throughout the process, so in that case it would only > result in a similar situation if the caller has irq disabled. The only > caller that might have irq disabled is the same caller that might be > in irq context before this series: mem_cgroup_usage(). > > On that note, and while I have your attention, I was wondering if we > can eliminate the flush call completely from mem_cgroup_usage(), and > read the global stats counters for root memcg instead of the root > counters. There might be subtle differences, but the root memcg usage > isn't super accurate now anyway (e.g. it doesn't include kernel > memory). root memcg stats are imprecise indeed and I have to admit I do not really know why we are adding more work for that case. I have tried to dig into git history for that yesterday but couldn't find a satisfying answer. It goes all the way down to 2d146aa3aa842 which has done the switch to rstat. Maybe Johannes remembers. Anyway, back to this particular patch. I still do not see strong reasons to be verbose about !in_task case so I would just drop this patch. -- Michal Hocko SUSE Labs
