[PATCH 0/4 RESEND] nfs4.0: Refetch lease_time after clientID reset

[PATCH 0/4 RESEND] nfs4.0: Refetch lease_time after clientID reset

[Donald Buczek]

(rebased on linux-next)

We've noticed, that nfs mounts with vers=4.0 do not pick up a updated
lease_time after a restart of the nfs server. This was discussed in
the thread "4.0 client and server restart with decreased lease time" on
linux-nfs [1].

This patch set fixes the issue for nsf4.0 clients so that hey behave as
nfs4.1 and nfs4.2 clients do.  After a new clientID is established, the
lease_time is re-fetched and used.

I've notcied, that the flag NFS_CS_CHECK_LEASE_TIME is not functional in
the existing code. It is set and tested, but never reset. Either
nfs4_setup_state_renewal should reset the flag after it verified the
lease_time or the flag could be removed altogether. I left it as is,
because I don't known what is preferred.

[1] https://marc.info/?t=154954022700002&r=1&w=2

Donald Buczek (4):
  nfs: Fix copy-and-paste error in debug message
  nfs4: Rename nfs41_setup_state_renewal
  nfs4: Move nfs4_setup_state_renewal
  nfs4.0: Refetch lease_time after clientid update

 fs/nfs/nfs4state.c | 46 +++++++++++++++++++++++-----------------------
 fs/nfs/nfs4xdr.c   |  2 +-
 2 files changed, 24 insertions(+), 24 deletions(-)

-- 
2.21.0

[PATCH 1/4 RESEND] nfs: Fix copy-and-paste error in debug message

[Donald Buczek]

The debug message of decode_attr_lease_time incorrectly
says "file size". Fix it to "lease time".

Signed-off-by: Donald Buczek <buczek@molgen.mpg.de>
---
 fs/nfs/nfs4xdr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
index 602446158bfb..06aaf017e1c6 100644
--- a/fs/nfs/nfs4xdr.c
+++ b/fs/nfs/nfs4xdr.c
@@ -3427,7 +3427,7 @@ static int decode_attr_lease_time(struct xdr_stream *xdr, uint32_t *bitmap, uint
 		*res = be32_to_cpup(p);
 		bitmap[0] &= ~FATTR4_WORD0_LEASE_TIME;
 	}
-	dprintk("%s: file size=%u\n", __func__, (unsigned int)*res);
+	dprintk("%s: lease time=%u\n", __func__, (unsigned int)*res);
 	return 0;
 }
 
-- 
2.22.0

[PATCH 2/4 RESEND] nfs4: Rename nfs41_setup_state_renewal

[Donald Buczek]

The function nfs41_setup_state_renewal is useful to the nfs 4.0 client
as well, so rename the function to nfs4_setup_state_renewal.

Signed-off-by: Donald Buczek <buczek@molgen.mpg.de>
---
 fs/nfs/nfs4state.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
index e2e3c4f04d3e..778ebfb00d13 100644
--- a/fs/nfs/nfs4state.c
+++ b/fs/nfs/nfs4state.c
@@ -286,7 +286,7 @@ static int nfs4_begin_drain_session(struct nfs_client *clp)
 
 #if defined(CONFIG_NFS_V4_1)
 
-static int nfs41_setup_state_renewal(struct nfs_client *clp)
+static int nfs4_setup_state_renewal(struct nfs_client *clp)
 {
 	int status;
 	struct nfs_fsinfo fsinfo;
@@ -313,7 +313,7 @@ static void nfs41_finish_session_reset(struct nfs_client *clp)
 	clear_bit(NFS4CLNT_SESSION_RESET, &clp->cl_state);
 	/* create_session negotiated new slot table */
 	clear_bit(NFS4CLNT_BIND_CONN_TO_SESSION, &clp->cl_state);
-	nfs41_setup_state_renewal(clp);
+	nfs4_setup_state_renewal(clp);
 }
 
 int nfs41_init_clientid(struct nfs_client *clp, const struct cred *cred)
-- 
2.22.0

[PATCH 3/4 RESEND] nfs4: Move nfs4_setup_state_renewal

[Donald Buczek]

The function nfs4_setup_state_renewal is to be used by
nfs4_init_clientid. Move it further to the top of the source file to
include it regardles of CONFIG_NFS_V4_1 and to save a forward
declaration. No code changed.

