On 2024/1/5 23:35, Alexander H Duyck wrote: > On Wed, 2024-01-03 at 17:56 +0800, Yunsheng Lin wrote: >> Currently there seems to be three page frag implementions >> which all try to allocate order 3 page, if that fails, it >> then fail back to allocate order 0 page, and each of them >> all allow order 3 page allocation to fail under certain >> condition by using specific gfp bits. >> >> The gfp bits for order 3 page allocation are different >> between different implementation, __GFP_NOMEMALLOC is >> or'd to forbid access to emergency reserves memory for >> __page_frag_cache_refill(), but it is not or'd in other >> implementions, __GFP_DIRECT_RECLAIM is masked off to avoid >> direct reclaim in skb_page_frag_refill(), but it is not >> masked off in __page_frag_cache_refill(). >> >> This patch unifies the gfp bits used between different >> implementions by or'ing __GFP_NOMEMALLOC and masking off >> __GFP_DIRECT_RECLAIM for order 3 page allocation to avoid >> possible pressure for mm. >> >> Signed-off-by: Yunsheng Lin <linyunsheng@xxxxxxxxxx> >> CC: Alexander Duyck <alexander.duyck@xxxxxxxxx> >> --- >> drivers/vhost/net.c | 2 +- >> mm/page_alloc.c | 4 ++-- >> net/core/sock.c | 2 +- >> 3 files changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c >> index f2ed7167c848..e574e21cc0ca 100644 >> --- a/drivers/vhost/net.c >> +++ b/drivers/vhost/net.c >> @@ -670,7 +670,7 @@ static bool vhost_net_page_frag_refill(struct vhost_net *net, unsigned int sz, >> /* Avoid direct reclaim but allow kswapd to wake */ >> pfrag->page = alloc_pages((gfp & ~__GFP_DIRECT_RECLAIM) | >> __GFP_COMP | __GFP_NOWARN | >> - __GFP_NORETRY, >> + __GFP_NORETRY | __GFP_NOMEMALLOC, >> SKB_FRAG_PAGE_ORDER); >> if (likely(pfrag->page)) { >> pfrag->size = PAGE_SIZE << SKB_FRAG_PAGE_ORDER; >> diff --git a/mm/page_alloc.c b/mm/page_alloc.c >> index 9a16305cf985..1f0b36dd81b5 100644 >> --- a/mm/page_alloc.c >> +++ b/mm/page_alloc.c >> @@ -4693,8 +4693,8 @@ static struct page *__page_frag_cache_refill(struct page_frag_cache *nc, >> gfp_t gfp = gfp_mask; >> >> #if (PAGE_SIZE < PAGE_FRAG_CACHE_MAX_SIZE) >> - gfp_mask |= __GFP_COMP | __GFP_NOWARN | __GFP_NORETRY | >> - __GFP_NOMEMALLOC; >> + gfp_mask = (gfp_mask & ~__GFP_DIRECT_RECLAIM) | __GFP_COMP | >> + __GFP_NOWARN | __GFP_NORETRY | __GFP_NOMEMALLOC; >> page = alloc_pages_node(NUMA_NO_NODE, gfp_mask, >> PAGE_FRAG_CACHE_MAX_ORDER); >> nc->size = page ? PAGE_FRAG_CACHE_MAX_SIZE : PAGE_SIZE; >> diff --git a/net/core/sock.c b/net/core/sock.c >> index 446e945f736b..d643332c3ee5 100644 >> --- a/net/core/sock.c >> +++ b/net/core/sock.c >> @@ -2900,7 +2900,7 @@ bool skb_page_frag_refill(unsigned int sz, struct page_frag *pfrag, gfp_t gfp) >> /* Avoid direct reclaim but allow kswapd to wake */ >> pfrag->page = alloc_pages((gfp & ~__GFP_DIRECT_RECLAIM) | >> __GFP_COMP | __GFP_NOWARN | >> - __GFP_NORETRY, >> + __GFP_NORETRY | __GFP_NOMEMALLOC, >> SKB_FRAG_PAGE_ORDER); >> if (likely(pfrag->page)) { >> pfrag->size = PAGE_SIZE << SKB_FRAG_PAGE_ORDER; > > Looks fine to me. > > One thing you may want to consider would be to place this all in an > inline function that could just consolidate all the code. Do you think it is possible to further unify the implementations of the 'struct page_frag_cache' and 'struct page_frag', so adding a inline function for above is unnecessary? > > Reviewed-by: Alexander Duyck <alexanderduyck@xxxxxx> > > . >