Re: [PATCH 1/1] mm: Fix struct page layout on 32-bit systems

From: Ilias Apalodimas
Date: Mon Apr 19 2021 - 03:16:12 EST


Hi Christoph,

On Mon, Apr 19, 2021 at 08:34:41AM +0200, Christoph Hellwig wrote:
> On Fri, Apr 16, 2021 at 04:27:55PM +0100, Matthew Wilcox wrote:
> > On Thu, Apr 15, 2021 at 08:08:32PM +0200, Jesper Dangaard Brouer wrote:
> > > See below patch. Where I swap32 the dma address to satisfy
> > > page->compound having bit zero cleared. (It is the simplest fix I could
> > > come up with).
> >
> > I think this is slightly simpler, and as a bonus code that assumes the
> > old layout won't compile.
>
> So, why do we even do this crappy overlay of a dma address? This just
> all seems like a giant hack. Random subsystems should not just steal
> a few struct page fields as that just turns into the desasters like the
> one we've seen here or probably something worse next time.

The page pool API was using page->private in the past to store these kind of
info. That caused a problem to begin with, since it would fail on 32-bit
systems with 64bit DMA. We had a similar discussion on the past but decided
struct page is the right place to store that [1].

Another advantage is that we can now use the information from the networking
subsystem and enable recycling of SKBs and SKB fragments, by using the stored
metadata of struct page [2].

[1] https://lore.kernel.org/netdev/20190207.132519.1698007650891404763.davem@xxxxxxxxxxxxx/
[2] https://lore.kernel.org/netdev/20210409223801.104657-1-mcroce@xxxxxxxxxxxxxxxxxxx/

Cheers
/Ilias