| Message ID | 20230706063905.543800-1-linmiaohe@huawei.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | memory tier: rename destroy_memory_type() to put_memory_type() | expand |
Miaohe Lin <linmiaohe@huawei.com> writes: > 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 <ying.huang@intel.com> > Signed-off-by: Miaohe Lin <linmiaohe@huawei.com> LGTM, Thanks! Reviewed-by: "Huang, Ying" <ying.huang@intel.com> > --- > 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); > }
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 <ying.huang@intel.com> > Signed-off-by: Miaohe Lin <linmiaohe@huawei.com> > --- > 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 <yangx.jy@fujitsu.com> Best Regards, Xiao Yang > } > mutex_unlock(&memory_tier_lock); > }
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 <ying.huang@intel.com> >> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com> >> --- >> 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 <yangx.jy@fujitsu.com> Thanks for your review and comment.
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); }
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 <ying.huang@intel.com> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com> --- drivers/dax/kmem.c | 4 ++-- include/linux/memory-tiers.h | 4 ++-- mm/memory-tiers.c | 6 +++--- 3 files changed, 7 insertions(+), 7 deletions(-)