From patchwork Thu Nov 10 22:44:43 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Amir Goldstein X-Patchwork-Id: 9422191 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 2C4AF6022E for ; Thu, 10 Nov 2016 22:45:29 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 2C82529844 for ; Thu, 10 Nov 2016 22:45:29 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 2130029849; Thu, 10 Nov 2016 22:45:29 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.3 required=2.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, RCVD_IN_DNSWL_HI, RCVD_IN_SORBS_SPAM, T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 181AD29844 for ; Thu, 10 Nov 2016 22:45:27 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756052AbcKJWpY (ORCPT ); Thu, 10 Nov 2016 17:45:24 -0500 Received: from mail-wm0-f65.google.com ([74.125.82.65]:36765 "EHLO mail-wm0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755961AbcKJWpW (ORCPT ); Thu, 10 Nov 2016 17:45:22 -0500 Received: by mail-wm0-f65.google.com with SMTP id m203so4216159wma.3; Thu, 10 Nov 2016 14:45:20 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=rvYDZZWth1pny+LqzgRvB3Fy2QIqRgyrCEInaexI3Zw=; b=gNUMpuBquexyuG8KWF9er2o4f8uIus6ICgA0BzxAU/1hiiirR9KZmkYucb8K6lexti 9AeQBNh6OIzAqdvbFnAQhL8mhYL2aHoHFosOIWy7LnjdcK0aPVKl91v/ISl84zUAklqP uSI5+xjdPX/Z0Px0/T8DRAUJyQd6p2x/EM0I4M4Uk0mx0jlzGyXPc4Bv4bQreQzXnbS1 QX4wCCZNV/llsFn6uQYRHPgdPvPDiAtSwKmCbJ19SM2ecCZ/Re3f7yJ8EJ2YxM6/kwXI RxQ/atgEZqj4LzB+OqlX3b5NikwStVLAm8ijsKVVLEk1+jssZM7Rr1ZV/66Hgd8dXow3 tbeg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=rvYDZZWth1pny+LqzgRvB3Fy2QIqRgyrCEInaexI3Zw=; b=iQSKBXyI+nM41hnjoeBqZYwCKO1sCLCLuvDw3PCH3AbAGrnepxtEC6wQ4ZhoVS5cp/ KwJ/Uajit4foNILhOmQNQJtfS724+pyUakBDYZIF+wapjRjph2U7HlTyBfeB32ysw95S +lgPX3Fv3J6KuAo0meXwGM1jMyBi7klzPkfz1sF6EL9iINF0GISNm37aLYfjkpPn5EH/ 4Qe35kAYJ04IHN+i7FThCH/O/2JnD07ShgB0YG5u7cbK2UqcD5oTDm7Jwy8mW13kS6Rc 7cn2SR4Tm60c3E/1youVVU9iAaSAUepEZo621ZQ2YpKkdFB1VfEkkD8/5nk11zrumYsK +obQ== X-Gm-Message-State: ABUngvfq8meoiRwRPEBCt3o1O+rFXs+s56j+lT96lZwnzhB03JnWEphFAPvAA6/rPqGJjg== X-Received: by 10.194.21.35 with SMTP id s3mr6382124wje.192.1478817919281; Thu, 10 Nov 2016 14:45:19 -0800 (PST) Received: from amir-VirtualBox.Home (bzq-79-177-89-54.red.bezeqint.net. [79.177.89.54]) by smtp.gmail.com with ESMTPSA id n72sm15444310wmd.11.2016.11.10.14.45.17 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Thu, 10 Nov 2016 14:45:18 -0800 (PST) From: Amir Goldstein To: Miklos Szeredi Cc: Konstantin Khlebnikov , Al Viro , linux-unionfs@vger.kernel.org, linux-fsdevel@vger.kernel.org Subject: [RFC][PATH 4/4] ovl: relax lock_rename when moving files between work and upper dir Date: Fri, 11 Nov 2016 00:44:43 +0200 Message-Id: <1478817883-27662-5-git-send-email-amir73il@gmail.com> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1478817883-27662-1-git-send-email-amir73il@gmail.com> References: <1478817883-27662-1-git-send-email-amir73il@gmail.com> Sender: linux-fsdevel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP If work dir and upper dir have a common parent, then it is safe to move files between them without taking the super block s_vfs_rename_mutex. Since both upper dir and work dir (as well as $workdir/work) are delete locked on overlay mount, they cannot be moved. Therefore, if upper and work dir have the same parent at mount time, this cannot change thougout the time that overlay is mounted. As a result, moving files between work dir and upper dir cannot change the tree topology and cannot result in loops. For the same reason, it is safe to take the inode locks on work+upper before moving files, in any consistent order, as they cannot be descendants of each other. Implement a pair of helpers {unlock,lock}_rename_safe() to be used instead of the full blown {unlock,lock}_rename() pair in the case of work/upper dir having a common parent. Signed-off-by: Amir Goldstein --- fs/namei.c | 20 ++++++++++++++++++++ fs/overlayfs/copy_up.c | 7 +++---- fs/overlayfs/dir.c | 25 +++++++++++++++++-------- fs/overlayfs/overlayfs.h | 2 ++ include/linux/namei.h | 2 ++ 5 files changed, 44 insertions(+), 12 deletions(-) diff --git a/fs/namei.c b/fs/namei.c index 6040d1e..8e3be1b 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -2867,6 +2867,26 @@ void unlock_rename(struct dentry *p1, struct dentry *p2) } EXPORT_SYMBOL(unlock_rename); +/* + * p1 and p2 are delete locked and so are their parents, all the way + * to a common ancestor and they are not decendant of each other, + * so it is safe to move children between them without holding + * s_vfs_rename_mutex and it is safe to lock them in any order. + */ +void lock_rename_safe(struct dentry *p1, struct dentry *p2) +{ + inode_lock_nested(p1->d_inode, I_MUTEX_PARENT); + inode_lock_nested(p2->d_inode, I_MUTEX_PARENT2); +} +EXPORT_SYMBOL(lock_rename_safe); + +void unlock_rename_safe(struct dentry *p1, struct dentry *p2) +{ + inode_unlock(p1->d_inode); + inode_unlock(p2->d_inode); +} +EXPORT_SYMBOL(unlock_rename_safe); + int vfs_create(struct inode *dir, struct dentry *dentry, umode_t mode, bool want_excl) { diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c index f18c1a6..02b5386 100644 --- a/fs/overlayfs/copy_up.c +++ b/fs/overlayfs/copy_up.c @@ -367,9 +367,8 @@ int ovl_copy_up_one(struct dentry *parent, struct dentry *dentry, return PTR_ERR(link); } - err = -EIO; - if (lock_rename(workdir, upperdir) != NULL) { - pr_err("overlayfs: failed to lock workdir+upperdir\n"); + err = ovl_lock_rename_workdir(workdir, upperdir); + if (err) { goto out_unlock; } upperdentry = ovl_dentry_upper(dentry); @@ -386,7 +385,7 @@ int ovl_copy_up_one(struct dentry *parent, struct dentry *dentry, ovl_set_timestamps(upperdir, &pstat); } out_unlock: - unlock_rename(workdir, upperdir); + ovl_unlock_rename_workdir(workdir, upperdir); do_delayed_call(&done); return err; diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c index f24b6b9..f0e9b8f 100644 --- a/fs/overlayfs/dir.c +++ b/fs/overlayfs/dir.c @@ -202,15 +202,16 @@ static int ovl_create_upper(struct dentry *dentry, struct inode *inode, return err; } -static int ovl_lock_rename_workdir(struct dentry *workdir, - struct dentry *upperdir) +int ovl_lock_rename_workdir(struct dentry *workdir, struct dentry *upperdir) { /* Workdir should not be the same as upperdir */ if (workdir == upperdir) goto err; - /* Workdir should not be subdir of upperdir and vice versa */ - if (lock_rename(workdir, upperdir) != NULL) + /* FIXME: check this once in fill_super */ + if (upperdir->d_parent == workdir->d_parent->d_parent) + lock_rename_safe(workdir, upperdir); + else if (lock_rename(workdir, upperdir) != NULL) goto err_unlock; return 0; @@ -222,6 +223,14 @@ static int ovl_lock_rename_workdir(struct dentry *workdir, return -EIO; } +void ovl_unlock_rename_workdir(struct dentry *workdir, struct dentry *upperdir) +{ + if (upperdir->d_parent == workdir->d_parent->d_parent) + unlock_rename_safe(workdir, upperdir); + else + unlock_rename(workdir, upperdir); +} + static struct dentry *ovl_clear_empty(struct dentry *dentry, struct list_head *list) { @@ -283,7 +292,7 @@ static struct dentry *ovl_clear_empty(struct dentry *dentry, ovl_cleanup_whiteouts(upper, list); ovl_cleanup(wdir, upper); - unlock_rename(workdir, upperdir); + ovl_unlock_rename_workdir(workdir, upperdir); /* dentry's upper doesn't match now, get rid of it */ d_drop(dentry); @@ -295,7 +304,7 @@ static struct dentry *ovl_clear_empty(struct dentry *dentry, out_dput: dput(opaquedir); out_unlock: - unlock_rename(workdir, upperdir); + ovl_unlock_rename_workdir(workdir, upperdir); out: return ERR_PTR(err); } @@ -450,7 +459,7 @@ static int ovl_create_over_whiteout(struct dentry *dentry, struct inode *inode, out_dput: dput(newdentry); out_unlock: - unlock_rename(workdir, upperdir); + ovl_unlock_rename_workdir(workdir, upperdir); out: if (!hardlink) { posix_acl_release(acl); @@ -657,7 +666,7 @@ static int ovl_remove_and_whiteout(struct dentry *dentry, bool is_dir) out_dput_upper: dput(upper); out_unlock: - unlock_rename(workdir, upperdir); + ovl_unlock_rename_workdir(workdir, upperdir); out_dput: dput(opaquedir); out: diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h index f6e4d35..63ca647 100644 --- a/fs/overlayfs/overlayfs.h +++ b/fs/overlayfs/overlayfs.h @@ -211,6 +211,8 @@ int ovl_create_real(struct inode *dir, struct dentry *newdentry, struct kstat *stat, const char *link, struct dentry *hardlink, bool debug); void ovl_cleanup(struct inode *dir, struct dentry *dentry); +int ovl_lock_rename_workdir(struct dentry *workdir, struct dentry *upperdir); +void ovl_unlock_rename_workdir(struct dentry *workdir, struct dentry *upperdir); /* copy_up.c */ int ovl_copy_up(struct dentry *dentry); diff --git a/include/linux/namei.h b/include/linux/namei.h index f29abda..31e3f4f 100644 --- a/include/linux/namei.h +++ b/include/linux/namei.h @@ -88,6 +88,8 @@ extern int follow_up(struct path *); extern struct dentry *lock_rename(struct dentry *, struct dentry *); extern void unlock_rename(struct dentry *, struct dentry *); +extern void lock_rename_safe(struct dentry *, struct dentry *); +extern void unlock_rename_safe(struct dentry *, struct dentry *); extern void nd_jump_link(struct path *path);