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 48F97C001DF for ; Tue, 25 Jul 2023 07:04:29 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 90BFA6B0071; Tue, 25 Jul 2023 03:04:28 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 8BBE56B0074; Tue, 25 Jul 2023 03:04:28 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 783E28E0001; Tue, 25 Jul 2023 03:04:28 -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 6A26E6B0071 for ; Tue, 25 Jul 2023 03:04:28 -0400 (EDT) Received: from smtpin14.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id 2560640D52 for ; Tue, 25 Jul 2023 07:04:28 +0000 (UTC) X-FDA: 81049245816.14.DDFE25B Received: from mga06.intel.com (mga06b.intel.com [134.134.136.31]) by imf23.hostedemail.com (Postfix) with ESMTP id 4804B140014 for ; Tue, 25 Jul 2023 07:04:25 +0000 (UTC) Authentication-Results: imf23.hostedemail.com; dkim=pass header.d=intel.com header.s=Intel header.b="Jgh/zowp"; spf=pass (imf23.hostedemail.com: domain of ying.huang@intel.com designates 134.134.136.31 as permitted sender) smtp.mailfrom=ying.huang@intel.com; dmarc=pass (policy=none) header.from=intel.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1690268666; 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=MkZL/TN9QrGmZ6+6cS//HvwqOl7bVQib2URvKnX7jHA=; b=Lk57F8IxTZx9g7BmXKsB+Kvu7pk+4rfueERS2Gur/YSAAjekgIYCqBahtYD2Pe/MA/g3eg 4XiHoFPXZoHn7lDb/yFf9RHe+6oGCKYi1b3LMZ33PYZxf1UZXJw5j4V3mOvNLSq/PJfkLQ 6aFbRicAbN6kylCnMS9VyFtIkPK9RcI= ARC-Authentication-Results: i=1; imf23.hostedemail.com; dkim=pass header.d=intel.com header.s=Intel header.b="Jgh/zowp"; spf=pass (imf23.hostedemail.com: domain of ying.huang@intel.com designates 134.134.136.31 as permitted sender) smtp.mailfrom=ying.huang@intel.com; dmarc=pass (policy=none) header.from=intel.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1690268666; a=rsa-sha256; cv=none; b=u6zxjlga12jOCqXYWxL7ZfnkhBdMOd4cStvbsPIbcKmkKX6VqEdHFZIpAlSKY41YfS7U3X ns/fGmyUT0C+HdOQ8uaYyPfk9wVUrgHHS2I9eMo7/Qh/4uzX23JwAm8zuXqrAR7cU6QPRZ FJ8AVfPBJYccR1kFlc77tNfwryFhsFg= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1690268665; x=1721804665; h=from:to:cc:subject:references:date:in-reply-to: message-id:mime-version; bh=z+bpg4f1Gpa03QOb988ceLF/hGx+yZITJmrLI5YMZc0=; b=Jgh/zowpjaPaeUEipcSBDhKuyuQurFypYcccJYaD88uIJ+5RPrA50EM8 Vf8HRwj5AcCpKMRAsWxMLmcad2UQ6ONRKUrFwzOUD+RFqnqzvGrmq1jEi uRiE0ZWnhiv8q3BTw6OfpcdNObSurJeySC2IKVgKuZHLBrFYsJr1+Q+pd TrSNP2JBrwhHCJoC2DaRk0/fbyEGRojg+l4B/TabsdIOLMexc8L9hqyQ6 Jku2KMUeeC72uIbAr7XnlGaC/qNicH85E3SErokwb6lwXr1ye6vNMhQ/k ddVFTPMjpv6RUNGec8tBNz4DvLeay27oqDRgth3ZBCYUOwFZQE5tsyXSN Q==; X-IronPort-AV: E=McAfee;i="6600,9927,10781"; a="431438958" X-IronPort-AV: E=Sophos;i="6.01,230,1684825200"; d="scan'208";a="431438958" Received: from orsmga007.jf.intel.com ([10.7.209.58]) by orsmga104.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 25 Jul 2023 00:04:23 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10781"; a="719946762" X-IronPort-AV: E=Sophos;i="6.01,230,1684825200"; d="scan'208";a="719946762" Received: from yhuang6-desk2.sh.intel.com (HELO yhuang6-desk2.ccr.corp.intel.com) ([10.238.208.55]) by orsmga007-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 25 Jul 2023 00:04:19 -0700 From: "Huang, Ying" To: Alistair Popple Cc: Andrew Morton , , , , , , "Aneesh Kumar K . V" , Wei Xu , Dan Williams , Dave Hansen , "Davidlohr Bueso" , Johannes Weiner , "Jonathan Cameron" , Michal Hocko , Yang Shi , Rafael J Wysocki Subject: Re: [PATCH RESEND 4/4] dax, kmem: calculate abstract distance with general interface References: <20230721012932.190742-1-ying.huang@intel.com> <20230721012932.190742-5-ying.huang@intel.com> <87edkwznsf.fsf@nvdebian.thelocal> Date: Tue, 25 Jul 2023 15:02:42 +0800 In-Reply-To: <87edkwznsf.fsf@nvdebian.thelocal> (Alistair Popple's message of "Tue, 25 Jul 2023 13:11:12 +1000") Message-ID: <87cz0gxylp.fsf@yhuang6-desk2.ccr.corp.intel.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.2 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=ascii X-Rspamd-Queue-Id: 4804B140014 X-Rspam-User: X-Stat-Signature: ouhycx7wz8gmph7h3yhba8c3eqnikezh X-Rspamd-Server: rspam01 X-HE-Tag: 1690268665-867847 X-HE-Meta: U2FsdGVkX18RDwybbB8UNzQU69ZEJPisLtgvaeWAYayey9imZrKKu1nJgTbNuDZYj6f0pv42UheSky3RX2kr5o/rVgcFwD06UUOYyJh+IsUGOVJDXTyrmRELJgh8Gw0BnsJzQOUhk6TXuZinywyvZUS8G/n+NermFEcfyikMe66guYrxV/Ut7mgEpO4CZYYu9K84riVLgIred6S1aXV176q15eLrpjW+pn/0or61/xf/KKkfVx1FfzdwXruYWHjClmEbzeh9dyhDHnpOBp/X6iIqZFOofBNq7+QKgl3wy8vtViD5ST4NQ/PFSpAeqUCXpLtvs9WiP8Ky5yc4YhYczl2cb80cOpEg1bsKoWWlZYW7rrLNg7JIsKZiwVSEkCIG+mc2Aae1VaUwfnU0voYlNXPvZVb/wMxkd94K00Pa7TkATHOLUmpr3qXPHSnQapRgdEuFDwjjuFHSwr9XtBEtyP0Cfw1q1L8ug6d6iows4bxE8IWvSpl+nxnhdlMkf2jS/IR6K5Vho7QR5jHmkIOAI5G/zh/fGPWm85YuIurpCMTHuEh9L+WMPFq9FqiAMfvSNtxDOWVKIsl9MHzYord7e44m89M93jp3juhBJnVQS+M1ScOs9r7FWFNvgw39vUoqQmIU0jzFQOIy1l4hF5L98GrnzxThDBBV7tjiTvXojaPuy9zR1/KdoZ5CVmShHPztuLPKYayYdAd4Mthz0kM47380Zx/QCdAQeKbOzie/qPDkDGxOUZyBXXM0uy4TjTBuNuh0i01xbDJmIx4Bt0ABRCd4TTiZA5DKARt5af1mdcTLbCS1QrqdBAapMFe8zKErHNfPkRDU0g7JrsVEG8L5+75YUUASeNJGDbJUluEhotM7cU1ldQMraLmXlQxx7TYJvajwhNX9qdaMXFbL/v7F58vhN1nYhy9EGSOc7lEbbNF/aJZNOE9iXuwNplgJDhT+Mz+AuJf5tRDaDxtSEFy /2jVen4a EbrZYMx6aR+2uTz+de5aEb2s9IibC7w3LA5fSUCeCgCfasmj02NXsjcfM3efajO612iGJ8Y54Me7EX3xUjcdcdURWcjXLsmDEVr3c373mINBejAcLyB9aM77d0eG2Bqjk60BA1ewYErNE00OFhC3BN4Wn9x0dQ3RpJ51ePuoHKMqKY0sJCbLgSOarxscrYlDrcJu3ErCAIZZyOx3eeuguTwTj9WEtFcA0MUYi7jjhHkSK1KT8mUBACQ50eTmlAPPq36N5ykhWPg51B2lqbOvyf8xSaopUrAsd1TkTBrFFABiBq4qxdQwS7KqXvEQoD/ghbf9PGjWCxFkDT/X2chwZOz8cI4w/FMFjj/fUzu3V8SWrT0z+DVgfIqFAEN9jr/fOnB+NRg5hmHM3hN0hyiuWc5Z6pjVjLaRCSBULeHs3TFRDI+fhFS8GtXvFrEtpudwJppO/bK5yXqFVaJCctn5WHreux01mTksVGtdhbyIbxMQ7GO0= 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: Alistair Popple writes: > Huang Ying writes: > >> Previously, a fixed abstract distance MEMTIER_DEFAULT_DAX_ADISTANCE is >> used for slow memory type in kmem driver. This limits the usage of >> kmem driver, for example, it cannot be used for HBM (high bandwidth >> memory). >> >> So, we use the general abstract distance calculation mechanism in kmem >> drivers to get more accurate abstract distance on systems with proper >> support. The original MEMTIER_DEFAULT_DAX_ADISTANCE is used as >> fallback only. >> >> Now, multiple memory types may be managed by kmem. These memory types >> are put into the "kmem_memory_types" list and protected by >> kmem_memory_type_lock. > > See below but I wonder if kmem_memory_types could be a common helper > rather than kdax specific? > >> Signed-off-by: "Huang, Ying" >> Cc: Aneesh Kumar K.V >> Cc: Wei Xu >> Cc: Alistair Popple >> Cc: Dan Williams >> Cc: Dave Hansen >> Cc: Davidlohr Bueso >> Cc: Johannes Weiner >> Cc: Jonathan Cameron >> Cc: Michal Hocko >> Cc: Yang Shi >> Cc: Rafael J Wysocki >> --- >> drivers/dax/kmem.c | 54 +++++++++++++++++++++++++++--------- >> include/linux/memory-tiers.h | 2 ++ >> mm/memory-tiers.c | 2 +- >> 3 files changed, 44 insertions(+), 14 deletions(-) >> >> diff --git a/drivers/dax/kmem.c b/drivers/dax/kmem.c >> index 898ca9505754..837165037231 100644 >> --- a/drivers/dax/kmem.c >> +++ b/drivers/dax/kmem.c >> @@ -49,14 +49,40 @@ struct dax_kmem_data { >> struct resource *res[]; >> }; >> >> -static struct memory_dev_type *dax_slowmem_type; >> +static DEFINE_MUTEX(kmem_memory_type_lock); >> +static LIST_HEAD(kmem_memory_types); >> + >> +static struct memory_dev_type *kmem_find_alloc_memorty_type(int adist) >> +{ >> + bool found = false; >> + struct memory_dev_type *mtype; >> + >> + mutex_lock(&kmem_memory_type_lock); >> + list_for_each_entry(mtype, &kmem_memory_types, list) { >> + if (mtype->adistance == adist) { >> + found = true; >> + break; >> + } >> + } >> + if (!found) { >> + mtype = alloc_memory_type(adist); >> + if (!IS_ERR(mtype)) >> + list_add(&mtype->list, &kmem_memory_types); >> + } >> + mutex_unlock(&kmem_memory_type_lock); >> + >> + return mtype; >> +} >> + >> static int dev_dax_kmem_probe(struct dev_dax *dev_dax) >> { >> struct device *dev = &dev_dax->dev; >> unsigned long total_len = 0; >> struct dax_kmem_data *data; >> + struct memory_dev_type *mtype; >> int i, rc, mapped = 0; >> int numa_node; >> + int adist = MEMTIER_DEFAULT_DAX_ADISTANCE; >> >> /* >> * Ensure good NUMA information for the persistent memory. >> @@ -71,6 +97,11 @@ static int dev_dax_kmem_probe(struct dev_dax *dev_dax) >> return -EINVAL; >> } >> >> + mt_calc_adistance(numa_node, &adist); >> + mtype = kmem_find_alloc_memorty_type(adist); >> + if (IS_ERR(mtype)) >> + return PTR_ERR(mtype); >> + > > I wrote my own quick and dirty module to test this and wrote basically > the same code sequence. > > I notice your using a list of memory types here though. I think it would > be nice to have a common helper that other users could call to do the > mt_calc_adistance() / kmem_find_alloc_memory_type() / > init_node_memory_type() sequence and cleanup as my naive approach would > result in a new memory_dev_type per device even though adist might be > the same. A common helper would make it easy to de-dup those. If it's useful, we can move kmem_find_alloc_memory_type() to memory-tier.c after some revision. But I tend to move it after we have the second user. What do you think about that? -- Best Regards, Huang, Ying >> for (i = 0; i < dev_dax->nr_range; i++) { >> struct range range; >> >> @@ -88,7 +119,7 @@ static int dev_dax_kmem_probe(struct dev_dax *dev_dax) >> return -EINVAL; >> } >> >> - init_node_memory_type(numa_node, dax_slowmem_type); >> + init_node_memory_type(numa_node, mtype); >> >> rc = -ENOMEM; >> data = kzalloc(struct_size(data, res, dev_dax->nr_range), GFP_KERNEL); >> @@ -167,7 +198,7 @@ static int dev_dax_kmem_probe(struct dev_dax *dev_dax) >> err_res_name: >> kfree(data); >> err_dax_kmem_data: >> - clear_node_memory_type(numa_node, dax_slowmem_type); >> + clear_node_memory_type(numa_node, mtype); >> return rc; >> } >> >> @@ -219,7 +250,7 @@ static void dev_dax_kmem_remove(struct dev_dax *dev_dax) >> * for that. This implies this reference will be around >> * till next reboot. >> */ >> - clear_node_memory_type(node, dax_slowmem_type); >> + clear_node_memory_type(node, NULL); >> } >> } >> #else >> @@ -251,12 +282,6 @@ static int __init dax_kmem_init(void) >> if (!kmem_name) >> return -ENOMEM; >> >> - dax_slowmem_type = alloc_memory_type(MEMTIER_DEFAULT_DAX_ADISTANCE); >> - if (IS_ERR(dax_slowmem_type)) { >> - rc = PTR_ERR(dax_slowmem_type); >> - goto err_dax_slowmem_type; >> - } >> - >> rc = dax_driver_register(&device_dax_kmem_driver); >> if (rc) >> goto error_dax_driver; >> @@ -264,18 +289,21 @@ static int __init dax_kmem_init(void) >> return rc; >> >> error_dax_driver: >> - destroy_memory_type(dax_slowmem_type); >> -err_dax_slowmem_type: >> kfree_const(kmem_name); >> return rc; >> } >> >> static void __exit dax_kmem_exit(void) >> { >> + struct memory_dev_type *mtype, *mtn; >> + >> dax_driver_unregister(&device_dax_kmem_driver); >> if (!any_hotremove_failed) >> kfree_const(kmem_name); >> - destroy_memory_type(dax_slowmem_type); >> + list_for_each_entry_safe(mtype, mtn, &kmem_memory_types, list) { >> + list_del(&mtype->list); >> + destroy_memory_type(mtype); >> + } >> } >> >> MODULE_AUTHOR("Intel Corporation"); >> diff --git a/include/linux/memory-tiers.h b/include/linux/memory-tiers.h >> index 9377239c8d34..aca22220cb5c 100644 >> --- a/include/linux/memory-tiers.h >> +++ b/include/linux/memory-tiers.h >> @@ -24,6 +24,8 @@ struct memory_tier; >> struct memory_dev_type { >> /* list of memory types that are part of same tier as this type */ >> struct list_head tier_sibiling; >> + /* list of memory types that are managed by one driver */ >> + struct list_head list; >> /* abstract distance for this specific memory type */ >> int adistance; >> /* Nodes of same abstract distance */ >> diff --git a/mm/memory-tiers.c b/mm/memory-tiers.c >> index 9a734ef2edfb..38005c60fa2d 100644 >> --- a/mm/memory-tiers.c >> +++ b/mm/memory-tiers.c >> @@ -581,7 +581,7 @@ EXPORT_SYMBOL_GPL(init_node_memory_type); >> void clear_node_memory_type(int node, struct memory_dev_type *memtype) >> { >> mutex_lock(&memory_tier_lock); >> - if (node_memory_types[node].memtype == memtype) >> + if (node_memory_types[node].memtype == memtype || !memtype) >> node_memory_types[node].map_count--; >> /* >> * If we umapped all the attached devices to this node,