Re: [PATCH V4 2/2] cgroup/rstat: Avoid thundering herd problem by kswapd across NUMA nodes

[Yosry Ahmed <yosryahmed@google.com>]

[..]
> >> +    /* Obtained lock, record this cgrp as the ongoing flusher */
> >> +    if (!READ_ONCE(cgrp_rstat_ongoing_flusher)) {
> >
> > Can the above condition will ever be false?
> >
>
> Yes, I think so, because I realized that cgroup_rstat_flush_locked() can
> release/"yield" the lock.  Thus, other CPUs/threads have a chance to
> call cgroup_rstat_flush, and try to become the "ongoing-flusher".

Right, there may actually be multiple ongoing flushers. I am now
wondering if it would be better if we drop cgrp_rstat_ongoing_flusher
completely, add a per-cgroup under_flush boolean/flag, and have the
cgroup iterate its parents here to check if any of them is under_flush
and wait for it instead.

Yes, we have to add parent iteration here, but I think it may be fine
because the flush path is already expensive. This will allow us to
detect if any ongoing flush is overlapping with us, not just the one
that happened to update cgrp_rstat_ongoing_flusher first.

WDYT?