[PATCHv3] EDAC/mc: Use kzalloc_flex()

[PATCHv3] EDAC/mc: Use kzalloc_flex()

[Rosen Penev]

Convert struct mem_ctl_info to use flex array and use the new flex
array helpers to simplify initialization.

Add __counted_by for extra runtime analysis when requested.

Move counting assignment immediately after allocation as required by
__counted_by.

Move memcpy to make it clear that this should have been kmemdup_array.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
Reviewed-by: Yazen Ghannam <yazen.ghannam@amd.com>
Reviewed-by: Qiuxu Zhuo <qiuxu.zhuo@intel.com>
---
 v3: move memcpy after n_layers assignment
 v2: reword message slightly.
 drivers/edac/edac_mc.c | 10 +++-------
 include/linux/edac.h   | 23 ++++++++++++-----------
 2 files changed, 15 insertions(+), 18 deletions(-)

diff --git a/drivers/edac/edac_mc.c b/drivers/edac/edac_mc.c
index 29e9828422bb..07d3f73bcd23 100644
--- a/drivers/edac/edac_mc.c
+++ b/drivers/edac/edac_mc.c
@@ -203,7 +203,6 @@ static void mci_release(struct device *dev)
 		kfree(mci->csrows);
 	}
 	kfree(mci->pvt_info);
-	kfree(mci->layers);
 	kfree(mci);
 }

@@ -361,13 +360,12 @@ struct mem_ctl_info *edac_mc_alloc(unsigned int mc_num,
 			per_rank = true;
 	}

-	mci = kzalloc_obj(struct mem_ctl_info);
+	mci = kzalloc_flex(*mci, layers, n_layers);
 	if (!mci)
 		return NULL;

-	mci->layers = kzalloc_objs(struct edac_mc_layer, n_layers);
-	if (!mci->layers)
-		goto error;
+	mci->n_layers = n_layers;
+	memcpy(mci->layers, layers, sizeof(*layer) * n_layers);

 	mci->pvt_info = kzalloc(sz_pvt, GFP_KERNEL);
 	if (!mci->pvt_info)
@@ -379,8 +377,6 @@ struct mem_ctl_info *edac_mc_alloc(unsigned int mc_num,
 	/* setup index and various internal pointers */
 	mci->mc_idx = mc_num;
 	mci->tot_dimms = tot_dimms;
-	mci->n_layers = n_layers;
-	memcpy(mci->layers, layers, sizeof(*layer) * n_layers);
 	mci->nr_csrows = tot_csrows;
 	mci->num_cschannel = tot_channels;
 	mci->csbased = per_rank;
diff --git a/include/linux/edac.h b/include/linux/edac.h
index fa32f2aca22f..deba46b3ee25 100644
--- a/include/linux/edac.h
+++ b/include/linux/edac.h
@@ -541,17 +541,6 @@ struct mem_ctl_info {
 	struct csrow_info **csrows;
 	unsigned int nr_csrows, num_cschannel;

-	/*
-	 * Memory Controller hierarchy
-	 *
-	 * There are basically two types of memory controller: the ones that
-	 * sees memory sticks ("dimms"), and the ones that sees memory ranks.
-	 * All old memory controllers enumerate memories per rank, but most
-	 * of the recent drivers enumerate memories per DIMM, instead.
-	 * When the memory controller is per rank, csbased is true.
-	 */
-	unsigned int n_layers;
-	struct edac_mc_layer *layers;
 	bool csbased;

 	/*
@@ -609,6 +598,18 @@ struct mem_ctl_info {
 	u8 fake_inject_layer[EDAC_MAX_LAYERS];
 	bool fake_inject_ue;
 	u16 fake_inject_count;
+
+	/*
+	 * Memory Controller hierarchy
+	 *
+	 * There are basically two types of memory controller: the ones that
+	 * sees memory sticks ("dimms"), and the ones that sees memory ranks.
+	 * All old memory controllers enumerate memories per rank, but most
+	 * of the recent drivers enumerate memories per DIMM, instead.
+	 * When the memory controller is per rank, csbased is true.
+	 */
+	unsigned int n_layers;
+	struct edac_mc_layer layers[] __counted_by(n_layers);
 };

 #define mci_for_each_dimm(mci, dimm)				\
--
2.53.0

Re: [PATCHv3] EDAC/mc: Use kzalloc_flex()

[Borislav Petkov]

On Thu, Mar 26, 2026 at 07:48:28PM -0700, Rosen Penev wrote:
> Convert struct mem_ctl_info to use flex array and use the new flex
> array helpers to simplify initialization.

Meh, simplify... rather to teach an old dog like me new tricks... :)

> Add __counted_by for extra runtime analysis when requested.
> 
> Move counting assignment immediately after allocation as required by
> __counted_by.

