mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] cache: Make cpu_cache_invalidate_memregion() error out if there are no handlers
@ 2026-10-01 11:00 Bence Csokas
  2026-10-02 17:27 ` Conor Dooley
  0 siblings, 1 reply; 2+ messages in thread
From: Bence Csokas @ 2026-10-01 11:00 UTC (permalink / raw)
  To: Andrew Morton, Conor Dooley, Jonathan Cameron, Yicong Yang
  Cc: linux-kernel, Jonathan Cameron, Bence Csokas

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.

Reviewed-by: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>
Signed-off-by: Bence Csokas <bence.csokas@arm.com>
---
I noticed cpu_cache_invalidate_memregion() returns 0 if there are no
registered `cache_coherency_ops_inst`s, which I find counter-intuitive.

From what I've seen, there are no in-tree users who do not check
cpu_cache_has_invalidate_memregion(). However, IMO the better solution
would be if the function would perform this check, and not the callee.

Furthermore, arch/x86/mm/pat/set_memory.c also implements this function,
but actually does the feature check, and WARNs and returns -ENXIO if it
fails that check.

This patch that provides a low-overhead fix.
---
Changes in v1:
- Rebased to master
- Collected tags
- Link to RFC: https://patch.msgid.link/20260922-cci_enxio-v1-0-00bade212729@arm.com

To: Conor Dooley <conor@kernel.org>
To: Jonathan Cameron <jic23@kernel.org>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: linux-kernel@vger.kernel.org
---
 lib/cache_maint.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/cache_maint.c b/lib/cache_maint.c
index 9256a9ffc34c..132d2816d6b1 100644
--- a/lib/cache_maint.c
+++ b/lib/cache_maint.c
@@ -59,12 +59,12 @@ static int cache_inval_done_one(struct cache_coherency_ops_inst *cci)
 
 static int cache_invalidate_memregion(phys_addr_t addr, size_t size)
 {
-	int ret;
 	struct cache_coherency_ops_inst *cci;
 	struct cc_inval_params params = {
 		.addr = addr,
 		.size = size,
 	};
+	int ret = -ENXIO;
 
 	guard(rwsem_read)(&cache_ops_instance_list_lock);
 	list_for_each_entry(cci, &cache_ops_instance_list, node) {
@@ -78,7 +78,7 @@ static int cache_invalidate_memregion(phys_addr_t addr, size_t size)
 			return ret;
 	}
 
-	return 0;
+	return ret;
 }
 
 struct cache_coherency_ops_inst *

---
base-commit: 551c722f40809618230001baccf219193e22fc5a
change-id: 20260921-cci_enxio-f3d31d9acec7

Best regards,
--  
Bence Csokas <bence.csokas@arm.com>


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] cache: Make cpu_cache_invalidate_memregion() error out if there are no handlers
  2026-10-01 11:00 [PATCH] cache: Make cpu_cache_invalidate_memregion() error out if there are no handlers Bence Csokas
@ 2026-10-02 17:27 ` Conor Dooley
  0 siblings, 0 replies; 2+ messages in thread
From: Conor Dooley @ 2026-10-02 17:27 UTC (permalink / raw)
  To: Bence Csokas
  Cc: Andrew Morton, Jonathan Cameron, Yicong Yang, linux-kernel,
	Jonathan Cameron

[-- Attachment #1: Type: text/plain, Size: 2817 bytes --]

On Thu, Oct 01, 2026 at 01:00:07PM +0200, Bence Csokas 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.
> 
> Reviewed-by: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>
> Signed-off-by: Bence Csokas <bence.csokas@arm.com>
> ---
> I noticed cpu_cache_invalidate_memregion() returns 0 if there are no
> registered `cache_coherency_ops_inst`s, which I find counter-intuitive.
> 
> >From what I've seen, there are no in-tree users who do not check
> cpu_cache_has_invalidate_memregion(). However, IMO the better solution
> would be if the function would perform this check, and not the callee.
> 
> Furthermore, arch/x86/mm/pat/set_memory.c also implements this function,
> but actually does the feature check, and WARNs and returns -ENXIO if it
> fails that check.
> 
> This patch that provides a low-overhead fix.

Hmm, I almost lost this. Guess my patchwork lei query needs an update.
Just happened to remember seeing this fly by!

> ---
> Changes in v1:
> - Rebased to master
> - Collected tags
> - Link to RFC: https://patch.msgid.link/20260922-cci_enxio-v1-0-00bade212729@arm.com
> 
> To: Conor Dooley <conor@kernel.org>
> To: Jonathan Cameron <jic23@kernel.org>
> To: Andrew Morton <akpm@linux-foundation.org>
> Cc: linux-kernel@vger.kernel.org
> ---
>  lib/cache_maint.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/cache_maint.c b/lib/cache_maint.c
> index 9256a9ffc34c..132d2816d6b1 100644
> --- a/lib/cache_maint.c
> +++ b/lib/cache_maint.c
> @@ -59,12 +59,12 @@ static int cache_inval_done_one(struct cache_coherency_ops_inst *cci)
>  
>  static int cache_invalidate_memregion(phys_addr_t addr, size_t size)
>  {
> -	int ret;
>  	struct cache_coherency_ops_inst *cci;
>  	struct cc_inval_params params = {
>  		.addr = addr,
>  		.size = size,
>  	};
> +	int ret = -ENXIO;
>  
>  	guard(rwsem_read)(&cache_ops_instance_list_lock);
>  	list_for_each_entry(cci, &cache_ops_instance_list, node) {
> @@ -78,7 +78,7 @@ static int cache_invalidate_memregion(phys_addr_t addr, size_t size)
>  			return ret;
>  	}
>  
> -	return 0;
> +	return ret;
>  }
>  
>  struct cache_coherency_ops_inst *
> 
> ---
> base-commit: 551c722f40809618230001baccf219193e22fc5a
> change-id: 20260921-cci_enxio-f3d31d9acec7
> 
> Best regards,
> --  
> Bence Csokas <bence.csokas@arm.com>
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-10-02 17:27 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-01 11:00 [PATCH] cache: Make cpu_cache_invalidate_memregion() error out if there are no handlers Bence Csokas
2026-10-02 17:27 ` Conor Dooley

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®