Very nice.
Reviewed-by: Ronnie Sahlberg <lsahlber@xxxxxxxxxx
>
On Fri, Nov 22, 2019 at 5:35 AM Pavel Shilovsky <piastryyy@xxxxxxxxx> wrote:
>
> When an OPEN command is cancelled we mark a mid as
> cancelled and let the demultiplex thread process it
> by closing an open handle. The problem is there is
> a race between a system call thread and the demultiplex
> thread and there may be a situation when the mid has
> been already processed before it is set as cancelled.
>
> Fix this by processing cancelled requests when mids
> are being destroyed which means that there is only
> one thread referencing a particular mid. Also set
> mids as cancelled unconditionally on their state.
>
> Cc: Stable <stable@xxxxxxxxxxxxxxx>
> Signed-off-by: Pavel Shilovsky <pshilov@xxxxxxxxxxxxx>
> ---
> fs/cifs/connect.c | 6 ------
> fs/cifs/transport.c | 10 ++++++++--
> 2 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index e63d16d8048a..59feb2de389e 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -1229,12 +1229,6 @@ cifs_demultiplex_thread(void *p)
> for (i = 0; i < num_mids; i++) {
> if (mids[i] != NULL) {
> mids[i]->resp_buf_size = server->pdu_size;
> - if ((mids[i]->mid_flags & MID_WAIT_CANCELLED) &&
> - mids[i]->mid_state == MID_RESPONSE_RECEIVED &&
> - server->ops->handle_cancelled_mid)
> - server->ops->handle_cancelled_mid(
> - mids[i]->resp_buf,
> - server);
>
> if (!mids[i]->multiRsp || mids[i]->multiEnd)
> mids[i]->callback(mids[i]);
> diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c
> index bb52751ba783..987ffcd5ca3a 100644
> --- a/fs/cifs/transport.c
> +++ b/fs/cifs/transport.c
> @@ -93,8 +93,14 @@ static void _cifs_mid_q_entry_release(struct kref *refcount)
> __u16 smb_cmd = le16_to_cpu(midEntry->command);
> unsigned long now;
> unsigned long roundtrip_time;
> - struct TCP_Server_Info *server = midEntry->server;
> #endif
> + struct TCP_Server_Info *server = midEntry->server;
> +
> + if (midEntry->resp_buf && (midEntry->mid_flags & MID_WAIT_CANCELLED) &&
> + midEntry->mid_state == MID_RESPONSE_RECEIVED &&
> + server->ops->handle_cancelled_mid)
> + server->ops->handle_cancelled_mid(midEntry->resp_buf, server);
> +
> midEntry->mid_state = MID_FREE;
> atomic_dec(&midCount);
> if (midEntry->large_buf)
> @@ -1115,8 +1121,8 @@ compound_send_recv(const unsigned int xid, struct cifs_ses *ses,
> midQ[i]->mid, le16_to_cpu(midQ[i]->command));
> send_cancel(server, &rqst[i], midQ[i]);
> spin_lock(&GlobalMid_Lock);
> + midQ[i]->mid_flags |= MID_WAIT_CANCELLED;
> if (midQ[i]->mid_state == MID_REQUEST_SUBMITTED) {
> - midQ[i]->mid_flags |= MID_WAIT_CANCELLED;
> midQ[i]->callback = cifs_cancelled_callback;
> cancelled_mid[i] = true;
> credits[i].value = 0;
> --
> 2.17.1
>
