[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAJD7tkaybPFoM697dtp0CiEJ2zmSYiH2+0yL+KG_LD=ZiscOJA@mail.gmail.com>
Date: Fri, 28 Jun 2024 15:15:00 -0700
From: Yosry Ahmed <yosryahmed@...gle.com>
To: Jesper Dangaard Brouer <hawk@...nel.org>
Cc: Shakeel Butt <shakeel.butt@...ux.dev>, tj@...nel.org, cgroups@...r.kernel.org,
hannes@...xchg.org, lizefan.x@...edance.com, longman@...hat.com,
kernel-team@...udflare.com, linux-mm@...ck.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH V4 2/2] cgroup/rstat: Avoid thundering herd problem by
kswapd across NUMA nodes
[..]
> >> + /* 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?
Powered by blists - more mailing lists