Re: [PATCH 3/6] md/raid5: scale the stripe_cache_size limit with system memory

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Integer overflow in `raid5_max_cache_size` on 32-bit platforms with 4GB+ RAM.
--

--- Patch [3]: [PATCH 3/6] md/raid5: scale the stripe_cache_size limit with system memory ---
commit 9086a4a6d9e318381885104d84e3f0ec9bbeec13
Author: Hiroshi Nishida <[email protected]>

md/raid5: scale the stripe_cache_size limit with system memory

This commit derives the upper bound for the per-array stripe_cache_size from
system memory rather than a fixed limit. It scales the maximum cache size to
1/8 of RAM, allowing large-memory hosts to grow the cache while preventing
small systems from exceeding total RAM.

> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index 7f72981121fd5..e41d3fc92dd05 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c

[ ... ]

> @@ -6922,13 +6928,42 @@ raid5_show_stripe_cache_size(struct mddev *mddev, char *page)
>  	return ret;
>  }
>  
> +/*
> + * Upper bound that the per-array stripe_cache_size may be raised to.  The
> + * stripe cache costs roughly max_nr_stripes * (sizeof(stripe_head) +
> + * pool_size * (sizeof(bio) + PAGE_SIZE)).  The limit was historically a fixed
> + * 32768 stripes, which both under-serves large-memory hosts backing wide
> + * arrays and, on a small box, still permits a cache larger than RAM.  Derive
> + * it from memory instead -- at most 1/8 of RAM -- but never below the
> + * historical 32768, so the limit only ever grows relative to today.  A
> + * non-zero stripe_cache_size_max module parameter overrides the heuristic.
> + */
> +#define RAID5_CACHE_SIZE_FLOOR		32768
> +#define RAID5_CACHE_SIZE_RAM_SHIFT	3	/* cap the cache at 1/8 of RAM */
> +
> +static unsigned long raid5_max_cache_size(struct r5conf *conf)
> +{
> +	unsigned long per_stripe, limit;
> +
> +	if (stripe_cache_size_max) {
> +		limit = stripe_cache_size_max;
> +	} else {
> +		per_stripe = sizeof(struct stripe_head) +
> +			     conf->pool_size * (sizeof(struct bio) + PAGE_SIZE);
> +		limit = ((totalram_pages() << PAGE_SHIFT) >>
> +			 RAID5_CACHE_SIZE_RAM_SHIFT) / per_stripe;

[Severity: Medium]
Will this calculation overflow on 32-bit platforms with 4GB or more of RAM?

Since totalram_pages() returns an unsigned long, on a 32-bit system with high
memory (e.g., using PAE), totalram_pages() can be 1,048,576 or more.
Left-shifting this by PAGE_SHIFT (12) will overflow the 32-bit integer,
yielding a severely truncated value or 0.

This seems like it would cause the dynamic limit to inappropriately clamp to
the RAID5_CACHE_SIZE_FLOOR on such systems, breaking the intended memory-based
scaling.

> +		limit = max_t(unsigned long, limit, RAID5_CACHE_SIZE_FLOOR);
> +	}
> +	return min_t(unsigned long, limit, INT_MAX);
> +}
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3