The Samba-Bugzilla – Attachment 3676 Details for
Bug 5720
Concurrent mount/umount processes to same windows machine, different shares hangs umount processes or crashes kernel
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
patch -- eliminate races between simultaneous mount and unmount
cifs-mount-race.patch (text/plain), 30.74 KB, created by
Jeff Layton
on 2008-10-14 15:17:27 UTC
(
hide
)
Description:
patch -- eliminate races between simultaneous mount and unmount
Filename:
MIME Type:
Creator:
Jeff Layton
Created:
2008-10-14 15:17:27 UTC
Size:
30.74 KB
patch
obsolete
>------------------------------------------------------------------------------- >cifs-clean-up-server-protocol- >------------------------------------------------------------------------------- >cifs: clean up server protocol handling for TCP_Server_Info > >From: Jeff Layton <jlayton@redhat.com> > >We're currently declaring both a sockaddr_in and sockaddr6_in on the >stack, but we really only need storage for one of them. Declare a >sockaddr struct and cast it to the proper type. Also, eliminate the >protocolType field in the TCP_Server_Info struct. It's redundant since >we have a sin_family field in the sockaddr anyway. > >We may need to revisit this if SCTP is ever implemented, but for now >this will simplify the code. > >Signed-off-by: Jeff Layton <jlayton@redhat.com> >--- > > fs/cifs/cifsglob.h | 8 -------- > fs/cifs/connect.c | 31 +++++++++++++++---------------- > 2 files changed, 15 insertions(+), 24 deletions(-) > > >diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h >index 8161bdb..e50a03b 100644 >--- a/fs/cifs/cifsglob.h >+++ b/fs/cifs/cifsglob.h >@@ -84,13 +84,6 @@ enum securityEnum { > MSKerberos, /* MS Kerberos via SPNEGO */ > }; > >-enum protocolEnum { >- IPV4 = 0, >- IPV6, >- SCTP >- /* Netbios frames protocol not supported at this time */ >-}; >- > struct mac_key { > unsigned int len; > union { >@@ -137,7 +130,6 @@ struct TCP_Server_Info { > void *Server_NlsInfo; /* BB - placeholder for future NLS info */ > unsigned short server_codepage; /* codepage for the server */ > unsigned long ip_address; /* IP addr for the server if known */ >- enum protocolEnum protocolType; > char versionMajor; > char versionMinor; > bool svlocal:1; /* local server or remote */ >diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c >index 4c13bcd..2641549 100644 >--- a/fs/cifs/connect.c >+++ b/fs/cifs/connect.c >@@ -186,7 +186,7 @@ cifs_reconnect(struct TCP_Server_Info *server) > > while ((!kthread_should_stop()) && (server->tcpStatus != CifsGood)) { > try_to_freeze(); >- if (server->protocolType == IPV6) { >+ if (server->addr.sockAddr6.sin6_family == AF_INET6) { > rc = ipv6_connect(&server->addr.sockAddr6, > &server->ssocket); > } else { >@@ -1851,10 +1851,10 @@ cifs_mount(struct super_block *sb, struct cifs_sb_info *cifs_sb, > { > int rc = 0; > int xid; >- int address_type = AF_INET; > struct socket *csocket = NULL; >- struct sockaddr_in sin_server; >- struct sockaddr_in6 sin_server6; >+ struct sockaddr addr; >+ struct sockaddr_in *sin_server = (struct sockaddr_in *) &addr; >+ struct sockaddr_in6 *sin_server6 = (struct sockaddr_in6 *) &addr; > struct smb_vol volume_info; > struct cifsSesInfo *pSesInfo = NULL; > struct cifsSesInfo *existingCifsSes = NULL; >@@ -1887,16 +1887,16 @@ cifs_mount(struct super_block *sb, struct cifs_sb_info *cifs_sb, > > if (volume_info.UNCip && volume_info.UNC) { > rc = cifs_inet_pton(AF_INET, volume_info.UNCip, >- &sin_server.sin_addr.s_addr); >+ &sin_server->sin_addr.s_addr); > > if (rc <= 0) { > /* not ipv4 address, try ipv6 */ > rc = cifs_inet_pton(AF_INET6, volume_info.UNCip, >- &sin_server6.sin6_addr.in6_u); >+ &sin_server6->sin6_addr.in6_u); > if (rc > 0) >- address_type = AF_INET6; >+ addr.sa_family = AF_INET6; > } else { >- address_type = AF_INET; >+ addr.sa_family = AF_INET; > } > > if (rc <= 0) { >@@ -1954,16 +1954,16 @@ cifs_mount(struct super_block *sb, struct cifs_sb_info *cifs_sb, > cFYI(1, ("Existing tcp session with server found")); > } else { /* create socket */ > if (volume_info.port) >- sin_server.sin_port = htons(volume_info.port); >+ sin_server->sin_port = htons(volume_info.port); > else >- sin_server.sin_port = 0; >- if (address_type == AF_INET6) { >+ sin_server->sin_port = 0; >+ if (addr.sa_family == AF_INET6) { > cFYI(1, ("attempting ipv6 connect")); > /* BB should we allow ipv6 on port 139? */ > /* other OS never observed in Wild doing 139 with v6 */ >- rc = ipv6_connect(&sin_server6, &csocket); >+ rc = ipv6_connect(sin_server6, &csocket); > } else >- rc = ipv4_connect(&sin_server, &csocket, >+ rc = ipv4_connect(sin_server, &csocket, > volume_info.source_rfc1001_name, > volume_info.target_rfc1001_name); > if (rc < 0) { >@@ -1980,12 +1980,11 @@ cifs_mount(struct super_block *sb, struct cifs_sb_info *cifs_sb, > sock_release(csocket); > goto out; > } else { >- memcpy(&srvTcp->addr.sockAddr, &sin_server, >+ memcpy(&srvTcp->addr.sockAddr, sin_server, > sizeof(struct sockaddr_in)); > atomic_set(&srvTcp->inFlight, 0); > /* BB Add code for ipv6 case too */ > srvTcp->ssocket = csocket; >- srvTcp->protocolType = IPV4; > srvTcp->hostname = extract_hostname(volume_info.UNC); > if (IS_ERR(srvTcp->hostname)) { > rc = PTR_ERR(srvTcp->hostname); >@@ -2037,7 +2036,7 @@ cifs_mount(struct super_block *sb, struct cifs_sb_info *cifs_sb, > else { > pSesInfo->server = srvTcp; > sprintf(pSesInfo->serverName, "%u.%u.%u.%u", >- NIPQUAD(sin_server.sin_addr.s_addr)); >+ NIPQUAD(sin_server->sin_addr.s_addr)); > } > > if (!rc) { >------------------------------------------------------------------------------- >cifs-disable-sharing-of-server >------------------------------------------------------------------------------- >cifs: disable sharing of server, session and tcon > >From: Jeff Layton <jlayton@redhat.com> > >The code that allows these structs to be shared is extremely racy. >Disable them for now until we can come up with a way to do this that's >race free. > >Note that this leaves a substantial amount of dead code in cifs_mount. >I'm leery of removing it at this point until we know for sure that we >aren't going to reuse it later. > >Signed-off-by: Jeff Layton <jlayton@redhat.com> >--- > > fs/cifs/connect.c | 105 ----------------------------------------------------- > 1 files changed, 0 insertions(+), 105 deletions(-) > > >diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c >index 2641549..63e9200 100644 >--- a/fs/cifs/connect.c >+++ b/fs/cifs/connect.c >@@ -1331,94 +1331,6 @@ cifs_parse_mount_options(char *options, const char *devname, > return 0; > } > >-static struct cifsSesInfo * >-cifs_find_tcp_session(struct in_addr *target_ip_addr, >- struct in6_addr *target_ip6_addr, >- char *userName, struct TCP_Server_Info **psrvTcp) >-{ >- struct list_head *tmp; >- struct cifsSesInfo *ses; >- >- *psrvTcp = NULL; >- >- read_lock(&GlobalSMBSeslock); >- list_for_each(tmp, &GlobalSMBSessionList) { >- ses = list_entry(tmp, struct cifsSesInfo, cifsSessionList); >- if (!ses->server) >- continue; >- >- if (target_ip_addr && >- ses->server->addr.sockAddr.sin_addr.s_addr != target_ip_addr->s_addr) >- continue; >- else if (target_ip6_addr && >- memcmp(&ses->server->addr.sockAddr6.sin6_addr, >- target_ip6_addr, sizeof(*target_ip6_addr))) >- continue; >- /* BB lock server and tcp session; increment use count here?? */ >- >- /* found a match on the TCP session */ >- *psrvTcp = ses->server; >- >- /* BB check if reconnection needed */ >- if (strncmp(ses->userName, userName, MAX_USERNAME_SIZE) == 0) { >- read_unlock(&GlobalSMBSeslock); >- /* Found exact match on both TCP and >- SMB sessions */ >- return ses; >- } >- /* else tcp and smb sessions need reconnection */ >- } >- read_unlock(&GlobalSMBSeslock); >- >- return NULL; >-} >- >-static struct cifsTconInfo * >-find_unc(__be32 new_target_ip_addr, char *uncName, char *userName) >-{ >- struct list_head *tmp; >- struct cifsTconInfo *tcon; >- __be32 old_ip; >- >- read_lock(&GlobalSMBSeslock); >- >- list_for_each(tmp, &GlobalTreeConnectionList) { >- cFYI(1, ("Next tcon")); >- tcon = list_entry(tmp, struct cifsTconInfo, cifsConnectionList); >- if (!tcon->ses || !tcon->ses->server) >- continue; >- >- old_ip = tcon->ses->server->addr.sockAddr.sin_addr.s_addr; >- cFYI(1, ("old ip addr: %x == new ip %x ?", >- old_ip, new_target_ip_addr)); >- >- if (old_ip != new_target_ip_addr) >- continue; >- >- /* BB lock tcon, server, tcp session and increment use count? */ >- /* found a match on the TCP session */ >- /* BB check if reconnection needed */ >- cFYI(1, ("IP match, old UNC: %s new: %s", >- tcon->treeName, uncName)); >- >- if (strncmp(tcon->treeName, uncName, MAX_TREE_SIZE)) >- continue; >- >- cFYI(1, ("and old usr: %s new: %s", >- tcon->treeName, uncName)); >- >- if (strncmp(tcon->ses->userName, userName, MAX_USERNAME_SIZE)) >- continue; >- >- /* matched smb session (user name) */ >- read_unlock(&GlobalSMBSeslock); >- return tcon; >- } >- >- read_unlock(&GlobalSMBSeslock); >- return NULL; >-} >- > int > get_dfs_path(int xid, struct cifsSesInfo *pSesInfo, const char *old_path, > const struct nls_table *nls_codepage, unsigned int *pnum_referrals, >@@ -1936,20 +1848,6 @@ cifs_mount(struct super_block *sb, struct cifs_sb_info *cifs_sb, > } > } > >- if (address_type == AF_INET) >- existingCifsSes = cifs_find_tcp_session(&sin_server.sin_addr, >- NULL /* no ipv6 addr */, >- volume_info.username, &srvTcp); >- else if (address_type == AF_INET6) { >- cFYI(1, ("looking for ipv6 address")); >- existingCifsSes = cifs_find_tcp_session(NULL /* no ipv4 addr */, >- &sin_server6.sin6_addr, >- volume_info.username, &srvTcp); >- } else { >- rc = -EINVAL; >- goto out; >- } >- > if (srvTcp) { > cFYI(1, ("Existing tcp session with server found")); > } else { /* create socket */ >@@ -2154,9 +2052,6 @@ cifs_mount(struct super_block *sb, struct cifs_sb_info *cifs_sb, > cERROR(1, ("mount option dynperm ignored if cifsacl " > "mount option supported")); > >- tcon = >- find_unc(sin_server.sin_addr.s_addr, volume_info.UNC, >- volume_info.username); > if (tcon) { > cFYI(1, ("Found match on UNC path")); > /* we can have only one retry value for a connection >------------------------------------------------------------------------------- >cifs-eliminate-usage-of-kthrea >------------------------------------------------------------------------------- >cifs: eliminate usage of kthread_stop for cifsd > >From: Jeff Layton <jlayton@redhat.com> > >When cifs_demultiplex_thread was converted to a kthread based kernel >thread, great pains were taken to make it so that kthread_stop would be >used to bring it down. This just added unnecessary complexity since we >needed to use a signal anyway to break out of kernel_recvmsg. > >Also, cifs_demultiplex_thread does a bit of cleanup as it's exiting, and >we need to be certain that this gets done. It's possible for a kthread >to exit before its main function is ever run if kthread_stop is called >soon after its creation. While I'm not sure that this is a real problem >with cifsd now, it could be at some point in the future if cifs_mount is >ever changed to bring down the thread quickly. > >The upshot here is that using kthread_stop to bring down the thread just >adds extra complexity with no real benefit. This patch changes the code >to use the original method to bring down the thread, but still leaves it >so that the thread is actually started with kthread_run. > >This seems to fix the deadlock caused by the reproducer in this bug >report: > >https://bugzilla.samba.org/show_bug.cgi?id=5720 > >Signed-off-by: Jeff Layton <jlayton@redhat.com> >--- > > fs/cifs/connect.c | 44 +++++++++++++++----------------------------- > 1 files changed, 15 insertions(+), 29 deletions(-) > > >diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c >index 63e9200..8fa8c5c 100644 >--- a/fs/cifs/connect.c >+++ b/fs/cifs/connect.c >@@ -124,7 +124,7 @@ cifs_reconnect(struct TCP_Server_Info *server) > struct mid_q_entry *mid_entry; > > spin_lock(&GlobalMid_Lock); >- if (kthread_should_stop()) { >+ if (server->tcpStatus == CifsExiting) { > /* the demux thread will exit normally > next time through the loop */ > spin_unlock(&GlobalMid_Lock); >@@ -184,7 +184,8 @@ cifs_reconnect(struct TCP_Server_Info *server) > spin_unlock(&GlobalMid_Lock); > up(&server->tcpSem); > >- while ((!kthread_should_stop()) && (server->tcpStatus != CifsGood)) { >+ while ((server->tcpStatus != CifsExiting) && >+ (server->tcpStatus != CifsGood)) { > try_to_freeze(); > if (server->addr.sockAddr6.sin6_family == AF_INET6) { > rc = ipv6_connect(&server->addr.sockAddr6, >@@ -201,7 +202,7 @@ cifs_reconnect(struct TCP_Server_Info *server) > } else { > atomic_inc(&tcpSesReconnectCount); > spin_lock(&GlobalMid_Lock); >- if (!kthread_should_stop()) >+ if (server->tcpStatus != CifsExiting) > server->tcpStatus = CifsGood; > server->sequence_number = 0; > spin_unlock(&GlobalMid_Lock); >@@ -356,7 +357,7 @@ cifs_demultiplex_thread(struct TCP_Server_Info *server) > GFP_KERNEL); > > set_freezable(); >- while (!kthread_should_stop()) { >+ while (server->tcpStatus != CifsExiting) { > if (try_to_freeze()) > continue; > if (bigbuf == NULL) { >@@ -397,7 +398,7 @@ incomplete_rcv: > kernel_recvmsg(csocket, &smb_msg, > &iov, 1, pdu_length, 0 /* BB other flags? */); > >- if (kthread_should_stop()) { >+ if (server->tcpStatus == CifsExiting) { > break; > } else if (server->tcpStatus == CifsNeedReconnect) { > cFYI(1, ("Reconnect after server stopped responding")); >@@ -522,7 +523,7 @@ incomplete_rcv: > total_read += length) { > length = kernel_recvmsg(csocket, &smb_msg, &iov, 1, > pdu_length - total_read, 0); >- if (kthread_should_stop() || >+ if ((server->tcpStatus == CifsExiting) || > (length == -EINTR)) { > /* then will exit */ > reconnect = 2; >@@ -651,14 +652,6 @@ multi_t2_fnd: > spin_unlock(&GlobalMid_Lock); > wake_up_all(&server->response_q); > >- /* don't exit until kthread_stop is called */ >- set_current_state(TASK_UNINTERRUPTIBLE); >- while (!kthread_should_stop()) { >- schedule(); >- set_current_state(TASK_UNINTERRUPTIBLE); >- } >- set_current_state(TASK_RUNNING); >- > /* check if we have blocked requests that need to free */ > /* Note that cifs_max_pending is normally 50, but > can be set at module install time to as little as two */ >@@ -2119,14 +2112,7 @@ cifs_mount(struct super_block *sb, struct cifs_sb_info *cifs_sb, > spin_lock(&GlobalMid_Lock); > srvTcp->tcpStatus = CifsExiting; > spin_unlock(&GlobalMid_Lock); >- if (srvTcp->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 */ >- force_sig(SIGKILL, srvTcp->tsk); >- kthread_stop(srvTcp->tsk); >- } >+ force_sig(SIGKILL, srvTcp->tsk); > } > /* If find_unc succeeded then rc == 0 so we can not end */ > if (tcon) /* up accidently freeing someone elses tcon struct */ >@@ -2140,18 +2126,18 @@ cifs_mount(struct super_block *sb, struct cifs_sb_info *cifs_sb, > /* if the socketUseCount is now zero */ > if ((temp_rc == -ESHUTDOWN) && > (pSesInfo->server) && >- (pSesInfo->server->tsk)) { >+ (pSesInfo->server->tsk)) > force_sig(SIGKILL, > pSesInfo->server->tsk); >- kthread_stop(pSesInfo->server->tsk); >- } > } else { > cFYI(1, ("No session or bad tcon")); > if ((pSesInfo->server) && > (pSesInfo->server->tsk)) { >+ spin_lock(&GlobalMid_Lock); >+ srvTcp->tcpStatus = CifsExiting; >+ spin_unlock(&GlobalMid_Lock); > force_sig(SIGKILL, > pSesInfo->server->tsk); >- kthread_stop(pSesInfo->server->tsk); > } > } > sesInfoFree(pSesInfo); >@@ -3462,10 +3448,8 @@ cifs_umount(struct super_block *sb, struct cifs_sb_info *cifs_sb) > return 0; > } else if (rc == -ESHUTDOWN) { > cFYI(1, ("Waking up socket by sending signal")); >- if (cifsd_task) { >+ if (cifsd_task) > force_sig(SIGKILL, cifsd_task); >- kthread_stop(cifsd_task); >- } > rc = 0; > } /* else - we have an smb session > left on this socket do not kill cifsd */ >@@ -3595,7 +3579,9 @@ int cifs_setup_session(unsigned int xid, struct cifsSesInfo *pSesInfo, > cERROR(1, ("Send error in SessSetup = %d", rc)); > } else { > cFYI(1, ("CIFS Session Established successfully")); >+ spin_lock(&GlobalMid_Lock); > pSesInfo->status = CifsGood; >+ spin_unlock(&GlobalMid_Lock); > } > > ss_err_exit: >------------------------------------------------------------------------------- >cifs-handle-the-tcp_server_inf >------------------------------------------------------------------------------- >cifs: handle the TCP_Server_Info->tsk field more carefully > >From: Jeff Layton <jlayton@redhat.com> > >We currently handle the TCP_Server_Info->tsk field without any locking, >but with some half-measures to try and prevent races. These aren't >really sufficient though. When taking down cifsd, use xchg() to swap >the contents of the tsk field with NULL so we don't end up trying >to send it more than one signal. Also, don't allow cifsd to exit until >the signal is received if we expect one. > >Signed-off-by: Jeff Layton <jlayton@redhat.com> >--- > > fs/cifs/connect.c | 41 ++++++++++++++++++++++++++++------------- > 1 files changed, 28 insertions(+), 13 deletions(-) > > >diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c >index 8fa8c5c..af0546f 100644 >--- a/fs/cifs/connect.c >+++ b/fs/cifs/connect.c >@@ -748,6 +748,7 @@ multi_t2_fnd: > write_unlock(&GlobalSMBSeslock); > > kfree(server->hostname); >+ task_to_wake = xchg(&server->tsk, NULL); > kfree(server); > > length = atomic_dec_return(&tcpSesAllocCount); >@@ -755,6 +756,16 @@ multi_t2_fnd: > mempool_resize(cifs_req_poolp, length + cifs_min_rcv, > GFP_KERNEL); > >+ /* if server->tsk was NULL then wait for a signal before exiting */ >+ if (!task_to_wake) { >+ set_current_state(TASK_INTERRUPTIBLE); >+ while(!signal_pending(current)) { >+ schedule(); >+ set_current_state(TASK_INTERRUPTIBLE); >+ } >+ set_current_state(TASK_RUNNING); >+ } >+ > return 0; > } > >@@ -1750,6 +1761,16 @@ convert_delimiter(char *path, char delim) > } > } > >+static void >+kill_cifsd(struct TCP_Server_Info *server) >+{ >+ struct task_struct *task; >+ >+ task = xchg(&server->tsk, NULL); >+ if (task) >+ force_sig(SIGKILL, task); >+} >+ > int > cifs_mount(struct super_block *sb, struct cifs_sb_info *cifs_sb, > char *mount_data, const char *devname) >@@ -2112,7 +2133,7 @@ cifs_mount(struct super_block *sb, struct cifs_sb_info *cifs_sb, > spin_lock(&GlobalMid_Lock); > srvTcp->tcpStatus = CifsExiting; > spin_unlock(&GlobalMid_Lock); >- force_sig(SIGKILL, srvTcp->tsk); >+ kill_cifsd(srvTcp); > } > /* If find_unc succeeded then rc == 0 so we can not end */ > if (tcon) /* up accidently freeing someone elses tcon struct */ >@@ -2125,19 +2146,15 @@ 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)) >- force_sig(SIGKILL, >- pSesInfo->server->tsk); >+ (pSesInfo->server)) >+ kill_cifsd(pSesInfo->server); > } else { > cFYI(1, ("No session or bad tcon")); >- if ((pSesInfo->server) && >- (pSesInfo->server->tsk)) { >+ if (pSesInfo->server) { > spin_lock(&GlobalMid_Lock); > srvTcp->tcpStatus = CifsExiting; > spin_unlock(&GlobalMid_Lock); >- force_sig(SIGKILL, >- pSesInfo->server->tsk); >+ kill_cifsd(pSesInfo->server); > } > } > sesInfoFree(pSesInfo); >@@ -3424,7 +3441,6 @@ 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; > char *tmp; > > xid = GetXid(); >@@ -3440,7 +3456,6 @@ 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; > cFYI(1, ("About to do SMBLogoff ")); > rc = CIFSSMBLogoff(xid, ses); > if (rc == -EBUSY) { >@@ -3448,8 +3463,8 @@ cifs_umount(struct super_block *sb, struct cifs_sb_info *cifs_sb) > return 0; > } else if (rc == -ESHUTDOWN) { > cFYI(1, ("Waking up socket by sending signal")); >- if (cifsd_task) >- force_sig(SIGKILL, cifsd_task); >+ if (ses->server) >+ kill_cifsd(ses->server); > rc = 0; > } /* else - we have an smb session > left on this socket do not kill cifsd */ >------------------------------------------------------------------------------- >cifs-add-global-tcp-session-li >------------------------------------------------------------------------------- >cifs: add global TCP session list and reenable TCP session sharing > >From: Jeff Layton <jlayton@redhat.com> > >Now that the socket/session/tcon sharing code is disabled, we can start >implementing a new scheme. The idea is that this should be a heirarchy: > >- A socket has a number of SMB sessions associated with it >- An SMB session has a number of tcon's associated with it > >In addition to making the sharing of these structures race-free, we want >to reduce our reliance on global lists as much as possible, and use >fine-grained locking with well-defined rules about what each lock >protects. > >Start by adding a new global list of TCP sessions. Add TCP sessions to >the list as they're created, and remove them as they're torn down. > >Another optimization we could consider is separate lists for IPv4 and >IPv6, but this should be a good start. > >Signed-off-by: Jeff Layton <jlayton@redhat.com> >--- > > fs/cifs/cifs_debug.c | 2 - > fs/cifs/cifsfs.c | 8 +++ > fs/cifs/cifsglob.h | 5 ++ > fs/cifs/cifsproto.h | 1 > fs/cifs/cifssmb.c | 14 ++---- > fs/cifs/connect.c | 123 +++++++++++++++++++++++++++++++++----------------- > 6 files changed, 99 insertions(+), 54 deletions(-) > > >diff --git a/fs/cifs/cifs_debug.c b/fs/cifs/cifs_debug.c >index 69a12aa..c998751 100644 >--- a/fs/cifs/cifs_debug.c >+++ b/fs/cifs/cifs_debug.c >@@ -144,7 +144,7 @@ static int cifs_debug_data_proc_show(struct seq_file *m, void *v) > seq_printf(m, "TCP status: %d\n\tLocal Users To " > "Server: %d SecMode: 0x%x Req On Wire: %d", > ses->server->tcpStatus, >- atomic_read(&ses->server->socketUseCount), >+ ses->server->refcount, > ses->server->secMode, > atomic_read(&ses->server->inFlight)); > >diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c >index 25ecbd5..c27cee8 100644 >--- a/fs/cifs/cifsfs.c >+++ b/fs/cifs/cifsfs.c >@@ -91,6 +91,12 @@ extern mempool_t *cifs_mid_poolp; > > extern struct kmem_cache *cifs_oplock_cachep; > >+/* global list of TCP_Server_Info structs */ >+struct list_head cifs_tcp_session_list; >+ >+/* protects cifs_tcp_session_list and TCP_Server_Info refcount */ >+spinlock_t cifs_tcp_session_lock; >+ > static int > cifs_read_super(struct super_block *sb, void *data, > const char *devname, int silent) >@@ -1017,6 +1023,7 @@ init_cifs(void) > INIT_LIST_HEAD(&GlobalSMBSessionList); > INIT_LIST_HEAD(&GlobalTreeConnectionList); > INIT_LIST_HEAD(&GlobalOplock_Q); >+ INIT_LIST_HEAD(&cifs_tcp_session_list); > #ifdef CONFIG_CIFS_EXPERIMENTAL > INIT_LIST_HEAD(&GlobalDnotifyReqList); > INIT_LIST_HEAD(&GlobalDnotifyRsp_Q); >@@ -1043,6 +1050,7 @@ init_cifs(void) > GlobalMaxActiveXid = 0; > memset(Local_System_Name, 0, 15); > rwlock_init(&GlobalSMBSeslock); >+ spin_lock_init(&cifs_tcp_session_lock); > spin_lock_init(&GlobalMid_Lock); > > if (cifs_max_pending < 2) { >diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h >index e50a03b..30eb5f5 100644 >--- a/fs/cifs/cifsglob.h >+++ b/fs/cifs/cifsglob.h >@@ -115,6 +115,8 @@ struct cifs_cred { > */ > > struct TCP_Server_Info { >+ struct list_head tcp_session_list; >+ int refcount; /* reference counter */ > /* 15 character server name + 0x20 16th byte indicating type = srv */ > char server_RFC1001_name[SERVER_NAME_LEN_WITH_NULL]; > char unicode_server_Name[SERVER_NAME_LEN_WITH_NULL * 2]; >@@ -133,7 +135,6 @@ struct TCP_Server_Info { > char versionMajor; > char versionMinor; > bool svlocal:1; /* local server or remote */ >- atomic_t socketUseCount; /* number of open cifs sessions on socket */ > atomic_t inFlight; /* number of requests on the wire to server */ > #ifdef CONFIG_CIFS_STATS2 > atomic_t inSend; /* requests trying to send */ >@@ -591,6 +592,8 @@ GLOBAL_EXTERN struct smbUidInfo *GlobalUidList[UID_HASH]; > GLOBAL_EXTERN struct list_head GlobalSMBSessionList; > GLOBAL_EXTERN struct list_head GlobalTreeConnectionList; > GLOBAL_EXTERN rwlock_t GlobalSMBSeslock; /* protects list inserts on 3 above */ >+extern struct list_head cifs_tcp_session_list; >+extern spinlock_t cifs_tcp_session_lock; > > GLOBAL_EXTERN struct list_head GlobalOplock_Q; > >diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h >index 0cff7fe..abd4580 100644 >--- a/fs/cifs/cifsproto.h >+++ b/fs/cifs/cifsproto.h >@@ -102,6 +102,7 @@ extern void acl_to_uid_mode(struct inode *inode, const char *path, > const __u16 *pfid); > extern int mode_to_acl(struct inode *inode, const char *path, __u64); > >+extern void cifs_put_tcp_session(struct TCP_Server_Info *server); > extern int cifs_mount(struct super_block *, struct cifs_sb_info *, char *, > const char *); > extern int cifs_umount(struct super_block *, struct cifs_sb_info *); >diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c >index 6f4ffe1..7b39c49 100644 >--- a/fs/cifs/cifssmb.c >+++ b/fs/cifs/cifssmb.c >@@ -664,8 +664,8 @@ CIFSSMBNegotiate(unsigned int xid, struct cifsSesInfo *ses) > rc = -EIO; > goto neg_err_exit; > } >- >- if (server->socketUseCount.counter > 1) { >+ spin_lock(&cifs_tcp_session_lock); >+ if (server->refcount > 1) { > if (memcmp(server->server_GUID, > pSMBr->u.extended_response. > GUID, 16) != 0) { >@@ -677,6 +677,7 @@ CIFSSMBNegotiate(unsigned int xid, struct cifsSesInfo *ses) > } else > memcpy(server->server_GUID, > pSMBr->u.extended_response.GUID, 16); >+ spin_unlock(&cifs_tcp_session_lock); > > if (count == 16) { > server->secType = RawNTLMSSP; >@@ -825,13 +826,8 @@ CIFSSMBLogoff(const int xid, struct cifsSesInfo *ses) > pSMB->AndXCommand = 0xFF; > rc = SendReceiveNoRsp(xid, ses, (struct smb_hdr *) pSMB, 0); > if (ses->server) { >- atomic_dec(&ses->server->socketUseCount); >- if (atomic_read(&ses->server->socketUseCount) == 0) { >- spin_lock(&GlobalMid_Lock); >- ses->server->tcpStatus = CifsExiting; >- spin_unlock(&GlobalMid_Lock); >- rc = -ESHUTDOWN; >- } >+ cifs_put_tcp_session(ses->server); >+ rc = 0; > } > up(&ses->sesSem); > >diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c >index af0546f..3d5e53d 100644 >--- a/fs/cifs/connect.c >+++ b/fs/cifs/connect.c >@@ -647,6 +647,11 @@ multi_t2_fnd: > } > } /* end while !EXITING */ > >+ /* take it off the list, if it's not already */ >+ spin_lock(&cifs_tcp_session_lock); >+ list_del_init(&server->tcp_session_list); >+ spin_unlock(&cifs_tcp_session_lock); >+ > spin_lock(&GlobalMid_Lock); > server->tcpStatus = CifsExiting; > spin_unlock(&GlobalMid_Lock); >@@ -1335,6 +1340,63 @@ cifs_parse_mount_options(char *options, const char *devname, > return 0; > } > >+static struct TCP_Server_Info * >+cifs_find_tcp_session(struct sockaddr *addr) >+{ >+ struct list_head *tmp; >+ struct TCP_Server_Info *server; >+ struct sockaddr_in *addr4 = (struct sockaddr_in *) addr; >+ struct sockaddr_in6 *addr6 = (struct sockaddr_in6 *) addr; >+ >+ spin_lock(&cifs_tcp_session_lock); >+ list_for_each(tmp, &cifs_tcp_session_list) { >+ server = list_entry(tmp, struct TCP_Server_Info, >+ tcp_session_list); >+ >+ /* Only accept sessions that have done successful negotiate */ >+ if (server->tcpStatus == CifsNew) >+ continue; >+ >+ if (addr->sa_family == AF_INET && >+ (addr4->sin_addr.s_addr != >+ server->addr.sockAddr.sin_addr.s_addr)) >+ continue; >+ else if (addr->sa_family == AF_INET6 && >+ memcmp(&server->addr.sockAddr6.sin6_addr, >+ &addr6->sin6_addr, sizeof(addr6->sin6_addr))) >+ continue; >+ >+ ++server->refcount; >+ spin_unlock(&cifs_tcp_session_lock); >+ return server; >+ } >+ spin_unlock(&cifs_tcp_session_lock); >+ return NULL; >+} >+ >+void >+cifs_put_tcp_session(struct TCP_Server_Info *server) >+{ >+ struct task_struct *task; >+ >+ spin_lock(&cifs_tcp_session_lock); >+ if (--server->refcount > 0) { >+ spin_unlock(&cifs_tcp_session_lock); >+ return; >+ } >+ >+ list_del_init(&server->tcp_session_list); >+ spin_unlock(&cifs_tcp_session_lock); >+ >+ spin_lock(&GlobalMid_Lock); >+ server->tcpStatus = CifsExiting; >+ spin_unlock(&GlobalMid_Lock); >+ >+ task = xchg(&server->tsk, NULL); >+ if (task) >+ force_sig(SIGKILL, task); >+} >+ > int > get_dfs_path(int xid, struct cifsSesInfo *pSesInfo, const char *old_path, > const struct nls_table *nls_codepage, unsigned int *pnum_referrals, >@@ -1761,16 +1823,6 @@ convert_delimiter(char *path, char delim) > } > } > >-static void >-kill_cifsd(struct TCP_Server_Info *server) >-{ >- struct task_struct *task; >- >- task = xchg(&server->tsk, NULL); >- if (task) >- force_sig(SIGKILL, task); >-} >- > int > cifs_mount(struct super_block *sb, struct cifs_sb_info *cifs_sb, > char *mount_data, const char *devname) >@@ -1862,6 +1914,8 @@ cifs_mount(struct super_block *sb, struct cifs_sb_info *cifs_sb, > } > } > >+ srvTcp = cifs_find_tcp_session(&addr); >+ > if (srvTcp) { > cFYI(1, ("Existing tcp session with server found")); > } else { /* create socket */ >@@ -1926,6 +1980,12 @@ cifs_mount(struct super_block *sb, struct cifs_sb_info *cifs_sb, > memcpy(srvTcp->server_RFC1001_name, > volume_info.target_rfc1001_name, 16); > srvTcp->sequence_number = 0; >+ INIT_LIST_HEAD(&srvTcp->tcp_session_list); >+ ++srvTcp->refcount; >+ spin_lock(&cifs_tcp_session_lock); >+ list_add(&cifs_tcp_session_list, >+ &srvTcp->tcp_session_list); >+ spin_unlock(&cifs_tcp_session_lock); > } > } > >@@ -1977,8 +2037,6 @@ cifs_mount(struct super_block *sb, struct cifs_sb_info *cifs_sb, > rc = cifs_setup_session(xid, pSesInfo, > cifs_sb->local_nls); > up(&pSesInfo->sesSem); >- if (!rc) >- atomic_inc(&srvTcp->socketUseCount); > } > } > >@@ -2127,35 +2185,20 @@ cifs_mount(struct super_block *sb, struct cifs_sb_info *cifs_sb, > > /* on error free sesinfo and tcon struct if needed */ > 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); >- kill_cifsd(srvTcp); >- } >- /* If find_unc succeeded then rc == 0 so we can not end */ >- if (tcon) /* up accidently freeing someone elses tcon struct */ >+ /* If find_unc succeeded then rc == 0 so we can not end */ >+ /* up accidently freeing someone elses tcon struct */ >+ if (tcon) > tconInfoFree(tcon); >+ > if (existingCifsSes == NULL) { > if (pSesInfo) { > if ((pSesInfo->server) && >- (pSesInfo->status == CifsGood)) { >- int temp_rc; >- temp_rc = CIFSSMBLogoff(xid, pSesInfo); >- /* if the socketUseCount is now zero */ >- if ((temp_rc == -ESHUTDOWN) && >- (pSesInfo->server)) >- kill_cifsd(pSesInfo->server); >- } else { >+ (pSesInfo->status == CifsGood)) >+ CIFSSMBLogoff(xid, pSesInfo); >+ else { > cFYI(1, ("No session or bad tcon")); >- if (pSesInfo->server) { >- spin_lock(&GlobalMid_Lock); >- srvTcp->tcpStatus = CifsExiting; >- spin_unlock(&GlobalMid_Lock); >- kill_cifsd(pSesInfo->server); >- } >+ if (pSesInfo->server) >+ cifs_put_tcp_session(pSesInfo->server); > } > sesInfoFree(pSesInfo); > /* pSesInfo = NULL; */ >@@ -3461,13 +3504,7 @@ cifs_umount(struct super_block *sb, struct cifs_sb_info *cifs_sb) > if (rc == -EBUSY) { > FreeXid(xid); > return 0; >- } else if (rc == -ESHUTDOWN) { >- cFYI(1, ("Waking up socket by sending signal")); >- if (ses->server) >- kill_cifsd(ses->server); >- rc = 0; >- } /* else - we have an smb session >- left on this socket do not kill cifsd */ >+ } > } else > cFYI(1, ("No session or bad tcon")); > }
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=3676">View the attachment on a separate page</a>.</b>
View Attachment As Raw
Actions:
View
Attachments on
bug 5720
:
3508
|
3510
|
3531
| 3676