On 12/17/2010 08:38 PM, Jeff Layton wrote:
> The TCP_Server_Info is refcounted and every SMB session holds a
> reference to it. Thus, smb_ses_list is always going to be empty when
> cifsd is coming down. This is dead code.
>
> Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx>
> ---
> fs/cifs/connect.c | 44 +++-----------------------------------------
> 1 files changed, 3 insertions(+), 41 deletions(-)
>
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index 9fe620e2..67d3d0d 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -346,7 +346,6 @@ cifs_demultiplex_thread(struct TCP_Server_Info *server)
> struct kvec iov;
> struct socket *csocket = server->ssocket;
> struct list_head *tmp;
> - struct cifsSesInfo *ses;
> struct task_struct *task_to_wake = NULL;
> struct mid_q_entry *mid_entry;
> char temp;
> @@ -677,44 +676,19 @@ multi_t2_fnd:
> if (smallbuf) /* no sense logging a debug message if NULL */
> cifs_small_buf_release(smallbuf);
>
> - /*
> - * BB: we shouldn't have to do any of this. It shouldn't be
> - * possible to exit from the thread with active SMB sessions
> - */
> - spin_lock(&cifs_tcp_ses_lock);
> - if (list_empty(&server->pending_mid_q)) {
> - /* loop through server session structures attached to this and
> - mark them dead */
> - list_for_each(tmp, &server->smb_ses_list) {
> - ses = list_entry(tmp, struct cifsSesInfo,
> - smb_ses_list);
> - ses->status = CifsExiting;
> - ses->server = NULL;
> - }
> - spin_unlock(&cifs_tcp_ses_lock);
> - } else {
> - /* 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 */
> - list_for_each(tmp, &server->smb_ses_list) {
> - ses = list_entry(tmp, struct cifsSesInfo,
> - smb_ses_list);
> - ses->status = CifsExiting;
> - }
> -
> + if (!list_empty(&server->pending_mid_q)) {
> spin_lock(&GlobalMid_Lock);
> list_for_each(tmp, &server->pending_mid_q) {
> - mid_entry = list_entry(tmp, struct mid_q_entry, qhead);
> + mid_entry = list_entry(tmp, struct mid_q_entry, qhead);
> if (mid_entry->midState == MID_REQUEST_SUBMITTED) {
> cFYI(1, "Clearing Mid 0x%x - waking up ",
> - mid_entry->mid);
> + mid_entry->mid);
> task_to_wake = mid_entry->tsk;
> if (task_to_wake)
> wake_up_process(task_to_wake);
> }
> }
> spin_unlock(&GlobalMid_Lock);
> - spin_unlock(&cifs_tcp_ses_lock);
> /* 1/8th of sec is more than enough time for them to exit */
> msleep(125);
> }
> @@ -732,18 +706,6 @@ multi_t2_fnd:
> coming home not much else we can do but free the memory */
> }
>
> - /* last chance to mark ses pointers invalid
> - if there are any pointing to this (e.g
> - if a crazy root user tried to kill cifsd
> - kernel thread explicitly this might happen) */
The above comment sounds little scary. Though, I don't see how this
could happen, I think it's better to test the above scenario to be sure
(I guess you would have already taken care?)
> - /* BB: This shouldn't be necessary, see above */
> - spin_lock(&cifs_tcp_ses_lock);
> - list_for_each(tmp, &server->smb_ses_list) {
> - ses = list_entry(tmp, struct cifsSesInfo, smb_ses_list);
> - ses->server = NULL;
> - }
> - spin_unlock(&cifs_tcp_ses_lock);
> -
> kfree(server->hostname);
> task_to_wake = xchg(&server->tsk, NULL);
> kfree(server);
--
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
