cifs: convert GlobalSMBSeslock from a rwlock to regular spinlock

Convert this lock to a regular spinlock

A rwlock_t offers little value here. It's more expensive than a regular
spinlock unless you have a fairly large section of code that runs under
the read lock and can benefit from the concurrency.

Additionally, we need to ensure that the refcounting for files isn't
racy and to do that we need to lock areas that can increment it for
write. That means that the areas that can actually use a read_lock are
very few and relatively infrequently used.

While we're at it, change the name to something easier to type, and fix
a bug in find_writable_file. cifsFileInfo_put can sleep and shouldn't be
called while holding the lock.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
Reviewed-by: Suresh Jayaraman <sjayaraman@suse.de>
Signed-off-by: Steve French <sfrench@us.ibm.com>
This commit is contained in:
Jeff Layton 2010-10-15 15:34:03 -04:00 committed by Steve French
parent 7a16f1961a
commit 4477288a10
6 changed files with 38 additions and 38 deletions

View File

@ -940,8 +940,8 @@ init_cifs(void)
GlobalTotalActiveXid = 0; GlobalTotalActiveXid = 0;
GlobalMaxActiveXid = 0; GlobalMaxActiveXid = 0;
memset(Local_System_Name, 0, 15); memset(Local_System_Name, 0, 15);
rwlock_init(&GlobalSMBSeslock);
rwlock_init(&cifs_tcp_ses_lock); rwlock_init(&cifs_tcp_ses_lock);
spin_lock_init(&cifs_file_list_lock);
spin_lock_init(&GlobalMid_Lock); spin_lock_init(&GlobalMid_Lock);
if (cifs_max_pending < 2) { if (cifs_max_pending < 2) {

View File

@ -720,7 +720,7 @@ GLOBAL_EXTERN rwlock_t cifs_tcp_ses_lock;
* If cifs_tcp_ses_lock and the lock below are both needed to be held, then * If cifs_tcp_ses_lock and the lock below are both needed to be held, then
* the cifs_tcp_ses_lock must be grabbed first and released last. * the cifs_tcp_ses_lock must be grabbed first and released last.
*/ */
GLOBAL_EXTERN rwlock_t GlobalSMBSeslock; GLOBAL_EXTERN spinlock_t cifs_file_list_lock;
/* Outstanding dir notify requests */ /* Outstanding dir notify requests */
GLOBAL_EXTERN struct list_head GlobalDnotifyReqList; GLOBAL_EXTERN struct list_head GlobalDnotifyReqList;

View File

@ -91,13 +91,13 @@ static void mark_open_files_invalid(struct cifsTconInfo *pTcon)
struct list_head *tmp1; struct list_head *tmp1;
/* list all files open on tree connection and mark them invalid */ /* list all files open on tree connection and mark them invalid */
write_lock(&GlobalSMBSeslock); spin_lock(&cifs_file_list_lock);
list_for_each_safe(tmp, tmp1, &pTcon->openFileList) { list_for_each_safe(tmp, tmp1, &pTcon->openFileList) {
open_file = list_entry(tmp, struct cifsFileInfo, tlist); open_file = list_entry(tmp, struct cifsFileInfo, tlist);
open_file->invalidHandle = true; open_file->invalidHandle = true;
open_file->oplock_break_cancelled = true; open_file->oplock_break_cancelled = true;
} }
write_unlock(&GlobalSMBSeslock); spin_unlock(&cifs_file_list_lock);
/* BB Add call to invalidate_inodes(sb) for all superblocks mounted /* BB Add call to invalidate_inodes(sb) for all superblocks mounted
to this tcon */ to this tcon */
} }

View File

