lkml.org 
[lkml]   [2022]   [Dec]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCHv2 11/11] dmapool: link blocks across pages
On Fri, Dec 16, 2022 at 12:16:25PM -0800, Keith Busch wrote:
> unsigned int size;
> unsigned int allocation;
> unsigned int boundary;
> + size_t nr_blocks;
> + size_t nr_active;
> + size_t nr_pages;

Should these be unsigned int like the counters above?

> +static inline struct dma_block *pool_block_pop(struct dma_pool *pool)
> +{
> + struct dma_block *block = pool->next_block;
> +
> + if (block) {
> + pool->next_block = block->next_block;
> + pool->nr_active++;
> + }
> + return block;
> +}
> +
> +static inline void pool_block_push(struct dma_pool *pool, struct dma_block *block,
> + dma_addr_t dma)
> +{
> + block->dma = dma;
> + block->next_block = pool->next_block;
> + pool->next_block = block;
> +}

Any point in marking these inline vs just letting the ocmpile do
it's job?

> @@ -162,6 +176,10 @@ struct dma_pool *dma_pool_create(const char *name, struct device *dev,
> retval->size = size;
> retval->boundary = boundary;
> retval->allocation = allocation;
> + retval->nr_blocks = 0;
> + retval->nr_active = 0;
> + retval->nr_pages = 0;
> + retval->next_block = NULL;

Maybe just switch to kzmalloc so that you don't have to bother
initializing invdividual fields. It's not like dma_pool_create is
called from anything near a fast path.

> static void pool_initialise_page(struct dma_pool *pool, struct dma_page *page)
> {
> + unsigned int next_boundary = pool->boundary, offset = 0;
> + struct dma_block *block;
> +
> + while (offset + pool->size <= pool->allocation) {
> + if (offset + pool->size > next_boundary) {
> + offset = next_boundary;
> next_boundary += pool->boundary;
> + continue;
> }
> +
> + block = page->vaddr + offset;
> + pool_block_push(pool, block, page->dma + offset);

So I guess with this pool_initialise_page needs to be called under
the lock anyway, but just doing it silently in the previous patch
seems a bit odd.

> +static inline void pool_check_block(struct dma_pool *pool, struct dma_block *block,
> + gfp_t mem_flags)

I didn't spot this earlier, but inline on a relatively expensive debug
helper is a bit silly.

Otherwise this looks like a nice improvement by using a better and
simpler data structure.

\
 
 \ /
  Last update: 2023-03-26 23:19    [W:0.183 / U:0.960 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site