So far so good.

> Move memcpy to make it clear that this should have been kmemdup_array.

But you lost me here. What do you mean with the "should have been
kmemdup_array"?

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCHv3] EDAC/mc: Use kzalloc_flex()

[Rosen Penev]

On Mon, Mar 30, 2026 at 8:15 AM Borislav Petkov <bp@alien8.de> wrote:
>
> On Thu, Mar 26, 2026 at 07:48:28PM -0700, Rosen Penev wrote:
> > Convert struct mem_ctl_info to use flex array and use the new flex
> > array helpers to simplify initialization.
>
> Meh, simplify... rather to teach an old dog like me new tricks... :)
>
> > Add __counted_by for extra runtime analysis when requested.
> >
> > Move counting assignment immediately after allocation as required by
> > __counted_by.
>
> So far so good.
>
> > Move memcpy to make it clear that this should have been kmemdup_array.
>
> But you lost me here. What do you mean with the "should have been
> kmemdup_array"?
kzalloc_flex == kzalloc + kcalloc.
kmemdup == kzalloc + memcpy.
kmemdup_array == kcalloc + memcpy.

So all together,
kzalloc + kcalloc + memcpy.

Or

kzalloc_flex + memcpy.

Placing the memcpy right after counting variable assignment enables
coccinelle to transform this whole thing into a single

kmemdup_flex

call if such a thing were to eventually be introduced.
>
> Thx.
>
> --
> Regards/Gruss,
>     Boris.
>
> https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCHv3] EDAC/mc: Use kzalloc_flex()

[Borislav Petkov]

On Mon, Mar 30, 2026 at 01:49:36PM -0700, Rosen Penev wrote:
> call if such a thing were to eventually be introduced.

Oh, hypothetical what-if. Yeah, pls drop all that gunk from commit messages
pls and simply concentrate on why the patch exists.

But you don't have to for this one - I'll massage it.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCHv3] EDAC/mc: Use kzalloc_flex()

[Rosen Penev]

On Mon, Mar 30, 2026 at 2:11 PM Borislav Petkov <bp@alien8.de> wrote:
>
> On Mon, Mar 30, 2026 at 01:49:36PM -0700, Rosen Penev wrote:
> > call if such a thing were to eventually be introduced.
>
> Oh, hypothetical what-if. Yeah, pls drop all that gunk from commit messages
> pls and simply concentrate on why the patch exists.
Not sure what you mean.
>
> But you don't have to for this one - I'll massage it.
Feel free.
>
> Thx.
>
> --
> Regards/Gruss,
>     Boris.
>
> https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCHv3] EDAC/mc: Use kzalloc_flex()

[Borislav Petkov]

On Mon, Mar 30, 2026 at 02:26:10PM -0700, Rosen Penev wrote:
> > Oh, hypothetical what-if. Yeah, pls drop all that gunk from commit messages
> > pls and simply concentrate on why the patch exists.
> Not sure what you mean.

I mean this: a commit message should simply state why a patch exists. Here's
what I did with yours:

    Convert struct mem_ctl_info to use flex array and use the new flex array
    helpers to enable runtime bounds checking, including annotating the array
    length member with __counted_by() for extra runtime analysis when requested.
    
    Move counter assignment immediately after allocation as required by
    __counted_by().
    
    Move memcpy() after the counter assignment so that it is initialized before
    the first reference to the flex array, as the new attribute requires.

The idea is that when one reads the commit message months, or even years from
now - something we all have to do on a daily basis - it should have all the
necessary information why the change was done.

And nothing else. The emphasis being on the latter part. In this case, what
this should have been and what some tools can do when lines are magically
ordered doesn't really matter. The change must be worth to exist for itself.
In this case, runtime bounds checking, which is something we all want.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCHv3] EDAC/mc: Use kzalloc_flex()

[Rosen Penev]

On Mon, Mar 30, 2026 at 3:32 PM Borislav Petkov <bp@alien8.de> wrote:
>
> On Mon, Mar 30, 2026 at 02:26:10PM -0700, Rosen Penev wrote:
> > > Oh, hypothetical what-if. Yeah, pls drop all that gunk from commit messages
> > > pls and simply concentrate on why the patch exists.
> > Not sure what you mean.
>
> I mean this: a commit message should simply state why a patch exists. Here's
> what I did with yours:
>
>     Convert struct mem_ctl_info to use flex array and use the new flex array
>     helpers to enable runtime bounds checking, including annotating the array
>     length member with __counted_by() for extra runtime analysis when requested.
>
>     Move counter assignment immediately after allocation as required by
>     __counted_by().
>
>     Move memcpy() after the counter assignment so that it is initialized before
>     the first reference to the flex array, as the new attribute requires.
Looks great.
>
> The idea is that when one reads the commit message months, or even years from
> now - something we all have to do on a daily basis - it should have all the
> necessary information why the change was done.
>
> And nothing else. The emphasis being on the latter part. In this case, what
> this should have been and what some tools can do when lines are magically
> ordered doesn't really matter. The change must be worth to exist for itself.
> In this case, runtime bounds checking, which is something we all want.
>
> Thx.
>
> --
> Regards/Gruss,
>     Boris.
>
> https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCHv3] EDAC/mc: Use kzalloc_flex()

