| Message ID | 155002294008.5597.13759027075590385810.stgit@firesoul (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Fix page_pool API and dma address storage | expand |
On Wed, Feb 13, 2019 at 02:55:40AM +0100, Jesper Dangaard Brouer wrote: > The page_pool API is using page->private to store DMA addresses. > As pointed out by David Miller we can't use that on 32-bit architectures > with 64-bit DMA > > This patch adds a new dma_addr_t struct to allow storing DMA addresses > > Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com> > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> > Acked-by: Andrew Morton <akpm@linux-foundation.org> > --- > include/linux/mm_types.h | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h > index 2c471a2c43fa..0a36a22228e7 100644 > --- a/include/linux/mm_types.h > +++ b/include/linux/mm_types.h > @@ -95,6 +95,13 @@ struct page { > */ > unsigned long private; > }; > + struct { /* page_pool used by netstack */ > + /** > + * @dma_addr: might require a 64-bit value even on > + * 32-bit architectures. > + */ > + dma_addr_t dma_addr; > + }; [ I'm slow, but I've just noticed this change into struct page. ] Is there a change that the dma_addr would have bit 0 set? If yes it may lead to false-positive PageTail() and really strange behaviour. I think it's better to put some padding into the struct to avoid aliasing to compound_head. See commit 1d798ca3f164 ("mm: make compound_head() robust") for context. > struct { /* slab, slob and slub */ > union { > struct list_head slab_list; /* uses lru */ >
On Wed, Apr 29, 2020 at 03:38:43AM +0300, Kirill A. Shutemov wrote: > On Wed, Feb 13, 2019 at 02:55:40AM +0100, Jesper Dangaard Brouer wrote: > > The page_pool API is using page->private to store DMA addresses. > > As pointed out by David Miller we can't use that on 32-bit architectures > > with 64-bit DMA > > + struct { /* page_pool used by netstack */ > > + /** > > + * @dma_addr: might require a 64-bit value even on > > + * 32-bit architectures. > > + */ > > + dma_addr_t dma_addr; > > + }; > > [ I'm slow, but I've just noticed this change into struct page. ] > > Is there a change that the dma_addr would have bit 0 set? If yes it may > lead to false-positive PageTail() and really strange behaviour. No. It's the DMA address of the page, so it's going to be page aligned and have the bottom 12 (or so) bits cleared. It's not feasible for some wacky IOMMU to use the bottom N bits for its own purposes because you can, say, add three to the DMA address of the page and expect the device to DMA to the third byte within the page. Wacky IOMMUs use the top bits for storing "interesting" information.
diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h index 2c471a2c43fa..0a36a22228e7 100644 --- a/include/linux/mm_types.h +++ b/include/linux/mm_types.h @@ -95,6 +95,13 @@ struct page { */ unsigned long private; }; + struct { /* page_pool used by netstack */ + /** + * @dma_addr: might require a 64-bit value even on + * 32-bit architectures. + */ + dma_addr_t dma_addr; + }; struct { /* slab, slob and slub */ union { struct list_head slab_list; /* uses lru */