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 X-Spam-Level: X-Spam-Status: No, score=-15.3 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7EF49C4709A for ; Thu, 3 Jun 2021 08:18:24 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 29F05613C9 for ; Thu, 3 Jun 2021 08:18:24 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 29F05613C9 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=intel.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id ABFD86B006C; Thu, 3 Jun 2021 04:18:23 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id A97336B006E; Thu, 3 Jun 2021 04:18:23 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 95ECB6B0070; Thu, 3 Jun 2021 04:18:23 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0049.hostedemail.com [216.40.44.49]) by kanga.kvack.org (Postfix) with ESMTP id 683AF6B006C for ; Thu, 3 Jun 2021 04:18:23 -0400 (EDT) Received: from smtpin09.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay03.hostedemail.com (Postfix) with ESMTP id 07408808E8DD for ; Thu, 3 Jun 2021 08:18:23 +0000 (UTC) X-FDA: 78211710486.09.13B7983 Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by imf03.hostedemail.com (Postfix) with ESMTP id 446F8C00CBFA for ; Thu, 3 Jun 2021 08:18:05 +0000 (UTC) IronPort-SDR: XazuN+pVplKeZTERiFrBH4/wMeun6yr16QH8jwRSOpgxkTdKWoBmKJsGLHUyO+2tyYk26NX4+a 8ux2Ns0j4ATg== X-IronPort-AV: E=McAfee;i="6200,9189,10003"; a="203806152" X-IronPort-AV: E=Sophos;i="5.83,244,1616482800"; d="scan'208";a="203806152" Received: from orsmga008.jf.intel.com ([10.7.209.65]) by fmsmga103.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 03 Jun 2021 01:18:14 -0700 IronPort-SDR: Vz3EMkgnrxzqyYR1QM0yVeeXgdPhuwAbc+3+6GHSra/WD7adb3BFtp85h8YhsmuqOf6LrS/i1L mL3dtJ7W9hYQ== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.83,244,1616482800"; d="scan'208";a="446209848" Received: from shbuild999.sh.intel.com (HELO localhost) ([10.239.147.94]) by orsmga008.jf.intel.com with ESMTP; 03 Jun 2021 01:18:08 -0700 Date: Thu, 3 Jun 2021 16:18:07 +0800 From: Feng Tang To: Michal Hocko , Andrew Morton Cc: linux-mm@kvack.org, David Rientjes , Dave Hansen , Ben Widawsky , Andrea Arcangeli , Mel Gorman , Mike Kravetz , Randy Dunlap , Vlastimil Babka , Andi Kleen , Dan Williams , ying.huang@intel.com Subject: Re: [v4 PATCH 2/3] mm/mempolicy: don't handle MPOL_LOCAL like a fake MPOL_PREFERRED policy Message-ID: <20210603081807.GE56979@shbuild999.sh.intel.com> References: <1622560492-1294-1-git-send-email-feng.tang@intel.com> <1622560492-1294-3-git-send-email-feng.tang@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.24 (2015-08-30) X-Rspamd-Queue-Id: 446F8C00CBFA Authentication-Results: imf03.hostedemail.com; dkim=none; spf=none (imf03.hostedemail.com: domain of feng.tang@intel.com has no SPF policy when checking 192.55.52.115) smtp.mailfrom=feng.tang@intel.com; dmarc=fail reason="No valid SPF, No valid DKIM" header.from=intel.com (policy=none) X-Rspamd-Server: rspam03 X-Stat-Signature: xh5uz785tnz5a7baej43qyusxinshaxi X-HE-Tag: 1622708285-521049 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 Thu, Jun 03, 2021 at 09:41:38AM +0200, Michal Hocko wrote: > On Tue 01-06-21 23:14:51, Feng Tang wrote: > > MPOL_LOCAL policy has been setup as a real policy, but it is still handled > > like a faked POL_PREFERRED policy with one internal MPOL_F_LOCAL flag bit > > set, and there are many places having to judge the real 'prefer' or the > > 'local' policy, which are quite confusing. > > > > In current code, there are 4 cases that MPOL_LOCAL are used: > > 1. user specifies 'local' policy > > 2. user specifies 'prefer' policy, but with empty nodemask > > 3. system 'default' policy is used > > 4. 'prefer' policy + valid 'preferred' node with MPOL_F_STATIC_NODES > > flag set, and when it is 'rebind' to a nodemask which doesn't > > contains the 'preferred' node, it will perform as 'local' policy > > > > So make 'local' a real policy instead of a fake 'prefer' one, and kill > > MPOL_F_LOCAL bit, which can greatly reduce the confusion for code reading. > > > > For case 4, the logic of mpol_rebind_preferred() is confusing, as Michal > > Hocko pointed out: > > > > : I do believe that rebinding preferred policy is just bogus and it should > > : be dropped altogether on the ground that a preference is a mere hint from > > : userspace where to start the allocation. Unless I am missing something > > : cpusets will be always authoritative for the final placement. The > > : preferred node just acts as a starting point and it should be really > > : preserved when cpusets changes. Otherwise we have a very subtle behavior > > : corner cases. > > > > So dump all the tricky transformation between 'prefer' and 'local', > > and just record the new nodemask of rebinding. > > > > Suggested-by: Michal Hocko > > Signed-off-by: Feng Tang > > Acked-by: Michal Hocko Thanks! > But this having another pair of eyes would be definitely helpful. > Still one nit though Yes, more review and suggestions are welcome and appreciated. > > @@ -234,30 +229,27 @@ static int mpol_set_nodemask(struct mempolicy *pol, > > /* if mode is MPOL_DEFAULT, pol is NULL. This is right. */ > > if (pol == NULL) > > return 0; > > + > > + if (pol->mode == MPOL_LOCAL) > > + return 0; > > + > > This would benefit from a comment. The one above pol NULL check is just > desperately unhelpful. I would go with the following > /* > * Default (pol==NULL) resp. local memory policies are not a > * subject of any remapping. They also do not need any special > * constructor. > */ > if (!pol || pol->mode == MPOL_LOCAL) > return 0; Thanks for the imporovement. Andrew, Could you help to take the below add-on patch for the 2/3 patch? thanks! - Feng --- >From f6023fbbc0833eebde525aa4d93fd3a7a09ddb8b Mon Sep 17 00:00:00 2001 From: Feng Tang Date: Thu, 3 Jun 2021 16:01:22 +0800 Subject: [PATCH] mm/mempolicy: refine code and comments of mpol_set_nodemask Signed-off-by: Feng Tang --- mm/mempolicy.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/mm/mempolicy.c b/mm/mempolicy.c index 32ca8fc..304b8f2 100644 --- a/mm/mempolicy.c +++ b/mm/mempolicy.c @@ -226,11 +226,12 @@ static int mpol_set_nodemask(struct mempolicy *pol, { int ret; - /* if mode is MPOL_DEFAULT, pol is NULL. This is right. */ - if (pol == NULL) - return 0; - - if (pol->mode == MPOL_LOCAL) + /* + * Default (pol==NULL) resp. local memory policies are not a + * subject of any remapping. They also do not need any special + * constructor. + */ + if (!pol || pol->mode == MPOL_LOCAL) return 0; /* Check N_MEMORY */ -- 2.7.4