Re: [PATCH -V3 1/1] cgroup: Add inotify event on change tasks file (fork, exit, move pid from file)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi

sorry for noise, but I have a question


    mode = cgroup_file_mode(cft);
    error = cgroup_create_file(dentry, mode | S_IFREG, cgrp->root->sb);
    if (!error) {
        cfe->type = (void *)cft;
        cfe->dentry = dentry;
        dentry->d_fsdata = cfe;
        list_add_tail(&cfe->node, &parent->files);
        cfe = NULL;
    }
    dput(dentry);
out:
    kfree(cfe);
    return error;

this is from for-3.5 version. dput executes if cgroup_create_file fails and done correct. but in cgroup_create_file dget calls just before return, so if cgroup_create_file we call dput without dget. is this correct behavioural??


On 05/04/2012 08:54 PM, Tejun Heo wrote:
Hello,

On Fri, May 04, 2012 at 04:55:44PM +0400, Alexander Nikiforov wrote:
-static int cgroup_add_file(struct cgroup *cgrp, struct cgroup_subsys *subsys,
+static struct cfent* cgroup_add_file(struct cgroup *cgrp, struct cgroup_subsys *subsys,
  			   const struct cftype *cft)
  {
  	struct dentry *dir = cgrp->dentry;
@@ -2696,12 +2717,13 @@ static int cgroup_add_file(struct cgroup *cgrp, struct cgroup_subsys *subsys,

  	cfe = kzalloc(sizeof(*cfe), GFP_KERNEL);
  	if (!cfe)
-		return -ENOMEM;
+		return NULL;

  	dentry = lookup_one_len(name, dir, strlen(name));
  	if (IS_ERR(dentry)) {
  		error = PTR_ERR(dentry);
-		goto out;
+		kfree(cfe);
+		return NULL;
  	}

  	mode = cgroup_file_mode(cft);
@@ -2711,29 +2733,34 @@ static int cgroup_add_file(struct cgroup *cgrp, struct cgroup_subsys *subsys,
  		cfe->dentry = dentry;
  		dentry->d_fsdata = cfe;
  		list_add_tail(&cfe->node,&parent->files);
-		cfe = NULL;
  	}
  	dput(dentry);
-out:
-	kfree(cfe);
-	return error;
+
+	return cfe;
  }
Are you sure you're not creating a memory leak here?
code make the same things, just without goto. move kfree inside if
(IS_ERR(dentry), but before dput(dentry)  cfe set to NULL, so
kfree(cfe) will do nothing. I think it's without any mem leaks.
I'm confused.  Who's freeing cfe after cgroup_create_file() failure?
Also, why are you changing this at all?

Thanks.



--
Best regards,
     Alex Nikiforov,
     Mobile SW, Advanced Software Group,
     Moscow R&D center, Samsung Electronics

--
To unsubscribe from this list: send the line "unsubscribe cgroups" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]     [Monitors]

  Powered by Linux