From: Rachita Kothiyal Date: Mon, 31 Mar 2008 21:44:48 +0000 (-0400) Subject: Unionfs: lock parents' branch configuration fixes X-Git-Url: https://git.fsl.cs.sunysb.edu/?a=commitdiff_plain;h=a20be893c31b300a179604550a427bcfd3bdb7a9;p=unionfs-odf.git Unionfs: lock parents' branch configuration fixes Ensure that we lock the branch configuration of parent and child dentries in operations which need it, and in the right order. Signed-off-by: Erez Zadok --- diff --git a/fs/unionfs/commonfops.c b/fs/unionfs/commonfops.c index 68023164de..15f4a0ac2d 100644 --- a/fs/unionfs/commonfops.c +++ b/fs/unionfs/commonfops.c @@ -367,8 +367,9 @@ out: * Revalidate the struct file * @file: file to revalidate * @willwrite: true if caller may cause changes to the file; false otherwise. + * Caller must lock/unlock dentry's branch configuration. */ -int unionfs_file_revalidate(struct file *file, bool willwrite) +int unionfs_file_revalidate_locked(struct file *file, bool willwrite) { struct super_block *sb; struct dentry *dentry; @@ -378,7 +379,6 @@ int unionfs_file_revalidate(struct file *file, bool willwrite) int err = 0; dentry = file->f_path.dentry; - unionfs_lock_dentry(dentry, UNIONFS_DMUTEX_CHILD); sb = dentry->d_sb; /* @@ -483,7 +483,17 @@ out: out_nofree: if (!err) unionfs_check_file(file); - unionfs_unlock_dentry(dentry); + return err; +} + +int unionfs_file_revalidate(struct file *file, bool willwrite) +{ + int err; + + unionfs_lock_dentry(file->f_path.dentry, UNIONFS_DMUTEX_CHILD); + err = unionfs_file_revalidate_locked(file, willwrite); + unionfs_unlock_dentry(file->f_path.dentry); + return err; } @@ -606,9 +616,18 @@ int unionfs_open(struct inode *inode, struct file *file) struct dentry *dentry = file->f_path.dentry; int bindex = 0, bstart = 0, bend = 0; int size; + int valid = 0; unionfs_read_lock(inode->i_sb, UNIONFS_SMUTEX_PARENT); unionfs_lock_dentry(dentry, UNIONFS_DMUTEX_CHILD); + if (dentry != dentry->d_parent) + unionfs_lock_dentry(dentry->d_parent, UNIONFS_DMUTEX_PARENT); + + valid = __unionfs_d_revalidate_chain(dentry->d_parent, NULL, false); + if (unlikely(!valid)) { + err = -ESTALE; + goto out_nofree; + } /* * With nfs exporting we cannot get a disconnected dentry here, @@ -682,6 +701,8 @@ out_nofree: unionfs_check_file(file); unionfs_check_inode(inode); } + if (dentry != dentry->d_parent) + unionfs_unlock_dentry(dentry->d_parent); unionfs_unlock_dentry(dentry); unionfs_read_unlock(inode->i_sb); return err; @@ -878,8 +899,9 @@ int unionfs_flush(struct file *file, fl_owner_t id) int bindex, bstart, bend; unionfs_read_lock(dentry->d_sb, UNIONFS_SMUTEX_PARENT); + unionfs_lock_dentry(dentry, UNIONFS_DMUTEX_CHILD); - err = unionfs_file_revalidate(file, true); + err = unionfs_file_revalidate_locked(file, true); if (unlikely(err)) goto out; unionfs_check_file(file); @@ -905,6 +927,7 @@ int unionfs_flush(struct file *file, fl_owner_t id) out: unionfs_check_file(file); + unionfs_unlock_dentry(file->f_path.dentry); unionfs_read_unlock(dentry->d_sb); return err; } diff --git a/fs/unionfs/dentry.c b/fs/unionfs/dentry.c index 418945dd29..5a7fca4eb2 100644 --- a/fs/unionfs/dentry.c +++ b/fs/unionfs/dentry.c @@ -381,6 +381,7 @@ bool __unionfs_d_revalidate_chain(struct dentry *dentry, struct nameidata *nd, chain_len = 0; sbgen = atomic_read(&UNIONFS_SB(dentry->d_sb)->generation); dtmp = dentry->d_parent; + verify_locked(dentry); if (dentry != dtmp) unionfs_lock_dentry(dtmp, UNIONFS_DMUTEX_REVAL_PARENT); dgen = atomic_read(&UNIONFS_D(dtmp)->generation); @@ -471,7 +472,7 @@ bool __unionfs_d_revalidate_chain(struct dentry *dentry, struct nameidata *nd, out_this: /* finally, lock this dentry and revalidate it */ - verify_locked(dentry); + verify_locked(dentry); /* verify child is locked */ if (dentry != dentry->d_parent) unionfs_lock_dentry(dentry->d_parent, UNIONFS_DMUTEX_REVAL_PARENT); @@ -509,24 +510,20 @@ static int unionfs_d_revalidate(struct dentry *dentry, struct nameidata *nd) return err; } -/* - * At this point no one can reference this dentry, so we don't have to be - * careful about concurrent access. - */ static void unionfs_d_release(struct dentry *dentry) { int bindex, bstart, bend; unionfs_read_lock(dentry->d_sb, UNIONFS_SMUTEX_CHILD); + /* must lock our branch configuration here */ + unionfs_lock_dentry(dentry, UNIONFS_DMUTEX_CHILD); unionfs_check_dentry(dentry); /* this could be a negative dentry, so check first */ - if (unlikely(!UNIONFS_D(dentry))) { - printk(KERN_ERR "unionfs: dentry without private data: %.*s\n", - dentry->d_name.len, dentry->d_name.name); - goto out; - } else if (dbstart(dentry) < 0) - goto out_free; /* due to a (normal) failed lookup */ + if (unlikely(!UNIONFS_D(dentry) || dbstart(dentry) < 0)) { + unionfs_unlock_dentry(dentry); + goto out; /* due to a (normal) failed lookup */ + } /* Release all the lower dentries */ bstart = dbstart(dentry); @@ -544,11 +541,10 @@ static void unionfs_d_release(struct dentry *dentry) kfree(UNIONFS_D(dentry)->lower_paths); UNIONFS_D(dentry)->lower_paths = NULL; -out_free: - /* No need to unlock it, because it is disappeared. */ - free_dentry_private_data(dentry); + unionfs_unlock_dentry(dentry); out: + free_dentry_private_data(dentry); unionfs_read_unlock(dentry->d_sb); return; } @@ -563,6 +559,7 @@ static void unionfs_d_iput(struct dentry *dentry, struct inode *inode) BUG_ON(!dentry); unionfs_read_lock(dentry->d_sb, UNIONFS_SMUTEX_CHILD); + unionfs_lock_dentry(dentry, UNIONFS_DMUTEX_CHILD); if (!UNIONFS_D(dentry) || dbstart(dentry) < 0) goto drop_lower_inodes; @@ -592,6 +589,7 @@ drop_lower_inodes: iput(inode); + unionfs_unlock_dentry(dentry); unionfs_read_unlock(dentry->d_sb); } diff --git a/fs/unionfs/inode.c b/fs/unionfs/inode.c index c456f30010..f800d43edf 100644 --- a/fs/unionfs/inode.c +++ b/fs/unionfs/inode.c @@ -110,16 +110,16 @@ static int unionfs_create(struct inode *parent, struct dentry *dentry, struct nameidata lower_nd; unionfs_read_lock(dentry->d_sb, UNIONFS_SMUTEX_CHILD); + unionfs_lock_dentry(dentry, UNIONFS_DMUTEX_CHILD); unionfs_lock_dentry(dentry->d_parent, UNIONFS_DMUTEX_PARENT); + valid = __unionfs_d_revalidate_chain(dentry->d_parent, nd, false); - unionfs_unlock_dentry(dentry->d_parent); if (unlikely(!valid)) { err = -ESTALE; /* same as what real_lookup does */ goto out; } - unionfs_lock_dentry(dentry, UNIONFS_DMUTEX_CHILD); - valid = __unionfs_d_revalidate_chain(dentry, nd, false); + valid = __unionfs_d_revalidate_one_locked(dentry, nd, false); /* * It's only a bug if this dentry was not negative and couldn't be * revalidated (shouldn't happen). @@ -170,14 +170,13 @@ static int unionfs_create(struct inode *parent, struct dentry *dentry, } out: - if (!err) - unionfs_postcopyup_setmnt(dentry); - - unionfs_check_inode(parent); if (!err) { + unionfs_postcopyup_setmnt(dentry); + unionfs_check_inode(parent); unionfs_check_dentry(dentry); unionfs_check_nd(nd); } + unionfs_unlock_dentry(dentry->d_parent); unionfs_unlock_dentry(dentry); unionfs_read_unlock(dentry->d_sb); return err; @@ -197,7 +196,7 @@ static struct dentry *unionfs_lookup(struct inode *parent, unionfs_read_lock(dentry->d_sb, UNIONFS_SMUTEX_CHILD); if (dentry != dentry->d_parent) - unionfs_lock_dentry(dentry->d_parent, UNIONFS_DMUTEX_PARENT); + unionfs_lock_dentry(dentry->d_parent, UNIONFS_DMUTEX_ROOT); /* save the dentry & vfsmnt from namei */ if (nd) { @@ -219,6 +218,7 @@ static struct dentry *unionfs_lookup(struct inode *parent, if (!IS_ERR(ret)) { if (ret) dentry = ret; + unionfs_copy_attr_times(dentry->d_inode); /* parent times may have changed */ unionfs_copy_attr_times(dentry->d_parent->d_inode); } @@ -398,12 +398,20 @@ static int unionfs_symlink(struct inode *dir, struct dentry *dentry, struct dentry *lower_dentry = NULL; struct dentry *lower_dir_dentry = NULL; umode_t mode; + int valid; unionfs_read_lock(dentry->d_sb, UNIONFS_SMUTEX_CHILD); unionfs_lock_dentry(dentry, UNIONFS_DMUTEX_CHILD); + unionfs_lock_dentry(dentry->d_parent, UNIONFS_DMUTEX_PARENT); + + valid = __unionfs_d_revalidate_chain(dentry->d_parent, NULL, false); + if (unlikely(!valid)) { + err = -ESTALE; + goto out; + } if (unlikely(dentry->d_inode && - !__unionfs_d_revalidate_chain(dentry, NULL, false))) { + !__unionfs_d_revalidate_one_locked(dentry, NULL, false))) { err = -ESTALE; goto out; } @@ -447,12 +455,12 @@ static int unionfs_symlink(struct inode *dir, struct dentry *dentry, d_drop(dentry); out: - if (!err) + if (!err) { unionfs_postcopyup_setmnt(dentry); - - unionfs_check_inode(dir); - if (!err) + unionfs_check_inode(dir); unionfs_check_dentry(dentry); + } + unionfs_unlock_dentry(dentry->d_parent); unionfs_unlock_dentry(dentry); unionfs_read_unlock(dentry->d_sb); @@ -466,12 +474,20 @@ static int unionfs_mkdir(struct inode *parent, struct dentry *dentry, int mode) struct dentry *lower_parent_dentry = NULL; char *name = NULL; int i, bend, bindex; + int valid; unionfs_read_lock(dentry->d_sb, UNIONFS_SMUTEX_CHILD); unionfs_lock_dentry(dentry, UNIONFS_DMUTEX_CHILD); + unionfs_lock_dentry(dentry->d_parent, UNIONFS_DMUTEX_PARENT); + + valid = __unionfs_d_revalidate_chain(dentry->d_parent, NULL, false); + if (unlikely(!valid)) { + err = -ESTALE; /* same as what real_lookup does */ + goto out; + } if (unlikely(dentry->d_inode && - !__unionfs_d_revalidate_chain(dentry, NULL, false))) { + !__unionfs_d_revalidate_one_locked(dentry, NULL, false))) { err = -ESTALE; goto out; } @@ -563,6 +579,7 @@ out: } unionfs_check_inode(parent); unionfs_check_dentry(dentry); + unionfs_unlock_dentry(dentry->d_parent); unionfs_unlock_dentry(dentry); unionfs_read_unlock(dentry->d_sb); @@ -575,12 +592,20 @@ static int unionfs_mknod(struct inode *dir, struct dentry *dentry, int mode, int err = 0; struct dentry *lower_dentry = NULL; struct dentry *lower_parent_dentry = NULL; + int valid; unionfs_read_lock(dentry->d_sb, UNIONFS_SMUTEX_CHILD); unionfs_lock_dentry(dentry, UNIONFS_DMUTEX_CHILD); + unionfs_lock_dentry(dentry->d_parent, UNIONFS_DMUTEX_PARENT); + + valid = __unionfs_d_revalidate_chain(dentry->d_parent, NULL, false); + if (unlikely(!valid)) { + err = -ESTALE; /* same as what real_lookup does */ + goto out; + } if (unlikely(dentry->d_inode && - !__unionfs_d_revalidate_chain(dentry, NULL, false))) { + !__unionfs_d_revalidate_one_locked(dentry, NULL, false))) { err = -ESTALE; goto out; } @@ -630,12 +655,12 @@ out: if (!dentry->d_inode) d_drop(dentry); - if (!err) + if (!err) { unionfs_postcopyup_setmnt(dentry); - - unionfs_check_inode(dir); - if (!err) + unionfs_check_inode(dir); unionfs_check_dentry(dentry); + } + unionfs_unlock_dentry(dentry->d_parent); unionfs_unlock_dentry(dentry); unionfs_read_unlock(dentry->d_sb); diff --git a/fs/unionfs/union.h b/fs/unionfs/union.h index 97d73c1a5a..e826427cb1 100644 --- a/fs/unionfs/union.h +++ b/fs/unionfs/union.h @@ -306,11 +306,11 @@ static inline void unionfs_double_lock_dentry(struct dentry *d1, { BUG_ON(d1 == d2); if (d1 < d2) { - unionfs_lock_dentry(d1, UNIONFS_DMUTEX_PARENT); unionfs_lock_dentry(d2, UNIONFS_DMUTEX_CHILD); + unionfs_lock_dentry(d1, UNIONFS_DMUTEX_PARENT); } else { - unionfs_lock_dentry(d2, UNIONFS_DMUTEX_PARENT); unionfs_lock_dentry(d1, UNIONFS_DMUTEX_CHILD); + unionfs_lock_dentry(d2, UNIONFS_DMUTEX_PARENT); } } @@ -361,6 +361,7 @@ extern int unionfs_setlk(struct file *file, int cmd, struct file_lock *fl); extern int unionfs_getlk(struct file *file, struct file_lock *fl); /* Common file operations. */ +extern int unionfs_file_revalidate_locked(struct file *file, bool willwrite); extern int unionfs_file_revalidate(struct file *file, bool willwrite); extern int unionfs_open(struct inode *inode, struct file *file); extern int unionfs_file_release(struct inode *inode, struct file *file); diff --git a/fs/unionfs/unlink.c b/fs/unionfs/unlink.c index d7203924de..c77f4308f8 100644 --- a/fs/unionfs/unlink.c +++ b/fs/unionfs/unlink.c @@ -102,12 +102,20 @@ int unionfs_unlink(struct inode *dir, struct dentry *dentry) { int err = 0; struct inode *inode = dentry->d_inode; + int valid; BUG_ON(S_ISDIR(inode->i_mode)); unionfs_read_lock(dentry->d_sb, UNIONFS_SMUTEX_CHILD); unionfs_lock_dentry(dentry, UNIONFS_DMUTEX_CHILD); + unionfs_lock_dentry(dentry->d_parent, UNIONFS_DMUTEX_PARENT); - if (unlikely(!__unionfs_d_revalidate_chain(dentry, NULL, false))) { + valid = __unionfs_d_revalidate_chain(dentry->d_parent, NULL, false); + if (unlikely(!valid)) { + err = -ESTALE; + goto out; + } + valid = __unionfs_d_revalidate_one_locked(dentry, NULL, false); + if (unlikely(!valid)) { err = -ESTALE; goto out; } @@ -144,6 +152,7 @@ out: /* update parent dir times if unlink/whiteout succeeded */ unionfs_copy_attr_times(dir); } + unionfs_unlock_dentry(dentry->d_parent); unionfs_unlock_dentry(dentry); unionfs_read_unlock(dentry->d_sb); return err;