diff mbox series

[net-next,v4,3/5] page_pool: introduce page_pool_alloc() API

Message ID 20230612130256.4572-4-linyunsheng@huawei.com (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series introduce page_pool_alloc() API | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 5148 this patch: 5148
netdev/cc_maintainers success CCed 7 of 7 maintainers
netdev/build_clang success Errors and warnings before: 1977 this patch: 1977
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 5378 this patch: 5378
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 49 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Yunsheng Lin June 12, 2023, 1:02 p.m. UTC
Currently page pool supports the below use cases:
use case 1: allocate page without page splitting using
            page_pool_alloc_pages() API if the driver knows
            that the memory it need is always bigger than
            half of the page allocated from page pool.
use case 2: allocate page frag with page splitting using
            page_pool_alloc_frag() API if the driver knows
            that the memory it need is always smaller than
            or equal to the half of the page allocated from
            page pool.

There is emerging use case [1] & [2] that is a mix of the
above two case: the driver doesn't know the size of memory it
need beforehand, so the driver may use something like below to
allocate memory with least memory utilization and performance
penalty:

if (size << 1 > max_size)
	page = page_pool_alloc_pages();
else
	page = page_pool_alloc_frag();

To avoid the driver doing something like above, add the
page_pool_alloc() API to support the above use case, and update
the true size of memory that is acctually allocated by updating
'*size' back to the driver in order to avoid the truesize
underestimate problem.

1. https://lore.kernel.org/all/d3ae6bd3537fbce379382ac6a42f67e22f27ece2.1683896626.git.lorenzo@kernel.org/
2. https://lore.kernel.org/all/20230526054621.18371-3-liangchen.linux@gmail.com/

Signed-off-by: Yunsheng Lin <linyunsheng@huawei.com>
CC: Lorenzo Bianconi <lorenzo@kernel.org>
CC: Alexander Duyck <alexander.duyck@gmail.com>
---
 include/net/page_pool.h | 43 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

Comments

Alexander Lobakin June 13, 2023, 1:08 p.m. UTC | #1
From: Yunsheng Lin <linyunsheng@huawei.com>
Date: Mon, 12 Jun 2023 21:02:54 +0800

> Currently page pool supports the below use cases:
> use case 1: allocate page without page splitting using
>             page_pool_alloc_pages() API if the driver knows
>             that the memory it need is always bigger than
>             half of the page allocated from page pool.
> use case 2: allocate page frag with page splitting using
>             page_pool_alloc_frag() API if the driver knows
>             that the memory it need is always smaller than
>             or equal to the half of the page allocated from
>             page pool.
> 
> There is emerging use case [1] & [2] that is a mix of the
> above two case: the driver doesn't know the size of memory it
> need beforehand, so the driver may use something like below to
> allocate memory with least memory utilization and performance
> penalty:
> 
> if (size << 1 > max_size)
> 	page = page_pool_alloc_pages();
> else
> 	page = page_pool_alloc_frag();
> 
> To avoid the driver doing something like above, add the
> page_pool_alloc() API to support the above use case, and update
> the true size of memory that is acctually allocated by updating
> '*size' back to the driver in order to avoid the truesize
> underestimate problem.
> 
> 1. https://lore.kernel.org/all/d3ae6bd3537fbce379382ac6a42f67e22f27ece2.1683896626.git.lorenzo@kernel.org/
> 2. https://lore.kernel.org/all/20230526054621.18371-3-liangchen.linux@gmail.com/
> 
> Signed-off-by: Yunsheng Lin <linyunsheng@huawei.com>
> CC: Lorenzo Bianconi <lorenzo@kernel.org>
> CC: Alexander Duyck <alexander.duyck@gmail.com>
> ---
>  include/net/page_pool.h | 43 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 43 insertions(+)
> 
> diff --git a/include/net/page_pool.h b/include/net/page_pool.h
> index 0b8cd2acc1d7..c135cd157cea 100644
> --- a/include/net/page_pool.h
> +++ b/include/net/page_pool.h
> @@ -260,6 +260,49 @@ static inline struct page *page_pool_dev_alloc_frag(struct page_pool *pool,
>  	return page_pool_alloc_frag(pool, offset, size, gfp);
>  }
>  
> +static inline struct page *page_pool_alloc(struct page_pool *pool,
> +					   unsigned int *offset,
> +					   unsigned int *size, gfp_t gfp)

Oh, really nice. Wouldn't you mind if I base my series on top of this? :)

Also, with %PAGE_SIZE of 32k+ and default MTU, there is truesize
underestimation. I haven't looked at the latest conversations as I had a
small vacation, sowwy :s What's the current opinion on this?

[...]

Thanks,
Olek
Alexander Lobakin June 13, 2023, 1:11 p.m. UTC | #2
From: Alexander Lobakin <aleksander.lobakin@intel.com>
Date: Tue, 13 Jun 2023 15:08:41 +0200

> From: Yunsheng Lin <linyunsheng@huawei.com>
> Date: Mon, 12 Jun 2023 21:02:54 +0800
> 
>> Currently page pool supports the below use cases:
>> use case 1: allocate page without page splitting using
>>             page_pool_alloc_pages() API if the driver knows
>>             that the memory it need is always bigger than
>>             half of the page allocated from page pool.
>> use case 2: allocate page frag with page splitting using
>>             page_pool_alloc_frag() API if the driver knows
>>             that the memory it need is always smaller than
>>             or equal to the half of the page allocated from
>>             page pool.
>>
>> There is emerging use case [1] & [2] that is a mix of the
>> above two case: the driver doesn't know the size of memory it
>> need beforehand, so the driver may use something like below to
>> allocate memory with least memory utilization and performance
>> penalty:
>>
>> if (size << 1 > max_size)
>> 	page = page_pool_alloc_pages();
>> else
>> 	page = page_pool_alloc_frag();
>>
>> To avoid the driver doing something like above, add the
>> page_pool_alloc() API to support the above use case, and update
>> the true size of memory that is acctually allocated by updating
>> '*size' back to the driver in order to avoid the truesize
>> underestimate problem.
>>
>> 1. https://lore.kernel.org/all/d3ae6bd3537fbce379382ac6a42f67e22f27ece2.1683896626.git.lorenzo@kernel.org/
>> 2. https://lore.kernel.org/all/20230526054621.18371-3-liangchen.linux@gmail.com/
>>
>> Signed-off-by: Yunsheng Lin <linyunsheng@huawei.com>
>> CC: Lorenzo Bianconi <lorenzo@kernel.org>
>> CC: Alexander Duyck <alexander.duyck@gmail.com>
>> ---
>>  include/net/page_pool.h | 43 +++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 43 insertions(+)
>>
>> diff --git a/include/net/page_pool.h b/include/net/page_pool.h
>> index 0b8cd2acc1d7..c135cd157cea 100644
>> --- a/include/net/page_pool.h
>> +++ b/include/net/page_pool.h
>> @@ -260,6 +260,49 @@ static inline struct page *page_pool_dev_alloc_frag(struct page_pool *pool,
>>  	return page_pool_alloc_frag(pool, offset, size, gfp);
>>  }
>>  
>> +static inline struct page *page_pool_alloc(struct page_pool *pool,
>> +					   unsigned int *offset,
>> +					   unsigned int *size, gfp_t gfp)
> 
> Oh, really nice. Wouldn't you mind if I base my series on top of this? :)
> 
> Also, with %PAGE_SIZE of 32k+ and default MTU, there is truesize
> underestimation. I haven't looked at the latest conversations as I had a
> small vacation, sowwy :s What's the current opinion on this?

Please ignore this, seems like I didn't manage to read 2 lines below,
you explicitly mention in the comment that you already handle this >_<

Thanks,
Olek
Yunsheng Lin June 14, 2023, 3:17 a.m. UTC | #3
On 2023/6/13 21:11, Alexander Lobakin wrote:
> From: Alexander Lobakin <aleksander.lobakin@intel.com>
> Date: Tue, 13 Jun 2023 15:08:41 +0200
> 
>> From: Yunsheng Lin <linyunsheng@huawei.com>
>> Date: Mon, 12 Jun 2023 21:02:54 +0800
>>
>>> Currently page pool supports the below use cases:
>>> use case 1: allocate page without page splitting using
>>>             page_pool_alloc_pages() API if the driver knows
>>>             that the memory it need is always bigger than
>>>             half of the page allocated from page pool.
>>> use case 2: allocate page frag with page splitting using
>>>             page_pool_alloc_frag() API if the driver knows
>>>             that the memory it need is always smaller than
>>>             or equal to the half of the page allocated from
>>>             page pool.
>>>
>>> There is emerging use case [1] & [2] that is a mix of the
>>> above two case: the driver doesn't know the size of memory it
>>> need beforehand, so the driver may use something like below to
>>> allocate memory with least memory utilization and performance
>>> penalty:
>>>
>>> if (size << 1 > max_size)
>>> 	page = page_pool_alloc_pages();
>>> else
>>> 	page = page_pool_alloc_frag();
>>>
>>> To avoid the driver doing something like above, add the
>>> page_pool_alloc() API to support the above use case, and update
>>> the true size of memory that is acctually allocated by updating
>>> '*size' back to the driver in order to avoid the truesize
>>> underestimate problem.
>>>
>>> 1. https://lore.kernel.org/all/d3ae6bd3537fbce379382ac6a42f67e22f27ece2.1683896626.git.lorenzo@kernel.org/
>>> 2. https://lore.kernel.org/all/20230526054621.18371-3-liangchen.linux@gmail.com/
>>>
>>> Signed-off-by: Yunsheng Lin <linyunsheng@huawei.com>
>>> CC: Lorenzo Bianconi <lorenzo@kernel.org>
>>> CC: Alexander Duyck <alexander.duyck@gmail.com>
>>> ---
>>>  include/net/page_pool.h | 43 +++++++++++++++++++++++++++++++++++++++++
>>>  1 file changed, 43 insertions(+)
>>>
>>> diff --git a/include/net/page_pool.h b/include/net/page_pool.h
>>> index 0b8cd2acc1d7..c135cd157cea 100644
>>> --- a/include/net/page_pool.h
>>> +++ b/include/net/page_pool.h
>>> @@ -260,6 +260,49 @@ static inline struct page *page_pool_dev_alloc_frag(struct page_pool *pool,
>>>  	return page_pool_alloc_frag(pool, offset, size, gfp);
>>>  }
>>>  
>>> +static inline struct page *page_pool_alloc(struct page_pool *pool,
>>> +					   unsigned int *offset,
>>> +					   unsigned int *size, gfp_t gfp)
>>
>> Oh, really nice. Wouldn't you mind if I base my series on top of this? :)

No, I wouldn't mind. I am glad that if using the new API can both save memory
and improve performance for your case:)

