Re: [PATCH] NFSv4: Add lease_time and lease_expired to 'nfs4:' line of mountstats

[Dave Wysochanski <dwysocha@redhat.com>]

On Mon, 2019-05-20 at 11:10 -0400, Chuck Lever wrote:
> > On May 17, 2019, at 5:06 PM, Dave Wysochanski <dwysocha@redhat.com>
> > wrote:
> > 
> > On the NFS client there is no low-impact way to determine the nfs4
> > lease time or whether the lease is expired, so add these to
> > mountstats
> > with times displayed in seconds.
> > 
> > If the lease is not expired, display lease_expired=0. Otherwise,
> > display lease_expired=seconds_since_expired, similar to 'age:' line
> > in mountstats.
> > 
> > Signed-off-by: Dave Wysochanski <dwysocha@redhat.com>
> > ---
> > fs/nfs/super.c | 11 +++++++++++
> > 1 file changed, 11 insertions(+)
> > 
> > diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> > index c27ac96..6e52f0c 100644
> > --- a/fs/nfs/super.c
> > +++ b/fs/nfs/super.c
> > @@ -730,6 +730,16 @@ int nfs_show_options(struct seq_file *m,
> > struct dentry *root)
> > EXPORT_SYMBOL_GPL(nfs_show_options);
> > 
> > #if IS_ENABLED(CONFIG_NFS_V4)
> > +static void show_lease(struct seq_file *m, struct nfs_server
> > *server)
> > +{
> > +	struct nfs_client *clp = server->nfs_client;
> > +	unsigned long expire;
> > +
> > +	seq_printf(m, ",lease_time=%ld", clp->cl_lease_time / HZ);
> > +	expire = clp->cl_last_renewal + clp->cl_lease_time;
> > +	seq_printf(m, ",lease_expired=%ld",
> > +		   time_after(expire, jiffies) ?  0 : (jiffies -
> > expire) / HZ);
> > +}
> > #ifdef CONFIG_NFS_V4_1
> > static void show_sessions(struct seq_file *m, struct nfs_server
> > *server)
> > {
> > @@ -838,6 +848,7 @@ int nfs_show_stats(struct seq_file *m, struct
> > dentry *root)
> > 		seq_printf(m, ",acl=0x%x", nfss->acl_bitmask);
> > 		show_sessions(m, nfss);
> > 		show_pnfs(m, nfss);
> > +		show_lease(m, nfss);
> > 	}
> > #endif
> > 
> > -- 
> > 1.8.3.1
> > 
> 
> I didn't look closely at the patch content, but IMO this is a good
> observability enhancement.
> 
> 
> 

Thanks Chuck.  Trond or Anna do you have any concerns about this patch?

I didn't bump NFS_IOSTAT_VERS as I understand that only covers the
counts not the other lines in nfs_show_stats, correct?

Thanks.