2023-07-06 07:11:54

by Miaohe Lin

Subject: [PATCH] memory tier: rename destroy_memory_type() to put_memory_type()

It appears that destroy_memory_type() isn't a very good name because
we usually will not free the memory_type here. So rename it to a more
appropriate name i.e. put_memory_type().

Suggested-by: Huang, Ying <[email protected]>
Signed-off-by: Miaohe Lin <[email protected]>
---
drivers/dax/kmem.c | 4 ++--
include/linux/memory-tiers.h | 4 ++--
mm/memory-tiers.c | 6 +++---
3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/dax/kmem.c b/drivers/dax/kmem.c
index 898ca9505754..c57acb73e3db 100644
--- a/drivers/dax/kmem.c
+++ b/drivers/dax/kmem.c
@@ -264,7 +264,7 @@ static int __init dax_kmem_init(void)
return rc;

error_dax_driver:
- destroy_memory_type(dax_slowmem_type);
+ put_memory_type(dax_slowmem_type);
err_dax_slowmem_type:
kfree_const(kmem_name);
return rc;
@@ -275,7 +275,7 @@ static void __exit dax_kmem_exit(void)
dax_driver_unregister(&device_dax_kmem_driver);
if (!any_hotremove_failed)
kfree_const(kmem_name);
- destroy_memory_type(dax_slowmem_type);
+ put_memory_type(dax_slowmem_type);
}

MODULE_AUTHOR("Intel Corporation");
diff --git a/include/linux/memory-tiers.h b/include/linux/memory-tiers.h
index fc9647b1b4f9..437441cdf78f 100644
--- a/include/linux/memory-tiers.h
+++ b/include/linux/memory-tiers.h
@@ -33,7 +33,7 @@ struct memory_dev_type {
#ifdef CONFIG_NUMA
extern bool numa_demotion_enabled;
struct memory_dev_type *alloc_memory_type(int adistance);
-void destroy_memory_type(struct memory_dev_type *memtype);
+void put_memory_type(struct memory_dev_type *memtype);
void init_node_memory_type(int node, struct memory_dev_type *default_type);
void clear_node_memory_type(int node, struct memory_dev_type *memtype);
#ifdef CONFIG_MIGRATION
@@ -68,7 +68,7 @@ static inline struct memory_dev_type *alloc_memory_type(int adistance)
return NULL;
}

-static inline void destroy_memory_type(struct memory_dev_type *memtype)
+static inline void put_memory_type(struct memory_dev_type *memtype)
{

}
diff --git a/mm/memory-tiers.c b/mm/memory-tiers.c
index 1719fa3bcf02..c49ab03f49b1 100644
--- a/mm/memory-tiers.c
+++ b/mm/memory-tiers.c
@@ -560,11 +560,11 @@ struct memory_dev_type *alloc_memory_type(int adistance)
}
EXPORT_SYMBOL_GPL(alloc_memory_type);

-void destroy_memory_type(struct memory_dev_type *memtype)
+void put_memory_type(struct memory_dev_type *memtype)
{
kref_put(&memtype->kref, release_memtype);
}
-EXPORT_SYMBOL_GPL(destroy_memory_type);
+EXPORT_SYMBOL_GPL(put_memory_type);

void init_node_memory_type(int node, struct memory_dev_type *memtype)
{
@@ -586,7 +586,7 @@ void clear_node_memory_type(int node, struct memory_dev_type *memtype)
*/
if (!node_memory_types[node].map_count) {
node_memory_types[node].memtype = NULL;
- destroy_memory_type(memtype);
+ put_memory_type(memtype);
}
mutex_unlock(&memory_tier_lock);
}
--
2.33.0

2023-07-06 12:31:54

by Xiao Yang

Subject: Re: [PATCH] memory tier: rename destroy_memory_type() to put_memory_type()

