Re: [PATCH v13 1/2] ACPI:RAS2: Add driver for the ACPI RAS2 feature table

Jonathan Cameron
2025-12-08T16:29:29Z

On Tue, 25 Nov 2025 08:36:27 +0100
Borislav Petkov <bp@alien8.de> wrote:

> On Fri, Nov 21, 2025 at 06:28:20PM +0000, shiju.jose@huawei.com wrote:
> > From: Shiju Jose <shiju.jose@huawei.com>
> >
> > ACPI 6.5 Specification, section 5.2.21, defined RAS2 feature table (RAS2).
> > Driver adds support for RAS2 feature table, which provides interfaces for
> > platform RAS features, for eg. HW-based memory scrubbing, and logical to
> > PA translation service. RAS2 uses PCC channel subspace for communicating
> > with the ACPI compliant HW platform.
> >
> > Co-developed-by: A Somasundaram <somasundaram.a@hpe.com>
> > Signed-off-by: A Somasundaram <somasundaram.a@hpe.com>
> > Co-developed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > Tested-by: Daniel Ferguson <danielf@os.amperecomputing.com>
> > Signed-off-by: Shiju Jose <shiju.jose@huawei.com>

Just picking out one point where I disagree.

>
> > +
> > + struct ras2_mem_ctx **pctx_list __free(kfree) = kzalloc(ras2_tab->num_pcc_descs * sizeof(*pctx_list), GFP_KERNEL);
>
> Function member declarations at the beginning of the function, pls, and then
> you can remove this ugly linebreak too.

No to this suggestion. Doing so breaks the rules that Linus has laid
out for use of __free. Constructor and destructor must be next to each
other. See guidance in cleanup.h. This stuff is new, so it's perfectly
fine for a subsystem maintainer to say they don't want it used, but
if it is used then it should be done according to the agreed style.

Various folk have already experienced a grumpy Linus when they put the
declarations separate from the constructor.

FWIW there is a long thread on the ksummit list that repeats and
extends the discussions that lead to the guidance notes in the header.

https://lore.kernel.org/ksummit/B92B21C0-093D-4F52-A7E3-1A7DDC83749B@zytor.com/T/#t

Jonathan