orangefs: reorganize setattr functions to track attribute changes

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 <martin@omnibond.com>
Signed-off-by: Mike Marshall <hubcap@omnibond.com>

diff --git a/fs/orangefs/acl.c b/fs/orangefs/acl.c
index 72d2ff1..eced272 100644 (file)
--- 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 2e630c1..2708bf8 100644 (file)
--- 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 140314b..1dd710e 100644 (file)
--- 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 4f0cf14..a74d9e8 100644 (file)
--- 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 d44cbe9..a4fac52 100644 (file)
--- 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 @@ out:
  * 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 f27da3b..8fa30c1 100644 (file)
--- 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);
 }
 
 /*