On 12/17/2010 08:38 PM, Jeff Layton wrote:
> In order to incorporate async requests, we need to allow for a more
> general way to do things on receive, rather than just waking up a
> process.
>
> Turn the task pointer in the mid_q_entry into a callback function and a
> generic data pointer. When a response comes in, or the socket is
> reconnected, cifsd can call the callback function in order to wake up
> the process.
>
> The default is to just wake up the current process which should mean no
> change in behavior for existing code.
>
> Also, clean up the locking in cifs_reconnect. There doesn't seem to be
> any need to hold both the srv_mutex and GlobalMid_Lock when walking the
> list of mids.
>
> Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx>
> ---
> fs/cifs/cifs_debug.c | 8 +++---
> fs/cifs/cifsglob.h | 15 ++++++++++++-
> fs/cifs/connect.c | 56 +++++++++++++++++++++++++-------------------------
> fs/cifs/transport.c | 19 +++++++++++++++-
> 4 files changed, 63 insertions(+), 35 deletions(-)
>
> diff --git a/fs/cifs/cifs_debug.c b/fs/cifs/cifs_debug.c
> index ac7a38f..30d01bc 100644
> --- a/fs/cifs/cifs_debug.c
> +++ b/fs/cifs/cifs_debug.c
> @@ -79,11 +79,11 @@ void cifs_dump_mids(struct TCP_Server_Info *server)
> spin_lock(&GlobalMid_Lock);
> list_for_each(tmp, &server->pending_mid_q) {
> mid_entry = list_entry(tmp, struct mid_q_entry, qhead);
> - cERROR(1, "State: %d Cmd: %d Pid: %d Tsk: %p Mid %d",
> + cERROR(1, "State: %d Cmd: %d Pid: %d Cbdata: %p Mid %d",
> mid_entry->midState,
> (int)mid_entry->command,
> mid_entry->pid,
> - mid_entry->tsk,
> + mid_entry->callback_data,
> mid_entry->mid);
> #ifdef CONFIG_CIFS_STATS2
> cERROR(1, "IsLarge: %d buf: %p time rcv: %ld now: %ld",
> @@ -218,11 +218,11 @@ static int cifs_debug_data_proc_show(struct seq_file *m, void *v)
> mid_entry = list_entry(tmp3, struct mid_q_entry,
> qhead);
> seq_printf(m, "\tState: %d com: %d pid:"
> - " %d tsk: %p mid %d\n",
> + " %d cbdata: %p mid %d\n",
> mid_entry->midState,
> (int)mid_entry->command,
> mid_entry->pid,
> - mid_entry->tsk,
> + mid_entry->callback_data,
> mid_entry->mid);
> }
> spin_unlock(&GlobalMid_Lock);
> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> index 8c3c165..db90fe0 100644
> --- a/fs/cifs/cifsglob.h
> +++ b/fs/cifs/cifsglob.h
> @@ -508,6 +508,18 @@ static inline void cifs_stats_bytes_read(struct cifsTconInfo *tcon,
>
> #endif
>
> +struct mid_q_entry;
> +
> +/*
> + * This is the prototype for the mid callback function. When creating one,
> + * take special care to avoid deadlocks. Things to bear in mind:
> + *
> + * - it will be called by cifsd
> + * - the GlobalMid_Lock will be held
> + * - the mid will be removed from the pending_mid_q list
> + */
> +typedef void (mid_callback_t)(struct mid_q_entry *mid);
> +
> /* one of these for every pending CIFS request to the server */
> struct mid_q_entry {
> struct list_head qhead; /* mids waiting on reply from this server */
> @@ -519,7 +531,8 @@ struct mid_q_entry {
> unsigned long when_sent; /* time when smb send finished */
> unsigned long when_received; /* when demux complete (taken off wire) */
> #endif
> - struct task_struct *tsk; /* task waiting for response */
> + mid_callback_t *callback; /* call completion callback */
> + void *callback_data; /* general purpose pointer for callback */
> struct smb_hdr *resp_buf; /* response buffer */
> int midState; /* wish this were enum but can not pass to wait_event */
> __u8 command; /* smb command code */
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index 515cafa..734fb09 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -152,6 +152,7 @@ cifs_reconnect(struct TCP_Server_Info *server)
>
> /* before reconnecting the tcp session, mark the smb session (uid)
> and the tid bad so they are not used until reconnected */
> + cFYI(1, "%s: marking sessions and tcons for reconnect", __func__);
> spin_lock(&cifs_tcp_ses_lock);
> list_for_each(tmp, &server->smb_ses_list) {
> ses = list_entry(tmp, struct cifsSesInfo, smb_ses_list);
> @@ -163,7 +164,9 @@ cifs_reconnect(struct TCP_Server_Info *server)
> }
> }
> spin_unlock(&cifs_tcp_ses_lock);
> +
> /* do not want to be sending data on a socket we are freeing */
> + cFYI(1, "%s: tearing down socket", __func__);
> mutex_lock(&server->srv_mutex);
> if (server->ssocket) {
> cFYI(1, "State: 0x%x Flags: 0x%lx", server->ssocket->state,
> @@ -180,22 +183,22 @@ cifs_reconnect(struct TCP_Server_Info *server)
> kfree(server->session_key.response);
> server->session_key.response = NULL;
> server->session_key.len = 0;
> + mutex_unlock(&server->srv_mutex);
>
> + /*
> + * move in-progress mids to a private list so that we can walk it later
> + * without needing a lock. We'll mark them for retry after reconnect.
> + */
> + cFYI(1, "%s: issuing mid callbacks", __func__);
Is the above comment valid? This patchset doesn't use a separate list IIUC?
--
Suresh Jayaraman
--
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
