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 8FF05C02198 for ; Thu, 13 Feb 2025 01:32:59 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 27EA86B0089; Wed, 12 Feb 2025 20:32:59 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 207536B008A; Wed, 12 Feb 2025 20:32:59 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 0A8966B008C; Wed, 12 Feb 2025 20:32:59 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id DDFA96B0089 for ; Wed, 12 Feb 2025 20:32:58 -0500 (EST) Received: from smtpin25.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id 65E3B1415F8 for ; Thu, 13 Feb 2025 01:32:58 +0000 (UTC) X-FDA: 83113197636.25.6638774 Received: from out30-124.freemail.mail.aliyun.com (out30-124.freemail.mail.aliyun.com [115.124.30.124]) by imf16.hostedemail.com (Postfix) with ESMTP id A0B17180012 for ; Thu, 13 Feb 2025 01:32:55 +0000 (UTC) Authentication-Results: imf16.hostedemail.com; dkim=pass header.d=linux.alibaba.com header.s=default header.b=EhotGycd; spf=pass (imf16.hostedemail.com: domain of ying.huang@linux.alibaba.com designates 115.124.30.124 as permitted sender) smtp.mailfrom=ying.huang@linux.alibaba.com; dmarc=pass (policy=none) header.from=linux.alibaba.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1739410376; a=rsa-sha256; cv=none; b=nuMjMScZ6MuLs8uEhc19MMojGqIeTetbFgYoT0bMQ0yB6ypPWvFFTz321dyS96BQ3bCpWy YZY5h+NbjONwQd0RliSl/FR4GFeZLkS4x3gw2eYYqVpbGvJhl5WTaMys1KFENR2t3r+lnA I8h1k7cBGp/zhB/WDcWBPzL/O38c7cA= ARC-Authentication-Results: i=1; imf16.hostedemail.com; dkim=pass header.d=linux.alibaba.com header.s=default header.b=EhotGycd; spf=pass (imf16.hostedemail.com: domain of ying.huang@linux.alibaba.com designates 115.124.30.124 as permitted sender) smtp.mailfrom=ying.huang@linux.alibaba.com; dmarc=pass (policy=none) header.from=linux.alibaba.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1739410376; 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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=Sf4lxMxbRNIuQuKh4DRNiD6dS8R5PiKaGlbqvYykW5g=; b=Y3M7ensl9SNVSWkh+9KtG8w0VIA5pnu5Fz7Iki0anqY8Wav1DyfE9I3D31/uTQmyBEDWcL VWP+vG2Eibh0bDH6pgT8Dcv/W6GiJ2trhgPMhB28bOOm6xaoDqqKzgljnKm3x+4enEvdtz lS9zu+kHFbm4v8PbMn2nEHHYyQc+iyE= DKIM-Signature:v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.alibaba.com; s=default; t=1739410372; h=From:To:Subject:Date:Message-ID:MIME-Version:Content-Type; bh=Sf4lxMxbRNIuQuKh4DRNiD6dS8R5PiKaGlbqvYykW5g=; b=EhotGycdyoOIgvx48XY6ekjBejfhl8mU7Tqh9AQukvKnNjNtKW5RjBnzMZZ5sAe586d5w0uyKhcrbtC/WXBVMYUT+cq8S3wIrowbDQwjZyNlci4gTtZ81jtpVM1uxE0GjpJ3RdAHsdBYNOMdA4Did4uIC02lGrDHfI2F2ZgxvPI= Received: from DESKTOP-5N7EMDA(mailfrom:ying.huang@linux.alibaba.com fp:SMTPD_---0WPLZXLb_1739410370 cluster:ay36) by smtp.aliyun-inc.com; Thu, 13 Feb 2025 09:32:51 +0800 From: "Huang, Ying" To: Joshua Hahn Cc: gourry@gourry.net, hyeonggon.yoo@sk.com, honggyu.kim@sk.com, akpm@linux-foundation.org, rafael@kernel.org, lenb@kernel.org, gregkh@linuxfoundation.org, rakie.kim@sk.com, dan.j.williams@intel.com, Jonathan.Cameron@huawei.com, dave.jiang@intel.com, horen.chuang@linux.dev, hannes@cmpxchg.org, linux-kernel@vger.org, linux-acpi@vger.kernel.org, linux-mm@kvack.org, kernel-team@meta.com Subject: Re: [PATCH v5] mm/mempolicy: Weighted Interleave Auto-tuning In-Reply-To: <20250212170645.1080771-1-joshua.hahnjy@gmail.com> (Joshua Hahn's message of "Wed, 12 Feb 2025 09:06:40 -0800") References: <20250212170645.1080771-1-joshua.hahnjy@gmail.com> Date: Thu, 13 Feb 2025 09:32:49 +0800 Message-ID: <87tt8y1vem.fsf@DESKTOP-5N7EMDA> User-Agent: Gnus/5.13 (Gnus v5.13) MIME-Version: 1.0 Content-Type: text/plain; charset=ascii X-Stat-Signature: u65dyjw88ke3xkssfgaa7f5b3xa34gf1 X-Rspamd-Server: rspam06 X-Rspamd-Queue-Id: A0B17180012 X-Rspam-User: X-HE-Tag: 1739410375-149568 X-HE-Meta: U2FsdGVkX191/5dCR+0oe92OUhtaxw9uekEt1/6H5jn8wejOYaAOAMlP+8/S4rkZfhiHWu290C4VRdVSWDzxz7UmgptOFzioDeF7AC3v/TXG/JgUPRPo0Lt3C5pnFHgPVvpT1K6HpC7WyqMTDyAq5KqdzNDtNCTq6M58BJI9YFW7lacb3onKhtM8+AD/mPg6WG8UZbGd5O66uNY8RE8mSrXo3oR68UcL4w4eizkgvB02vpIsQutIfEfukipugW4y4TJH5bJu5TVm+SM7lMQuo99uzVW10eYBSyBLy6QpZQwMWf1LV+Uy9Gtjh7mkWa4x2qQRS4wF1evy0PUYcnBH9Y5WnHiP98oYk+ed4ps4VavJVwW5K3x8eynebUTFeQinBerJRXopOBs17k/u7D8o5LYakSWFweznFky4cBsMUx/KeNBZ8Nxd15CPw+lnEr6wnVXynprQFX1kdcjs3ZJCijtz3qyQW5Lj6c1ulFF5xPr87Kyb5YCtBmkI6VJ1xYDmTzy8pGdhPy6J6gmcjXXKpXYYO2Yf6t8FHOJQQNs8dZ35I0gOuwoInweyjZ8y3ARIY9hA4V1wpPM7PRqCpKnZaJ32sP/GKe/dQCDgnC20RRl3xAPIlo6SdXBwhBWJlLWZTKkzu2IsAZ9QTgiXnw6QfhNwwfd44lVNq98nXowO4ZL2bVW79fTkEND/Dxz02aP47jlo0HiLUkRzsr5gA4aDGVfoR7MT4YaHESYO78u/kyyiNZqi/GA3SZrZd/u7s/63bE8gO0DsHng5B3iFNf6CLbgTFJfS6KvSJddVmci6PZ556ijvbXnPHqSQt/AWuqwio0X6LSAhJ3wDsEm6gEIYj5Aw03apJn5zoW5aCntgf55jqGGU6vORkR264RNVZ8f/RLAUYPFtMgmOjuho6jbJamc0MZ6OuKLuxUyMbW98QefAZy7uRQL/QCrUqUUEV+VU/tJEp4hz85HiOOe8E2l Lepdk2q2 qy5Z4Hngt3WokCFyoMh5rbyk/s43fhumPnnZq5Gfu/vgTCZi+Tw/58JAshts3VM0eK8GQ3aK3sL3nMDk9p3m+ZF6J6ud9nbmxt+1yE9sPMj8+PDgmuow/ujZzjDuqJ87fQxFansaOsz0VLzqBGOujQKRZtRoWWsS2+t2VjiSYIb0o/LjyXUwsMwVGkdBrF6Aylo7YfIRBM6Kfi6WMQ2YtUagxCIvb5iaJBFlAgbA8O5NAGWx9Dzdo/3jBOOoNP6emhQ3O8FtlbbO1dUXeOLpgB64wgfnvK2ZZKLai0lF6oS5oRvsr2rohcq6hUXHtLxQLfMExg3ky4frldfI= 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: Joshua Hahn writes: > On Wed, 12 Feb 2025 10:49:32 +0800 "Huang, Ying" wrote: > >> Hi, Joshua, >> >> Thanks for your patch and sorry for late reply. > > Hi Ying, no worries! Thank you for taking time to review this patch. > >> Joshua Hahn writes: [snip] >> > + >> > +static ssize_t weighted_interleave_mode_store(struct kobject *kobj, >> > + struct kobj_attribute *attr, const char *buf, size_t count) >> > +{ >> > + uint64_t *bw; >> > + u8 *old_iw, *new_iw; >> > + >> > + if (count == 0) >> > + return -EINVAL; >> > + >> > + if (sysfs_streq(buf, "N") || sysfs_streq(buf, "0")) { >> >> kstrtobool() can be used here. It can deal with 'count == 0' case too. > > These kernel string tools are very helpful, thank you for bringing > them to my attention : -) > >> > + weighted_interleave_auto = false; >> > + return count; >> > + } else if (!sysfs_streq(buf, "Y") && !sysfs_streq(buf, "1")) { >> > + return -EINVAL; >> > + } >> > + >> > + new_iw = kcalloc(nr_node_ids, sizeof(u8), GFP_KERNEL); >> > + if (!new_iw) >> > + return -ENOMEM; >> > + >> > + mutex_lock(&iw_table_lock); >> > + bw = node_bw_table; >> > + >> > + if (!bw) { >> > + mutex_unlock(&iw_table_lock); >> > + kfree(new_iw); >> > + return -ENODEV; >> > + } >> > + >> > + old_iw = rcu_dereference_protected(iw_table, >> > + lockdep_is_held(&iw_table_lock)); >> > + >> > + reduce_interleave_weights(bw, new_iw); >> > + rcu_assign_pointer(iw_table, new_iw); >> > + mutex_unlock(&iw_table_lock); >> > + >> > + synchronize_rcu(); >> > + kfree(old_iw); >> > + >> > + weighted_interleave_auto = true; >> >> Why assign weighted_interleave_auto after synchronize_rcu()? To reduce >> the race window, it's better to change weighted_interleave_auto and >> iw_table together? Is it better to put them into a data structure and >> change them together always? >> >> struct weighted_interleave_state { >> bool weighted_interleave_auto; >> u8 iw_table[0] >> }; > > I see, I think your explanation makes sense. For the first question, > I think your point makes sense, so I will move the updating to be > inside the rcu section. > > As for the combined data structure, I think that this makes sense, > but I have a few thoughts. First, there are some times when we don't > update both of them, like moving from auto --> manual, and whenever > we just update iw_table, we don't need to update the weighted_interleave > auto field. I also have a concern that this might make the code a bit > harder to read, but that is just my humble opinion. I think the overhead is relatively small. With that, we can avoid the inconsistency between weighted_interleave_auto and iw_table[]. struct_size() or struct_size_t() family helpers can be used to manage the flexible array at the end of the struct. --- Best Regards, Huang, Ying