Netdev Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Yunsheng Lin <linyunsheng@huawei.com>
To: Matthew Wilcox <willy@infradead.org>
Cc: <davem@davemloft.net>, <kuba@kernel.org>, <hawk@kernel.org>,
	<ilias.apalodimas@linaro.org>, <mcroce@microsoft.com>,
	<alexander.duyck@gmail.com>, <netdev@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>, <linuxarm@openeuler.org>,
	<chenhao288@hisilicon.com>
Subject: Re: [PATCH net] page_pool: mask the page->signature before the checking
Date: Thu, 5 Aug 2021 10:14:39 +0800	[thread overview]
Message-ID: <19955a79-3a6a-9534-7665-7f868eb7db1f@huawei.com> (raw)
In-Reply-To: <YQtDynWsDxZ/T41e@casper.infradead.org>

On 2021/8/5 9:50, Matthew Wilcox wrote:
> On Thu, Aug 05, 2021 at 09:06:57AM +0800, Yunsheng Lin wrote:
>> As mentioned in commit c07aea3ef4d4 ("mm: add a signature
>> in struct page"):
>> "The page->signature field is aliased to page->lru.next and
>> page->compound_head."
>>
>> And as the comment in page_is_pfmemalloc():
>> "lru.next has bit 1 set if the page is allocated from the
>> pfmemalloc reserves. Callers may simply overwrite it if they
>> do not need to preserve that information."
>>
>> The page->signature is or’ed with PP_SIGNATURE when a page is
>> allocated in page pool, see __page_pool_alloc_pages_slow(),
>> and page->signature is checked directly with PP_SIGNATURE in
>> page_pool_return_skb_page(), which might cause resoure leaking
>> problem for a page from page pool if bit 1 of lru.next is set for
>> a pfmemalloc page.
>>
>> As bit 0 is page->compound_head, So mask both bit 0 and 1 before
>> the checking in page_pool_return_skb_page().
> 
> No, you don't understand.  We *want* the check to fail if we were low
> on memory so we return the emergency allocation.

If the check failed, but the page pool assume the page is not from page
pool and will not do the resource cleaning(like dma unmapping), as the
page pool still use the page with pfmemalloc set and dma map the page
if pp_flags & PP_FLAG_DMA_MAP is true in __page_pool_alloc_pages_slow().

The returning the emergency allocation you mentioned seems to be handled
in __page_pool_put_page(), see:

https://elixir.bootlin.com/linux/latest/source/net/core/page_pool.c#L411

We just use the page with pfmemalloc one time and do the resource cleaning
before returning the page back to page allocator. Or did I miss something
here?

> .
> 

  reply	other threads:[~2021-08-05  2:14 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-05  1:06 Yunsheng Lin
2021-08-05  1:50 ` Matthew Wilcox
2021-08-05  2:14   ` Yunsheng Lin [this message]
2021-08-05  8:54     ` Ilias Apalodimas
2021-08-05  9:31       ` Yunsheng Lin
2021-08-05 14:47         ` Ilias Apalodimas

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=19955a79-3a6a-9534-7665-7f868eb7db1f@huawei.com \
    --to=linyunsheng@huawei.com \
    --cc=alexander.duyck@gmail.com \
    --cc=chenhao288@hisilicon.com \
    --cc=davem@davemloft.net \
    --cc=hawk@kernel.org \
    --cc=ilias.apalodimas@linaro.org \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxarm@openeuler.org \
    --cc=mcroce@microsoft.com \
    --cc=netdev@vger.kernel.org \
    --cc=willy@infradead.org \
    --subject='Re: [PATCH net] page_pool: mask the page->signature before the checking' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).