lkml.org 
[lkml]   [2009]   [May]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH] cgroups: handle failure of cgroup_populate_dir() at mount/remount
    * Li Zefan <lizf@cn.fujitsu.com> [2009-05-22 11:00:12]:

    > Now we have 'stat' file in both memory and cpuacct subsystems. If we
    > mount these 2 subsystems with option 'noprefix', the creation of 'stat'
    > file for cpuacct will fail, but without any notificatin to the user.
    >
    > With this patch, we fail the mount/remount in this case.
    >
    > Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
    > ---
    > kernel/cgroup.c | 18 ++++++++++++++++--
    > 1 files changed, 16 insertions(+), 2 deletions(-)
    >
    > diff --git a/kernel/cgroup.c b/kernel/cgroup.c
    > index a7267bf..eab83f7 100644
    > --- a/kernel/cgroup.c
    > +++ b/kernel/cgroup.c
    > @@ -896,6 +896,7 @@ static int parse_cgroupfs_options(char *data,
    > static int cgroup_remount(struct super_block *sb, int *flags, char *data)
    > {
    > int ret = 0;
    > + unsigned long subsys_bits;
    > struct cgroupfs_root *root = sb->s_fs_info;
    > struct cgroup *cgrp = &root->top_cgroup;
    > struct cgroup_sb_opts opts;
    > @@ -914,12 +915,17 @@ static int cgroup_remount(struct super_block *sb, int *flags, char *data)
    > goto out_unlock;
    > }
    >
    > + subsys_bits = root->subsys_bits;
    > ret = rebind_subsystems(root, opts.subsys_bits);
    > if (ret)
    > goto out_unlock;
    >
    > /* (re)populate subsystem files */
    > - cgroup_populate_dir(cgrp);
    > + ret = cgroup_populate_dir(cgrp);
    > + if (ret) {

    if (ret) means a failure right? So we rebind_subsystems to the older
    subsys_bits? Could you please add a comment, the code can be misread.

    > + rebind_subsystems(root, subsys_bits);
    > + goto out_unlock;
    > + }
    >
    > if (opts.release_agent)
    > strcpy(root->release_agent_path, opts.release_agent);
    > @@ -1122,9 +1128,13 @@ static int cgroup_get_sb(struct file_system_type *fs_type,
    > BUG_ON(!list_empty(&root_cgrp->children));
    > BUG_ON(root->number_of_cgroups != 1);
    >
    > - cgroup_populate_dir(root_cgrp);
    > + ret = cgroup_populate_dir(root_cgrp);
    > +
    > mutex_unlock(&inode->i_mutex);
    > mutex_unlock(&cgroup_mutex);
    > +
    > + if (ret)
    > + goto drop_new_super;
    > }
    >
    > simple_set_mnt(mnt, sb);
    > @@ -1803,6 +1813,10 @@ int cgroup_add_file(struct cgroup *cgrp,
    > dput(dentry);
    > } else
    > error = PTR_ERR(dentry);
    > +
    > + if (error)
    > + printk(KERN_WARNING "cgroup: Failed to create file '%s': %d\n",
    > + name, error);
    > return error;
    > }
    >

    Looks good, should we mark cgroup_populate_dir with __must_check?

    --
    Balbir


    \
     
     \ /
      Last update: 2009-05-25 15:21    [W:3.212 / U:0.128 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site