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 EC23FCCA47C for ; Tue, 14 Jun 2022 08:41:07 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 7ED668D0233; Tue, 14 Jun 2022 04:41:07 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 79CFB8D0229; Tue, 14 Jun 2022 04:41:07 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 664CF8D0233; Tue, 14 Jun 2022 04:41:07 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id 5625B8D0229 for ; Tue, 14 Jun 2022 04:41:07 -0400 (EDT) Received: from smtpin23.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay13.hostedemail.com (Postfix) with ESMTP id 2511861339 for ; Tue, 14 Jun 2022 08:41:07 +0000 (UTC) X-FDA: 79576196574.23.6A61883 Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by imf07.hostedemail.com (Postfix) with ESMTP id 33B5E40084 for ; Tue, 14 Jun 2022 08:41:05 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1655196066; x=1686732066; h=message-id:subject:from:to:cc:date:in-reply-to: references:mime-version:content-transfer-encoding; bh=fd/lbLyA5AI0yS74VK2NbXXwkzPBzN8ClOqlIpUds/I=; b=ZA7yX4LHGIteXo2F7S7KN/ajURphDxHPB/Tyh3Jb2L0KRm3XlTL3VYCE WVbtPfedqBAMciVD3GuBDCY2/8ngoNB8ZY2SlgH12rr72ipccvrw+9Fw8 t6GB01BABV1mSesD18PKZp7w/DgQI36KFu6agVyCt+CP7KZAqDc/EznqS sL6w8IBfnzYuF2U16FShf8FUuFmda/ddzXj60w4sHMcQvsrKMBkjUd+D4 9EmocLNAPmmI/guy4eK2UVi1xlQ6xW2AMH+mAs/IK0hG5cLD9VSSwhabf AtdI6YiZpe9PI1cEcp0rvl5jaj3G0OqGDF2/wXQr2GSGBuUYF98c40qKC g==; X-IronPort-AV: E=McAfee;i="6400,9594,10377"; a="303961020" X-IronPort-AV: E=Sophos;i="5.91,299,1647327600"; d="scan'208";a="303961020" Received: from fmsmga008.fm.intel.com ([10.253.24.58]) by fmsmga101.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 14 Jun 2022 01:41:03 -0700 X-IronPort-AV: E=Sophos;i="5.91,299,1647327600"; d="scan'208";a="640222026" Received: from unknown (HELO yhuang6-mobl1.ccr.corp.intel.com) ([10.254.215.153]) by fmsmga008-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 14 Jun 2022 01:40:58 -0700 Message-ID: <245802940528e11c879d4b54a9c25ef8497a9547.camel@intel.com> Subject: Re: [PATCH v6 03/13] mm/demotion: Return error on write to numa_demotion sysfs From: Ying Huang To: Aneesh Kumar K V , linux-mm@kvack.org, akpm@linux-foundation.org Cc: Wei Xu , Greg Thelen , Yang Shi , Davidlohr Bueso , Tim C Chen , Brice Goglin , Michal Hocko , Linux Kernel Mailing List , Hesham Almatary , Dave Hansen , Jonathan Cameron , Alistair Popple , Dan Williams , Feng Tang , Jagdish Gediya , Baolin Wang , David Rientjes Date: Tue, 14 Jun 2022 16:40:56 +0800 In-Reply-To: References: <20220610135229.182859-1-aneesh.kumar@linux.ibm.com> <20220610135229.182859-4-aneesh.kumar@linux.ibm.com> <7ed1f9f544937b5c82ab380a4977e5ae22a98c43.camel@intel.com> <9da3c6ef-ba0d-6229-2188-0956222b04f1@linux.ibm.com> <33b42a802a07721c639db99ed208ed43f743bb37.camel@intel.com> Content-Type: text/plain; charset="UTF-8" User-Agent: Evolution 3.38.3-1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1655196066; a=rsa-sha256; cv=none; b=uxcgsXB5D5lnmnNgswRjOfbS1m6O1BhQRYcepdhLq2o8D2KTM3ClXfLNKUStzXy0WuUWGf WGiS+u+Yl9fN8G08QaU1acEHZFxZR7F92yICzfSxwSzOaoy3DDrC5S5ckEC18WM1VE4j0b SU1dMIhBzCCo5iVtTPnL/akOCCQXtRM= ARC-Authentication-Results: i=1; imf07.hostedemail.com; dkim=pass header.d=intel.com header.s=Intel header.b=ZA7yX4LH; dmarc=pass (policy=none) header.from=intel.com; spf=none (imf07.hostedemail.com: domain of ying.huang@intel.com has no SPF policy when checking 192.55.52.88) smtp.mailfrom=ying.huang@intel.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1655196066; 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=23eb+cD62d3+IkYSzv5bejd9MmQk/edcHZs72Qe+Sak=; b=NMxz4248QCR2UhlbacscbpDkiDFHiIKOK2OiR59Y/np04WyNbdoF2n58O0bVn0S9L9HxcS hWVTFVjMqHo56328azG7v0PV+xNe2kWqJ7GaRpFgu6MLre5Js1luiJ4I1iXt/34kOk/wHJ RKdOR2co6m7s4EKLpEGCBGOQwP1AVzM= X-Rspamd-Queue-Id: 33B5E40084 X-Rspam-User: Authentication-Results: imf07.hostedemail.com; dkim=pass header.d=intel.com header.s=Intel header.b=ZA7yX4LH; dmarc=pass (policy=none) header.from=intel.com; spf=none (imf07.hostedemail.com: domain of ying.huang@intel.com has no SPF policy when checking 192.55.52.88) smtp.mailfrom=ying.huang@intel.com X-Rspamd-Server: rspam06 X-Stat-Signature: ckesoht5ac57j41hyddduwbu3piy7wft X-HE-Tag: 1655196065-785178 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 Mon, 2022-06-13 at 11:18 +0530, Aneesh Kumar K V wrote: > On 6/13/22 11:03 AM, Ying Huang wrote: > > On Mon, 2022-06-13 at 09:05 +0530, Aneesh Kumar K V wrote: > > > On 6/13/22 8:56 AM, Ying Huang wrote: > > > > On Fri, 2022-06-10 at 19:22 +0530, Aneesh Kumar K.V wrote: > > > > > With CONFIG_MIGRATION disabled return EINVAL on write. > > > > > > > > > > Signed-off-by: Aneesh Kumar K.V > > > > > --- > > > > >    mm/memory-tiers.c | 3 +++ > > > > >    1 file changed, 3 insertions(+) > > > > > > > > > > diff --git a/mm/memory-tiers.c b/mm/memory-tiers.c > > > > > index 9c6b40d7e0bf..c3123a457d90 100644 > > > > > --- a/mm/memory-tiers.c > > > > > +++ b/mm/memory-tiers.c > > > > > @@ -105,6 +105,9 @@ static ssize_t numa_demotion_enabled_store(struct kobject *kobj, > > > > >    { > > > > >     ssize_t ret; > > > > >     > > > > > > > > > > > > > > > > > > > > + if (!IS_ENABLED(CONFIG_MIGRATION)) > > > > > + return -EINVAL; > > > > > + > > > > > > > > How about enclose numa_demotion_enabled_xxx related code with CONFIG_MIGRATION? > > > > > > > > > > IIUC there is a desire to use IS_ENABLED() in the kernel instead of > > > #ifdef since that helps in more compile time checks. Because there are > > > no dead codes during compile now with IS_ENABLED(). > > > > IS_ENABLED() is used to reduce usage of "#ifdef" in ".c" file, > > especially inside a function. We have good build test coverage with > > 0Day now. > > > > To avoid code size inflate, it's better to use #ifdef CONFIG_MIGRATION. > > > > For a diff like below I am finding IS_ENABLED better. > > size memory-tiers.o.isenabled memory-tiers.o >     text data bss dec hex filename >     4776 989 5 5770 168a memory-tiers.o.isenabled >     5257 990 5 6252 186c memory-tiers.o > > > modified mm/memory-tiers.c > @@ -710,12 +710,11 @@ static int __meminit > migrate_on_reclaim_callback(struct notifier_block *self, > >   static void __init migrate_on_reclaim_init(void) >   { > - > - if (IS_ENABLED(CONFIG_MIGRATION)) { > +#ifdef CONFIG_MIGRATION >    node_demotion = kcalloc(MAX_NUMNODES, sizeof(struct demotion_nodes), >    GFP_KERNEL); >    WARN_ON(!node_demotion); > - } > +#endif >    hotplug_memory_notifier(migrate_on_reclaim_callback, 100); >   } > > @@ -844,14 +843,19 @@ static ssize_t numa_demotion_enabled_show(struct > kobject *kobj, >    numa_demotion_enabled ? "true" : "false"); >   } > > +#ifdef CONFIG_MIGRATION >   static ssize_t numa_demotion_enabled_store(struct kobject *kobj, >    struct kobj_attribute *attr, >    const char *buf, size_t count) >   { > - ssize_t ret; > - > - if (!IS_ENABLED(CONFIG_MIGRATION)) > - return -EINVAL; > + return -EINVAL; > +} > +#else > +static ssize_t numa_demotion_enabled_store(struct kobject *kobj, > + struct kobj_attribute *attr, > + const char *buf, size_t count) > +{ > + ssize_t ret; > >    ret = kstrtobool(buf, &numa_demotion_enabled); >    if (ret) > @@ -859,6 +863,7 @@ static ssize_t numa_demotion_enabled_store(struct > kobject *kobj, > >    return count; >   } > +#endif > >   static struct kobj_attribute numa_demotion_enabled_attr = >    __ATTR(demotion_enabled, 0644, numa_demotion_enabled_show, > > I also find that #ifdef config not easier to the eyes. If there is a > large code that we can end up #ifdef out, then it might be worth it. > IIUC, we might want to keep the establish_migration target to find > top_tier rank and lower_tier mask. Once we do that only thing that we > could comment out is the node_demotion sysfs creation and I was > considering to keep that even if migration is disabled with a write to > the file returning EINVAL. I could switch that if you strongly feel that > we should hide node_demotion sysfs file. Per my understanding, we can enclose most code about demoting/promoting inside CONFIG_MIGRATION, including numa/demotion_enabled sysfs interface. In this way, the code size can be reduced. Best Regards, Huang, Ying