(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
The debug message of decode_attr_lease_time incorrectly
says "file size". Fix it to "lease time".
Signed-off-by: Donald Buczek <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
> ---
> 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);
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 <[email protected]>
>> ---
>> 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
[email protected]
Tel: +49 30 8413 1433
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 <[email protected]>
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