Re: [PATCH 4/5] net: page_pool: refactor dma_map into own function page_pool_dma_map
From: Ilias Apalodimas <ilias.apalodimas@linaro.org>
Date: 2021-03-02 21:40:07
Also in:
linux-nfs, lkml
Hi Mel, Can you please CC me in future revisions. I almost missed that! On Mon, Mar 01, 2021 at 04:11:59PM +0000, Mel Gorman wrote:
From: Jesper Dangaard Brouer <redacted> In preparation for next patch, move the dma mapping into its own function, as this will make it easier to follow the changes. V2: make page_pool_dma_map return boolean (Ilias)
[...]
quoted hunk ↗ jump to hunk
static struct page *__page_pool_alloc_pages_slow(struct page_pool *pool, gfp_t _gfp) { + unsigned int pp_flags = pool->p.flags; struct page *page; gfp_t gfp = _gfp; - dma_addr_t dma; /* We could always set __GFP_COMP, and avoid this branch, as * prep_new_page() can handle order-0 with __GFP_COMP.@@ -211,30 +234,14 @@ static struct page *__page_pool_alloc_pages_slow(struct page_pool *pool, if (!page) return NULL; - if (!(pool->p.flags & PP_FLAG_DMA_MAP)) - goto skip_dma_map; - - /* Setup DMA mapping: use 'struct page' area for storing DMA-addr - * since dma_addr_t can be either 32 or 64 bits and does not always fit - * into page private data (i.e 32bit cpu with 64bit DMA caps) - * This mapping is kept for lifetime of page, until leaving pool. - */ - dma = dma_map_page_attrs(pool->p.dev, page, 0, - (PAGE_SIZE << pool->p.order), - pool->p.dma_dir, DMA_ATTR_SKIP_CPU_SYNC); - if (dma_mapping_error(pool->p.dev, dma)) { + if (pp_flags & PP_FLAG_DMA_MAP &&
Nit pick but can we have if ((pp_flags & PP_FLAG_DMA_MAP) && ...
+ unlikely(!page_pool_dma_map(pool, page))) {
put_page(page);
return NULL;
}
- page->dma_addr = dma;
- if (pool->p.flags & PP_FLAG_DMA_SYNC_DEV)
- page_pool_dma_sync_for_device(pool, page, pool->p.max_len);
-
-skip_dma_map:
/* Track how many pages are held 'in-flight' */
pool->pages_state_hold_cnt++;
-
trace_page_pool_state_hold(pool, page, pool->pages_state_hold_cnt);
/* When page just alloc'ed is should/must have refcnt 1. */
--
2.26.2Otherwise Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>