@ -246,14 +246,14 @@ cifs_new_fileinfo(__u16 fileHandle, struct file *file,
atomic_set(&pCifsFile->count, 1); atomic_set(&pCifsFile->count, 1);
INIT_WORK(&pCifsFile->oplock_break, cifs_oplock_break); INIT_WORK(&pCifsFile->oplock_break, cifs_oplock_break);
write_lock(&GlobalSMBSeslock); spin_lock(&cifs_file_list_lock);
list_add(&pCifsFile->tlist, &(tlink_tcon(tlink)->openFileList)); list_add(&pCifsFile->tlist, &(tlink_tcon(tlink)->openFileList));
/* if readable file instance put first in list*/ /* if readable file instance put first in list*/
if (file->f_mode & FMODE_READ) if (file->f_mode & FMODE_READ)
list_add(&pCifsFile->flist, &pCifsInode->openFileList); list_add(&pCifsFile->flist, &pCifsInode->openFileList);
else else
list_add_tail(&pCifsFile->flist, &pCifsInode->openFileList); list_add_tail(&pCifsFile->flist, &pCifsInode->openFileList);
write_unlock(&GlobalSMBSeslock); spin_unlock(&cifs_file_list_lock);
if ((oplock & 0xF) == OPLOCK_EXCLUSIVE) { if ((oplock & 0xF) == OPLOCK_EXCLUSIVE) {
pCifsInode->clientCanCacheAll = true; pCifsInode->clientCanCacheAll = true;
@ -607,13 +607,13 @@ int cifs_close(struct inode *inode, struct file *file)
pTcon = tlink_tcon(pSMBFile->tlink); pTcon = tlink_tcon(pSMBFile->tlink);
if (pSMBFile) { if (pSMBFile) {
struct cifsLockInfo *li, *tmp; struct cifsLockInfo *li, *tmp;
write_lock(&GlobalSMBSeslock); spin_lock(&cifs_file_list_lock);
pSMBFile->closePend = true; pSMBFile->closePend = true;
if (pTcon) { if (pTcon) {
/* no sense reconnecting to close a file that is /* no sense reconnecting to close a file that is
already closed */ already closed */
if (!pTcon->need_reconnect) { if (!pTcon->need_reconnect) {
write_unlock(&GlobalSMBSeslock); spin_unlock(&cifs_file_list_lock);
timeout = 2; timeout = 2;
while ((atomic_read(&pSMBFile->count) != 1) while ((atomic_read(&pSMBFile->count) != 1)
&& (timeout <= 2048)) { && (timeout <= 2048)) {
@ -633,9 +633,9 @@ int cifs_close(struct inode *inode, struct file *file)
rc = CIFSSMBClose(xid, pTcon, rc = CIFSSMBClose(xid, pTcon,
pSMBFile->netfid); pSMBFile->netfid);
} else } else
write_unlock(&GlobalSMBSeslock); spin_unlock(&cifs_file_list_lock);
} else } else
write_unlock(&GlobalSMBSeslock); spin_unlock(&cifs_file_list_lock);
/* Delete any outstanding lock records. /* Delete any outstanding lock records.
We'll lose them when the file is closed anyway. */ We'll lose them when the file is closed anyway. */
@ -646,16 +646,16 @@ int cifs_close(struct inode *inode, struct file *file)
} }
mutex_unlock(&pSMBFile->lock_mutex); mutex_unlock(&pSMBFile->lock_mutex);
write_lock(&GlobalSMBSeslock); spin_lock(&cifs_file_list_lock);
list_del(&pSMBFile->flist); list_del(&pSMBFile->flist);
list_del(&pSMBFile->tlist); list_del(&pSMBFile->tlist);
write_unlock(&GlobalSMBSeslock); spin_unlock(&cifs_file_list_lock);
cifsFileInfo_put(file->private_data); cifsFileInfo_put(file->private_data);
file->private_data = NULL; file->private_data = NULL;
} else } else
rc = -EBADF; rc = -EBADF;
read_lock(&GlobalSMBSeslock); spin_lock(&cifs_file_list_lock);
if (list_empty(&(CIFS_I(inode)->openFileList))) { if (list_empty(&(CIFS_I(inode)->openFileList))) {
cFYI(1, "closing last open instance for inode %p", inode); cFYI(1, "closing last open instance for inode %p", inode);
/* if the file is not open we do not know if we can cache info /* if the file is not open we do not know if we can cache info
@ -663,7 +663,7 @@ int cifs_close(struct inode *inode, struct file *file)
CIFS_I(inode)->clientCanCacheRead = false; CIFS_I(inode)->clientCanCacheRead = false;
CIFS_I(inode)->clientCanCacheAll = false; CIFS_I(inode)->clientCanCacheAll = false;
} }
read_unlock(&GlobalSMBSeslock); spin_unlock(&cifs_file_list_lock);
if ((rc == 0) && CIFS_I(inode)->write_behind_rc) if ((rc == 0) && CIFS_I(inode)->write_behind_rc)
rc = CIFS_I(inode)->write_behind_rc; rc = CIFS_I(inode)->write_behind_rc;
FreeXid(xid); FreeXid(xid);
@ -685,18 +685,18 @@ int cifs_closedir(struct inode *inode, struct file *file)
struct cifsTconInfo *pTcon = tlink_tcon(pCFileStruct->tlink); struct cifsTconInfo *pTcon = tlink_tcon(pCFileStruct->tlink);
cFYI(1, "Freeing private data in close dir"); cFYI(1, "Freeing private data in close dir");
write_lock(&GlobalSMBSeslock); spin_lock(&cifs_file_list_lock);
if (!pCFileStruct->srch_inf.endOfSearch && if (!pCFileStruct->srch_inf.endOfSearch &&
!pCFileStruct->invalidHandle) { !pCFileStruct->invalidHandle) {
pCFileStruct->invalidHandle = true; pCFileStruct->invalidHandle = true;
write_unlock(&GlobalSMBSeslock); spin_unlock(&cifs_file_list_lock);
rc = CIFSFindClose(xid, pTcon, pCFileStruct->netfid); rc = CIFSFindClose(xid, pTcon, pCFileStruct->netfid);
cFYI(1, "Closing uncompleted readdir with rc %d", cFYI(1, "Closing uncompleted readdir with rc %d",
rc); rc);
/* not much we can do if it fails anyway, ignore rc */ /* not much we can do if it fails anyway, ignore rc */
rc = 0; rc = 0;
} else } else
write_unlock(&GlobalSMBSeslock); spin_unlock(&cifs_file_list_lock);
ptmp = pCFileStruct->srch_inf.ntwrk_buf_start; ptmp = pCFileStruct->srch_inf.ntwrk_buf_start;
if (ptmp) { if (ptmp) {
cFYI(1, "closedir free smb buf in srch struct"); cFYI(1, "closedir free smb buf in srch struct");
@ -1182,7 +1182,7 @@ struct cifsFileInfo *find_readable_file(struct cifsInodeInfo *cifs_inode,
if (!(cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MULTIUSER)) if (!(cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MULTIUSER))
fsuid_only = false; fsuid_only = false;
read_lock(&GlobalSMBSeslock); spin_lock(&cifs_file_list_lock);
/* we could simply get the first_list_entry since write-only entries /* we could simply get the first_list_entry since write-only entries
are always at the end of the list but since the first entry might are always at the end of the list but since the first entry might
have a close pending, we go through the whole list */ have a close pending, we go through the whole list */
@ -1196,7 +1196,7 @@ struct cifsFileInfo *find_readable_file(struct cifsInodeInfo *cifs_inode,
/* found a good file */ /* found a good file */
/* lock it so it will not be closed on us */ /* lock it so it will not be closed on us */
cifsFileInfo_get(open_file); cifsFileInfo_get(open_file);
read_unlock(&GlobalSMBSeslock); spin_unlock(&cifs_file_list_lock);
return open_file; return open_file;
} /* else might as well continue, and look for } /* else might as well continue, and look for
another, or simply have the caller reopen it another, or simply have the caller reopen it
@ -1204,7 +1204,7 @@ struct cifsFileInfo *find_readable_file(struct cifsInodeInfo *cifs_inode,
} else /* write only file */ } else /* write only file */
break; /* write only files are last so must be done */ break; /* write only files are last so must be done */
} }
read_unlock(&GlobalSMBSeslock); spin_unlock(&cifs_file_list_lock);
return NULL; return NULL;
} }
#endif #endif
@ -1231,7 +1231,7 @@ struct cifsFileInfo *find_writable_file(struct cifsInodeInfo *cifs_inode,
if (!(cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MULTIUSER)) if (!(cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MULTIUSER))
fsuid_only = false; fsuid_only = false;
read_lock(&GlobalSMBSeslock); spin_lock(&cifs_file_list_lock);
refind_writable: refind_writable:
list_for_each_entry(open_file, &cifs_inode->openFileList, flist) { list_for_each_entry(open_file, &cifs_inode->openFileList, flist) {
if (open_file->closePend) if (open_file->closePend)
@ -1245,11 +1245,11 @@ refind_writable:
if (!open_file->invalidHandle) { if (!open_file->invalidHandle) {
/* found a good writable file */ /* found a good writable file */
read_unlock(&GlobalSMBSeslock); spin_unlock(&cifs_file_list_lock);
return open_file; return open_file;
} }
read_unlock(&GlobalSMBSeslock); spin_unlock(&cifs_file_list_lock);
/* Had to unlock since following call can block */ /* Had to unlock since following call can block */
rc = cifs_reopen_file(open_file, false); rc = cifs_reopen_file(open_file, false);
if (!rc) { if (!rc) {
@ -1257,7 +1257,7 @@ refind_writable:
return open_file; return open_file;
else { /* start over in case this was deleted */ else { /* start over in case this was deleted */
/* since the list could be modified */ /* since the list could be modified */
read_lock(&GlobalSMBSeslock); spin_lock(&cifs_file_list_lock);
cifsFileInfo_put(open_file); cifsFileInfo_put(open_file);
goto refind_writable; goto refind_writable;
} }
@ -1271,7 +1271,7 @@ refind_writable:
to hold up writepages here (rather than to hold up writepages here (rather than
in caller) with continuous retries */ in caller) with continuous retries */
cFYI(1, "wp failed on reopen file"); cFYI(1, "wp failed on reopen file");
read_lock(&GlobalSMBSeslock); spin_lock(&cifs_file_list_lock);
/* can not use this handle, no write /* can not use this handle, no write
pending on this one after all */ pending on this one after all */
cifsFileInfo_put(open_file); cifsFileInfo_put(open_file);
@ -1292,7 +1292,7 @@ refind_writable:
any_available = true; any_available = true;
goto refind_writable; goto refind_writable;
} }
read_unlock(&GlobalSMBSeslock); spin_unlock(&cifs_file_list_lock);
return NULL; return NULL;
} }
@ -2200,16 +2200,16 @@ static int is_inode_writable(struct cifsInodeInfo *cifs_inode)
{ {
struct cifsFileInfo *open_file; struct cifsFileInfo *open_file;
read_lock(&GlobalSMBSeslock); spin_lock(&cifs_file_list_lock);
list_for_each_entry(open_file, &cifs_inode->openFileList, flist) { list_for_each_entry(open_file, &cifs_inode->openFileList, flist) {
if (open_file->closePend) if (open_file->closePend)
continue; continue;
if (OPEN_FMODE(open_file->f_flags) & FMODE_WRITE) { if (OPEN_FMODE(open_file->f_flags) & FMODE_WRITE) {
read_unlock(&GlobalSMBSeslock); spin_unlock(&cifs_file_list_lock);
return 1; return 1;
} }
} }
read_unlock(&GlobalSMBSeslock); spin_unlock(&cifs_file_list_lock);
return 0; return 0;
} }
@ -2373,8 +2373,8 @@ void cifs_oplock_break(struct work_struct *work)
* finished grabbing reference for us. Make sure it's done by * finished grabbing reference for us. Make sure it's done by
* waiting for GlobalSMSSeslock. * waiting for GlobalSMSSeslock.
*/ */
write_lock(&GlobalSMBSeslock); spin_lock(&cifs_file_list_lock);
write_unlock(&GlobalSMBSeslock); spin_unlock(&cifs_file_list_lock);
cifs_oplock_break_put(cfile); cifs_oplock_break_put(cfile);
} }

