[PATCH 0/3 V3] libxfs: better cache allocation error handling

[PATCH 0/3 V3] libxfs: better cache allocation error handling

[Eric Sandeen]

OK, 3rd try.

This is to handle an underlying problem that an actual ENOMEM cache
node allocation failure is indistinguishable from "current cache full"
and so ENOMEM may send us into a pointless cache-expansion loop which
ultimately overflows the cache size variable, rather than just reporting
the bad news up the chain to callers.

Darrick suggested "I think I'd almost rather we change cache_node_allocate
to return int and the cache node as an outparam since we do that elsewhere"

on the last review cycle, and so I've done that here, I agree that it
is cleaner.

The first patch is maybe too granular but for some reason this stuff
hurts my brain a little and I thought it helped with clarity; it's a
no-op change to cache_node_allocate's signature, returning an int
and passing the cache node as a param.

Second patch should actually catch and bubble up any low-level error.

3rd patch is defensive against a case which in theory should almost never
happen after the other fixes.

[PATCH 1/3] libxfs: change cache_node_allocate function signature

[Eric Sandeen]

Modify cache_node_allocate() so that it returns a 0 / 1 status for
cache miss / cache hit, with the cache node itself as a parameter. This
will allow the next patches to properly differentiate underlying states
and error conditions.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 libxfs/cache.c | 26 +++++++++++++++++---------
 1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/libxfs/cache.c b/libxfs/cache.c
index d5d9ba56..a6dabf92 100644
--- a/libxfs/cache.c
+++ b/libxfs/cache.c
@@ -284,11 +284,14 @@ cache_shake(
 /*
  * Allocate a new hash node (updating atomic counter in the process),
  * unless doing so will push us over the maximum cache size.
+ * Return 1 with nodep set on success.
+ * Return 0 and NULL nodep otherwise (cache full, or alloc failure)
  */
-static struct cache_node *
+static int
 cache_node_allocate(
 	struct cache *		cache,
-	cache_key_t		key)
+	cache_key_t		key,
+	struct cache_node **	nodep)
 {
 	unsigned int		nodesfree;
 	struct cache_node *	node;
@@ -302,21 +305,25 @@ cache_node_allocate(
 	}
 	cache->c_misses++;
 	pthread_mutex_unlock(&cache->c_mutex);
-	if (!nodesfree)
-		return NULL;
+	if (!nodesfree) {
+		*nodep = NULL;
+		return 0;
+	}
 	node = cache->alloc(key);
 	if (node == NULL) {	/* uh-oh */
 		pthread_mutex_lock(&cache->c_mutex);
 		cache->c_count--;
 		pthread_mutex_unlock(&cache->c_mutex);
-		return NULL;
+		*nodep = NULL;
+		return 0;
 	}
 	pthread_mutex_init(&node->cn_mutex, NULL);
 	list_head_init(&node->cn_mru);
 	node->cn_count = 1;
 	node->cn_priority = 0;
 	node->cn_old_priority = -1;
-	return node;
+	*nodep = node;
+	return 1;
 }
 
 int
