Use the system-wide zero page instead of a custom zero page.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

diff --git a/include/linux/raid/pq.h b/include/linux/raid/pq.h
index 98030accf641..1460c15dea63 100644
--- a/include/linux/raid/pq.h
+++ b/include/linux/raid/pq.h
@@ -11,8 +11,9 @@
 #ifdef __KERNEL__
 
 #include <linux/blkdev.h>
+#include <linux/mm.h>
 
-extern const char raid6_empty_zero_page[PAGE_SIZE];
+#define raid6_empty_zero_page ((const char *)page_address(ZERO_PAGE(0)))
 
 #else /* ! __KERNEL__ */
 /* Used for testing in user space */
diff --git a/lib/raid6/algos.c b/lib/raid6/algos.c
index cd2e88ee1f14..03f1a8c179f7 100644
--- a/lib/raid6/algos.c
+++ b/lib/raid6/algos.c
@@ -18,9 +18,6 @@
 #else
 #include <linux/module.h>
 #include <linux/gfp.h>
-/* In .bss so it's zeroed */
-const char raid6_empty_zero_page[PAGE_SIZE] __attribute__((aligned(256)));
-EXPORT_SYMBOL(raid6_empty_zero_page);
 #endif
 
 struct raid6_calls raid6_call;
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

On Sat, 15 Mar 2025 16:06:22 +0800 Herbert Xu <herbert@gondor.apana.org.au> wrote:

> Use the system-wide zero page instead of a custom zero page.
> 

I'll assume the MD maintainers will process this.

> --- a/include/linux/raid/pq.h
> +++ b/include/linux/raid/pq.h
> @@ -11,8 +11,9 @@
>  #ifdef __KERNEL__
>  
>  #include <linux/blkdev.h>
> +#include <linux/mm.h>
>  
> -extern const char raid6_empty_zero_page[PAGE_SIZE];
> +#define raid6_empty_zero_page ((const char *)page_address(ZERO_PAGE(0)))

This will of course meet the aligned(256) requirement. 

I do think it would be nicer to write this as a real inlined C function
and to convert usage sites to raid6_empty_zero_page().  IOW, let's tell
the truth rather than pretending that raid6_empty_zero_page is a global
variable.

>  #else /* ! __KERNEL__ */
>  /* Used for testing in user space */
> diff --git a/lib/raid6/algos.c b/lib/raid6/algos.c
> index cd2e88ee1f14..03f1a8c179f7 100644
> --- a/lib/raid6/algos.c
> +++ b/lib/raid6/algos.c
> @@ -18,9 +18,6 @@
>  #else
>  #include <linux/module.h>
>  #include <linux/gfp.h>
> -/* In .bss so it's zeroed */
> -const char raid6_empty_zero_page[PAGE_SIZE] __attribute__((aligned(256)));
> -EXPORT_SYMBOL(raid6_empty_zero_page);
>  #endif

Is there any possibility that the MD drivers will point DMA hardware at
the global zero page, thereby invalidating that page from CPU caches in
some manner?

On Sat, Mar 15, 2025 at 09:06:31PM -0700, Andrew Morton wrote:
>
> I do think it would be nicer to write this as a real inlined C function
> and to convert usage sites to raid6_empty_zero_page().  IOW, let's tell
> the truth rather than pretending that raid6_empty_zero_page is a global
> variable.

OK I can do that.
 
> Is there any possibility that the MD drivers will point DMA hardware at
> the global zero page, thereby invalidating that page from CPU caches in
> some manner?

The only spots that can hand this off to DMA are:

crypto/async_tx/async_pq.c:                     srcs[i] = (void*)raid6_empty_zero_page;
crypto/async_tx/async_raid6_recov.c:                            ptrs[i] = (void *) raid6_empty_zero_page;
crypto/async_tx/async_raid6_recov.c:                            ptrs[i] = (void*)raid6_empty_zero_page;

But they all turn out to be synchronous fallback code paths that
do not involve DMA at all.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

On Sun, 16 Mar 2025 13:51:50 +0800 Herbert Xu <herbert@gondor.apana.org.au> wrote:

> > Is there any possibility that the MD drivers will point DMA hardware at
> > the global zero page, thereby invalidating that page from CPU caches in
> > some manner?
> 
> The only spots that can hand this off to DMA are:

Cool, thanks for checking.

> crypto/async_tx/async_pq.c:                     srcs[i] = (void*)raid6_empty_zero_page;
> crypto/async_tx/async_raid6_recov.c:                            ptrs[i] = (void *) raid6_empty_zero_page;
> crypto/async_tx/async_raid6_recov.c:                            ptrs[i] = (void*)raid6_empty_zero_page;
> 
> But they all turn out to be synchronous fallback code paths that
> do not involve DMA at all.

And all three cast a pointer to a void* ;)