2010/12/10 Jeff Layton <jlayton@xxxxxxxxxx>:
> Currently, when a request is cancelled via signal, we delete the mid
> immediately. If the request was already transmitted however, the client
> is still likely to receive a response. When it does, it won't recognize
> it however and will pop a printk.
>
> It's also a little dangerous to just delete the mid entry like this. We
> may end up reusing that mid. If we do then we could potentially get the
> response from the first request confused with the later one.
>
> Prevent the reuse of mids by marking them as cancelled and keeping them
> on the pending_mid_q list. If the reply comes in, we'll delete it from
> the list then. If it never comes, then we'll delete it at reconnect
> or when cifsd comes down.
>
> Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx>
> ---
> fs/cifs/cifsglob.h | 2 +-
> fs/cifs/cifsproto.h | 1 +
> fs/cifs/connect.c | 44 +++++++++++++++++++++++++++++++++++++-------
> fs/cifs/transport.c | 30 ++++++++++++++++++++++--------
> 4 files changed, 61 insertions(+), 16 deletions(-)
>
> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> index cc43ada..fb75f04 100644
> --- a/fs/cifs/cifsglob.h
> +++ b/fs/cifs/cifsglob.h
> @@ -621,7 +621,7 @@ static inline void free_dfs_info_array(struct dfs_info3_param *param,
> #define MID_REQUEST_SUBMITTED 2
> #define MID_RESPONSE_RECEIVED 4
> #define MID_RETRY_NEEDED 8 /* session closed while this request out */
> -#define MID_NO_RESP_NEEDED 0x10
> +#define MID_REQUEST_CANCELLED 0x10 /* discard any reply */
>
> /* Types of response buffer returned from SendReceive2 */
> #define CIFS_NO_BUFFER 0 /* Response buffer not returned */
> diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
> index fe77e69..a8fc606 100644
> --- a/fs/cifs/cifsproto.h
> +++ b/fs/cifs/cifsproto.h
> @@ -61,6 +61,7 @@ extern char *cifs_compose_mount_options(const char *sb_mountdata,
> const char *fullpath, const struct dfs_info3_param *ref,
> char **devname);
> /* extern void renew_parental_timestamps(struct dentry *direntry);*/
> +extern void DeleteMidQEntry(struct mid_q_entry *midEntry);
> extern int SendReceive(const unsigned int /* xid */ , struct cifsSesInfo *,
> struct smb_hdr * /* input */ ,
> struct smb_hdr * /* out */ ,
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index 7e20ece..0feb592 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -133,7 +133,7 @@ cifs_reconnect(struct TCP_Server_Info *server)
> {
> int rc = 0;
> struct list_head *tmp, *tmp2;
> - struct list_head retry;
> + struct list_head retry, dispose;
> struct cifsSesInfo *ses;
> struct cifsTconInfo *tcon;
> struct mid_q_entry *mid_entry;
> @@ -192,14 +192,23 @@ cifs_reconnect(struct TCP_Server_Info *server)
> */
> cFYI(1, "%s: moving mids to retry list", __func__);
> INIT_LIST_HEAD(&retry);
> + INIT_LIST_HEAD(&dispose);
> spin_lock(&GlobalMid_Lock);
> list_for_each_safe(tmp, tmp2, &server->pending_mid_q) {
> mid_entry = list_entry(tmp, struct mid_q_entry, qhead);
> if (mid_entry->midState == MID_REQUEST_SUBMITTED)
> list_move(tmp, &retry);
> + else if (mid_entry->midState == MID_REQUEST_CANCELLED)
> + list_move(tmp, &dispose);
> }
> spin_unlock(&GlobalMid_Lock);
>
> + /* now walk private dispose list and delete entries */
> + list_for_each_safe(tmp, tmp2, &dispose) {
> + mid_entry = list_entry(tmp, struct mid_q_entry, qhead);
> + DeleteMidQEntry(mid_entry);
> + }
> +
> while ((server->tcpStatus != CifsExiting) &&
> (server->tcpStatus != CifsGood)) {
> try_to_freeze();
> @@ -219,7 +228,7 @@ cifs_reconnect(struct TCP_Server_Info *server)
> }
> }
>
> - /* now, issue callback for all mids in flight */
> + /* issue callback for all mids in flight */
> list_for_each_safe(tmp, tmp2, &retry) {
> list_del_init(tmp);
> mid_entry = list_entry(tmp, struct mid_q_entry, qhead);
> @@ -575,9 +584,13 @@ incomplete_rcv:
> list_for_each(tmp, &server->pending_mid_q) {
> mid_entry = list_entry(tmp, struct mid_q_entry, qhead);
>
> - if ((mid_entry->mid == smb_buffer->Mid) &&
> - (mid_entry->midState == MID_REQUEST_SUBMITTED) &&
> - (mid_entry->command == smb_buffer->Command)) {
> + if (mid_entry->mid != smb_buffer->Mid)
> + goto next_mid;
> + if (mid_entry->command != smb_buffer->Command)
> + goto next_mid;
> + if (mid_entry->midState == MID_REQUEST_CANCELLED)
> + break;
> + if (mid_entry->midState == MID_REQUEST_SUBMITTED) {
> if (check2ndT2(smb_buffer,server->maxBuf) > 0) {
> /* We have a multipart transact2 resp */
> isMultiRsp = true;
> @@ -623,11 +636,16 @@ multi_t2_fnd:
> server->lstrp = jiffies;
> break;
> }
> +next_mid:
> mid_entry = NULL;
> }
> spin_unlock(&GlobalMid_Lock);
>
> if (mid_entry != NULL) {
> + if (mid_entry->midState == MID_REQUEST_CANCELLED) {
> + DeleteMidQEntry(mid_entry);
> + continue;
> + }
> mid_entry->callback(mid_entry);
> /* Was previous buf put in mpx struct for multi-rsp? */
> if (!isMultiRsp) {
> @@ -704,6 +722,9 @@ multi_t2_fnd:
> }
> spin_unlock(&cifs_tcp_ses_lock);
> } else {
> + struct mid_q_entry *tmp_mid;
> + struct list_head dispose;
> +
> /* although we can not zero the server struct pointer yet,
> since there are active requests which may depnd on them,
> mark the corresponding SMB sessions as exiting too */
> @@ -713,17 +734,26 @@ multi_t2_fnd:
> ses->status = CifsExiting;
> }
>
> + INIT_LIST_HEAD(&dispose);
> spin_lock(&GlobalMid_Lock);
> - list_for_each(tmp, &server->pending_mid_q) {
> - mid_entry = list_entry(tmp, struct mid_q_entry, qhead);
> + list_for_each_entry_safe(mid_entry, tmp_mid,
> + &server->pending_mid_q, qhead) {
> if (mid_entry->midState == MID_REQUEST_SUBMITTED) {
> cFYI(1, "Clearing Mid 0x%x - issuing callback",
> mid_entry->mid);
> mid_entry->callback(mid_entry);
> + } else if (mid_entry->midState == MID_REQUEST_CANCELLED) {
> + cFYI(1, "Clearing Mid 0x%x - Cancelled",
> + mid_entry->mid);
> + list_move(&mid_entry->qhead, &dispose);
Why do we need another list here? It seems to me that we can simply
delete cancelled mid.
> }
> }
> spin_unlock(&GlobalMid_Lock);
> spin_unlock(&cifs_tcp_ses_lock);
> +
> + /* now delete all of the cancelled mids */
> + list_for_each_entry_safe(mid_entry, tmp_mid, &dispose, qhead)
> + DeleteMidQEntry(mid_entry);
> /* 1/8th of sec is more than enough time for them to exit */
> msleep(125);
> }
> diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c
> index 79647db..97a1170 100644
> --- a/fs/cifs/transport.c
> +++ b/fs/cifs/transport.c
> @@ -81,7 +81,7 @@ AllocMidQEntry(const struct smb_hdr *smb_buffer, struct TCP_Server_Info *server)
> return temp;
> }
>
> -static void
> +void
> DeleteMidQEntry(struct mid_q_entry *midEntry)
> {
> #ifdef CONFIG_CIFS_STATS2
> @@ -480,8 +480,13 @@ SendReceive2(const unsigned int xid, struct cifsSesInfo *ses,
> goto out;
>
> rc = wait_for_response(ses->server, midQ);
> - if (rc != 0)
> - goto out;
> + if (rc != 0) {
> + /* no longer considered to be "in-flight" */
> + midQ->midState = MID_REQUEST_CANCELLED;
> + atomic_dec(&ses->server->inFlight);
> + wake_up(&ses->server->request_q);
> + return rc;
> + }
>
> rc = handle_mid_result(midQ, ses->server);
> if (rc != 0)
> @@ -623,8 +628,13 @@ SendReceive(const unsigned int xid, struct cifsSesInfo *ses,
> goto out;
>
> rc = wait_for_response(ses->server, midQ);
> - if (rc != 0)
> - goto out;
> + if (rc != 0) {
> + /* no longer considered to be "in-flight" */
> + midQ->midState = MID_REQUEST_CANCELLED;
> + atomic_dec(&ses->server->inFlight);
> + wake_up(&ses->server->request_q);
> + return rc;
> + }
>
> rc = handle_mid_result(midQ, ses->server);
> if (rc != 0)
> @@ -842,10 +852,14 @@ SendReceiveBlockingLock(const unsigned int xid, struct cifsTconInfo *tcon,
> }
> }
>
> - if (wait_for_response(ses->server, midQ) == 0) {
> - /* We got the response - restart system call. */
> - rstart = 1;
> + rc = wait_for_response(ses->server, midQ);
> + if (rc) {
> + midQ->midState = MID_REQUEST_CANCELLED;
> + return rc;
> }
> +
> + /* We got the response - restart system call. */
> + rstart = 1;
> }
>
> rc = handle_mid_result(midQ, ses->server);
> --
> 1.7.3.2
>
> --
> 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
>
--
Best regards,
Pavel Shilovsky.
--
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
