* Minor cleanup of compound_send_recv
@ 2019-06-24 18:29 Steve French
2019-06-24 21:41 ` ronnie sahlberg
2019-07-01 12:46 ` Aurélien Aptel
0 siblings, 2 replies; 4+ messages in thread
From: Steve French @ 2019-06-24 18:29 UTC (permalink / raw)
To: CIFS; +Cc: Aurélien Aptel
[-- Attachment #1: Type: text/plain, Size: 247 bytes --]
In Aurelien's earlier patch series I noticed a cleanup (converting
ses->server to a local variable server=ses->server) which made code
easier to read in this function. This doesn't require compounding but
probably helps his
--
Thanks,
Steve
[-- Attachment #2: 0001-smb3-minor-cleanup-of-compound_send_recv.patch --]
[-- Type: text/x-patch, Size: 5561 bytes --]
From 1135a5fc08f45f918a68b3a604ae347cd70f3921 Mon Sep 17 00:00:00 2001
From: Aurelien Aptel <aaptel@suse.com>
Date: Mon, 24 Jun 2019 13:00:12 -0500
Subject: [PATCH] smb3: minor cleanup of compound_send_recv
Trivial cleanup. Will make future multichannel code smaller
as well.
Signed-off-by: Aurelien Aptel <aaptel@suse.com>
---
fs/cifs/transport.c | 46 +++++++++++++++++++++++----------------------
1 file changed, 24 insertions(+), 22 deletions(-)
diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c
index 60661b3f983a..5d6d44bfe10a 100644
--- a/fs/cifs/transport.c
+++ b/fs/cifs/transport.c
@@ -979,6 +979,7 @@ compound_send_recv(const unsigned int xid, struct cifs_ses *ses,
};
unsigned int instance;
char *buf;
+ struct TCP_Server_Info *server;
optype = flags & CIFS_OP_MASK;
@@ -990,7 +991,8 @@ compound_send_recv(const unsigned int xid, struct cifs_ses *ses,
return -EIO;
}
- if (ses->server->tcpStatus == CifsExiting)
+ server = ses->server;
+ if (server->tcpStatus == CifsExiting)
return -ENOENT;
/*
@@ -1001,7 +1003,7 @@ compound_send_recv(const unsigned int xid, struct cifs_ses *ses,
* other requests.
* This can be handled by the eventual session reconnect.
*/
- rc = wait_for_compound_request(ses->server, num_rqst, flags,
+ rc = wait_for_compound_request(server, num_rqst, flags,
&instance);
if (rc)
return rc;
@@ -1017,7 +1019,7 @@ compound_send_recv(const unsigned int xid, struct cifs_ses *ses,
* of smb data.
*/
- mutex_lock(&ses->server->srv_mutex);
+ mutex_lock(&server->srv_mutex);
/*
* All the parts of the compound chain belong obtained credits from the
@@ -1026,24 +1028,24 @@ compound_send_recv(const unsigned int xid, struct cifs_ses *ses,
* we obtained credits and return -EAGAIN in such cases to let callers
* handle it.
*/
- if (instance != ses->server->reconnect_instance) {
- mutex_unlock(&ses->server->srv_mutex);
+ if (instance != server->reconnect_instance) {
+ mutex_unlock(&server->srv_mutex);
for (j = 0; j < num_rqst; j++)
- add_credits(ses->server, &credits[j], optype);
+ add_credits(server, &credits[j], optype);
return -EAGAIN;
}
for (i = 0; i < num_rqst; i++) {
- midQ[i] = ses->server->ops->setup_request(ses, &rqst[i]);
+ midQ[i] = server->ops->setup_request(ses, &rqst[i]);
if (IS_ERR(midQ[i])) {
- revert_current_mid(ses->server, i);
+ revert_current_mid(server, i);
for (j = 0; j < i; j++)
cifs_delete_mid(midQ[j]);
- mutex_unlock(&ses->server->srv_mutex);
+ mutex_unlock(&server->srv_mutex);
/* Update # of requests on wire to server */
for (j = 0; j < num_rqst; j++)
- add_credits(ses->server, &credits[j], optype);
+ add_credits(server, &credits[j], optype);
return PTR_ERR(midQ[i]);
}
@@ -1059,19 +1061,19 @@ compound_send_recv(const unsigned int xid, struct cifs_ses *ses,
else
midQ[i]->callback = cifs_compound_last_callback;
}
- cifs_in_send_inc(ses->server);
- rc = smb_send_rqst(ses->server, num_rqst, rqst, flags);
- cifs_in_send_dec(ses->server);
+ cifs_in_send_inc(server);
+ rc = smb_send_rqst(server, num_rqst, rqst, flags);
+ cifs_in_send_dec(server);
for (i = 0; i < num_rqst; i++)
cifs_save_when_sent(midQ[i]);
if (rc < 0) {
- revert_current_mid(ses->server, num_rqst);
- ses->server->sequence_number -= 2;
+ revert_current_mid(server, num_rqst);
+ server->sequence_number -= 2;
}
- mutex_unlock(&ses->server->srv_mutex);
+ mutex_unlock(&server->srv_mutex);
/*
* If sending failed for some reason or it is an oplock break that we
@@ -1079,7 +1081,7 @@ compound_send_recv(const unsigned int xid, struct cifs_ses *ses,
*/
if (rc < 0 || (flags & CIFS_NO_SRV_RSP)) {
for (i = 0; i < num_rqst; i++)
- add_credits(ses->server, &credits[i], optype);
+ add_credits(server, &credits[i], optype);
goto out;
}
@@ -1099,7 +1101,7 @@ compound_send_recv(const unsigned int xid, struct cifs_ses *ses,
rqst[0].rq_nvec);
for (i = 0; i < num_rqst; i++) {
- rc = wait_for_response(ses->server, midQ[i]);
+ rc = wait_for_response(server, midQ[i]);
if (rc != 0)
break;
}
@@ -1107,7 +1109,7 @@ compound_send_recv(const unsigned int xid, struct cifs_ses *ses,
for (; i < num_rqst; i++) {
cifs_dbg(VFS, "Cancelling wait for mid %llu cmd: %d\n",
midQ[i]->mid, le16_to_cpu(midQ[i]->command));
- send_cancel(ses->server, &rqst[i], midQ[i]);
+ send_cancel(server, &rqst[i], midQ[i]);
spin_lock(&GlobalMid_Lock);
if (midQ[i]->mid_state == MID_REQUEST_SUBMITTED) {
midQ[i]->mid_flags |= MID_WAIT_CANCELLED;
@@ -1123,7 +1125,7 @@ compound_send_recv(const unsigned int xid, struct cifs_ses *ses,
if (rc < 0)
goto out;
- rc = cifs_sync_mid_result(midQ[i], ses->server);
+ rc = cifs_sync_mid_result(midQ[i], server);
if (rc != 0) {
/* mark this mid as cancelled to not free it below */
cancelled_mid[i] = true;
@@ -1140,14 +1142,14 @@ compound_send_recv(const unsigned int xid, struct cifs_ses *ses,
buf = (char *)midQ[i]->resp_buf;
resp_iov[i].iov_base = buf;
resp_iov[i].iov_len = midQ[i]->resp_buf_size +
- ses->server->vals->header_preamble_size;
+ server->vals->header_preamble_size;
if (midQ[i]->large_buf)
resp_buf_type[i] = CIFS_LARGE_BUFFER;
else
resp_buf_type[i] = CIFS_SMALL_BUFFER;
- rc = ses->server->ops->check_receive(midQ[i], ses->server,
+ rc = server->ops->check_receive(midQ[i], server,
flags & CIFS_LOG_ERROR);
/* mark it so buf will not be freed by cifs_delete_mid */
--
2.20.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: Minor cleanup of compound_send_recv
2019-06-24 18:29 Minor cleanup of compound_send_recv Steve French
@ 2019-06-24 21:41 ` ronnie sahlberg
2019-06-24 23:53 ` Steve French
2019-07-01 12:46 ` Aurélien Aptel
1 sibling, 1 reply; 4+ messages in thread
From: ronnie sahlberg @ 2019-06-24 21:41 UTC (permalink / raw)
To: Steve French; +Cc: CIFS, Aurélien Aptel
On Tue, Jun 25, 2019 at 7:24 AM Steve French <smfrench@gmail.com> wrote:
>
> In Aurelien's earlier patch series I noticed a cleanup (converting
> ses->server to a local variable server=ses->server) which made code
> easier to read in this function. This doesn't require compounding but
> probably helps his
>
LGTM.
Possibly initialize server where you declare it as we do that for
*server almost everywhere else :
+ struct TCP_Server_Info *server = ses->server;
Reviewed-by: Ronnie Sahlberg <lsahlber@redhat.com>
>
>
> --
> Thanks,
>
> Steve
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Minor cleanup of compound_send_recv
2019-06-24 21:41 ` ronnie sahlberg
@ 2019-06-24 23:53 ` Steve French
0 siblings, 0 replies; 4+ messages in thread
From: Steve French @ 2019-06-24 23:53 UTC (permalink / raw)
To: ronnie sahlberg; +Cc: CIFS, Aurélien Aptel
On Mon, Jun 24, 2019 at 4:41 PM ronnie sahlberg
<ronniesahlberg@gmail.com> wrote:
>
> On Tue, Jun 25, 2019 at 7:24 AM Steve French <smfrench@gmail.com> wrote:
> >
> > In Aurelien's earlier patch series I noticed a cleanup (converting
> > ses->server to a local variable server=ses->server) which made code
> > easier to read in this function. This doesn't require compounding but
> > probably helps his
> >
>
> LGTM.
>
> Possibly initialize server where you declare it as we do that for
> *server almost everywhere else :
>
> + struct TCP_Server_Info *server = ses->server;
I didn't set it there because later in the function we check:
if ((ses == NULL) || (ses->server == NULL))
so I need to make sure the assignment is after the null check
(even if the null check is superfluous it would trigger a static check
warning to initialize server before the null check)
--
Thanks,
Steve
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Minor cleanup of compound_send_recv
2019-06-24 18:29 Minor cleanup of compound_send_recv Steve French
2019-06-24 21:41 ` ronnie sahlberg
@ 2019-07-01 12:46 ` Aurélien Aptel
1 sibling, 0 replies; 4+ messages in thread
From: Aurélien Aptel @ 2019-07-01 12:46 UTC (permalink / raw)
To: Steve French, CIFS
Hi,
I was away for 2 weeks, reading through all my emails now.
Steve French <smfrench@gmail.com> writes:
> In Aurelien's earlier patch series I noticed a cleanup (converting
> ses->server to a local variable server=ses->server) which made code
> easier to read in this function. This doesn't require compounding but
> probably helps his
Assuming we keep my approach that is a good idea, thanks.
--
Aurélien Aptel / SUSE Labs Samba Team
GPG: 1839 CB5F 9F5B FB9B AA97 8C99 03C8 A49B 521B D5D3
SUSE Linux GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah HRB 21284 (AG Nürnberg)
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-07-01 12:46 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-24 18:29 Minor cleanup of compound_send_recv Steve French
2019-06-24 21:41 ` ronnie sahlberg
2019-06-24 23:53 ` Steve French
2019-07-01 12:46 ` Aurélien Aptel
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.