[bug report] memcg: replace memcg ID idr with xarray

[bug report] memcg: replace memcg ID idr with xarray

[Dan Carpenter]

Hello Shakeel Butt,

Commit ad80273d43b7 ("memcg: replace memcg ID idr with xarray") from
Aug 9, 2024 (linux-next), leads to the following Smatch static
checker warning:

	mm/memcontrol.c:3592 mem_cgroup_alloc()
	warn: passing zero to 'ERR_PTR'

mm/memcontrol.c
    3525 static struct mem_cgroup *mem_cgroup_alloc(struct mem_cgroup *parent)
    3526 {
    3527         struct memcg_vmstats_percpu *statc, *pstatc;
    3528         struct mem_cgroup *memcg;
    3529         int node, cpu;
    3530         int __maybe_unused i;
    3531         long error = -ENOMEM;
    3532 
    3533         memcg = kzalloc(struct_size(memcg, nodeinfo, nr_node_ids), GFP_KERNEL);
    3534         if (!memcg)
    3535                 return ERR_PTR(error);
    3536 
    3537         error = xa_alloc(&mem_cgroup_ids, &memcg->id.id, NULL,
    3538                          XA_LIMIT(1, MEM_CGROUP_ID_MAX), GFP_KERNEL);
    3539         if (error)
    3540                 goto fail;
    3541 
    3542         memcg->vmstats = kzalloc(sizeof(struct memcg_vmstats),
    3543                                  GFP_KERNEL_ACCOUNT);
    3544         if (!memcg->vmstats)
    3545                 goto fail;

These error paths should all return -ENOMEM.

    3546 
    3547         memcg->vmstats_percpu = alloc_percpu_gfp(struct memcg_vmstats_percpu,
    3548                                                  GFP_KERNEL_ACCOUNT);
    3549         if (!memcg->vmstats_percpu)
    3550                 goto fail;
                         ^^^^^^^^^

    3551 
    3552         for_each_possible_cpu(cpu) {
    3553                 if (parent)
    3554                         pstatc = per_cpu_ptr(parent->vmstats_percpu, cpu);
    3555                 statc = per_cpu_ptr(memcg->vmstats_percpu, cpu);
    3556                 statc->parent = parent ? pstatc : NULL;
    3557                 statc->vmstats = memcg->vmstats;
    3558         }
    3559 
    3560         for_each_node(node)
    3561                 if (!alloc_mem_cgroup_per_node_info(memcg, node))
    3562                         goto fail;
                                 ^^^^^^^^^^
    3563 
    3564         if (memcg_wb_domain_init(memcg, GFP_KERNEL))
    3565                 goto fail;
                         ^^^^^^^^^^
It might be nice to propogate the error code from memcg_wb_domain_init()

    3566 
    3567         INIT_WORK(&memcg->high_work, high_work_func);
    3568         vmpressure_init(&memcg->vmpressure);
    3569         INIT_LIST_HEAD(&memcg->memory_peaks);
    3570         INIT_LIST_HEAD(&memcg->swap_peaks);
    3571         spin_lock_init(&memcg->peaks_lock);
    3572         memcg->socket_pressure = jiffies;
    3573         memcg1_memcg_init(memcg);
    3574         memcg->kmemcg_id = -1;
    3575         INIT_LIST_HEAD(&memcg->objcg_list);
    3576 #ifdef CONFIG_CGROUP_WRITEBACK
    3577         INIT_LIST_HEAD(&memcg->cgwb_list);
    3578         for (i = 0; i < MEMCG_CGWB_FRN_CNT; i++)
    3579                 memcg->cgwb_frn[i].done =
    3580                         __WB_COMPLETION_INIT(&memcg_cgwb_frn_waitq);
    3581 #endif
    3582 #ifdef CONFIG_TRANSPARENT_HUGEPAGE
    3583         spin_lock_init(&memcg->deferred_split_queue.split_queue_lock);
    3584         INIT_LIST_HEAD(&memcg->deferred_split_queue.split_queue);
    3585         memcg->deferred_split_queue.split_queue_len = 0;
    3586 #endif
    3587         lru_gen_init_memcg(memcg);
    3588         return memcg;
    3589 fail:
    3590         mem_cgroup_id_remove(memcg);
    3591         __mem_cgroup_free(memcg);
--> 3592         return ERR_PTR(error);
    3593 }

regards,
dan carpenter

Re: [bug report] memcg: replace memcg ID idr with xarray

[Shakeel Butt]

On Thu, Aug 15, 2024 at 01:46:03PM GMT, Dan Carpenter wrote:
> Hello Shakeel Butt,
> 
> Commit ad80273d43b7 ("memcg: replace memcg ID idr with xarray") from
> Aug 9, 2024 (linux-next), leads to the following Smatch static
> checker warning:
> 
> 	mm/memcontrol.c:3592 mem_cgroup_alloc()
> 	warn: passing zero to 'ERR_PTR'

Thanks a lot for the report. I will send v2 soon.