| Message ID | Z9U0_uj1E2MlYhGx@gondor.apana.org.au (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | lib/raid6: Replace custom zero page with ZERO_PAGE | expand |
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,
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* ;)
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;
Use the system-wide zero page instead of a custom zero page. Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>