All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Hiroshi Nishida" <nishidafmly@gmail.com>
Cc: linux-raid@vger.kernel.org, yukuai@fygo.io
Subject: Re: [PATCH 3/6] md/raid5: scale the stripe_cache_size limit with system memory
Date: Fri, 10 Jul 2026 13:34:53 +0000	[thread overview]
Message-ID: <20260710133453.E022F1F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260710132346.7295-4-nishidafmly@gmail.com>

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 <nishidafmly@gmail.com>

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/20260710132346.7295-1-nishidafmly@gmail.com?part=3

  reply	other threads:[~2026-07-10 13:34 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-10 13:23 [PATCH 0/6] md/raid5: size stripe-cache and worker tuning from the hardware Hiroshi Nishida
2026-07-10 13:23 ` [PATCH 1/6] md/raid5: size the worker group array by nr_node_ids Hiroshi Nishida
2026-07-10 13:39   ` sashiko-bot
2026-07-10 13:23 ` [PATCH 2/6] md/raid5: size stripe-cache hash locks from the CPU count Hiroshi Nishida
2026-07-10 13:41   ` sashiko-bot
2026-07-10 13:23 ` [PATCH 3/6] md/raid5: scale the stripe_cache_size limit with system memory Hiroshi Nishida
2026-07-10 13:34   ` sashiko-bot [this message]
2026-07-10 13:23 ` [PATCH 4/6] md/raid5: make the stripe batch size a module parameter Hiroshi Nishida
2026-07-10 13:36   ` sashiko-bot
2026-07-10 13:23 ` [PATCH 5/6] md/raid5: scale the default stripe cache size with system memory Hiroshi Nishida
2026-07-10 13:37   ` sashiko-bot
2026-07-10 13:23 ` [PATCH 6/6] md/raid5: derive the default group_thread_cnt from the hardware Hiroshi Nishida
2026-07-10 13:42   ` sashiko-bot

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=20260710133453.E022F1F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=linux-raid@vger.kernel.org \
    --cc=nishidafmly@gmail.com \
    --cc=sashiko-reviews@lists.linux.dev \
    --cc=yukuai@fygo.io \
    /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.