[Borislav Petkov]

On Mon, Mar 30, 2026 at 04:25:26PM -0700, Rosen Penev wrote:
> On Mon, Mar 30, 2026 at 3:32 PM Borislav Petkov <bp@alien8.de> wrote:
> >
> > On Mon, Mar 30, 2026 at 02:26:10PM -0700, Rosen Penev wrote:
> > > > Oh, hypothetical what-if. Yeah, pls drop all that gunk from commit messages
> > > > pls and simply concentrate on why the patch exists.
> > > Not sure what you mean.
> >
> > I mean this: a commit message should simply state why a patch exists. Here's
> > what I did with yours:
> >
> >     Convert struct mem_ctl_info to use flex array and use the new flex array
> >     helpers to enable runtime bounds checking, including annotating the array
> >     length member with __counted_by() for extra runtime analysis when requested.
> >
> >     Move counter assignment immediately after allocation as required by
> >     __counted_by().
> >
> >     Move memcpy() after the counter assignment so that it is initialized before
> >     the first reference to the flex array, as the new attribute requires.
> Looks great.
> >
> > The idea is that when one reads the commit message months, or even years from
> > now - something we all have to do on a daily basis - it should have all the
> > necessary information why the change was done.
> >
> > And nothing else. The emphasis being on the latter part. In this case, what
> > this should have been and what some tools can do when lines are magically
> > ordered doesn't really matter. The change must be worth to exist for itself.
> > In this case, runtime bounds checking, which is something we all want.

Applied, thanks.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCHv3] EDAC/mc: Use kzalloc_flex()

[Gustavo A. R. Silva]

On 3/31/26 06:18, Borislav Petkov wrote:
> On Mon, Mar 30, 2026 at 04:25:26PM -0700, Rosen Penev wrote:
>> On Mon, Mar 30, 2026 at 3:32 PM Borislav Petkov <bp@alien8.de> wrote:
>>>
>>> On Mon, Mar 30, 2026 at 02:26:10PM -0700, Rosen Penev wrote:
>>>>> Oh, hypothetical what-if. Yeah, pls drop all that gunk from commit messages
>>>>> pls and simply concentrate on why the patch exists.
>>>> Not sure what you mean.
>>>
>>> I mean this: a commit message should simply state why a patch exists. Here's
>>> what I did with yours:
>>>
>>>      Convert struct mem_ctl_info to use flex array and use the new flex array
>>>      helpers to enable runtime bounds checking, including annotating the array
>>>      length member with __counted_by() for extra runtime analysis when requested.
>>>
>>>      Move counter assignment immediately after allocation as required by
>>>      __counted_by().

This is misinformation and should be phrased differently[1]

-Gustavo

[1] https://lore.kernel.org/linux-hardening/37378f49-437f-438b-ad6c-d60480feb306@embeddedor.com/

>>>
>>>      Move memcpy() after the counter assignment so that it is initialized before
>>>      the first reference to the flex array, as the new attribute requires.
>> Looks great.
>>>
>>> The idea is that when one reads the commit message months, or even years from
>>> now - something we all have to do on a daily basis - it should have all the
>>> necessary information why the change was done.
>>>
>>> And nothing else. The emphasis being on the latter part. In this case, what
>>> this should have been and what some tools can do when lines are magically
>>> ordered doesn't really matter. The change must be worth to exist for itself.
>>> In this case, runtime bounds checking, which is something we all want.
> 
> Applied, thanks.
> 

Re: [PATCHv3] EDAC/mc: Use kzalloc_flex()

[Borislav Petkov]

On Tue, Mar 31, 2026 at 09:28:47AM -0600, Gustavo A. R. Silva wrote:
> This is misinformation and should be phrased differently[1]
> 
> -Gustavo
> 
> [1] https://lore.kernel.org/linux-hardening/37378f49-437f-438b-ad6c-d60480feb306@embeddedor.com/

Yah, I was wondering about that but thought there are some other requirements
I don't know about.

Btw, can you do me a favor pls and summarize your very good blog article:

https://people.kernel.org/gustavoars/how-to-use-the-new-counted_by-attribute-in-c-and-linux

into a text file in Documentation/ so that we have the valid requirements for
using this compiler glue properly, in the kernel?

Thx.

Now lemme zap that line.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette