Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.35 as permitted sender) client-ip=23.128.96.35;
Message-ID: <a9e80092-ade3-cbbd-ea52-bf1d591897ea@intel.com>
Date:   Thu, 14 Sep 2023 10:31:42 -0700
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Betterbird/102.13.0
Subject: Re: [PATCH -V3 4/4] dax, kmem: calculate abstract distance with
 general interface
To:     Huang Ying <ying.huang@intel.com>,
        Andrew Morton <akpm@linux-foundation.org>
CC:     <linux-mm@kvack.org>, <linux-kernel@vger.kernel.org>,
        Bharata B Rao <bharata@amd.com>,
        "Aneesh Kumar K . V" <aneesh.kumar@linux.ibm.com>,
        Wei Xu <weixugc@google.com>,
        Alistair Popple <apopple@nvidia.com>,
        Dan Williams <dan.j.williams@intel.com>,
        Dave Hansen <dave.hansen@intel.com>,
        "Davidlohr Bueso" <dave@stgolabs.net>,
        Johannes Weiner <hannes@cmpxchg.org>,
        "Jonathan Cameron" <Jonathan.Cameron@huawei.com>,
        Michal Hocko <mhocko@kernel.org>,
        Yang Shi <shy828301@gmail.com>,
        Rafael J Wysocki <rafael.j.wysocki@intel.com>
References: <20230912082101.342002-1-ying.huang@intel.com>
 <20230912082101.342002-5-ying.huang@intel.com>
