From patchwork Thu Apr 18 18:41:00 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: hubcap@kernel.org X-Patchwork-Id: 10907859 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 0FC151515 for ; Thu, 18 Apr 2019 18:42:58 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id E932F285DB for ; Thu, 18 Apr 2019 18:42:57 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id DC6EC28D5F; Thu, 18 Apr 2019 18:42:57 +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=-8.0 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI,RCVD_IN_DNSWL_HI 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 BDB04285DB for ; Thu, 18 Apr 2019 18:42:56 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2390001AbfDRSmz (ORCPT ); Thu, 18 Apr 2019 14:42:55 -0400 Received: from mail.kernel.org ([198.145.29.99]:60694 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2389877AbfDRSmx (ORCPT ); Thu, 18 Apr 2019 14:42:53 -0400 Received: from localhost.localdomain (unknown [24.213.116.99]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 445532064A; Thu, 18 Apr 2019 18:42:51 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1555612971; bh=DJOD7TkA8l10xl8UVz7NtaUSC5cpTI5tm/fyb2OVpO8=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=epiy9Zr1aJStiSP4kBgx9+ouMZJJwLCpaX6z/UfdFmFGdkRHpXhqC4sZe8cBkUoGw HhqBP+j79DgxYOA12HhvC+LqG+ZG9LtKNUD7IiMPZeu5oCJ2qer2JTj7VXMGM35Yrm VAHW40eNnT5e0pC/yn9d3Iu1Uhy1Jn+UjiXMPmgY= From: hubcap@kernel.org To: linux-fsdevel@vger.kernel.org, christoph@lameter.com Cc: Martin Brandenburg , Mike Marshall Subject: [PATCH 08/22] orangefs: reorganize setattr functions to track attribute changes Date: Thu, 18 Apr 2019 14:41:00 -0400 Message-Id: <20190418184113.9152-9-hubcap@kernel.org> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20190418184113.9152-1-hubcap@kernel.org> References: <20190418184113.9152-1-hubcap@kernel.org> MIME-Version: 1.0 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 From: Martin Brandenburg OrangeFS accepts a mask indicating which attributes were changed. The kernel must not set any bits except those that were actually changed. The kernel must set the uid/gid of the request to the actual uid/gid responsible for the change. Code path for notify_change initiated setattrs is orangefs_setattr(dentry, iattr) -> __orangefs_setattr(inode, iattr) In kernel changes are initiated by calling __orangefs_setattr. Code path for writeback is orangefs_write_inode -> orangefs_inode_setattr attr_valid and attr_uid and attr_gid change together under i_lock. I_DIRTY changes separately. __orangefs_setattr lock if needs to be cleaned first, unlock and retry set attr_valid copy data in unlock mark_inode_dirty orangefs_inode_setattr lock copy attributes out unlock clear getattr_time # __writeback_single_inode clears dirty orangefs_inode_getattr # possible to get here with attr_valid set and not dirty lock if getattr_time ok or attr_valid set, unlock and return unlock do server operation # another thread may getattr or setattr, so check for that lock if getattr_time ok or attr_valid, unlock and return else, copy in update getattr_time unlock Signed-off-by: Martin Brandenburg Signed-off-by: Mike Marshall --- fs/orangefs/acl.c | 4 +- fs/orangefs/inode.c | 76 ++++++++++++++++++----- fs/orangefs/namei.c | 35 +++++------ fs/orangefs/orangefs-kernel.h | 8 ++- fs/orangefs/orangefs-utils.c | 114 +++++++++++++--------------------- fs/orangefs/super.c | 11 +--- 6 files changed, 129 insertions(+), 119 deletions(-) diff --git a/fs/orangefs/acl.c b/fs/orangefs/acl.c index 72d2ff17d27b..eced272a3c57 100644 --- a/fs/orangefs/acl.c +++ b/fs/orangefs/acl.c @@ -142,7 +142,7 @@ int orangefs_set_acl(struct inode *inode, struct posix_acl *acl, int type) rc = __orangefs_set_acl(inode, acl, type); } else { iattr.ia_valid = ATTR_MODE; - rc = orangefs_inode_setattr(inode, &iattr); + rc = __orangefs_setattr(inode, &iattr); } return rc; @@ -185,7 +185,7 @@ int orangefs_init_acl(struct inode *inode, struct inode *dir) inode->i_mode = mode; iattr.ia_mode = mode; iattr.ia_valid |= ATTR_MODE; - orangefs_inode_setattr(inode, &iattr); + __orangefs_setattr(inode, &iattr); } return error; diff --git a/fs/orangefs/inode.c b/fs/orangefs/inode.c index 2e630c1f7ae2..2708bf8af9cf 100644 --- a/fs/orangefs/inode.c +++ b/fs/orangefs/inode.c @@ -1,6 +1,7 @@ // SPDX-License-Identifier: GPL-2.0 /* * (C) 2001 Clemson University and The University of Chicago + * Copyright 2018 Omnibond Systems, L.L.C. * * See COPYING in top-level directory. */ @@ -202,22 +203,31 @@ static int orangefs_setattr_size(struct inode *inode, struct iattr *iattr) return ret; } -/* - * Change attributes of an object referenced by dentry. - */ -int orangefs_setattr(struct dentry *dentry, struct iattr *iattr) +int __orangefs_setattr(struct inode *inode, struct iattr *iattr) { - struct inode *inode = dentry->d_inode; int ret; - gossip_debug(GOSSIP_INODE_DEBUG, - "%s: called on %pd\n", - __func__, - dentry); - - ret = setattr_prepare(dentry, iattr); - if (ret) - goto out; + if (iattr->ia_valid & ATTR_MODE) { + if (iattr->ia_mode & (S_ISVTX)) { + if (is_root_handle(inode)) { + /* + * allow sticky bit to be set on root (since + * it shows up that way by default anyhow), + * but don't show it to the server + */ + iattr->ia_mode -= S_ISVTX; + } else { + gossip_debug(GOSSIP_UTILS_DEBUG, + "User attempted to set sticky bit on non-root directory; returning EINVAL.\n"); + return -EINVAL; + } + } + if (iattr->ia_mode & (S_ISUID)) { + gossip_debug(GOSSIP_UTILS_DEBUG, + "Attempting to set setuid bit (not supported); returning EINVAL.\n"); + return -EINVAL; + } + } if (iattr->ia_valid & ATTR_SIZE) { ret = orangefs_setattr_size(inode, iattr); @@ -225,7 +235,24 @@ int orangefs_setattr(struct dentry *dentry, struct iattr *iattr) goto out; } +again: + spin_lock(&inode->i_lock); + if (ORANGEFS_I(inode)->attr_valid) { + if (uid_eq(ORANGEFS_I(inode)->attr_uid, current_fsuid()) && + gid_eq(ORANGEFS_I(inode)->attr_gid, current_fsgid())) { + ORANGEFS_I(inode)->attr_valid = iattr->ia_valid; + } else { + spin_unlock(&inode->i_lock); + write_inode_now(inode, 1); + goto again; + } + } else { + ORANGEFS_I(inode)->attr_valid = iattr->ia_valid; + ORANGEFS_I(inode)->attr_uid = current_fsuid(); + ORANGEFS_I(inode)->attr_gid = current_fsgid(); + } setattr_copy(inode, iattr); + spin_unlock(&inode->i_lock); mark_inode_dirty(inode); if (iattr->ia_valid & ATTR_MODE) @@ -234,7 +261,25 @@ int orangefs_setattr(struct dentry *dentry, struct iattr *iattr) ret = 0; out: - gossip_debug(GOSSIP_INODE_DEBUG, "%s: ret:%d:\n", __func__, ret); + return ret; +} + +/* + * Change attributes of an object referenced by dentry. + */ +int orangefs_setattr(struct dentry *dentry, struct iattr *iattr) +{ + int ret; + gossip_debug(GOSSIP_INODE_DEBUG, "__orangefs_setattr: called on %pd\n", + dentry); + ret = setattr_prepare(dentry, iattr); + if (ret) + goto out; + ret = __orangefs_setattr(d_inode(dentry), iattr); + sync_inode_metadata(d_inode(dentry), 1); +out: + gossip_debug(GOSSIP_INODE_DEBUG, "orangefs_setattr: returning %d\n", + ret); return ret; } @@ -300,7 +345,7 @@ int orangefs_update_time(struct inode *inode, struct timespec64 *time, int flags iattr.ia_valid |= ATTR_CTIME; if (flags & S_MTIME) iattr.ia_valid |= ATTR_MTIME; - return orangefs_inode_setattr(inode, &iattr); + return __orangefs_setattr(inode, &iattr); } /* ORANGEFS2 implementation of VFS inode operations for files */ @@ -360,6 +405,7 @@ static int orangefs_set_inode(struct inode *inode, void *data) struct orangefs_object_kref *ref = (struct orangefs_object_kref *) data; ORANGEFS_I(inode)->refn.fs_id = ref->fs_id; ORANGEFS_I(inode)->refn.khandle = ref->khandle; + ORANGEFS_I(inode)->attr_valid = 0; hash_init(ORANGEFS_I(inode)->xattr_cache); return 0; } diff --git a/fs/orangefs/namei.c b/fs/orangefs/namei.c index 140314b76e10..1dd710e5f376 100644 --- a/fs/orangefs/namei.c +++ b/fs/orangefs/namei.c @@ -82,11 +82,10 @@ static int orangefs_create(struct inode *dir, __func__, dentry); - dir->i_mtime = dir->i_ctime = current_time(dir); memset(&iattr, 0, sizeof iattr); - iattr.ia_valid |= ATTR_MTIME; - orangefs_inode_setattr(dir, &iattr); - mark_inode_dirty_sync(dir); + iattr.ia_valid |= ATTR_MTIME | ATTR_CTIME; + iattr.ia_mtime = iattr.ia_ctime = current_time(dir); + __orangefs_setattr(dir, &iattr); ret = 0; out: op_release(new_op); @@ -208,11 +207,10 @@ static int orangefs_unlink(struct inode *dir, struct dentry *dentry) if (!ret) { drop_nlink(inode); - dir->i_mtime = dir->i_ctime = current_time(dir); memset(&iattr, 0, sizeof iattr); - iattr.ia_valid |= ATTR_MTIME; - orangefs_inode_setattr(dir, &iattr); - mark_inode_dirty_sync(dir); + iattr.ia_valid |= ATTR_MTIME | ATTR_CTIME; + iattr.ia_mtime = iattr.ia_ctime = current_time(dir); + __orangefs_setattr(dir, &iattr); } return ret; } @@ -295,11 +293,10 @@ static int orangefs_symlink(struct inode *dir, get_khandle_from_ino(inode), dentry); - dir->i_mtime = dir->i_ctime = current_time(dir); memset(&iattr, 0, sizeof iattr); - iattr.ia_valid |= ATTR_MTIME; - orangefs_inode_setattr(dir, &iattr); - mark_inode_dirty_sync(dir); + iattr.ia_valid |= ATTR_MTIME | ATTR_CTIME; + iattr.ia_mtime = iattr.ia_ctime = current_time(dir); + __orangefs_setattr(dir, &iattr); ret = 0; out: op_release(new_op); @@ -366,11 +363,10 @@ static int orangefs_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode * NOTE: we have no good way to keep nlink consistent for directories * across clients; keep constant at 1. */ - dir->i_mtime = dir->i_ctime = current_time(dir); memset(&iattr, 0, sizeof iattr); - iattr.ia_valid |= ATTR_MTIME; - orangefs_inode_setattr(dir, &iattr); - mark_inode_dirty_sync(dir); + iattr.ia_valid |= ATTR_MTIME | ATTR_CTIME; + iattr.ia_mtime = iattr.ia_ctime = current_time(dir); + __orangefs_setattr(dir, &iattr); out: op_release(new_op); return ret; @@ -393,11 +389,10 @@ static int orangefs_rename(struct inode *old_dir, "orangefs_rename: called (%pd2 => %pd2) ct=%d\n", old_dentry, new_dentry, d_count(new_dentry)); - new_dir->i_mtime = new_dir->i_ctime = current_time(new_dir); memset(&iattr, 0, sizeof iattr); - iattr.ia_valid |= ATTR_MTIME; - orangefs_inode_setattr(new_dir, &iattr); - mark_inode_dirty_sync(new_dir); + iattr.ia_valid |= ATTR_MTIME | ATTR_CTIME; + iattr.ia_mtime = iattr.ia_ctime = current_time(new_dir); + __orangefs_setattr(new_dir, &iattr); new_op = op_alloc(ORANGEFS_VFS_OP_RENAME); if (!new_op) diff --git a/fs/orangefs/orangefs-kernel.h b/fs/orangefs/orangefs-kernel.h index 4f0cf14c18f6..a74d9e8c5f9e 100644 --- a/fs/orangefs/orangefs-kernel.h +++ b/fs/orangefs/orangefs-kernel.h @@ -193,6 +193,9 @@ struct orangefs_inode_s { sector_t last_failed_block_index_read; unsigned long getattr_time; + int attr_valid; + kuid_t attr_uid; + kgid_t attr_gid; DECLARE_HASHTABLE(xattr_cache, 4); }; @@ -345,7 +348,8 @@ struct inode *orangefs_new_inode(struct super_block *sb, dev_t dev, struct orangefs_object_kref *ref); -int orangefs_setattr(struct dentry *dentry, struct iattr *iattr); +int __orangefs_setattr(struct inode *, struct iattr *); +int orangefs_setattr(struct dentry *, struct iattr *); int orangefs_getattr(const struct path *path, struct kstat *stat, u32 request_mask, unsigned int flags); @@ -403,7 +407,7 @@ int orangefs_inode_getattr(struct inode *, int); int orangefs_inode_check_changed(struct inode *inode); -int orangefs_inode_setattr(struct inode *inode, struct iattr *iattr); +int orangefs_inode_setattr(struct inode *inode); bool orangefs_cancel_op_in_progress(struct orangefs_kernel_op_s *op); diff --git a/fs/orangefs/orangefs-utils.c b/fs/orangefs/orangefs-utils.c index d44cbe96719a..a4fac527f85d 100644 --- a/fs/orangefs/orangefs-utils.c +++ b/fs/orangefs/orangefs-utils.c @@ -136,51 +136,37 @@ static int orangefs_inode_perms(struct ORANGEFS_sys_attr_s *attrs) * NOTE: in kernel land, we never use the sys_attr->link_target for * anything, so don't bother copying it into the sys_attr object here. */ -static inline int copy_attributes_from_inode(struct inode *inode, - struct ORANGEFS_sys_attr_s *attrs, - struct iattr *iattr) +static inline void copy_attributes_from_inode(struct inode *inode, + struct ORANGEFS_sys_attr_s *attrs) { - umode_t tmp_mode; - - if (!iattr || !inode || !attrs) { - gossip_err("NULL iattr (%p), inode (%p), attrs (%p) " - "in copy_attributes_from_inode!\n", - iattr, - inode, - attrs); - return -EINVAL; - } - /* - * We need to be careful to only copy the attributes out of the - * iattr object that we know are valid. - */ + struct orangefs_inode_s *orangefs_inode = ORANGEFS_I(inode); attrs->mask = 0; - if (iattr->ia_valid & ATTR_UID) { - attrs->owner = from_kuid(&init_user_ns, iattr->ia_uid); + if (orangefs_inode->attr_valid & ATTR_UID) { + attrs->owner = from_kuid(&init_user_ns, inode->i_uid); attrs->mask |= ORANGEFS_ATTR_SYS_UID; gossip_debug(GOSSIP_UTILS_DEBUG, "(UID) %d\n", attrs->owner); } - if (iattr->ia_valid & ATTR_GID) { - attrs->group = from_kgid(&init_user_ns, iattr->ia_gid); + if (orangefs_inode->attr_valid & ATTR_GID) { + attrs->group = from_kgid(&init_user_ns, inode->i_gid); attrs->mask |= ORANGEFS_ATTR_SYS_GID; gossip_debug(GOSSIP_UTILS_DEBUG, "(GID) %d\n", attrs->group); } - if (iattr->ia_valid & ATTR_ATIME) { + if (orangefs_inode->attr_valid & ATTR_ATIME) { attrs->mask |= ORANGEFS_ATTR_SYS_ATIME; - if (iattr->ia_valid & ATTR_ATIME_SET) { - attrs->atime = (time64_t)iattr->ia_atime.tv_sec; + if (orangefs_inode->attr_valid & ATTR_ATIME_SET) { + attrs->atime = (time64_t)inode->i_atime.tv_sec; attrs->mask |= ORANGEFS_ATTR_SYS_ATIME_SET; } } - if (iattr->ia_valid & ATTR_MTIME) { + if (orangefs_inode->attr_valid & ATTR_MTIME) { attrs->mask |= ORANGEFS_ATTR_SYS_MTIME; - if (iattr->ia_valid & ATTR_MTIME_SET) { - attrs->mtime = (time64_t)iattr->ia_mtime.tv_sec; + if (orangefs_inode->attr_valid & ATTR_MTIME_SET) { + attrs->mtime = (time64_t)inode->i_mtime.tv_sec; attrs->mask |= ORANGEFS_ATTR_SYS_MTIME_SET; } } - if (iattr->ia_valid & ATTR_CTIME) + if (orangefs_inode->attr_valid & ATTR_CTIME) attrs->mask |= ORANGEFS_ATTR_SYS_CTIME; /* @@ -189,36 +175,10 @@ static inline int copy_attributes_from_inode(struct inode *inode, * worry about ATTR_SIZE */ - if (iattr->ia_valid & ATTR_MODE) { - tmp_mode = iattr->ia_mode; - if (tmp_mode & (S_ISVTX)) { - if (is_root_handle(inode)) { - /* - * allow sticky bit to be set on root (since - * it shows up that way by default anyhow), - * but don't show it to the server - */ - tmp_mode -= S_ISVTX; - } else { - gossip_debug(GOSSIP_UTILS_DEBUG, - "%s: setting sticky bit not supported.\n", - __func__); - return -EINVAL; - } - } - - if (tmp_mode & (S_ISUID)) { - gossip_debug(GOSSIP_UTILS_DEBUG, - "%s: setting setuid bit not supported.\n", - __func__); - return -EINVAL; - } - - attrs->perms = ORANGEFS_util_translate_mode(tmp_mode); + if (orangefs_inode->attr_valid & ATTR_MODE) { + attrs->perms = ORANGEFS_util_translate_mode(inode->i_mode); attrs->mask |= ORANGEFS_ATTR_SYS_PERM; } - - return 0; } static int orangefs_inode_type(enum orangefs_ds_type objtype) @@ -283,10 +243,16 @@ int orangefs_inode_getattr(struct inode *inode, int flags) gossip_debug(GOSSIP_UTILS_DEBUG, "%s: called on inode %pU flags %d\n", __func__, get_khandle_from_ino(inode), flags); +again: spin_lock(&inode->i_lock); /* Must have all the attributes in the mask and be within cache time. */ if ((!flags && time_before(jiffies, orangefs_inode->getattr_time)) || - inode->i_state & I_DIRTY) { + orangefs_inode->attr_valid) { + if (orangefs_inode->attr_valid) { + spin_unlock(&inode->i_lock); + write_inode_now(inode, 1); + goto again; + } spin_unlock(&inode->i_lock); return 0; } @@ -311,10 +277,16 @@ int orangefs_inode_getattr(struct inode *inode, int flags) if (ret != 0) goto out; +again2: spin_lock(&inode->i_lock); /* Must have all the attributes in the mask and be within cache time. */ if ((!flags && time_before(jiffies, orangefs_inode->getattr_time)) || - inode->i_state & I_DIRTY) { + orangefs_inode->attr_valid) { + if (orangefs_inode->attr_valid) { + spin_unlock(&inode->i_lock); + write_inode_now(inode, 1); + goto again2; + } gossip_debug(GOSSIP_UTILS_DEBUG, "%s: in cache or dirty\n", __func__); ret = 0; @@ -438,7 +410,7 @@ int orangefs_inode_check_changed(struct inode *inode) * issues a orangefs setattr request to make sure the new attribute values * take effect if successful. returns 0 on success; -errno otherwise */ -int orangefs_inode_setattr(struct inode *inode, struct iattr *iattr) +int orangefs_inode_setattr(struct inode *inode) { struct orangefs_inode_s *orangefs_inode = ORANGEFS_I(inode); struct orangefs_kernel_op_s *new_op; @@ -448,24 +420,26 @@ int orangefs_inode_setattr(struct inode *inode, struct iattr *iattr) if (!new_op) return -ENOMEM; + spin_lock(&inode->i_lock); + new_op->upcall.uid = from_kuid(&init_user_ns, orangefs_inode->attr_uid); + new_op->upcall.gid = from_kgid(&init_user_ns, orangefs_inode->attr_gid); new_op->upcall.req.setattr.refn = orangefs_inode->refn; - ret = copy_attributes_from_inode(inode, - &new_op->upcall.req.setattr.attributes, - iattr); - if (ret >= 0) { - ret = service_operation(new_op, __func__, - get_interruptible_flag(inode)); + copy_attributes_from_inode(inode, + &new_op->upcall.req.setattr.attributes); + orangefs_inode->attr_valid = 0; + spin_unlock(&inode->i_lock); - gossip_debug(GOSSIP_UTILS_DEBUG, - "orangefs_inode_setattr: returning %d\n", - ret); - } + ret = service_operation(new_op, __func__, + get_interruptible_flag(inode)); + gossip_debug(GOSSIP_UTILS_DEBUG, + "orangefs_inode_setattr: returning %d\n", ret); + if (ret) + orangefs_make_bad_inode(inode); op_release(new_op); if (ret == 0) orangefs_inode->getattr_time = jiffies - 1; - return ret; } diff --git a/fs/orangefs/super.c b/fs/orangefs/super.c index f27da3bbafac..8fa30c13b7ed 100644 --- a/fs/orangefs/super.c +++ b/fs/orangefs/super.c @@ -155,17 +155,8 @@ static void orangefs_destroy_inode(struct inode *inode) static int orangefs_write_inode(struct inode *inode, struct writeback_control *wbc) { - struct iattr iattr; gossip_debug(GOSSIP_SUPER_DEBUG, "orangefs_write_inode\n"); - iattr.ia_valid = ATTR_MODE | ATTR_UID | ATTR_GID | ATTR_ATIME | - ATTR_ATIME_SET | ATTR_MTIME | ATTR_MTIME_SET | ATTR_CTIME; - iattr.ia_mode = inode->i_mode; - iattr.ia_uid = inode->i_uid; - iattr.ia_gid = inode->i_gid; - iattr.ia_atime = inode->i_atime; - iattr.ia_mtime = inode->i_mtime; - iattr.ia_ctime = inode->i_ctime; - return orangefs_inode_setattr(inode, &iattr); + return orangefs_inode_setattr(inode); } /*