Signed-off-by: Donald Buczek <buczek@molgen.mpg.de>
---
 fs/nfs/nfs4state.c | 42 +++++++++++++++++++++---------------------
 1 file changed, 21 insertions(+), 21 deletions(-)

diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
index 778ebfb00d13..c2df257f426f 100644
--- a/fs/nfs/nfs4state.c
+++ b/fs/nfs/nfs4state.c
@@ -87,6 +87,27 @@ const nfs4_stateid current_stateid = {
 
 static DEFINE_MUTEX(nfs_clid_init_mutex);
 
+static int nfs4_setup_state_renewal(struct nfs_client *clp)
+{
+	int status;
+	struct nfs_fsinfo fsinfo;
+	unsigned long now;
+
+	if (!test_bit(NFS_CS_CHECK_LEASE_TIME, &clp->cl_res_state)) {
+		nfs4_schedule_state_renewal(clp);
+		return 0;
+	}
+
+	now = jiffies;
+	status = nfs4_proc_get_lease_time(clp, &fsinfo);
+	if (status == 0) {
+		nfs4_set_lease_period(clp, fsinfo.lease_time * HZ, now);
+		nfs4_schedule_state_renewal(clp);
+	}
+
+	return status;
+}
+
 int nfs4_init_clientid(struct nfs_client *clp, const struct cred *cred)
 {
 	struct nfs4_setclientid_res clid = {
@@ -286,27 +307,6 @@ static int nfs4_begin_drain_session(struct nfs_client *clp)
 
 #if defined(CONFIG_NFS_V4_1)
 
-static int nfs4_setup_state_renewal(struct nfs_client *clp)
-{
-	int status;
-	struct nfs_fsinfo fsinfo;
-	unsigned long now;
-
-	if (!test_bit(NFS_CS_CHECK_LEASE_TIME, &clp->cl_res_state)) {
-		nfs4_schedule_state_renewal(clp);
-		return 0;
-	}
-
-	now = jiffies;
-	status = nfs4_proc_get_lease_time(clp, &fsinfo);
-	if (status == 0) {
-		nfs4_set_lease_period(clp, fsinfo.lease_time * HZ, now);
-		nfs4_schedule_state_renewal(clp);
-	}
-
-	return status;
-}
-
 static void nfs41_finish_session_reset(struct nfs_client *clp)
 {
 	clear_bit(NFS4CLNT_LEASE_CONFIRM, &clp->cl_state);
-- 
2.22.0

Re: [PATCH 3/4 RESEND] nfs4: Move nfs4_setup_state_renewal

[Schumaker, Anna]

Hi Donald,

On Fri, 2019-06-28 at 14:36 +0200, Donald Buczek wrote:
> The function nfs4_setup_state_renewal is to be used by
> nfs4_init_clientid. Move it further to the top of the source file to
> include it regardles of CONFIG_NFS_V4_1 and to save a forward
> declaration. No code changed.
> 
> Signed-off-by: Donald Buczek <buczek@molgen.mpg.de>
> ---
>  fs/nfs/nfs4state.c | 42 +++++++++++++++++++++---------------------
>  1 file changed, 21 insertions(+), 21 deletions(-)
> 
> diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
> index 778ebfb00d13..c2df257f426f 100644
> --- a/fs/nfs/nfs4state.c
> +++ b/fs/nfs/nfs4state.c
> @@ -87,6 +87,27 @@ const nfs4_stateid current_stateid = {
>  
>  static DEFINE_MUTEX(nfs_clid_init_mutex);
>  
> +static int nfs4_setup_state_renewal(struct nfs_client *clp)
> +{
> +	int status;
> +	struct nfs_fsinfo fsinfo;
> +	unsigned long now;
> +
> +	if (!test_bit(NFS_CS_CHECK_LEASE_TIME, &clp->cl_res_state)) {
> +		nfs4_schedule_state_renewal(clp);
> +		return 0;
> +	}
> +
> +	now = jiffies;
> +	status = nfs4_proc_get_lease_time(clp, &fsinfo);

nfs4_proc_get_lease_time() is defined under a CONFIG_NFS_V4_1 block, so
this still won't compile. As far as I can tell, there isn't anything
v4.1 specific to nfs4_proc_get_lease_time() and the corresponding xdr
code so it's probably safe to move all this out too.

Anna

> +	if (status == 0) {
> +		nfs4_set_lease_period(clp, fsinfo.lease_time * HZ,
> now);
> +		nfs4_schedule_state_renewal(clp);
> +	}
> +
> +	return status;
> +}
> +
>  int nfs4_init_clientid(struct nfs_client *clp, const struct cred
> *cred)
>  {
>  	struct nfs4_setclientid_res clid = {
> @@ -286,27 +307,6 @@ static int nfs4_begin_drain_session(struct
> nfs_client *clp)
>  
>  #if defined(CONFIG_NFS_V4_1)
>  
> -static int nfs4_setup_state_renewal(struct nfs_client *clp)
> -{
> -	int status;
> -	struct nfs_fsinfo fsinfo;
> -	unsigned long now;
> -
> -	if (!test_bit(NFS_CS_CHECK_LEASE_TIME, &clp->cl_res_state)) {
> -		nfs4_schedule_state_renewal(clp);
> -		return 0;
> -	}
> -
> -	now = jiffies;
> -	status = nfs4_proc_get_lease_time(clp, &fsinfo);
> -	if (status == 0) {
> -		nfs4_set_lease_period(clp, fsinfo.lease_time * HZ,
> now);
> -		nfs4_schedule_state_renewal(clp);
> -	}
> -
> -	return status;
> -}
> -
>  static void nfs41_finish_session_reset(struct nfs_client *clp)
>  {
>  	clear_bit(NFS4CLNT_LEASE_CONFIRM, &clp->cl_state);

Re: [PATCH 3/4 RESEND] nfs4: Move nfs4_setup_state_renewal

[Donald Buczek]

Dear Anna,

On 28.06.19 20:29, Schumaker, Anna wrote:
> Hi Donald,
> 
> On Fri, 2019-06-28 at 14:36 +0200, Donald Buczek wrote:
>> The function nfs4_setup_state_renewal is to be used by
>> nfs4_init_clientid. Move it further to the top of the source file to
>> include it regardles of CONFIG_NFS_V4_1 and to save a forward
>> declaration. No code changed.
>>
>> Signed-off-by: Donald Buczek <buczek@molgen.mpg.de>
>> ---
>>   fs/nfs/nfs4state.c | 42 +++++++++++++++++++++---------------------
>>   1 file changed, 21 insertions(+), 21 deletions(-)
>>
>> diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
>> index 778ebfb00d13..c2df257f426f 100644
>> --- a/fs/nfs/nfs4state.c
>> +++ b/fs/nfs/nfs4state.c
>> @@ -87,6 +87,27 @@ const nfs4_stateid current_stateid = {
>>   
>>   static DEFINE_MUTEX(nfs_clid_init_mutex);
>>   
>> +static int nfs4_setup_state_renewal(struct nfs_client *clp)
>> +{
>> +	int status;
>> +	struct nfs_fsinfo fsinfo;
>> +	unsigned long now;
>> +
>> +	if (!test_bit(NFS_CS_CHECK_LEASE_TIME, &clp->cl_res_state)) {
>> +		nfs4_schedule_state_renewal(clp);
>> +		return 0;
>> +	}
>> +
>> +	now = jiffies;
>> +	status = nfs4_proc_get_lease_time(clp, &fsinfo);
> 
> nfs4_proc_get_lease_time() is defined under a CONFIG_NFS_V4_1 block, so
> this still won't compile. As far as I can tell, there isn't anything
> v4.1 specific to nfs4_proc_get_lease_time() and the corresponding xdr
> code so it's probably safe to move all this out too.

You're right. I'll fix that.

Thank you

   Donald

> 
> Anna
> 
>> +	if (status == 0) {
>> +		nfs4_set_lease_period(clp, fsinfo.lease_time * HZ,
>> now);
>> +		nfs4_schedule_state_renewal(clp);
>> +	}
>> +
>> +	return status;
>> +}
>> +
>>   int nfs4_init_clientid(struct nfs_client *clp, const struct cred
>> *cred)
>>   {
>>   	struct nfs4_setclientid_res clid = {
>> @@ -286,27 +307,6 @@ static int nfs4_begin_drain_session(struct
>> nfs_client *clp)
>>   
>>   #if defined(CONFIG_NFS_V4_1)
>>   
>> -static int nfs4_setup_state_renewal(struct nfs_client *clp)
>> -{
>> -	int status;
>> -	struct nfs_fsinfo fsinfo;
>> -	unsigned long now;
>> -
>> -	if (!test_bit(NFS_CS_CHECK_LEASE_TIME, &clp->cl_res_state)) {
>> -		nfs4_schedule_state_renewal(clp);
>> -		return 0;
>> -	}
>> -
>> -	now = jiffies;
>> -	status = nfs4_proc_get_lease_time(clp, &fsinfo);
>> -	if (status == 0) {
>> -		nfs4_set_lease_period(clp, fsinfo.lease_time * HZ,
>> now);
>> -		nfs4_schedule_state_renewal(clp);
>> -	}
>> -
>> -	return status;
>> -}
>> -
>>   static void nfs41_finish_session_reset(struct nfs_client *clp)
>>   {
>>   	clear_bit(NFS4CLNT_LEASE_CONFIRM, &clp->cl_state);

-- 
Donald Buczek
buczek@molgen.mpg.de
Tel: +49 30 8413 1433

Re: [PATCH 3/4 RESEND] nfs4: Move nfs4_setup_state_renewal

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 2375 bytes --]

Hi Donald,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on nfs/linux-next]
[also build test ERROR on v5.2-rc6 next-20190625]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Donald-Buczek/nfs-Fix-copy-and-paste-error-in-debug-message/20190701-153246
base:   git://git.linux-nfs.org/projects/trondmy/linux-nfs.git linux-next
config: arm-sunxi_defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (GCC) 7.4.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.4.0 make.cross ARCH=arm 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   fs//nfs/nfs4state.c: In function 'nfs4_setup_state_renewal':
>> fs//nfs/nfs4state.c:102:11: error: implicit declaration of function 'nfs4_proc_get_lease_time'; did you mean 'nfs4_proc_get_locations'? [-Werror=implicit-function-declaration]
     status = nfs4_proc_get_lease_time(clp, &fsinfo);
              ^~~~~~~~~~~~~~~~~~~~~~~~
              nfs4_proc_get_locations
   At top level:
   fs//nfs/nfs4state.c:90:12: warning: 'nfs4_setup_state_renewal' defined but not used [-Wunused-function]
    static int nfs4_setup_state_renewal(struct nfs_client *clp)
               ^~~~~~~~~~~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors

vim +102 fs//nfs/nfs4state.c

    89	
    90	static int nfs4_setup_state_renewal(struct nfs_client *clp)
    91	{
    92		int status;
    93		struct nfs_fsinfo fsinfo;
    94		unsigned long now;
    95	
    96		if (!test_bit(NFS_CS_CHECK_LEASE_TIME, &clp->cl_res_state)) {
    97			nfs4_schedule_state_renewal(clp);
    98			return 0;
    99		}
   100	
   101		now = jiffies;
 > 102		status = nfs4_proc_get_lease_time(clp, &fsinfo);
   103		if (status == 0) {
   104			nfs4_set_lease_period(clp, fsinfo.lease_time * HZ, now);
   105			nfs4_schedule_state_renewal(clp);
   106		}
   107	
   108		return status;
   109	}
   110	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 25712 bytes --]

[PATCH 4/4 RESEND] nfs4.0: Refetch lease_time after clientid update

[Donald Buczek]

RFC 7530 requires us to refetch the lease time attribute once a new
clientID is established. This is already implemented for the
nfs4.1(+) clients by nfs41_init_clientid, which calls
nfs41_finish_session_reset, which calls nfs4_setup_state_renewal.

Let nfs4_init_clientid, which is used for 4.0 clients, call
nfs4_setup_state_renewal as well.

Signed-off-by: Donald Buczek <buczek@molgen.mpg.de>
---
 fs/nfs/nfs4state.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
index c2df257f426f..f32b02c2bc73 100644
--- a/fs/nfs/nfs4state.c
+++ b/fs/nfs/nfs4state.c
@@ -135,7 +135,7 @@ int nfs4_init_clientid(struct nfs_client *clp, const struct cred *cred)
 	if (status != 0)
 		goto out;
 	clear_bit(NFS4CLNT_LEASE_CONFIRM, &clp->cl_state);
-	nfs4_schedule_state_renewal(clp);
+	nfs4_setup_state_renewal(clp);
 out:
 	return status;
 }
-- 
2.22.0