From: David Laight <david.laight.linux@gmail.com>
To: Usama Arif <usama.arif@linux.dev>
Cc: phillip@squashfs.org.uk,
Andrew Morton <akpm@linux-foundation.org>,
linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
squashfs-devel@lists.sourceforge.net, brauner@kernel.org,
hannes@cmpxchg.org, shakeel.butt@linux.dev, jlayton@kernel.org,
boris@bur.io, riel@surriel.com, kernel-team@meta.com
Subject: Re: [PATCH] squashfs: avoid thundering-herd cache wakeups
Date: Mon, 3 Aug 2026 12:39:58 +0100 [thread overview]
Message-ID: <20260803123958.414dd2ed@pumpkin> (raw)
In-Reply-To: <664bff99-ef20-44b9-9c5f-aa6c8755133e@linux.dev>
On Mon, 3 Aug 2026 11:16:36 +0100
Usama Arif <usama.arif@linux.dev> wrote:
> On 24/07/2026 20:05, Usama Arif wrote:
> > squashfs_cache_get() puts a task to sleep when its block is not cached and
> > every cache entry is busy. Those sleeps are non-exclusive, so a single
> > squashfs_cache_put() makes every waiter runnable when one entry is freed.
> > A wakee returns to squashfs_cache_get() only if it observes cache->unused
> > before the entry is reclaimed. Such tasks can rescan and share a block
> > published meanwhile; later wakees see zero and queue again inside
> > wait_event() without rescanning.
> >
> > The waste is dramatic under load. On a Meta production host serving a
> > Python web application from a packaged squashfs image, a 30-second trace
> > caught 1,045,132 cache-release wake calls and 19,511,556 remote wakeups:
> > 18.7 per release, although each release added only one reusable cache
> > entry. This was causing significant spikes in CPU usage.
> >
> > Simply making the waits exclusive (i.e. using
> > wait_event_state_exclusive()) is not enough. A waiter can make
> > progress two ways, when an entry frees (capacity), or when another task
> > publishes the block it wants and it shares that entry. The only wait
> > condition available is cache->unused, which captures capacity but not
> > publication, and wake-one can release just a single sharer at a time. A
> > waiter woken for capacity may also find its block already published, share
> > it, and leave the freed entry unclaimed, so that wake must be handed on.
> > A block-targeted wake and that handoff need a custom wake callback with
> > per-waiter state; wait_event_state_exclusive() provides neither.
> >
> > Wake selectively instead. Waiters are exclusive and keyed by requested
> > block: freeing an entry wakes one waiter (capacity), publishing a block
> > wakes every waiter for that block (sharing), and a capacity waiter that
> > ends up sharing hands its wake to the next waiter. Waiters enqueue while
> > holding cache->lock so lookup and publication are ordered against sleeping.
> >
> > The result is an ~2x increase in throughput on stat and read.
> > Measured on a 32-CPU VM against a read-only squashfs (gzip, per-cpu
> > decompressor, default 8 metadata / 3 fragment cache entries) staged in tmpfs
> > with caches dropped each iteration to force cold decompression:
> >
> > elbencho, 64 threads
> > metadata stat 700 -> 1320 files/s 1.9x
> > small-file read 40 -> 60 MiB/s 1.6x
> >
> > filebench, 128 threads, open+read+stat+close (mean of 3x 30s)
> > throughput 11,314 -> 25,186 ops/s 2.2x
> > latency 11.30 -> 5.05 ms/op 2.2x lower
> > sched:sched_wakeup 9.18M -> 3.44M 2.7x fewer
> > context switches 12.62M -> 5.77M 2.2x fewer
> >
> > The 2.7x cut in scheduler wakeups explains the results: it roughly doubles
> > throughput and halves latency under contention, and is a no-op on
> > workloads that never queue for a cache entry.
> >
> > Signed-off-by: Usama Arif <usama.arif@linux.dev>
> > ---
> > fs/squashfs/cache.c | 66 ++++++++++++++++++++++++++++++++++++++++-----
> > 1 file changed, 60 insertions(+), 6 deletions(-)
>
> Hi,
>
> Just wanted to check if there are any reviews or comments on this patch?
>
> It is attempting to solvie a real thundering herd problem we see in our fleet.
> It also doubles throughput for small file reads and stat for squashfs.
But it adds a priority inversion problem.
It the task that is woken cannot be scheduled (either because if its priority
or because it is pinned/real-time and the cpu it would be run on has preemption
disabled, then none of the other threads can run either.
I had terrible trouble trying to get a 'thundering herd' from a
pthread_broadcast() call....
David
>
> Thanks,
> Usama
>
>
> >
> > diff --git a/fs/squashfs/cache.c b/fs/squashfs/cache.c
> > index 67abd4dff222..5c790c204a1a 100644
> > --- a/fs/squashfs/cache.c
> > +++ b/fs/squashfs/cache.c
> > @@ -45,6 +45,40 @@
> > #include "squashfs.h"
> > #include "page_actor.h"
> >
> > +/*
> > + * A NULL wake key selects one waiter for newly available capacity. A block
> > + * key selects every waiter which can share the newly published cache entry.
> > + */
> > +struct squashfs_cache_wait {
> > + wait_queue_entry_t wait;
> > + u64 block;
> > + bool capacity_wake;
> > +};
> > +
> > +static int squashfs_cache_wake_function(wait_queue_entry_t *wait,
> > + unsigned int mode, int sync, void *key)
> > +{
> > + struct squashfs_cache_wait *cache_wait =
> > + container_of(wait, struct squashfs_cache_wait, wait);
> > + u64 *block = key;
> > + int ret;
> > +
> > + if (block && cache_wait->block != *block)
> > + return 0;
> > +
> > + /* A failed wake remains queued, so finish_wait() sees the reset. */
> > + WRITE_ONCE(cache_wait->capacity_wake, !block);
> > + ret = autoremove_wake_function(wait, mode, sync, NULL);
> > + if (!ret)
> > + WRITE_ONCE(cache_wait->capacity_wake, false);
> > + return ret;
> > +}
> > +
> > +static void squashfs_cache_wake_block(struct squashfs_cache *cache, u64 block)
> > +{
> > + __wake_up(&cache->wait_queue, TASK_NORMAL, 0, &block);
> > +}
> > +
> > /*
> > * Look-up block in cache, and increment usage count. If not in cache, read
> > * and decompress it from disk.
> > @@ -54,6 +88,8 @@ struct squashfs_cache_entry *squashfs_cache_get(struct super_block *sb,
> > {
> > int i, n;
> > struct squashfs_cache_entry *entry;
> > + struct squashfs_cache_wait wait = { .block = block };
> > + bool consumed, pending, wake_block, wake_next;
> >
> > spin_lock(&cache->lock);
> >
> > @@ -72,9 +108,16 @@ struct squashfs_cache_entry *squashfs_cache_get(struct super_block *sb,
> > * go to sleep waiting for one to become available.
> > */
> > if (cache->unused == 0) {
> > + init_wait(&wait.wait);
> > + wait.wait.func = squashfs_cache_wake_function;
> > cache->num_waiters++;
> > + WRITE_ONCE(wait.capacity_wake, false);
> > + /* Enqueue before dropping the lock to avoid missing publish. */
> > + prepare_to_wait_exclusive(&cache->wait_queue,
> > + &wait.wait, TASK_UNINTERRUPTIBLE);
> > spin_unlock(&cache->lock);
> > - wait_event(cache->wait_queue, cache->unused);
> > + schedule();
> > + finish_wait(&cache->wait_queue, &wait.wait);
> > spin_lock(&cache->lock);
> > cache->num_waiters--;
> > continue;
> > @@ -105,8 +148,12 @@ struct squashfs_cache_entry *squashfs_cache_get(struct super_block *sb,
> > entry->pending = 1;
> > entry->num_waiters = 0;
> > entry->error = 0;
> > + wake_block = cache->num_waiters;
> > spin_unlock(&cache->lock);
> >
> > + if (wake_block)
> > + squashfs_cache_wake_block(cache, block);
> > +
> > entry->length = squashfs_read_data(sb, block, length,
> > &entry->next_index, entry->actor);
> >
> > @@ -138,7 +185,8 @@ struct squashfs_cache_entry *squashfs_cache_get(struct super_block *sb,
> > * for reuse.
> > */
> > entry = &cache->entry[i];
> > - if (entry->refcount == 0)
> > + consumed = entry->refcount == 0;
> > + if (consumed)
> > cache->unused--;
> > entry->refcount++;
> >
> > @@ -146,12 +194,18 @@ struct squashfs_cache_entry *squashfs_cache_get(struct super_block *sb,
> > * If the entry is currently being filled in by another process
> > * go to sleep waiting for it to become available.
> > */
> > - if (entry->pending) {
> > + pending = entry->pending;
> > + if (pending)
> > entry->num_waiters++;
> > - spin_unlock(&cache->lock);
> > + /* Pass on capacity if this waiter shared rather than consumed it. */
> > + wake_next = READ_ONCE(wait.capacity_wake) && !consumed &&
> > + cache->unused && cache->num_waiters;
> > + spin_unlock(&cache->lock);
> > +
> > + if (wake_next)
> > + wake_up(&cache->wait_queue);
> > + if (pending)
> > wait_event(entry->wait_queue, !entry->pending);
> > - } else
> > - spin_unlock(&cache->lock);
> >
> > goto out;
> > }
>
>
next prev parent reply other threads:[~2026-08-03 11:40 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-24 19:05 [PATCH] squashfs: avoid thundering-herd cache wakeups Usama Arif
2026-08-03 10:16 ` Usama Arif
2026-08-03 11:39 ` David Laight [this message]
2026-08-03 13:05 ` Usama Arif
2026-08-03 14:43 ` Phillip Lougher
2026-08-03 16:35 ` Usama Arif
2026-08-07 16:01 ` Phillip Lougher
2026-08-07 18:43 ` Usama Arif
2026-08-07 18:58 ` Andrew Morton
2026-08-07 20:11 ` Usama Arif
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260803123958.414dd2ed@pumpkin \
--to=david.laight.linux@gmail.com \
--cc=akpm@linux-foundation.org \
--cc=boris@bur.io \
--cc=brauner@kernel.org \
--cc=hannes@cmpxchg.org \
--cc=jlayton@kernel.org \
--cc=kernel-team@meta.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=phillip@squashfs.org.uk \
--cc=riel@surriel.com \
--cc=shakeel.butt@linux.dev \
--cc=squashfs-devel@lists.sourceforge.net \
--cc=usama.arif@linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.