@@ -366,8 +373,8 @@ __cache_node_purge(
  * hit, in which case this will all be over quickly and painlessly.
  * Otherwise, we allocate a new node, taking care not to expand the
  * cache beyond the requested maximum size (shrink it if it would).
- * Returns one if hit in cache, otherwise zero.  A node is _always_
- * returned, however.
+ * Returns zero if hit in cache, one if a new node was allocated.  A node
+ * is always returned.
  */
 int
 cache_node_get(
@@ -457,9 +464,10 @@ next_object:
 		/*
 		 * not found, allocate a new entry
 		 */
-		node = cache_node_allocate(cache, key);
+		cache_node_allocate(cache, key, &node);
 		if (node)
 			break;
+
 		priority = cache_shake(cache, priority, false);
 		/*
 		 * We start at 0; if we free CACHE_SHAKE_COUNT we get
-- 
2.55.0

[PATCH 2/3] libxfs: Return -ENOMEM on actual cache node allocation failures

[Eric Sandeen]

Today, if we get an underlying ENOMEM from an actual cache node
allocation attempt, it's indistinguishable from "current cache size
is filled" and we'll keep doubling cache size to no avail until the
size itself overflows.

This patch differentiates "no node returned" between the "cache full;
grow it" case and the "underlying allocation actually got ENOMEM" case
so that the caller can do the right thing, and return true ENOMEM
up the stack for callers to handle.

Finally, actually check this returned error at the cache_node_get
callsite (in __cache_lookup).

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 libxfs/cache.c | 22 ++++++++++++++++------
 libxfs/rdwr.c  |  9 ++++-----
 2 files changed, 20 insertions(+), 11 deletions(-)

diff --git a/libxfs/cache.c b/libxfs/cache.c
index a6dabf92..60bfcdc5 100644
--- a/libxfs/cache.c
+++ b/libxfs/cache.c
@@ -285,7 +285,8 @@ cache_shake(
  * Allocate a new hash node (updating atomic counter in the process),
  * unless doing so will push us over the maximum cache size.
  * Return 1 with nodep set on success.
- * Return 0 and NULL nodep otherwise (cache full, or alloc failure)
+ * Return 0 and NULL nodep if the cache is full (caller should shake/expand).
+ * Return -ENOMEM and NULL nodep if the underlying allocation failed.
  */
 static int
 cache_node_allocate(
@@ -315,7 +316,7 @@ cache_node_allocate(
 		cache->c_count--;
 		pthread_mutex_unlock(&cache->c_mutex);
 		*nodep = NULL;
-		return 0;
+		return -ENOMEM;
 	}
 	pthread_mutex_init(&node->cn_mutex, NULL);
 	list_head_init(&node->cn_mru);
@@ -373,8 +374,8 @@ __cache_node_purge(
  * hit, in which case this will all be over quickly and painlessly.
  * Otherwise, we allocate a new node, taking care not to expand the
  * cache beyond the requested maximum size (shrink it if it would).
- * Returns zero if hit in cache, one if a new node was allocated.  A node
- * is always returned.
+ * Returns zero if hit in cache, one if a new node was allocated.  Returns
+ * -ENOMEM if allocation fails after cache shaking is exhausted.
  */
 int
 cache_node_get(
@@ -391,6 +392,7 @@ cache_node_get(
 	unsigned int		hashidx;
 	int			priority = 0;
 	int			purged = 0;
+	int			error = 0;
 
 	hashidx = cache->hash(key, cache->c_hashsize, cache->c_hashshift);
 	hash = cache->c_hash + hashidx;
@@ -464,8 +466,8 @@ next_object:
 		/*
 		 * not found, allocate a new entry
 		 */
-		cache_node_allocate(cache, key, &node);
-		if (node)
+		error = cache_node_allocate(cache, key, &node);
+		if (error > 0)
 			break;
 
 		priority = cache_shake(cache, priority, false);
@@ -475,6 +477,14 @@ next_object:
 		 * If we exceed CACHE_MAX_PRIORITY all slots are full; grow it.
 		 */
 		if (priority > CACHE_MAX_PRIORITY) {
+			/*
+			 * We've shaken every priority level.  If the last
+			 * attempt failed with a real ENOMEM rather than a
+			 * full cache, neither shaking nor growing can help;
+			 * return the error to the caller.
+			 */
+			if (error < 0)
+				return error;
 			priority = 0;
 			cache_expand(cache);
 		}
diff --git a/libxfs/rdwr.c b/libxfs/rdwr.c
index 90f2d566..d8ba832e 100644
--- a/libxfs/rdwr.c
+++ b/libxfs/rdwr.c
@@ -411,17 +411,16 @@ __cache_lookup(
 	struct cache_node	*cn = NULL;
 	struct cache		*bcache = key->buftarg->bcache;
 	struct xfs_buf		*bp;
+	int			ret;
 
 	*bpp = NULL;
 
-	cache_node_get(bcache, key, &cn);
-	if (!cn)
-		return -ENOMEM;
+	ret = cache_node_get(bcache, key, &cn);
+	if (ret < 0)
+		return ret;
 	bp = container_of(cn, struct xfs_buf, b_node);
 
 	if (use_xfs_buf_lock) {
-		int		ret;
-
 		ret = pthread_mutex_trylock(&bp->b_lock);
 		if (ret) {
 			ASSERT(ret == EAGAIN);
-- 
2.55.0

[PATCH 3/3] libxfs: do not allow cache growth to overflow

[Eric Sandeen]

Nothing stops cache_expand() from growing c_maxcount (via *2)
repeatedly until it overflows.

Add a bounds check here and return failure if for any reason we try to
grow too much.

With the prior fixes, we should never get this far, but it's worth
defending against anyway.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 libxfs/cache.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/libxfs/cache.c b/libxfs/cache.c
index 60bfcdc5..03b62bc7 100644
--- a/libxfs/cache.c
+++ b/libxfs/cache.c
@@ -79,16 +79,24 @@ cache_init(
 	return cache;
 }
 
-static void
+/* Double the cache size limit; return false if it would overflow. */
+static bool
 cache_expand(
 	struct cache *		cache)
 {
+	bool			expanded = false;
+
 	pthread_mutex_lock(&cache->c_mutex);
+	if (cache->c_maxcount <= UINT_MAX / 2) {
 #ifdef CACHE_DEBUG
-	fprintf(stderr, "doubling cache size to %u\n", 2 * cache->c_maxcount);
+		fprintf(stderr, "doubling cache size to %u\n",
+				2 * cache->c_maxcount);
 #endif
-	cache->c_maxcount *= 2;
+		cache->c_maxcount *= 2;
+		expanded = true;
+	}
 	pthread_mutex_unlock(&cache->c_mutex);
+	return expanded;
 }
 
 void
@@ -375,7 +383,8 @@ __cache_node_purge(
  * Otherwise, we allocate a new node, taking care not to expand the
  * cache beyond the requested maximum size (shrink it if it would).
  * Returns zero if hit in cache, one if a new node was allocated.  Returns
- * -ENOMEM if allocation fails after cache shaking is exhausted.
+ * -ENOMEM if allocation fails after cache shaking is exhausted, or if the
+ * cache cannot be expanded further without overflowing.
  */
 int
 cache_node_get(
@@ -485,8 +494,9 @@ next_object:
 			 */
 			if (error < 0)
 				return error;
+			if (!cache_expand(cache))
+				return -ENOMEM;
 			priority = 0;
-			cache_expand(cache);
 		}
 	}
 
-- 
2.55.0

Re: [PATCH 1/3] libxfs: change cache_node_allocate function signature

[Darrick J. Wong]

On Fri, Sep 25, 2026 at 02:41:43PM -0500, Eric Sandeen wrote:
> Modify cache_node_allocate() so that it returns a 0 / 1 status for
> cache miss / cache hit, with the cache node itself as a parameter. This
> will allow the next patches to properly differentiate underlying states
> and error conditions.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>

I like this much better, thanks!
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
>  libxfs/cache.c | 26 +++++++++++++++++---------
>  1 file changed, 17 insertions(+), 9 deletions(-)
> 
> diff --git a/libxfs/cache.c b/libxfs/cache.c
> index d5d9ba56..a6dabf92 100644
> --- a/libxfs/cache.c
> +++ b/libxfs/cache.c
> @@ -284,11 +284,14 @@ cache_shake(
>  /*
>   * Allocate a new hash node (updating atomic counter in the process),
>   * unless doing so will push us over the maximum cache size.
> + * Return 1 with nodep set on success.
> + * Return 0 and NULL nodep otherwise (cache full, or alloc failure)
>   */
> -static struct cache_node *
> +static int
>  cache_node_allocate(
>  	struct cache *		cache,
> -	cache_key_t		key)
> +	cache_key_t		key,
> +	struct cache_node **	nodep)
>  {
>  	unsigned int		nodesfree;
>  	struct cache_node *	node;
> @@ -302,21 +305,25 @@ cache_node_allocate(
>  	}
>  	cache->c_misses++;
>  	pthread_mutex_unlock(&cache->c_mutex);
> -	if (!nodesfree)
> -		return NULL;
> +	if (!nodesfree) {
> +		*nodep = NULL;
> +		return 0;
> +	}
>  	node = cache->alloc(key);
>  	if (node == NULL) {	/* uh-oh */
>  		pthread_mutex_lock(&cache->c_mutex);
>  		cache->c_count--;
>  		pthread_mutex_unlock(&cache->c_mutex);
> -		return NULL;
> +		*nodep = NULL;
> +		return 0;
>  	}
>  	pthread_mutex_init(&node->cn_mutex, NULL);
>  	list_head_init(&node->cn_mru);
>  	node->cn_count = 1;
>  	node->cn_priority = 0;
>  	node->cn_old_priority = -1;
> -	return node;
> +	*nodep = node;
> +	return 1;
>  }
>  
>  int
> @@ -366,8 +373,8 @@ __cache_node_purge(
>   * hit, in which case this will all be over quickly and painlessly.
>   * Otherwise, we allocate a new node, taking care not to expand the
>   * cache beyond the requested maximum size (shrink it if it would).
> - * Returns one if hit in cache, otherwise zero.  A node is _always_
> - * returned, however.
> + * Returns zero if hit in cache, one if a new node was allocated.  A node
> + * is always returned.
>   */
>  int
>  cache_node_get(
> @@ -457,9 +464,10 @@ next_object:
>  		/*
>  		 * not found, allocate a new entry
>  		 */
> -		node = cache_node_allocate(cache, key);
> +		cache_node_allocate(cache, key, &node);
>  		if (node)
>  			break;
> +
>  		priority = cache_shake(cache, priority, false);
>  		/*
>  		 * We start at 0; if we free CACHE_SHAKE_COUNT we get
> -- 
> 2.55.0
> 
> 

Re: [PATCH 2/3] libxfs: Return -ENOMEM on actual cache node allocation failures

[Darrick J. Wong]

On Fri, Sep 25, 2026 at 02:41:44PM -0500, Eric Sandeen wrote:
> Today, if we get an underlying ENOMEM from an actual cache node
> allocation attempt, it's indistinguishable from "current cache size
> is filled" and we'll keep doubling cache size to no avail until the
> size itself overflows.
> 
> This patch differentiates "no node returned" between the "cache full;
> grow it" case and the "underlying allocation actually got ENOMEM" case
> so that the caller can do the right thing, and return true ENOMEM
> up the stack for callers to handle.
> 
> Finally, actually check this returned error at the cache_node_get
> callsite (in __cache_lookup).
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
>  libxfs/cache.c | 22 ++++++++++++++++------
>  libxfs/rdwr.c  |  9 ++++-----
>  2 files changed, 20 insertions(+), 11 deletions(-)
> 
> diff --git a/libxfs/cache.c b/libxfs/cache.c
> index a6dabf92..60bfcdc5 100644
> --- a/libxfs/cache.c
> +++ b/libxfs/cache.c
> @@ -285,7 +285,8 @@ cache_shake(
>   * Allocate a new hash node (updating atomic counter in the process),
>   * unless doing so will push us over the maximum cache size.
>   * Return 1 with nodep set on success.
> - * Return 0 and NULL nodep otherwise (cache full, or alloc failure)
> + * Return 0 and NULL nodep if the cache is full (caller should shake/expand).
> + * Return -ENOMEM and NULL nodep if the underlying allocation failed.
>   */
>  static int
>  cache_node_allocate(
> @@ -315,7 +316,7 @@ cache_node_allocate(
>  		cache->c_count--;
>  		pthread_mutex_unlock(&cache->c_mutex);
>  		*nodep = NULL;
> -		return 0;
> +		return -ENOMEM;
>  	}
>  	pthread_mutex_init(&node->cn_mutex, NULL);
>  	list_head_init(&node->cn_mru);
> @@ -373,8 +374,8 @@ __cache_node_purge(
>   * hit, in which case this will all be over quickly and painlessly.
>   * Otherwise, we allocate a new node, taking care not to expand the
>   * cache beyond the requested maximum size (shrink it if it would).
> - * Returns zero if hit in cache, one if a new node was allocated.  A node
> - * is always returned.
> + * Returns zero if hit in cache, one if a new node was allocated.  Returns
> + * -ENOMEM if allocation fails after cache shaking is exhausted.
>   */
>  int
>  cache_node_get(
> @@ -391,6 +392,7 @@ cache_node_get(
>  	unsigned int		hashidx;
>  	int			priority = 0;
>  	int			purged = 0;
> +	int			error = 0;
>  
>  	hashidx = cache->hash(key, cache->c_hashsize, cache->c_hashshift);
>  	hash = cache->c_hash + hashidx;
> @@ -464,8 +466,8 @@ next_object:
>  		/*
>  		 * not found, allocate a new entry
>  		 */
> -		cache_node_allocate(cache, key, &node);
> -		if (node)
> +		error = cache_node_allocate(cache, key, &node);
> +		if (error > 0)

I think you could keep the node nullness check, though in the end it
doesn't make much difference.  It's not like you can return
Result<struct cache_node> or anything. :)

Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D


>  			break;
>  
>  		priority = cache_shake(cache, priority, false);
> @@ -475,6 +477,14 @@ next_object:
>  		 * If we exceed CACHE_MAX_PRIORITY all slots are full; grow it.
>  		 */
>  		if (priority > CACHE_MAX_PRIORITY) {
> +			/*
> +			 * We've shaken every priority level.  If the last
> +			 * attempt failed with a real ENOMEM rather than a
> +			 * full cache, neither shaking nor growing can help;
> +			 * return the error to the caller.
> +			 */
> +			if (error < 0)
> +				return error;
>  			priority = 0;
>  			cache_expand(cache);
>  		}
> diff --git a/libxfs/rdwr.c b/libxfs/rdwr.c
> index 90f2d566..d8ba832e 100644
> --- a/libxfs/rdwr.c
> +++ b/libxfs/rdwr.c
> @@ -411,17 +411,16 @@ __cache_lookup(
>  	struct cache_node	*cn = NULL;
>  	struct cache		*bcache = key->buftarg->bcache;
>  	struct xfs_buf		*bp;
> +	int			ret;
>  
>  	*bpp = NULL;
>  
> -	cache_node_get(bcache, key, &cn);
> -	if (!cn)
> -		return -ENOMEM;
> +	ret = cache_node_get(bcache, key, &cn);
> +	if (ret < 0)
> +		return ret;
>  	bp = container_of(cn, struct xfs_buf, b_node);
>  
>  	if (use_xfs_buf_lock) {
> -		int		ret;
> -
>  		ret = pthread_mutex_trylock(&bp->b_lock);
>  		if (ret) {
>  			ASSERT(ret == EAGAIN);
> -- 
> 2.55.0
> 
> 

Re: [PATCH 3/3] libxfs: do not allow cache growth to overflow

[Darrick J. Wong]

On Fri, Sep 25, 2026 at 02:41:45PM -0500, Eric Sandeen wrote:
> Nothing stops cache_expand() from growing c_maxcount (via *2)
> repeatedly until it overflows.
> 
> Add a bounds check here and return failure if for any reason we try to
> grow too much.
> 
> With the prior fixes, we should never get this far, but it's worth
> defending against anyway.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>

Looks good,
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
>  libxfs/cache.c | 20 +++++++++++++++-----
>  1 file changed, 15 insertions(+), 5 deletions(-)
> 
> diff --git a/libxfs/cache.c b/libxfs/cache.c
> index 60bfcdc5..03b62bc7 100644
> --- a/libxfs/cache.c
> +++ b/libxfs/cache.c
> @@ -79,16 +79,24 @@ cache_init(
>  	return cache;
>  }
>  
> -static void
> +/* Double the cache size limit; return false if it would overflow. */
> +static bool
>  cache_expand(
>  	struct cache *		cache)
>  {
> +	bool			expanded = false;
> +
>  	pthread_mutex_lock(&cache->c_mutex);
> +	if (cache->c_maxcount <= UINT_MAX / 2) {
>  #ifdef CACHE_DEBUG
> -	fprintf(stderr, "doubling cache size to %u\n", 2 * cache->c_maxcount);
> +		fprintf(stderr, "doubling cache size to %u\n",
> +				2 * cache->c_maxcount);
>  #endif
> -	cache->c_maxcount *= 2;
> +		cache->c_maxcount *= 2;
> +		expanded = true;
> +	}
>  	pthread_mutex_unlock(&cache->c_mutex);
> +	return expanded;
>  }
>  
>  void
> @@ -375,7 +383,8 @@ __cache_node_purge(
>   * Otherwise, we allocate a new node, taking care not to expand the
>   * cache beyond the requested maximum size (shrink it if it would).
>   * Returns zero if hit in cache, one if a new node was allocated.  Returns
> - * -ENOMEM if allocation fails after cache shaking is exhausted.
> + * -ENOMEM if allocation fails after cache shaking is exhausted, or if the
> + * cache cannot be expanded further without overflowing.
>   */
>  int
>  cache_node_get(
> @@ -485,8 +494,9 @@ next_object:
>  			 */
>  			if (error < 0)
>  				return error;
> +			if (!cache_expand(cache))
> +				return -ENOMEM;
>  			priority = 0;
> -			cache_expand(cache);
>  		}
>  	}
>  
> -- 
> 2.55.0
> 
> 

Re: [PATCH 0/3 V3] libxfs: better cache allocation error handling

[Christoph Hellwig]

On Fri, Sep 25, 2026 at 02:41:42PM -0500, Eric Sandeen wrote:
> OK, 3rd try.
> 
> This is to handle an underlying problem that an actual ENOMEM cache
> node allocation failure is indistinguishable from "current cache full"
> and so ENOMEM may send us into a pointless cache-expansion loop which
> ultimately overflows the cache size variable, rather than just reporting
> the bad news up the chain to callers.
> 
> Darrick suggested "I think I'd almost rather we change cache_node_allocate
> to return int and the cache node as an outparam since we do that elsewhere"
> 
> on the last review cycle, and so I've done that here, I agree that it
> is cleaner.

Sorry for chiming in so late, but wouldn't it make more sense to keep
returning the node, and have a bool for what to if it is NULL?
The 0/1/-errno pattern already requires a lot of attention in the kernel,
but in userland where we have errno and -1 returns that are usually not
specifically checked it feels like even more trouble waiting to happen.

That being said, the changes themselves do look correct, so this should
not be a strong objection.

Re: [PATCH 0/3 V3] libxfs: better cache allocation error handling

[Eric Sandeen]

On 9/28/26 3:16 AM, Christoph Hellwig wrote:
> On Fri, Sep 25, 2026 at 02:41:42PM -0500, Eric Sandeen wrote:
>> OK, 3rd try.
>>
>> This is to handle an underlying problem that an actual ENOMEM cache
>> node allocation failure is indistinguishable from "current cache full"
>> and so ENOMEM may send us into a pointless cache-expansion loop which
>> ultimately overflows the cache size variable, rather than just reporting
>> the bad news up the chain to callers.
>>
>> Darrick suggested "I think I'd almost rather we change cache_node_allocate
>> to return int and the cache node as an outparam since we do that elsewhere"
>>
>> on the last review cycle, and so I've done that here, I agree that it
>> is cleaner.
> 
> Sorry for chiming in so late, but wouldn't it make more sense to keep
> returning the node, and have a bool for what to if it is NULL?
> The 0/1/-errno pattern already requires a lot of attention in the kernel,
> but in userland where we have errno and -1 returns that are usually not
> specifically checked it feels like even more trouble waiting to happen.

Well, "[PATCH 1/2] xfs_repair: distinguish true ENOMEM from "not found" in 
ache_node_get" did this, which is what I think you're describing?

 static struct cache_node *
 cache_node_allocate(
 	struct cache *		cache,
-	cache_key_t		key)
+	cache_key_t		key,
+	bool			*enomem)

*shrug* I really don't care at this point, both versions are on the list.

We can merge either one, or none of them. I don't think this is worth any
more effort. :)

> That being said, the changes themselves do look correct, so this should
> not be a strong objection.

Fair enough :)

Thanks,
-Eric

Re: [PATCH 2/3] libxfs: Return -ENOMEM on actual cache node allocation failures

[Eric Sandeen]

On 9/25/26 5:58 PM, Darrick J. Wong wrote:
> On Fri, Sep 25, 2026 at 02:41:44PM -0500, Eric Sandeen wrote:
>> Today, if we get an underlying ENOMEM from an actual cache node
>> allocation attempt, it's indistinguishable from "current cache size
>> is filled" and we'll keep doubling cache size to no avail until the
>> size itself overflows.
>>
>> This patch differentiates "no node returned" between the "cache full;
>> grow it" case and the "underlying allocation actually got ENOMEM" case
>> so that the caller can do the right thing, and return true ENOMEM
>> up the stack for callers to handle.
>>
>> Finally, actually check this returned error at the cache_node_get
>> callsite (in __cache_lookup).

...

>> @@ -464,8 +466,8 @@ next_object:
>>  		/*
>>  		 * not found, allocate a new entry
>>  		 */
>> -		cache_node_allocate(cache, key, &node);
>> -		if (node)
>> +		error = cache_node_allocate(cache, key, &node);
>> +		if (error > 0)
> 
> I think you could keep the node nullness check, though in the end it
> doesn't make much difference.  It's not like you can return
> Result<struct cache_node> or anything. :)

Hm, yeah, that would be nicer / more clear. If Andrey wants to merge this
version of the series, maybe it can be changed on merge.

Or maybe we just drop this whole thing and move on to more useful work. :)

> Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

Thanks,
-Eric

> --D