On Fri, 16 Mar 2012 18:09:28 +0300
Pavel Shilovsky <piastry@xxxxxxxxxxx> wrote:
> that is essential for CIFS/SMB/SMB2 oplock breaks and SMB2 echos.
>
> Signed-off-by: Pavel Shilovsky <piastry@xxxxxxxxxxx>
> ---
> fs/cifs/cifsglob.h | 14 ++++++++++++--
> fs/cifs/transport.c | 22 ++++++++++++++--------
> 2 files changed, 26 insertions(+), 10 deletions(-)
>
> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> index d55de96..2309a67 100644
> --- a/fs/cifs/cifsglob.h
> +++ b/fs/cifs/cifsglob.h
> @@ -315,12 +315,22 @@ in_flight(struct TCP_Server_Info *server)
> return num;
> }
>
> +static inline int*
> +get_credits_field(struct TCP_Server_Info *server)
> +{
> + /*
> + * This will change to switch statement when we reserve slots for echos
> + * and oplock breaks.
> + */
> + return &server->credits;
> +}
> +
> static inline bool
> -has_credits(struct TCP_Server_Info *server)
> +has_credits(struct TCP_Server_Info *server, int *credits)
Why are you passing "credits" by reference here? Might as well pass by
value...
> {
> int num;
> spin_lock(&server->req_lock);
> - num = server->credits;
> + num = *credits;
> spin_unlock(&server->req_lock);
> return num > 0;
> }
> diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c
> index 11787cc..20dc7ba 100644
> --- a/fs/cifs/transport.c
> +++ b/fs/cifs/transport.c
> @@ -255,26 +255,26 @@ smb_send(struct TCP_Server_Info *server, struct smb_hdr *smb_buffer,
> }
>
> static int
> -wait_for_free_request(struct TCP_Server_Info *server, const int long_op)
> +wait_for_free_credits(struct TCP_Server_Info *server, const int optype,
> + int *credits)
> {
> int rc;
>
> spin_lock(&server->req_lock);
> -
> - if (long_op == CIFS_ASYNC_OP) {
> + if (optype == CIFS_ASYNC_OP) {
> /* oplock breaks must not be held up */
> server->in_flight++;
> - server->credits--;
> + *credits -= 1;
> spin_unlock(&server->req_lock);
> return 0;
> }
>
> while (1) {
> - if (server->credits <= 0) {
> + if (*credits <= 0) {
> spin_unlock(&server->req_lock);
> cifs_num_waiters_inc(server);
> rc = wait_event_killable(server->request_q,
> - has_credits(server));
> + has_credits(server, credits));
> cifs_num_waiters_dec(server);
> if (rc)
> return rc;
> @@ -291,8 +291,8 @@ wait_for_free_request(struct TCP_Server_Info *server, const int long_op)
> */
>
> /* update # of requests on the wire to server */
> - if (long_op != CIFS_BLOCKING_OP) {
> - server->credits--;
> + if (optype != CIFS_BLOCKING_OP) {
> + *credits -= 1;
> server->in_flight++;
> }
> spin_unlock(&server->req_lock);
> @@ -302,6 +302,12 @@ wait_for_free_request(struct TCP_Server_Info *server, const int long_op)
> return 0;
> }
>
> +static int
> +wait_for_free_request(struct TCP_Server_Info *server, const int optype)
> +{
> + return wait_for_free_credits(server, optype, get_credits_field(server));
> +}
> +
> static int allocate_mid(struct cifs_ses *ses, struct smb_hdr *in_buf,
> struct mid_q_entry **ppmidQ)
> {
Other than that nit, this looks fine...
Reviewed-by: Jeff Layton <jlayton@xxxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