View File

@ -560,7 +560,7 @@ is_valid_oplock_break(struct smb_hdr *buf, struct TCP_Server_Info *srv)
continue; continue;
cifs_stats_inc(&tcon->num_oplock_brks); cifs_stats_inc(&tcon->num_oplock_brks);
read_lock(&GlobalSMBSeslock); spin_lock(&cifs_file_list_lock);
list_for_each(tmp2, &tcon->openFileList) { list_for_each(tmp2, &tcon->openFileList) {
netfile = list_entry(tmp2, struct cifsFileInfo, netfile = list_entry(tmp2, struct cifsFileInfo,
tlist); tlist);
@ -572,7 +572,7 @@ is_valid_oplock_break(struct smb_hdr *buf, struct TCP_Server_Info *srv)
* closed anyway. * closed anyway.
*/ */
if (netfile->closePend) { if (netfile->closePend) {
read_unlock(&GlobalSMBSeslock); spin_unlock(&cifs_file_list_lock);
read_unlock(&cifs_tcp_ses_lock); read_unlock(&cifs_tcp_ses_lock);
return true; return true;
} }
@ -594,11 +594,11 @@ is_valid_oplock_break(struct smb_hdr *buf, struct TCP_Server_Info *srv)
cifs_oplock_break_get(netfile); cifs_oplock_break_get(netfile);
netfile->oplock_break_cancelled = false; netfile->oplock_break_cancelled = false;
read_unlock(&GlobalSMBSeslock); spin_unlock(&cifs_file_list_lock);
read_unlock(&cifs_tcp_ses_lock); read_unlock(&cifs_tcp_ses_lock);
return true; return true;
} }
read_unlock(&GlobalSMBSeslock); spin_unlock(&cifs_file_list_lock);
read_unlock(&cifs_tcp_ses_lock); read_unlock(&cifs_tcp_ses_lock);
cFYI(1, "No matching file for oplock break"); cFYI(1, "No matching file for oplock break");
return true; return true;

