lkml.org 
[lkml]   [2023]   [Mar]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH v2 5/9] memcg: replace stats_flush_lock with an atomic
On Tue, Mar 28, 2023 at 3:17 PM Yosry Ahmed <yosryahmed@google.com> wrote:
>
> As Johannes notes in [1], stats_flush_lock is currently used to:
> (a) Protect updated to stats_flush_threshold.
> (b) Protect updates to flush_next_time.
> (c) Serializes calls to cgroup_rstat_flush() based on those ratelimits.
>
> However:
>
> 1. stats_flush_threshold is already an atomic
>
> 2. flush_next_time is not atomic. The writer is locked, but the reader
> is lockless. If the reader races with a flush, you could see this:
>
> if (time_after(jiffies, flush_next_time))
> spin_trylock()
> flush_next_time = now + delay
> flush()
> spin_unlock()
> spin_trylock()
> flush_next_time = now + delay
> flush()
> spin_unlock()
>
> which means we already can get flushes at a higher frequency than
> FLUSH_TIME during races. But it isn't really a problem.
>
> The reader could also see garbled partial updates, so it needs at
> least READ_ONCE and WRITE_ONCE protection.
>
> 3. Serializing cgroup_rstat_flush() calls against the ratelimit
> factors is currently broken because of the race in 2. But the race
> is actually harmless, all we might get is the occasional earlier
> flush. If there is no delta, the flush won't do much. And if there
> is, the flush is justified.
>
> So the lock can be removed all together. However, the lock also served
> the purpose of preventing a thundering herd problem for concurrent
> flushers, see [2]. Use an atomic instead to serve the purpose of
> unifying concurrent flushers.
>
> [1]https://lore.kernel.org/lkml/20230323172732.GE739026@cmpxchg.org/
> [2]https://lore.kernel.org/lkml/20210716212137.1391164-2-shakeelb@google.com/
>
> Signed-off-by: Yosry Ahmed <yosryahmed@google.com>
> Acked-by: Johannes Weiner <hannes@cmpxchg.org>

Acked-by: Shakeel Butt <shakeelb@google.com>

\
 
 \ /
  Last update: 2023-03-29 00:24    [W:0.163 / U:0.524 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site