From: trondmy@kernel.org
To: linux-nfs@vger.kernel.org
Subject: [PATCH v2 14/26] NFS: Separate tracking of file mode cache validity from the uid/gid
Date: Wed, 14 Apr 2021 09:43:41 -0400 [thread overview]
Message-ID: <20210414134353.11860-15-trondmy@kernel.org> (raw)
In-Reply-To: <20210414134353.11860-14-trondmy@kernel.org>
From: Trond Myklebust <trond.myklebust@hammerspace.com>
chown()/chgrp() and chmod() are separate operations, and in addition,
there are mode operations that are performed automatically by the
server. So let's track mode validity separately from the file ownership
validity.
Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
---
fs/nfs/dir.c | 5 +++--
fs/nfs/inode.c | 18 ++++++++++++------
fs/nfs/nfs4proc.c | 14 +++++++++-----
fs/nfs/nfstrace.h | 4 +++-
fs/nfs/write.c | 2 +-
include/linux/nfs_fs.h | 2 ++
6 files changed, 30 insertions(+), 15 deletions(-)
diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index f748d2294261..d2835d211a73 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -2948,7 +2948,7 @@ static int nfs_execute_ok(struct inode *inode, int mask)
if (S_ISDIR(inode->i_mode))
return 0;
- if (nfs_check_cache_invalid(inode, NFS_INO_INVALID_OTHER)) {
+ if (nfs_check_cache_invalid(inode, NFS_INO_INVALID_MODE)) {
if (mask & MAY_NOT_BLOCK)
return -ECHILD;
ret = __nfs_revalidate_inode(server, inode);
@@ -3006,7 +3006,8 @@ int nfs_permission(struct user_namespace *mnt_userns,
if (mask & MAY_NOT_BLOCK)
return -ECHILD;
- res = nfs_revalidate_inode(inode, NFS_INO_INVALID_OTHER);
+ res = nfs_revalidate_inode(inode, NFS_INO_INVALID_MODE |
+ NFS_INO_INVALID_OTHER);
if (res == 0)
res = generic_permission(&init_user_ns, inode, mask);
goto out;
diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
index 7bf9138330f2..81e3e140e923 100644
--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -199,7 +199,8 @@ void nfs_set_cache_invalid(struct inode *inode, unsigned long flags)
if (have_delegation) {
if (!(flags & NFS_INO_REVAL_FORCED))
- flags &= ~NFS_INO_INVALID_OTHER;
+ flags &= ~(NFS_INO_INVALID_MODE |
+ NFS_INO_INVALID_OTHER);
flags &= ~(NFS_INO_INVALID_CHANGE
| NFS_INO_INVALID_SIZE
| NFS_INO_INVALID_XATTR);
@@ -472,7 +473,7 @@ nfs_fhget(struct super_block *sb, struct nfs_fh *fh, struct nfs_fattr *fattr, st
nfsi->cache_validity = 0;
if ((fattr->valid & NFS_ATTR_FATTR_MODE) == 0
&& nfs_server_capable(inode, NFS_CAP_MODE))
- nfs_set_cache_invalid(inode, NFS_INO_INVALID_OTHER);
+ nfs_set_cache_invalid(inode, NFS_INO_INVALID_MODE);
/* Why so? Because we want revalidate for devices/FIFOs, and
* that's precisely what we have in nfs_file_inode_operations.
*/
@@ -803,8 +804,10 @@ static u32 nfs_get_valid_attrmask(struct inode *inode)
reply_mask |= STATX_SIZE;
if (!(cache_validity & NFS_INO_INVALID_NLINK))
reply_mask |= STATX_NLINK;
+ if (!(cache_validity & NFS_INO_INVALID_MODE))
+ reply_mask |= STATX_MODE;
if (!(cache_validity & NFS_INO_INVALID_OTHER))
- reply_mask |= STATX_UID | STATX_GID | STATX_MODE;
+ reply_mask |= STATX_UID | STATX_GID;
if (!(cache_validity & NFS_INO_INVALID_BLOCKS))
reply_mask |= STATX_BLOCKS;
return reply_mask;
@@ -872,7 +875,9 @@ int nfs_getattr(struct user_namespace *mnt_userns, const struct path *path,
do_update |= cache_validity & NFS_INO_INVALID_SIZE;
if (request_mask & STATX_NLINK)
do_update |= cache_validity & NFS_INO_INVALID_NLINK;
- if (request_mask & (STATX_UID | STATX_GID | STATX_MODE))
+ if (request_mask & STATX_MODE)
+ do_update |= cache_validity & NFS_INO_INVALID_MODE;
+ if (request_mask & (STATX_UID | STATX_GID))
do_update |= cache_validity & NFS_INO_INVALID_OTHER;
if (request_mask & STATX_BLOCKS)
do_update |= cache_validity & NFS_INO_INVALID_BLOCKS;
@@ -1510,7 +1515,7 @@ static int nfs_check_inode_attributes(struct inode *inode, struct nfs_fattr *fat
if ((fattr->valid & NFS_ATTR_FATTR_MODE) && (inode->i_mode & S_IALLUGO) != (fattr->mode & S_IALLUGO))
invalid |= NFS_INO_INVALID_ACCESS
| NFS_INO_INVALID_ACL
- | NFS_INO_INVALID_OTHER;
+ | NFS_INO_INVALID_MODE;
if ((fattr->valid & NFS_ATTR_FATTR_OWNER) && !uid_eq(inode->i_uid, fattr->uid))
invalid |= NFS_INO_INVALID_ACCESS
| NFS_INO_INVALID_ACL
@@ -1947,6 +1952,7 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr)
| NFS_INO_INVALID_SIZE
| NFS_INO_INVALID_BLOCKS
| NFS_INO_INVALID_NLINK
+ | NFS_INO_INVALID_MODE
| NFS_INO_INVALID_OTHER;
if (S_ISDIR(inode->i_mode))
nfs_force_lookup_revalidate(inode);
@@ -2037,7 +2043,7 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr)
}
} else if (server->caps & NFS_CAP_MODE) {
nfsi->cache_validity |= save_cache_validity &
- (NFS_INO_INVALID_OTHER
+ (NFS_INO_INVALID_MODE
| NFS_INO_REVAL_FORCED);
cache_revalidated = false;
}
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index a74c1c3c4192..bc90f2a12d5d 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -302,9 +302,10 @@ static void nfs4_bitmap_copy_adjust(__u32 *dst, const __u32 *src,
if (!(cache_validity & NFS_INO_INVALID_CHANGE))
dst[0] &= ~FATTR4_WORD0_CHANGE;
+ if (!(cache_validity & NFS_INO_INVALID_MODE))
+ dst[1] &= ~FATTR4_WORD1_MODE;
if (!(cache_validity & NFS_INO_INVALID_OTHER))
- dst[1] &= ~(FATTR4_WORD1_MODE | FATTR4_WORD1_OWNER |
- FATTR4_WORD1_OWNER_GROUP);
+ dst[1] &= ~(FATTR4_WORD1_OWNER | FATTR4_WORD1_OWNER_GROUP);
}
static void nfs4_setup_readdir(u64 cookie, __be32 *verifier, struct dentry *dentry,
@@ -3344,7 +3345,9 @@ static int nfs4_do_setattr(struct inode *inode, const struct cred *cred,
unsigned long adjust_flags = NFS_INO_INVALID_CHANGE;
int err;
- if (sattr->ia_valid & (ATTR_MODE|ATTR_UID|ATTR_GID))
+ if (sattr->ia_valid & (ATTR_MODE | ATTR_KILL_SUID | ATTR_KILL_SGID))
+ adjust_flags |= NFS_INO_INVALID_MODE;
+ if (sattr->ia_valid & (ATTR_UID | ATTR_GID))
adjust_flags |= NFS_INO_INVALID_OTHER;
do {
@@ -5431,9 +5434,10 @@ static void nfs4_bitmask_set(__u32 bitmask[NFS4_BITMASK_SZ], const __u32 *src,
bitmask[0] |= FATTR4_WORD0_CHANGE;
if (cache_validity & NFS_INO_INVALID_ATIME)
bitmask[1] |= FATTR4_WORD1_TIME_ACCESS;
+ if (cache_validity & NFS_INO_INVALID_MODE)
+ bitmask[1] |= FATTR4_WORD1_MODE;
if (cache_validity & NFS_INO_INVALID_OTHER)
- bitmask[1] |= FATTR4_WORD1_MODE | FATTR4_WORD1_OWNER |
- FATTR4_WORD1_OWNER_GROUP;
+ bitmask[1] |= FATTR4_WORD1_OWNER | FATTR4_WORD1_OWNER_GROUP;
if (cache_validity & NFS_INO_INVALID_NLINK)
bitmask[1] |= FATTR4_WORD1_NUMLINKS;
if (label && label->len && cache_validity & NFS_INO_INVALID_LABEL)
diff --git a/fs/nfs/nfstrace.h b/fs/nfs/nfstrace.h
index a0ebc53160dd..41a161cd31f6 100644
--- a/fs/nfs/nfstrace.h
+++ b/fs/nfs/nfstrace.h
@@ -49,6 +49,7 @@ TRACE_DEFINE_ENUM(NFS_INO_DATA_INVAL_DEFER);
TRACE_DEFINE_ENUM(NFS_INO_INVALID_BLOCKS);
TRACE_DEFINE_ENUM(NFS_INO_INVALID_XATTR);
TRACE_DEFINE_ENUM(NFS_INO_INVALID_NLINK);
+TRACE_DEFINE_ENUM(NFS_INO_INVALID_MODE);
#define nfs_show_cache_validity(v) \
__print_flags(v, "|", \
@@ -67,7 +68,8 @@ TRACE_DEFINE_ENUM(NFS_INO_INVALID_NLINK);
{ NFS_INO_DATA_INVAL_DEFER, "DATA_INVAL_DEFER" }, \
{ NFS_INO_INVALID_BLOCKS, "INVALID_BLOCKS" }, \
{ NFS_INO_INVALID_XATTR, "INVALID_XATTR" }, \
- { NFS_INO_INVALID_NLINK, "INVALID_NLINK" })
+ { NFS_INO_INVALID_NLINK, "INVALID_NLINK" }, \
+ { NFS_INO_INVALID_MODE, "INVALID_MODE" })
TRACE_DEFINE_ENUM(NFS_INO_ADVISE_RDPLUS);
TRACE_DEFINE_ENUM(NFS_INO_STALE);
diff --git a/fs/nfs/write.c b/fs/nfs/write.c
index 7a39b3d424da..61d1174935b6 100644
--- a/fs/nfs/write.c
+++ b/fs/nfs/write.c
@@ -1604,7 +1604,7 @@ static int nfs_writeback_done(struct rpc_task *task,
/* Deal with the suid/sgid bit corner case */
if (nfs_should_remove_suid(inode)) {
spin_lock(&inode->i_lock);
- nfs_set_cache_invalid(inode, NFS_INO_INVALID_OTHER);
+ nfs_set_cache_invalid(inode, NFS_INO_INVALID_MODE);
spin_unlock(&inode->i_lock);
}
return 0;
diff --git a/include/linux/nfs_fs.h b/include/linux/nfs_fs.h
index 41165b988dfb..ffba254d2098 100644
--- a/include/linux/nfs_fs.h
+++ b/include/linux/nfs_fs.h
@@ -247,12 +247,14 @@ struct nfs4_copy_state {
#define NFS_INO_INVALID_BLOCKS BIT(14) /* cached blocks are invalid */
#define NFS_INO_INVALID_XATTR BIT(15) /* xattrs are invalid */
#define NFS_INO_INVALID_NLINK BIT(16) /* cached nlinks is invalid */
+#define NFS_INO_INVALID_MODE BIT(17) /* cached mode is invalid */
#define NFS_INO_INVALID_ATTR (NFS_INO_INVALID_CHANGE \
| NFS_INO_INVALID_CTIME \
| NFS_INO_INVALID_MTIME \
| NFS_INO_INVALID_SIZE \
| NFS_INO_INVALID_NLINK \
+ | NFS_INO_INVALID_MODE \
| NFS_INO_INVALID_OTHER) /* inode metadata is invalid */
/*
--
2.30.2
next prev parent reply other threads:[~2021-04-14 13:44 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-04-14 13:43 [PATCH v2 00/26] Attribute revalidation updates trondmy
2021-04-14 13:43 ` [PATCH v2 01/26] NFS: Deal correctly with attribute generation counter overflow trondmy
2021-04-14 13:43 ` [PATCH v2 02/26] NFS: Fix up inode cache tracing trondmy
2021-04-14 13:43 ` [PATCH v2 03/26] NFS: Mask out unsupported attributes in nfs_getattr() trondmy
2021-04-14 13:43 ` [PATCH v2 04/26] NFS: NFS_INO_REVAL_PAGECACHE should mark the change attribute invalid trondmy
2021-04-14 13:43 ` [PATCH v2 05/26] NFS: Fix up revalidation of space used trondmy
2021-04-14 13:43 ` [PATCH v2 06/26] NFS: Don't revalidate attributes that are not being asked for trondmy
2021-04-14 13:43 ` [PATCH v2 07/26] NFS: Fix up statx() results trondmy
2021-04-14 13:43 ` [PATCH v2 08/26] NFS: nfs_setattr_update_inode() should clear the suid/sgid bits trondmy
2021-04-14 13:43 ` [PATCH v2 09/26] NFS: Add a cache validity flag argument to nfs_revalidate_inode() trondmy
2021-04-14 13:43 ` [PATCH v2 10/26] NFS: Replace use of NFS_INO_REVAL_PAGECACHE when checking cache validity trondmy
2021-04-14 13:43 ` [PATCH v2 11/26] NFS: Don't set NFS_INO_REVAL_PAGECACHE in the inode " trondmy
2021-04-14 13:43 ` [PATCH v2 12/26] NFSv4: Fix nfs4_bitmap_copy_adjust() trondmy
2021-04-14 13:43 ` [PATCH v2 13/26] NFS: Separate tracking of file nlinks cache validity from the mode/uid/gid trondmy
2021-04-14 13:43 ` trondmy [this message]
2021-04-14 13:43 ` [PATCH v2 15/26] NFS: Fix up handling of outstanding layoutcommit in nfs_update_inode() trondmy
2021-04-14 13:43 ` [PATCH v2 16/26] NFS: Remove a line of code that has no effect " trondmy
2021-04-14 13:43 ` [PATCH v2 17/26] NFS: Simplify cache consistency in nfs_check_inode_attributes() trondmy
2021-04-14 13:43 ` [PATCH v2 18/26] NFSv4: Fix value of decode_fsinfo_maxsz trondmy
2021-04-14 13:43 ` [PATCH v2 19/26] NFSv4: Don't modify the change attribute cached in the inode trondmy
2021-04-14 13:43 ` [PATCH v2 20/26] NFSv4: Add support for the NFSv4.2 "change_attr_type" attribute trondmy
2021-04-14 13:43 ` [PATCH v2 21/26] NFS: Use information about the change attribute to optimise updates trondmy
2021-04-14 13:43 ` [PATCH v2 22/26] NFS: Another inode revalidation improvement trondmy
2021-04-14 13:43 ` [PATCH v2 23/26] NFSv4: nfs4_inc/dec_nlink_locked should also invalidate ctime trondmy
2021-04-14 13:43 ` [PATCH v2 24/26] NFSv4: link must update the inode nlink trondmy
2021-04-14 13:43 ` [PATCH v2 25/26] NFS: Don't store NFS_INO_REVAL_FORCED trondmy
2021-04-14 13:43 ` [PATCH v2 26/26] NFS: Split attribute support out from the server capabilities trondmy
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210414134353.11860-15-trondmy@kernel.org \
--to=trondmy@kernel.org \
--cc=linux-nfs@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).