cifs: disable sharing session and tcon and add new TCP sharing code
authorJeff Layton <jlayton@redhat.com>
Fri, 14 Nov 2008 18:44:38 +0000 (13:44 -0500)
committerSteve French <sfrench@us.ibm.com>
Fri, 14 Nov 2008 23:42:32 +0000 (23:42 +0000)
The code that allows these structs to be shared is extremely racy.
Disable the sharing of SMB and tcon structs for now until we can
come up with a way to do this that's race free.

We want to continue to share TCP sessions, however since they are
required for multiuser mounts. For that, implement a new (hopefully
race-free) scheme. Add a new global list of TCP sessions, and take
care to get a reference to it whenever we're dealing with one.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
Signed-off-by: Steve French <sfrench@us.ibm.com>
fs/cifs/cifs_debug.c
fs/cifs/cifsfs.c
fs/cifs/cifsglob.h
fs/cifs/cifsproto.h
fs/cifs/cifssmb.c
fs/cifs/connect.c

index ba8723d..40b5108 100644 (file)
@@ -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->srv_count,
                                ses->server->secMode,
                                atomic_read(&ses->server->inFlight));
 
index af16a24..2946dab 100644 (file)
@@ -1059,7 +1059,7 @@ init_cifs(void)
 {
        int rc = 0;
        cifs_proc_init();
-       INIT_LIST_HEAD(&global_cifs_sock_list);
+       INIT_LIST_HEAD(&cifs_tcp_ses_list);
        INIT_LIST_HEAD(&GlobalSMBSessionList); /* BB to be removed by jl */
        INIT_LIST_HEAD(&GlobalTreeConnectionList); /* BB to be removed by jl */
        INIT_LIST_HEAD(&GlobalOplock_Q);
@@ -1089,6 +1089,7 @@ init_cifs(void)
        GlobalMaxActiveXid = 0;
        memset(Local_System_Name, 0, 15);
        rwlock_init(&GlobalSMBSeslock);
+       rwlock_init(&cifs_tcp_ses_lock);
        spin_lock_init(&GlobalMid_Lock);
 
        if (cifs_max_pending < 2) {
index 13dc484..313f7bf 100644 (file)
@@ -123,6 +123,7 @@ struct cifs_cred {
 struct TCP_Server_Info {
        struct list_head tcp_ses_list;
        struct list_head smb_ses_list;
+       int srv_count; /* 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];
@@ -144,7 +145,6 @@ struct TCP_Server_Info {
        bool svlocal:1;                 /* local server or remote */
        bool noblocksnd;                /* use blocking sendmsg */
        bool noautotune;                /* do not autotune send buf sizes */
-       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,13 +591,18 @@ require use of the stronger protocol */
 #define GLOBAL_EXTERN extern
 #endif
 
-
-/* the list of TCP_Server_Info structures, ie each of the sockets
+/*
+ * the list of TCP_Server_Info structures, ie each of the sockets
  * connecting our client to a distinct server (ip address), is
- * chained together by global_cifs_sock_list. The list of all our SMB
+ * chained together by cifs_tcp_ses_list. The list of all our SMB
  * sessions (and from that the tree connections) can be found
- * by iterating over global_cifs_sock_list */
-GLOBAL_EXTERN struct list_head global_cifs_sock_list;
+ * by iterating over cifs_tcp_ses_list
+ */
+GLOBAL_EXTERN struct list_head         cifs_tcp_ses_list;
+
+/* protects cifs_tcp_ses_list and srv_count for each tcp session */
+GLOBAL_EXTERN rwlock_t         cifs_tcp_ses_lock;
+
 GLOBAL_EXTERN struct list_head GlobalSMBSessionList; /* BB to be removed by jl*/
 GLOBAL_EXTERN struct list_head GlobalTreeConnectionList; /* BB to be removed */
 GLOBAL_EXTERN rwlock_t GlobalSMBSeslock;  /* protects list inserts on 3 above */
index 6f21ecb..0250a99 100644 (file)
@@ -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 *);
index 7f0651b..cd9e9a1 100644 (file)
@@ -664,8 +664,9 @@ CIFSSMBNegotiate(unsigned int xid, struct cifsSesInfo *ses)
                        rc = -EIO;
                        goto neg_err_exit;
                }
-
-               if (server->socketUseCount.counter > 1) {
+               read_lock(&cifs_tcp_ses_lock);
+               if (server->srv_count > 1) {
+                       read_unlock(&cifs_tcp_ses_lock);
                        if (memcmp(server->server_GUID,
                                   pSMBr->u.extended_response.
                                   GUID, 16) != 0) {
@@ -674,9 +675,11 @@ CIFSSMBNegotiate(unsigned int xid, struct cifsSesInfo *ses)
                                        pSMBr->u.extended_response.GUID,
                                        16);
                        }
-               } else
+               } else {
+                       read_unlock(&cifs_tcp_ses_lock);
                        memcpy(server->server_GUID,
                               pSMBr->u.extended_response.GUID, 16);
+               }
 
                if (count == 16) {
                        server->secType = RawNTLMSSP;
@@ -830,12 +833,9 @@ CIFSSMBLogoff(const int xid, struct cifsSesInfo *ses)
        pSMB->AndXCommand = 0xFF;
        rc = SendReceiveNoRsp(xid, ses, (struct smb_hdr *) pSMB, 0);
 session_already_dead:
-       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;
+       if (ses->server) {
+               cifs_put_tcp_session(ses->server);
+               rc = 0;
        }
        up(&ses->sesSem);
 
index 30ab8dc..a031425 100644 (file)
@@ -659,6 +659,11 @@ multi_t2_fnd:
                }
        } /* end while !EXITING */
 
