Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 29 Jun 2022 14:06:03 -0700
From:   Mike Kravetz <mike.kravetz@oracle.com>
To:     Muchun Song <songmuchun@bytedance.com>
Cc:     James Houghton <jthoughton@google.com>,
        Peter Xu <peterx@redhat.com>,
        David Hildenbrand <david@redhat.com>,
        David Rientjes <rientjes@google.com>,
        Axel Rasmussen <axelrasmussen@google.com>,
        Mina Almasry <almasrymina@google.com>,
        Jue Wang <juew@google.com>,
        Manish Mishra <manish.mishra@nutanix.com>,
        "Dr . David Alan Gilbert" <dgilbert@redhat.com>,
        linux-mm@kvack.org, linux-kernel@vger.kernel.org
Subject: Re: [RFC PATCH 02/26] hugetlb: sort hstates in hugetlb_init_hstates
Message-ID: <Yry+uxxA2/wsK8G9@monkey>
References: <20220624173656.2033256-1-jthoughton@google.com>
 <20220624173656.2033256-3-jthoughton@google.com>
 <Yrn6DfTINknKNS04@monkey>
 <CADrL8HUD14o6XybhYDdozAUkJ4Zt6nE8=dm-_osKg2CmvOFzHg@mail.gmail.com>
 <YrvzlTU2z8/IrEuK@FVFYT0MHHV2J.usts.net>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <YrvzlTU2z8/IrEuK@FVFYT0MHHV2J.usts.net>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?kuo9+AJiv5yM76d84gQBAuoax/wtoI01gzBpvdVInfRoxiTybHxbgdVbqsRx?=
 =?us-ascii?Q?ah4IuzMSnT+47OqDOtAmVSzVUo1M9T0UcEZe4Ffy8qe/eMboSCBF25goOmPb?=
 =?us-ascii?Q?64biDU/TyY3DBYniS8kgd5dYumGbgMH7DPW2AepDLw7ZxvIp6NM/j4vErw7f?=
 =?us-ascii?Q?P6TMtFASJqHsZ/Y1eA8Y5I56xWPUdAtH/jr5+GvFJ+em4aYfUBCcfLQ9fqa9?=
 =?us-ascii?Q?H8clcKA/k/D5AyyQQw4oeDyZP11TdLjsQ7zXEkqkWSy7rV6lq6mYa+pXS5ED?=
 =?us-ascii?Q?wug3yJLsc52nKeR/kj/JpExCbrZDbusrCjbye+qK+leExzh7/qQYNqkPBRov?=
 =?us-ascii?Q?uAmYjWu2xtrQwQ6Tzy4sgx/GIVV2FpbyYa6a0gJIU3bj8hqeTcUKx5SpmMya?=
 =?us-ascii?Q?Op/XzU4kcU5knktu7zD8yLVScokr7qfZInSYe59m5/0fKUL4I3nC0RGKGxYl?=
 =?us-ascii?Q?mPjs6A6NWRsRq49JXqW0GwFj85uf0DqBESqhA+S+BAsJRUfYR0KaSXcHDNn7?=
 =?us-ascii?Q?GRDcJoFY+WPga17Nd7btwi1TE3Wnnpp1Vv1emMQrwgWsUlOiYtgAn9RYzzb+?=
 =?us-ascii?Q?TIPyId6Y18B3RCGHJU3vMKaZLu5h8RBILt7Y/yU9gw9QBTwZTCJGu0OEkKmF?=
 =?us-ascii?Q?lK8RGXzaQOhzRn94P3qJCTS4niJ+sOz59nPErvz2uaSzN4Rjl8gljqMb7IXW?=
 =?us-ascii?Q?RMPpH0cvrlNNYCS+KFMZZgQ97FBFguR2MXMxtuia+YaToIzUJncd9RTFZe0Z?=
 =?us-ascii?Q?hCFWn6isb03NmGJxwYrrGedQJEQQ7tK55i98jlTEzVGZwYRwc2LxOQd8hCUP?=
 =?us-ascii?Q?COsMju8lm3WcCzUGFqKiS+aR6Urvc3verlhay5a7zME6AT/Zk0a0XVWNEMtI?=
 =?us-ascii?Q?krxEWspMefYwrmqBU9sdHGxvuk/9MUsd/2AZuK086WyCyhIgAdLDKvghM6r6?=
 =?us-ascii?Q?hPtBj9Vgxx9kCrPGn+fr3dWynRqGCW0GTOPXsvP8rN0fy0Sq8mKz+oQmYt+F?=
 =?us-ascii?Q?cpjDe9Ba5p8AZdIPpxXPs+kYoZKIMH0S2oFwFxjnbIIWBmujmlyVTddXZB5R?=
 =?us-ascii?Q?2/ziZROj4FrS58NDN0HCGV2cgaWubfJLNd+FAo1LJbj4utTeLM+ubTGU1JHX?=
 =?us-ascii?Q?tpeb+hiDBjrLgzjPJ2kQYLaz3tZQRYwnDnGoL1O17o/mbtKC4gfdho4+Sjqz?=
 =?us-ascii?Q?cjUjs4fdpj0G7B8GHl9ei7CGYPca0uSuNs2IX7BN/b97HYFwcKoUsGja5ozi?=
 =?us-ascii?Q?u8hJXEffzhRLHOUfx6+MGxuWGIh54uVzGhlze6kczeCV/udYftCc+KUuUpp/?=
 =?us-ascii?Q?9hvkU5dJdLxdz0WV+kVybD0IrCE/TQMnCdMdKzdkHhCkKgOlshTP6cIEyN1o?=
 =?us-ascii?Q?CMvme5jjLcW/aIV2BXPaFbE78SrkOLcI8tWhjII7aTtnaqNA7eIvK59vN6FU?=
 =?us-ascii?Q?X2hpIxs0YHrDmJlChp95AN+kuboPyX6ly1ozJa62Cky1YBrNKjQa8UyyCTVM?=
 =?us-ascii?Q?Pyr3cixoSJAkn1PpM8HV0vo0o5thmoCRUSmSSjwjeEgxQDAz6m8MxKpg3f2d?=
 =?us-ascii?Q?kUo4D1XJOXxA4fkRGhfwmroL/WmOyHHDmbn3wWbSTBQeqCOASbZkQaQoXc85?=
 =?us-ascii?Q?Tg=3D=3D?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: f1d54f32-6261-4dcd-741f-08da5a132eb8
