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 419D8C36010 for ; Wed, 2 Apr 2025 01:28:56 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id E5BE3280003; Tue, 1 Apr 2025 21:28:54 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id DE557280001; Tue, 1 Apr 2025 21:28:54 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id C5D54280003; Tue, 1 Apr 2025 21:28:54 -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 A4118280001 for ; Tue, 1 Apr 2025 21:28:54 -0400 (EDT) Received: from smtpin30.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 1FBCCB9C77 for ; Wed, 2 Apr 2025 01:28:55 +0000 (UTC) X-FDA: 83287369830.30.8DFF5D4 Received: from invmail4.hynix.com (exvmail4.skhynix.com [166.125.252.92]) by imf06.hostedemail.com (Postfix) with ESMTP id B34C8180005 for ; Wed, 2 Apr 2025 01:28:52 +0000 (UTC) Authentication-Results: imf06.hostedemail.com; dkim=none; spf=pass (imf06.hostedemail.com: domain of rakie.kim@sk.com designates 166.125.252.92 as permitted sender) smtp.mailfrom=rakie.kim@sk.com; dmarc=none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1743557333; 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-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=GY+5xWZh6H7R8faxgM6LeDXoEX4Ku0Vf/4EG2tiT/74=; b=ZGFYI61ARaNnD57PoM4/42ju46AzAzF4sSExydyV+ypxdxWMrO6A9sVgSVWhuyulXDp5zu z9+Jmd1Q9RgB8JwfKV8jjhoKGiYTZfayNTHJ9LwMPqSHAv4nGZwIK3BDxlel23G2BnAtJQ gYxpadLwMtEi+xAD8CAbQoxbz1wnwgY= ARC-Authentication-Results: i=1; imf06.hostedemail.com; dkim=none; spf=pass (imf06.hostedemail.com: domain of rakie.kim@sk.com designates 166.125.252.92 as permitted sender) smtp.mailfrom=rakie.kim@sk.com; dmarc=none ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1743557333; a=rsa-sha256; cv=none; b=tMjP+gR8hVDbiIFh7Kb+ENGnF/kGPXORSahObexPESNXfzTDejsTtj1ADSOHERiRDpnnu0 61lclc4/+st1WlHuC4ylfClXG8HxU9Ne/d4dkX4e1ZpnH9MUQ9uTKD+v1wtaukXzrq5c+q rhJy2QU886oq1H0zirriDrT8JL5x0f4= X-AuditID: a67dfc5b-681ff7000002311f-18-67ec92d1caba From: Rakie Kim To: Gregory Price Cc: akpm@linux-foundation.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, linux-cxl@vger.kernel.org, joshua.hahnjy@gmail.com, dan.j.williams@intel.com, ying.huang@linux.alibaba.com, david@redhat.com, Jonathan.Cameron@huawei.com, kernel_team@skhynix.com, honggyu.kim@sk.com, yunjeong.mun@sk.com, Rakie Kim Subject: Re: [PATCH v4 3/3] mm/mempolicy: Support memory hotplug in weighted interleave Date: Wed, 2 Apr 2025 10:28:37 +0900 Message-ID: <20250402012845.1064-1-rakie.kim@sk.com> X-Mailer: git-send-email 2.48.1.windows.1 In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFrrHLMWRmVeSWpSXmKPExsXC9ZZnoe6lSW/SDe4d5bSYs34Nm8X0qRcY Lb6u/8Vs8fPucXaLVQuvsVkc3zqP3eL8rFMsFpd3zWGzuLfmP6vF6jUZDlweO2fdZffobrvM 7tFy5C2rx+I9L5k8Nn2axO5xYsZvFo+dDy093u+7yubxeZNcAGcUl01Kak5mWWqRvl0CV8ak K7+ZCv7IVJx5eo2lgbFHvIuRk0NCwESip72PtYuRA8zetKIYxGQTUJI4tjcGxBQRUJVou+Le xcjFwSzwiEnidPtSVpBOYYFwiX+fPoHZLEA10z42gNm8QFMmXt/ACjFdU6Lh0j0mEJtTwExi 9/UPYHEhAR6JVxv2M0LUC0qcnPmEBcRmFpCXaN46mxmi9zmbxIluCwhbUuLgihssExj5ZyFp mYWkZQEj0ypGocy8stzEzBwTvYzKvMwKveT83E2MwFBfVvsnegfjpwvBhxgFOBiVeHgbeN+k C7EmlhVX5h5ilOBgVhLhjfj6Ml2INyWxsiq1KD++qDQntfgQozQHi5I4r9G38hQhgfTEktTs 1NSC1CKYLBMHp1QDo83EPxsWPs9RCta5mqBrs/SNUurdu6efLta49nzPcp672nEi1+patrX7 reL6Ibnv5DwZr2I27gxb5ZU2fm1bOcT/xCeUT2HNOq5q8Wsuk7CrVbpI8YfAh/e28f5RUWuU ymoImaogYmAnvW+zwLWvq4yOHTx65IDqeVOpm8eEe+/5VbhYCuaZKbEUZyQaajEXFScCAGMO dz5xAgAA X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFrrJLMWRmVeSWpSXmKPExsXCNUNNS/fipDfpBtN3mVjMWb+GzWL61AuM Fl/X/2K2+Hn3OLvF52evmS1WLbzGZnF86zx2i8NzT7JanJ91isXi8q45bBb31vxntTh07Tmr xeo1GRa/t61gc+Dz2DnrLrtHd9tldo+WI29ZPRbvecnksenTJHaPEzN+s3jsfGjp8X7fVTaP b7c9PBa/+MDk8XmTXAB3FJdNSmpOZllqkb5dAlfGpCu/mQr+yFSceXqNpYGxR7yLkYNDQsBE YtOKYhCTTUBJ4tjeGBBTREBVou2KexcjFwezwCMmidPtS1m7GDk5hAXCJf59+gRmswDVTPvY AGbzAk2ZeH0DmC0hoCnRcOkeE4jNKWAmsfv6B7C4kACPxKsN+xkh6gUlTs58wgJiMwvISzRv nc08gZFnFpLULCSpBYxMqxhFMvPKchMzc0z1irMzKvMyK/SS83M3MQIDfFntn4k7GL9cdj/E KMDBqMTDK9DzJl2INbGsuDL3EKMEB7OSCG/E15fpQrwpiZVVqUX58UWlOanFhxilOViUxHm9 wlMThATSE0tSs1NTC1KLYLJMHJxSDYy6+xdef2gTH/x4hdOxz6c0/k9ZO/FT1iVzhQ1b8hjW aqXWzm73b+4+ePui++blrTffqt5MbfjAlbHLTM3wXQDXkw2rDcLKKtcYRaSGnrlx+95exjta ypsLJVb4avRZ2ojwxGpvTX28+kP/4qjJb81/tYW9qfl8z35rylolfstVk8uUdQ4pffRSYinO SDTUYi4qTgQArKrENWwCAAA= X-CFilter-Loop: Reflected X-Rspamd-Queue-Id: B34C8180005 X-Stat-Signature: yxzni9eza68nyng77s3scscttpzmgdmb X-Rspam-User: X-Rspamd-Server: rspam12 X-HE-Tag: 1743557332-154054 X-HE-Meta: U2FsdGVkX19cZ4PC05u1qlIlTeojqWDNFw8ICcGUZTQAxu4wvrvaSKQYPxCFy5wUuVlOO3P20m6TWSggoJ5mO8/IqX56ggk1wiFJkyyHf0ueXG3VWUtuvsJnyVULsFpeBo5rr/7n/WidKXAtC4dMKD7RtnmMdUkaBeLnBlljcDhuHK2/C806eivBWKOINd7FcxLzpOzb99IBzO9ZaQCMkl5EPRil3qnzRulpN5Rua9HRJ2tN/51B4/peHwcmuzeNctoZaBlm9mzk/J/iqGsHoaMdKkfo4L0fDkgbAomu7NWBrnz2lwiPxTkuyh8q1Rlea+sX12GqqW5ngF1pLqSSuL1fiAykuT0e91PLL6zuJRQLnyF+VF3ottf0gASM5nwNex+4P0t7/MZ50q1WXF7d2qoPydl9ex7Wy/bnsspIqmyTTo5Diz5xLx8hvfyBpaGJbMLz9h0JioNsK/7kQikD3VYll5ebhswi1qJLjMhZGN1AuAfJWgRhUIdJh8Q9lu2XBUbb3Ff9BVuG4ErVPZTyZH5zoGzVQN/wbihfweCGECxswABpZ83+6D3/18JsADtPIP3MKS2w80F6V+y5uCDKjYIFTolAxR8mAwIO2dlxWql0a+M0J11hPctzXheUw116CIcIXqXBeu5FKtBCgByk+eSUlp9qHsSPLZTlIJCUI8hnd2D1d+5OErjYIzco1eWRbCMqZrEYAs/ImT+kN+BOUEsAW4TImZCKdDsH46mQcaZ7+ORsE0gR4q4AlF6LZyGXLu6A7vlFyZn9+9ERTLJUb/0XFMY/lWeoBcShWG29poUK4bJayYLSOlK6oQNFPCRd8U+qaSa4v1ivGmD3hq9rpmwdhftU6WitfKC32L+r3bKKpU8T2xUDMXPtPRt+PgVHrFVvRX7YqH+/0h1StrSFc7Evt44PC9fg5EoyLRG9zoY6pw8N2KrpZMUeX+/7PlDRVuKgjjUvQWJElffNIC6 JuNN7CzY msFyOyCE6j/LuwYbxXTzr06qwPMji8lNzFnqh+2GM6EwJsQcdBjExI9c6MmkIIAMAumXXlr9R7+s2eIx8XaOK4i85MWQs+BpMe34vj1YKL2Z5Uv2d9Ojyw9k0RYx2TsOgaS7B 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: List-Subscribe: List-Unsubscribe: On Tue, 1 Apr 2025 16:32:42 -0400 Gregory Price wrote: > On Tue, Apr 01, 2025 at 06:08:59PM +0900, Rakie Kim wrote: > > static void sysfs_wi_release(struct kobject *wi_kobj) > > @@ -3464,35 +3477,84 @@ static const struct kobj_type wi_ktype = { > > > > static int sysfs_wi_node_add(int nid) > > { > ... snip .. > > + mutex_lock(&wi_group->kobj_lock); > > + if (wi_group->nattrs[nid]) { > > + mutex_unlock(&wi_group->kobj_lock); > > + pr_info("Node [%d] already exists\n", nid); > > + kfree(new_attr); > > + kfree(name); > > + return 0; > > + } > > > > - if (sysfs_create_file(&wi_group->wi_kobj, &node_attr->kobj_attr.attr)) { > > - kfree(node_attr->kobj_attr.attr.name); > > - kfree(node_attr); > > - pr_err("failed to add attribute to weighted_interleave\n"); > > - return -ENOMEM; > > + wi_group->nattrs[nid] = new_attr; > > + mutex_unlock(&wi_group->kobj_lock); > > + > > Shouldn't all of this > vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv > > + sysfs_attr_init(&wi_group->nattrs[nid]->kobj_attr.attr); > > + wi_group->nattrs[nid]->kobj_attr.attr.name = name; > > + wi_group->nattrs[nid]->kobj_attr.attr.mode = 0644; > > + wi_group->nattrs[nid]->kobj_attr.show = node_show; > > + wi_group->nattrs[nid]->kobj_attr.store = node_store; > > + wi_group->nattrs[nid]->nid = nid; > > + > > + ret = sysfs_create_file(&wi_group->wi_kobj, > > + &wi_group->nattrs[nid]->kobj_attr.attr); > > + if (ret) { > > + kfree(wi_group->nattrs[nid]->kobj_attr.attr.name); > > + kfree(wi_group->nattrs[nid]); > > + wi_group->nattrs[nid] = NULL; > > + pr_err("Failed to add attribute to weighted_interleave: %d\n", ret); > > } > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > > Be happening inside the lock as well? I agree that applying your suggestion would make the code more robust. I will update the patch to follow your recommendation. > > > + > > + switch(action) { > > + case MEM_ONLINE: > > + if (node_state(nid, N_MEMORY)) { > > Hm, I see the issue here, ok, this node_state check isn't needed, as it > will always be true. So this function needs to handle duplicates still. Yes, you're right. The `node_state(nid, N_MEMORY)` check I added here is redundant because it will always be true in this context. I will remove it to avoid unnecessary duplication. > vvv > > + err = sysfs_wi_node_add(nid); > > + if (err) { > > + pr_err("failed to add sysfs [node%d]\n", nid); > > + return NOTIFY_BAD; > > + } > > + } > > + break; > > + case MEM_OFFLINE: > > + if (!node_state(nid, N_MEMORY)) > > This check is good for the time being. This check looks appropriate for now and I'll keep it as-is. > > > + sysfs_wi_node_release(nid); > > + break; > > + } > > + > > +notifier_end: > > + return NOTIFY_OK; > > } > > > > > > But really I think we probably just want to change to build on top of this: > https://lore.kernel.org/all/20250401092716.537512-2-osalvador@suse.de/ > > And use register_node_notifier with NODE_BECAME_MEMORYLESS and NODE_BECAME_MEM_AWARE > > ~Gregory Thank you for sharing the link regarding `node_notify`. I agree that the mechanism you pointed out would be a better fit for this patch. By using `register_node_notifier` with `NODE_BECAME_MEMORYLESS` and `NODE_BECAME_MEM_AWARE`, we can avoid unnecessary callbacks and implement this functionality more efficiently. However, I think it would be better to apply the current patch first and then update it to use `node_notify` once that support is finalized and available upstream. Rakie