Commit d7d7a66a authored by Shyam Prasad N's avatar Shyam Prasad N Committed by Steve French
Browse files

cifs: avoid use of global locks for high contention data 



During analysis of multichannel perf, it was seen that
the global locks cifs_tcp_ses_lock and GlobalMid_Lock, which
were shared between various data structures were causing a
lot of contention points.

With this change, we're breaking down the use of these locks
by introducing new locks at more granular levels. i.e.
server->srv_lock, ses->ses_lock and tcon->tc_lock to protect
the unprotected fields of server, session and tcon structs;
and server->mid_lock to protect mid related lists and entries
at server level.

Signed-off-by: default avatarShyam Prasad N <sprasad@microsoft.com>
Signed-off-by: default avatarSteve French <stfrench@microsoft.com>
parent 1bfa25ee

fs/cifs/cifs_debug.c

+4 −4
Original line number Diff line number Diff line
@@ -55,7 +55,7 @@ void cifs_dump_mids(struct TCP_Server_Info *server) 
		return;



	cifs_dbg(VFS, "Dump pending requests:\n");

	spin_lock(&GlobalMid_Lock);

	spin_lock(&server->mid_lock);

	list_for_each_entry(mid_entry, &server->pending_mid_q, qhead) {

		cifs_dbg(VFS, "State: %d Cmd: %d Pid: %d Cbdata: %p Mid %llu\n",

			 mid_entry->mid_state,
@@ -78,7 +78,7 @@ void cifs_dump_mids(struct TCP_Server_Info *server) 
				mid_entry->resp_buf, 62);

		}

	}

	spin_unlock(&GlobalMid_Lock);

	spin_unlock(&server->mid_lock);

#endif /* CONFIG_CIFS_DEBUG2 */

}
@@ -463,7 +463,7 @@ static int cifs_debug_data_proc_show(struct seq_file *m, void *v) 
			seq_printf(m, "\n\t\t[NONE]");



		seq_puts(m, "\n\n\tMIDs: ");

		spin_lock(&GlobalMid_Lock);

		spin_lock(&server->mid_lock);

		list_for_each_entry(mid_entry, &server->pending_mid_q, qhead) {

			seq_printf(m, "\n\tState: %d com: %d pid:"

					" %d cbdata: %p mid %llu\n",
@@ -473,7 +473,7 @@ static int cifs_debug_data_proc_show(struct seq_file *m, void *v) 
					mid_entry->callback_data,

					mid_entry->mid);

		}

		spin_unlock(&GlobalMid_Lock);

		spin_unlock(&server->mid_lock);

		seq_printf(m, "\n--\n");

	}

	if (c == 0)

fs/cifs/cifsencrypt.c

