From: Jeff Layton <jlayton@redhat.com>
To: linux-cifs-client@lists.samba.org
Cc: linux-fsdevel@vger.kernel.org
Subject: [PATCH] CIFS: make cifsd (more) signal-safe
Date: Tue, 5 Jun 2007 15:23:40 -0400 [thread overview]
Message-ID: <20070605152340.f09fa6f2.jlayton@redhat.com> (raw)
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@redhat.com>
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 */
next reply other threads:[~2007-06-05 19:23 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2007-06-05 19:23 Jeff Layton [this message]
2007-06-06 8:55 ` [PATCH] CIFS: make cifsd (more) signal-safe Christoph Hellwig
2007-06-08 16:35 ` [PATCH] RFC: have tcp_recvmsg() check kthread_should_stop() and treat it as if it were signalled Jeff Layton
2007-06-09 1:30 ` Herbert Xu
2007-06-09 11:08 ` Jeff Layton
2007-06-25 19:41 ` Satyam Sharma
2007-06-25 19:52 ` Jeff Layton
2007-06-25 22:09 ` Satyam Sharma
2007-06-26 0:46 ` Jeff Layton
2007-06-26 11:54 ` Oleg Nesterov
2007-06-26 22:53 ` Satyam Sharma
2007-06-27 1:29 ` Satyam Sharma
2007-06-27 12:24 ` Oleg Nesterov
2007-06-28 0:44 ` Satyam Sharma
2007-06-28 14:12 ` Oleg Nesterov
2007-06-28 15:44 ` Satyam Sharma
2007-06-28 16:19 ` Satyam Sharma
2007-06-28 16:24 ` Satyam Sharma
2007-06-28 17:08 ` Oleg Nesterov
2007-06-28 18:41 ` Jeff Layton
2007-06-28 19:22 ` Satyam Sharma
2007-06-08 17:00 ` Jeff Layton
2007-06-21 14:35 ` [linux-cifs-client] Re: [PATCH] CIFS: make cifsd (more) signal-safe Jeff Layton
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20070605152340.f09fa6f2.jlayton@redhat.com \
--to=jlayton@redhat.com \
--cc=linux-cifs-client@lists.samba.org \
--cc=linux-fsdevel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.