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

["Bence Csókás" <bence.csokas@arm.com>]

Hi,

On 2026. 09. 23. 19:31, Jonathan Cameron wrote:
> 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.

Yes, if the callers need to handle these separate cases (as in call 
sites [1] and [2]), then for sure keep the existing logic. But some call 
sites may not need to handle these cases differently, and will simply 
return -ENXIO if they find no support. In those cases, they may simply do

	return cpu_cache_invalidate_memregion();

(I now realize that I forgot the `return` in my previous post, sorry for 
the confusion.)

But at the end of the day, I don't have any strong opinion on this. For 
now, current callers can stay as-is, and for future callers, we'll 
decide in that thread.

> 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
> 

So the consensus is that my hunch was correct, and this is something to 
address? Should I resubmit this patch as non-RFC then?

Bence