+3 −3
Original line number Diff line number Diff line
@@ -141,13 +141,13 @@ int cifs_sign_rqst(struct smb_rqst *rqst, struct TCP_Server_Info *server, 
	if ((cifs_pdu == NULL) || (server == NULL))

		return -EINVAL;



	spin_lock(&cifs_tcp_ses_lock);

	spin_lock(&server->srv_lock);

	if (!(cifs_pdu->Flags2 & SMBFLG2_SECURITY_SIGNATURE) ||

	    server->tcpStatus == CifsNeedNegotiate) {

		spin_unlock(&cifs_tcp_ses_lock);

		spin_unlock(&server->srv_lock);

		return rc;

	}

	spin_unlock(&cifs_tcp_ses_lock);

	spin_unlock(&server->srv_lock);



	if (!server->session_estab) {

		memcpy(cifs_pdu->Signature.SecuritySignature, "BSRSPYL", 8);

fs/cifs/cifsfs.c

+3 −0
Original line number Diff line number Diff line
@@ -731,14 +731,17 @@ static void cifs_umount_begin(struct super_block *sb) 
	tcon = cifs_sb_master_tcon(cifs_sb);



	spin_lock(&cifs_tcp_ses_lock);

	spin_lock(&tcon->tc_lock);

	if ((tcon->tc_count > 1) || (tcon->status == TID_EXITING)) {

		/* we have other mounts to same share or we have

		   already tried to force umount this and woken up

		   all waiting network requests, nothing to do */

		spin_unlock(&tcon->tc_lock);

		spin_unlock(&cifs_tcp_ses_lock);

		return;

	} else if (tcon->tc_count == 1)

		tcon->status = TID_EXITING;

	spin_unlock(&tcon->tc_lock);

	spin_unlock(&cifs_tcp_ses_lock);



	/* cancel_brl_requests(tcon); */ /* BB mark all brl mids as exiting */

fs/cifs/cifsglob.h

+73 −26
Original line number Diff line number Diff line
@@ -605,6 +605,7 @@ inc_rfc1001_len(void *buf, int count) 
struct TCP_Server_Info {

	struct list_head tcp_ses_list;

	struct list_head smb_ses_list;

	spinlock_t srv_lock;  /* protect anything here that is not protected */

	__u64 conn_id; /* connection identifier (useful for debugging) */

	int srv_count; /* reference counter */

	/* 15 character server name + 0x20 16th byte indicating type = srv */
@@ -622,6 +623,7 @@ struct TCP_Server_Info { 
#endif

	wait_queue_head_t response_q;

	wait_queue_head_t request_q; /* if more than maxmpx to srvr must block*/

	spinlock_t mid_lock;  /* protect mid queue and it's entries */

	struct list_head pending_mid_q;

	bool noblocksnd;		/* use blocking sendmsg */

	bool noautotune;		/* do not autotune send buf sizes */
@@ -1008,6 +1010,7 @@ struct cifs_ses { 
	struct list_head rlist; /* reconnect list */

	struct list_head tcon_list;

	struct cifs_tcon *tcon_ipc;

	spinlock_t ses_lock;  /* protect anything here that is not protected */

	struct mutex session_mutex;

	struct TCP_Server_Info *server;	/* pointer to server info */

	int ses_count;		/* reference counter */
@@ -1169,6 +1172,7 @@ struct cifs_tcon { 
	struct list_head tcon_list;

	int tc_count;

	struct list_head rlist; /* reconnect list */

	spinlock_t tc_lock;  /* protect anything here that is not protected */

	atomic_t num_local_opens;  /* num of all opens including disconnected */

	atomic_t num_remote_opens; /* num of all network opens on server */

	struct list_head openFileList;
@@ -1899,33 +1903,78 @@ require use of the stronger protocol */ 
 */



/****************************************************************************

 *  Locking notes.  All updates to global variables and lists should be

 *                  protected by spinlocks or semaphores.

 * Here are all the locks (spinlock, mutex, semaphore) in cifs.ko, arranged according

 * to the locking order. i.e. if two locks are to be held together, the lock that

 * appears higher in this list needs to be taken before the other.

 *

 *  Spinlocks

 *  ---------

 *  GlobalMid_Lock protects:

 *	list operations on pending_mid_q and oplockQ

 *      updates to XID counters, multiplex id  and SMB sequence numbers

 *      list operations on global DnotifyReqList

 *      updates to ses->status and TCP_Server_Info->tcpStatus

 *      updates to server->CurrentMid

 *  tcp_ses_lock protects:

 *	list operations on tcp and SMB session lists

 *  tcon->open_file_lock protects the list of open files hanging off the tcon

 *  inode->open_file_lock protects the openFileList hanging off the inode

 *  cfile->file_info_lock protects counters and fields in cifs file struct

 *  f_owner.lock protects certain per file struct operations

 *  mapping->page_lock protects certain per page operations

 * If you hold a lock that is lower in this list, and you need to take a higher lock

 * (or if you think that one of the functions that you're calling may need to), first

 * drop the lock you hold, pick up the higher lock, then the lower one. This will

 * ensure that locks are picked up only in one direction in the below table

 * (top to bottom).

 *

 *  Note that the cifs_tcon.open_file_lock should be taken before

 *  not after the cifsInodeInfo.open_file_lock

 * Also, if you expect a function to be called with a lock held, explicitly document

 * this in the comments on top of your function definition.

 *

 *  Semaphores

 *  ----------

 *  cifsInodeInfo->lock_sem protects:

 *	the list of locks held by the inode

 * And also, try to keep the critical sections (lock hold time) to be as minimal as

 * possible. Blocking / calling other functions with a lock held always increase

 * the risk of a possible deadlock.

 *

 * Following this rule will avoid unnecessary deadlocks, which can get really hard to

 * debug. Also, any new lock that you introduce, please add to this list in the correct

 * order.

 *

 * Please populate this list whenever you introduce new locks in your changes. Or in

 * case I've missed some existing locks. Please ensure that it's added in the list

 * based on the locking order expected.

 *

 * =====================================================================================

 * Lock				Protects			Initialization fn

 * =====================================================================================

 * vol_list_lock

 * vol_info->ctx_lock		vol_info->ctx

 * cifs_sb_info->tlink_tree_lock	cifs_sb_info->tlink_tree	cifs_setup_cifs_sb

 * TCP_Server_Info->		TCP_Server_Info			cifs_get_tcp_session

 * reconnect_mutex

 * TCP_Server_Info->srv_mutex	TCP_Server_Info			cifs_get_tcp_session

 * cifs_ses->session_mutex		cifs_ses		sesInfoAlloc

 *				cifs_tcon

 * cifs_tcon->open_file_lock	cifs_tcon->openFileList		tconInfoAlloc

 *				cifs_tcon->pending_opens

 * cifs_tcon->stat_lock		cifs_tcon->bytes_read		tconInfoAlloc

 *				cifs_tcon->bytes_written

 * cifs_tcp_ses_lock		cifs_tcp_ses_list		sesInfoAlloc

 * GlobalMid_Lock		GlobalMaxActiveXid		init_cifs

 *				GlobalCurrentXid

 *				GlobalTotalActiveXid

 * TCP_Server_Info->srv_lock	(anything in struct not protected by another lock and can change)

 * TCP_Server_Info->mid_lock	TCP_Server_Info->pending_mid_q	cifs_get_tcp_session

 *				->CurrentMid

 *				(any changes in mid_q_entry fields)

 * TCP_Server_Info->req_lock	TCP_Server_Info->in_flight	cifs_get_tcp_session

 *				->credits

 *				->echo_credits

 *				->oplock_credits

 *				->reconnect_instance

 * cifs_ses->ses_lock		(anything that is not protected by another lock and can change)

 * cifs_ses->iface_lock		cifs_ses->iface_list		sesInfoAlloc

 *				->iface_count

 *				->iface_last_update

 * cifs_ses->chan_lock		cifs_ses->chans

 *				->chans_need_reconnect

 *				->chans_in_reconnect

 * cifs_tcon->tc_lock		(anything that is not protected by another lock and can change)

 * cifsInodeInfo->open_file_lock	cifsInodeInfo->openFileList	cifs_alloc_inode

 * cifsInodeInfo->writers_lock	cifsInodeInfo->writers		cifsInodeInfo_alloc

 * cifsInodeInfo->lock_sem	cifsInodeInfo->llist		cifs_init_once

 *				->can_cache_brlcks

 * cifsInodeInfo->deferred_lock	cifsInodeInfo->deferred_closes	cifsInodeInfo_alloc

 * cached_fid->fid_mutex		cifs_tcon->crfid		tconInfoAlloc

 * cifsFileInfo->fh_mutex		cifsFileInfo			cifs_new_fileinfo

 * cifsFileInfo->file_info_lock	cifsFileInfo->count		cifs_new_fileinfo

 *				->invalidHandle			initiate_cifs_search

 *				->oplock_break_cancelled

 * cifs_aio_ctx->aio_mutex		cifs_aio_ctx			cifs_aio_ctx_alloc

 ****************************************************************************/



#ifdef DECLARE_GLOBALS_HERE
@@ -1946,9 +1995,7 @@ extern struct list_head cifs_tcp_ses_list; 
/*

 * This lock protects the cifs_tcp_ses_list, the list of smb sessions per

 * tcp session, and the list of tcon's per smb session. It also protects

 * the reference counters for the server, smb session, and tcon. It also

 * protects some fields in the TCP_Server_Info struct such as dstaddr. Finally,

 * changes to the tcon->tidStatus should be done while holding this lock.

 * the reference counters for the server, smb session, and tcon.

 * generally the locks should be taken in order tcp_ses_lock before

 * tcon->open_file_lock and that before file->file_info_lock since the

 * structure order is cifs_socket-->cifs_ses-->cifs_tcon-->cifs_file

fs/cifs/cifssmb.c

+14 −14
Original line number Diff line number Diff line
@@ -74,13 +74,13 @@ cifs_mark_open_files_invalid(struct cifs_tcon *tcon) 
	struct list_head *tmp1;



	/* only send once per connect */

	spin_lock(&cifs_tcp_ses_lock);

	spin_lock(&tcon->ses->ses_lock);

	if ((tcon->ses->ses_status != SES_GOOD) || (tcon->status != TID_NEED_RECON)) {

		spin_unlock(&cifs_tcp_ses_lock);

		spin_unlock(&tcon->ses->ses_lock);

		return;

	}

	tcon->status = TID_IN_FILES_INVALIDATE;

	spin_unlock(&cifs_tcp_ses_lock);

	spin_unlock(&tcon->ses->ses_lock);



	/* list all files open on tree connection and mark them invalid */

	spin_lock(&tcon->open_file_lock);
@@ -98,10 +98,10 @@ cifs_mark_open_files_invalid(struct cifs_tcon *tcon) 
	memset(tcon->crfid.fid, 0, sizeof(struct cifs_fid));

	mutex_unlock(&tcon->crfid.fid_mutex);



	spin_lock(&cifs_tcp_ses_lock);

	spin_lock(&tcon->tc_lock);

	if (tcon->status == TID_IN_FILES_INVALIDATE)

		tcon->status = TID_NEED_TCON;

	spin_unlock(&cifs_tcp_ses_lock);

	spin_unlock(&tcon->tc_lock);



	/*

	 * BB Add call to invalidate_inodes(sb) for all superblocks mounted
@@ -134,18 +134,18 @@ cifs_reconnect_tcon(struct cifs_tcon *tcon, int smb_command) 
	 * only tree disconnect, open, and write, (and ulogoff which does not

	 * have tcon) are allowed as we start force umount

	 */

	spin_lock(&cifs_tcp_ses_lock);

	spin_lock(&tcon->tc_lock);

	if (tcon->status == TID_EXITING) {

		if (smb_command != SMB_COM_WRITE_ANDX &&

		    smb_command != SMB_COM_OPEN_ANDX &&

		    smb_command != SMB_COM_TREE_DISCONNECT) {

			spin_unlock(&cifs_tcp_ses_lock);

			spin_unlock(&tcon->tc_lock);

			cifs_dbg(FYI, "can not send cmd %d while umounting\n",

				 smb_command);

			return -ENODEV;

		}

	}

	spin_unlock(&cifs_tcp_ses_lock);

	spin_unlock(&tcon->tc_lock);



	retries = server->nr_targets;
@@ -165,12 +165,12 @@ cifs_reconnect_tcon(struct cifs_tcon *tcon, int smb_command) 
		}



		/* are we still trying to reconnect? */

		spin_lock(&cifs_tcp_ses_lock);

		spin_lock(&server->srv_lock);

		if (server->tcpStatus != CifsNeedReconnect) {

			spin_unlock(&cifs_tcp_ses_lock);

			spin_unlock(&server->srv_lock);

			break;

		}

		spin_unlock(&cifs_tcp_ses_lock);

		spin_unlock(&server->srv_lock);



		if (retries && --retries)

			continue;
@@ -201,13 +201,13 @@ cifs_reconnect_tcon(struct cifs_tcon *tcon, int smb_command) 
	 * and the server never sends an answer the socket will be closed

	 * and tcpStatus set to reconnect.

	 */

	spin_lock(&cifs_tcp_ses_lock);

	spin_lock(&server->srv_lock);

	if (server->tcpStatus == CifsNeedReconnect) {

		spin_unlock(&cifs_tcp_ses_lock);

		spin_unlock(&server->srv_lock);

		rc = -EHOSTDOWN;

		goto out;

	}

	spin_unlock(&cifs_tcp_ses_lock);

	spin_unlock(&server->srv_lock);



	/*

	 * need to prevent multiple threads trying to simultaneously