* [PATCH 0/4] cifs: fix oopses and mem corruption with concurrent mount/umount (try #3)
@ 2008-10-22 14:13 Jeff Layton
2008-10-22 14:13 ` [PATCH 1/4] cifs: clean up server protocol handling for TCP_Server_Info Jeff Layton
` (3 more replies)
0 siblings, 4 replies; 6+ messages in thread
From: Jeff Layton @ 2008-10-22 14:13 UTC (permalink / raw)
To: smfrench; +Cc: linux-cifs-client, linux-kernel, smfrench
This patchset is intended to fix the oopses, memory corruption and mount
failures when using the reproducer detailed here:
https://bugzilla.samba.org/show_bug.cgi?id=5720
This is the third attempt at this. Since the second attempt, Steve has
committed the patch to remove kthread_stop from the cifsd shutdown
codepath. That will fix the deadlocks, but opens the window wider for
the oopses and memory corruption in this code.
The main difference in the remaining patches is that this set combines
the patches that disable the structure sharing with the one that
re-enables the TCP session sharing. It also adds a new patch that
re-enables the sharing of SMB sessions. With this set, only tree
connections are no longer shared.
With this, I've been able to run the reproducer in the above BZ for
several hours, whereas before it would regularly crash after just a few
minutes.
Jeff Layton (4):
cifs: clean up server protocol handling for TCP_Server_Info
cifs: handle the TCP_Server_Info->tsk field more carefully
cifs: disable sharing session and tcon and add new TCP sharing code
cifs: reinstate sharing of SMB sessions
fs/cifs/cifs_debug.c | 54 ++++----
fs/cifs/cifsfs.c | 25 +++--
fs/cifs/cifsglob.h | 20 +--
fs/cifs/cifssmb.c | 20 +---
fs/cifs/connect.c | 354 +++++++++++++++++++++++---------------------------
fs/cifs/misc.c | 15 +--
6 files changed, 223 insertions(+), 265 deletions(-)
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/4] cifs: clean up server protocol handling for TCP_Server_Info
2008-10-22 14:13 [PATCH 0/4] cifs: fix oopses and mem corruption with concurrent mount/umount (try #3) Jeff Layton
@ 2008-10-22 14:13 ` Jeff Layton
2008-10-22 14:13 ` [PATCH 2/4] cifs: handle the TCP_Server_Info->tsk field more carefully Jeff Layton
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Jeff Layton @ 2008-10-22 14:13 UTC (permalink / raw)
To: smfrench; +Cc: linux-cifs-client, linux-kernel, smfrench
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 | 39 +++++++++++++++++++--------------------
2 files changed, 19 insertions(+), 28 deletions(-)
diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index d25cc60..d45acb5 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 1126f7a..bf222e1 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -188,7 +188,7 @@ cifs_reconnect(struct TCP_Server_Info *server)
while ((server->tcpStatus != CifsExiting) &&
(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 {
@@ -1847,10 +1847,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;
@@ -1883,16 +1883,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) {
@@ -1932,14 +1932,14 @@ 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,
+ 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 (address_type == AF_INET6) {
+ 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,
+ &sin_server6->sin6_addr,
volume_info.username, &srvTcp);
} else {
rc = -EINVAL;
@@ -1950,16 +1950,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) {
@@ -1976,12 +1976,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);
@@ -2033,7 +2032,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) {
--
1.5.5.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/4] cifs: handle the TCP_Server_Info->tsk field more carefully
2008-10-22 14:13 [PATCH 0/4] cifs: fix oopses and mem corruption with concurrent mount/umount (try #3) Jeff Layton
2008-10-22 14:13 ` [PATCH 1/4] cifs: clean up server protocol handling for TCP_Server_Info Jeff Layton
@ 2008-10-22 14:13 ` Jeff Layton
2008-10-22 14:14 ` [PATCH 3/4] cifs: disable sharing session and tcon and add new TCP sharing code Jeff Layton
2008-10-22 14:14 ` [PATCH 4/4] cifs: reinstate sharing of SMB sessions Jeff Layton
3 siblings, 0 replies; 6+ messages in thread
From: Jeff Layton @ 2008-10-22 14:13 UTC (permalink / raw)
To: smfrench; +Cc: linux-cifs-client, linux-kernel, smfrench
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 bf222e1..0b2adef 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -749,6 +749,7 @@ multi_t2_fnd:
write_unlock(&GlobalSMBSeslock);
kfree(server->hostname);
+ task_to_wake = xchg(&server->tsk, NULL);
kfree(server);
length = atomic_dec_return(&tcpSesAllocCount);
@@ -756,6 +757,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;
}
@@ -1841,6 +1852,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)
@@ -2226,7 +2247,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 */
@@ -2239,19 +2260,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);
@@ -3538,7 +3555,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();
@@ -3554,7 +3570,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) {
@@ -3562,8 +3577,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 */
--
1.5.5.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 3/4] cifs: disable sharing session and tcon and add new TCP sharing code
2008-10-22 14:13 [PATCH 0/4] cifs: fix oopses and mem corruption with concurrent mount/umount (try #3) Jeff Layton
2008-10-22 14:13 ` [PATCH 1/4] cifs: clean up server protocol handling for TCP_Server_Info Jeff Layton
2008-10-22 14:13 ` [PATCH 2/4] cifs: handle the TCP_Server_Info->tsk field more carefully Jeff Layton
@ 2008-10-22 14:14 ` Jeff Layton
2008-10-22 14:14 ` [PATCH 4/4] cifs: reinstate sharing of SMB sessions Jeff Layton
3 siblings, 0 replies; 6+ messages in thread
From: Jeff Layton @ 2008-10-22 14:14 UTC (permalink / raw)
To: smfrench; +Cc: linux-cifs-client, linux-kernel, smfrench
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>
---
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 | 192 ++++++++++++++++----------------------------------
6 files changed, 81 insertions(+), 141 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 c6aad77..306c278 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)
@@ -1018,6 +1024,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);
@@ -1044,6 +1051,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 d45acb5..fdf9fc9 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 */
@@ -592,6 +593,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 843a85f..11d3ccc 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 0b2adef..a1aa90d 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -648,6 +648,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);
@@ -1338,92 +1343,61 @@ 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;
+ struct TCP_Server_Info *server;
+ struct sockaddr_in *addr4 = (struct sockaddr_in *) addr;
+ struct sockaddr_in6 *addr6 = (struct sockaddr_in6 *) addr;
- *psrvTcp = NULL;
+ 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);
- read_lock(&GlobalSMBSeslock);
- list_for_each(tmp, &GlobalSMBSessionList) {
- ses = list_entry(tmp, struct cifsSesInfo, cifsSessionList);
- if (!ses->server)
+ /* Only accept sessions that have done successful negotiate */
+ 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->refcount;
+ spin_unlock(&cifs_tcp_session_lock);
+ return server;
}
- read_unlock(&GlobalSMBSeslock);
-
+ spin_unlock(&cifs_tcp_session_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));
-
- if (strncmp(tcon->treeName, uncName, MAX_TREE_SIZE))
- continue;
+ struct task_struct *task;
- cFYI(1, ("and old usr: %s new: %s",
- tcon->treeName, uncName));
+ spin_lock(&cifs_tcp_session_lock);
+ if (--server->refcount > 0) {
+ spin_unlock(&cifs_tcp_session_lock);
+ return;
+ }
- if (strncmp(tcon->ses->userName, userName, MAX_USERNAME_SIZE))
- continue;
+ list_del_init(&server->tcp_session_list);
+ spin_unlock(&cifs_tcp_session_lock);
- /* 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
@@ -1852,16 +1826,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)
@@ -1953,20 +1917,7 @@ 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;
- }
-
+ srvTcp = cifs_find_tcp_session(&addr);
if (srvTcp) {
cFYI(1, ("Existing tcp session with server found"));
} else { /* create socket */
@@ -2031,6 +1982,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);
}
}
@@ -2082,8 +2039,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);
}
}
@@ -2171,9 +2126,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
@@ -2241,35 +2193,21 @@ 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; */
@@ -3575,13 +3513,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"));
}
--
1.5.5.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 4/4] cifs: reinstate sharing of SMB sessions
2008-10-22 14:13 [PATCH 0/4] cifs: fix oopses and mem corruption with concurrent mount/umount (try #3) Jeff Layton
` (2 preceding siblings ...)
2008-10-22 14:14 ` [PATCH 3/4] cifs: disable sharing session and tcon and add new TCP sharing code Jeff Layton
@ 2008-10-22 14:14 ` Jeff Layton
3 siblings, 0 replies; 6+ messages in thread
From: Jeff Layton @ 2008-10-22 14:14 UTC (permalink / raw)
To: smfrench; +Cc: linux-cifs-client, linux-kernel, smfrench
This patch reinstates the sharing of SMB sessions. Rather than trying to
keep them all on a global list, we instead keep a list of SMB sessions
for each TCP session. This makes some of the code more efficient in
places where we iterate over all SMB sessions.
This also clarifies the locking rules and simplifies the code.
Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
fs/cifs/cifs_debug.c | 54 +++++++++--------
fs/cifs/cifsfs.c | 17 +++---
fs/cifs/cifsglob.h | 9 ++-
fs/cifs/cifsproto.h | 1 -
fs/cifs/cifssmb.c | 10 +---
fs/cifs/connect.c | 156 +++++++++++++++++++++++++++++---------------------
fs/cifs/misc.c | 15 ++---
7 files changed, 137 insertions(+), 125 deletions(-)
diff --git a/fs/cifs/cifs_debug.c b/fs/cifs/cifs_debug.c
index c998751..81ccc60 100644
--- a/fs/cifs/cifs_debug.c
+++ b/fs/cifs/cifs_debug.c
@@ -107,9 +107,9 @@ void cifs_dump_mids(struct TCP_Server_Info *server)
#ifdef CONFIG_PROC_FS
static int cifs_debug_data_proc_show(struct seq_file *m, void *v)
{
- struct list_head *tmp;
- struct list_head *tmp1;
+ struct list_head *tmp, *tmp2, *tmp3;
struct mid_q_entry *mid_entry;
+ struct TCP_Server_Info *server;
struct cifsSesInfo *ses;
struct cifsTconInfo *tcon;
int i;
@@ -122,43 +122,45 @@ static int cifs_debug_data_proc_show(struct seq_file *m, void *v)
seq_printf(m, "Servers:");
i = 0;
- read_lock(&GlobalSMBSeslock);
- list_for_each(tmp, &GlobalSMBSessionList) {
+ 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);
i++;
- ses = list_entry(tmp, struct cifsSesInfo, cifsSessionList);
- if ((ses->serverDomain == NULL) || (ses->serverOS == NULL) ||
- (ses->serverNOS == NULL)) {
- seq_printf(m, "\nentry for %s not fully "
- "displayed\n\t", ses->serverName);
- } else {
- seq_printf(m,
+ spin_lock(&server->smb_session_lock);
+ list_for_each(tmp2, &server->smb_session_list) {
+ ses = list_entry(tmp2, struct cifsSesInfo,
+ smb_session_list);
+ if ((ses->serverDomain == NULL) ||
+ (ses->serverOS == NULL) ||
+ (ses->serverNOS == NULL)) {
+ seq_printf(m, "\nentry for %s not fully "
+ "displayed\n\t", ses->serverName);
+ } else {
+ seq_printf(m,
"\n%d) Name: %s Domain: %s Mounts: %d OS:"
" %s \n\tNOS: %s\tCapability: 0x%x\n\tSMB"
" session status: %d\t",
i, ses->serverName, ses->serverDomain,
- atomic_read(&ses->inUse),
- ses->serverOS, ses->serverNOS,
+ ses->refcount, ses->serverOS, ses->serverNOS,
ses->capabilities, ses->status);
- }
- if (ses->server) {
+ }
seq_printf(m, "TCP status: %d\n\tLocal Users To "
- "Server: %d SecMode: 0x%x Req On Wire: %d",
- ses->server->tcpStatus,
- ses->server->refcount,
- ses->server->secMode,
- atomic_read(&ses->server->inFlight));
+ "Server: %d SecMode: 0x%x Req On Wire: %d",
+ server->tcpStatus, server->refcount, server->secMode,
+ atomic_read(&server->inFlight));
#ifdef CONFIG_CIFS_STATS2
seq_printf(m, " In Send: %d In MaxReq Wait: %d",
- atomic_read(&ses->server->inSend),
- atomic_read(&ses->server->num_waiters));
+ atomic_read(&server->inSend),
+ atomic_read(&server->num_waiters));
#endif
seq_puts(m, "\nMIDs:\n");
spin_lock(&GlobalMid_Lock);
- list_for_each(tmp1, &ses->server->pending_mid_q) {
- mid_entry = list_entry(tmp1, struct
+ list_for_each(tmp3, &server->pending_mid_q) {
+ mid_entry = list_entry(tmp3, struct
mid_q_entry,
qhead);
seq_printf(m, "State: %d com: %d pid:"
@@ -171,9 +173,9 @@ static int cifs_debug_data_proc_show(struct seq_file *m, void *v)
}
spin_unlock(&GlobalMid_Lock);
}
-
+ spin_unlock(&server->smb_session_lock);
}
- read_unlock(&GlobalSMBSeslock);
+ spin_unlock(&cifs_tcp_session_lock);
seq_putc(m, '\n');
seq_puts(m, "Shares:");
diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index 306c278..d1ff2f9 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -992,24 +992,24 @@ static int cifs_oplock_thread(void *dummyarg)
static int cifs_dnotify_thread(void *dummyarg)
{
struct list_head *tmp;
- struct cifsSesInfo *ses;
+ struct TCP_Server_Info *server;
do {
if (try_to_freeze())
continue;
set_current_state(TASK_INTERRUPTIBLE);
schedule_timeout(15*HZ);
- read_lock(&GlobalSMBSeslock);
/* check if any stuck requests that need
to be woken up and wakeq so the
thread can wake up and error out */
- list_for_each(tmp, &GlobalSMBSessionList) {
- ses = list_entry(tmp, struct cifsSesInfo,
- cifsSessionList);
- if (ses->server && atomic_read(&ses->server->inFlight))
- wake_up_all(&ses->server->response_q);
+ 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);
+ if (atomic_read(&server->inFlight))
+ wake_up_all(&server->response_q);
}
- read_unlock(&GlobalSMBSeslock);
+ spin_unlock(&cifs_tcp_session_lock);
} while (!kthread_should_stop());
return 0;
@@ -1021,7 +1021,6 @@ init_cifs(void)
int rc = 0;
cifs_proc_init();
/* INIT_LIST_HEAD(&GlobalServerList);*/ /* BB not implemented yet */
- INIT_LIST_HEAD(&GlobalSMBSessionList);
INIT_LIST_HEAD(&GlobalTreeConnectionList);
INIT_LIST_HEAD(&GlobalOplock_Q);
INIT_LIST_HEAD(&cifs_tcp_session_list);
diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index fdf9fc9..8432300 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -115,7 +115,9 @@ struct cifs_cred {
*/
struct TCP_Server_Info {
- struct list_head tcp_session_list;
+ struct list_head tcp_session_list; /* link to global tcp sess list */
+ struct list_head smb_session_list; /* list of SMB sessions */
+ spinlock_t smb_session_lock; /* protects smb_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];
@@ -185,13 +187,13 @@ struct cifsUidInfo {
* Session structure. One of these for each uid session with a particular host
*/
struct cifsSesInfo {
- struct list_head cifsSessionList;
+ struct list_head smb_session_list;
struct semaphore sesSem;
#if 0
struct cifsUidInfo *uidInfo; /* pointer to user info */
#endif
struct TCP_Server_Info *server; /* pointer to server info */
- atomic_t inUse; /* # of mounts (tree connections) on this ses */
+ int refcount; /* reference counter */
enum statusEnum status;
unsigned overrideSecFlg; /* if non-zero override global sec flags */
__u16 ipc_tid; /* special tid for connection to IPC share */
@@ -590,7 +592,6 @@ require use of the stronger protocol */
GLOBAL_EXTERN struct smbUidInfo *GlobalUidList[UID_HASH];
/* GLOBAL_EXTERN struct list_head GlobalServerList; BB not implemented yet */
-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;
diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
index abd4580..0cff7fe 100644
--- a/fs/cifs/cifsproto.h
+++ b/fs/cifs/cifsproto.h
@@ -102,7 +102,6 @@ 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 11d3ccc..c9e9243 100644
--- a/fs/cifs/cifssmb.c
+++ b/fs/cifs/cifssmb.c
@@ -802,11 +802,6 @@ CIFSSMBLogoff(const int xid, struct cifsSesInfo *ses)
else
return -EIO;
- atomic_dec(&ses->inUse);
- if (atomic_read(&ses->inUse) > 0) {
- up(&ses->sesSem);
- return -EBUSY;
- }
rc = small_smb_init(SMB_COM_LOGOFF_ANDX, 2, NULL, (void **)&pSMB);
if (rc) {
up(&ses->sesSem);
@@ -825,10 +820,6 @@ CIFSSMBLogoff(const int xid, struct cifsSesInfo *ses)
pSMB->AndXCommand = 0xFF;
rc = SendReceiveNoRsp(xid, ses, (struct smb_hdr *) pSMB, 0);
- if (ses->server) {
- cifs_put_tcp_session(ses->server);
- rc = 0;
- }
up(&ses->sesSem);
/* if session dead then we do not need to do ulogoff,
@@ -836,6 +827,7 @@ CIFSSMBLogoff(const int xid, struct cifsSesInfo *ses)
error */
if (rc == -EAGAIN)
rc = 0;
+
return rc;
}
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index a1aa90d..5f446de 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -139,23 +139,18 @@ cifs_reconnect(struct TCP_Server_Info *server)
/* before reconnecting the tcp session, mark the smb session (uid)
and the tid bad so they are not used until reconnected */
- read_lock(&GlobalSMBSeslock);
- list_for_each(tmp, &GlobalSMBSessionList) {
- ses = list_entry(tmp, struct cifsSesInfo, cifsSessionList);
- if (ses->server) {
- if (ses->server == server) {
- ses->status = CifsNeedReconnect;
- ses->ipc_tid = 0;
- }
- }
- /* else tcp and smb sessions need reconnection */
+ spin_lock(&server->smb_session_lock);
+ list_for_each(tmp, &server->smb_session_list) {
+ ses = list_entry(tmp, struct cifsSesInfo, smb_session_list);
+ ses->status = CifsNeedReconnect;
+ ses->ipc_tid = 0;
}
+ spin_unlock(&server->smb_session_lock);
list_for_each(tmp, &GlobalTreeConnectionList) {
tcon = list_entry(tmp, struct cifsTconInfo, cifsConnectionList);
if ((tcon->ses) && (tcon->ses->server == server))
tcon->tidStatus = CifsNeedReconnect;
}
- read_unlock(&GlobalSMBSeslock);
/* do not want to be sending data on a socket we are freeing */
down(&server->tcpSem);
if (server->ssocket) {
@@ -685,29 +680,25 @@ multi_t2_fnd:
if (smallbuf) /* no sense logging a debug message if NULL */
cifs_small_buf_release(smallbuf);
- read_lock(&GlobalSMBSeslock);
+ spin_lock(&server->smb_session_lock);
if (list_empty(&server->pending_mid_q)) {
/* loop through server session structures attached to this and
mark them dead */
- list_for_each(tmp, &GlobalSMBSessionList) {
- ses =
- list_entry(tmp, struct cifsSesInfo,
- cifsSessionList);
- if (ses->server == server) {
- ses->status = CifsExiting;
- ses->server = NULL;
- }
+ list_for_each(tmp, &server->smb_session_list) {
+ ses = list_entry(tmp, struct cifsSesInfo,
+ smb_session_list);
+ ses->status = CifsExiting;
+ ses->server = NULL;
}
- read_unlock(&GlobalSMBSeslock);
+ spin_unlock(&server->smb_session_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, &GlobalSMBSessionList) {
+ list_for_each(tmp, &server->smb_session_list) {
ses = list_entry(tmp, struct cifsSesInfo,
- cifsSessionList);
- if (ses->server == server)
- ses->status = CifsExiting;
+ smb_session_list);
+ ses->status = CifsExiting;
}
spin_lock(&GlobalMid_Lock);
@@ -722,7 +713,7 @@ multi_t2_fnd:
}
}
spin_unlock(&GlobalMid_Lock);
- read_unlock(&GlobalSMBSeslock);
+ spin_unlock(&server->smb_session_lock);
/* 1/8th of sec is more than enough time for them to exit */
msleep(125);
}
@@ -744,14 +735,12 @@ multi_t2_fnd:
if there are any pointing to this (e.g
if a crazy root user tried to kill cifsd
kernel thread explicitly this might happen) */
- write_lock(&GlobalSMBSeslock);
- list_for_each(tmp, &GlobalSMBSessionList) {
- ses = list_entry(tmp, struct cifsSesInfo,
- cifsSessionList);
- if (ses->server == server)
- ses->server = NULL;
+ spin_lock(&server->smb_session_lock);
+ list_for_each(tmp, &server->smb_session_list) {
+ ses = list_entry(tmp, struct cifsSesInfo, smb_session_list);
+ ses->server = NULL;
}
- write_unlock(&GlobalSMBSeslock);
+ spin_unlock(&server->smb_session_lock);
kfree(server->hostname);
task_to_wake = xchg(&server->tsk, NULL);
@@ -1377,7 +1366,7 @@ cifs_find_tcp_session(struct sockaddr *addr)
return NULL;
}
-void
+static void
cifs_put_tcp_session(struct TCP_Server_Info *server)
{
struct task_struct *task;
@@ -1400,6 +1389,50 @@ cifs_put_tcp_session(struct TCP_Server_Info *server)
force_sig(SIGKILL, task);
}
+static struct cifsSesInfo *
+cifs_find_smb_session(struct TCP_Server_Info *server, char *username)
+{
+ struct list_head *tmp;
+ struct cifsSesInfo *ses;
+
+ spin_lock(&server->smb_session_lock);
+ list_for_each(tmp, &server->smb_session_list) {
+ ses = list_entry(tmp, struct cifsSesInfo, smb_session_list);
+ if (strncmp(ses->userName, username, MAX_USERNAME_SIZE) == 0)
+ continue;
+
+ ++ses->refcount;
+ spin_unlock(&server->smb_session_lock);
+ return ses;
+ }
+ spin_unlock(&server->smb_session_lock);
+ return NULL;
+}
+
+static void
+cifs_put_smb_session(struct cifsSesInfo *ses)
+{
+ int xid;
+ struct TCP_Server_Info *server = ses->server;
+
+ spin_lock(&server->smb_session_lock);
+ if (--ses->refcount > 0) {
+ spin_unlock(&server->smb_session_lock);
+ return;
+ }
+
+ list_del_init(&ses->smb_session_list);
+ spin_unlock(&server->smb_session_lock);
+
+ if (ses->status == CifsGood) {
+ xid = GetXid();
+ CIFSSMBLogoff(xid, ses);
+ _FreeXid(xid);
+ }
+ sesInfoFree(ses);
+ cifs_put_tcp_session(server);
+}
+
int
get_dfs_path(int xid, struct cifsSesInfo *pSesInfo, const char *old_path,
const struct nls_table *nls_codepage, unsigned int *pnum_referrals,
@@ -1838,7 +1871,6 @@ cifs_mount(struct super_block *sb, struct cifs_sb_info *cifs_sb,
struct sockaddr_in6 *sin_server6 = (struct sockaddr_in6 *) &addr;
struct smb_vol volume_info;
struct cifsSesInfo *pSesInfo = NULL;
- struct cifsSesInfo *existingCifsSes = NULL;
struct cifsTconInfo *tcon = NULL;
struct TCP_Server_Info *srvTcp = NULL;
@@ -1983,6 +2015,8 @@ cifs_mount(struct super_block *sb, struct cifs_sb_info *cifs_sb,
volume_info.target_rfc1001_name, 16);
srvTcp->sequence_number = 0;
INIT_LIST_HEAD(&srvTcp->tcp_session_list);
+ INIT_LIST_HEAD(&srvTcp->smb_session_list);
+ spin_lock_init(&srvTcp->smb_session_lock);
++srvTcp->refcount;
spin_lock(&cifs_tcp_session_lock);
list_add(&cifs_tcp_session_list,
@@ -1991,10 +2025,16 @@ cifs_mount(struct super_block *sb, struct cifs_sb_info *cifs_sb,
}
}
- if (existingCifsSes) {
- pSesInfo = existingCifsSes;
+ pSesInfo = cifs_find_smb_session(srvTcp, volume_info.username);
+ if (pSesInfo) {
cFYI(1, ("Existing smb sess found (status=%d)",
pSesInfo->status));
+ /*
+ * The existing SMB session already has a reference to srvTcp,
+ * so we can put back the extra one we got before
+ */
+ cifs_put_tcp_session(srvTcp);
+
down(&pSesInfo->sesSem);
if (pSesInfo->status == CifsNeedReconnect) {
cFYI(1, ("Session needs reconnect"));
@@ -2008,9 +2048,15 @@ cifs_mount(struct super_block *sb, struct cifs_sb_info *cifs_sb,
if (pSesInfo == NULL)
rc = -ENOMEM;
else {
+ /* new SMB session uses our srvTcp ref */
pSesInfo->server = srvTcp;
sprintf(pSesInfo->serverName, "%u.%u.%u.%u",
NIPQUAD(sin_server->sin_addr.s_addr));
+ ++pSesInfo->refcount;
+ spin_lock(&srvTcp->smb_session_lock);
+ list_add(&srvTcp->smb_session_list,
+ &pSesInfo->smb_session_list);
+ spin_unlock(&srvTcp->smb_session_lock);
}
if (!rc) {
@@ -2173,7 +2219,6 @@ cifs_mount(struct super_block *sb, struct cifs_sb_info *cifs_sb,
}
}
if (!rc) {
- atomic_inc(&pSesInfo->inUse);
tcon->retry = volume_info.retry;
tcon->nocase = volume_info.nocase;
tcon->seal = volume_info.seal;
@@ -2191,28 +2236,18 @@ cifs_mount(struct super_block *sb, struct cifs_sb_info *cifs_sb,
/* BB FIXME fix time_gran to be larger for LANMAN sessions */
sb->s_time_gran = 100;
-/* on error free sesinfo and tcon struct if needed */
+ /* on error free sesinfo and tcon struct if needed */
if (rc) {
/* 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))
- CIFSSMBLogoff(xid, pSesInfo);
- else {
- cFYI(1, ("No session or bad tcon"));
- if (pSesInfo->server)
- cifs_put_tcp_session(
- pSesInfo->server);
- }
- sesInfoFree(pSesInfo);
- /* pSesInfo = NULL; */
- }
- }
+ /* should also end up putting our tcp session ref if needed */
+ if (pSesInfo)
+ cifs_put_smb_session(pSesInfo);
+ else
+ cifs_put_tcp_session(srvTcp);
} else {
atomic_inc(&tcon->useCount);
cifs_sb->tcon = tcon;
@@ -3506,16 +3541,7 @@ cifs_umount(struct super_block *sb, struct cifs_sb_info *cifs_sb)
}
DeleteTconOplockQEntries(cifs_sb->tcon);
tconInfoFree(cifs_sb->tcon);
- if ((ses) && (ses->server)) {
- /* save off task so we do not refer to ses later */
- cFYI(1, ("About to do SMBLogoff "));
- rc = CIFSSMBLogoff(xid, ses);
- if (rc == -EBUSY) {
- FreeXid(xid);
- return 0;
- }
- } else
- cFYI(1, ("No session or bad tcon"));
+ cifs_put_smb_session(ses);
}
cifs_sb->tcon = NULL;
@@ -3523,8 +3549,6 @@ cifs_umount(struct super_block *sb, struct cifs_sb_info *cifs_sb)
cifs_sb->prepathlen = 0;
cifs_sb->prepath = NULL;
kfree(tmp);
- if (ses)
- sesInfoFree(ses);
FreeXid(xid);
return rc;
diff --git a/fs/cifs/misc.c b/fs/cifs/misc.c
index 88786ba..4a5c6bd 100644
--- a/fs/cifs/misc.c
+++ b/fs/cifs/misc.c
@@ -75,12 +75,10 @@ sesInfoAlloc(void)
ret_buf = kzalloc(sizeof(struct cifsSesInfo), GFP_KERNEL);
if (ret_buf) {
- write_lock(&GlobalSMBSeslock);
atomic_inc(&sesInfoAllocCount);
ret_buf->status = CifsNew;
- list_add(&ret_buf->cifsSessionList, &GlobalSMBSessionList);
+ INIT_LIST_HEAD(&ret_buf->smb_session_list);
init_MUTEX(&ret_buf->sesSem);
- write_unlock(&GlobalSMBSeslock);
}
return ret_buf;
}
@@ -93,10 +91,7 @@ sesInfoFree(struct cifsSesInfo *buf_to_free)
return;
}
- write_lock(&GlobalSMBSeslock);
atomic_dec(&sesInfoAllocCount);
- list_del(&buf_to_free->cifsSessionList);
- write_unlock(&GlobalSMBSeslock);
kfree(buf_to_free->serverOS);
kfree(buf_to_free->serverDomain);
kfree(buf_to_free->serverNOS);
@@ -350,9 +345,9 @@ header_assemble(struct smb_hdr *buffer, char smb_command /* command */ ,
if (current->fsuid != treeCon->ses->linux_uid) {
cFYI(1, ("Multiuser mode and UID "
"did not match tcon uid"));
- read_lock(&GlobalSMBSeslock);
- list_for_each(temp_item, &GlobalSMBSessionList) {
- ses = list_entry(temp_item, struct cifsSesInfo, cifsSessionList);
+ spin_lock(&treeCon->ses->server->smb_session_lock);
+ list_for_each(temp_item, &treeCon->ses->server->smb_session_list) {
+ ses = list_entry(temp_item, struct cifsSesInfo, smb_session_list);
if (ses->linux_uid == current->fsuid) {
if (ses->server == treeCon->ses->server) {
cFYI(1, ("found matching uid substitute right smb_uid"));
@@ -364,7 +359,7 @@ header_assemble(struct smb_hdr *buffer, char smb_command /* command */ ,
}
}
}
- read_unlock(&GlobalSMBSeslock);
+ spin_unlock(&treeCon->ses->server->smb_session_lock);
}
}
}
--
1.5.5.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/4] cifs: clean up server protocol handling for TCP_Server_Info
2008-10-30 15:16 [PATCH 0/4] cifs: fix oopses and mem corruption with concurrent mount/umount (try #4) Jeff Layton
@ 2008-10-30 15:16 ` Jeff Layton
0 siblings, 0 replies; 6+ messages in thread
From: Jeff Layton @ 2008-10-30 15:16 UTC (permalink / raw)
To: smfrench; +Cc: smfrench, linux-cifs-client, linux-kernel
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 | 39 +++++++++++++++++++--------------------
2 files changed, 19 insertions(+), 28 deletions(-)
diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index 1fc051e..58b8d5c 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 e9f9248..7d0b2e5 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -193,7 +193,7 @@ cifs_reconnect(struct TCP_Server_Info *server)
while ((server->tcpStatus != CifsExiting) &&
(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, server->noautotune);
} else {
@@ -1892,10 +1892,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;
@@ -1928,16 +1928,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) {
@@ -1977,14 +1977,14 @@ 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,
+ 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 (address_type == AF_INET6) {
+ 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,
+ &sin_server6->sin6_addr,
volume_info.username, &srvTcp);
} else {
rc = -EINVAL;
@@ -1995,17 +1995,17 @@ 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,
volume_info.noblocksnd);
} else
- rc = ipv4_connect(&sin_server, &csocket,
+ rc = ipv4_connect(sin_server, &csocket,
volume_info.source_rfc1001_name,
volume_info.target_rfc1001_name,
volume_info.noblocksnd,
@@ -2026,12 +2026,11 @@ cifs_mount(struct super_block *sb, struct cifs_sb_info *cifs_sb,
} else {
srvTcp->noblocksnd = volume_info.noblocksnd;
srvTcp->noautotune = volume_info.noautotune;
- 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);
@@ -2083,7 +2082,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) {
--
1.5.5.1
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2008-10-30 15:16 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-10-22 14:13 [PATCH 0/4] cifs: fix oopses and mem corruption with concurrent mount/umount (try #3) Jeff Layton
2008-10-22 14:13 ` [PATCH 1/4] cifs: clean up server protocol handling for TCP_Server_Info Jeff Layton
2008-10-22 14:13 ` [PATCH 2/4] cifs: handle the TCP_Server_Info->tsk field more carefully Jeff Layton
2008-10-22 14:14 ` [PATCH 3/4] cifs: disable sharing session and tcon and add new TCP sharing code Jeff Layton
2008-10-22 14:14 ` [PATCH 4/4] cifs: reinstate sharing of SMB sessions Jeff Layton
2008-10-30 15:16 [PATCH 0/4] cifs: fix oopses and mem corruption with concurrent mount/umount (try #4) Jeff Layton
2008-10-30 15:16 ` [PATCH 1/4] cifs: clean up server protocol handling for TCP_Server_Info Jeff Layton
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®