On Mon, 31 Jul 2023 13:20:04 -0700 Michael Chan wrote: > I think I am beginning to understand what the confusion is. These 32K > page fragments within the page may not belong to the same (GRO) > packet. Right. > So we cannot dma_sync the whole page at the same time. I wouldn't phrase it like that. > Without setting PP_FLAG_DMA_SYNC_DEV, the driver code should be > something like this: > > mapping = page_pool_get_dma_addr(page) + offset; > dma_sync_single_for_device(dev, mapping, BNXT_RX_PAGE_SIZE, bp->rx_dir); > > offset may be 0, 32K, etc. > > Since the PP_FLAG_DMA_SYNC_DEV logic is not aware of this offset, we > actually must do our own dma_sync and not use PP_FLAG_DMA_SYNC_DEV in > this case. Does that sound right? No, no, all I'm saying is that with the current code (in page pool) you can't be very intelligent about the sync'ing. Every time a page enters the pool - the whole page should be synced. But that's fine, it's still better to let page pool do the syncing than trying to do it manually in the driver (since freshly allocated pages do not have to be synced). I think the confusion comes partially from the fact that the driver only ever deals with fragments (32k), but internally page pool does recycling in full pages (64k). And .max_len is part of the recycling machinery, so to speak, not part of the allocation machinery. tl;dr just set .max_len = PAGE_SIZE and all will be right.