Content-Language: en-US
From:   Dave Jiang <dave.jiang@intel.com>
In-Reply-To: <20230912082101.342002-5-ying.huang@intel.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?NUpiaXVJWTZVZ0NKVC9xOVZEMGlWVUlPSURhYStRMENKNWo1d1ZjNzZsaDdG?=
 =?utf-8?B?aW5NUVErd3AxYjRLUklRbG9IQUc4citheUtZN1hyWng0dzNRSG53YmRieVEy?=
 =?utf-8?B?SHJ5N2Eyc0t2UXVHc3VRR2RSb3lYM1dRUTRXSDNTR3I3ampUWjQ3ZE41eDZS?=
 =?utf-8?B?OW96MXM5SkQ3Y1VPYzFvcUh3Uk10UEp2OFVhdjIwcDlxeXpPZ0hGZGFJeFpz?=
 =?utf-8?B?Nkk1bS82aGZqRE9XSkhzVnBuSXlwbkMyakw1MlJPYVlCcWk1b0lMUTVYQXpV?=
 =?utf-8?B?bVFHWGlpcW9IK0tnaEpibCt6TUh2bHlLTHAyamEzMVlQSlVNZnJxN3dpVnIz?=
 =?utf-8?B?YzVtb1FVSTlBOEt1ZHBGYTRPM1lNOWNkUzl5eWI5dzVYN2NnNGRlVGNFWUE1?=
 =?utf-8?B?ei9hVm5zM2tkayt4bCtjWmY1aEhGeFR5UWdvR2orUXBJS1orb3ZvSEFlZUww?=
 =?utf-8?B?SEN5TnVMSllhbmI1eWd4TFBPVlNIY3o1Y0ZsSU1tODgrMUo2Q3dxWGNFa3N0?=
 =?utf-8?B?QTBPeHQxRmJhYmE3UjB5eFgvSDRYZ1YwUnNTK2o5eUF6R0pmb3JIV3pzYzh3?=
 =?utf-8?B?MlVSK1NSSjQ5eGpVUW40WFNiOHlNNXJ6VjlFaEpCUkNGUStsdTBrRmFDWmxz?=
 =?utf-8?B?ZW51R0hlb3lnZlAvdnNsbHdTeFBFekNjd3JhamtzYnZzLzdtY0EwZmh3cHNK?=
 =?utf-8?B?OEd4Q3BUZHRFbEpiYjZxbXBWMmdhWXlUSU80WjROaUY3Sk96Y0h5OXNJSDFG?=
 =?utf-8?B?ZGg5OUwxM2laWkdCU3dDTWRkTDdHN1hFbDVkSHRuU2ZJOFMzNkdDWmlpeUc5?=
 =?utf-8?B?Ni9HWkdZSzNMTHcrQkhNbnBZOTVMdzk5WkJpU09KRlZka2l3RGhIRDRzaDFx?=
 =?utf-8?B?dHErWHdBdGw5eitJVElEWTB2Ni9JN2NsbEtZV2EvazV5dDkrR1YzQTlXYnJ4?=
 =?utf-8?B?UmF5MnRZY3NDeVVsc1pFZXlHbjh0N2RSWk96TjRab0tMV0JCa1o4RGVjdStX?=
 =?utf-8?B?YndJSnh4MTVpTFVJSmd5R3ZlM2Jpak9FQnhRZ3pKSEdBK3JOREdPbmhGUmts?=
 =?utf-8?B?TFk0NlV5SFYzMmd2cFRnK1c4Ky9PMHBBK1VaRkFiTXpIQktBSHdURkJnRDJJ?=
 =?utf-8?B?RW5YTVZmZ3FGNEg1dEovQjNlTFB5alN6WVd4QzZpYk5QVFgyZ3dGdHViRmMr?=
 =?utf-8?B?VTBCKzBvNkV1bHFDWUV1SzBveVJOcklqQVZBTXprMDBBWlRvblNqSStLVnlL?=
 =?utf-8?B?dXUzeE9BbERpM1gyaXF6ajJiTnhLOCt3cnNkYUpRLzk3VmQ0MWlZK005Z25S?=
 =?utf-8?B?OXNhQm5NTGNObGY4eHVEY21nWVlpNm1pTVBrUnZ6c3UycUVrNmw2N1Q4WDRG?=
 =?utf-8?B?K09TK1ZiVjAwMU1FNEhwOEZjVU9nNzhzNU1pYnlGRjRnOTJML0p5WTNNYkVE?=
 =?utf-8?B?d3czZlBBczVkT0gvZ0RHQnBkWkxEWlBOcGR3ZUQ2UExiakRpQmRBbno0NzBx?=
 =?utf-8?B?Z3hEVEZlUGZxMFQ0VFlCWVhlVGJkMkloRzVLV0dvVnVKMHdPcDFNb0R3L1VQ?=
 =?utf-8?B?cHlPZnVRTThSeGl0Q2N5UUlDU0wzRmRKOG5Ma09heDJ5RHNEazVDYU1ZYW1H?=
 =?utf-8?B?djBLTCtRcHIzZGU5L2RyVDcwa2xkOXkvbnhuZmNkWjBNNzRFRlpHSk9Vc3JW?=
 =?utf-8?B?NE5DRGNDcUREcWpzV2IxbSttN1VYNUNxS2tVKytJN045alFvdVFUM29HQk92?=
 =?utf-8?B?SnF2OU84dWd0eU50QkVHN0JOYWRkb0V0WUxVay9FdnlTWkkzNlRmZzhuYzMx?=
 =?utf-8?B?V0Jra3hzWnU3U2NpUG9vYXEzSGhhVlpURGVoQUZPdG5DM0FURGhTTU9mYmtu?=
 =?utf-8?B?U3FsdTJGODAxd1d0WWg3M1dCbDF0L3dHOVpTTFllakphQlNjL3ZVd2JsSjVD?=
 =?utf-8?B?ODMvU09IWEpuK1FZMVluTGxmeFVmMFA5a0NuRnNFK2RPdklGRmkvUnIzNUxq?=
 =?utf-8?B?L0QxRzhmME02QUtMbDRVbGJsdkVMODNpOHUyZWtCbU5ZYVJxT05xOEVJc05B?=
 =?utf-8?B?dWJCWi9Oc1p2TTV5WUoyTmJkTUtobkc2MDUyZjJnZGlSR0RrZUlvemxPU3li?=
 =?utf-8?B?alJrUDU4N0lLVmZDS0VlQUdXZWx0VnVpZnA1T21GOVo0em94SVBYNkdlUWdB?=
 =?utf-8?B?Znc9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 392843a1-8f5a-4971-26c4-08dbb5487871
