linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Muchun Song <songmuchun@bytedance.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: corbet@lwn.net, mike.kravetz@oracle.com, mcgrof@kernel.org,
	keescook@chromium.org, yzaikin@google.com, osalvador@suse.de,
	david@redhat.com, masahiroy@kernel.org,
	linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-mm@kvack.org, duanxiongchun@bytedance.com,
	smuchun@gmail.com
Subject: Re: [PATCH v8 1/4] mm: hugetlb_vmemmap: introduce CONFIG_HUGETLB_PAGE_HAS_OPTIMIZE_VMEMMAP
Date: Tue, 19 Apr 2022 14:23:56 +0800	[thread overview]
Message-ID: <Yl5VfI1h6nUd8s8q@FVFYT0MHHV2J.usts.net> (raw)
In-Reply-To: <YleQiQW7gFTO7SMk@FVFYT0MHHV2J.usts.net>

On Thu, Apr 14, 2022 at 11:10:01AM +0800, Muchun Song wrote:
> On Wed, Apr 13, 2022 at 12:08:04PM -0700, Andrew Morton wrote:
> > On Wed, 13 Apr 2022 22:47:45 +0800 Muchun Song <songmuchun@bytedance.com> wrote:
> > 
> > > If the size of "struct page" is not the power of two but with the feature
> > > of minimizing overhead of struct page associated with each HugeTLB is
> > > enabled, then the vmemmap pages of HugeTLB will be corrupted after
> > > remapping (panic is about to happen in theory).  But this only exists when
> > > !CONFIG_MEMCG && !CONFIG_SLUB on x86_64.  However, it is not a conventional
> > > configuration nowadays.  So it is not a real word issue, just the result
> > > of a code review.
> > 
> > The patch does add a whole bunch of tricky junk to address something
> > which won't happen.  How about we simply disable
> > CONFIG_HUGETLB_PAGE_OPTIMIZE_VMEMMAP if (!CONFIG_MEMCG &&
> > !CONFIG_SLUB)?
> >
>  
> I'm afraid not. The size of 'struct page' also depends on
> LAST_CPUPID_NOT_IN_PAGE_FLAGS which could be defined
> when CONFIG_NODES_SHIFT or CONFIG_KASAN_SW_TAGS
> or CONFIG_NR_CPUS is configured with a large value.  Then
> the size would be more than 64 bytes.
> 
> Seems like the approach [1] is more simple and feasible,

Sorry, forgot to post the Link.

[1] https://lore.kernel.org/all/20220323125523.79254-2-songmuchun@bytedance.com/

> which also could prevent the users from doing unexpected
> configurations, however, it is objected by Masahiro.
> Shall we look back at the approach again?
>

Hi all,

Friendly ping.

I have implemented 3 approaches to address this issue.

  1) V8 has added a lot of tricky code.
  2) V5 has added a feadback from Kbuild to Kconfig, as Masahiro
     said, it is terrible.
  3) V1 [2] has added a check of is_power_of_2() into hugetlb_vmemmap.c.

Iterated and explored through 8 versions, v1 seems to be the easiest way
to address this.  I think reusing v1 may be the best choice now.
What do you think?

[2] https://lore.kernel.org/all/20220228071022.26143-2-songmuchun@bytedance.com/

Thanks.



  reply	other threads:[~2022-04-19  6:24 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-13 14:47 [PATCH v8 0/4] add hugetlb_optimize_vmemmap sysctl Muchun Song
2022-04-13 14:47 ` [PATCH v8 1/4] mm: hugetlb_vmemmap: introduce CONFIG_HUGETLB_PAGE_HAS_OPTIMIZE_VMEMMAP Muchun Song
2022-04-13 19:08   ` Andrew Morton
2022-04-14  3:10     ` Muchun Song
2022-04-19  6:23       ` Muchun Song [this message]
2022-04-20 17:11   ` Masahiro Yamada
2022-04-20 23:30     ` Mike Kravetz
2022-04-21  3:18       ` Muchun Song
2022-04-13 14:47 ` [PATCH v8 2/4] mm: memory_hotplug: override memmap_on_memory when hugetlb_free_vmemmap=on Muchun Song
2022-04-13 14:47 ` [PATCH v8 3/4] mm: hugetlb_vmemmap: use kstrtobool for hugetlb_vmemmap param parsing Muchun Song
2022-04-13 14:47 ` [PATCH v8 4/4] mm: hugetlb_vmemmap: add hugetlb_optimize_vmemmap sysctl Muchun Song
2022-04-13 19:10   ` Andrew Morton
2022-04-14  4:04     ` Muchun Song

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=Yl5VfI1h6nUd8s8q@FVFYT0MHHV2J.usts.net \
    --to=songmuchun@bytedance.com \
    --cc=akpm@linux-foundation.org \
    --cc=corbet@lwn.net \
    --cc=david@redhat.com \
    --cc=duanxiongchun@bytedance.com \
    --cc=keescook@chromium.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=masahiroy@kernel.org \
    --cc=mcgrof@kernel.org \
    --cc=mike.kravetz@oracle.com \
    --cc=osalvador@suse.de \
    --cc=smuchun@gmail.com \
    --cc=yzaikin@google.com \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox