| Message ID | 20230626121053.1916447-1-linmiaohe@huawei.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | memory tier: Use helper function destroy_memory_type() | expand |
Miaohe Lin <linmiaohe@huawei.com> writes: > Use helper function destroy_memory_type() to release memtype instead > of open code it to help improve code readability a bit. No functional > change intended. > > Signed-off-by: Miaohe Lin <linmiaohe@huawei.com> > --- > mm/memory-tiers.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/mm/memory-tiers.c b/mm/memory-tiers.c > index e593e56e530b..0b8b76078c12 100644 > --- a/mm/memory-tiers.c > +++ b/mm/memory-tiers.c > @@ -587,7 +587,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; > - kref_put(&memtype->kref, release_memtype); > + destroy_memory_type(memtype); Not need to be changed in this patch. It appears that destroy_memory_type() isn't a very good name, because we usually will not free the memory_type here. Rename it to "put_memory_type()"? Best Regards, Huang, Ying > } > mutex_unlock(&memory_tier_lock); > }
On 2023/7/3 10:28, Huang, Ying wrote: > Miaohe Lin <linmiaohe@huawei.com> writes: > >> Use helper function destroy_memory_type() to release memtype instead >> of open code it to help improve code readability a bit. No functional >> change intended. >> >> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com> >> --- >> mm/memory-tiers.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/mm/memory-tiers.c b/mm/memory-tiers.c >> index e593e56e530b..0b8b76078c12 100644 >> --- a/mm/memory-tiers.c >> +++ b/mm/memory-tiers.c >> @@ -587,7 +587,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; >> - kref_put(&memtype->kref, release_memtype); >> + destroy_memory_type(memtype); > > Not need to be changed in this patch. It appears that > destroy_memory_type() isn't a very good name, because we usually will > not free the memory_type here. Rename it to "put_memory_type()"? Do you mean rename destroy_memory_type to put_memory_type in a separate patch? This sounds reasonable to me. But destroy_memory_type is a EXPORT_SYMBOL_GPL symbol, is it fine to do the rename work? Thanks.
Miaohe Lin <linmiaohe@huawei.com> writes: > On 2023/7/3 10:28, Huang, Ying wrote: >> Miaohe Lin <linmiaohe@huawei.com> writes: >> >>> Use helper function destroy_memory_type() to release memtype instead >>> of open code it to help improve code readability a bit. No functional >>> change intended. >>> >>> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com> >>> --- >>> mm/memory-tiers.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/mm/memory-tiers.c b/mm/memory-tiers.c >>> index e593e56e530b..0b8b76078c12 100644 >>> --- a/mm/memory-tiers.c >>> +++ b/mm/memory-tiers.c >>> @@ -587,7 +587,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; >>> - kref_put(&memtype->kref, release_memtype); >>> + destroy_memory_type(memtype); >> >> Not need to be changed in this patch. It appears that >> destroy_memory_type() isn't a very good name, because we usually will >> not free the memory_type here. Rename it to "put_memory_type()"? > > Do you mean rename destroy_memory_type to put_memory_type in a > separate patch? Yes. > This sounds reasonable to me. But destroy_memory_type is a > EXPORT_SYMBOL_GPL symbol, is it fine to do the rename work? I think so. This isn't a kernel ABI. And not many people use it now. Best Regards, Huang, Ying
On 2023/7/3 11:07, Huang, Ying wrote: > Miaohe Lin <linmiaohe@huawei.com> writes: > >> On 2023/7/3 10:28, Huang, Ying wrote: >>> Miaohe Lin <linmiaohe@huawei.com> writes: >>> >>>> Use helper function destroy_memory_type() to release memtype instead >>>> of open code it to help improve code readability a bit. No functional >>>> change intended. >>>> >>>> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com> >>>> --- >>>> mm/memory-tiers.c | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/mm/memory-tiers.c b/mm/memory-tiers.c >>>> index e593e56e530b..0b8b76078c12 100644 >>>> --- a/mm/memory-tiers.c >>>> +++ b/mm/memory-tiers.c >>>> @@ -587,7 +587,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; >>>> - kref_put(&memtype->kref, release_memtype); >>>> + destroy_memory_type(memtype); >>> >>> Not need to be changed in this patch. It appears that >>> destroy_memory_type() isn't a very good name, because we usually will >>> not free the memory_type here. Rename it to "put_memory_type()"? >> >> Do you mean rename destroy_memory_type to put_memory_type in a >> separate patch? > > Yes. > >> This sounds reasonable to me. But destroy_memory_type is a >> EXPORT_SYMBOL_GPL symbol, is it fine to do the rename work? > > I think so. This isn't a kernel ABI. And not many people use it now. Will do it. Thanks for your advice. > > Best Regards, > Huang, Ying > > > . >
diff --git a/mm/memory-tiers.c b/mm/memory-tiers.c index e593e56e530b..0b8b76078c12 100644 --- a/mm/memory-tiers.c +++ b/mm/memory-tiers.c @@ -587,7 +587,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; - kref_put(&memtype->kref, release_memtype); + destroy_memory_type(memtype); } mutex_unlock(&memory_tier_lock); }
Use helper function destroy_memory_type() to release memtype instead of open code it to help improve code readability a bit. No functional change intended. Signed-off-by: Miaohe Lin <linmiaohe@huawei.com> --- mm/memory-tiers.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)