Re: [PATCH] ARM: mm: fix no-MMU ZERO_PAGE() implementation

[Giulio Benetti]

Hello Russell, Arnd, All,

On 19/10/22 11:09, Russell King (Oracle) wrote:
> On Tue, Oct 18, 2022 at 09:03:01AM +0200, Arnd Bergmann wrote:
> > In addition to your fix, I see that arm is the only architecture
> > that defines 'empty_zero_page' as a pointer to the page, when
> > everything else just makes it a pointer to the data itself,
> > or an 'extern char empty_zero_page[]' array, which we may want
> > to change for consistency.
>
> ARM's implementation is the utterly sensible implementation IMHO.
>
> When the only users in the kernel _were_ ZERO_PAGE() for this, which
> is defined to return a struct page pointer, there was no need to make
> "empty_zero_page" anything but a struct page pointer, rather than a
> runtime translation from an address to a struct page.
>
> IMHO, we should _not_ be exposing empty_zero_page to devices - we
> certainly do not want the DMA API performing cache maintenance on
> this page since the primary purpose of this page is to fill in
> userspace BSS pages that have not been written.
>
> ACPI's use is just to have a cookie for invalid handles, and using
> the struct page pointer is good enough.
>
> The only problem one is the RAID6 code, but that is disabled:
>
> /* Set to 1 to use kernel-wide empty_zero_page */
> #define RAID6_USE_EMPTY_ZERO_PAGE 0
>
> #if RAID6_USE_EMPTY_ZERO_PAGE
> # define raid6_empty_zero_page empty_zero_page
> #else
> extern const char raid6_empty_zero_page[PAGE_SIZE];
> #endif

For this I've sent a patch to remove the unused code:
https://lore.kernel.org/all/20221019160407.7550-1-giulio.benetti@xxxxxxxxxxxxxxxxxxxxxx/

> So, the only one that needs fixing is the SPI usage, which IMHO
> is wrong. ARM being different finds what I consider a driver bug.
> Good for 32-bit ARM. :)

Oh, I've sent a patch for substituting ZERO_PAGE(0) and it's already
been applied to spi's for-next:
https://lore.kernel.org/all/166619141690.565256.8563939546728659746.b4-ty@xxxxxxxxxx/
So this doesn't break the build but there is still a bug.

Just to understand if I've understood correctly. The correct fix would
be to kzalloc() a dma_dummy_tx buffer and use it in place of
ZERO_PAGE(0), right?

Can you also please point me some link explaining the structure of this
topic? I've already started to read:
https://docs.kernel.org/core-api/index.html#memory-management

Maybe it is enough. I'd appreciate a lot any further links to get into
this if any.

Thank you

Kind regards
--
Giulio Benetti
CEO/CTO@Benetti Engineering sas