On Sun, May 16, 2021 at 11:27:10AM -0700, Linus Torvalds wrote:
On Sun, May 16, 2021 at 11:22 AM Matthew Wilcox willy@infradead.org wrote:
Nobody's been willing to guarantee that all 32-bit architectures keep the top 20 bits clear for their DMA addresses. I've certainly seen hardware (maybe PA-RISC? MIPS?) which uses the top few bits of the DMA address to indicate things like "coherent" or "bypasses IOMMU". Rather than trying to find out, I thought this was the safer option.
Fair enough. I just find it somewhat odd.
But I still find this a bit ugly. Maybe we could just have made that one sub-structure "__aligned(4)", and avoided this all, and let the compiler generate the split load (or, more likely, just let the compiler generate a regular load from an unaligned location).
IOW, just
struct { /* page_pool used by netstack */ /** * @dma_addr: might require a 64-bit value even on * 32-bit architectures. */ dma_addr_t dma_addr; } __aligned((4));without the magic shifting games?
That was the other problem fixed by this patch -- on big-endian 32-bit platforms with 64-bit dma_addr_t (mips, ppc), a DMA address with bit 32 set inadvertently sets the PageTail bit. So we need to store the low bits in the first word, even on big-endian platforms.
There's an upcoming patch to move dma_addr out of the union with compound_head, but that requires changing page_is_pfmemalloc() to use an indicator other than page->index == -1. Once we do that, we can have fun with __aligned(). Since we knew it would have to be backported, this felt like the least risky patch to start with.