On Thu, Jul 18, 2024 at 5:41 PM Shakeel Butt <shakeel.butt@xxxxxxxxx> wrote: > > Hi Jesper, > > On Wed, Jul 17, 2024 at 06:36:28PM GMT, Jesper Dangaard Brouer wrote: > > > [...] > > > > > > Looking at the production numbers for the time the lock is held for level 0: > > > > @locked_time_level[0]: > > [4M, 8M) 623 |@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@ | > > [8M, 16M) 860 |@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@| > > [16M, 32M) 295 |@@@@@@@@@@@@@@@@@ | > > [32M, 64M) 275 |@@@@@@@@@@@@@@@@ | > > > > Is it possible to get the above histogram for other levels as well? I > know this is 12 numa node machine, how many total CPUs are there? > > > The time is in nanosec, so M corresponds to ms (milliseconds). > > > > With 36 flushes per second (as shown earlier) this is a flush every > > 27.7ms. It is not unreasonable (from above data) that the flush time > > also spend 27ms, which means that we spend a full CPU second flushing. > > That is spending too much time flushing. > > One idea to further reduce this time is more fine grained flush > skipping. At the moment we either skip the whole flush or not. How > about we make this decision per-cpu? We already have per-cpu updates > data and if it is less than MEMCG_CHARGE_BATCH, skip flush on that cpu. Good idea. I think we would need a per-subsystem callback to decide whether we want to flush the cgroup or not. This needs to happen in the core rstat flushing code (not the memcg flushing code), as we need to make sure we do not remove the cgroup from the per-cpu updated tree if we don't flush it. More generally, I think we should be able to have a "force" flush API that skips all optimizations and ensures that a flush occurs. I think this will be needed in the cgroup_rstat_exit() path, where stats of a cgroup being freed must be propagated to its parent, no matter how insignificant they may be, to avoid inconsistencies.
