Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756838AbXL1Uxl (ORCPT ); Fri, 28 Dec 2007 15:53:41 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1753189AbXL1Uor (ORCPT ); Fri, 28 Dec 2007 15:44:47 -0500 Received: from filer.fsl.cs.sunysb.edu ([130.245.126.2]:37211 "EHLO filer.fsl.cs.sunysb.edu" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753846AbXL1UoY (ORCPT ); Fri, 28 Dec 2007 15:44:24 -0500 From: Erez Zadok To: akpm@linux-foundation.org Cc: linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org, viro@ftp.linux.org.uk, hch@infradead.org, Erez Zadok Subject: [PATCH 08/30] Unionfs: create new symlinks only in first branch Date: Fri, 28 Dec 2007 15:42:42 -0500 Message-Id: <1198874589904-git-send-email-ezk@cs.sunysb.edu> X-Mailer: git-send-email 1.5.2.2 X-MailKey: Erez_Zadok In-Reply-To: <11988745841003-git-send-email-ezk@cs.sunysb.edu> References: <11988745841003-git-send-email-ezk@cs.sunysb.edu> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 8153 Lines: 282 When creating a new symlink, always create it in the first branch, which is always writeable, not in the branch which may have a whiteout in it. This makes the policy for the creation of new symlinks consistent with that of new files/directories, as well as improves efficiency a bit. Signed-off-by: Erez Zadok --- fs/unionfs/inode.c | 210 +++++++++++++++++++++++---------------------------- 1 files changed, 95 insertions(+), 115 deletions(-) diff --git a/fs/unionfs/inode.c b/fs/unionfs/inode.c index 8076d0b..78cdfa2 100644 --- a/fs/unionfs/inode.c +++ b/fs/unionfs/inode.c @@ -368,16 +368,16 @@ out: return err; } -static int unionfs_symlink(struct inode *dir, struct dentry *dentry, +static int unionfs_symlink(struct inode *parent, struct dentry *dentry, const char *symname) { int err = 0; struct dentry *lower_dentry = NULL; - struct dentry *whiteout_dentry = NULL; - struct dentry *lower_dir_dentry = NULL; - umode_t mode; - int bindex = 0, bstart; + struct dentry *wh_dentry = NULL; + struct dentry *lower_parent_dentry = NULL; char *name = NULL; + int valid = 0; + umode_t mode; unionfs_read_lock(dentry->d_sb); unionfs_lock_dentry(dentry); @@ -388,147 +388,127 @@ static int unionfs_symlink(struct inode *dir, struct dentry *dentry, goto out; } - /* We start out in the leftmost branch. */ - bstart = dbstart(dentry); - - lower_dentry = unionfs_lower_dentry(dentry); - /* - * check if whiteout exists in this branch, i.e. lookup .wh.foo - * first. If present, delete it + * It's only a bug if this dentry was not negative and couldn't be + * revalidated (shouldn't happen). */ - name = alloc_whname(dentry->d_name.name, dentry->d_name.len); - if (unlikely(IS_ERR(name))) { - err = PTR_ERR(name); - goto out; - } + BUG_ON(!valid && dentry->d_inode); - whiteout_dentry = - lookup_one_len(name, lower_dentry->d_parent, - dentry->d_name.len + UNIONFS_WHLEN); - if (IS_ERR(whiteout_dentry)) { - err = PTR_ERR(whiteout_dentry); + /* + * We shouldn't create things in a read-only branch; this check is a + * bit redundant as we don't allow branch 0 to be read-only at the + * moment + */ + err = is_robranch_super(dentry->d_sb, 0); + if (err) { + err = -EROFS; goto out; } - if (!whiteout_dentry->d_inode) { - dput(whiteout_dentry); - whiteout_dentry = NULL; - } else { + /* + * We _always_ create on branch 0 + */ + lower_dentry = unionfs_lower_dentry_idx(dentry, 0); + if (lower_dentry) { /* - * found a .wh.foo entry, unlink it and then call - * vfs_symlink(). + * check if whiteout exists in this branch, i.e. lookup .wh.foo + * first. */ - lower_dir_dentry = lock_parent(whiteout_dentry); - - err = is_robranch_super(dentry->d_sb, bstart); - if (!err) - err = vfs_unlink(lower_dir_dentry->d_inode, - whiteout_dentry); - dput(whiteout_dentry); - - fsstack_copy_attr_times(dir, lower_dir_dentry->d_inode); - /* propagate number of hard-links */ - dir->i_nlink = unionfs_get_nlinks(dir); + name = alloc_whname(dentry->d_name.name, dentry->d_name.len); + if (unlikely(IS_ERR(name))) { + err = PTR_ERR(name); + goto out; + } - unlock_dir(lower_dir_dentry); + wh_dentry = lookup_one_len(name, lower_dentry->d_parent, + dentry->d_name.len + UNIONFS_WHLEN); + if (IS_ERR(wh_dentry)) { + err = PTR_ERR(wh_dentry); + wh_dentry = NULL; + goto out; + } - if (err) { - /* exit if the error returned was NOT -EROFS */ - if (!IS_COPYUP_ERR(err)) - goto out; + if (wh_dentry->d_inode) { /* - * should now try to create symlink in the another - * branch. + * .wh.foo has been found, so let's unlink it */ - bstart--; - } - } + struct dentry *lower_dir_dentry; + + lower_dir_dentry = lock_parent(wh_dentry); + err = vfs_unlink(lower_dir_dentry->d_inode, wh_dentry); + unlock_dir(lower_dir_dentry); - /* - * deleted whiteout if it was present, now do a normal vfs_symlink() - * with possible recursive directory creation - */ - for (bindex = bstart; bindex >= 0; bindex--) { - lower_dentry = unionfs_lower_dentry_idx(dentry, bindex); - if (!lower_dentry) { /* - * if lower_dentry is NULL, create the entire - * dentry directory structure in branch 'bindex'. - * lower_dentry will NOT be null when bindex == - * bstart because lookup passed as a negative - * unionfs dentry pointing to a lone negative - * underlying dentry + * Whiteouts are special files and should be deleted + * no matter what (as if they never existed), in + * order to allow this create operation to succeed. + * This is especially important in sticky + * directories: a whiteout may have been created by + * one user, but the newly created file may be + * created by another user. Therefore, in order to + * maintain Unix semantics, if the vfs_unlink above + * ailed, then we have to try to directly unlink the + * whiteout. Note: in the ODF version of unionfs, + * whiteout are handled much more cleanly. */ - lower_dentry = create_parents(dir, dentry, - dentry->d_name.name, - bindex); - if (!lower_dentry || IS_ERR(lower_dentry)) { - if (IS_ERR(lower_dentry)) - err = PTR_ERR(lower_dentry); - if (!IS_COPYUP_ERR(err)) - printk(KERN_ERR - "unionfs: create_parents for " - "symlink failed: bindex=%d " - "err=%d\n", bindex, err); - continue; + if (err == -EPERM) { + struct inode *inode = lower_dir_dentry->d_inode; + err = inode->i_op->unlink(inode, wh_dentry); + } + if (err) { + printk(KERN_ERR "unionfs: symlink: could not " + "unlink whiteout, err = %d\n", err); + goto out; } } + } else { + /* + * if lower_dentry is NULL, create the entire + * dentry directory structure in branch 0. + */ + lower_dentry = create_parents(parent, dentry, + dentry->d_name.name, 0); + if (IS_ERR(lower_dentry)) { + err = PTR_ERR(lower_dentry); + goto out; + } + } - lower_dir_dentry = lock_parent(lower_dentry); + lower_parent_dentry = lock_parent(lower_dentry); + if (IS_ERR(lower_parent_dentry)) { + err = PTR_ERR(lower_parent_dentry); + goto out; + } - err = is_robranch_super(dentry->d_sb, bindex); + mode = S_IALLUGO; + err = vfs_symlink(lower_parent_dentry->d_inode, lower_dentry, + symname, mode); + if (!err) { + err = PTR_ERR(unionfs_interpose(dentry, parent->i_sb, 0)); if (!err) { - mode = S_IALLUGO; - err = vfs_symlink(lower_dir_dentry->d_inode, - lower_dentry, symname, mode); - } - unlock_dir(lower_dir_dentry); - - if (err || !lower_dentry->d_inode) { - /* - * break out of for loop if error returned was NOT - * -EROFS. - */ - if (!IS_COPYUP_ERR(err)) - break; - } else { - /* - * Only INTERPOSE_LOOKUP can return a value other - * than 0 on err. - */ - err = PTR_ERR(unionfs_interpose(dentry, - dir->i_sb, 0)); - if (!err) { - fsstack_copy_attr_times(dir, - lower_dir_dentry-> - d_inode); - fsstack_copy_inode_size(dir, - lower_dir_dentry-> - d_inode); - /* - * update number of links on parent - * directory. - */ - dir->i_nlink = unionfs_get_nlinks(dir); - } - break; + unionfs_copy_attr_times(parent); + fsstack_copy_inode_size(parent, + lower_parent_dentry->d_inode); + /* update no. of links on parent directory */ + parent->i_nlink = unionfs_get_nlinks(parent); } } -out: - if (!dentry->d_inode) - d_drop(dentry); + unlock_dir(lower_parent_dentry); +out: + dput(wh_dentry); kfree(name); + if (!err) unionfs_postcopyup_setmnt(dentry); - unionfs_check_inode(dir); + unionfs_check_inode(parent); + if (!err) + unionfs_check_dentry(dentry->d_parent); unionfs_check_dentry(dentry); unionfs_unlock_dentry(dentry); unionfs_read_unlock(dentry->d_sb); - return err; } -- 1.5.2.2 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/