At 2022-05-31 02:29:18, "Jakub Kicinski" <[email protected]> wrote:
>On Mon, 30 May 2022 12:27:05 -0700 Jakub Kicinski wrote:
>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>> index e008a3df0485..360a545ee5e8 100644
>> --- a/mm/page_alloc.c
>> +++ b/mm/page_alloc.c
>> @@ -5537,6 +5537,7 @@ EXPORT_SYMBOL(free_pages);
>> * sk_buff->head, or to be used in the "frags" portion of skb_shared_info.
>> */
>> static struct page *__page_frag_cache_refill(struct page_frag_cache *nc,
>> + unsigned int fragsz,
>> gfp_t gfp_mask)
>> {
>> struct page *page = NULL;
>> @@ -5549,7 +5550,7 @@ static struct page *__page_frag_cache_refill(struct page_frag_cache *nc,
>> PAGE_FRAG_CACHE_MAX_ORDER);
>> nc->size = page ? PAGE_FRAG_CACHE_MAX_SIZE : PAGE_SIZE;
>> #endif
>> - if (unlikely(!page))
>> + if (unlikely(!page && fragsz <= PAGE_SIZE))
>> page = alloc_pages_node(NUMA_NO_NODE, gfp, 0);
>>
>> nc->va = page ? page_address(page) : NULL;
>> @@ -5576,7 +5577,7 @@ void *page_frag_alloc_align(struct page_frag_cache *nc,
>>
>> if (unlikely(!nc->va)) {
>> refill:
>> - page = __page_frag_cache_refill(nc, gfp_mask);
>> + page = __page_frag_cache_refill(nc, fragsz, gfp_mask);
>> if (!page)
>> return NULL;
>
>Oh, well, the reuse also needs an update. We can slap a similar
>condition next to the pfmemalloc check.
The sample code above cannot completely solve the current problem.
For example, when fragsz is greater than PAGE_FRAG_CACHE_MAX_SIZE(32768),
__page_frag_cache_refill will return a memory of only 32768 bytes, so
should we continue to expand the PAGE_FRAG_CACHE_MAX_SIZE? Maybe more
work needs to be done
On Tue, 31 May 2022 22:41:12 +0800 Chen Lin wrote:
> At 2022-05-31 02:29:18, "Jakub Kicinski" <[email protected]> wrote:
> >Oh, well, the reuse also needs an update. We can slap a similar
> >condition next to the pfmemalloc check.
>
> The sample code above cannot completely solve the current problem.
> For example, when fragsz is greater than PAGE_FRAG_CACHE_MAX_SIZE(32768),
> __page_frag_cache_refill will return a memory of only 32768 bytes, so
> should we continue to expand the PAGE_FRAG_CACHE_MAX_SIZE? Maybe more
> work needs to be done
Right, but I can think of two drivers off the top of my head which will
allocate <=32k frags but none which will allocate more.
At 2022-05-31 22:14:12, "Jakub Kicinski" <[email protected]> wrote:
>On Tue, 31 May 2022 22:41:12 +0800 Chen Lin wrote:
>> At 2022-05-31 02:29:18, "Jakub Kicinski" <[email protected]> wrote:
>> >Oh, well, the reuse also needs an update. We can slap a similar
>> >condition next to the pfmemalloc check.
>>
>> The sample code above cannot completely solve the current problem.
>> For example, when fragsz is greater than PAGE_FRAG_CACHE_MAX_SIZE(32768),
>> __page_frag_cache_refill will return a memory of only 32768 bytes, so
>> should we continue to expand the PAGE_FRAG_CACHE_MAX_SIZE? Maybe more
>> work needs to be done
>
>Right, but I can think of two drivers off the top of my head which will
>allocate <=32k frags but none which will allocate more.
In fact, it is rare to apply for more than one page, so is it necessary to
change it to support?
we can just warning and return, also it is easy to synchronize this simple
protective measures to lower Linux versions.
On Tue, 31 May 2022 23:36:22 +0800 Chen Lin wrote:
> At 2022-05-31 22:14:12, "Jakub Kicinski" <[email protected]> wrote:
> >On Tue, 31 May 2022 22:41:12 +0800 Chen Lin wrote:
> >> The sample code above cannot completely solve the current problem.
> >> For example, when fragsz is greater than PAGE_FRAG_CACHE_MAX_SIZE(32768),
> >> __page_frag_cache_refill will return a memory of only 32768 bytes, so
> >> should we continue to expand the PAGE_FRAG_CACHE_MAX_SIZE? Maybe more
> >> work needs to be done
> >
> >Right, but I can think of two drivers off the top of my head which will
> >allocate <=32k frags but none which will allocate more.
>
> In fact, it is rare to apply for more than one page, so is it necessary to
> change it to support?
I don't really care if it's supported TBH, but I dislike adding
a branch to the fast path just to catch one or two esoteric bad
callers.
Maybe you can wrap the check with some debug CONFIG_ so it won't
run on production builds?
> we can just warning and return, also it is easy to synchronize this simple
> protective measures to lower Linux versions.