>>
>> Also, with %PAGE_SIZE of 32k+ and default MTU, there is truesize
>> underestimation. I haven't looked at the latest conversations as I had a
>> small vacation, sowwy :s What's the current opinion on this?
> 
> Please ignore this, seems like I didn't manage to read 2 lines below,
> you explicitly mention in the comment that you already handle this >_<
> 
> Thanks,
> Olek
> 
> .
>
diff mbox series

Patch

diff --git a/include/net/page_pool.h b/include/net/page_pool.h
index 0b8cd2acc1d7..c135cd157cea 100644
--- a/include/net/page_pool.h
+++ b/include/net/page_pool.h
@@ -260,6 +260,49 @@  static inline struct page *page_pool_dev_alloc_frag(struct page_pool *pool,
 	return page_pool_alloc_frag(pool, offset, size, gfp);
 }
 
+static inline struct page *page_pool_alloc(struct page_pool *pool,
+					   unsigned int *offset,
+					   unsigned int *size, gfp_t gfp)
+{
+	unsigned int max_size = PAGE_SIZE << pool->p.order;
+	struct page *page;
+
+	*size = ALIGN(*size, dma_get_cache_alignment());
+
+	if (WARN_ON(*size > max_size))
+		return NULL;
+
+	if ((*size << 1) > max_size || PAGE_POOL_DMA_USE_PP_FRAG_COUNT) {
+		*size = max_size;
+		*offset = 0;
+		return page_pool_alloc_pages(pool, gfp);
+	}
+
+	page = __page_pool_alloc_frag(pool, offset, *size, gfp);
+	if (unlikely(!page))
+		return NULL;
+
+	/* There is very likely not enough space for another frag, so append the
+	 * remaining size to the current frag to avoid truesize underestimate
+	 * problem.
+	 */
+	if (pool->frag_offset + *size > max_size) {
+		*size = max_size - *offset;
+		pool->frag_offset = max_size;
+	}
+
+	return page;
+}
+
+static inline struct page *page_pool_dev_alloc(struct page_pool *pool,
+					       unsigned int *offset,
+					       unsigned int *size)
+{
+	gfp_t gfp = (GFP_ATOMIC | __GFP_NOWARN);
+
+	return page_pool_alloc(pool, offset, size, gfp);
+}
+
 /* get the stored dma direction. A driver might decide to treat this locally and
  * avoid the extra cache line from page_pool to determine the direction
  */