On Mon, Dec 20, 2010 at 11:12 PM, Nick Piggin <npiggin@xxxxxxxxx> wrote: > > cgroup fs: avoid switching ->d_op on live dentry > > Switching d_op on a live dentry is racy in general, so avoid it. In this case > it is a negative dentry, which is safer, but there are still concurrent ops > which may be called on d_op in that case (eg. d_revalidate). So in general > a filesystem may not do this. Fix cgroupfs so as not to do this. > > This caused problematic interaction with other development work, thanks > to Sedat Dilek and Eric Paris for reporting the issue here: > > http://marc.info/?l=linux-next&m=129287924727475&w=2 > > Signed-off-by: Nick Piggin <npiggin@xxxxxxxxx> Acked-by: Paul Menage <menage@xxxxxxxxxx> Thanks, Paul > > --- > One of the patches in my vfs scaling series tripped over this, comments? > > kernel/cgroup.c | 27 ++++++++++++++++++++++----- > 1 file changed, 22 insertions(+), 5 deletions(-) > > Index: linux-2.6/kernel/cgroup.c > =================================================================== > --- linux-2.6.orig/kernel/cgroup.c 2010-12-21 16:02:32.000000000 +1100 > +++ linux-2.6/kernel/cgroup.c 2010-12-21 16:51:39.000000000 +1100 > @@ -763,6 +763,8 @@ EXPORT_SYMBOL_GPL(cgroup_unlock); > * -> cgroup_mkdir. > */ > > +static struct dentry *cgroup_lookup(struct inode *dir, > + struct dentry *dentry, struct nameidata *nd); > static int cgroup_mkdir(struct inode *dir, struct dentry *dentry, int mode); > static int cgroup_rmdir(struct inode *unused_dir, struct dentry *dentry); > static int cgroup_populate_dir(struct cgroup *cgrp); > @@ -2180,7 +2182,7 @@ static const struct file_operations cgro > }; > > static const struct inode_operations cgroup_dir_inode_operations = { > - .lookup = simple_lookup, > + .lookup = cgroup_lookup, > .mkdir = cgroup_mkdir, > .rmdir = cgroup_rmdir, > .rename = cgroup_rename, > @@ -2196,13 +2198,29 @@ static inline struct cftype *__file_cft( > return __d_cft(file->f_dentry); > } > > -static int cgroup_create_file(struct dentry *dentry, mode_t mode, > - struct super_block *sb) > +static int cgroup_delete_dentry(struct dentry *dentry) > +{ > + return 1; > +} > + > +static struct dentry *cgroup_lookup(struct inode *dir, > + struct dentry *dentry, struct nameidata *nd) > { > - static const struct dentry_operations cgroup_dops = { > + static const struct dentry_operations cgroup_dentry_operations = { > + .d_delete = cgroup_delete_dentry, > .d_iput = cgroup_diput, > }; > > + if (dentry->d_name.len > NAME_MAX) > + return ERR_PTR(-ENAMETOOLONG); > + dentry->d_op = &cgroup_dentry_operations; > + d_add(dentry, NULL); > + return NULL; > +} > + > +static int cgroup_create_file(struct dentry *dentry, mode_t mode, > + struct super_block *sb) > +{ > struct inode *inode; > > if (!dentry) > @@ -2228,7 +2246,6 @@ static int cgroup_create_file(struct den > inode->i_size = 0; > inode->i_fop = &cgroup_file_operations; > } > - dentry->d_op = &cgroup_dops; > d_instantiate(dentry, inode); > dget(dentry); /* Extra count - pin the dentry in core */ > return 0; > -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
