linux-kernel - Re: [PATCH v2 -next] mm: memcg: make alloc_mem_cgroup_per_node

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Tue, 7 May 2024 21:31:39 +0800
From: xiujianfeng <xiujianfeng@...wei.com>
To: Michal Hocko <mhocko@...e.com>
CC: <hannes@...xchg.org>, <roman.gushchin@...ux.dev>,
	<shakeel.butt@...ux.dev>, <muchun.song@...ux.dev>,
	<akpm@...ux-foundation.org>, <cgroups@...r.kernel.org>, <linux-mm@...ck.org>,
	<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 -next] mm: memcg: make alloc_mem_cgroup_per_node_info()
 return bool



On 2024/5/7 21:10, Michal Hocko wrote:
> On Tue 07-05-24 11:08:32, Xiu Jianfeng wrote:
>> Currently alloc_mem_cgroup_per_node_info() returns 1 if failed,
>> make it return bool, false for failure and true for success.
> 
> This describes what the patch does rather than why it is doing that.
> The former is clear from the diff while the motivation for this change
> is unclear. I would propose something like:
> 
> alloc_mem_cgroup_per_node_info() returns int that doesn't map to any
> errno error code. The only existing caller doesn't really need an error
> code so change the the function to return bool (true on success) because
> this is slightly less confusing and more consistent with the other code.

Thanks, it looks much better now.

> 
>> Signed-off-by: Xiu Jianfeng <xiujianfeng@...wei.com>
> 
> With changelog clarified feel free to add
> Acked-by: Michal Hocko <mhocko@...e.com>
> 
>> ---
>>  mm/memcontrol.c | 10 +++++-----
>>  1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
>> index d11536ef59ef..69d70feb8e68 100644
>> --- a/mm/memcontrol.c
>> +++ b/mm/memcontrol.c
>> @@ -5653,13 +5653,13 @@ struct mem_cgroup *mem_cgroup_get_from_ino(unsigned long ino)
>>  }
>>  #endif
>>  
>> -static int alloc_mem_cgroup_per_node_info(struct mem_cgroup *memcg, int node)
>> +static bool alloc_mem_cgroup_per_node_info(struct mem_cgroup *memcg, int node)
>>  {
>>  	struct mem_cgroup_per_node *pn;
>>  
>>  	pn = kzalloc_node(sizeof(*pn), GFP_KERNEL, node);
>>  	if (!pn)
>> -		return 1;
>> +		return false;
>>  
>>  	pn->lruvec_stats = kzalloc_node(sizeof(struct lruvec_stats), GFP_KERNEL,
>>  					node);
>> @@ -5675,11 +5675,11 @@ static int alloc_mem_cgroup_per_node_info(struct mem_cgroup *memcg, int node)
>>  	pn->memcg = memcg;
>>  
>>  	memcg->nodeinfo[node] = pn;
>> -	return 0;
>> +	return true;
>>  fail:
>>  	kfree(pn->lruvec_stats);
>>  	kfree(pn);
>> -	return 1;
>> +	return false;
>>  }
>>  
>>  static void free_mem_cgroup_per_node_info(struct mem_cgroup *memcg, int node)
>> @@ -5751,7 +5751,7 @@ static struct mem_cgroup *mem_cgroup_alloc(struct mem_cgroup *parent)
>>  	}
>>  
>>  	for_each_node(node)
>> -		if (alloc_mem_cgroup_per_node_info(memcg, node))
>> +		if (!alloc_mem_cgroup_per_node_info(memcg, node))
>>  			goto fail;
>>  
>>  	if (memcg_wb_domain_init(memcg, GFP_KERNEL))
>> -- 
>> 2.34.1
>>
>