X-MS-Exchange-CrossTenant-AuthSource: PH7PR11MB5984.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 14 Sep 2023 17:31:46.7623
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: ASmN57I54iqk9GIJcbn8lQGDoRg/ZuEdtbLwCZATtDKS4GMnRFBZB2NW8f5K5HucC4jIjlsoCxkgmkBrGbJGbg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR11MB6476
X-OriginatorOrg: intel.com
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (groat.vger.email [0.0.0.0]); Thu, 14 Sep 2023 10:32:26 -0700 (PDT)
X-Spam-Status: No, score=-2.3 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED,
	DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS
	autolearn=unavailable autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on groat.vger.email


On 9/12/23 01:21, Huang Ying wrote:
> 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.
> 
> Signed-off-by: "Huang, Ying" <ying.huang@intel.com>
> Tested-by: Bharata B Rao <bharata@amd.com>
> Cc: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> Cc: Wei Xu <weixugc@google.com>
> Cc: Alistair Popple <apopple@nvidia.com>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Cc: Dave Hansen <dave.hansen@intel.com>
> Cc: Davidlohr Bueso <dave@stgolabs.net>
> Cc: Johannes Weiner <hannes@cmpxchg.org>
> Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Cc: Michal Hocko <mhocko@kernel.org>
> Cc: Yang Shi <shy828301@gmail.com>
> Cc: Dave Jiang <dave.jiang@intel.com>
> Cc: Rafael J Wysocki <rafael.j.wysocki@intel.com>

Reviewed-by: Dave Jiang <dave.jiang@intel.com>

> ---
>  drivers/dax/kmem.c           | 62 ++++++++++++++++++++++++++++--------
>  include/linux/memory-tiers.h |  2 ++
>  mm/memory-tiers.c            |  2 +-
>  3 files changed, 52 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/dax/kmem.c b/drivers/dax/kmem.c
> index c57acb73e3db..369c698b7706 100644
> --- a/drivers/dax/kmem.c
> +++ b/drivers/dax/kmem.c
> @@ -49,14 +49,52 @@ 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_memory_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 void kmem_put_memory_types(void)
> +{
> +	struct memory_dev_type *mtype, *mtn;
> +
> +	mutex_lock(&kmem_memory_type_lock);
> +	list_for_each_entry_safe(mtype, mtn, &kmem_memory_types, list) {
> +		list_del(&mtype->list);
> +		put_memory_type(mtype);
> +	}
> +	mutex_unlock(&kmem_memory_type_lock);
> +}
> +
>  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 +109,11 @@ static int dev_dax_kmem_probe(struct dev_dax *dev_dax)
>  		return -EINVAL;
>  	}
>  
> +	mt_calc_adistance(numa_node, &adist);
> +	mtype = kmem_find_alloc_memory_type(adist);
> +	if (IS_ERR(mtype))
> +		return PTR_ERR(mtype);
> +
>  	for (i = 0; i < dev_dax->nr_range; i++) {
>  		struct range range;
>  
> @@ -88,7 +131,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 +210,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 +262,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 +294,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,8 +301,7 @@ static int __init dax_kmem_init(void)
>  	return rc;
>  
>  error_dax_driver:
> -	put_memory_type(dax_slowmem_type);
> -err_dax_slowmem_type:
> +	kmem_put_memory_types();
>  	kfree_const(kmem_name);
>  	return rc;
>  }
> @@ -275,7 +311,7 @@ static void __exit dax_kmem_exit(void)
>  	dax_driver_unregister(&device_dax_kmem_driver);
>  	if (!any_hotremove_failed)
>  		kfree_const(kmem_name);
> -	put_memory_type(dax_slowmem_type);
> +	kmem_put_memory_types();
>  }
>  
>  MODULE_AUTHOR("Intel Corporation");
> diff --git a/include/linux/memory-tiers.h b/include/linux/memory-tiers.h
> index 9d27ca3b143e..ab6651402d7e 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 fa1a8b418f9a..ca68ef17554b 100644
> --- a/mm/memory-tiers.c
> +++ b/mm/memory-tiers.c
> @@ -586,7 +586,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,