On 2023/7/6 14:39, Miaohe Lin wrote:
> It appears that destroy_memory_type() isn't a very good name because
> we usually will not free the memory_type here. So rename it to a more
> appropriate name i.e. put_memory_type().
>
> Suggested-by: Huang, Ying <[email protected]>
> Signed-off-by: Miaohe Lin <[email protected]>
> ---
> drivers/dax/kmem.c | 4 ++--
> include/linux/memory-tiers.h | 4 ++--
> mm/memory-tiers.c | 6 +++---
> 3 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/dax/kmem.c b/drivers/dax/kmem.c
> index 898ca9505754..c57acb73e3db 100644
> --- a/drivers/dax/kmem.c
> +++ b/drivers/dax/kmem.c
> @@ -264,7 +264,7 @@ static int __init dax_kmem_init(void)
> return rc;
>
> error_dax_driver:
> - destroy_memory_type(dax_slowmem_type);
> + put_memory_type(dax_slowmem_type);
> err_dax_slowmem_type:
> kfree_const(kmem_name);
> return rc;
> @@ -275,7 +275,7 @@ static void __exit dax_kmem_exit(void)
> dax_driver_unregister(&device_dax_kmem_driver);
> if (!any_hotremove_failed)
> kfree_const(kmem_name);
> - destroy_memory_type(dax_slowmem_type);
> + put_memory_type(dax_slowmem_type);
> }
>
> MODULE_AUTHOR("Intel Corporation");
> diff --git a/include/linux/memory-tiers.h b/include/linux/memory-tiers.h
> index fc9647b1b4f9..437441cdf78f 100644
> --- a/include/linux/memory-tiers.h
> +++ b/include/linux/memory-tiers.h
> @@ -33,7 +33,7 @@ struct memory_dev_type {
> #ifdef CONFIG_NUMA
> extern bool numa_demotion_enabled;
> struct memory_dev_type *alloc_memory_type(int adistance);
> -void destroy_memory_type(struct memory_dev_type *memtype);
> +void put_memory_type(struct memory_dev_type *memtype);
> void init_node_memory_type(int node, struct memory_dev_type *default_type);
> void clear_node_memory_type(int node, struct memory_dev_type *memtype);
> #ifdef CONFIG_MIGRATION
> @@ -68,7 +68,7 @@ static inline struct memory_dev_type *alloc_memory_type(int adistance)
> return NULL;
> }
>
> -static inline void destroy_memory_type(struct memory_dev_type *memtype)
> +static inline void put_memory_type(struct memory_dev_type *memtype)
> {
>
> }
> diff --git a/mm/memory-tiers.c b/mm/memory-tiers.c
> index 1719fa3bcf02..c49ab03f49b1 100644
> --- a/mm/memory-tiers.c
> +++ b/mm/memory-tiers.c
> @@ -560,11 +560,11 @@ struct memory_dev_type *alloc_memory_type(int adistance)
> }
> EXPORT_SYMBOL_GPL(alloc_memory_type);
>
> -void destroy_memory_type(struct memory_dev_type *memtype)
> +void put_memory_type(struct memory_dev_type *memtype)
> {
> kref_put(&memtype->kref, release_memtype);
> }
> -EXPORT_SYMBOL_GPL(destroy_memory_type);
> +EXPORT_SYMBOL_GPL(put_memory_type);
>
> void init_node_memory_type(int node, struct memory_dev_type *memtype)
> {
> @@ -586,7 +586,7 @@ void clear_node_memory_type(int node, struct memory_dev_type *memtype)
> */
> if (!node_memory_types[node].map_count) {
> node_memory_types[node].memtype = NULL;
> - destroy_memory_type(memtype);
> + put_memory_type(memtype);
Hi Maohe,

I didn't find that destroy_memory_type(memtype) is called here on
mainline kernel. Did I miss something?

Other than that, it looks good to me.
Reviewed-by: Xiao Yang <[email protected]>

Best Regards,
Xiao Yang

> }
> mutex_unlock(&memory_tier_lock);
> }

2023-07-06 13:24:22

by Miaohe Lin

Subject: Re: [PATCH] memory tier: rename destroy_memory_type() to put_memory_type()