X-MS-Exchange-CrossTenant-AuthSource: BY5PR10MB4196.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 29 Jun 2022 21:06:06.2350
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: BtXe/Howt+RiKFc00touBeujhr6gK20OxDIoGjv8VxG7HQKPnRp1xgDXgoY4qnyQNGOkr36+0/S3RaAR/nel8w==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BYAPR10MB3014
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.517,18.0.883
 definitions=2022-06-29_22:2022-06-28,2022-06-29 signatures=0
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 mlxlogscore=999 suspectscore=0
 phishscore=0 adultscore=0 malwarescore=0 bulkscore=0 spamscore=0
 mlxscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2204290000 definitions=main-2206290072
X-Proofpoint-ORIG-GUID: 9bWhM3SS3TVu9qLUwFb24NJTATsi5icl
X-Proofpoint-GUID: 9bWhM3SS3TVu9qLUwFb24NJTATsi5icl
X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_LOW,
        RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE
        autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 06/29/22 14:39, Muchun Song wrote:
> On Tue, Jun 28, 2022 at 08:40:27AM -0700, James Houghton wrote:
> > On Mon, Jun 27, 2022 at 11:42 AM Mike Kravetz <mike.kravetz@oracle.com> wrote:
> > >
> > > On 06/24/22 17:36, James Houghton wrote:
> > > > When using HugeTLB high-granularity mapping, we need to go through the
> > > > supported hugepage sizes in decreasing order so that we pick the largest
> > > > size that works. Consider the case where we're faulting in a 1G hugepage
> > > > for the first time: we want hugetlb_fault/hugetlb_no_page to map it with
> > > > a PUD. By going through the sizes in decreasing order, we will find that
> > > > PUD_SIZE works before finding out that PMD_SIZE or PAGE_SIZE work too.
> > > >
> > >
> > > This may/will cause problems for gigantic hugetlb pages allocated at boot
> > > time.  See alloc_bootmem_huge_page() where a pointer to the associated hstate
> > > is encoded within the allocated hugetlb page.  These pages are added to
> > > hugetlb pools by the routine gather_bootmem_prealloc() which uses the saved
> > > hstate to add prep the gigantic page and add to the correct pool.  Currently,
> > > gather_bootmem_prealloc is called after hugetlb_init_hstates.  So, changing
> > > hstate order will cause errors.
> > >
> > > I do not see any reason why we could not call gather_bootmem_prealloc before
> > > hugetlb_init_hstates to avoid this issue.
> > 
> > Thanks for catching this, Mike. Your suggestion certainly seems to
> > work, but it also seems kind of error prone. I'll have to look at the
> > code more closely, but maybe it would be better if I just maintained a
> > separate `struct hstate *sorted_hstate_ptrs[]`, where the original
> 
> I don't think this is a good idea.  If you really rely on the order of
> the initialization in this patch.  The easier solution is changing
> huge_bootmem_page->hstate to huge_bootmem_page->hugepagesz. Then we
> can use size_to_hstate(huge_bootmem_page->hugepagesz) in
> gather_bootmem_prealloc().
> 

That is a much better solution.  Thanks Muchun!

-- 
Mike Kravetz