From cd12fe88351666e8c67d62f595b4695e08f1d6c4 Mon Sep 17 00:00:00 2001 From: Erez_Zadok Date: Fri, 29 Jun 2007 02:00:24 -0400 Subject: [PATCH] Unionfs: Change the semantics of sb info's rwsem This rw semaphore is used to make sure that a branch management operation... 1) will not begin before all currently in-flight operations complete 2) any new operations do not execute until the currently running branch management operation completes Reworked the patch a bit, added comments, and fixed some bugs, from the version originally committed into the master branch. TODO: rename the functions unionfs_{read,write}_{,un}lock() to something more descriptive. Signed-off-by: Josef 'Jeff' Sipek Signed-off-by: Erez Zadok --- fs/unionfs/commonfops.c | 30 ++++++------------ fs/unionfs/copyup.c | 10 ------ fs/unionfs/dentry.c | 7 +++++ fs/unionfs/dirfops.c | 10 +++--- fs/unionfs/dirhelper.c | 10 ------ fs/unionfs/file.c | 16 +++++----- fs/unionfs/inode.c | 68 +++++++++++++++++++++++++---------------- fs/unionfs/main.c | 23 +++++++------- fs/unionfs/rename.c | 2 ++ fs/unionfs/super.c | 32 +++++++++++++------ fs/unionfs/union.h | 15 ++++++--- fs/unionfs/unlink.c | 4 +++ fs/unionfs/xattr.c | 8 +++++ 13 files changed, 132 insertions(+), 103 deletions(-) diff --git a/fs/unionfs/commonfops.c b/fs/unionfs/commonfops.c index 4cfe32181bd..f7f49b4cabf 100644 --- a/fs/unionfs/commonfops.c +++ b/fs/unionfs/commonfops.c @@ -127,10 +127,8 @@ static void cleanup_file(struct file *file) printk(KERN_ERR "unionfs: no superblock for " "file %p\n", file); else { - unionfs_read_lock(sb); /* decrement count of open files */ branchput(sb, i); - unionfs_read_unlock(sb); /* * fput will perform an mntput for us on the * correct branch. Although we're using the @@ -172,9 +170,7 @@ static int open_all_files(struct file *file) dget(lower_dentry); unionfs_mntget(dentry, bindex); - unionfs_read_lock(sb); branchget(sb, bindex); - unionfs_read_unlock(sb); lower_file = dentry_open(lower_dentry, @@ -220,9 +216,7 @@ static int open_highest_file(struct file *file, int willwrite) dget(lower_dentry); unionfs_mntget(dentry, bstart); - unionfs_read_lock(sb); branchget(sb, bstart); - unionfs_read_unlock(sb); lower_file = dentry_open(lower_dentry, unionfs_lower_mnt_idx(dentry, bstart), file->f_flags); @@ -270,9 +264,7 @@ static int do_delayed_copyup(struct file *file) bend = fbend(file); for (bindex = bstart; bindex <= bend; bindex++) { if (unionfs_lower_file_idx(file, bindex)) { - unionfs_read_lock(dentry->d_sb); branchput(dentry->d_sb, bindex); - unionfs_read_unlock(dentry->d_sb); fput(unionfs_lower_file_idx(file, bindex)); unionfs_set_lower_file_idx(file, bindex, NULL); } @@ -430,9 +422,7 @@ static int __open_dir(struct inode *inode, struct file *file) * The branchget goes after the open, because otherwise * we would miss the reference on release. */ - unionfs_read_lock(inode->i_sb); branchget(inode->i_sb, bindex); - unionfs_read_unlock(inode->i_sb); } return 0; @@ -493,9 +483,7 @@ static int __open_file(struct inode *inode, struct file *file) return PTR_ERR(lower_file); unionfs_set_lower_file(file, lower_file); - unionfs_read_lock(inode->i_sb); branchget(inode->i_sb, bstart); - unionfs_read_unlock(inode->i_sb); return 0; } @@ -509,6 +497,7 @@ int unionfs_open(struct inode *inode, struct file *file) int size; unionfs_read_lock(inode->i_sb); + file->private_data = kzalloc(sizeof(struct unionfs_file_info), GFP_KERNEL); if (!UNIONFS_F(file)) { @@ -559,9 +548,7 @@ int unionfs_open(struct inode *inode, struct file *file) if (!lower_file) continue; - unionfs_read_lock(file->f_dentry->d_sb); branchput(file->f_dentry->d_sb, bindex); - unionfs_read_unlock(file->f_dentry->d_sb); /* fput calls dput for lower_dentry */ fput(lower_file); } @@ -585,7 +572,11 @@ out_nofree: return err; } -/* release all lower object references & free the file info structure */ +/* + * release all lower object references & free the file info structure + * + * No need to grab sb info's rwsem. + */ int unionfs_file_release(struct inode *inode, struct file *file) { struct file *lower_file = NULL; @@ -618,9 +609,7 @@ int unionfs_file_release(struct inode *inode, struct file *file) if (lower_file) { fput(lower_file); - unionfs_read_lock(sb); branchput(sb, bindex); - unionfs_read_unlock(sb); } } kfree(fileinfo->lower_files); @@ -742,7 +731,8 @@ long unionfs_ioctl(struct file *file, unsigned int cmd, unsigned long arg) { long err; - unionfs_read_lock(file->f_dentry->d_sb); + unionfs_read_lock(file->f_path.dentry->d_sb); + if ((err = unionfs_file_revalidate(file, 1))) goto out; @@ -767,7 +757,7 @@ long unionfs_ioctl(struct file *file, unsigned int cmd, unsigned long arg) } out: - unionfs_read_unlock(file->f_dentry->d_sb); + unionfs_read_unlock(file->f_path.dentry->d_sb); unionfs_check_file(file); return err; } @@ -776,7 +766,7 @@ int unionfs_flush(struct file *file, fl_owner_t id) { int err = 0; struct file *lower_file = NULL; - struct dentry *dentry = file->f_dentry; + struct dentry *dentry = file->f_path.dentry; int bindex, bstart, bend; unionfs_read_lock(dentry->d_sb); diff --git a/fs/unionfs/copyup.c b/fs/unionfs/copyup.c index 359bbcc7be5..9a68813fed3 100644 --- a/fs/unionfs/copyup.c +++ b/fs/unionfs/copyup.c @@ -193,9 +193,7 @@ static int __copyup_reg_data(struct dentry *dentry, /* open old file */ unionfs_mntget(dentry, old_bindex); - unionfs_read_lock(sb); branchget(sb, old_bindex); - unionfs_read_unlock(sb); /* dentry_open calls dput and mntput if it returns an error */ input_file = dentry_open(old_lower_dentry, unionfs_lower_mnt_idx(dentry, old_bindex), @@ -213,9 +211,7 @@ static int __copyup_reg_data(struct dentry *dentry, /* open new file */ dget(new_lower_dentry); output_mnt = unionfs_mntget(sb->s_root, new_bindex); - unionfs_read_lock(sb); branchget(sb, new_bindex); - unionfs_read_unlock(sb); output_file = dentry_open(new_lower_dentry, output_mnt, O_WRONLY | O_LARGEFILE); if (IS_ERR(output_file)) { @@ -290,17 +286,13 @@ out_close_out: fput(output_file); out_close_in2: - unionfs_read_lock(sb); branchput(sb, new_bindex); - unionfs_read_unlock(sb); out_close_in: fput(input_file); out: - unionfs_read_lock(sb); branchput(sb, old_bindex); - unionfs_read_unlock(sb); return err; } @@ -453,9 +445,7 @@ out_unlink: /* need to close the file */ fput(*copyup_file); - unionfs_read_lock(sb); branchput(sb, new_bindex); - unionfs_read_unlock(sb); } /* diff --git a/fs/unionfs/dentry.c b/fs/unionfs/dentry.c index 6310c32b74b..fb425ed1b2a 100644 --- a/fs/unionfs/dentry.c +++ b/fs/unionfs/dentry.c @@ -408,11 +408,15 @@ static int unionfs_d_revalidate(struct dentry *dentry, struct nameidata *nd) { int err; + unionfs_read_lock(dentry->d_sb); + unionfs_lock_dentry(dentry); err = __unionfs_d_revalidate_chain(dentry, nd, 0); unionfs_unlock_dentry(dentry); unionfs_check_dentry(dentry); + unionfs_read_unlock(dentry->d_sb); + return err; } @@ -424,6 +428,8 @@ static void unionfs_d_release(struct dentry *dentry) { int bindex, bstart, bend; + unionfs_read_lock(dentry->d_sb); + unionfs_check_dentry(dentry); /* this could be a negative dentry, so check first */ if (!UNIONFS_D(dentry)) { @@ -459,6 +465,7 @@ out_free: free_dentry_private_data(dentry); out: + unionfs_read_unlock(dentry->d_sb); return; } diff --git a/fs/unionfs/dirfops.c b/fs/unionfs/dirfops.c index a7ba9476a63..8503411a048 100644 --- a/fs/unionfs/dirfops.c +++ b/fs/unionfs/dirfops.c @@ -97,7 +97,8 @@ static int unionfs_readdir(struct file *file, void *dirent, filldir_t filldir) int bend; loff_t offset; - unionfs_read_lock(file->f_dentry->d_sb); + unionfs_read_lock(file->f_path.dentry->d_sb); + if ((err = unionfs_file_revalidate(file, 0))) goto out; @@ -179,7 +180,7 @@ static int unionfs_readdir(struct file *file, void *dirent, filldir_t filldir) file->f_pos = rdstate2offset(uds); out: - unionfs_read_unlock(file->f_dentry->d_sb); + unionfs_read_unlock(file->f_path.dentry->d_sb); return err; } @@ -198,7 +199,8 @@ static loff_t unionfs_dir_llseek(struct file *file, loff_t offset, int origin) struct unionfs_dir_state *rdstate; loff_t err; - unionfs_read_lock(file->f_dentry->d_sb); + unionfs_read_lock(file->f_path.dentry->d_sb); + if ((err = unionfs_file_revalidate(file, 0))) goto out; @@ -255,7 +257,7 @@ static loff_t unionfs_dir_llseek(struct file *file, loff_t offset, int origin) } out: - unionfs_read_unlock(file->f_dentry->d_sb); + unionfs_read_unlock(file->f_path.dentry->d_sb); return err; } diff --git a/fs/unionfs/dirhelper.c b/fs/unionfs/dirhelper.c index 80d142786dc..a72f7110b7b 100644 --- a/fs/unionfs/dirhelper.c +++ b/fs/unionfs/dirhelper.c @@ -97,7 +97,6 @@ int delete_whiteouts(struct dentry *dentry, int bindex, struct sioq_args args; sb = dentry->d_sb; - unionfs_read_lock(sb); BUG_ON(!S_ISDIR(dentry->d_inode->i_mode)); BUG_ON(bindex < dbstart(dentry)); @@ -124,7 +123,6 @@ int delete_whiteouts(struct dentry *dentry, int bindex, mutex_unlock(&lower_dir->i_mutex); out: - unionfs_read_unlock(sb); return err; } @@ -193,7 +191,6 @@ int check_empty(struct dentry *dentry, struct unionfs_dir_state **namelist) sb = dentry->d_sb; - unionfs_read_lock(sb); BUG_ON(!S_ISDIR(dentry->d_inode->i_mode)); @@ -231,9 +228,7 @@ int check_empty(struct dentry *dentry, struct unionfs_dir_state **namelist) dget(lower_dentry); unionfs_mntget(dentry, bindex); - unionfs_read_lock(sb); branchget(sb, bindex); - unionfs_read_unlock(sb); lower_file = dentry_open(lower_dentry, unionfs_lower_mnt_idx(dentry, bindex), @@ -241,9 +236,7 @@ int check_empty(struct dentry *dentry, struct unionfs_dir_state **namelist) if (IS_ERR(lower_file)) { err = PTR_ERR(lower_file); dput(lower_dentry); - unionfs_read_lock(sb); branchput(sb, bindex); - unionfs_read_unlock(sb); goto out; } @@ -258,9 +251,7 @@ int check_empty(struct dentry *dentry, struct unionfs_dir_state **namelist) /* fput calls dput for lower_dentry */ fput(lower_file); - unionfs_read_lock(sb); branchput(sb, bindex); - unionfs_read_unlock(sb); if (err < 0) goto out; @@ -275,7 +266,6 @@ out: kfree(buf); } - unionfs_read_unlock(sb); return err; } diff --git a/fs/unionfs/file.c b/fs/unionfs/file.c index df413ebfa7b..8f42da3d2d4 100644 --- a/fs/unionfs/file.c +++ b/fs/unionfs/file.c @@ -23,7 +23,7 @@ static ssize_t unionfs_read(struct file *file, char __user *buf, { int err; - unionfs_read_lock(file->f_dentry->d_sb); + unionfs_read_lock(file->f_path.dentry->d_sb); if ((err = unionfs_file_revalidate(file, 0))) goto out; unionfs_check_file(file); @@ -35,7 +35,7 @@ static ssize_t unionfs_read(struct file *file, char __user *buf, unionfs_lower_dentry(file->f_path.dentry)); out: - unionfs_read_unlock(file->f_dentry->d_sb); + unionfs_read_unlock(file->f_path.dentry->d_sb); unionfs_check_file(file); return err; } @@ -46,7 +46,7 @@ static ssize_t unionfs_aio_read(struct kiocb *iocb, const struct iovec *iov, int err = 0; struct file *file = iocb->ki_filp; - unionfs_read_lock(file->f_dentry->d_sb); + unionfs_read_lock(file->f_path.dentry->d_sb); if ((err = unionfs_file_revalidate(file, 0))) goto out; unionfs_check_file(file); @@ -61,7 +61,7 @@ static ssize_t unionfs_aio_read(struct kiocb *iocb, const struct iovec *iov, unionfs_lower_dentry(file->f_path.dentry)); out: - unionfs_read_unlock(file->f_dentry->d_sb); + unionfs_read_unlock(file->f_path.dentry->d_sb); unionfs_check_file(file); return err; } @@ -71,7 +71,7 @@ static ssize_t unionfs_write(struct file *file, const char __user *buf, { int err = 0; - unionfs_read_lock(file->f_dentry->d_sb); + unionfs_read_lock(file->f_path.dentry->d_sb); if ((err = unionfs_file_revalidate(file, 1))) goto out; unionfs_check_file(file); @@ -84,7 +84,7 @@ static ssize_t unionfs_write(struct file *file, const char __user *buf, } out: - unionfs_read_unlock(file->f_dentry->d_sb); + unionfs_read_unlock(file->f_path.dentry->d_sb); return err; } @@ -100,7 +100,7 @@ static int unionfs_mmap(struct file *file, struct vm_area_struct *vma) int willwrite; struct file *lower_file; - unionfs_read_lock(file->f_dentry->d_sb); + unionfs_read_lock(file->f_path.dentry->d_sb); /* This might be deferred to mmap's writepage */ willwrite = ((vma->vm_flags | VM_SHARED | VM_WRITE) == vma->vm_flags); @@ -130,7 +130,7 @@ static int unionfs_mmap(struct file *file, struct vm_area_struct *vma) } out: - unionfs_read_unlock(file->f_dentry->d_sb); + unionfs_read_unlock(file->f_path.dentry->d_sb); if (!err) { /* copyup could cause parent dir times to change */ unionfs_copy_attr_times(file->f_dentry->d_parent->d_inode); diff --git a/fs/unionfs/inode.c b/fs/unionfs/inode.c index 20b234d0eba..384800ca2fd 100644 --- a/fs/unionfs/inode.c +++ b/fs/unionfs/inode.c @@ -30,16 +30,6 @@ static int unionfs_create(struct inode *parent, struct dentry *dentry, char *name = NULL; int valid = 0; - /* - * We have to read-lock the superblock rwsem, and we have to - * revalidate the parent dentry and this one. A branch-management - * operation could have taken place, mid-way through a VFS operation - * that eventually reaches here. So we have to ensure consistency, - * just as we do with the file operations. - * - * XXX: we may need to do this for all other inode ops that take a - * dentry. - */ unionfs_read_lock(dentry->d_sb); unionfs_lock_dentry(dentry); @@ -244,6 +234,11 @@ out: return err; } +/* + * unionfs_lookup is the only special function which takes a dentry, yet we + * do NOT want to call __unionfs_d_revalidate_chain because by definition, + * we don't have a valid dentry here yet. + */ static struct dentry *unionfs_lookup(struct inode *parent, struct dentry *dentry, struct nameidata *nd) @@ -251,11 +246,7 @@ static struct dentry *unionfs_lookup(struct inode *parent, struct path path_save; struct dentry *ret; - /* - * lookup is the only special function which takes a dentry, yet we - * do NOT want to call __unionfs_d_revalidate_chain because by - * definition, we don't have a valid dentry here yet. - */ + unionfs_read_lock(dentry->d_sb); /* save the dentry & vfsmnt from namei */ if (nd) { @@ -281,6 +272,8 @@ static struct dentry *unionfs_lookup(struct inode *parent, unionfs_check_inode(parent); unionfs_check_dentry(dentry); unionfs_check_dentry(dentry->d_parent); + unionfs_read_unlock(dentry->d_sb); + return ret; } @@ -294,6 +287,7 @@ static int unionfs_link(struct dentry *old_dentry, struct inode *dir, struct dentry *whiteout_dentry; char *name = NULL; + unionfs_read_lock(old_dentry->d_sb); unionfs_double_lock_dentry(new_dentry, old_dentry); if (!__unionfs_d_revalidate_chain(old_dentry, NULL, 0)) { @@ -426,6 +420,8 @@ out: unionfs_check_inode(dir); unionfs_check_dentry(new_dentry); unionfs_check_dentry(old_dentry); + unionfs_read_unlock(old_dentry->d_sb); + return err; } @@ -440,6 +436,7 @@ static int unionfs_symlink(struct inode *dir, struct dentry *dentry, int bindex = 0, bstart; char *name = NULL; + unionfs_read_lock(dentry->d_sb); unionfs_lock_dentry(dentry); if (dentry->d_inode && @@ -585,6 +582,8 @@ out: unionfs_check_inode(dir); unionfs_check_dentry(dentry); + unionfs_read_unlock(dentry->d_sb); + return err; } @@ -598,6 +597,7 @@ static int unionfs_mkdir(struct inode *parent, struct dentry *dentry, int mode) int whiteout_unlinked = 0; struct sioq_args args; + unionfs_read_lock(dentry->d_sb); unionfs_lock_dentry(dentry); if (dentry->d_inode && @@ -732,6 +732,8 @@ out: unionfs_unlock_dentry(dentry); unionfs_check_inode(parent); unionfs_check_dentry(dentry); + unionfs_read_unlock(dentry->d_sb); + return err; } @@ -745,6 +747,7 @@ static int unionfs_mknod(struct inode *dir, struct dentry *dentry, int mode, char *name = NULL; int whiteout_unlinked = 0; + unionfs_read_lock(dentry->d_sb); unionfs_lock_dentry(dentry); if (dentry->d_inode && @@ -858,6 +861,8 @@ out: unionfs_check_inode(dir); unionfs_check_dentry(dentry); + unionfs_read_unlock(dentry->d_sb); + return err; } @@ -867,6 +872,7 @@ static int unionfs_readlink(struct dentry *dentry, char __user *buf, int err; struct dentry *lower_dentry; + unionfs_read_lock(dentry->d_sb); unionfs_lock_dentry(dentry); if (!__unionfs_d_revalidate_chain(dentry, NULL, 0)) { @@ -891,23 +897,28 @@ static int unionfs_readlink(struct dentry *dentry, char __user *buf, out: unionfs_unlock_dentry(dentry); unionfs_check_dentry(dentry); + unionfs_read_unlock(dentry->d_sb); + return err; } -/* We don't lock the dentry here, because readlink does the heavy lifting. */ +/* + * unionfs_follow_link takes a dentry, but it is simple. It only needs to + * allocate some memory and then call our ->readlink method. Our + * unionfs_readlink *does* lock our dentry and revalidate the dentry. + * Therefore, we do not have to lock our dentry here, to prevent a deadlock; + * nor do we need to revalidate it either. It is safe to not lock our + * dentry here because unionfs_follow_link does not do anything (prior to + * calling ->readlink) which could become inconsistent due to branch + * management. + */ static void *unionfs_follow_link(struct dentry *dentry, struct nameidata *nd) { char *buf; int len = PAGE_SIZE, err; mm_segment_t old_fs; - unionfs_lock_dentry(dentry); - - if (dentry->d_inode && - !__unionfs_d_revalidate_chain(dentry, nd, 0)) { - err = -ESTALE; - goto out; - } + unionfs_read_lock(dentry->d_sb); /* This is freed by the put_link method assuming a successful call. */ buf = kmalloc(len, GFP_KERNEL); @@ -931,14 +942,17 @@ static void *unionfs_follow_link(struct dentry *dentry, struct nameidata *nd) err = 0; out: - unionfs_unlock_dentry(dentry); unionfs_check_dentry(dentry); + unionfs_read_unlock(dentry->d_sb); return ERR_PTR(err); } +/* FIXME: We may not have to lock here */ static void unionfs_put_link(struct dentry *dentry, struct nameidata *nd, void *cookie) { + unionfs_read_lock(dentry->d_sb); + unionfs_lock_dentry(dentry); if (!__unionfs_d_revalidate_chain(dentry, nd, 0)) printk("unionfs: put_link failed to revalidate dentry\n"); @@ -946,6 +960,7 @@ static void unionfs_put_link(struct dentry *dentry, struct nameidata *nd, unionfs_check_dentry(dentry); kfree(nd_get_link(nd)); + unionfs_read_unlock(dentry->d_sb); } /* @@ -987,9 +1002,7 @@ static int inode_permission(struct inode *inode, int mask, (!strcmp("nfs", (inode)->i_sb->s_type->name)) && (nd) && (nd->mnt) && (nd->mnt->mnt_sb)) { int perms; - unionfs_read_lock(nd->mnt->mnt_sb); perms = branchperms(nd->mnt->mnt_sb, bindex); - unionfs_read_unlock(nd->mnt->mnt_sb); if (perms & MAY_NFSRO) retval = generic_permission(inode, submask, NULL); @@ -1101,6 +1114,7 @@ static int unionfs_setattr(struct dentry *dentry, struct iattr *ia) int i; int copyup = 0; + unionfs_read_lock(dentry->d_sb); unionfs_lock_dentry(dentry); if (!__unionfs_d_revalidate_chain(dentry, NULL, 0)) { @@ -1175,6 +1189,8 @@ out: unionfs_unlock_dentry(dentry); unionfs_check_dentry(dentry); unionfs_check_dentry(dentry->d_parent); + unionfs_read_unlock(dentry->d_sb); + return err; } diff --git a/fs/unionfs/main.c b/fs/unionfs/main.c index 2f884abc307..e437edba375 100644 --- a/fs/unionfs/main.c +++ b/fs/unionfs/main.c @@ -271,7 +271,13 @@ int parse_branch_mode(const char *name) return perms; } -/* parse the dirs= mount argument */ +/* + * parse the dirs= mount argument + * + * We don't need to lock the superblock private data's rwsem, as we get + * called only by unionfs_read_super - it is still a long time before anyone + * can even get a reference to us. + */ static int parse_dirs_option(struct super_block *sb, struct unionfs_dentry_info *lower_root_info, char *options) { @@ -357,11 +363,9 @@ static int parse_dirs_option(struct super_block *sb, struct unionfs_dentry_info lower_root_info->lower_paths[bindex].dentry = nd.dentry; lower_root_info->lower_paths[bindex].mnt = nd.mnt; - unionfs_write_lock(sb); set_branchperms(sb, bindex, perms); set_branch_count(sb, bindex, 0); new_branch_id(sb, bindex); - unionfs_write_unlock(sb); if (lower_root_info->bstart < 0) lower_root_info->bstart = bindex; @@ -561,6 +565,10 @@ static struct dentry *unionfs_d_alloc_root(struct super_block *sb) return ret; } +/* + * There is no need to lock the unionfs_super_info's rwsem as there is no + * way anyone can have a reference to the superblock at this point in time. + */ static int unionfs_read_super(struct super_block *sb, void *raw_data, int silent) { @@ -607,19 +615,12 @@ static int unionfs_read_super(struct super_block *sb, void *raw_data, BUG_ON(bstart != 0); sbend(sb) = bend = lower_root_info->bend; for (bindex = bstart; bindex <= bend; bindex++) { - struct dentry *d; - - d = lower_root_info->lower_paths[bindex].dentry; - - unionfs_write_lock(sb); + struct dentry *d = lower_root_info->lower_paths[bindex].dentry; unionfs_set_lower_super_idx(sb, bindex, d->d_sb); - unionfs_write_unlock(sb); } /* max Bytes is the maximum bytes from highest priority branch */ - unionfs_read_lock(sb); sb->s_maxbytes = unionfs_lower_super_idx(sb, 0)->s_maxbytes; - unionfs_read_unlock(sb); sb->s_op = &unionfs_sops; diff --git a/fs/unionfs/rename.c b/fs/unionfs/rename.c index f9f9b1e0629..0316258d858 100644 --- a/fs/unionfs/rename.c +++ b/fs/unionfs/rename.c @@ -401,6 +401,7 @@ int unionfs_rename(struct inode *old_dir, struct dentry *old_dentry, int err = 0; struct dentry *wh_dentry; + unionfs_read_lock(old_dentry->d_sb); unionfs_double_lock_dentry(old_dentry, new_dentry); if (!__unionfs_d_revalidate_chain(old_dentry, NULL, 0)) { @@ -508,5 +509,6 @@ out: unionfs_unlock_dentry(new_dentry); unionfs_unlock_dentry(old_dentry); + unionfs_read_unlock(old_dentry->d_sb); return err; } diff --git a/fs/unionfs/super.c b/fs/unionfs/super.c index b1504121e6c..6c9bc1d72f2 100644 --- a/fs/unionfs/super.c +++ b/fs/unionfs/super.c @@ -65,6 +65,8 @@ static void unionfs_read_inode(struct inode *inode) * else that's needed, and the other is fine. This way we truncate the inode * size (and its pages) and then clear our own inode, which will do an iput * on our and the lower inode. + * + * No need to lock sb info's rwsem. */ static void unionfs_delete_inode(struct inode *inode) { @@ -76,7 +78,11 @@ static void unionfs_delete_inode(struct inode *inode) clear_inode(inode); } -/* final actions when unmounting a file system */ +/* + * final actions when unmounting a file system + * + * No need to lock rwsem. + */ static void unionfs_put_super(struct super_block *sb) { int bindex, bstart, bend; @@ -115,6 +121,9 @@ static int unionfs_statfs(struct dentry *dentry, struct kstatfs *buf) struct super_block *sb; struct dentry *lower_dentry; + sb = dentry->d_sb; + + unionfs_read_lock(sb); unionfs_lock_dentry(dentry); if (!__unionfs_d_revalidate_chain(dentry, NULL, 0)) { @@ -123,11 +132,7 @@ static int unionfs_statfs(struct dentry *dentry, struct kstatfs *buf) } unionfs_check_dentry(dentry); - sb = dentry->d_sb; - - unionfs_read_lock(sb); lower_dentry = unionfs_lower_dentry(sb->s_root); - unionfs_read_unlock(sb); err = vfs_statfs(lower_dentry, buf); /* set return buf to our f/s to avoid confusing user-level utils */ @@ -150,6 +155,7 @@ static int unionfs_statfs(struct dentry *dentry, struct kstatfs *buf) out: unionfs_unlock_dentry(dentry); unionfs_check_dentry(dentry); + unionfs_read_unlock(sb); return err; } @@ -790,6 +796,8 @@ out_error: * and the inode is not hashed anywhere. Used to clear anything * that needs to be, before the inode is completely destroyed and put * on the inode free list. + * + * No need to lock sb info's rwsem. */ static void unionfs_clear_inode(struct inode *inode) { @@ -874,6 +882,8 @@ void unionfs_destroy_inode_cache(void) /* * Called when we have a dirty inode, right here we only throw out * parts of our readdir list that are too old. + * + * No need to grab sb info's rwsem. */ static int unionfs_write_inode(struct inode *inode, int sync) { @@ -914,18 +924,20 @@ static void unionfs_umount_begin(struct vfsmount *mnt, int flags) sb = mnt->mnt_sb; + unionfs_read_lock(sb); + bstart = sbstart(sb); bend = sbend(sb); for (bindex = bstart; bindex <= bend; bindex++) { lower_mnt = unionfs_lower_mnt_idx(sb->s_root, bindex); - unionfs_read_lock(sb); lower_sb = unionfs_lower_super_idx(sb, bindex); - unionfs_read_unlock(sb); if (lower_mnt && lower_sb && lower_sb->s_op && lower_sb->s_op->umount_begin) lower_sb->s_op->umount_begin(lower_mnt, flags); } + + unionfs_read_unlock(sb); } static int unionfs_show_options(struct seq_file *m, struct vfsmount *mnt) @@ -937,6 +949,8 @@ static int unionfs_show_options(struct seq_file *m, struct vfsmount *mnt) int bindex, bstart, bend; int perms; + unionfs_read_lock(sb); + unionfs_lock_dentry(sb->s_root); tmp_page = (char*) __get_free_page(GFP_KERNEL); @@ -958,9 +972,7 @@ static int unionfs_show_options(struct seq_file *m, struct vfsmount *mnt) goto out; } - unionfs_read_lock(sb); perms = branchperms(sb, bindex); - unionfs_read_unlock(sb); seq_printf(m, "%s=%s", path, perms & MAY_WRITE ? "rw" : "ro"); @@ -973,6 +985,8 @@ out: unionfs_unlock_dentry(sb->s_root); + unionfs_read_unlock(sb); + return ret; } diff --git a/fs/unionfs/union.h b/fs/unionfs/union.h index b66ff60df6f..2bedaec4e28 100644 --- a/fs/unionfs/union.h +++ b/fs/unionfs/union.h @@ -134,7 +134,16 @@ struct unionfs_sb_info { int bend; atomic_t generation; - struct rw_semaphore rwsem; /* protects access to data+id fields */ + + /* + * This rwsem is used to make sure that a branch management + * operation... + * 1) will not begin before all currently in-flight operations + * complete + * 2) any new operations do not execute until the currently + * running branch management operation completes + */ + struct rw_semaphore rwsem; int high_branch_id; /* last unique branch ID given */ struct unionfs_data *data; }; @@ -376,9 +385,7 @@ static inline int is_robranch_super(const struct super_block *sb, int index) { int ret; - unionfs_read_lock(sb); ret = (!(branchperms(sb, index) & MAY_WRITE)) ? -EROFS : 0; - unionfs_read_unlock(sb); return ret; } @@ -389,11 +396,9 @@ static inline int is_robranch_idx(const struct dentry *dentry, int index) BUG_ON(index < 0); - unionfs_read_lock(dentry->d_sb); if ((!(branchperms(dentry->d_sb, index) & MAY_WRITE)) || IS_RDONLY(unionfs_lower_dentry_idx(dentry, index)->d_inode)) err = -EROFS; - unionfs_read_unlock(dentry->d_sb); return err; } diff --git a/fs/unionfs/unlink.c b/fs/unionfs/unlink.c index bffa458c856..47bebaba6d2 100644 --- a/fs/unionfs/unlink.c +++ b/fs/unionfs/unlink.c @@ -76,6 +76,7 @@ int unionfs_unlink(struct inode *dir, struct dentry *dentry) { int err = 0; + unionfs_read_lock(dentry->d_sb); unionfs_lock_dentry(dentry); if (!__unionfs_d_revalidate_chain(dentry, NULL, 0)) { @@ -103,6 +104,7 @@ out: unionfs_check_inode(dir); } unionfs_unlock_dentry(dentry); + unionfs_read_unlock(dentry->d_sb); return err; } @@ -143,6 +145,7 @@ int unionfs_rmdir(struct inode *dir, struct dentry *dentry) int err = 0; struct unionfs_dir_state *namelist = NULL; + unionfs_read_lock(dentry->d_sb); unionfs_lock_dentry(dentry); if (!__unionfs_d_revalidate_chain(dentry, NULL, 0)) { @@ -184,5 +187,6 @@ out: free_rdstate(namelist); unionfs_unlock_dentry(dentry); + unionfs_read_unlock(dentry->d_sb); return err; } diff --git a/fs/unionfs/xattr.c b/fs/unionfs/xattr.c index b5ae59c64ed..d6f4d536604 100644 --- a/fs/unionfs/xattr.c +++ b/fs/unionfs/xattr.c @@ -57,6 +57,7 @@ ssize_t unionfs_getxattr(struct dentry *dentry, const char *name, void *value, struct dentry *lower_dentry = NULL; int err = -EOPNOTSUPP; + unionfs_read_lock(dentry->d_sb); unionfs_lock_dentry(dentry); if (!__unionfs_d_revalidate_chain(dentry, NULL, 0)) { @@ -71,6 +72,7 @@ ssize_t unionfs_getxattr(struct dentry *dentry, const char *name, void *value, out: unionfs_unlock_dentry(dentry); unionfs_check_dentry(dentry); + unionfs_read_unlock(dentry->d_sb); return err; } @@ -84,6 +86,7 @@ int unionfs_setxattr(struct dentry *dentry, const char *name, struct dentry *lower_dentry = NULL; int err = -EOPNOTSUPP; + unionfs_read_lock(dentry->d_sb); unionfs_lock_dentry(dentry); if (!__unionfs_d_revalidate_chain(dentry, NULL, 0)) { @@ -99,6 +102,7 @@ int unionfs_setxattr(struct dentry *dentry, const char *name, out: unionfs_unlock_dentry(dentry); unionfs_check_dentry(dentry); + unionfs_read_unlock(dentry->d_sb); return err; } @@ -111,6 +115,7 @@ int unionfs_removexattr(struct dentry *dentry, const char *name) struct dentry *lower_dentry = NULL; int err = -EOPNOTSUPP; + unionfs_read_lock(dentry->d_sb); unionfs_lock_dentry(dentry); if (!__unionfs_d_revalidate_chain(dentry, NULL, 0)) { @@ -125,6 +130,7 @@ int unionfs_removexattr(struct dentry *dentry, const char *name) out: unionfs_unlock_dentry(dentry); unionfs_check_dentry(dentry); + unionfs_read_unlock(dentry->d_sb); return err; } @@ -138,6 +144,7 @@ ssize_t unionfs_listxattr(struct dentry *dentry, char *list, size_t size) int err = -EOPNOTSUPP; char *encoded_list = NULL; + unionfs_read_lock(dentry->d_sb); unionfs_lock_dentry(dentry); if (!__unionfs_d_revalidate_chain(dentry, NULL, 0)) { @@ -153,5 +160,6 @@ ssize_t unionfs_listxattr(struct dentry *dentry, char *list, size_t size) out: unionfs_unlock_dentry(dentry); unionfs_check_dentry(dentry); + unionfs_read_unlock(dentry->d_sb); return err; } -- 2.43.0