From: NeilBrown <neilb@suse.com>
To: "Schumaker\, Anna" <Anna.Schumaker@netapp.com>,
"trondmy\@hammerspace.com" <trondmy@hammerspace.com>,
"chuck.lever\@oracle.com" <chuck.lever@oracle.com>,
"aglo\@umich.edu" <aglo@umich.edu>
Cc: "linux-nfs\@vger.kernel.org" <linux-nfs@vger.kernel.org>
Subject: Re: [PATCH 3/9] NFS: send state management on a single connection.
Date: Wed, 24 Jul 2019 08:54:22 +1000 [thread overview]
Message-ID: <87r26gjra9.fsf@notabene.neil.brown.name> (raw)
In-Reply-To: <29913e2feb35dedd640224a6e9984a8b2c758c5e.camel@netapp.com>
[-- Attachment #1: Type: text/plain, Size: 7618 bytes --]
On Tue, Jul 23 2019, Schumaker, Anna wrote:
> Hi Neil,
>
> On Thu, 2019-05-30 at 10:41 +1000, NeilBrown wrote:
>> With NFSv4.1, different network connections need to be explicitly
>> bound to a session. During session startup, this is not possible
>> so only a single connection must be used for session startup.
>>
>> So add a task flag to disable the default round-robin choice of
>> connections (when nconnect > 1) and force the use of a single
>> connection.
>> Then use that flag on all requests for session management - for
>> consistence, include NFSv4.0 management (SETCLIENTID) and session
>> destruction
>>
>> Reported-by: Chuck Lever <chuck.lever@oracle.com>
>> Signed-off-by: NeilBrown <neilb@suse.com>
>> ---
>> fs/nfs/nfs4proc.c | 22 +++++++++++++---------
>> include/linux/sunrpc/sched.h | 1 +
>> net/sunrpc/clnt.c | 24 +++++++++++++++++++++++-
>> 3 files changed, 37 insertions(+), 10 deletions(-)
>>
>> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
>> index c29cbef6b53f..22b3dbfc4fa1 100644
>> --- a/fs/nfs/nfs4proc.c
>> +++ b/fs/nfs/nfs4proc.c
>> @@ -5978,7 +5978,7 @@ int nfs4_proc_setclientid(struct nfs_client
>> *clp, u32 program,
>> .rpc_message = &msg,
>> .callback_ops = &nfs4_setclientid_ops,
>> .callback_data = &setclientid,
>> - .flags = RPC_TASK_TIMEOUT,
>> + .flags = RPC_TASK_TIMEOUT | RPC_TASK_NO_ROUND_ROBIN,
>> };
>> int status;
>>
>> @@ -6044,7 +6044,8 @@ int nfs4_proc_setclientid_confirm(struct
>> nfs_client *clp,
>> dprintk("NFS call setclientid_confirm auth=%s, (client ID
>> %llx)\n",
>> clp->cl_rpcclient->cl_auth->au_ops->au_name,
>> clp->cl_clientid);
>> - status = rpc_call_sync(clp->cl_rpcclient, &msg,
>> RPC_TASK_TIMEOUT);
>> + status = rpc_call_sync(clp->cl_rpcclient, &msg,
>> + RPC_TASK_TIMEOUT |
>> RPC_TASK_NO_ROUND_ROBIN);
>> trace_nfs4_setclientid_confirm(clp, status);
>> dprintk("NFS reply setclientid_confirm: %d\n", status);
>> return status;
>> @@ -7633,7 +7634,7 @@ static int _nfs4_proc_secinfo(struct inode
>> *dir, const struct qstr *name, struct
>> NFS_SP4_MACH_CRED_SECINFO, &clnt, &msg);
>>
>> status = nfs4_call_sync(clnt, NFS_SERVER(dir), &msg,
>> &args.seq_args,
>> - &res.seq_res, 0);
>> + &res.seq_res, RPC_TASK_NO_ROUND_ROBIN);
>
> I'm confused about what setting RPC_TASK_NO_ROUND_ROBIN as the
> "cache_reply" argument to nfs4_call_sync() actually does. As far as I
> can tell, it's passed to nfs4_init_sequence() which sets it to the
> nfs4_sequence_args "sa_cache_this" field, which is a one bit boolean
> (defined in include/linux/nfs_xdr.h). So why pass the flag?
Thanks for reviewing my patch! Yes, that is an error.
I think I had confused nfs4_call_sync() with rpc_call_sync().
Similar names, different args.
For those calls, I want to get RPC_TASK_NO_ROUND_ROBIN set in the .flags
field of 'task_setup' in nfs4_call_sync_sequence().
Maybe we could add a 'flags' arg to nfs4_call_sync_sequence() and
add a new nfs4_call_sync_state() which sets RPC_TASK_NO_ROUND_ROBIN.
If you are happy with this approach, let me know and I send a proper
patch.
Thanks,
NeilBrown
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 39896afc6edf..dd2725fe7a74 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -1077,7 +1077,8 @@ static int nfs4_call_sync_sequence(struct rpc_clnt *clnt,
struct nfs_server *server,
struct rpc_message *msg,
struct nfs4_sequence_args *args,
- struct nfs4_sequence_res *res)
+ struct nfs4_sequence_res *res,
+ int flags)
{
int ret;
struct rpc_task *task;
@@ -1091,7 +1092,8 @@ static int nfs4_call_sync_sequence(struct rpc_clnt *clnt,
.rpc_client = clnt,
.rpc_message = msg,
.callback_ops = clp->cl_mvops->call_sync_ops,
- .callback_data = &data
+ .callback_data = &data,
+ .flags = flags,
};
task = rpc_run_task(&task_setup);
@@ -1112,7 +1114,20 @@ int nfs4_call_sync(struct rpc_clnt *clnt,
int cache_reply)
{
nfs4_init_sequence(args, res, cache_reply, 0);
- return nfs4_call_sync_sequence(clnt, server, msg, args, res);
+ return nfs4_call_sync_sequence(clnt, server, msg, args, res, 0);
+}
+
+int nfs4_call_sync_state(struct rpc_clnt *clnt,
+ struct nfs_server *server,
+ struct rpc_message *msg,
+ struct nfs4_sequence_args *args,
+ struct nfs4_sequence_res *res,
+ int cache_reply)
+{
+ /* State management commands are never round-robined */
+ nfs4_init_sequence(args, res, cache_reply, 0);
+ return nfs4_call_sync_sequence(clnt, server, msg, args, res,
+ RPC_TASK_NO_ROUND_ROBIN);
}
static void
@@ -7387,7 +7402,7 @@ static int _nfs40_proc_get_locations(struct inode *inode,
nfs4_init_sequence(&args.seq_args, &res.seq_res, 0, 1);
status = nfs4_call_sync_sequence(clnt, server, &msg,
- &args.seq_args, &res.seq_res);
+ &args.seq_args, &res.seq_res, 0);
if (status)
return status;
@@ -7440,7 +7455,7 @@ static int _nfs41_proc_get_locations(struct inode *inode,
nfs4_init_sequence(&args.seq_args, &res.seq_res, 0, 1);
status = nfs4_call_sync_sequence(clnt, server, &msg,
- &args.seq_args, &res.seq_res);
+ &args.seq_args, &res.seq_res, 0);
if (status == NFS4_OK &&
res.seq_res.sr_status_flags & SEQ4_STATUS_LEASE_MOVED)
status = -NFS4ERR_LEASE_MOVED;
@@ -7529,7 +7544,7 @@ static int _nfs40_proc_fsid_present(struct inode *inode, const struct cred *cred
nfs4_init_sequence(&args.seq_args, &res.seq_res, 0, 1);
status = nfs4_call_sync_sequence(clnt, server, &msg,
- &args.seq_args, &res.seq_res);
+ &args.seq_args, &res.seq_res, 0);
nfs_free_fhandle(res.fh);
if (status)
return status;
@@ -7570,7 +7585,7 @@ static int _nfs41_proc_fsid_present(struct inode *inode, const struct cred *cred
nfs4_init_sequence(&args.seq_args, &res.seq_res, 0, 1);
status = nfs4_call_sync_sequence(clnt, server, &msg,
- &args.seq_args, &res.seq_res);
+ &args.seq_args, &res.seq_res, 0);
nfs_free_fhandle(res.fh);
if (status == NFS4_OK &&
res.seq_res.sr_status_flags & SEQ4_STATUS_LEASE_MOVED)
@@ -7656,8 +7671,8 @@ static int _nfs4_proc_secinfo(struct inode *dir, const struct qstr *name, struct
nfs4_state_protect(NFS_SERVER(dir)->nfs_client,
NFS_SP4_MACH_CRED_SECINFO, &clnt, &msg);
- status = nfs4_call_sync(clnt, NFS_SERVER(dir), &msg, &args.seq_args,
- &res.seq_res, RPC_TASK_NO_ROUND_ROBIN);
+ status = nfs4_call_sync_state(clnt, NFS_SERVER(dir), &msg, &args.seq_args,
+ &res.seq_res, 0);
dprintk("NFS reply secinfo: %d\n", status);
put_cred(cred);
@@ -9357,8 +9372,8 @@ _nfs41_proc_secinfo_no_name(struct nfs_server *server, struct nfs_fh *fhandle,
}
dprintk("--> %s\n", __func__);
- status = nfs4_call_sync(clnt, server, &msg, &args.seq_args,
- &res.seq_res, RPC_TASK_NO_ROUND_ROBIN);
+ status = nfs4_call_sync_state(clnt, server, &msg, &args.seq_args,
+ &res.seq_res, 0);
dprintk("<-- %s status=%d\n", __func__, status);
put_cred(cred);
@@ -9497,7 +9512,7 @@ static int _nfs41_test_stateid(struct nfs_server *server,
dprintk("NFS call test_stateid %p\n", stateid);
nfs4_init_sequence(&args.seq_args, &res.seq_res, 0, 1);
status = nfs4_call_sync_sequence(rpc_client, server, &msg,
- &args.seq_args, &res.seq_res);
+ &args.seq_args, &res.seq_res, 0);
if (status != NFS_OK) {
dprintk("NFS reply test_stateid: failed, %d\n", status);
return status;
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
next prev parent reply other threads:[~2019-07-23 22:54 UTC|newest]
Thread overview: 66+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-05-30 0:41 [PATCH 0/9] Multiple network connections for a single NFS mount NeilBrown
2019-05-30 0:41 ` [PATCH 4/9] SUNRPC: enhance rpc_clnt_show_stats() to report on all xprts NeilBrown
2019-05-30 0:41 ` [PATCH 6/9] NFS: Add a mount option to specify number of TCP connections to use NeilBrown
2019-05-30 0:41 ` [PATCH 9/9] NFS: Allow multiple connections to a NFSv2 or NFSv3 server NeilBrown
2019-05-30 0:41 ` [PATCH 3/9] NFS: send state management on a single connection NeilBrown
2019-07-23 18:11 ` Schumaker, Anna
2019-07-23 22:54 ` NeilBrown [this message]
2019-07-31 2:05 ` [PATCH] NFS: add flags arg to nfs4_call_sync_sequence() NeilBrown
2019-05-30 0:41 ` [PATCH 5/9] SUNRPC: add links for all client xprts to debugfs NeilBrown
2019-05-30 0:41 ` [PATCH 7/9] NFSv4: Allow multiple connections to NFSv4.x servers NeilBrown
2019-05-30 0:41 ` [PATCH 8/9] pNFS: Allow multiple connections to the DS NeilBrown
2019-05-30 0:41 ` [PATCH 2/9] SUNRPC: Allow creation of RPC clients with multiple connections NeilBrown
2019-05-30 0:41 ` [PATCH 1/9] SUNRPC: Add basic load balancing to the transport switch NeilBrown
2019-05-30 17:05 ` [PATCH 0/9] Multiple network connections for a single NFS mount Tom Talpey
2019-05-30 17:20 ` Olga Kornievskaia
2019-05-30 17:41 ` Tom Talpey
2019-05-30 18:41 ` Olga Kornievskaia
2019-05-31 1:45 ` Tom Talpey
2019-05-30 22:38 ` NeilBrown
2019-05-31 1:48 ` Tom Talpey
2019-05-31 2:31 ` NeilBrown
2019-05-31 12:39 ` Tom Talpey
2019-05-30 23:53 ` Rick Macklem
2019-05-31 0:15 ` J. Bruce Fields
2019-05-31 1:01 ` NeilBrown
2019-05-31 2:20 ` Rick Macklem
2019-05-31 12:36 ` Tom Talpey
2019-05-31 13:33 ` Trond Myklebust
2019-05-30 17:56 ` Chuck Lever
2019-05-30 18:59 ` Olga Kornievskaia
2019-05-30 22:56 ` NeilBrown
2019-05-31 13:46 ` Chuck Lever
2019-05-31 15:38 ` J. Bruce Fields
2019-06-11 1:09 ` NeilBrown
2019-06-11 14:51 ` Chuck Lever
2019-06-11 15:05 ` Tom Talpey
2019-06-11 15:20 ` Trond Myklebust
2019-06-11 15:35 ` Chuck Lever
2019-06-11 16:41 ` Trond Myklebust
2019-06-11 17:32 ` Chuck Lever
2019-06-11 17:44 ` Trond Myklebust
2019-06-12 12:34 ` Steve Dickson
2019-06-12 12:47 ` Trond Myklebust
2019-06-12 13:10 ` Trond Myklebust
2019-06-11 15:34 ` Olga Kornievskaia
2019-06-11 17:46 ` Chuck Lever
2019-06-11 19:13 ` Olga Kornievskaia
2019-06-11 20:02 ` Tom Talpey
2019-06-11 20:09 ` Chuck Lever
2019-06-11 21:10 ` Olga Kornievskaia
2019-06-11 21:35 ` Tom Talpey
2019-06-11 22:55 ` NeilBrown
2019-06-12 12:55 ` Tom Talpey
2019-06-11 23:02 ` NeilBrown
2019-06-11 23:21 ` NeilBrown
2019-06-12 12:52 ` Tom Talpey
2019-06-11 23:42 ` NeilBrown
2019-06-12 12:39 ` Steve Dickson
2019-06-12 17:36 ` Chuck Lever
2019-06-12 23:03 ` NeilBrown
2019-06-13 16:13 ` Chuck Lever
2019-06-12 1:49 ` NeilBrown
2019-06-12 18:32 ` Chuck Lever
2019-06-12 23:37 ` NeilBrown
2019-06-13 16:27 ` Chuck Lever
2019-05-31 0:24 ` J. Bruce Fields
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=87r26gjra9.fsf@notabene.neil.brown.name \
--to=neilb@suse.com \
--cc=Anna.Schumaker@netapp.com \
--cc=aglo@umich.edu \
--cc=chuck.lever@oracle.com \
--cc=linux-nfs@vger.kernel.org \
--cc=trondmy@hammerspace.com \
/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).