From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id F0D5BECAAD1 for ; Thu, 1 Sep 2022 08:50:55 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 7AA1A8D0001; Thu, 1 Sep 2022 04:50:55 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 7570B6B0073; Thu, 1 Sep 2022 04:50:55 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 61EF48D0001; Thu, 1 Sep 2022 04:50:55 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id 524AA6B0072 for ; Thu, 1 Sep 2022 04:50:55 -0400 (EDT) Received: from smtpin07.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id 24F17161374 for ; Thu, 1 Sep 2022 08:50:55 +0000 (UTC) X-FDA: 79862896470.07.09DB680 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by imf04.hostedemail.com (Postfix) with ESMTP id ADEA240050 for ; Thu, 1 Sep 2022 08:50:54 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1662022254; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=1mB7ezn0iyp9ChcftBk9oPegsbCSz+N0Ye/fVxbqIEk=; b=faVi7IrblyJwc1l6MAQpJp/5DWiI0JKKDUChfJ/JDIIni0eCULZlyWNkGW3Av/QIE8o/Wx NGT6rE9NZfq6luZevRGYCIE6Sr4W6AkGJlgh0FCx+6IdFxh7bkqFtz/diwLN/+jGDBKdzk gqnUje0hErDEtnjdc1bIpb6a7eRKsZg= Received: from mail-wm1-f71.google.com (mail-wm1-f71.google.com [209.85.128.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-615-5HCJDsqkPwiyEz-l_E6l2g-1; Thu, 01 Sep 2022 04:50:51 -0400 X-MC-Unique: 5HCJDsqkPwiyEz-l_E6l2g-1 Received: by mail-wm1-f71.google.com with SMTP id f7-20020a1c6a07000000b003a60ede816cso580229wmc.0 for ; Thu, 01 Sep 2022 01:50:51 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:in-reply-to:subject:organization:from :references:cc:to:content-language:user-agent:mime-version:date :message-id:x-gm-message-state:from:to:cc; bh=1mB7ezn0iyp9ChcftBk9oPegsbCSz+N0Ye/fVxbqIEk=; b=8PbGrfK0ofWgCeOivoA9YELojBRzHX+XO4SbY9voBFY8ScH6PHY+PZ79Van3aoVrzC tsb8OtraEpHdbJUHYgGa59mgzeCyxRZ3VskWwmkRRJVaUb2EKDICGK4XAFWWqNtGiQXm OXBd7nQIfGK+x4e5crlhezxVeDtlb460P9Dn7Nj8m3hRFuNwr/5tzDfUoS3iHJ+RdrgO EvpGQ2tkrKgB2f1MYRt6Jenfo5uBijZuu0RwI+eE9HqRRa9HIK1/yYZJ74Wf2YZBXw6v 1Flmlzsz8WIcmXFxJ4iVaPTfQ2wIR4g7Jyr0MZo4k1bFHxKI9T/Oc5D1yepeMoOTUrl+ B2uA== X-Gm-Message-State: ACgBeo24/x+gcESmiTiGhZqZP0Je6NB44uv3bA0BIcSgff01Ayrjl6/K XNqiJjlS5eLYiU+GgM7zqLFRt3c5uP4nO7DdWVxnepI4lat6gFGwykahBC1qeSpDZMQ+J7p3cgb I8NqJ3MJJyf8= X-Received: by 2002:a05:6000:2aa:b0:225:63d0:83aa with SMTP id l10-20020a05600002aa00b0022563d083aamr14274057wry.499.1662022250200; Thu, 01 Sep 2022 01:50:50 -0700 (PDT) X-Google-Smtp-Source: AA6agR6CHrqsgTkQ9IMmDx49M4N8jRa/DKqcq/ceGU0tz49TF74mXtjCTBTiR37Dpgk60uCLAyzZTA== X-Received: by 2002:a05:6000:2aa:b0:225:63d0:83aa with SMTP id l10-20020a05600002aa00b0022563d083aamr14274041wry.499.1662022249860; Thu, 01 Sep 2022 01:50:49 -0700 (PDT) Received: from ?IPV6:2003:cb:c707:9e00:fec0:7e96:15cb:742? (p200300cbc7079e00fec07e9615cb0742.dip0.t-ipconnect.de. [2003:cb:c707:9e00:fec0:7e96:15cb:742]) by smtp.gmail.com with ESMTPSA id n8-20020a5d4848000000b00226d01a4635sm13974535wrs.35.2022.09.01.01.50.48 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 01 Sep 2022 01:50:49 -0700 (PDT) Message-ID: Date: Thu, 1 Sep 2022 10:50:48 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.12.0 To: Muchun Song , gregkh@linuxfoundation.org, rafael@kernel.org, mike.kravetz@oracle.com, akpm@linux-foundation.org, osalvador@suse.de, ying.huang@intel.com, aneesh.kumar@linux.ibm.com, rientjes@google.com Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org References: <20220901083023.42319-1-songmuchun@bytedance.com> From: David Hildenbrand Organization: Red Hat Subject: Re: [PATCH] mm: hugetlb: eliminate memory-less nodes handling In-Reply-To: <20220901083023.42319-1-songmuchun@bytedance.com> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1662022254; a=rsa-sha256; cv=none; b=GP/1M/nvoj/XitJK+ZxwFgvNwz64L5azCZuPzhRn8jrlSsOTm8sECXwkPbdnTOAnw5mrCb VTVb8qgUN89AWIetiHhczgCifeDhAiLHDu1l3Se/N+Hd4jrhxrTimPPerwNLU/m11dkbg6 rF5YhChHYJE1+nAsfXuIa5I+oFeKvjw= ARC-Authentication-Results: i=1; imf04.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=faVi7Irb; dmarc=pass (policy=none) header.from=redhat.com; spf=pass (imf04.hostedemail.com: domain of david@redhat.com designates 170.10.129.124 as permitted sender) smtp.mailfrom=david@redhat.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1662022254; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=1mB7ezn0iyp9ChcftBk9oPegsbCSz+N0Ye/fVxbqIEk=; b=KTyQh1L7DQ86D2mtQx+/JBEv571cuWNJxOA8kWQ4RRxL1PsxlpLW59gBfVREfEXNmZcy5O BIb1S92yByrFI1bs3lk7/PUvu+5P5kOvPBYjSCMWlc2tkThG6e5IBDsgiLf/FtBalIPd6K FsDPA+2XsNIratPh/BmtYcwYV0aFAYo= X-Rspam-User: Authentication-Results: imf04.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=faVi7Irb; dmarc=pass (policy=none) header.from=redhat.com; spf=pass (imf04.hostedemail.com: domain of david@redhat.com designates 170.10.129.124 as permitted sender) smtp.mailfrom=david@redhat.com X-Rspamd-Server: rspam07 X-Stat-Signature: 3jogrog4x8qqp17ord1pznz959kofs7t X-Rspamd-Queue-Id: ADEA240050 X-HE-Tag: 1662022254-363236 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On 01.09.22 10:30, Muchun Song wrote: > The memory-notify-based approach aims to handle meory-less nodes, however, it just adds > the complexity of code as pointed by David in thread [1]. The handling of memory-less > nodes is introduced by commit 4faf8d950ec4 ("hugetlb: handle memory hot-plug events"). > From its commit message, we cannot find any necessity of handling this case. So, we can > simply register/unregister sysfs entries in register_node/unregister_node to simlify the > code. > > https://lore.kernel.org/linux-mm/60933ffc-b850-976c-78a0-0ee6e0ea9ef0@redhat.com/ [1] > Suggested-by: David Hildenbrand > Signed-off-by: Muchun Song > --- > drivers/base/node.c | 7 +++++-- > include/linux/node.h | 5 +++++ > mm/hugetlb.c | 37 ++++++++++--------------------------- > 3 files changed, 20 insertions(+), 29 deletions(-) > > diff --git a/drivers/base/node.c b/drivers/base/node.c > index ed391cb09999..cf115d5a9b8a 100644 > --- a/drivers/base/node.c > +++ b/drivers/base/node.c > @@ -608,10 +608,12 @@ static int register_node(struct node *node, int num) > node->dev.groups = node_dev_groups; > error = device_register(&node->dev); > > - if (error) > + if (error) { > put_device(&node->dev); > - else > + } else { > + hugetlb_register_node(node); > compaction_register_node(node); > + } Good, so this matches what other code does. > > return error; > } > @@ -625,6 +627,7 @@ static int register_node(struct node *node, int num) > */ > void unregister_node(struct node *node) > { > + hugetlb_unregister_node(node); > compaction_unregister_node(node); > node_remove_accesses(node); > node_remove_caches(node); > diff --git a/include/linux/node.h b/include/linux/node.h > index 427a5975cf40..f5d41498c2bf 100644 > --- a/include/linux/node.h > +++ b/include/linux/node.h > @@ -138,6 +138,11 @@ extern void unregister_memory_block_under_nodes(struct memory_block *mem_blk); > extern int register_memory_node_under_compute_node(unsigned int mem_nid, > unsigned int cpu_nid, > unsigned access); > + > +#ifdef CONFIG_HUGETLBFS > +void hugetlb_register_node(struct node *node); > +void hugetlb_unregister_node(struct node *node); > +#endif compaction_register_node() resides in include/linux/compaction.h, so I wonder if this should go into hugetlb.h (unless it causes trouble) > #else > static inline void node_dev_init(void) > { > diff --git a/mm/hugetlb.c b/mm/hugetlb.c > index d0617d64d718..722e862bb6be 100644 > --- a/mm/hugetlb.c > +++ b/mm/hugetlb.c > @@ -3898,6 +3898,7 @@ static void __init hugetlb_sysfs_init(void) > } > > #ifdef CONFIG_NUMA > +static bool hugetlb_initialized __ro_after_init; We set it out of hugetlb_register_all_nodes(), so it conceptually not correct. We either need a better name here or set it from generic init code. You could call it hugetlb_sysfs_initialized() and set that from hugetlb_sysfs_init(), which is called just before hugetlb_register_all_nodes(). [ shouldn't hugetlb_register_all_nodes() get called from hugetlb_sysfs_init() ? it's all about sysfs as well ... ] > > /* > * node_hstate/s - associate per node hstate attributes, via their kobjects, > @@ -3953,7 +3954,7 @@ static struct hstate *kobj_to_node_hstate(struct kobject *kobj, int *nidp) > * Unregister hstate attributes from a single node device. > * No-op if no hstate attributes attached. > */ > -static void hugetlb_unregister_node(struct node *node) > +void hugetlb_unregister_node(struct node *node) > { > struct hstate *h; > struct node_hstate *nhs = &node_hstates[node->dev.id]; > @@ -3983,19 +3984,22 @@ static void hugetlb_unregister_node(struct node *node) > * Register hstate attributes for a single node device. > * No-op if attributes already registered. > */ > -static int hugetlb_register_node(struct node *node) > +void hugetlb_register_node(struct node *node) > { > struct hstate *h; > struct node_hstate *nhs = &node_hstates[node->dev.id]; > int err; > > + if (!hugetlb_initialized) > + return; > + > if (nhs->hugepages_kobj) > - return 0; /* already allocated */ > + return; /* already allocated */ > > nhs->hugepages_kobj = kobject_create_and_add("hugepages", > &node->dev.kobj); > if (!nhs->hugepages_kobj) > - return -ENOMEM; > + return; > > for_each_hstate(h) { > err = hugetlb_sysfs_add_hstate(h, nhs->hugepages_kobj, > @@ -4005,28 +4009,9 @@ static int hugetlb_register_node(struct node *node) > pr_err("HugeTLB: Unable to add hstate %s for node %d\n", > h->name, node->dev.id); > hugetlb_unregister_node(node); > - return -ENOMEM; > + break; > } > } > - return 0; > -} > - > -static int __meminit hugetlb_memory_callback(struct notifier_block *self, > - unsigned long action, void *arg) > -{ > - int ret = 0; > - struct memory_notify *mnb = arg; > - int nid = mnb->status_change_nid; > - > - if (nid == NUMA_NO_NODE) > - return NOTIFY_DONE; > - > - if (action == MEM_GOING_ONLINE) > - ret = hugetlb_register_node(node_devices[nid]); > - else if (action == MEM_CANCEL_ONLINE || action == MEM_OFFLINE) > - hugetlb_unregister_node(node_devices[nid]); > - > - return notifier_from_errno(ret); > } > > /* > @@ -4038,11 +4023,9 @@ static void __init hugetlb_register_all_nodes(void) > { > int nid; > > - get_online_mems(); > - hotplug_memory_notifier(hugetlb_memory_callback, 0); > + hugetlb_initialized = true; > for_each_node_state(nid, N_MEMORY) > hugetlb_register_node(node_devices[nid]); > - put_online_mems(); > } > #else /* !CONFIG_NUMA */ > Apart from the comments, looks good and clean to me. Thanks! -- Thanks, David / dhildenb