вт, 19 февр. 2019 г. в 22:12, Ronnie Sahlberg <lsahlber@xxxxxxxxxx>:
>
> Change wait_for_free_request to allow specifying how many requests(credits)
> to wait for.
> Pass 1 from all callsites.
>
> This should not change any behavior but will allow future callers to
> atomically wait for >1 requests.
>
> Signed-off-by: Ronnie Sahlberg <lsahlber@xxxxxxxxxx>
> ---
> fs/cifs/cifsglob.h | 4 ++--
> fs/cifs/smb2ops.c | 2 +-
> fs/cifs/transport.c | 25 ++++++++++++++-----------
> 3 files changed, 17 insertions(+), 14 deletions(-)
>
> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> index 5bf463c2d9dc..1b3f62eee4d0 100644
> --- a/fs/cifs/cifsglob.h
> +++ b/fs/cifs/cifsglob.h
> @@ -732,13 +732,13 @@ in_flight(struct TCP_Server_Info *server)
> }
>
> static inline bool
> -has_credits(struct TCP_Server_Info *server, int *credits)
> +has_credits(struct TCP_Server_Info *server, int *credits, int num_credits)
> {
> int num;
> spin_lock(&server->req_lock);
> num = *credits;
> spin_unlock(&server->req_lock);
> - return num > 0;
> + return num >= num_credits;
> }
>
> static inline void
> diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
> index 080929a64c64..372a3563b79b 100644
> --- a/fs/cifs/smb2ops.c
> +++ b/fs/cifs/smb2ops.c
> @@ -185,7 +185,7 @@ smb2_wait_mtu_credits(struct TCP_Server_Info *server, unsigned int size,
> spin_unlock(&server->req_lock);
> cifs_num_waiters_inc(server);
> rc = wait_event_killable(server->request_q,
> - has_credits(server, &server->credits));
> + has_credits(server, &server->credits, 1));
> cifs_num_waiters_dec(server);
> if (rc)
> return rc;
> diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c
> index d4f1224fb7cb..8ce5b6e787c6 100644
> --- a/fs/cifs/transport.c
> +++ b/fs/cifs/transport.c
> @@ -478,7 +478,8 @@ smb_send(struct TCP_Server_Info *server, struct smb_hdr *smb_buffer,
>
> static int
> wait_for_free_credits(struct TCP_Server_Info *server, const int timeout,
> - int *credits, unsigned int *instance)
> + int num_credits, int *credits,
> + unsigned int *instance)
> {
> int rc;
>
> @@ -495,11 +496,11 @@ wait_for_free_credits(struct TCP_Server_Info *server, const int timeout,
> }
>
> while (1) {
> - if (*credits <= 0) {
> + if (*credits < num_credits) {
> spin_unlock(&server->req_lock);
> cifs_num_waiters_inc(server);
> rc = wait_event_killable(server->request_q,
> - has_credits(server, credits));
> + has_credits(server, credits, num_credits));
> cifs_num_waiters_dec(server);
> if (rc)
> return rc;
> @@ -517,8 +518,8 @@ wait_for_free_credits(struct TCP_Server_Info *server, const int timeout,
>
> /* update # of requests on the wire to server */
> if (timeout != CIFS_BLOCKING_OP) {
> - *credits -= 1;
> - server->in_flight++;
> + *credits -= num_credits;
> + server->in_flight += num_credits;
> *instance = server->reconnect_instance;
> }
> spin_unlock(&server->req_lock);
> @@ -530,7 +531,7 @@ wait_for_free_credits(struct TCP_Server_Info *server, const int timeout,
>
> static int
> wait_for_free_request(struct TCP_Server_Info *server, const int timeout,
> - const int optype, unsigned int *instance)
> + int num, const int optype, unsigned int *instance)
Waiting for more than one credit without a timeout is a source of
possibility to be stuck. We already have 'timeout' argument which
contains some part of flags passed to transport layer from PDU. We
might consider renaming 'optype' argument into flags and use 'timeout'
with its true meaning - timeout of the waiting for credits.
Once implemented, we can define some default period of time which we
can *afford* to wait for credits and then fall back to sequential code
if we timed out.
I also suggest to add another function wait_for_compound_credits and
make wait_for_free_request calls it - doesn't require changes to all
the places where the latter is being used.
> {
> int *val;
>
> @@ -538,7 +539,7 @@ wait_for_free_request(struct TCP_Server_Info *server, const int timeout,
> /* Since an echo is already inflight, no need to wait to send another */
> if (*val <= 0 && optype == CIFS_ECHO_OP)
> return -EAGAIN;
> - return wait_for_free_credits(server, timeout, val, instance);
> + return wait_for_free_credits(server, timeout, num, val, instance);
> }
>
> int
> @@ -646,7 +647,8 @@ cifs_call_async(struct TCP_Server_Info *server, struct smb_rqst *rqst,
> optype = flags & CIFS_OP_MASK;
>
> if ((flags & CIFS_HAS_CREDITS) == 0) {
> - rc = wait_for_free_request(server, timeout, optype, &instance);
> + rc = wait_for_free_request(server, timeout, 1, optype,
> + &instance);
> if (rc)
> return rc;
> credits.value = 1;
> @@ -923,7 +925,7 @@ compound_send_recv(const unsigned int xid, struct cifs_ses *ses,
> * Ensure we obtain 1 credit per request in the compound chain.
> */
> for (i = 0; i < num_rqst; i++) {
> - rc = wait_for_free_request(ses->server, timeout, optype,
> + rc = wait_for_free_request(ses->server, timeout, 1, optype,
> &instance);
>
> if (rc == 0) {
> @@ -1212,7 +1214,8 @@ SendReceive(const unsigned int xid, struct cifs_ses *ses,
> return -EIO;
> }
>
> - rc = wait_for_free_request(ses->server, timeout, 0, &credits.instance);
> + rc = wait_for_free_request(ses->server, timeout, 1, 0,
> + &credits.instance);
> if (rc)
> return rc;
>
> @@ -1354,7 +1357,7 @@ SendReceiveBlockingLock(const unsigned int xid, struct cifs_tcon *tcon,
> return -EIO;
> }
>
> - rc = wait_for_free_request(ses->server, CIFS_BLOCKING_OP, 0,
> + rc = wait_for_free_request(ses->server, CIFS_BLOCKING_OP, 1, 0,
> &instance);
> if (rc)
> return rc;
> --
> 2.13.6
>
--
Best regards,
Pavel Shilovsky
