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 B365BC021A0 for ; Sun, 16 Feb 2025 00:40:52 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 8EF386B0095; Sat, 15 Feb 2025 19:40:51 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 89DAA6B0098; Sat, 15 Feb 2025 19:40:51 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 766006B009D; Sat, 15 Feb 2025 19:40:51 -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 539F76B0095 for ; Sat, 15 Feb 2025 19:40:51 -0500 (EST) Received: from smtpin12.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id F386BB33A5 for ; Sun, 16 Feb 2025 00:40:50 +0000 (UTC) X-FDA: 83123952660.12.189848C Received: from out30-131.freemail.mail.aliyun.com (out30-131.freemail.mail.aliyun.com [115.124.30.131]) by imf11.hostedemail.com (Postfix) with ESMTP id 232C04000C for ; Sun, 16 Feb 2025 00:40:47 +0000 (UTC) Authentication-Results: imf11.hostedemail.com; dkim=pass header.d=linux.alibaba.com header.s=default header.b=i9P2NcwM; spf=pass (imf11.hostedemail.com: domain of ying.huang@linux.alibaba.com designates 115.124.30.131 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=1739666449; 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=sUXs3VgNZrKBSdXD9s6NX4ZAwm8NWwUce+uHnj7ZBLk=; b=IZYGloq2XQGnPG6kVSZzahYIwx1dBMiCDe6WVBrXtIqP1qgy7mLrna3Aww85IRmCZJFu0G 3r0Scx9wSv23eAo0W2/ukVBoRAINurURnh3+hugtG9llG4hZF8fQelvG1IbgU/ImkpIBLi bcnM8m01tRngywVwMd6Izx19lABqrOE= ARC-Authentication-Results: i=1; imf11.hostedemail.com; dkim=pass header.d=linux.alibaba.com header.s=default header.b=i9P2NcwM; spf=pass (imf11.hostedemail.com: domain of ying.huang@linux.alibaba.com designates 115.124.30.131 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=1739666449; a=rsa-sha256; cv=none; b=2LF3FtCo5a9L6ZUcOfxSUPICJZ7amupa3odLE7uhma1TLtQlqbBIflOuPJXhl6pSSPBIVh Gbg3QKU60Itc/oZfd4ORsEr4J4OgeW5qwKXreNxHmuP7tcIRXV2Gqn7cbmKOdtAz5fvjzh 45oij9BwMTkdQd640t6ywi18lbbNfL8= DKIM-Signature:v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.alibaba.com; s=default; t=1739666444; h=From:To:Subject:Date:Message-ID:MIME-Version:Content-Type; bh=sUXs3VgNZrKBSdXD9s6NX4ZAwm8NWwUce+uHnj7ZBLk=; b=i9P2NcwMThWIEs6YSoAY7GM+tx5Ai/46AiqSd4v4wVwbD2bvd/WZYxVbL3JAErSyMB0mhxjEwNDIXxdjiTw8sGVqFCAoduxxjtMVWcQSkmo9+BhUEjE44+D3ds20c000Ij9x5Vv8Ym7YC1wTKSR81yiv91pyPjP7eeE5nZFYHoM= Received: from DESKTOP-5N7EMDA(mailfrom:ying.huang@linux.alibaba.com fp:SMTPD_---0WPVDkyP_1739666423 cluster:ay36) by smtp.aliyun-inc.com; Sun, 16 Feb 2025 08:40:43 +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: <20250214154557.329912-1-joshua.hahnjy@gmail.com> (Joshua Hahn's message of "Fri, 14 Feb 2025 07:45:48 -0800") References: <20250214154557.329912-1-joshua.hahnjy@gmail.com> Date: Sun, 16 Feb 2025 08:40:23 +0800 Message-ID: <87wmdq3eo8.fsf@DESKTOP-5N7EMDA> User-Agent: Gnus/5.13 (Gnus v5.13) MIME-Version: 1.0 Content-Type: text/plain; charset=ascii X-Rspam-User: X-Stat-Signature: c5jmyis1khb7usn9x5q5hq8661fkaqcn X-Rspamd-Server: rspam05 X-Rspamd-Queue-Id: 232C04000C X-HE-Tag: 1739666447-941455 X-HE-Meta: U2FsdGVkX18Bpvx43WFisG+bK752voL+zTjpf2koBfBgPxtXSvLeLufij9EXdv9FhDd/DWzau7ueuuw+RmIUFT30RDoTsnI0IzwxHWIe5FUNuHqjUEoLHKC96WXkUeW/ep7qTz6NRSjt0gEEXx8Q4on9o0CgPbFJk/bOD1fvPcvuQah+r0vtrs1i8uaKBoKrorZSOvoqm4ZmBzFU1u5OCrQc824Y9kx8vF2RdqkW32ihxr+bgQ4jWgVf1XplCpywROybbXIyuOfDqkbP86odxKWV4bQPOrBX3u/OEZDL1UubRPDEce6r5T+9Bslx+TzLMmxNx8qT8Zd0UQKWs7AQO63nlsVtxLa1DXhYmIMolV7BjqJiymS7CNXnxRfXDieL0GRskFQmKv9c3coCNhV4qS6kqI6JnOfcyd51G/T8WusrdxGpz0EW5LsT2im9HjzAqzfzcoQgIHGBVmJaWkJ9hG8O3g3brDMPnAeL8V2bJBgQoK42wYPbt+nfmvFGCvwLOAtMa3oe/LAGlWVHSeB3sIIeEMdm/3KvGmQoeRNzTrmvbKkQzt+16JgUeRL1B4ab/XIJ0QHTQkkZJhfQ+oiRoY8X7EZ2y5E0lvaGBzgNAGHasIZeYPUXcjinCCDNZZEx5HqSWlBmlKlMF36M5cZX7re594rsJTdXaV3OZRJwdPxyR0ox/ikQ9XDCeoeg3c0uEtdGMe3oO8ACOgLFOLjCDSTny0aWrpWDYdyCRfpFVdIxksTTeOVP53lCzrqKM9fi1RODH6OtpnygasUW2DlqhpoRtEy90Wt+dRbKs2HA37z0ggwJzWGHxurmJj3rM44wkJ2mJBT3fNCunQckhP9jV480IaVsodGCs2zMPdA5Q7P1/QyjdNLo8OGU7icYGHBf65lGAqFhjX8g8vxVMEzGmp7t/Tm2YXl7r1krt7Pqy53BglO5mCleFIKzuy64eNKEtuttIlpnid5nwUjTbnW PKPCUelp /M3niDF1o6jkRstrLU6COGJssNNwEKGPOyskIxDSnyCQubVV7BWlNc4wbh/bcKkpyLbRPwcbCh2RLRirxqjim0uWZbdgsXFPaAk6yu4tSj3qnpNwcUPDWPU52RFPHrdEG9RfxydhNaUD8VsOz35cGaFjEMGgLQOKpG5wVrSh+7foRGX+uqdLm3rTyrvfQA0u6Fzktnba2iSxrDgcM74Qcum83tHsXrjevLOimsfPNah4gPvmMjemcJKSIuf5kHRvkBOX/uJsUzgRFcNwrEuIRaCvptJX9KIzvNvwZiYkQgt1V/ywDpGyjLR0dPI6oezuXwq8XtMTGPwZML0k= 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 Thu, 13 Feb 2025 09:32:49 +0800 "Huang, Ying" wrote: > >> Joshua Hahn writes: >> >> > On Wed, 12 Feb 2025 10:49:32 +0800 "Huang, Ying" wrote: >> > >> >> Hi, Joshua, >> >> > > [...snip...] > >> >> > + 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. > > That sounds good to me. I don't have any strong opinions about this > change, so I am happy to combine them into a struct. I just want to > make sure I am understanding your perspective correctly: what is the > incosistency between weighted_interleave_auto and iw_table[]? > If I move the weighted_interleave_auto = true statement inside the > rcu section, will the inconsistency still be there? Because weighted_interleave_auto and iw_table are 2 variables, you may read new weighted_interleave_auto and old iw_table or vice versa. If we put them into one struct and write/read the pointer to the struct with rcu_assign_pointer() / rcu_dereference(), we can avoid this. > Just want to make sure so that I am not missing anything important! > > Thank you again for your great feedback. I hope you have a happy Friday! Thanks! --- Best Regards, Huang, Ying