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@xxxxxxxxxx>
>
> With changelog clarified feel free to add
> Acked-by: Michal Hocko <mhocko@xxxxxxxx>
>
>> ---
>> 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
>>
>
