Re: [PATCH RFC] cache: Make cpu_cache_invalidate_memregion() error out if there are no handlers

[Jonathan Cameron <jic23@kernel.org>]

On Wed, 23 Sep 2026 10:21:15 +0200
Bence Csókás <bence.csokas@arm.com> wrote:

> Hi,
> 
> On 2026. 09. 23. 4:18, Andrew Morton wrote:
> > On Tue, 22 Sep 2026 09:29:31 +0200 Bence Csokas <bence.csokas@arm.com> wrote:
> >   
> >> Currently, if there are no registered handlers,
> >> cpu_cache_invalidate_memregion() returns 0. Callers are expected to check
> >> cpu_cache_has_invalidate_memregion() and return -ENXIO themselves, instead
> >> of calling this function. If a caller forgets to do this, they may
> >> erroneously think the operation succeeded. This is in contrast to x86,
> >> where cpu_cache_invalidate_memregion() itself checks if the operation is
> >> supported, and returns -ENXIO if it is not.  
> > 
> > Do you expect any callers to be altered as a result of this change?
> > 
> > If so, full details would be helpful.  
> 
> Current callers, not necessarily. Future callers, maybe.

There is a race condition if someone unbinds the handlers that might
expose an error after this (correctly) when it didn't before.

In general that race exists in the setup path as well because we don't have
any way to know 'how many' devices to expect with assumption being
they have all turned up if any have.  Not great, but short of adding
something to the firmware specification there wasn't a solution and
that process would be slow.  We can revisit that if it becomes a problem
in practice.


> 
> Presently, as far as I see, there are 2 consumers: CXL and NVDIMM, with 
> 3 call sites in total.
> 
> 2 of these call sites [1] [2] are in the form of:
> 
> 	if (!has_invalidate()) {
> 		if (TEST)
> 			return 0;
> 		return -ENXIO;
> 	}
> 	invalidate();
> 
> These types of call sites can stay as-is.
> 
> The remaining one [3] is in a void function, and in the form:
> 
> 	if (has_invalidate())
> 		invalidate();
> 
> Lastly, while not a consumer, here's what x86 [4] does. It's possible a 
> future caller will want to replicate this:
> 
> 	if (!has_invalidate())
> 		return -ENXIO;
> 	invalidate();
> 
> These types of call sites can do away with calling has_invalidate() 
> going forward.

I'd be against any callers doing that as would rely on future implementations
of the interface.  No support vs got an error tend to need different handling
and relying on particular error codes is likely to bite us eventually.


Jonathan

> 
> [1] 
> https://elixir.bootlin.com/linux/v7.3-rc3/source/drivers/cxl/core/region.c#L242
> [2] 
> https://elixir.bootlin.com/linux/v7.3-rc3/source/drivers/nvdimm/region_devs.c#L93
> [3] 
> https://elixir.bootlin.com/linux/v7.3-rc3/source/drivers/nvdimm/region.c#L113
> [4] 
> https://elixir.bootlin.com/linux/v7.3-rc3/source/arch/x86/mm/pat/set_memory.c#L372
> 
> Bence