Re: [PATCH v6] mm: cma: support sysfs

[Minchan Kim]

On Tue, Mar 23, 2021 at 09:47:27PM -0700, John Hubbard wrote:
> On 3/23/21 8:27 PM, Minchan Kim wrote:
> ...
> > > > +static int __init cma_sysfs_init(void)
> > > > +{
> > > > +       unsigned int i;
> > > > +
> > > > +       cma_kobj_root = kobject_create_and_add("cma", mm_kobj);
> > > > +       if (!cma_kobj_root)
> > > > +               return -ENOMEM;
> > > > +
> > > > +       for (i = 0; i < cma_area_count; i++) {
> > > > +               int err;
> > > > +               struct cma *cma;
> > > > +               struct cma_kobject *cma_kobj;
> > > > +
> > > > +               cma_kobj = kzalloc(sizeof(*cma_kobj), GFP_KERNEL);
> > > > +               if (!cma_kobj) {
> > > > +                       kobject_put(cma_kobj_root);
> > > > +                       return -ENOMEM;
> > > 
> > > This leaks little cma_kobj's all over the floor. :)
> > 
> > I thought kobject_put(cma_kobj_root) should deal with it. No?
> > 
> If this fails when i > 0, there will be cma_kobj instances that
> were stashed in the cma_areas[] array. But this code only deletes
> the most recently allocated cma_kobj, not anything allocated on
> previous iterations of the loop.

Oh, I misunderstood that destroying of root kobject will release
children recursively. Seems not true. Go back to old version.


index 16c81c9cb9b7..418951a3f138 100644
--- a/mm/cma_sysfs.c
+++ b/mm/cma_sysfs.c
@@ -80,20 +80,19 @@ static struct kobj_type cma_ktype = {
 static int __init cma_sysfs_init(void)
 {
        unsigned int i;
+       int err;
+       struct cma *cma;
+       struct cma_kobject *cma_kobj;

        cma_kobj_root = kobject_create_and_add("cma", mm_kobj);
        if (!cma_kobj_root)
                return -ENOMEM;

        for (i = 0; i < cma_area_count; i++) {
-               int err;
-               struct cma *cma;
-               struct cma_kobject *cma_kobj;
-
                cma_kobj = kzalloc(sizeof(*cma_kobj), GFP_KERNEL);
                if (!cma_kobj) {
-                       kobject_put(cma_kobj_root);
-                       return -ENOMEM;
+                       err = -ENOMEM;
+                       goto out;
                }

                cma = &cma_areas[i];
@@ -103,11 +102,21 @@ static int __init cma_sysfs_init(void)
                                           cma_kobj_root, "%s", cma->name);
                if (err) {
                        kobject_put(&cma_kobj->kobj);
-                       kobject_put(cma_kobj_root);
-                       return err;
+                       goto out;
                }
        }

        return 0;
+out:
+       while (--i >= 0) {
+               cma = &cma_areas[i];
+
+               kobject_put(&cma->kobj->kobj);
+               kfree(cma->kobj);
+               cma->kobj = NULL;
+       }
+       kobject_put(cma_kobj_root);
+
+       return err;
 }
 subsys_initcall(cma_sysfs_init);