I recently sent a similar, smaller patch for this problem. After some discussion with Steve and Shaggy, I think I better understand why cifsd allows signals through, and I realize that my earlier patch wasn't comprehensive enough The mount and unmount calls will send a KILL signal to cifsd to wake it up if it happens to be blocked in kernel_recvmsg. The problem is that it doesn't distinguish between "legitimate" signals sent for this reason and spurious signals sent by a userspace process (for instance). While this is definitely a "don't do that" sort of situation, we might as well try to have cifsd be as signal-safe as possible. The following patch does this by making sure that we set tcpStatus to CifsExiting before sending cifsd a signal, and having cifsd check for that when it sees that it's been signalled. If the tcpStatus is not set correctly, it ignores it, flushes signals and moves on. I've tested a similar backported version of this on an earlier kernel, but have not tested this particular patch as of yet. Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c index f4e9266..461fbaa 100644 --- a/fs/cifs/connect.c +++ b/fs/cifs/connect.c @@ -197,6 +197,11 @@ cifs_reconnect(struct TCP_Server_Info *server) server->server_RFC1001_name); } if(rc) { + if (rc == -ERESTARTSYS) { + cFYI(1,("reconnect interrupted by signal")); + flush_signals(server->tsk); + continue; + } cFYI(1,("reconnect error %d",rc)); msleep(3000); } else { @@ -425,8 +430,14 @@ cifs_demultiplex_thread(struct TCP_Server_Info *server) break; } if (!try_to_freeze() && (length == -EINTR)) { - cFYI(1,("cifsd thread killed")); - break; + if (server->tcpStatus == CifsExiting) { + cFYI(1,("cifsd thread killed")); + break; + } else { + cFYI(1,("cifsd caught spurious signal, ignoring it")); + flush_signals(server->tsk); + continue; + } } cFYI(1,("Reconnect after unexpected peek error %d", length)); @@ -526,11 +537,15 @@ cifs_demultiplex_thread(struct TCP_Server_Info *server) total_read += length) { length = kernel_recvmsg(csocket, &smb_msg, &iov, 1, pdu_length - total_read, 0); - if( kthread_should_stop() || - (length == -EINTR)) { - /* then will exit */ + if (kthread_should_stop() || + (length == -EINTR && + server->tcpStatus == CifsExiting)) { reconnect = 2; break; + } else if (length == -EINTR) { + cFYI(1, ("cifsd caught spurious signal, ignoring it")); + flush_signals(server->tsk); + continue; } else if (server->tcpStatus == CifsNeedReconnect) { cifs_reconnect(server); csocket = server->ssocket; @@ -1692,6 +1707,26 @@ void reset_cifs_unix_caps(int xid, struct cifsTconInfo * tcon, } } +static inline void stop_cifsd(struct TCP_Server_Info *srv) +{ + struct task_struct *tsk = srv->tsk; + + cFYI(1,("shutting down cifsd thread")); + + spin_lock(&GlobalMid_Lock); + srv->tcpStatus = CifsExiting; + spin_unlock(&GlobalMid_Lock); + + /* If we could verify that kthread_stop would + * always wake up processes blocked in + * tcp in recv_mesg then we could remove the + * send_sig call */ + send_sig(SIGKILL, tsk, 1); + + if (tsk) + kthread_stop(tsk); +} + int cifs_mount(struct super_block *sb, struct cifs_sb_info *cifs_sb, char *mount_data, const char *devname) @@ -2064,22 +2099,9 @@ cifs_mount(struct super_block *sb, struct cifs_sb_info *cifs_sb, if (rc) { /* if session setup failed, use count is zero but we still need to free cifsd thread */ - if (atomic_read(&srvTcp->socketUseCount) == 0) { - spin_lock(&GlobalMid_Lock); - srvTcp->tcpStatus = CifsExiting; - spin_unlock(&GlobalMid_Lock); - if (srvTcp->tsk) { - struct task_struct *tsk; - /* If we could verify that kthread_stop would - always wake up processes blocked in - tcp in recv_mesg then we could remove the - send_sig call */ - send_sig(SIGKILL,srvTcp->tsk,1); - tsk = srvTcp->tsk; - if(tsk) - kthread_stop(tsk); - } - } + if (atomic_read(&srvTcp->socketUseCount) == 0) + stop_cifsd(srvTcp); + /* If find_unc succeeded then rc == 0 so we can not end */ if (tcon) /* up accidently freeing someone elses tcon struct */ tconInfoFree(tcon); @@ -2091,13 +2113,8 @@ cifs_mount(struct super_block *sb, struct cifs_sb_info *cifs_sb, temp_rc = CIFSSMBLogoff(xid, pSesInfo); /* if the socketUseCount is now zero */ if ((temp_rc == -ESHUTDOWN) && - (pSesInfo->server) && (pSesInfo->server->tsk)) { - struct task_struct *tsk; - send_sig(SIGKILL,pSesInfo->server->tsk,1); - tsk = pSesInfo->server->tsk; - if (tsk) - kthread_stop(tsk); - } + (pSesInfo->server) && (pSesInfo->server->tsk)) + stop_cifsd(pSesInfo->server); } else cFYI(1, ("No session or bad tcon")); sesInfoFree(pSesInfo); @@ -3321,7 +3338,7 @@ cifs_umount(struct super_block *sb, struct cifs_sb_info *cifs_sb) int rc = 0; int xid; struct cifsSesInfo *ses = NULL; - struct task_struct *cifsd_task; + struct TCP_Server_Info *server; char * tmp; xid = GetXid(); @@ -3335,19 +3352,15 @@ cifs_umount(struct super_block *sb, struct cifs_sb_info *cifs_sb) } tconInfoFree(cifs_sb->tcon); if ((ses) && (ses->server)) { - /* save off task so we do not refer to ses later */ - cifsd_task = ses->server->tsk; + /* save off server so we do not refer to ses later */ + server = ses->server; cFYI(1, ("About to do SMBLogoff ")); rc = CIFSSMBLogoff(xid, ses); if (rc == -EBUSY) { FreeXid(xid); return 0; } else if (rc == -ESHUTDOWN) { - cFYI(1,("Waking up socket by sending it signal")); - if (cifsd_task) { - send_sig(SIGKILL,cifsd_task,1); - kthread_stop(cifsd_task); - } + stop_cifsd(server); rc = 0; } /* else - we have an smb session left on this socket do not kill cifsd */ - To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html