Re: [PATCH v12 1/2] ACPI:RAS2: Add ACPI RAS2 driver

[Borislav Petkov <bp@alien8.de>]

On Mon, Sep 15, 2025 at 11:50:16AM +0000, Shiju Jose wrote:
> This has been added as suggested by Jonathan considering the interleaved NUMA node.
> Link to the related discussion in V11:
> https://lore.kernel.org/all/20250821100655.00003942@huawei.com/#t

Sorry, this doesn't work this way.

If something in the code is being done which is not obvious and trivial, then
the reason for it is written down in a prominent place so that it is clear to
people.

Not pointing to a discussion or some funky place on the web where someone
might've said something.

Your patch submission should contain that info and not have reviewers ask for
it.

> | node 0 | node 1 | node 0 |   PA address map.
> Can you give your suggestion what we should do about it?

I don't know what the problem is to begin with...

> I think Option (2) seems better?  If so, can the EDAC scrub interface  be
> updated to include attributes for publishing the supported PA range for the
> memory device to scrub?

The memory ranges should already be available somewhere in the NUMA/mm code or
so and for starters, we should start a scrub for all ranges and do the
single-range only when there really is a good reason for it.

Also, you don't have to expose any ranges to userspace in order to start
a scrub activity - you can simply start the scrub in the affected range
automatically.

Like I preached the last time, your aim should be to make as much of the
variables that control the scrub automatic and not expose everything to
userspace so that some userspace tool decides. The tool should simply start
the scrub and the kernel should DTRT.

> This returns error on the first failure.
> 
> What if there was a success before? Does that aux_device need to be removed?
> 
> If not, then why return failure at all? Why not just try to add all devices? Some may fail and some may succeed.
> ============================= 
> 
> We thought second option is a better because a successfully added aux dev for a memory device and corresponding
> EDAC interface continue exist and support the scrub/a memory feature. 
> We do not mind doing stop on a failure adding an aux_device and free previously crated aux devices, though
> it may require some additional dynamically allocated memory space to store the successfully created aux devices
> so that free them on a failure later. Hope that is acceptable?

So how are you going to present to people a subset of devices loaded? And what
is the point at all? 

Is there a valid use case where you can use only a subset of the devices to
even try to support such nonsense?

-- 
Regards/Gruss,
    Boris.

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