From: Alison Schofield <alison.schofield@intel.com>
To: Rosen Penev <rosenp@gmail.com>
Cc: <nvdimm@lists.linux.dev>, Dan Williams <dan.j.williams@intel.com>,
"Vishal Verma" <vishal.l.verma@intel.com>,
Dave Jiang <dave.jiang@intel.com>,
"Ira Weiny" <ira.weiny@intel.com>,
open list <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] nvdimm: use struct_size for allocation
Date: Fri, 27 Mar 2026 10:53:50 -0700 [thread overview]
Message-ID: <acbELuIOm6GTRRGC@aschofie-mobl2.lan> (raw)
In-Reply-To: <20260327025251.7688-1-rosenp@gmail.com>
On Thu, Mar 26, 2026 at 07:52:51PM -0700, Rosen Penev wrote:
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
This is not a mechanical struct_size() conversion. It changes the
allocation math for flush_wpq but there is no commit log explanation
of why that change is correct or safe. There's no commit log at all.
This code in not just counting bytes. It encodes assumptions about
the no-hint case and the per-DIMM indexing model. If you intend to
change that, please study those paths and explain why the new math
preserves correctness across all topologies.
> ---
> drivers/nvdimm/region_devs.c | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/nvdimm/region_devs.c b/drivers/nvdimm/region_devs.c
> index e35c2e18518f..1350a34a34ce 100644
> --- a/drivers/nvdimm/region_devs.c
> +++ b/drivers/nvdimm/region_devs.c
> @@ -104,7 +104,7 @@ static int nd_region_invalidate_memregion(struct nd_region *nd_region)
>
> static int get_flush_data(struct nd_region *nd_region, size_t *size, int *num_flush)
> {
> - size_t flush_data_size = sizeof(void *);
> + size_t flush_data_size = 0;
> int _num_flush = 0;
> int i;
>
> @@ -117,11 +117,10 @@ static int get_flush_data(struct nd_region *nd_region, size_t *size, int *num_fl
> return -EBUSY;
>
> /* at least one null hint slot per-dimm for the "no-hint" case */
> - flush_data_size += sizeof(void *);
> _num_flush = min_not_zero(_num_flush, nvdimm->num_flush);
> if (!nvdimm->num_flush)
> continue;
> - flush_data_size += nvdimm->num_flush * sizeof(void *);
> + flush_data_size += nvdimm->num_flush;
> }
>
> *size = flush_data_size;
> @@ -145,7 +144,7 @@ int nd_region_activate(struct nd_region *nd_region)
> if (rc)
> return rc;
>
> - ndrd = devm_kzalloc(dev, sizeof(*ndrd) + flush_data_size, GFP_KERNEL);
> + ndrd = devm_kzalloc(dev, struct_size(ndrd, flush_wpq, flush_data_size), GFP_KERNEL);
> if (!ndrd)
> return -ENOMEM;
> dev_set_drvdata(dev, ndrd);
> --
> 2.53.0
>
>
prev parent reply other threads:[~2026-03-27 17:54 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-03-27 2:52 [PATCH] nvdimm: use struct_size for allocation Rosen Penev
2026-03-27 17:53 ` Alison Schofield [this message]
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=acbELuIOm6GTRRGC@aschofie-mobl2.lan \
--to=alison.schofield@intel.com \
--cc=dan.j.williams@intel.com \
--cc=dave.jiang@intel.com \
--cc=ira.weiny@intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=nvdimm@lists.linux.dev \
--cc=rosenp@gmail.com \
--cc=vishal.l.verma@intel.com \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox