From: Christoph Hellwig <hch@xxxxxxxxxxxxx> Date: Thu, 20 Apr 2023 09:15:23 -0700 > On Thu, Apr 20, 2023 at 03:59:39PM +0200, Alexander Lobakin wrote: >> Hmm, currently almost all Ethernet drivers map Rx pages once and then >> just recycle them, keeping the original DMA mapping. Which means pages >> can have the same first mapping for very long time, often even for the >> lifetime of the struct device. Same for XDP sockets, the lifetime of DMA >> mappings equals the lifetime of sockets. >> Does it mean we'd better review that approach and try switching to >> dma_alloc_*() family (non-coherent/caching in our case)? > > Yes, exactly. dma_alloc_noncoherent can be used exactly as alloc_pages > + dma_map_* by the driver (including the dma_sync_* calls on reuse), but > has a huge number of advantages. > >> Also, I remember I tried to do that for one my driver, but the thing >> that all those functions zero the whole page(s) before returning them to >> the driver ruins the performance -- we don't need to zero buffers for >> receiving packets and spend a ton of cycles on it (esp. in cases when 4k >> gets zeroed each time, but your main body of traffic is 64-byte frames). > > Hmm, the single zeroing when doing the initial allocation shows up > in these profiles? When there's no recycling of pages, then yes. And since recycling is done asynchronously, sometimes new allocations happen either way. Anyways, that was roughly a couple years ago right when you introduced dma_alloc_noncoherent(). Things might've been changed since then. I could try again while next is closed (i.e. starting this Sunday), the only thing I'd like to mention: Page Pool allocates pages via alloc_pages_bulk_array_node(). Bulking helps a lot (and PP uses bulks of 16 IIRC), explicit node setting helps when Rx queues are distributed between several nodes. We can then have one struct device for several nodes. As I can see, there's now no function to allocate in bulks and no explicit node setting option (e.g. mlx5 works around this using set_dev_node() + allocate + set_dev_node(orig_node)). Could such options be added in near future? That would help a lot switching to the functions intended for use when DMA mappings can stay for a long time. >From what I see from the code, that shouldn't be a problem (except for non-direct DMA cases, where we'd need to introduce new callbacks or extend the existing ones). Thanks, Olek
