LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Mike Kravetz <mike.kravetz@oracle.com>
To: Oscar Salvador <osalvador@suse.de>
Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	Muchun Song <songmuchun@bytedance.com>,
	Michal Hocko <mhocko@suse.com>,
	David Hildenbrand <david@redhat.com>,
	Matthew Wilcox <willy@infradead.org>,
	Naoya Horiguchi <naoya.horiguchi@linux.dev>,
	Mina Almasry <almasrymina@google.com>,
	Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [PATCH v2 1/3] hugetlb: simplify prep_compound_gigantic_page ref count racing code
Date: Tue, 10 Aug 2021 09:51:03 -0700	[thread overview]
Message-ID: <5d5a543a-026a-7b9b-30fd-47a31ac592d7@oracle.com> (raw)
In-Reply-To: <19f47b787269b95bb76d81bb1e6bfcc3@suse.de>

On 8/10/21 2:29 AM, Oscar Salvador wrote:
> On 2021-08-09 20:48, Mike Kravetz wrote:
>> Code in prep_compound_gigantic_page waits for a rcu grace period if it
>> notices a temporarily inflated ref count on a tail page.  This was due
>> to the identified potential race with speculative page cache references
>> which could only last for a rcu grace period.  This is overly complicated
>> as this situation is VERY unlikely to ever happen.  Instead, just quickly
>> return an error.
>> Also, only print a warning in prep_compound_gigantic_page instead of
>> multiple callers.
> 
> The above makes sense to me.

Thanks for taking a look!

> My only question would be: gather_bootmem_prealloc() is an __init function.
> Can we have speculative refcounts due to e.g: pagecache at that early stage?
> I think we cannot, but I am not really sure.

I had the same thought when adding that code.  We cannot have a
speculative refcount this early after boot.  However, further
thinking below...

> 
> We might be able to remove that else() in case we cannot have such scenarios.
> 

Instead of removing the else, I think we should put a BUG_ON() just to
make sure the condition never happens in the future.  Otherwise, we would
just abandon the gigantic page and leave memory (1GB or so) unavailable.

Even if it were possible to have speculative references this early in
the boot process, the likelihood of it happening here is still VERY
small.  So, I would not expect a BUG_ON() to ever be hit in development or
testing.  Rather, with our luck it would show up in some production
environment.

Since handling the race in this routine is so simple, I chose to just
add the code to handle it.
-- 
Mike Kravetz

  reply	other threads:[~2021-08-10 16:51 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-09 18:48 [PATCH v2 0/3] hugetlb: fix potential ref counting races Mike Kravetz
2021-08-09 18:48 ` [PATCH v2 1/3] hugetlb: simplify prep_compound_gigantic_page ref count racing code Mike Kravetz
2021-08-10  9:29   ` Oscar Salvador
2021-08-10 16:51     ` Mike Kravetz [this message]
2021-08-09 18:48 ` [PATCH v2 2/3] hugetlb: drop ref count earlier after page allocation Mike Kravetz
2021-08-09 18:48 ` [PATCH v2 3/3] hugetlb: before freeing hugetlb page set dtor to appropriate value Mike Kravetz

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=5d5a543a-026a-7b9b-30fd-47a31ac592d7@oracle.com \
    --to=mike.kravetz@oracle.com \
    --cc=akpm@linux-foundation.org \
    --cc=almasrymina@google.com \
    --cc=david@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@suse.com \
    --cc=naoya.horiguchi@linux.dev \
    --cc=osalvador@suse.de \
    --cc=songmuchun@bytedance.com \
    --cc=willy@infradead.org \
    --subject='Re: [PATCH v2 1/3] hugetlb: simplify prep_compound_gigantic_page ref count racing code' \
    /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).