On 2023/7/6 19:58, Xiao Yang wrote:
> On 2023/7/6 14:39, Miaohe Lin wrote:
>> It appears that destroy_memory_type() isn't a very good name because
>> we usually will not free the memory_type here. So rename it to a more
>> appropriate name i.e. put_memory_type().
>>
>> Suggested-by: Huang, Ying <[email protected]>
>> Signed-off-by: Miaohe Lin <[email protected]>
>> ---
>>   drivers/dax/kmem.c           | 4 ++--
>>   include/linux/memory-tiers.h | 4 ++--
>>   mm/memory-tiers.c            | 6 +++---
>>   3 files changed, 7 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/dax/kmem.c b/drivers/dax/kmem.c
>> index 898ca9505754..c57acb73e3db 100644
>> --- a/drivers/dax/kmem.c
>> +++ b/drivers/dax/kmem.c
>> @@ -264,7 +264,7 @@ static int __init dax_kmem_init(void)
>>       return rc;
>>     error_dax_driver:
>> -    destroy_memory_type(dax_slowmem_type);
>> +    put_memory_type(dax_slowmem_type);
>>   err_dax_slowmem_type:
>>       kfree_const(kmem_name);
>>       return rc;
>> @@ -275,7 +275,7 @@ static void __exit dax_kmem_exit(void)
>>       dax_driver_unregister(&device_dax_kmem_driver);
>>       if (!any_hotremove_failed)
>>           kfree_const(kmem_name);
>> -    destroy_memory_type(dax_slowmem_type);
>> +    put_memory_type(dax_slowmem_type);
>>   }
>>     MODULE_AUTHOR("Intel Corporation");
>> diff --git a/include/linux/memory-tiers.h b/include/linux/memory-tiers.h
>> index fc9647b1b4f9..437441cdf78f 100644
>> --- a/include/linux/memory-tiers.h
>> +++ b/include/linux/memory-tiers.h
>> @@ -33,7 +33,7 @@ struct memory_dev_type {
>>   #ifdef CONFIG_NUMA
>>   extern bool numa_demotion_enabled;
>>   struct memory_dev_type *alloc_memory_type(int adistance);
>> -void destroy_memory_type(struct memory_dev_type *memtype);
>> +void put_memory_type(struct memory_dev_type *memtype);
>>   void init_node_memory_type(int node, struct memory_dev_type *default_type);
>>   void clear_node_memory_type(int node, struct memory_dev_type *memtype);
>>   #ifdef CONFIG_MIGRATION
>> @@ -68,7 +68,7 @@ static inline struct memory_dev_type *alloc_memory_type(int adistance)
>>       return NULL;
>>   }
>>   -static inline void destroy_memory_type(struct memory_dev_type *memtype)
>> +static inline void put_memory_type(struct memory_dev_type *memtype)
>>   {
>>     }
>> diff --git a/mm/memory-tiers.c b/mm/memory-tiers.c
>> index 1719fa3bcf02..c49ab03f49b1 100644
>> --- a/mm/memory-tiers.c
>> +++ b/mm/memory-tiers.c
>> @@ -560,11 +560,11 @@ struct memory_dev_type *alloc_memory_type(int adistance)
>>   }
>>   EXPORT_SYMBOL_GPL(alloc_memory_type);
>>   -void destroy_memory_type(struct memory_dev_type *memtype)
>> +void put_memory_type(struct memory_dev_type *memtype)
>>   {
>>       kref_put(&memtype->kref, release_memtype);
>>   }
>> -EXPORT_SYMBOL_GPL(destroy_memory_type);
>> +EXPORT_SYMBOL_GPL(put_memory_type);
>>     void init_node_memory_type(int node, struct memory_dev_type *memtype)
>>   {
>> @@ -586,7 +586,7 @@ void clear_node_memory_type(int node, struct memory_dev_type *memtype)
>>        */
>>       if (!node_memory_types[node].map_count) {
>>           node_memory_types[node].memtype = NULL;
>> -        destroy_memory_type(memtype);
>> +        put_memory_type(memtype);
> Hi Maohe,
>
> I didn't find that destroy_memory_type(memtype) is called here on mainline kernel. Did I miss something?

It's on linux-next tree:
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=e5bf0402b80d80b66e9765b2c160b5199a5c7d3b

>
> Other than that, it looks good to me.
> Reviewed-by: Xiao Yang <[email protected]>

Thanks for your review and comment.