[PATCH] memory tier: Use helper function destroy_memory_type()

[PATCH] memory tier: Use helper function destroy_memory_type()

[Miaohe Lin]

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);
 	}
 	mutex_unlock(&memory_tier_lock);
 }
-- 
2.27.0

Re: [PATCH] memory tier: Use helper function destroy_memory_type()

[Huang, Ying]

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);
>  }

Re: [PATCH] memory tier: Use helper function destroy_memory_type()

[Miaohe Lin]

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.

Re: [PATCH] memory tier: Use helper function destroy_memory_type()

[Huang, Ying]

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

Re: [PATCH] memory tier: Use helper function destroy_memory_type()

[Miaohe Lin]

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
> 
> 
> .
>