+       /* take it off the list, if it's not already */
+       write_lock(&cifs_tcp_ses_lock);
+       list_del_init(&server->tcp_ses_list);
+       write_unlock(&cifs_tcp_ses_lock);
+
        spin_lock(&GlobalMid_Lock);
        server->tcpStatus = CifsExiting;
        spin_unlock(&GlobalMid_Lock);
@@ -1357,92 +1362,66 @@ 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)
+static struct TCP_Server_Info *
+cifs_find_tcp_session(struct sockaddr *addr)
 {
        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)
+       struct TCP_Server_Info *server;
+       struct sockaddr_in *addr4 = (struct sockaddr_in *) addr;
+       struct sockaddr_in6 *addr6 = (struct sockaddr_in6 *) addr;
+
+       write_lock(&cifs_tcp_ses_lock);
+       list_for_each(tmp, &cifs_tcp_ses_list) {
+               server = list_entry(tmp, struct TCP_Server_Info,
+                                   tcp_ses_list);
+
+               /*
+                * the demux thread can exit on its own while still in CifsNew
+                * so don't accept any sockets in that state. Since the
+                * tcpStatus never changes back to CifsNew it's safe to check
+                * for this without a lock.
+                */
+               if (server->tcpStatus == CifsNew)
                        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;
+               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;
 
-               /* 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 */
+               ++server->srv_count;
+               write_unlock(&cifs_tcp_ses_lock);
+               return server;
        }
-       read_unlock(&GlobalSMBSeslock);
-
+       write_unlock(&cifs_tcp_ses_lock);
        return NULL;
 }
 
-static struct cifsTconInfo *
-find_unc(__be32 new_target_ip_addr, char *uncName, char *userName)
+void
+cifs_put_tcp_session(struct TCP_Server_Info *server)
 {
-       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));
+       struct task_struct *task;
 
-               if (strncmp(tcon->treeName, uncName, MAX_TREE_SIZE))
-                       continue;
+       write_lock(&cifs_tcp_ses_lock);
+       if (--server->srv_count > 0) {
+               write_unlock(&cifs_tcp_ses_lock);
+               return;
+       }
 
-               cFYI(1, ("and old usr: %s new: %s",
-                       tcon->treeName, uncName));
+       list_del_init(&server->tcp_ses_list);
+       write_unlock(&cifs_tcp_ses_lock);
 
-               if (strncmp(tcon->ses->userName, userName, MAX_USERNAME_SIZE))
-                       continue;
-
-               /* matched smb session (user name) */
-               read_unlock(&GlobalSMBSeslock);
-               return tcon;
-       }
+       spin_lock(&GlobalMid_Lock);
+       server->tcpStatus = CifsExiting;
+       spin_unlock(&GlobalMid_Lock);
 
-       read_unlock(&GlobalSMBSeslock);
-       return NULL;
+       task = xchg(&server->tsk, NULL);
+       if (task)
+               force_sig(SIGKILL, task);
 }
 
 int
@@ -1881,16 +1860,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);
-}
-
 static void setup_cifs_sb(struct smb_vol *pvolume_info,
                          struct cifs_sb_info *cifs_sb)
 {
@@ -2069,21 +2038,10 @@ cifs_mount(struct super_block *sb, struct cifs_sb_info *cifs_sb,
                }
        }
 
-       if (addr.sa_family == AF_INET)
-               existingCifsSes = cifs_find_tcp_session(&sin_server->sin_addr,
-                       NULL /* no ipv6 addr */,
-                       volume_info.username, &srvTcp);
-       else if (addr.sa_family == 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) {
+       srvTcp = cifs_find_tcp_session(&addr);
+       if (srvTcp) {
+               cFYI(1, ("Existing tcp session with server found"));
+       } else {        /* create socket */
                if (addr.sa_family == AF_INET6) {
                        cFYI(1, ("attempting ipv6 connect"));
                        /* BB should we allow ipv6 on port 139? */
@@ -2153,6 +2111,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_ses_list);
+                       ++srvTcp->srv_count;
+                       write_lock(&cifs_tcp_ses_lock);
+                       list_add(&srvTcp->tcp_ses_list,
+                                &cifs_tcp_ses_list);
+                       write_unlock(&cifs_tcp_ses_lock);
                }
        }
 
@@ -2204,8 +2168,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);
                }
        }
 
@@ -2213,9 +2175,6 @@ cifs_mount(struct super_block *sb, struct cifs_sb_info *cifs_sb,
        if (!rc) {
                setup_cifs_sb(&volume_info, cifs_sb);
 
-               tcon =
-                   find_unc(sin_server->sin_addr.s_addr, volume_info.UNC,
-                            volume_info.username);
                if (tcon) {
                        cFYI(1, ("Found match on UNC path"));
                        if (tcon->seal != volume_info.seal)
@@ -2278,35 +2237,21 @@ cifs_mount(struct super_block *sb, struct cifs_sb_info *cifs_sb,
 /* on error free sesinfo and tcon struct if needed */
 mount_fail_check:
        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; */
@@ -3613,13 +3558,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"));
        }