Hi,
May I ask if there are any concerns or opinions regarding the
introduction of the new mount option?
If there is a more suitable solution, we can discuss it, and I can
work on implementing it.
Best regards,
Chengen Du
On Wed, Apr 19, 2023 at 3:18 PM Daire Byrne <daire@xxxxxxxx> wrote:
>
> Just to say, I am personally for this or some other way to opt out of
> the increased ACCESS calls on select servers (e.g. ones with high
> latency or with lots of multi user logins).
>
> I see it as similar to "actimeo" and "nocto" options in allowing users
> to reduce "correctness" at their own risk if it helps with their
> workloads and they deem the risk worth it.
>
> I am currently reverting the original patches in our kernel for our
> nfs re-exporting workloads.
>
> Daire
>
>
> On Tue, 11 Apr 2023 at 04:03, Chengen Du <chengen.du@xxxxxxxxxxxxx> wrote:
> >
> > This mount option is used to skip clearing the file access cache
> > upon login. Some users or applications switch to other privileged
> > users via commands such as 'su' to operate on NFS-mounted folders.
> > In such cases, the privileged user's login time will be renewed,
> > and NFS ACCESS operations will need to be re-sent, potentially
> > leading to performance degradation.
> > In some production environments where the access cache can be
> > trusted due to stable group membership, this option could be
> > particularly useful.
> >
> > Signed-off-by: Chengen Du <chengen.du@xxxxxxxxxxxxx>
> > ---
> > fs/nfs/dir.c | 21 ++++++++++++---------
> > fs/nfs/fs_context.c | 8 ++++++++
> > fs/nfs/super.c | 1 +
> > include/linux/nfs_fs_sb.h | 1 +
> > 4 files changed, 22 insertions(+), 9 deletions(-)
> >
> > diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
> > index 6fbcbb8d6587..9a6d86e2044e 100644
> > --- a/fs/nfs/dir.c
> > +++ b/fs/nfs/dir.c
> > @@ -2953,12 +2953,14 @@ static struct nfs_access_entry *nfs_access_search_rbtree(struct inode *inode, co
> > return NULL;
> > }
> >
> > -static u64 nfs_access_login_time(const struct task_struct *task,
> > - const struct cred *cred)
> > +static inline
> > +bool nfs_check_access_stale(const struct task_struct *task,
> > + const struct cred *cred,
> > + const struct nfs_access_entry *cache)
> > {
> > const struct task_struct *parent;
> > const struct cred *pcred;
> > - u64 ret;
> > + u64 login_time;
> >
> > rcu_read_lock();
> > for (;;) {
> > @@ -2968,15 +2970,15 @@ static u64 nfs_access_login_time(const struct task_struct *task,
> > break;
> > task = parent;
> > }
> > - ret = task->start_time;
> > + login_time = task->start_time;
> > rcu_read_unlock();
> > - return ret;
> > +
> > + return ((s64)(login_time - cache->timestamp) > 0);
> > }
> >
> > static int nfs_access_get_cached_locked(struct inode *inode, const struct cred *cred, u32 *mask, bool may_block)
> > {
> > struct nfs_inode *nfsi = NFS_I(inode);
> > - u64 login_time = nfs_access_login_time(current, cred);
> > struct nfs_access_entry *cache;
> > bool retry = true;
> > int err;
> > @@ -3005,7 +3007,8 @@ static int nfs_access_get_cached_locked(struct inode *inode, const struct cred *
> > retry = false;
> > }
> > err = -ENOENT;
> > - if ((s64)(login_time - cache->timestamp) > 0)
> > + if (!(NFS_SERVER(inode)->flags & NFS_MOUNT_NOFASC) &&
> > + nfs_check_access_stale(current, cred, cache))
> > goto out;
> > *mask = cache->mask;
> > list_move_tail(&cache->lru, &nfsi->access_cache_entry_lru);
> > @@ -3025,7 +3028,6 @@ static int nfs_access_get_cached_rcu(struct inode *inode, const struct cred *cre
> > * but do it without locking.
> > */
> > struct nfs_inode *nfsi = NFS_I(inode);
> > - u64 login_time = nfs_access_login_time(current, cred);
> > struct nfs_access_entry *cache;
> > int err = -ECHILD;
> > struct list_head *lh;
> > @@ -3040,7 +3042,8 @@ static int nfs_access_get_cached_rcu(struct inode *inode, const struct cred *cre
> > cache = NULL;
> > if (cache == NULL)
> > goto out;
> > - if ((s64)(login_time - cache->timestamp) > 0)
> > + if (!(NFS_SERVER(inode)->flags & NFS_MOUNT_NOFASC) &&
> > + nfs_check_access_stale(current, cred, cache))
> > goto out;
> > if (nfs_check_cache_invalid(inode, NFS_INO_INVALID_ACCESS))
> > goto out;
> > diff --git a/fs/nfs/fs_context.c b/fs/nfs/fs_context.c
> > index 9bcd53d5c7d4..5cd9b2882c79 100644
> > --- a/fs/nfs/fs_context.c
> > +++ b/fs/nfs/fs_context.c
> > @@ -88,6 +88,7 @@ enum nfs_param {
> > Opt_vers,
> > Opt_wsize,
> > Opt_write,
> > + Opt_fasc,
> > };
> >
> > enum {
> > @@ -194,6 +195,7 @@ static const struct fs_parameter_spec nfs_fs_parameters[] = {
> > fsparam_string("vers", Opt_vers),
> > fsparam_enum ("write", Opt_write, nfs_param_enums_write),
> > fsparam_u32 ("wsize", Opt_wsize),
> > + fsparam_flag_no("fasc", Opt_fasc),
> > {}
> > };
> >
> > @@ -861,6 +863,12 @@ static int nfs_fs_context_parse_param(struct fs_context *fc,
> > case Opt_sloppy:
> > ctx->sloppy = true;
> > break;
> > + case Opt_fasc:
> > + if (result.negated)
> > + ctx->flags |= NFS_MOUNT_NOFASC;
> > + else
> > + ctx->flags &= ~NFS_MOUNT_NOFASC;
> > + break;
> > }
> >
> > return 0;
> > diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> > index 05ae23657527..059513d403f8 100644
> > --- a/fs/nfs/super.c
> > +++ b/fs/nfs/super.c
> > @@ -444,6 +444,7 @@ static void nfs_show_mount_options(struct seq_file *m, struct nfs_server *nfss,
> > { NFS_MOUNT_NORDIRPLUS, ",nordirplus", "" },
> > { NFS_MOUNT_UNSHARED, ",nosharecache", "" },
> > { NFS_MOUNT_NORESVPORT, ",noresvport", "" },
> > + { NFS_MOUNT_NOFASC, ",nofasc", "" },
> > { 0, NULL, NULL }
> > };
> > const struct proc_nfs_info *nfs_infop;
> > diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
> > index ea2f7e6b1b0b..a39ae1bd2217 100644
> > --- a/include/linux/nfs_fs_sb.h
> > +++ b/include/linux/nfs_fs_sb.h
> > @@ -153,6 +153,7 @@ struct nfs_server {
> > #define NFS_MOUNT_WRITE_EAGER 0x01000000
> > #define NFS_MOUNT_WRITE_WAIT 0x02000000
> > #define NFS_MOUNT_TRUNK_DISCOVERY 0x04000000
> > +#define NFS_MOUNT_NOFASC 0x08000000
> >
> > unsigned int fattr_valid; /* Valid attributes */
> > unsigned int caps; /* server capabilities */
> > --
> > 2.37.2
> >