View File

@ -528,14 +528,14 @@ static int find_cifs_entry(const int xid, struct cifsTconInfo *pTcon,
(index_to_find < first_entry_in_buffer)) { (index_to_find < first_entry_in_buffer)) {
/* close and restart search */ /* close and restart search */
cFYI(1, "search backing up - close and restart search"); cFYI(1, "search backing up - close and restart search");
write_lock(&GlobalSMBSeslock); spin_lock(&cifs_file_list_lock);
if (!cifsFile->srch_inf.endOfSearch && if (!cifsFile->srch_inf.endOfSearch &&
!cifsFile->invalidHandle) { !cifsFile->invalidHandle) {
cifsFile->invalidHandle = true; cifsFile->invalidHandle = true;
write_unlock(&GlobalSMBSeslock); spin_unlock(&cifs_file_list_lock);
CIFSFindClose(xid, pTcon, cifsFile->netfid); CIFSFindClose(xid, pTcon, cifsFile->netfid);
} else } else
write_unlock(&GlobalSMBSeslock); spin_unlock(&cifs_file_list_lock);
if (cifsFile->srch_inf.ntwrk_buf_start) { if (cifsFile->srch_inf.ntwrk_buf_start) {
cFYI(1, "freeing SMB ff cache buf on search rewind"); cFYI(1, "freeing SMB ff cache buf on search rewind");
if (cifsFile->srch_inf.smallBuf) if (cifsFile->srch_inf.smallBuf)