Re: [PATCH] squashfs: avoid thundering-herd cache wakeups

David Laight <[email protected]> Mon, 3 Aug 2026 12:39:58 +0100
Newsgroups org.kernel.vger.linux-fsdevel,org.kernel.vger.linux-kernel
Message-ID <20260803123958.414dd2ed@pumpkin>
On Mon, 3 Aug 2026 11:16:36 +0100
Usama Arif <[email protected]> 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 <[email protected]>
> > ---
> >  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;
> >  	}  
> 
>