[PATCH v5 0/5] transport/session code move to ops struct

[PATCH v5 0/5] transport/session code move to ops struct

[Pavel Shilovsky]

This is the rest of the patchset that makes the cifs transport code
work through ops struct of the server.

This version has improved credit patch (#2).

Pavel Shilovsky (5):
  CIFS: Move trans2 processing to ops struct
  CIFS: Extend credit mechanism to process request type
  CIFS: Move protocol specific negotiate code to ops struct
  CIFS: Move protocol specific session setup/logoff code to ops struct
  CIFS: Move protocol specific tcon/tdis code to ops struct

 fs/cifs/cifsglob.h  |   38 ++++++-
 fs/cifs/cifsproto.h |   20 ++--
 fs/cifs/cifssmb.c   |   25 +++---
 fs/cifs/connect.c   |  269 ++++++++++++---------------------------------------
 fs/cifs/sess.c      |    4 +-
 fs/cifs/smb1ops.c   |  209 +++++++++++++++++++++++++++++++++++++++-
 fs/cifs/transport.c |   54 ++++++-----
 7 files changed, 356 insertions(+), 263 deletions(-)

[PATCH v5 1/5] CIFS: Move trans2 processing to ops struct

[Pavel Shilovsky]

Reviewed-by: Jeff Layton <jlayton-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
Signed-off-by: Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
---
 fs/cifs/cifsglob.h |    3 +
 fs/cifs/connect.c  |  161 +------------------------------------------------
 fs/cifs/smb1ops.c  |  170 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 175 insertions(+), 159 deletions(-)

diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index 6df0cbe..2aac4e5 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -187,6 +187,9 @@ struct smb_version_operations {
 	/* verify the message */
 	int (*check_message)(char *, unsigned int);
 	bool (*is_oplock_break)(char *, struct TCP_Server_Info *);
+	/* process transaction2 response */
+	bool (*check_trans2)(struct mid_q_entry *, struct TCP_Server_Info *,
+			     char *, int);
 };
 
 struct smb_version_values {
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 78db68a..ca0288d 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -394,143 +394,6 @@ cifs_reconnect(struct TCP_Server_Info *server)
 	return rc;
 }
 
-/*
-	return codes:
-		0 	not a transact2, or all data present
-		>0 	transact2 with that much data missing
-		-EINVAL = invalid transact2
-
- */
-static int check2ndT2(char *buf)
-{
-	struct smb_hdr *pSMB = (struct smb_hdr *)buf;
-	struct smb_t2_rsp *pSMBt;
-	int remaining;
-	__u16 total_data_size, data_in_this_rsp;
-
-	if (pSMB->Command != SMB_COM_TRANSACTION2)
-		return 0;
-
-	/* check for plausible wct, bcc and t2 data and parm sizes */
-	/* check for parm and data offset going beyond end of smb */
-	if (pSMB->WordCount != 10) { /* coalesce_t2 depends on this */
-		cFYI(1, "invalid transact2 word count");
-		return -EINVAL;
-	}
-
-	pSMBt = (struct smb_t2_rsp *)pSMB;
-
-	total_data_size = get_unaligned_le16(&pSMBt->t2_rsp.TotalDataCount);
-	data_in_this_rsp = get_unaligned_le16(&pSMBt->t2_rsp.DataCount);
-
-	if (total_data_size == data_in_this_rsp)
-		return 0;
-	else if (total_data_size < data_in_this_rsp) {
-		cFYI(1, "total data %d smaller than data in frame %d",
-			total_data_size, data_in_this_rsp);
-		return -EINVAL;
-	}
-
-	remaining = total_data_size - data_in_this_rsp;
-
-	cFYI(1, "missing %d bytes from transact2, check next response",
-		remaining);
-	if (total_data_size > CIFSMaxBufSize) {
-		cERROR(1, "TotalDataSize %d is over maximum buffer %d",
-			total_data_size, CIFSMaxBufSize);
-		return -EINVAL;
-	}
-	return remaining;
-}
-
-static int coalesce_t2(char *second_buf, struct smb_hdr *target_hdr)
-{
-	struct smb_t2_rsp *pSMBs = (struct smb_t2_rsp *)second_buf;
-	struct smb_t2_rsp *pSMBt  = (struct smb_t2_rsp *)target_hdr;
-	char *data_area_of_tgt;
-	char *data_area_of_src;
-	int remaining;
-	unsigned int byte_count, total_in_tgt;
-	__u16 tgt_total_cnt, src_total_cnt, total_in_src;
-
-	src_total_cnt = get_unaligned_le16(&pSMBs->t2_rsp.TotalDataCount);
-	tgt_total_cnt = get_unaligned_le16(&pSMBt->t2_rsp.TotalDataCount);
-
-	if (tgt_total_cnt != src_total_cnt)
-		cFYI(1, "total data count of primary and secondary t2 differ "
-			"source=%hu target=%hu", src_total_cnt, tgt_total_cnt);
-
-	total_in_tgt = get_unaligned_le16(&pSMBt->t2_rsp.DataCount);
-
-	remaining = tgt_total_cnt - total_in_tgt;
-
-	if (remaining < 0) {
-		cFYI(1, "Server sent too much data. tgt_total_cnt=%hu "
-			"total_in_tgt=%hu", tgt_total_cnt, total_in_tgt);
-		return -EPROTO;
-	}
-
-	if (remaining == 0) {
-		/* nothing to do, ignore */
-		cFYI(1, "no more data remains");
-		return 0;
-	}
-
-	total_in_src = get_unaligned_le16(&pSMBs->t2_rsp.DataCount);
-	if (remaining < total_in_src)
-		cFYI(1, "transact2 2nd response contains too much data");
-
-	/* find end of first SMB data area */
-	data_area_of_tgt = (char *)&pSMBt->hdr.Protocol +
-				get_unaligned_le16(&pSMBt->t2_rsp.DataOffset);
-
-	/* validate target area */
-	data_area_of_src = (char *)&pSMBs->hdr.Protocol +
-				get_unaligned_le16(&pSMBs->t2_rsp.DataOffset);
-
-	data_area_of_tgt += total_in_tgt;
-
-	total_in_tgt += total_in_src;
-	/* is the result too big for the field? */
-	if (total_in_tgt > USHRT_MAX) {
-		cFYI(1, "coalesced DataCount too large (%u)", total_in_tgt);
-		return -EPROTO;
-	}
-	put_unaligned_le16(total_in_tgt, &pSMBt->t2_rsp.DataCount);
-
-	/* fix up the BCC */
-	byte_count = get_bcc(target_hdr);
-	byte_count += total_in_src;
-	/* is the result too big for the field? */
-	if (byte_count > USHRT_MAX) {
-		cFYI(1, "coalesced BCC too large (%u)", byte_count);
-		return -EPROTO;
-	}
-	put_bcc(byte_count, target_hdr);
-
-	byte_count = be32_to_cpu(target_hdr->smb_buf_length);
-	byte_count += total_in_src;
-	/* don't allow buffer to overflow */
-	if (byte_count > CIFSMaxBufSize + MAX_CIFS_HDR_SIZE - 4) {
-		cFYI(1, "coalesced BCC exceeds buffer size (%u)", byte_count);
-		return -ENOBUFS;
-	}
-	target_hdr->smb_buf_length = cpu_to_be32(byte_count);
-
-	/* copy second buffer into end of first buffer */
-	memcpy(data_area_of_tgt, data_area_of_src, total_in_src);
-
-	if (remaining != total_in_src) {
-		/* more responses to go */
-		cFYI(1, "waiting for more secondary responses");
-		return 1;
-	}
-
-	/* we are done */
-	cFYI(1, "found the last secondary response");
-	return 0;
-}
-
 static void
 cifs_echo_request(struct work_struct *work)
 {
@@ -803,29 +666,9 @@ static void
 handle_mid(struct mid_q_entry *mid, struct TCP_Server_Info *server,
 	   char *buf, int malformed)
 {
-	if (malformed == 0 && check2ndT2(buf) > 0) {
-		mid->multiRsp = true;
-		if (mid->resp_buf) {
-			/* merge response - fix up 1st*/
-			malformed = coalesce_t2(buf, mid->resp_buf);
-			if (malformed > 0)
-				return;
-
-			/* All parts received or packet is malformed. */
-			mid->multiEnd = true;
-			return dequeue_mid(mid, malformed);
-		}
-		if (!server->large_buf) {
-			/*FIXME: switch to already allocated largebuf?*/
-			cERROR(1, "1st trans2 resp needs bigbuf");
-		} else {
-			/* Have first buffer */
-			mid->resp_buf = buf;
-			mid->large_buf = true;
-			server->bigbuf = NULL;
-		}
+	if (server->ops->check_trans2 &&
+	    server->ops->check_trans2(mid, server, buf, malformed))
 		return;
-	}
 	mid->resp_buf = buf;
 	mid->large_buf = server->large_buf;
 	/* Was previous buf put in mpx struct for multi-rsp? */
diff --git a/fs/cifs/smb1ops.c b/fs/cifs/smb1ops.c
index 6dec38f..28359e7 100644
--- a/fs/cifs/smb1ops.c
+++ b/fs/cifs/smb1ops.c
@@ -213,6 +213,175 @@ cifs_get_next_mid(struct TCP_Server_Info *server)
 	return mid;
 }
 
+/*
+	return codes:
+		0	not a transact2, or all data present
+		>0	transact2 with that much data missing
+		-EINVAL	invalid transact2
+ */
+static int
+check2ndT2(char *buf)
+{
+	struct smb_hdr *pSMB = (struct smb_hdr *)buf;
+	struct smb_t2_rsp *pSMBt;
+	int remaining;
+	__u16 total_data_size, data_in_this_rsp;
+
+	if (pSMB->Command != SMB_COM_TRANSACTION2)
+		return 0;
+
+	/* check for plausible wct, bcc and t2 data and parm sizes */
+	/* check for parm and data offset going beyond end of smb */
+	if (pSMB->WordCount != 10) { /* coalesce_t2 depends on this */
+		cFYI(1, "invalid transact2 word count");
+		return -EINVAL;
+	}
+
+	pSMBt = (struct smb_t2_rsp *)pSMB;
+
+	total_data_size = get_unaligned_le16(&pSMBt->t2_rsp.TotalDataCount);
+	data_in_this_rsp = get_unaligned_le16(&pSMBt->t2_rsp.DataCount);
+
+	if (total_data_size == data_in_this_rsp)
+		return 0;
+	else if (total_data_size < data_in_this_rsp) {
+		cFYI(1, "total data %d smaller than data in frame %d",
+			total_data_size, data_in_this_rsp);
+		return -EINVAL;
+	}
+
+	remaining = total_data_size - data_in_this_rsp;
+
+	cFYI(1, "missing %d bytes from transact2, check next response",
+		remaining);
+	if (total_data_size > CIFSMaxBufSize) {
+		cERROR(1, "TotalDataSize %d is over maximum buffer %d",
+			total_data_size, CIFSMaxBufSize);
+		return -EINVAL;
+	}
+	return remaining;
+}
+
+static int
+coalesce_t2(char *second_buf, struct smb_hdr *target_hdr)
+{
+	struct smb_t2_rsp *pSMBs = (struct smb_t2_rsp *)second_buf;
+	struct smb_t2_rsp *pSMBt  = (struct smb_t2_rsp *)target_hdr;
+	char *data_area_of_tgt;
+	char *data_area_of_src;
+	int remaining;
+	unsigned int byte_count, total_in_tgt;
+	__u16 tgt_total_cnt, src_total_cnt, total_in_src;
+
+	src_total_cnt = get_unaligned_le16(&pSMBs->t2_rsp.TotalDataCount);
+	tgt_total_cnt = get_unaligned_le16(&pSMBt->t2_rsp.TotalDataCount);
+
+	if (tgt_total_cnt != src_total_cnt)
+		cFYI(1, "total data count of primary and secondary t2 differ "
+			"source=%hu target=%hu", src_total_cnt, tgt_total_cnt);
+
+	total_in_tgt = get_unaligned_le16(&pSMBt->t2_rsp.DataCount);
+
+	remaining = tgt_total_cnt - total_in_tgt;
+
+	if (remaining < 0) {
+		cFYI(1, "Server sent too much data. tgt_total_cnt=%hu "
+			"total_in_tgt=%hu", tgt_total_cnt, total_in_tgt);
+		return -EPROTO;
+	}
+
+	if (remaining == 0) {
+		/* nothing to do, ignore */
+		cFYI(1, "no more data remains");
+		return 0;
+	}
+
+	total_in_src = get_unaligned_le16(&pSMBs->t2_rsp.DataCount);
+	if (remaining < total_in_src)
+		cFYI(1, "transact2 2nd response contains too much data");
+
+	/* find end of first SMB data area */
+	data_area_of_tgt = (char *)&pSMBt->hdr.Protocol +
+				get_unaligned_le16(&pSMBt->t2_rsp.DataOffset);
+
+	/* validate target area */
+	data_area_of_src = (char *)&pSMBs->hdr.Protocol +
+				get_unaligned_le16(&pSMBs->t2_rsp.DataOffset);
+
+	data_area_of_tgt += total_in_tgt;
+
+	total_in_tgt += total_in_src;
+	/* is the result too big for the field? */
+	if (total_in_tgt > USHRT_MAX) {
+		cFYI(1, "coalesced DataCount too large (%u)", total_in_tgt);
+		return -EPROTO;
+	}
+	put_unaligned_le16(total_in_tgt, &pSMBt->t2_rsp.DataCount);
+
+	/* fix up the BCC */
+	byte_count = get_bcc(target_hdr);
+	byte_count += total_in_src;
+	/* is the result too big for the field? */
+	if (byte_count > USHRT_MAX) {
+		cFYI(1, "coalesced BCC too large (%u)", byte_count);
+		return -EPROTO;
+	}
+	put_bcc(byte_count, target_hdr);
+
+	byte_count = be32_to_cpu(target_hdr->smb_buf_length);
+	byte_count += total_in_src;
+	/* don't allow buffer to overflow */
+	if (byte_count > CIFSMaxBufSize + MAX_CIFS_HDR_SIZE - 4) {
+		cFYI(1, "coalesced BCC exceeds buffer size (%u)", byte_count);
+		return -ENOBUFS;
+	}
+	target_hdr->smb_buf_length = cpu_to_be32(byte_count);
+
+	/* copy second buffer into end of first buffer */
+	memcpy(data_area_of_tgt, data_area_of_src, total_in_src);
+
+	if (remaining != total_in_src) {
+		/* more responses to go */
+		cFYI(1, "waiting for more secondary responses");
+		return 1;
+	}
+
+	/* we are done */
+	cFYI(1, "found the last secondary response");
+	return 0;
+}
+
+static bool
+cifs_check_trans2(struct mid_q_entry *mid, struct TCP_Server_Info *server,
+		  char *buf, int malformed)
+{
+	if (malformed)
+		return false;
+	if (check2ndT2(buf) <= 0)
+		return false;
+	mid->multiRsp = true;
+	if (mid->resp_buf) {
+		/* merge response - fix up 1st*/
+		malformed = coalesce_t2(buf, mid->resp_buf);
+		if (malformed > 0)
+			return true;
+		/* All parts received or packet is malformed. */
+		mid->multiEnd = true;
+		dequeue_mid(mid, malformed);
+		return true;
+	}
+	if (!server->large_buf) {
+		/*FIXME: switch to already allocated largebuf?*/
+		cERROR(1, "1st trans2 resp needs bigbuf");
+	} else {
+		/* Have first buffer */
+		mid->resp_buf = buf;
+		mid->large_buf = true;
+		server->bigbuf = NULL;
+	}
+	return true;
+}
+
 struct smb_version_operations smb1_operations = {
 	.send_cancel = send_nt_cancel,
 	.compare_fids = cifs_compare_fids,
@@ -229,6 +398,7 @@ struct smb_version_operations smb1_operations = {
 	.check_message = checkSMB,
 	.dump_detail = cifs_dump_detail,
 	.is_oplock_break = is_valid_oplock_break,
+	.check_trans2 = cifs_check_trans2,
 };
 
 struct smb_version_values smb1_values = {
-- 
1.7.1

[PATCH v5 2/5] CIFS: Extend credit mechanism to process request type

[Pavel Shilovsky]

Split all requests to echos, oplocks and others - each group uses
its own credit slot. This is indicated by new flags

CIFS_ECHO_OP and CIFS_OBREAK_OP

that are not used now for CIFS. This change is required to support
SMB2 protocol because of different processing of these commands.

Signed-off-by: Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
---
 fs/cifs/cifsglob.h  |   16 +++++++++++---
 fs/cifs/cifsproto.h |    2 +-
 fs/cifs/cifssmb.c   |   21 ++++++++++---------
 fs/cifs/smb1ops.c   |   12 +++++++++-
 fs/cifs/transport.c |   54 +++++++++++++++++++++++++++++---------------------
 5 files changed, 65 insertions(+), 40 deletions(-)

diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index 2aac4e5..844b77c 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -171,9 +171,11 @@ struct smb_version_operations {
 	/* check response: verify signature, map error */
 	int (*check_receive)(struct mid_q_entry *, struct TCP_Server_Info *,
 			     bool);
-	void (*add_credits)(struct TCP_Server_Info *, const unsigned int);
+	void (*add_credits)(struct TCP_Server_Info *, const unsigned int,
+			    const int);
 	void (*set_credits)(struct TCP_Server_Info *, const int);
-	int * (*get_credits_field)(struct TCP_Server_Info *);
+	int * (*get_credits_field)(struct TCP_Server_Info *, const int);
+	unsigned int (*get_credits)(struct mid_q_entry *);
 	__u64 (*get_next_mid)(struct TCP_Server_Info *);
 	/* data offset from read response message */
 	unsigned int (*read_data_offset)(char *);
@@ -392,9 +394,10 @@ has_credits(struct TCP_Server_Info *server, int *credits)
 }
 
 static inline void
-add_credits(struct TCP_Server_Info *server, const unsigned int add)
+add_credits(struct TCP_Server_Info *server, const unsigned int add,
+	    const int optype)
 {
-	server->ops->add_credits(server, add);
+	server->ops->add_credits(server, add, optype);
 }
 
 static inline void
@@ -957,6 +960,11 @@ static inline void free_dfs_info_array(struct dfs_info3_param *param,
 #define   CIFS_LARGE_BUF_OP 0x020    /* large request buffer */
 #define   CIFS_NO_RESP      0x040    /* no response buffer required */
 
+/* Type of request operation */
+#define   CIFS_ECHO_OP      0x080    /* echo request */
+#define   CIFS_OBREAK_OP   0x0100    /* oplock break request */
+#define   CIFS_OP_MASK     0x0180    /* mask request type */
+
 /* Security Flags: indicate type of session setup needed */
 #define   CIFSSEC_MAY_SIGN	0x00001
 #define   CIFSSEC_MAY_NTLM	0x00002
diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
index 0a6cbfe..8c1c7c1 100644
--- a/fs/cifs/cifsproto.h
+++ b/fs/cifs/cifsproto.h
@@ -71,7 +71,7 @@ extern void DeleteMidQEntry(struct mid_q_entry *midEntry);
 extern int cifs_call_async(struct TCP_Server_Info *server, struct kvec *iov,
 			   unsigned int nvec, mid_receive_t *receive,
 			   mid_callback_t *callback, void *cbdata,
-			   bool ignore_pend);
+			   const int flags);
 extern int SendReceive(const unsigned int /* xid */ , struct cifs_ses *,
 			struct smb_hdr * /* input */ ,
 			struct smb_hdr * /* out */ ,
diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
index 5b40073..58f1ab5 100644
--- a/fs/cifs/cifssmb.c
+++ b/fs/cifs/cifssmb.c
@@ -720,7 +720,7 @@ cifs_echo_callback(struct mid_q_entry *mid)
 	struct TCP_Server_Info *server = mid->callback_data;
 
 	DeleteMidQEntry(mid);
-	add_credits(server, 1);
+	add_credits(server, 1, CIFS_ECHO_OP);
 }
 
 int
@@ -747,7 +747,7 @@ CIFSSMBEcho(struct TCP_Server_Info *server)
 	iov.iov_len = be32_to_cpu(smb->hdr.smb_buf_length) + 4;
 
 	rc = cifs_call_async(server, &iov, 1, NULL, cifs_echo_callback,
-			     server, true);
+			     server, CIFS_ASYNC_OP | CIFS_ECHO_OP);
 	if (rc)
 		cFYI(1, "Echo request failed: %d", rc);
 
@@ -1563,7 +1563,7 @@ cifs_readv_callback(struct mid_q_entry *mid)
 
 	queue_work(cifsiod_wq, &rdata->work);
 	DeleteMidQEntry(mid);
-	add_credits(server, 1);
+	add_credits(server, 1, 0);
 }
 
 /* cifs_async_readv - send an async write, and set up mid to handle result */
@@ -1619,7 +1619,7 @@ cifs_async_readv(struct cifs_readdata *rdata)
 	kref_get(&rdata->refcount);
 	rc = cifs_call_async(tcon->ses->server, rdata->iov, 1,
 			     cifs_readv_receive, cifs_readv_callback,
-			     rdata, false);
+			     rdata, 0);
 
 	if (rc == 0)
 		cifs_stats_inc(&tcon->num_reads);
@@ -2010,7 +2010,7 @@ cifs_writev_callback(struct mid_q_entry *mid)
 
 	queue_work(cifsiod_wq, &wdata->work);
 	DeleteMidQEntry(mid);
-	add_credits(tcon->ses->server, 1);
+	add_credits(tcon->ses->server, 1, 0);
 }
 
 /* cifs_async_writev - send an async write, and set up mid to handle result */
@@ -2090,7 +2090,7 @@ cifs_async_writev(struct cifs_writedata *wdata)
 
 	kref_get(&wdata->refcount);
 	rc = cifs_call_async(tcon->ses->server, iov, wdata->nr_pages + 1,
-			     NULL, cifs_writev_callback, wdata, false);
+			     NULL, cifs_writev_callback, wdata, 0);
 
 	if (rc == 0)
 		cifs_stats_inc(&tcon->num_writes);
@@ -2268,7 +2268,7 @@ CIFSSMBLock(const int xid, struct cifs_tcon *tcon,
 	LOCK_REQ *pSMB = NULL;
 /*	LOCK_RSP *pSMBr = NULL; */ /* No response data other than rc to parse */
 	int bytes_returned;
-	int timeout = 0;
+	int flags = 0;
 	__u16 count;
 
 	cFYI(1, "CIFSSMBLock timeout %d numLock %d", (int)waitFlag, numLock);
@@ -2278,10 +2278,11 @@ CIFSSMBLock(const int xid, struct cifs_tcon *tcon,
 		return rc;
 
 	if (lockType == LOCKING_ANDX_OPLOCK_RELEASE) {
-		timeout = CIFS_ASYNC_OP; /* no response expected */
+		/* no response expected */
+		flags = CIFS_ASYNC_OP | CIFS_OBREAK_OP;
 		pSMB->Timeout = 0;
 	} else if (waitFlag) {
-		timeout = CIFS_BLOCKING_OP; /* blocking operation, no timeout */
+		flags = CIFS_BLOCKING_OP; /* blocking operation, no timeout */
 		pSMB->Timeout = cpu_to_le32(-1);/* blocking - do not time out */
 	} else {
 		pSMB->Timeout = 0;
@@ -2314,7 +2315,7 @@ CIFSSMBLock(const int xid, struct cifs_tcon *tcon,
 			(struct smb_hdr *) pSMB, &bytes_returned);
 		cifs_small_buf_release(pSMB);
 	} else {
-		rc = SendReceiveNoRsp(xid, tcon->ses, (char *)pSMB, timeout);
+		rc = SendReceiveNoRsp(xid, tcon->ses, (char *)pSMB, flags);
 		/* SMB buffer freed by function above */
 	}
 	cifs_stats_inc(&tcon->num_locks);
diff --git a/fs/cifs/smb1ops.c b/fs/cifs/smb1ops.c
index 28359e7..f4f8394 100644
--- a/fs/cifs/smb1ops.c
+++ b/fs/cifs/smb1ops.c
@@ -101,7 +101,8 @@ cifs_find_mid(struct TCP_Server_Info *server, char *buffer)
 }
 
 static void
-cifs_add_credits(struct TCP_Server_Info *server, const unsigned int add)
+cifs_add_credits(struct TCP_Server_Info *server, const unsigned int add,
+		 const int optype)
 {
 	spin_lock(&server->req_lock);
 	server->credits += add;
@@ -120,11 +121,17 @@ cifs_set_credits(struct TCP_Server_Info *server, const int val)
 }
 
 static int *
-cifs_get_credits_field(struct TCP_Server_Info *server)
+cifs_get_credits_field(struct TCP_Server_Info *server, const int optype)
 {
 	return &server->credits;
 }
 
+static unsigned int
+cifs_get_credits(struct mid_q_entry *mid)
+{
+	return 1;
+}
+
 /*
  * Find a free multiplex id (SMB mid). Otherwise there could be
  * mid collisions which might cause problems, demultiplexing the
@@ -390,6 +397,7 @@ struct smb_version_operations smb1_operations = {
 	.add_credits = cifs_add_credits,
 	.set_credits = cifs_set_credits,
 	.get_credits_field = cifs_get_credits_field,
+	.get_credits = cifs_get_credits,
 	.get_next_mid = cifs_get_next_mid,
 	.read_data_offset = cifs_read_data_offset,
 	.read_data_length = cifs_read_data_length,
diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c
index 3097ee5..54cd8dd 100644
--- a/fs/cifs/transport.c
+++ b/fs/cifs/transport.c
@@ -254,13 +254,13 @@ smb_send(struct TCP_Server_Info *server, struct smb_hdr *smb_buffer,
 }
 
 static int
-wait_for_free_credits(struct TCP_Server_Info *server, const int optype,
+wait_for_free_credits(struct TCP_Server_Info *server, const int long_op,
 		      int *credits)
 {
 	int rc;
 
 	spin_lock(&server->req_lock);
-	if (optype == CIFS_ASYNC_OP) {
+	if (long_op == CIFS_ASYNC_OP) {
 		/* oplock breaks must not be held up */
 		server->in_flight++;
 		*credits -= 1;
@@ -290,7 +290,7 @@ wait_for_free_credits(struct TCP_Server_Info *server, const int optype,
 			 */
 
 			/* update # of requests on the wire to server */
-			if (optype != CIFS_BLOCKING_OP) {
+			if (long_op != CIFS_BLOCKING_OP) {
 				*credits -= 1;
 				server->in_flight++;
 			}
@@ -302,10 +302,11 @@ wait_for_free_credits(struct TCP_Server_Info *server, const int optype,
 }
 
 static int
-wait_for_free_request(struct TCP_Server_Info *server, const int optype)
+wait_for_free_request(struct TCP_Server_Info *server, const int long_op,
+		      const int optype)
 {
-	return wait_for_free_credits(server, optype,
-				     server->ops->get_credits_field(server));
+	return wait_for_free_credits(server, long_op,
+				server->ops->get_credits_field(server, optype));
 }
 
 static int allocate_mid(struct cifs_ses *ses, struct smb_hdr *in_buf,
@@ -384,12 +385,15 @@ cifs_setup_async_request(struct TCP_Server_Info *server, struct kvec *iov,
 int
 cifs_call_async(struct TCP_Server_Info *server, struct kvec *iov,
 		unsigned int nvec, mid_receive_t *receive,
-		mid_callback_t *callback, void *cbdata, bool ignore_pend)
+		mid_callback_t *callback, void *cbdata, const int flags)
 {
-	int rc;
+	int rc, long_op, optype;
 	struct mid_q_entry *mid;
 
-	rc = wait_for_free_request(server, ignore_pend ? CIFS_ASYNC_OP : 0);
+	long_op = flags & CIFS_TIMEOUT_MASK;
+	optype = flags & CIFS_OP_MASK;
+
+	rc = wait_for_free_request(server, long_op, optype);
 	if (rc)
 		return rc;
 
@@ -397,7 +401,7 @@ cifs_call_async(struct TCP_Server_Info *server, struct kvec *iov,
 	rc = cifs_setup_async_request(server, iov, nvec, &mid);
 	if (rc) {
 		mutex_unlock(&server->srv_mutex);
-		add_credits(server, 1);
+		add_credits(server, 1, optype);
 		wake_up(&server->request_q);
 		return rc;
 	}
@@ -419,7 +423,7 @@ cifs_call_async(struct TCP_Server_Info *server, struct kvec *iov,
 	return rc;
 out_err:
 	delete_mid(mid);
-	add_credits(server, 1);
+	add_credits(server, 1, optype);
 	wake_up(&server->request_q);
 	return rc;
 }
@@ -539,11 +543,13 @@ SendReceive2(const unsigned int xid, struct cifs_ses *ses,
 	     const int flags)
 {
 	int rc = 0;
-	int long_op;
+	int long_op, optype;
 	struct mid_q_entry *midQ;
 	char *buf = iov[0].iov_base;
+	unsigned int credits = 1;
 
 	long_op = flags & CIFS_TIMEOUT_MASK;
+	optype = flags & CIFS_OP_MASK;
 
 	*pRespBufType = CIFS_NO_BUFFER;  /* no response buf yet */
 
@@ -564,7 +570,7 @@ SendReceive2(const unsigned int xid, struct cifs_ses *ses,
 	 * use ses->maxReq.
 	 */
 
-	rc = wait_for_free_request(ses->server, long_op);
+	rc = wait_for_free_request(ses->server, optype, long_op);
 	if (rc) {
 		cifs_small_buf_release(buf);
 		return rc;
@@ -583,7 +589,7 @@ SendReceive2(const unsigned int xid, struct cifs_ses *ses,
 		mutex_unlock(&ses->server->srv_mutex);
 		cifs_small_buf_release(buf);
 		/* Update # of requests on wire to server */
-		add_credits(ses->server, 1);
+		add_credits(ses->server, 1, optype);
 		return rc;
 	}
 
@@ -613,7 +619,7 @@ SendReceive2(const unsigned int xid, struct cifs_ses *ses,
 			midQ->callback = DeleteMidQEntry;
 			spin_unlock(&GlobalMid_Lock);
 			cifs_small_buf_release(buf);
-			add_credits(ses->server, 1);
+			add_credits(ses->server, 1, optype);
 			return rc;
 		}
 		spin_unlock(&GlobalMid_Lock);
@@ -623,7 +629,7 @@ SendReceive2(const unsigned int xid, struct cifs_ses *ses,
 
 	rc = cifs_sync_mid_result(midQ, ses->server);
 	if (rc != 0) {
-		add_credits(ses->server, 1);
+		add_credits(ses->server, 1, optype);
 		return rc;
 	}
 
@@ -641,6 +647,8 @@ SendReceive2(const unsigned int xid, struct cifs_ses *ses,
 	else
 		*pRespBufType = CIFS_SMALL_BUFFER;
 
+	credits = ses->server->ops->get_credits(midQ);
+
 	rc = ses->server->ops->check_receive(midQ, ses->server,
 					     flags & CIFS_LOG_ERROR);
 
@@ -649,7 +657,7 @@ SendReceive2(const unsigned int xid, struct cifs_ses *ses,
 		midQ->resp_buf = NULL;
 out:
 	delete_mid(midQ);
-	add_credits(ses->server, 1);
+	add_credits(ses->server, credits, optype);
 
 	return rc;
 }
@@ -685,7 +693,7 @@ SendReceive(const unsigned int xid, struct cifs_ses *ses,
 		return -EIO;
 	}
 
-	rc = wait_for_free_request(ses->server, long_op);
+	rc = wait_for_free_request(ses->server, 0, long_op);
 	if (rc)
 		return rc;
 
@@ -699,7 +707,7 @@ SendReceive(const unsigned int xid, struct cifs_ses *ses,
 	if (rc) {
 		mutex_unlock(&ses->server->srv_mutex);
 		/* Update # of requests on wire to server */
-		add_credits(ses->server, 1);
+		add_credits(ses->server, 1, 0);
 		return rc;
 	}
 
@@ -731,7 +739,7 @@ SendReceive(const unsigned int xid, struct cifs_ses *ses,
 			/* no longer considered to be "in-flight" */
 			midQ->callback = DeleteMidQEntry;
 			spin_unlock(&GlobalMid_Lock);
-			add_credits(ses->server, 1);
+			add_credits(ses->server, 1, 0);
 			return rc;
 		}
 		spin_unlock(&GlobalMid_Lock);
@@ -739,7 +747,7 @@ SendReceive(const unsigned int xid, struct cifs_ses *ses,
 
 	rc = cifs_sync_mid_result(midQ, ses->server);
 	if (rc != 0) {
-		add_credits(ses->server, 1);
+		add_credits(ses->server, 1, 0);
 		return rc;
 	}
 
@@ -755,7 +763,7 @@ SendReceive(const unsigned int xid, struct cifs_ses *ses,
 	rc = cifs_check_receive(midQ, ses->server, 0);
 out:
 	delete_mid(midQ);
-	add_credits(ses->server, 1);
+	add_credits(ses->server, 1, 0);
 
 	return rc;
 }
@@ -820,7 +828,7 @@ SendReceiveBlockingLock(const unsigned int xid, struct cifs_tcon *tcon,
 		return -EIO;
 	}
 
-	rc = wait_for_free_request(ses->server, CIFS_BLOCKING_OP);
+	rc = wait_for_free_request(ses->server, 0, CIFS_BLOCKING_OP);
 	if (rc)
 		return rc;
 
-- 
1.7.1

[PATCH v5 3/5] CIFS: Move protocol specific negotiate code to ops struct

[Pavel Shilovsky]

Signed-off-by: Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
---
 fs/cifs/cifsglob.h  |    8 ++++++--
 fs/cifs/cifsproto.h |    5 ++---
 fs/cifs/cifssmb.c   |    4 ++--
 fs/cifs/connect.c   |   21 +++++++++------------
 fs/cifs/sess.c      |    2 +-
 fs/cifs/smb1ops.c   |   23 +++++++++++++++++++++++
 6 files changed, 43 insertions(+), 20 deletions(-)

diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index 844b77c..669de88 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -192,6 +192,10 @@ struct smb_version_operations {
 	/* process transaction2 response */
 	bool (*check_trans2)(struct mid_q_entry *, struct TCP_Server_Info *,
 			     char *, int);
+	/* check if we need to negotiate */
+	bool (*need_neg)(struct TCP_Server_Info *);
+	/* negotiate to the server */
+	int (*negotiate)(const int, struct cifs_ses *);
 };
 
 struct smb_version_values {
@@ -324,7 +328,7 @@ struct TCP_Server_Info {
 	struct mutex srv_mutex;
 	struct task_struct *tsk;
 	char server_GUID[16];
-	char sec_mode;
+	__u16 sec_mode;
 	bool session_estab; /* mark when very first sess is established */
 	u16 dialect; /* dialect index that server chose */
 	enum securityEnum secType;
@@ -459,7 +463,7 @@ struct cifs_ses {
 	char *serverOS;		/* name of operating system underlying server */
 	char *serverNOS;	/* name of network operating system of server */
 	char *serverDomain;	/* security realm of server */
-	int Suid;		/* remote smb uid  */
+	__u64 Suid;		/* remote smb uid  */
 	uid_t linux_uid;        /* overriding owner of files on the mount */
 	uid_t cred_uid;		/* owner of credentials */
 	int capabilities;
diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
index 8c1c7c1..32b569f 100644
--- a/fs/cifs/cifsproto.h
+++ b/fs/cifs/cifsproto.h
@@ -178,11 +178,10 @@ extern void cifs_dfs_release_automount_timer(void);
 void cifs_proc_init(void);
 void cifs_proc_clean(void);
 
-extern int cifs_negotiate_protocol(unsigned int xid,
-				  struct cifs_ses *ses);
+extern int cifs_negotiate_protocol(const int xid, struct cifs_ses *ses);
 extern int cifs_setup_session(unsigned int xid, struct cifs_ses *ses,
 			struct nls_table *nls_info);
-extern int CIFSSMBNegotiate(unsigned int xid, struct cifs_ses *ses);
+extern int CIFSSMBNegotiate(const int xid, struct cifs_ses *ses);
 
 extern int CIFSTCon(unsigned int xid, struct cifs_ses *ses,
 			const char *tree, struct cifs_tcon *tcon,
diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
index 58f1ab5..fbc5628 100644
--- a/fs/cifs/cifssmb.c
+++ b/fs/cifs/cifssmb.c
@@ -372,7 +372,7 @@ static inline void inc_rfc1001_len(void *pSMB, int count)
 }
 
 int
-CIFSSMBNegotiate(unsigned int xid, struct cifs_ses *ses)
+CIFSSMBNegotiate(const int xid, struct cifs_ses *ses)
 {
 	NEGOTIATE_REQ *pSMB;
 	NEGOTIATE_RSP *pSMBr;
@@ -456,7 +456,7 @@ CIFSSMBNegotiate(unsigned int xid, struct cifs_ses *ses)
 			rc = -EOPNOTSUPP;
 			goto neg_err_exit;
 		}
-		server->sec_mode = (__u8)le16_to_cpu(rsp->SecurityMode);
+		server->sec_mode = le16_to_cpu(rsp->SecurityMode);
 		server->maxReq = min_t(unsigned int,
 				       le16_to_cpu(rsp->MaxMpxCount),
 				       cifs_max_pending);
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index ca0288d..4780342 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -406,7 +406,7 @@ cifs_echo_request(struct work_struct *work)
 	 * done, which is indicated by maxBuf != 0. Also, no need to ping if
 	 * we got a response recently
 	 */
-	if (server->maxBuf == 0 ||
+	if (!server->ops->need_neg || server->ops->need_neg(server) ||
 	    time_before(jiffies, server->lstrp + SMB_ECHO_INTERVAL - HZ))
 		goto requeue_echo;
 
@@ -3942,24 +3942,22 @@ cifs_umount(struct cifs_sb_info *cifs_sb)
 	kfree(cifs_sb);
 }
 
-int cifs_negotiate_protocol(unsigned int xid, struct cifs_ses *ses)
+int
+cifs_negotiate_protocol(const int xid, struct cifs_ses *ses)
 {
 	int rc = 0;
 	struct TCP_Server_Info *server = ses->server;
 
+	if (!server->ops->need_neg || !server->ops->negotiate)
+		return -ENOSYS;
+
 	/* only send once per connect */
-	if (server->maxBuf != 0)
+	if (!server->ops->need_neg(server))
 		return 0;
 
 	set_credits(server, 1);
-	rc = CIFSSMBNegotiate(xid, ses);
-	if (rc == -EAGAIN) {
-		/* retry only once on 1st time connection */
-		set_credits(server, 1);
-		rc = CIFSSMBNegotiate(xid, ses);
-		if (rc == -EAGAIN)
-			rc = -EHOSTDOWN;
-	}
+
+	rc = server->ops->negotiate(xid, ses);
 	if (rc == 0) {
 		spin_lock(&GlobalMid_Lock);
 		if (server->tcpStatus == CifsNeedNegotiate)
@@ -3967,7 +3965,6 @@ int cifs_negotiate_protocol(unsigned int xid, struct cifs_ses *ses)
 		else
 			rc = -EHOSTDOWN;
 		spin_unlock(&GlobalMid_Lock);
-
 	}
 
 	return rc;
diff --git a/fs/cifs/sess.c b/fs/cifs/sess.c
index 551d0c2..f88fa4d 100644
--- a/fs/cifs/sess.c
+++ b/fs/cifs/sess.c
@@ -898,7 +898,7 @@ ssetup_ntlmssp_authenticate:
 	if (action & GUEST_LOGIN)
 		cFYI(1, "Guest login"); /* BB mark SesInfo struct? */
 	ses->Suid = smb_buf->Uid;   /* UID left in wire format (le) */
-	cFYI(1, "UID = %d ", ses->Suid);
+	cFYI(1, "UID = %llu ", ses->Suid);
 	/* response can have either 3 or 4 word count - Samba sends 3 */
 	/* and lanman response is 3 */
 	bytes_remaining = get_bcc(smb_buf);
diff --git a/fs/cifs/smb1ops.c b/fs/cifs/smb1ops.c
index f4f8394..5bfc26a 100644
--- a/fs/cifs/smb1ops.c
+++ b/fs/cifs/smb1ops.c
@@ -389,6 +389,27 @@ cifs_check_trans2(struct mid_q_entry *mid, struct TCP_Server_Info *server,
 	return true;
 }
 
+static bool
+cifs_need_neg(struct TCP_Server_Info *server)
+{
+	return server->maxBuf == 0;
+}
+
+static int
+cifs_negotiate(const int xid, struct cifs_ses *ses)
+{
+	int rc;
+	rc = CIFSSMBNegotiate(xid, ses);
+	if (rc == -EAGAIN) {
+		/* retry only once on 1st time connection */
+		set_credits(ses->server, 1);
+		rc = CIFSSMBNegotiate(xid, ses);
+		if (rc == -EAGAIN)
+			rc = -EHOSTDOWN;
+	}
+	return rc;
+}
+
 struct smb_version_operations smb1_operations = {
 	.send_cancel = send_nt_cancel,
 	.compare_fids = cifs_compare_fids,
@@ -407,6 +428,8 @@ struct smb_version_operations smb1_operations = {
 	.dump_detail = cifs_dump_detail,
 	.is_oplock_break = is_valid_oplock_break,
 	.check_trans2 = cifs_check_trans2,
+	.need_neg = cifs_need_neg,
+	.negotiate = cifs_negotiate,
 };
 
 struct smb_version_values smb1_values = {
-- 
1.7.1

[PATCH v5 4/5] CIFS: Move protocol specific session setup/logoff code to ops struct

[Pavel Shilovsky]

Signed-off-by: Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
---
 fs/cifs/cifsglob.h  |    5 +++++
 fs/cifs/cifsproto.h |    8 ++++----
 fs/cifs/connect.c   |   16 +++++++++-------
 fs/cifs/sess.c      |    2 +-
 fs/cifs/smb1ops.c   |    2 ++
 5 files changed, 21 insertions(+), 12 deletions(-)

diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index 669de88..812f27c 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -196,6 +196,11 @@ struct smb_version_operations {
 	bool (*need_neg)(struct TCP_Server_Info *);
 	/* negotiate to the server */
 	int (*negotiate)(const int, struct cifs_ses *);
+	/* setup smb sessionn */
+	int (*sess_setup)(const int, struct cifs_ses *,
+			  const struct nls_table *);
+	/* close smb session */
+	int (*logoff)(const int, struct cifs_ses *);
 };
 
 struct smb_version_values {
diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
index 32b569f..8e6cd38 100644
--- a/fs/cifs/cifsproto.h
+++ b/fs/cifs/cifsproto.h
@@ -112,8 +112,8 @@ extern void header_assemble(struct smb_hdr *, char /* command */ ,
 extern int small_smb_init_no_tc(const int smb_cmd, const int wct,
 				struct cifs_ses *ses,
 				void **request_buf);
-extern int CIFS_SessSetup(unsigned int xid, struct cifs_ses *ses,
-			     const struct nls_table *nls_cp);
+extern int CIFS_SessSetup(const int xid, struct cifs_ses *ses,
+			  const struct nls_table *nls_cp);
 extern struct timespec cifs_NTtimeToUnix(__le64 utc_nanoseconds_since_1601);
 extern u64 cifs_UnixTimeToNT(struct timespec);
 extern struct timespec cnvrtDosUnixTm(__le16 le_date, __le16 le_time,
@@ -179,8 +179,8 @@ void cifs_proc_init(void);
 void cifs_proc_clean(void);
 
 extern int cifs_negotiate_protocol(const int xid, struct cifs_ses *ses);
-extern int cifs_setup_session(unsigned int xid, struct cifs_ses *ses,
-			struct nls_table *nls_info);
+extern int cifs_setup_session(const int xid, struct cifs_ses *ses,
+			      struct nls_table *nls_info);
 extern int CIFSSMBNegotiate(const int xid, struct cifs_ses *ses);
 
 extern int CIFSTCon(unsigned int xid, struct cifs_ses *ses,
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 4780342..9679352 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -2263,9 +2263,9 @@ cifs_put_smb_ses(struct cifs_ses *ses)
 	list_del_init(&ses->smb_ses_list);
 	spin_unlock(&cifs_tcp_ses_lock);
 
-	if (ses->status == CifsGood) {
+	if (ses->status == CifsGood && server->ops->logoff) {
 		xid = GetXid();
-		CIFSSMBLogoff(xid, ses);
+		server->ops->logoff(xid, ses);
 		_FreeXid(xid);
 	}
 	sesInfoFree(ses);
@@ -3970,11 +3970,11 @@ cifs_negotiate_protocol(const int xid, struct cifs_ses *ses)
 	return rc;
 }
 
-
-int cifs_setup_session(unsigned int xid, struct cifs_ses *ses,
-			struct nls_table *nls_info)
+int
+cifs_setup_session(const int xid, struct cifs_ses *ses,
+		   struct nls_table *nls_info)
 {
-	int rc = 0;
+	int rc = -ENOSYS;
 	struct TCP_Server_Info *server = ses->server;
 
 	ses->flags = 0;
@@ -3985,7 +3985,9 @@ int cifs_setup_session(unsigned int xid, struct cifs_ses *ses,
 	cFYI(1, "Security Mode: 0x%x Capabilities: 0x%x TimeAdjust: %d",
 		 server->sec_mode, server->capabilities, server->timeAdj);
 
-	rc = CIFS_SessSetup(xid, ses, nls_info);
+	if (server->ops->sess_setup)
+		rc = server->ops->sess_setup(xid, ses, nls_info);
+
 	if (rc) {
 		cERROR(1, "Send error in SessSetup = %d", rc);
 	} else {
diff --git a/fs/cifs/sess.c b/fs/cifs/sess.c
index f88fa4d..16b2083 100644
--- a/fs/cifs/sess.c
+++ b/fs/cifs/sess.c
@@ -556,7 +556,7 @@ setup_ntlmv2_ret:
 }
 
 int
-CIFS_SessSetup(unsigned int xid, struct cifs_ses *ses,
+CIFS_SessSetup(const int xid, struct cifs_ses *ses,
 	       const struct nls_table *nls_cp)
 {
 	int rc = 0;
diff --git a/fs/cifs/smb1ops.c b/fs/cifs/smb1ops.c
index 5bfc26a..ba3071f 100644
--- a/fs/cifs/smb1ops.c
+++ b/fs/cifs/smb1ops.c
@@ -430,6 +430,8 @@ struct smb_version_operations smb1_operations = {
 	.check_trans2 = cifs_check_trans2,
 	.need_neg = cifs_need_neg,
 	.negotiate = cifs_negotiate,
+	.sess_setup = CIFS_SessSetup,
+	.logoff = CIFSSMBLogoff,
 };
 
 struct smb_version_values smb1_values = {
-- 
1.7.1

[PATCH v5 5/5] CIFS: Move protocol specific tcon/tdis code to ops struct

[Pavel Shilovsky]

and rename variables around the code changes.

Signed-off-by: Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
---
 fs/cifs/cifsglob.h  |    6 ++++
 fs/cifs/cifsproto.h |    5 +--
 fs/cifs/connect.c   |   71 ++++++++++++++++++++++++++++++---------------------
 fs/cifs/smb1ops.c   |    2 +
 4 files changed, 52 insertions(+), 32 deletions(-)

diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index 812f27c..1266e2e 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -160,6 +160,7 @@ struct mid_q_entry;
 struct TCP_Server_Info;
 struct cifsFileInfo;
 struct cifs_ses;
+struct cifs_tcon;
 
 struct smb_version_operations {
 	int (*send_cancel)(struct TCP_Server_Info *, void *,
@@ -201,6 +202,11 @@ struct smb_version_operations {
 			  const struct nls_table *);
 	/* close smb session */
 	int (*logoff)(const int, struct cifs_ses *);
+	/* connect to a server share */
+	int (*tree_connect)(const int, struct cifs_ses *, const char *,
+			    struct cifs_tcon *, const struct nls_table *);
+	/* close tree connecion */
+	int (*tree_disconnect)(const int, struct cifs_tcon *);
 };
 
 struct smb_version_values {
diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
index 8e6cd38..2f4f661 100644
--- a/fs/cifs/cifsproto.h
+++ b/fs/cifs/cifsproto.h
@@ -183,9 +183,8 @@ extern int cifs_setup_session(const int xid, struct cifs_ses *ses,
 			      struct nls_table *nls_info);
 extern int CIFSSMBNegotiate(const int xid, struct cifs_ses *ses);
 
-extern int CIFSTCon(unsigned int xid, struct cifs_ses *ses,
-			const char *tree, struct cifs_tcon *tcon,
-			const struct nls_table *);
+extern int CIFSTCon(const int xid, struct cifs_ses *ses, const char *tree,
+		    struct cifs_tcon *tcon, const struct nls_table *);
 
 extern int CIFSFindFirst(const int xid, struct cifs_tcon *tcon,
 		const char *searchName, const struct nls_table *nls_codepage,
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 9679352..d14bd45 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -2552,7 +2552,8 @@ cifs_put_tcon(struct cifs_tcon *tcon)
 	spin_unlock(&cifs_tcp_ses_lock);
 
 	xid = GetXid();
-	CIFSSMBTDis(xid, tcon);
+	if (ses->server->ops->tree_disconnect)
+		ses->server->ops->tree_disconnect(xid, tcon);
 	_FreeXid(xid);
 
 	cifs_fscache_release_super_cookie(tcon);
@@ -2577,6 +2578,11 @@ cifs_get_tcon(struct cifs_ses *ses, struct smb_vol *volume_info)
 		return tcon;
 	}
 
+	if (!ses->server->ops->tree_connect) {
+		rc = -ENOSYS;
+		goto out_fail;
+	}
+
 	tcon = tconInfoAlloc();
 	if (tcon == NULL) {
 		rc = -ENOMEM;
@@ -2599,13 +2605,15 @@ cifs_get_tcon(struct cifs_ses *ses, struct smb_vol *volume_info)
 		goto out_fail;
 	}
 
-	/* BB Do we need to wrap session_mutex around
-	 * this TCon call and Unix SetFS as
-	 * we do on SessSetup and reconnect? */
+	/*
+	 * BB Do we need to wrap session_mutex around this TCon call and Unix
+	 * SetFS as we do on SessSetup and reconnect?
+	 */
 	xid = GetXid();
-	rc = CIFSTCon(xid, ses, volume_info->UNC, tcon, volume_info->local_nls);
+	rc = ses->server->ops->tree_connect(xid, ses, volume_info->UNC, tcon,
+					    volume_info->local_nls);
 	FreeXid(xid);
-	cFYI(1, "CIFS Tcon rc = %d", rc);
+	cFYI(1, "Tcon rc = %d", rc);
 	if (rc)
 		goto out_fail;
 
@@ -2614,10 +2622,11 @@ cifs_get_tcon(struct cifs_ses *ses, struct smb_vol *volume_info)
 		cFYI(1, "DFS disabled (%d)", tcon->Flags);
 	}
 	tcon->seal = volume_info->seal;
-	/* we can have only one retry value for a connection
-	   to a share so for resources mounted more than once
-	   to the same server share the last value passed in
-	   for the retry flag is used */
+	/*
+	 * We can have only one retry value for a connection to a share so for
+	 * resources mounted more than once to the same server share the last
+	 * value passed in for the retry flag is used.
+	 */
 	tcon->retry = volume_info->retry;
 	tcon->nocase = volume_info->nocase;
 	tcon->local_lease = volume_info->local_lease;
@@ -2751,37 +2760,41 @@ out:
 }
 
 int
-get_dfs_path(int xid, struct cifs_ses *pSesInfo, const char *old_path,
-	     const struct nls_table *nls_codepage, unsigned int *pnum_referrals,
-	     struct dfs_info3_param **preferrals, int remap)
+get_dfs_path(int xid, struct cifs_ses *ses, const char *old_path,
+	     const struct nls_table *nls_codepage, unsigned int *num_referrals,
+	     struct dfs_info3_param **referrals, int remap)
 {
 	char *temp_unc;
 	int rc = 0;
 
-	*pnum_referrals = 0;
-	*preferrals = NULL;
+	if (!ses->server->ops->tree_connect)
+		return -ENOSYS;
+
+	*num_referrals = 0;
+	*referrals = NULL;
 
-	if (pSesInfo->ipc_tid == 0) {
+	if (ses->ipc_tid == 0) {
 		temp_unc = kmalloc(2 /* for slashes */ +
-			strnlen(pSesInfo->serverName,
-				SERVER_NAME_LEN_WITH_NULL * 2)
-				 + 1 + 4 /* slash IPC$ */  + 2,
-				GFP_KERNEL);
+			strnlen(ses->serverName, SERVER_NAME_LEN_WITH_NULL * 2)
+				+ 1 + 4 /* slash IPC$ */ + 2, GFP_KERNEL);
 		if (temp_unc == NULL)
 			return -ENOMEM;
 		temp_unc[0] = '\\';
 		temp_unc[1] = '\\';
-		strcpy(temp_unc + 2, pSesInfo->serverName);
-		strcpy(temp_unc + 2 + strlen(pSesInfo->serverName), "\\IPC$");
-		rc = CIFSTCon(xid, pSesInfo, temp_unc, NULL, nls_codepage);
-		cFYI(1, "CIFS Tcon rc = %d ipc_tid = %d", rc, pSesInfo->ipc_tid);
+		strcpy(temp_unc + 2, ses->serverName);
+		strcpy(temp_unc + 2 + strlen(ses->serverName), "\\IPC$");
+		rc = ses->server->ops->tree_connect(xid, ses, temp_unc, NULL,
+						    nls_codepage);
+		cFYI(1, "Tcon rc = %d ipc_tid = %d", rc, ses->ipc_tid);
 		kfree(temp_unc);
 	}
 	if (rc == 0)
-		rc = CIFSGetDFSRefer(xid, pSesInfo, old_path, preferrals,
-				     pnum_referrals, nls_codepage, remap);
-	/* BB map targetUNCs to dfs_info3 structures, here or
-		in CIFSGetDFSRefer BB */
+		rc = CIFSGetDFSRefer(xid, ses, old_path, referrals,
+				     num_referrals, nls_codepage, remap);
+	/*
+	 * BB - map targetUNCs to dfs_info3 structures, here or in
+	 * CIFSGetDFSRefer.
+	 */
 
 	return rc;
 }
@@ -3758,7 +3771,7 @@ out:
  * pointer may be NULL.
  */
 int
-CIFSTCon(unsigned int xid, struct cifs_ses *ses,
+CIFSTCon(const int xid, struct cifs_ses *ses,
 	 const char *tree, struct cifs_tcon *tcon,
 	 const struct nls_table *nls_codepage)
 {
diff --git a/fs/cifs/smb1ops.c b/fs/cifs/smb1ops.c
index ba3071f..1a3f08c 100644
--- a/fs/cifs/smb1ops.c
+++ b/fs/cifs/smb1ops.c
@@ -432,6 +432,8 @@ struct smb_version_operations smb1_operations = {
 	.negotiate = cifs_negotiate,
 	.sess_setup = CIFS_SessSetup,
 	.logoff = CIFSSMBLogoff,
+	.tree_connect = CIFSTCon,
+	.tree_disconnect = CIFSSMBTDis,
 };
 
 struct smb_version_values smb1_values = {
-- 
1.7.1

Re: [PATCH v5 2/5] CIFS: Extend credit mechanism to process request type

[Jeff Layton]

On Sat,  9 Jun 2012 10:26:01 +0400
Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org> wrote:

> Split all requests to echos, oplocks and others - each group uses
> its own credit slot. This is indicated by new flags
> 
> CIFS_ECHO_OP and CIFS_OBREAK_OP
> 
> that are not used now for CIFS. This change is required to support
> SMB2 protocol because of different processing of these commands.
> 
> Signed-off-by: Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
> ---
>  fs/cifs/cifsglob.h  |   16 +++++++++++---
>  fs/cifs/cifsproto.h |    2 +-
>  fs/cifs/cifssmb.c   |   21 ++++++++++---------
>  fs/cifs/smb1ops.c   |   12 +++++++++-
>  fs/cifs/transport.c |   54 +++++++++++++++++++++++++++++---------------------
>  5 files changed, 65 insertions(+), 40 deletions(-)
> 
> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> index 2aac4e5..844b77c 100644
> --- a/fs/cifs/cifsglob.h
> +++ b/fs/cifs/cifsglob.h
> @@ -171,9 +171,11 @@ struct smb_version_operations {
>  	/* check response: verify signature, map error */
>  	int (*check_receive)(struct mid_q_entry *, struct TCP_Server_Info *,
>  			     bool);
> -	void (*add_credits)(struct TCP_Server_Info *, const unsigned int);
> +	void (*add_credits)(struct TCP_Server_Info *, const unsigned int,
> +			    const int);
>  	void (*set_credits)(struct TCP_Server_Info *, const int);
> -	int * (*get_credits_field)(struct TCP_Server_Info *);
> +	int * (*get_credits_field)(struct TCP_Server_Info *, const int);
> +	unsigned int (*get_credits)(struct mid_q_entry *);
>  	__u64 (*get_next_mid)(struct TCP_Server_Info *);
>  	/* data offset from read response message */
>  	unsigned int (*read_data_offset)(char *);
> @@ -392,9 +394,10 @@ has_credits(struct TCP_Server_Info *server, int *credits)
>  }
>  
>  static inline void
> -add_credits(struct TCP_Server_Info *server, const unsigned int add)
> +add_credits(struct TCP_Server_Info *server, const unsigned int add,
> +	    const int optype)
>  {
> -	server->ops->add_credits(server, add);
> +	server->ops->add_credits(server, add, optype);
>  }
>  
>  static inline void
> @@ -957,6 +960,11 @@ static inline void free_dfs_info_array(struct dfs_info3_param *param,
>  #define   CIFS_LARGE_BUF_OP 0x020    /* large request buffer */
>  #define   CIFS_NO_RESP      0x040    /* no response buffer required */
>  
> +/* Type of request operation */
> +#define   CIFS_ECHO_OP      0x080    /* echo request */
> +#define   CIFS_OBREAK_OP   0x0100    /* oplock break request */
> +#define   CIFS_OP_MASK     0x0180    /* mask request type */
> +
>  /* Security Flags: indicate type of session setup needed */
>  #define   CIFSSEC_MAY_SIGN	0x00001
>  #define   CIFSSEC_MAY_NTLM	0x00002
> diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
> index 0a6cbfe..8c1c7c1 100644
> --- a/fs/cifs/cifsproto.h
> +++ b/fs/cifs/cifsproto.h
> @@ -71,7 +71,7 @@ extern void DeleteMidQEntry(struct mid_q_entry *midEntry);
>  extern int cifs_call_async(struct TCP_Server_Info *server, struct kvec *iov,
>  			   unsigned int nvec, mid_receive_t *receive,
>  			   mid_callback_t *callback, void *cbdata,
> -			   bool ignore_pend);
> +			   const int flags);
>  extern int SendReceive(const unsigned int /* xid */ , struct cifs_ses *,
>  			struct smb_hdr * /* input */ ,
>  			struct smb_hdr * /* out */ ,
> diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
> index 5b40073..58f1ab5 100644
> --- a/fs/cifs/cifssmb.c
> +++ b/fs/cifs/cifssmb.c
> @@ -720,7 +720,7 @@ cifs_echo_callback(struct mid_q_entry *mid)
>  	struct TCP_Server_Info *server = mid->callback_data;
>  
>  	DeleteMidQEntry(mid);
> -	add_credits(server, 1);
> +	add_credits(server, 1, CIFS_ECHO_OP);
>  }
>  
>  int
> @@ -747,7 +747,7 @@ CIFSSMBEcho(struct TCP_Server_Info *server)
>  	iov.iov_len = be32_to_cpu(smb->hdr.smb_buf_length) + 4;
>  
>  	rc = cifs_call_async(server, &iov, 1, NULL, cifs_echo_callback,
> -			     server, true);
> +			     server, CIFS_ASYNC_OP | CIFS_ECHO_OP);
>  	if (rc)
>  		cFYI(1, "Echo request failed: %d", rc);
>  
> @@ -1563,7 +1563,7 @@ cifs_readv_callback(struct mid_q_entry *mid)
>  
>  	queue_work(cifsiod_wq, &rdata->work);
>  	DeleteMidQEntry(mid);
> -	add_credits(server, 1);
> +	add_credits(server, 1, 0);
>  }
>  
>  /* cifs_async_readv - send an async write, and set up mid to handle result */
> @@ -1619,7 +1619,7 @@ cifs_async_readv(struct cifs_readdata *rdata)
>  	kref_get(&rdata->refcount);
>  	rc = cifs_call_async(tcon->ses->server, rdata->iov, 1,
>  			     cifs_readv_receive, cifs_readv_callback,
> -			     rdata, false);
> +			     rdata, 0);
>  
>  	if (rc == 0)
>  		cifs_stats_inc(&tcon->num_reads);
> @@ -2010,7 +2010,7 @@ cifs_writev_callback(struct mid_q_entry *mid)
>  
>  	queue_work(cifsiod_wq, &wdata->work);
>  	DeleteMidQEntry(mid);
> -	add_credits(tcon->ses->server, 1);
> +	add_credits(tcon->ses->server, 1, 0);
>  }
>  
>  /* cifs_async_writev - send an async write, and set up mid to handle result */
> @@ -2090,7 +2090,7 @@ cifs_async_writev(struct cifs_writedata *wdata)
>  
>  	kref_get(&wdata->refcount);
>  	rc = cifs_call_async(tcon->ses->server, iov, wdata->nr_pages + 1,
> -			     NULL, cifs_writev_callback, wdata, false);
> +			     NULL, cifs_writev_callback, wdata, 0);
>  
>  	if (rc == 0)
>  		cifs_stats_inc(&tcon->num_writes);
> @@ -2268,7 +2268,7 @@ CIFSSMBLock(const int xid, struct cifs_tcon *tcon,
>  	LOCK_REQ *pSMB = NULL;
>  /*	LOCK_RSP *pSMBr = NULL; */ /* No response data other than rc to parse */
>  	int bytes_returned;
> -	int timeout = 0;
> +	int flags = 0;
>  	__u16 count;
>  
>  	cFYI(1, "CIFSSMBLock timeout %d numLock %d", (int)waitFlag, numLock);
> @@ -2278,10 +2278,11 @@ CIFSSMBLock(const int xid, struct cifs_tcon *tcon,
>  		return rc;
>  
>  	if (lockType == LOCKING_ANDX_OPLOCK_RELEASE) {
> -		timeout = CIFS_ASYNC_OP; /* no response expected */
> +		/* no response expected */
> +		flags = CIFS_ASYNC_OP | CIFS_OBREAK_OP;
>  		pSMB->Timeout = 0;
>  	} else if (waitFlag) {
> -		timeout = CIFS_BLOCKING_OP; /* blocking operation, no timeout */
> +		flags = CIFS_BLOCKING_OP; /* blocking operation, no timeout */
>  		pSMB->Timeout = cpu_to_le32(-1);/* blocking - do not time out */
>  	} else {
>  		pSMB->Timeout = 0;
> @@ -2314,7 +2315,7 @@ CIFSSMBLock(const int xid, struct cifs_tcon *tcon,
>  			(struct smb_hdr *) pSMB, &bytes_returned);
>  		cifs_small_buf_release(pSMB);
>  	} else {
> -		rc = SendReceiveNoRsp(xid, tcon->ses, (char *)pSMB, timeout);
> +		rc = SendReceiveNoRsp(xid, tcon->ses, (char *)pSMB, flags);
>  		/* SMB buffer freed by function above */
>  	}
>  	cifs_stats_inc(&tcon->num_locks);
> diff --git a/fs/cifs/smb1ops.c b/fs/cifs/smb1ops.c
> index 28359e7..f4f8394 100644
> --- a/fs/cifs/smb1ops.c
> +++ b/fs/cifs/smb1ops.c
> @@ -101,7 +101,8 @@ cifs_find_mid(struct TCP_Server_Info *server, char *buffer)
>  }
>  
>  static void
> -cifs_add_credits(struct TCP_Server_Info *server, const unsigned int add)
> +cifs_add_credits(struct TCP_Server_Info *server, const unsigned int add,
> +		 const int optype)
>  {
>  	spin_lock(&server->req_lock);
>  	server->credits += add;
> @@ -120,11 +121,17 @@ cifs_set_credits(struct TCP_Server_Info *server, const int val)
>  }
>  
>  static int *
> -cifs_get_credits_field(struct TCP_Server_Info *server)
> +cifs_get_credits_field(struct TCP_Server_Info *server, const int optype)
>  {
>  	return &server->credits;
>  }
>  
> +static unsigned int
> +cifs_get_credits(struct mid_q_entry *mid)
> +{
> +	return 1;
> +}
> +
>  /*
>   * Find a free multiplex id (SMB mid). Otherwise there could be
>   * mid collisions which might cause problems, demultiplexing the
> @@ -390,6 +397,7 @@ struct smb_version_operations smb1_operations = {
>  	.add_credits = cifs_add_credits,
>  	.set_credits = cifs_set_credits,
>  	.get_credits_field = cifs_get_credits_field,
> +	.get_credits = cifs_get_credits,
>  	.get_next_mid = cifs_get_next_mid,
>  	.read_data_offset = cifs_read_data_offset,
>  	.read_data_length = cifs_read_data_length,
> diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c
> index 3097ee5..54cd8dd 100644
> --- a/fs/cifs/transport.c
> +++ b/fs/cifs/transport.c
> @@ -254,13 +254,13 @@ smb_send(struct TCP_Server_Info *server, struct smb_hdr *smb_buffer,
>  }
>  
>  static int
> -wait_for_free_credits(struct TCP_Server_Info *server, const int optype,
> +wait_for_free_credits(struct TCP_Server_Info *server, const int long_op,
>  		      int *credits)
>  {
>  	int rc;
>  
>  	spin_lock(&server->req_lock);
> -	if (optype == CIFS_ASYNC_OP) {
> +	if (long_op == CIFS_ASYNC_OP) {
>  		/* oplock breaks must not be held up */
>  		server->in_flight++;
>  		*credits -= 1;
> @@ -290,7 +290,7 @@ wait_for_free_credits(struct TCP_Server_Info *server, const int optype,
>  			 */
>  
>  			/* update # of requests on the wire to server */
> -			if (optype != CIFS_BLOCKING_OP) {
> +			if (long_op != CIFS_BLOCKING_OP) {
>  				*credits -= 1;
>  				server->in_flight++;
>  			}

Why rename the variable in this above function? "optype" seems like a better name...

> @@ -302,10 +302,11 @@ wait_for_free_credits(struct TCP_Server_Info *server, const int optype,
>  }
>  
>  static int
> -wait_for_free_request(struct TCP_Server_Info *server, const int optype)
> +wait_for_free_request(struct TCP_Server_Info *server, const int long_op,
> +		      const int optype)
>  {
> -	return wait_for_free_credits(server, optype,
> -				     server->ops->get_credits_field(server));
> +	return wait_for_free_credits(server, long_op,
> +				server->ops->get_credits_field(server, optype));
>  }
>  
>  static int allocate_mid(struct cifs_ses *ses, struct smb_hdr *in_buf,
> @@ -384,12 +385,15 @@ cifs_setup_async_request(struct TCP_Server_Info *server, struct kvec *iov,
>  int
>  cifs_call_async(struct TCP_Server_Info *server, struct kvec *iov,
>  		unsigned int nvec, mid_receive_t *receive,
> -		mid_callback_t *callback, void *cbdata, bool ignore_pend)
> +		mid_callback_t *callback, void *cbdata, const int flags)
>  {
> -	int rc;
> +	int rc, long_op, optype;
>  	struct mid_q_entry *mid;
>  
> -	rc = wait_for_free_request(server, ignore_pend ? CIFS_ASYNC_OP : 0);
> +	long_op = flags & CIFS_TIMEOUT_MASK;
> +	optype = flags & CIFS_OP_MASK;
> +
> +	rc = wait_for_free_request(server, long_op, optype);
>  	if (rc)
>  		return rc;
>  
> @@ -397,7 +401,7 @@ cifs_call_async(struct TCP_Server_Info *server, struct kvec *iov,
>  	rc = cifs_setup_async_request(server, iov, nvec, &mid);
>  	if (rc) {
>  		mutex_unlock(&server->srv_mutex);
> -		add_credits(server, 1);
> +		add_credits(server, 1, optype);
>  		wake_up(&server->request_q);
>  		return rc;
>  	}
> @@ -419,7 +423,7 @@ cifs_call_async(struct TCP_Server_Info *server, struct kvec *iov,
>  	return rc;
>  out_err:
>  	delete_mid(mid);
> -	add_credits(server, 1);
> +	add_credits(server, 1, optype);
>  	wake_up(&server->request_q);
>  	return rc;
>  }
> @@ -539,11 +543,13 @@ SendReceive2(const unsigned int xid, struct cifs_ses *ses,
>  	     const int flags)
>  {
>  	int rc = 0;
> -	int long_op;
> +	int long_op, optype;
>  	struct mid_q_entry *midQ;
>  	char *buf = iov[0].iov_base;
> +	unsigned int credits = 1;
>  
>  	long_op = flags & CIFS_TIMEOUT_MASK;
> +	optype = flags & CIFS_OP_MASK;
>  
>  	*pRespBufType = CIFS_NO_BUFFER;  /* no response buf yet */
>  
> @@ -564,7 +570,7 @@ SendReceive2(const unsigned int xid, struct cifs_ses *ses,
>  	 * use ses->maxReq.
>  	 */
>  
> -	rc = wait_for_free_request(ses->server, long_op);
> +	rc = wait_for_free_request(ses->server, optype, long_op);
>  	if (rc) {
>  		cifs_small_buf_release(buf);
>  		return rc;
> @@ -583,7 +589,7 @@ SendReceive2(const unsigned int xid, struct cifs_ses *ses,
>  		mutex_unlock(&ses->server->srv_mutex);
>  		cifs_small_buf_release(buf);
>  		/* Update # of requests on wire to server */
> -		add_credits(ses->server, 1);
> +		add_credits(ses->server, 1, optype);
>  		return rc;
>  	}
>  
> @@ -613,7 +619,7 @@ SendReceive2(const unsigned int xid, struct cifs_ses *ses,
>  			midQ->callback = DeleteMidQEntry;
>  			spin_unlock(&GlobalMid_Lock);
>  			cifs_small_buf_release(buf);
> -			add_credits(ses->server, 1);
> +			add_credits(ses->server, 1, optype);
>  			return rc;
>  		}
>  		spin_unlock(&GlobalMid_Lock);
> @@ -623,7 +629,7 @@ SendReceive2(const unsigned int xid, struct cifs_ses *ses,
>  
>  	rc = cifs_sync_mid_result(midQ, ses->server);
>  	if (rc != 0) {
> -		add_credits(ses->server, 1);
> +		add_credits(ses->server, 1, optype);
>  		return rc;
>  	}
>  
> @@ -641,6 +647,8 @@ SendReceive2(const unsigned int xid, struct cifs_ses *ses,
>  	else
>  		*pRespBufType = CIFS_SMALL_BUFFER;
>  
> +	credits = ses->server->ops->get_credits(midQ);
> +
>  	rc = ses->server->ops->check_receive(midQ, ses->server,
>  					     flags & CIFS_LOG_ERROR);
>  
> @@ -649,7 +657,7 @@ SendReceive2(const unsigned int xid, struct cifs_ses *ses,
>  		midQ->resp_buf = NULL;
>  out:
>  	delete_mid(midQ);
> -	add_credits(ses->server, 1);
> +	add_credits(ses->server, credits, optype);
>  
>  	return rc;
>  }
> @@ -685,7 +693,7 @@ SendReceive(const unsigned int xid, struct cifs_ses *ses,
>  		return -EIO;
>  	}
>  
> -	rc = wait_for_free_request(ses->server, long_op);
> +	rc = wait_for_free_request(ses->server, 0, long_op);
>  	if (rc)
>  		return rc;
>  
> @@ -699,7 +707,7 @@ SendReceive(const unsigned int xid, struct cifs_ses *ses,
>  	if (rc) {
>  		mutex_unlock(&ses->server->srv_mutex);
>  		/* Update # of requests on wire to server */
> -		add_credits(ses->server, 1);
> +		add_credits(ses->server, 1, 0);
>  		return rc;
>  	}
>  
> @@ -731,7 +739,7 @@ SendReceive(const unsigned int xid, struct cifs_ses *ses,
>  			/* no longer considered to be "in-flight" */
>  			midQ->callback = DeleteMidQEntry;
>  			spin_unlock(&GlobalMid_Lock);
> -			add_credits(ses->server, 1);
> +			add_credits(ses->server, 1, 0);
>  			return rc;
>  		}
>  		spin_unlock(&GlobalMid_Lock);
> @@ -739,7 +747,7 @@ SendReceive(const unsigned int xid, struct cifs_ses *ses,
>  
>  	rc = cifs_sync_mid_result(midQ, ses->server);
>  	if (rc != 0) {
> -		add_credits(ses->server, 1);
> +		add_credits(ses->server, 1, 0);
>  		return rc;
>  	}
>  
> @@ -755,7 +763,7 @@ SendReceive(const unsigned int xid, struct cifs_ses *ses,
>  	rc = cifs_check_receive(midQ, ses->server, 0);
>  out:
>  	delete_mid(midQ);
> -	add_credits(ses->server, 1);
> +	add_credits(ses->server, 1, 0);
>  
>  	return rc;
>  }
> @@ -820,7 +828,7 @@ SendReceiveBlockingLock(const unsigned int xid, struct cifs_tcon *tcon,
>  		return -EIO;
>  	}
>  
> -	rc = wait_for_free_request(ses->server, CIFS_BLOCKING_OP);
> +	rc = wait_for_free_request(ses->server, 0, CIFS_BLOCKING_OP);
>  	if (rc)
>  		return rc;
>  

Other than the nit about renaming that variable, this looks reasonable,
at least until we can come up with a better scheme to handle this...

Acked-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

Re: [PATCH v5 3/5] CIFS: Move protocol specific negotiate code to ops struct

[Jeff Layton]

On Sat,  9 Jun 2012 10:26:02 +0400
Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org> wrote:

> Signed-off-by: Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
> ---
>  fs/cifs/cifsglob.h  |    8 ++++++--
>  fs/cifs/cifsproto.h |    5 ++---
>  fs/cifs/cifssmb.c   |    4 ++--
>  fs/cifs/connect.c   |   21 +++++++++------------
>  fs/cifs/sess.c      |    2 +-
>  fs/cifs/smb1ops.c   |   23 +++++++++++++++++++++++
>  6 files changed, 43 insertions(+), 20 deletions(-)
> 
> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> index 844b77c..669de88 100644
> --- a/fs/cifs/cifsglob.h
> +++ b/fs/cifs/cifsglob.h
> @@ -192,6 +192,10 @@ struct smb_version_operations {
>  	/* process transaction2 response */
>  	bool (*check_trans2)(struct mid_q_entry *, struct TCP_Server_Info *,
>  			     char *, int);
> +	/* check if we need to negotiate */
> +	bool (*need_neg)(struct TCP_Server_Info *);
> +	/* negotiate to the server */
> +	int (*negotiate)(const int, struct cifs_ses *);
>  };
>  
>  struct smb_version_values {
> @@ -324,7 +328,7 @@ struct TCP_Server_Info {
>  	struct mutex srv_mutex;
>  	struct task_struct *tsk;
>  	char server_GUID[16];
> -	char sec_mode;
> +	__u16 sec_mode;
>  	bool session_estab; /* mark when very first sess is established */
>  	u16 dialect; /* dialect index that server chose */
>  	enum securityEnum secType;
> @@ -459,7 +463,7 @@ struct cifs_ses {
>  	char *serverOS;		/* name of operating system underlying server */
>  	char *serverNOS;	/* name of network operating system of server */
>  	char *serverDomain;	/* security realm of server */
> -	int Suid;		/* remote smb uid  */
> +	__u64 Suid;		/* remote smb uid  */
>  	uid_t linux_uid;        /* overriding owner of files on the mount */
>  	uid_t cred_uid;		/* owner of credentials */
>  	int capabilities;
> diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
> index 8c1c7c1..32b569f 100644
> --- a/fs/cifs/cifsproto.h
> +++ b/fs/cifs/cifsproto.h
> @@ -178,11 +178,10 @@ extern void cifs_dfs_release_automount_timer(void);
>  void cifs_proc_init(void);
>  void cifs_proc_clean(void);
>  
> -extern int cifs_negotiate_protocol(unsigned int xid,
> -				  struct cifs_ses *ses);
> +extern int cifs_negotiate_protocol(const int xid, struct cifs_ses *ses);
>  extern int cifs_setup_session(unsigned int xid, struct cifs_ses *ses,
>  			struct nls_table *nls_info);
> -extern int CIFSSMBNegotiate(unsigned int xid, struct cifs_ses *ses);
> +extern int CIFSSMBNegotiate(const int xid, struct cifs_ses *ses);
>  
>  extern int CIFSTCon(unsigned int xid, struct cifs_ses *ses,
>  			const char *tree, struct cifs_tcon *tcon,
> diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
> index 58f1ab5..fbc5628 100644
> --- a/fs/cifs/cifssmb.c
> +++ b/fs/cifs/cifssmb.c
> @@ -372,7 +372,7 @@ static inline void inc_rfc1001_len(void *pSMB, int count)
>  }
>  
>  int
> -CIFSSMBNegotiate(unsigned int xid, struct cifs_ses *ses)
> +CIFSSMBNegotiate(const int xid, struct cifs_ses *ses)
>  {
>  	NEGOTIATE_REQ *pSMB;
>  	NEGOTIATE_RSP *pSMBr;
> @@ -456,7 +456,7 @@ CIFSSMBNegotiate(unsigned int xid, struct cifs_ses *ses)
>  			rc = -EOPNOTSUPP;
>  			goto neg_err_exit;
>  		}
> -		server->sec_mode = (__u8)le16_to_cpu(rsp->SecurityMode);
> +		server->sec_mode = le16_to_cpu(rsp->SecurityMode);
>  		server->maxReq = min_t(unsigned int,
>  				       le16_to_cpu(rsp->MaxMpxCount),
>  				       cifs_max_pending);
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index ca0288d..4780342 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -406,7 +406,7 @@ cifs_echo_request(struct work_struct *work)
>  	 * done, which is indicated by maxBuf != 0. Also, no need to ping if
>  	 * we got a response recently
>  	 */
> -	if (server->maxBuf == 0 ||
> +	if (!server->ops->need_neg || server->ops->need_neg(server) ||
>  	    time_before(jiffies, server->lstrp + SMB_ECHO_INTERVAL - HZ))
>  		goto requeue_echo;
>  
> @@ -3942,24 +3942,22 @@ cifs_umount(struct cifs_sb_info *cifs_sb)
>  	kfree(cifs_sb);
>  }
>  
> -int cifs_negotiate_protocol(unsigned int xid, struct cifs_ses *ses)
> +int
> +cifs_negotiate_protocol(const int xid, struct cifs_ses *ses)
>  {
>  	int rc = 0;
>  	struct TCP_Server_Info *server = ses->server;
>  
> +	if (!server->ops->need_neg || !server->ops->negotiate)
> +		return -ENOSYS;
> +
>  	/* only send once per connect */
> -	if (server->maxBuf != 0)
> +	if (!server->ops->need_neg(server))
>  		return 0;
>  
>  	set_credits(server, 1);
> -	rc = CIFSSMBNegotiate(xid, ses);
> -	if (rc == -EAGAIN) {
> -		/* retry only once on 1st time connection */
> -		set_credits(server, 1);
> -		rc = CIFSSMBNegotiate(xid, ses);
> -		if (rc == -EAGAIN)
> -			rc = -EHOSTDOWN;
> -	}
> +
> +	rc = server->ops->negotiate(xid, ses);
>  	if (rc == 0) {
>  		spin_lock(&GlobalMid_Lock);
>  		if (server->tcpStatus == CifsNeedNegotiate)
> @@ -3967,7 +3965,6 @@ int cifs_negotiate_protocol(unsigned int xid, struct cifs_ses *ses)
>  		else
>  			rc = -EHOSTDOWN;
>  		spin_unlock(&GlobalMid_Lock);
> -
>  	}
>  
>  	return rc;
> diff --git a/fs/cifs/sess.c b/fs/cifs/sess.c
> index 551d0c2..f88fa4d 100644
> --- a/fs/cifs/sess.c
> +++ b/fs/cifs/sess.c
> @@ -898,7 +898,7 @@ ssetup_ntlmssp_authenticate:
>  	if (action & GUEST_LOGIN)
>  		cFYI(1, "Guest login"); /* BB mark SesInfo struct? */
>  	ses->Suid = smb_buf->Uid;   /* UID left in wire format (le) */
> -	cFYI(1, "UID = %d ", ses->Suid);
> +	cFYI(1, "UID = %llu ", ses->Suid);
>  	/* response can have either 3 or 4 word count - Samba sends 3 */
>  	/* and lanman response is 3 */
>  	bytes_remaining = get_bcc(smb_buf);
> diff --git a/fs/cifs/smb1ops.c b/fs/cifs/smb1ops.c
> index f4f8394..5bfc26a 100644
> --- a/fs/cifs/smb1ops.c
> +++ b/fs/cifs/smb1ops.c
> @@ -389,6 +389,27 @@ cifs_check_trans2(struct mid_q_entry *mid, struct TCP_Server_Info *server,
>  	return true;
>  }
>  
> +static bool
> +cifs_need_neg(struct TCP_Server_Info *server)
> +{
> +	return server->maxBuf == 0;
> +}
> +
> +static int
> +cifs_negotiate(const int xid, struct cifs_ses *ses)
> +{
> +	int rc;
> +	rc = CIFSSMBNegotiate(xid, ses);
> +	if (rc == -EAGAIN) {
> +		/* retry only once on 1st time connection */
> +		set_credits(ses->server, 1);
> +		rc = CIFSSMBNegotiate(xid, ses);
> +		if (rc == -EAGAIN)
> +			rc = -EHOSTDOWN;
> +	}
> +	return rc;
> +}
> +
>  struct smb_version_operations smb1_operations = {
>  	.send_cancel = send_nt_cancel,
>  	.compare_fids = cifs_compare_fids,
> @@ -407,6 +428,8 @@ struct smb_version_operations smb1_operations = {
>  	.dump_detail = cifs_dump_detail,
>  	.is_oplock_break = is_valid_oplock_break,
>  	.check_trans2 = cifs_check_trans2,
> +	.need_neg = cifs_need_neg,
> +	.negotiate = cifs_negotiate,
>  };
>  
>  struct smb_version_values smb1_values = {

Reviewed-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

Re: [PATCH v5 4/5] CIFS: Move protocol specific session setup/logoff code to ops struct

[Jeff Layton]

On Sat,  9 Jun 2012 10:26:03 +0400
Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org> wrote:

> Signed-off-by: Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
> ---
>  fs/cifs/cifsglob.h  |    5 +++++
>  fs/cifs/cifsproto.h |    8 ++++----
>  fs/cifs/connect.c   |   16 +++++++++-------
>  fs/cifs/sess.c      |    2 +-
>  fs/cifs/smb1ops.c   |    2 ++
>  5 files changed, 21 insertions(+), 12 deletions(-)
> 
> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> index 669de88..812f27c 100644
> --- a/fs/cifs/cifsglob.h
> +++ b/fs/cifs/cifsglob.h
> @@ -196,6 +196,11 @@ struct smb_version_operations {
>  	bool (*need_neg)(struct TCP_Server_Info *);
>  	/* negotiate to the server */
>  	int (*negotiate)(const int, struct cifs_ses *);
> +	/* setup smb sessionn */
> +	int (*sess_setup)(const int, struct cifs_ses *,
> +			  const struct nls_table *);
> +	/* close smb session */
> +	int (*logoff)(const int, struct cifs_ses *);
>  };
>  
>  struct smb_version_values {
> diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
> index 32b569f..8e6cd38 100644
> --- a/fs/cifs/cifsproto.h
> +++ b/fs/cifs/cifsproto.h
> @@ -112,8 +112,8 @@ extern void header_assemble(struct smb_hdr *, char /* command */ ,
>  extern int small_smb_init_no_tc(const int smb_cmd, const int wct,
>  				struct cifs_ses *ses,
>  				void **request_buf);
> -extern int CIFS_SessSetup(unsigned int xid, struct cifs_ses *ses,
> -			     const struct nls_table *nls_cp);
> +extern int CIFS_SessSetup(const int xid, struct cifs_ses *ses,
> +			  const struct nls_table *nls_cp);
>  extern struct timespec cifs_NTtimeToUnix(__le64 utc_nanoseconds_since_1601);
>  extern u64 cifs_UnixTimeToNT(struct timespec);
>  extern struct timespec cnvrtDosUnixTm(__le16 le_date, __le16 le_time,
> @@ -179,8 +179,8 @@ void cifs_proc_init(void);
>  void cifs_proc_clean(void);
>  
>  extern int cifs_negotiate_protocol(const int xid, struct cifs_ses *ses);
> -extern int cifs_setup_session(unsigned int xid, struct cifs_ses *ses,
> -			struct nls_table *nls_info);
> +extern int cifs_setup_session(const int xid, struct cifs_ses *ses,
> +			      struct nls_table *nls_info);
>  extern int CIFSSMBNegotiate(const int xid, struct cifs_ses *ses);
>  
>  extern int CIFSTCon(unsigned int xid, struct cifs_ses *ses,
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index 4780342..9679352 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -2263,9 +2263,9 @@ cifs_put_smb_ses(struct cifs_ses *ses)
>  	list_del_init(&ses->smb_ses_list);
>  	spin_unlock(&cifs_tcp_ses_lock);
>  
> -	if (ses->status == CifsGood) {
> +	if (ses->status == CifsGood && server->ops->logoff) {
>  		xid = GetXid();
> -		CIFSSMBLogoff(xid, ses);
> +		server->ops->logoff(xid, ses);
>  		_FreeXid(xid);
>  	}
>  	sesInfoFree(ses);
> @@ -3970,11 +3970,11 @@ cifs_negotiate_protocol(const int xid, struct cifs_ses *ses)
>  	return rc;
>  }
>  
> -
> -int cifs_setup_session(unsigned int xid, struct cifs_ses *ses,
> -			struct nls_table *nls_info)
> +int
> +cifs_setup_session(const int xid, struct cifs_ses *ses,
> +		   struct nls_table *nls_info)


While I think this whole "xid" thing is a bogus idea, if we're going to
do it, we should at least try to be consistent about the type. _GetXid
returns unsigned int but you're turning that into a signed int here.

I suggest that we don't do that. Let's declare henceforth that it's
unsigned. Any place that treats it as signed is a bug. We probably have
a lot of these bugs. I don't expect you to fix them all up, but you
should at least try to fix that up in the code that you're touching in
this series. We can get the rest later...

Sound ok?


>  {
> -	int rc = 0;
> +	int rc = -ENOSYS;
>  	struct TCP_Server_Info *server = ses->server;
>  
>  	ses->flags = 0;
> @@ -3985,7 +3985,9 @@ int cifs_setup_session(unsigned int xid, struct cifs_ses *ses,
>  	cFYI(1, "Security Mode: 0x%x Capabilities: 0x%x TimeAdjust: %d",
>  		 server->sec_mode, server->capabilities, server->timeAdj);
>  
> -	rc = CIFS_SessSetup(xid, ses, nls_info);
> +	if (server->ops->sess_setup)
> +		rc = server->ops->sess_setup(xid, ses, nls_info);
> +
>  	if (rc) {
>  		cERROR(1, "Send error in SessSetup = %d", rc);
>  	} else {
> diff --git a/fs/cifs/sess.c b/fs/cifs/sess.c
> index f88fa4d..16b2083 100644
> --- a/fs/cifs/sess.c
> +++ b/fs/cifs/sess.c
> @@ -556,7 +556,7 @@ setup_ntlmv2_ret:
>  }
>  
>  int
> -CIFS_SessSetup(unsigned int xid, struct cifs_ses *ses,
> +CIFS_SessSetup(const int xid, struct cifs_ses *ses,
>  	       const struct nls_table *nls_cp)
>  {
>  	int rc = 0;
> diff --git a/fs/cifs/smb1ops.c b/fs/cifs/smb1ops.c
> index 5bfc26a..ba3071f 100644
> --- a/fs/cifs/smb1ops.c
> +++ b/fs/cifs/smb1ops.c
> @@ -430,6 +430,8 @@ struct smb_version_operations smb1_operations = {
>  	.check_trans2 = cifs_check_trans2,
>  	.need_neg = cifs_need_neg,
>  	.negotiate = cifs_negotiate,
> +	.sess_setup = CIFS_SessSetup,
> +	.logoff = CIFSSMBLogoff,
>  };
>  
>  struct smb_version_values smb1_values = {


-- 
Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

Re: [PATCH v5 5/5] CIFS: Move protocol specific tcon/tdis code to ops struct

[Jeff Layton]

On Sat,  9 Jun 2012 10:26:04 +0400
Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org> wrote:

> and rename variables around the code changes.
> 
> Signed-off-by: Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
> ---
>  fs/cifs/cifsglob.h  |    6 ++++
>  fs/cifs/cifsproto.h |    5 +--
>  fs/cifs/connect.c   |   71 ++++++++++++++++++++++++++++++---------------------
>  fs/cifs/smb1ops.c   |    2 +
>  4 files changed, 52 insertions(+), 32 deletions(-)
> 
> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> index 812f27c..1266e2e 100644
> --- a/fs/cifs/cifsglob.h
> +++ b/fs/cifs/cifsglob.h
> @@ -160,6 +160,7 @@ struct mid_q_entry;
>  struct TCP_Server_Info;
>  struct cifsFileInfo;
>  struct cifs_ses;
> +struct cifs_tcon;
>  
>  struct smb_version_operations {
>  	int (*send_cancel)(struct TCP_Server_Info *, void *,
> @@ -201,6 +202,11 @@ struct smb_version_operations {
>  			  const struct nls_table *);
>  	/* close smb session */
>  	int (*logoff)(const int, struct cifs_ses *);
> +	/* connect to a server share */
> +	int (*tree_connect)(const int, struct cifs_ses *, const char *,
> +			    struct cifs_tcon *, const struct nls_table *);
> +	/* close tree connecion */
> +	int (*tree_disconnect)(const int, struct cifs_tcon *);
>  };
>  
>  struct smb_version_values {
> diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
> index 8e6cd38..2f4f661 100644
> --- a/fs/cifs/cifsproto.h
> +++ b/fs/cifs/cifsproto.h
> @@ -183,9 +183,8 @@ extern int cifs_setup_session(const int xid, struct cifs_ses *ses,
>  			      struct nls_table *nls_info);
>  extern int CIFSSMBNegotiate(const int xid, struct cifs_ses *ses);
>  
> -extern int CIFSTCon(unsigned int xid, struct cifs_ses *ses,
> -			const char *tree, struct cifs_tcon *tcon,
> -			const struct nls_table *);
> +extern int CIFSTCon(const int xid, struct cifs_ses *ses, const char *tree,
> +		    struct cifs_tcon *tcon, const struct nls_table *);
>  
>  extern int CIFSFindFirst(const int xid, struct cifs_tcon *tcon,
>  		const char *searchName, const struct nls_table *nls_codepage,
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index 9679352..d14bd45 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -2552,7 +2552,8 @@ cifs_put_tcon(struct cifs_tcon *tcon)
>  	spin_unlock(&cifs_tcp_ses_lock);
>  
>  	xid = GetXid();
> -	CIFSSMBTDis(xid, tcon);
> +	if (ses->server->ops->tree_disconnect)
> +		ses->server->ops->tree_disconnect(xid, tcon);
>  	_FreeXid(xid);
>  
>  	cifs_fscache_release_super_cookie(tcon);
> @@ -2577,6 +2578,11 @@ cifs_get_tcon(struct cifs_ses *ses, struct smb_vol *volume_info)
>  		return tcon;
>  	}
>  
> +	if (!ses->server->ops->tree_connect) {
> +		rc = -ENOSYS;
> +		goto out_fail;
> +	}
> +
>  	tcon = tconInfoAlloc();
>  	if (tcon == NULL) {
>  		rc = -ENOMEM;
> @@ -2599,13 +2605,15 @@ cifs_get_tcon(struct cifs_ses *ses, struct smb_vol *volume_info)
>  		goto out_fail;
>  	}
>  
> -	/* BB Do we need to wrap session_mutex around
> -	 * this TCon call and Unix SetFS as
> -	 * we do on SessSetup and reconnect? */
> +	/*
> +	 * BB Do we need to wrap session_mutex around this TCon call and Unix
> +	 * SetFS as we do on SessSetup and reconnect?
> +	 */
>  	xid = GetXid();
> -	rc = CIFSTCon(xid, ses, volume_info->UNC, tcon, volume_info->local_nls);
> +	rc = ses->server->ops->tree_connect(xid, ses, volume_info->UNC, tcon,
> +					    volume_info->local_nls);
>  	FreeXid(xid);
> -	cFYI(1, "CIFS Tcon rc = %d", rc);
> +	cFYI(1, "Tcon rc = %d", rc);
>  	if (rc)
>  		goto out_fail;
>  
> @@ -2614,10 +2622,11 @@ cifs_get_tcon(struct cifs_ses *ses, struct smb_vol *volume_info)
>  		cFYI(1, "DFS disabled (%d)", tcon->Flags);
>  	}
>  	tcon->seal = volume_info->seal;
> -	/* we can have only one retry value for a connection
> -	   to a share so for resources mounted more than once
> -	   to the same server share the last value passed in
> -	   for the retry flag is used */
> +	/*
> +	 * We can have only one retry value for a connection to a share so for
> +	 * resources mounted more than once to the same server share the last
> +	 * value passed in for the retry flag is used.
> +	 */
>  	tcon->retry = volume_info->retry;
>  	tcon->nocase = volume_info->nocase;
>  	tcon->local_lease = volume_info->local_lease;
> @@ -2751,37 +2760,41 @@ out:
>  }
>  
>  int
> -get_dfs_path(int xid, struct cifs_ses *pSesInfo, const char *old_path,
> -	     const struct nls_table *nls_codepage, unsigned int *pnum_referrals,
> -	     struct dfs_info3_param **preferrals, int remap)
> +get_dfs_path(int xid, struct cifs_ses *ses, const char *old_path,
> +	     const struct nls_table *nls_codepage, unsigned int *num_referrals,
> +	     struct dfs_info3_param **referrals, int remap)
>  {
>  	char *temp_unc;
>  	int rc = 0;
>  
> -	*pnum_referrals = 0;
> -	*preferrals = NULL;
> +	if (!ses->server->ops->tree_connect)
> +		return -ENOSYS;
> +
> +	*num_referrals = 0;
> +	*referrals = NULL;
>  
> -	if (pSesInfo->ipc_tid == 0) {
> +	if (ses->ipc_tid == 0) {
>  		temp_unc = kmalloc(2 /* for slashes */ +
> -			strnlen(pSesInfo->serverName,
> -				SERVER_NAME_LEN_WITH_NULL * 2)
> -				 + 1 + 4 /* slash IPC$ */  + 2,
> -				GFP_KERNEL);
> +			strnlen(ses->serverName, SERVER_NAME_LEN_WITH_NULL * 2)
> +				+ 1 + 4 /* slash IPC$ */ + 2, GFP_KERNEL);
>  		if (temp_unc == NULL)
>  			return -ENOMEM;
>  		temp_unc[0] = '\\';
>  		temp_unc[1] = '\\';
> -		strcpy(temp_unc + 2, pSesInfo->serverName);
> -		strcpy(temp_unc + 2 + strlen(pSesInfo->serverName), "\\IPC$");
> -		rc = CIFSTCon(xid, pSesInfo, temp_unc, NULL, nls_codepage);
> -		cFYI(1, "CIFS Tcon rc = %d ipc_tid = %d", rc, pSesInfo->ipc_tid);
> +		strcpy(temp_unc + 2, ses->serverName);
> +		strcpy(temp_unc + 2 + strlen(ses->serverName), "\\IPC$");
> +		rc = ses->server->ops->tree_connect(xid, ses, temp_unc, NULL,
> +						    nls_codepage);
> +		cFYI(1, "Tcon rc = %d ipc_tid = %d", rc, ses->ipc_tid);
>  		kfree(temp_unc);
>  	}
>  	if (rc == 0)
> -		rc = CIFSGetDFSRefer(xid, pSesInfo, old_path, preferrals,
> -				     pnum_referrals, nls_codepage, remap);
> -	/* BB map targetUNCs to dfs_info3 structures, here or
> -		in CIFSGetDFSRefer BB */
> +		rc = CIFSGetDFSRefer(xid, ses, old_path, referrals,
> +				     num_referrals, nls_codepage, remap);
> +	/*
> +	 * BB - map targetUNCs to dfs_info3 structures, here or in
> +	 * CIFSGetDFSRefer.
> +	 */
>  
>  	return rc;
>  }
> @@ -3758,7 +3771,7 @@ out:
>   * pointer may be NULL.
>   */
>  int
> -CIFSTCon(unsigned int xid, struct cifs_ses *ses,
> +CIFSTCon(const int xid, struct cifs_ses *ses,
>  	 const char *tree, struct cifs_tcon *tcon,
>  	 const struct nls_table *nls_codepage)
>  {
> diff --git a/fs/cifs/smb1ops.c b/fs/cifs/smb1ops.c
> index ba3071f..1a3f08c 100644
> --- a/fs/cifs/smb1ops.c
> +++ b/fs/cifs/smb1ops.c
> @@ -432,6 +432,8 @@ struct smb_version_operations smb1_operations = {
>  	.negotiate = cifs_negotiate,
>  	.sess_setup = CIFS_SessSetup,
>  	.logoff = CIFSSMBLogoff,
> +	.tree_connect = CIFSTCon,
> +	.tree_disconnect = CIFSSMBTDis,
>  };
>  
>  struct smb_version_values smb1_values = {

Looks fine other than the type change on the xid. Note too, that if you
wanted to just remove all of that xid stuff, I'd be even happier. Steve
seems to like it for some reason though, so we'll need to convince him.

IMO, it's just unnecessary clutter that gets in the way of the code
clarity. It's also very unclear that FreeXid() has a side effect (a
debug printk).

A better scheme would be to turn all of those FreeXid calls into an
explict cFYI that prints the name of the function and maybe the return
code. The xid could be replaced with current->pid, which would
eliminate GetXid.

-- 
Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

Re: [PATCH v5 5/5] CIFS: Move protocol specific tcon/tdis code to ops struct

[Steve French]

The main debugging benefit I get from the xid is simply being able to
track in logs which operations I am looking at when running something
like dbench with lots of processes in parallel or even when one
process but log entries got lost. Pid isn't always as helpful since
log entries get dropped - it is not always clear which call is failing
when pid does multiple operations.   Quick modifications of other log
entries to add xid is pretty easy when the xid in a particular
important cFYI or cERROR is not clear in a complex trace.

On Tue, Jun 19, 2012 at 5:59 PM, Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> On Sat,  9 Jun 2012 10:26:04 +0400
> Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org> wrote:
>
>> and rename variables around the code changes.
>>
>> Signed-off-by: Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
>> ---
>>  fs/cifs/cifsglob.h  |    6 ++++
>>  fs/cifs/cifsproto.h |    5 +--
>>  fs/cifs/connect.c   |   71 ++++++++++++++++++++++++++++++---------------------
>>  fs/cifs/smb1ops.c   |    2 +
>>  4 files changed, 52 insertions(+), 32 deletions(-)
>>
>> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
>> index 812f27c..1266e2e 100644
>> --- a/fs/cifs/cifsglob.h
>> +++ b/fs/cifs/cifsglob.h
>> @@ -160,6 +160,7 @@ struct mid_q_entry;
>>  struct TCP_Server_Info;
>>  struct cifsFileInfo;
>>  struct cifs_ses;
>> +struct cifs_tcon;
>>
>>  struct smb_version_operations {
>>       int (*send_cancel)(struct TCP_Server_Info *, void *,
>> @@ -201,6 +202,11 @@ struct smb_version_operations {
>>                         const struct nls_table *);
>>       /* close smb session */
>>       int (*logoff)(const int, struct cifs_ses *);
>> +     /* connect to a server share */
>> +     int (*tree_connect)(const int, struct cifs_ses *, const char *,
>> +                         struct cifs_tcon *, const struct nls_table *);
>> +     /* close tree connecion */
>> +     int (*tree_disconnect)(const int, struct cifs_tcon *);
>>  };
>>
>>  struct smb_version_values {
>> diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
>> index 8e6cd38..2f4f661 100644
>> --- a/fs/cifs/cifsproto.h
>> +++ b/fs/cifs/cifsproto.h
>> @@ -183,9 +183,8 @@ extern int cifs_setup_session(const int xid, struct cifs_ses *ses,
>>                             struct nls_table *nls_info);
>>  extern int CIFSSMBNegotiate(const int xid, struct cifs_ses *ses);
>>
>> -extern int CIFSTCon(unsigned int xid, struct cifs_ses *ses,
>> -                     const char *tree, struct cifs_tcon *tcon,
>> -                     const struct nls_table *);
>> +extern int CIFSTCon(const int xid, struct cifs_ses *ses, const char *tree,
>> +                 struct cifs_tcon *tcon, const struct nls_table *);
>>
>>  extern int CIFSFindFirst(const int xid, struct cifs_tcon *tcon,
>>               const char *searchName, const struct nls_table *nls_codepage,
>> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
>> index 9679352..d14bd45 100644
>> --- a/fs/cifs/connect.c
>> +++ b/fs/cifs/connect.c
>> @@ -2552,7 +2552,8 @@ cifs_put_tcon(struct cifs_tcon *tcon)
>>       spin_unlock(&cifs_tcp_ses_lock);
>>
>>       xid = GetXid();
>> -     CIFSSMBTDis(xid, tcon);
>> +     if (ses->server->ops->tree_disconnect)
>> +             ses->server->ops->tree_disconnect(xid, tcon);
>>       _FreeXid(xid);
>>
>>       cifs_fscache_release_super_cookie(tcon);
>> @@ -2577,6 +2578,11 @@ cifs_get_tcon(struct cifs_ses *ses, struct smb_vol *volume_info)
>>               return tcon;
>>       }
>>
>> +     if (!ses->server->ops->tree_connect) {
>> +             rc = -ENOSYS;
>> +             goto out_fail;
>> +     }
>> +
>>       tcon = tconInfoAlloc();
>>       if (tcon == NULL) {
>>               rc = -ENOMEM;
>> @@ -2599,13 +2605,15 @@ cifs_get_tcon(struct cifs_ses *ses, struct smb_vol *volume_info)
>>               goto out_fail;
>>       }
>>
>> -     /* BB Do we need to wrap session_mutex around
>> -      * this TCon call and Unix SetFS as
>> -      * we do on SessSetup and reconnect? */
>> +     /*
>> +      * BB Do we need to wrap session_mutex around this TCon call and Unix
>> +      * SetFS as we do on SessSetup and reconnect?
>> +      */
>>       xid = GetXid();
>> -     rc = CIFSTCon(xid, ses, volume_info->UNC, tcon, volume_info->local_nls);
>> +     rc = ses->server->ops->tree_connect(xid, ses, volume_info->UNC, tcon,
>> +                                         volume_info->local_nls);
>>       FreeXid(xid);
>> -     cFYI(1, "CIFS Tcon rc = %d", rc);
>> +     cFYI(1, "Tcon rc = %d", rc);
>>       if (rc)
>>               goto out_fail;
>>
>> @@ -2614,10 +2622,11 @@ cifs_get_tcon(struct cifs_ses *ses, struct smb_vol *volume_info)
>>               cFYI(1, "DFS disabled (%d)", tcon->Flags);
>>       }
>>       tcon->seal = volume_info->seal;
>> -     /* we can have only one retry value for a connection
>> -        to a share so for resources mounted more than once
>> -        to the same server share the last value passed in
>> -        for the retry flag is used */
>> +     /*
>> +      * We can have only one retry value for a connection to a share so for
>> +      * resources mounted more than once to the same server share the last
>> +      * value passed in for the retry flag is used.
>> +      */
>>       tcon->retry = volume_info->retry;
>>       tcon->nocase = volume_info->nocase;
>>       tcon->local_lease = volume_info->local_lease;
>> @@ -2751,37 +2760,41 @@ out:
>>  }
>>
>>  int
>> -get_dfs_path(int xid, struct cifs_ses *pSesInfo, const char *old_path,
>> -          const struct nls_table *nls_codepage, unsigned int *pnum_referrals,
>> -          struct dfs_info3_param **preferrals, int remap)
>> +get_dfs_path(int xid, struct cifs_ses *ses, const char *old_path,
>> +          const struct nls_table *nls_codepage, unsigned int *num_referrals,
>> +          struct dfs_info3_param **referrals, int remap)
>>  {
>>       char *temp_unc;
>>       int rc = 0;
>>
>> -     *pnum_referrals = 0;
>> -     *preferrals = NULL;
>> +     if (!ses->server->ops->tree_connect)
>> +             return -ENOSYS;
>> +
>> +     *num_referrals = 0;
>> +     *referrals = NULL;
>>
>> -     if (pSesInfo->ipc_tid == 0) {
>> +     if (ses->ipc_tid == 0) {
>>               temp_unc = kmalloc(2 /* for slashes */ +
>> -                     strnlen(pSesInfo->serverName,
>> -                             SERVER_NAME_LEN_WITH_NULL * 2)
>> -                              + 1 + 4 /* slash IPC$ */  + 2,
>> -                             GFP_KERNEL);
>> +                     strnlen(ses->serverName, SERVER_NAME_LEN_WITH_NULL * 2)
>> +                             + 1 + 4 /* slash IPC$ */ + 2, GFP_KERNEL);
>>               if (temp_unc == NULL)
>>                       return -ENOMEM;
>>               temp_unc[0] = '\\';
>>               temp_unc[1] = '\\';
>> -             strcpy(temp_unc + 2, pSesInfo->serverName);
>> -             strcpy(temp_unc + 2 + strlen(pSesInfo->serverName), "\\IPC$");
>> -             rc = CIFSTCon(xid, pSesInfo, temp_unc, NULL, nls_codepage);
>> -             cFYI(1, "CIFS Tcon rc = %d ipc_tid = %d", rc, pSesInfo->ipc_tid);
>> +             strcpy(temp_unc + 2, ses->serverName);
>> +             strcpy(temp_unc + 2 + strlen(ses->serverName), "\\IPC$");
>> +             rc = ses->server->ops->tree_connect(xid, ses, temp_unc, NULL,
>> +                                                 nls_codepage);
>> +             cFYI(1, "Tcon rc = %d ipc_tid = %d", rc, ses->ipc_tid);
>>               kfree(temp_unc);
>>       }
>>       if (rc == 0)
>> -             rc = CIFSGetDFSRefer(xid, pSesInfo, old_path, preferrals,
>> -                                  pnum_referrals, nls_codepage, remap);
>> -     /* BB map targetUNCs to dfs_info3 structures, here or
>> -             in CIFSGetDFSRefer BB */
>> +             rc = CIFSGetDFSRefer(xid, ses, old_path, referrals,
>> +                                  num_referrals, nls_codepage, remap);
>> +     /*
>> +      * BB - map targetUNCs to dfs_info3 structures, here or in
>> +      * CIFSGetDFSRefer.
>> +      */
>>
>>       return rc;
>>  }
>> @@ -3758,7 +3771,7 @@ out:
>>   * pointer may be NULL.
>>   */
>>  int
>> -CIFSTCon(unsigned int xid, struct cifs_ses *ses,
>> +CIFSTCon(const int xid, struct cifs_ses *ses,
>>        const char *tree, struct cifs_tcon *tcon,
>>        const struct nls_table *nls_codepage)
>>  {
>> diff --git a/fs/cifs/smb1ops.c b/fs/cifs/smb1ops.c
>> index ba3071f..1a3f08c 100644
>> --- a/fs/cifs/smb1ops.c
>> +++ b/fs/cifs/smb1ops.c
>> @@ -432,6 +432,8 @@ struct smb_version_operations smb1_operations = {
>>       .negotiate = cifs_negotiate,
>>       .sess_setup = CIFS_SessSetup,
>>       .logoff = CIFSSMBLogoff,
>> +     .tree_connect = CIFSTCon,
>> +     .tree_disconnect = CIFSSMBTDis,
>>  };
>>
>>  struct smb_version_values smb1_values = {
>
> Looks fine other than the type change on the xid. Note too, that if you
> wanted to just remove all of that xid stuff, I'd be even happier. Steve
> seems to like it for some reason though, so we'll need to convince him.
>
> IMO, it's just unnecessary clutter that gets in the way of the code
> clarity. It's also very unclear that FreeXid() has a side effect (a
> debug printk).
>
> A better scheme would be to turn all of those FreeXid calls into an
> explict cFYI that prints the name of the function and maybe the return
> code. The xid could be replaced with current->pid, which would
> eliminate GetXid.
>
> --
> Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Thanks,

Steve

Re: [PATCH v5 4/5] CIFS: Move protocol specific session setup/logoff code to ops struct

[Pavel Shilovsky]

2012/6/19 Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>:
> On Sat,  9 Jun 2012 10:26:03 +0400
> Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org> wrote:
>
>> Signed-off-by: Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
>> ---
>>  fs/cifs/cifsglob.h  |    5 +++++
>>  fs/cifs/cifsproto.h |    8 ++++----
>>  fs/cifs/connect.c   |   16 +++++++++-------
>>  fs/cifs/sess.c      |    2 +-
>>  fs/cifs/smb1ops.c   |    2 ++
>>  5 files changed, 21 insertions(+), 12 deletions(-)
>>
>> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
>> index 669de88..812f27c 100644
>> --- a/fs/cifs/cifsglob.h
>> +++ b/fs/cifs/cifsglob.h
>> @@ -196,6 +196,11 @@ struct smb_version_operations {
>>       bool (*need_neg)(struct TCP_Server_Info *);
>>       /* negotiate to the server */
>>       int (*negotiate)(const int, struct cifs_ses *);
>> +     /* setup smb sessionn */
>> +     int (*sess_setup)(const int, struct cifs_ses *,
>> +                       const struct nls_table *);
>> +     /* close smb session */
>> +     int (*logoff)(const int, struct cifs_ses *);
>>  };
>>
>>  struct smb_version_values {
>> diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
>> index 32b569f..8e6cd38 100644
>> --- a/fs/cifs/cifsproto.h
>> +++ b/fs/cifs/cifsproto.h
>> @@ -112,8 +112,8 @@ extern void header_assemble(struct smb_hdr *, char /* command */ ,
>>  extern int small_smb_init_no_tc(const int smb_cmd, const int wct,
>>                               struct cifs_ses *ses,
>>                               void **request_buf);
>> -extern int CIFS_SessSetup(unsigned int xid, struct cifs_ses *ses,
>> -                          const struct nls_table *nls_cp);
>> +extern int CIFS_SessSetup(const int xid, struct cifs_ses *ses,
>> +                       const struct nls_table *nls_cp);
>>  extern struct timespec cifs_NTtimeToUnix(__le64 utc_nanoseconds_since_1601);
>>  extern u64 cifs_UnixTimeToNT(struct timespec);
>>  extern struct timespec cnvrtDosUnixTm(__le16 le_date, __le16 le_time,
>> @@ -179,8 +179,8 @@ void cifs_proc_init(void);
>>  void cifs_proc_clean(void);
>>
>>  extern int cifs_negotiate_protocol(const int xid, struct cifs_ses *ses);
>> -extern int cifs_setup_session(unsigned int xid, struct cifs_ses *ses,
>> -                     struct nls_table *nls_info);
>> +extern int cifs_setup_session(const int xid, struct cifs_ses *ses,
>> +                           struct nls_table *nls_info);
>>  extern int CIFSSMBNegotiate(const int xid, struct cifs_ses *ses);
>>
>>  extern int CIFSTCon(unsigned int xid, struct cifs_ses *ses,
>> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
>> index 4780342..9679352 100644
>> --- a/fs/cifs/connect.c
>> +++ b/fs/cifs/connect.c
>> @@ -2263,9 +2263,9 @@ cifs_put_smb_ses(struct cifs_ses *ses)
>>       list_del_init(&ses->smb_ses_list);
>>       spin_unlock(&cifs_tcp_ses_lock);
>>
>> -     if (ses->status == CifsGood) {
>> +     if (ses->status == CifsGood && server->ops->logoff) {
>>               xid = GetXid();
>> -             CIFSSMBLogoff(xid, ses);
>> +             server->ops->logoff(xid, ses);
>>               _FreeXid(xid);
>>       }
>>       sesInfoFree(ses);
>> @@ -3970,11 +3970,11 @@ cifs_negotiate_protocol(const int xid, struct cifs_ses *ses)
>>       return rc;
>>  }
>>
>> -
>> -int cifs_setup_session(unsigned int xid, struct cifs_ses *ses,
>> -                     struct nls_table *nls_info)
>> +int
>> +cifs_setup_session(const int xid, struct cifs_ses *ses,
>> +                struct nls_table *nls_info)
>

Thanks for the review.

>
> While I think this whole "xid" thing is a bogus idea, if we're going to
> do it, we should at least try to be consistent about the type. _GetXid
> returns unsigned int but you're turning that into a signed int here.
>
> I suggest that we don't do that. Let's declare henceforth that it's
> unsigned. Any place that treats it as signed is a bug. We probably have
> a lot of these bugs. I don't expect you to fix them all up, but you
> should at least try to fix that up in the code that you're touching in
> this series. We can get the rest later...
>
> Sound ok?
>

cifs_get_smb_ses has

int xid;

and does

xid = GetXid();

that returns 'int' - then it calls cifs_setup_session with this xid.
It seems no problems with my patch here that converts
cifs_setup_session to take an int.

But GedXid() is

 42 #define GetXid()>------->------->------->------->------->-------\
 43 ({>----->------->------->------->------->------->------->-------\
 44 >-------int __xid = (int)_GetXid();>---->------->------->-------\
 45 >-------cFYI(1, "CIFS VFS: in %s as Xid: %d with uid: %d",>-----\
 46 >-------     __func__, __xid, current_fsuid());>>------->-------\
 47 >-------__xid;>->------->------->------->------->------->-------\
 48 })

so, it assigns _GetXid to 'int' and returns sign int value - this
confuse me a little. Why don't we return unsigned int here?

-- 
Best regards,
Pavel Shilovsky.

Re: [PATCH v5 4/5] CIFS: Move protocol specific session setup/logoff code to ops struct

[Steve French]

On Tue, Jun 19, 2012 at 12:21 PM, Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org> wrote:
> 2012/6/19 Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>:
>> On Sat,  9 Jun 2012 10:26:03 +0400
>> Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org> wrote:
>>
>>> Signed-off-by: Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
>>> ---
>>>  fs/cifs/cifsglob.h  |    5 +++++
>>>  fs/cifs/cifsproto.h |    8 ++++----
>>>  fs/cifs/connect.c   |   16 +++++++++-------
>>>  fs/cifs/sess.c      |    2 +-
>>>  fs/cifs/smb1ops.c   |    2 ++
>>>  5 files changed, 21 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
>>> index 669de88..812f27c 100644
>>> --- a/fs/cifs/cifsglob.h
>>> +++ b/fs/cifs/cifsglob.h
>>> @@ -196,6 +196,11 @@ struct smb_version_operations {
>>>       bool (*need_neg)(struct TCP_Server_Info *);
>>>       /* negotiate to the server */
>>>       int (*negotiate)(const int, struct cifs_ses *);
>>> +     /* setup smb sessionn */
>>> +     int (*sess_setup)(const int, struct cifs_ses *,
>>> +                       const struct nls_table *);
>>> +     /* close smb session */
>>> +     int (*logoff)(const int, struct cifs_ses *);
>>>  };
>>>
>>>  struct smb_version_values {
>>> diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
>>> index 32b569f..8e6cd38 100644
>>> --- a/fs/cifs/cifsproto.h
>>> +++ b/fs/cifs/cifsproto.h
>>> @@ -112,8 +112,8 @@ extern void header_assemble(struct smb_hdr *, char /* command */ ,
>>>  extern int small_smb_init_no_tc(const int smb_cmd, const int wct,
>>>                               struct cifs_ses *ses,
>>>                               void **request_buf);
>>> -extern int CIFS_SessSetup(unsigned int xid, struct cifs_ses *ses,
>>> -                          const struct nls_table *nls_cp);
>>> +extern int CIFS_SessSetup(const int xid, struct cifs_ses *ses,
>>> +                       const struct nls_table *nls_cp);
>>>  extern struct timespec cifs_NTtimeToUnix(__le64 utc_nanoseconds_since_1601);
>>>  extern u64 cifs_UnixTimeToNT(struct timespec);
>>>  extern struct timespec cnvrtDosUnixTm(__le16 le_date, __le16 le_time,
>>> @@ -179,8 +179,8 @@ void cifs_proc_init(void);
>>>  void cifs_proc_clean(void);
>>>
>>>  extern int cifs_negotiate_protocol(const int xid, struct cifs_ses *ses);
>>> -extern int cifs_setup_session(unsigned int xid, struct cifs_ses *ses,
>>> -                     struct nls_table *nls_info);
>>> +extern int cifs_setup_session(const int xid, struct cifs_ses *ses,
>>> +                           struct nls_table *nls_info);
>>>  extern int CIFSSMBNegotiate(const int xid, struct cifs_ses *ses);
>>>
>>>  extern int CIFSTCon(unsigned int xid, struct cifs_ses *ses,
>>> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
>>> index 4780342..9679352 100644
>>> --- a/fs/cifs/connect.c
>>> +++ b/fs/cifs/connect.c
>>> @@ -2263,9 +2263,9 @@ cifs_put_smb_ses(struct cifs_ses *ses)
>>>       list_del_init(&ses->smb_ses_list);
>>>       spin_unlock(&cifs_tcp_ses_lock);
>>>
>>> -     if (ses->status == CifsGood) {
>>> +     if (ses->status == CifsGood && server->ops->logoff) {
>>>               xid = GetXid();
>>> -             CIFSSMBLogoff(xid, ses);
>>> +             server->ops->logoff(xid, ses);
>>>               _FreeXid(xid);
>>>       }
>>>       sesInfoFree(ses);
>>> @@ -3970,11 +3970,11 @@ cifs_negotiate_protocol(const int xid, struct cifs_ses *ses)
>>>       return rc;
>>>  }
>>>
>>> -
>>> -int cifs_setup_session(unsigned int xid, struct cifs_ses *ses,
>>> -                     struct nls_table *nls_info)
>>> +int
>>> +cifs_setup_session(const int xid, struct cifs_ses *ses,
>>> +                struct nls_table *nls_info)
>>
>
> Thanks for the review.
>
>>
>> While I think this whole "xid" thing is a bogus idea, if we're going to
>> do it, we should at least try to be consistent about the type. _GetXid
>> returns unsigned int but you're turning that into a signed int here.
>>
>> I suggest that we don't do that. Let's declare henceforth that it's
>> unsigned. Any place that treats it as signed is a bug. We probably have
>> a lot of these bugs. I don't expect you to fix them all up, but you
>> should at least try to fix that up in the code that you're touching in
>> this series. We can get the rest later...
>>
>> Sound ok?
>>
>
> cifs_get_smb_ses has
>
> int xid;
>
> and does
>
> xid = GetXid();
>
> that returns 'int' - then it calls cifs_setup_session with this xid.
> It seems no problems with my patch here that converts
> cifs_setup_session to take an int.
>
> But GedXid() is
>
>  42 #define GetXid()>------->------->------->------->------->-------\
>  43 ({>----->------->------->------->------->------->------->-------\
>  44 >-------int __xid = (int)_GetXid();>---->------->------->-------\
>  45 >-------cFYI(1, "CIFS VFS: in %s as Xid: %d with uid: %d",>-----\
>  46 >-------     __func__, __xid, current_fsuid());>>------->-------\
>  47 >-------__xid;>->------->------->------->------->------->-------\
>  48 })
>
> so, it assigns _GetXid to 'int' and returns sign int value - this
> confuse me a little. Why don't we return unsigned int here?

uint to int change in GetXid probably was an incorrect fix for some warning
(and probably an old change).  Why not simply fix this to
return unsigned int in GetXid to be consistent.  As you noted
_GetXid returns unsigned int (and FreeXid wants an
unsigned int).   If we do a global fix to change
GetXid to be consistent - we can fix the name at the same
time (to remove camel case) if camel case still bugs people.

Of course - much of the GetXid and cFYI stuff could be removed
if we had dynamic trace points and a howto on how to use them
for cifs - someone started looking at this for various fs three or
four years ago but don't think they finished.
-- 
Thanks,

Steve

Re: [PATCH v5 2/5] CIFS: Extend credit mechanism to process request type

[Pavel Shilovsky]

2012/6/19 Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>:
> On Sat,  9 Jun 2012 10:26:01 +0400
> Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org> wrote:
>
>> Split all requests to echos, oplocks and others - each group uses
>> its own credit slot. This is indicated by new flags
>>
>> CIFS_ECHO_OP and CIFS_OBREAK_OP
>>
>> that are not used now for CIFS. This change is required to support
>> SMB2 protocol because of different processing of these commands.
>>
>> Signed-off-by: Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
>> ---
>>  fs/cifs/cifsglob.h  |   16 +++++++++++---
>>  fs/cifs/cifsproto.h |    2 +-
>>  fs/cifs/cifssmb.c   |   21 ++++++++++---------
>>  fs/cifs/smb1ops.c   |   12 +++++++++-
>>  fs/cifs/transport.c |   54 +++++++++++++++++++++++++++++---------------------
>>  5 files changed, 65 insertions(+), 40 deletions(-)
>>
>> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
>> index 2aac4e5..844b77c 100644
>> --- a/fs/cifs/cifsglob.h
>> +++ b/fs/cifs/cifsglob.h
>> @@ -171,9 +171,11 @@ struct smb_version_operations {
>>       /* check response: verify signature, map error */
>>       int (*check_receive)(struct mid_q_entry *, struct TCP_Server_Info *,
>>                            bool);
>> -     void (*add_credits)(struct TCP_Server_Info *, const unsigned int);
>> +     void (*add_credits)(struct TCP_Server_Info *, const unsigned int,
>> +                         const int);
>>       void (*set_credits)(struct TCP_Server_Info *, const int);
>> -     int * (*get_credits_field)(struct TCP_Server_Info *);
>> +     int * (*get_credits_field)(struct TCP_Server_Info *, const int);
>> +     unsigned int (*get_credits)(struct mid_q_entry *);
>>       __u64 (*get_next_mid)(struct TCP_Server_Info *);
>>       /* data offset from read response message */
>>       unsigned int (*read_data_offset)(char *);
>> @@ -392,9 +394,10 @@ has_credits(struct TCP_Server_Info *server, int *credits)
>>  }
>>
>>  static inline void
>> -add_credits(struct TCP_Server_Info *server, const unsigned int add)
>> +add_credits(struct TCP_Server_Info *server, const unsigned int add,
>> +         const int optype)
>>  {
>> -     server->ops->add_credits(server, add);
>> +     server->ops->add_credits(server, add, optype);
>>  }
>>
>>  static inline void
>> @@ -957,6 +960,11 @@ static inline void free_dfs_info_array(struct dfs_info3_param *param,
>>  #define   CIFS_LARGE_BUF_OP 0x020    /* large request buffer */
>>  #define   CIFS_NO_RESP      0x040    /* no response buffer required */
>>
>> +/* Type of request operation */
>> +#define   CIFS_ECHO_OP      0x080    /* echo request */
>> +#define   CIFS_OBREAK_OP   0x0100    /* oplock break request */
>> +#define   CIFS_OP_MASK     0x0180    /* mask request type */
>> +
>>  /* Security Flags: indicate type of session setup needed */
>>  #define   CIFSSEC_MAY_SIGN   0x00001
>>  #define   CIFSSEC_MAY_NTLM   0x00002
>> diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
>> index 0a6cbfe..8c1c7c1 100644
>> --- a/fs/cifs/cifsproto.h
>> +++ b/fs/cifs/cifsproto.h
>> @@ -71,7 +71,7 @@ extern void DeleteMidQEntry(struct mid_q_entry *midEntry);
>>  extern int cifs_call_async(struct TCP_Server_Info *server, struct kvec *iov,
>>                          unsigned int nvec, mid_receive_t *receive,
>>                          mid_callback_t *callback, void *cbdata,
>> -                        bool ignore_pend);
>> +                        const int flags);
>>  extern int SendReceive(const unsigned int /* xid */ , struct cifs_ses *,
>>                       struct smb_hdr * /* input */ ,
>>                       struct smb_hdr * /* out */ ,
>> diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
>> index 5b40073..58f1ab5 100644
>> --- a/fs/cifs/cifssmb.c
>> +++ b/fs/cifs/cifssmb.c
>> @@ -720,7 +720,7 @@ cifs_echo_callback(struct mid_q_entry *mid)
>>       struct TCP_Server_Info *server = mid->callback_data;
>>
>>       DeleteMidQEntry(mid);
>> -     add_credits(server, 1);
>> +     add_credits(server, 1, CIFS_ECHO_OP);
>>  }
>>
>>  int
>> @@ -747,7 +747,7 @@ CIFSSMBEcho(struct TCP_Server_Info *server)
>>       iov.iov_len = be32_to_cpu(smb->hdr.smb_buf_length) + 4;
>>
>>       rc = cifs_call_async(server, &iov, 1, NULL, cifs_echo_callback,
>> -                          server, true);
>> +                          server, CIFS_ASYNC_OP | CIFS_ECHO_OP);
>>       if (rc)
>>               cFYI(1, "Echo request failed: %d", rc);
>>
>> @@ -1563,7 +1563,7 @@ cifs_readv_callback(struct mid_q_entry *mid)
>>
>>       queue_work(cifsiod_wq, &rdata->work);
>>       DeleteMidQEntry(mid);
>> -     add_credits(server, 1);
>> +     add_credits(server, 1, 0);
>>  }
>>
>>  /* cifs_async_readv - send an async write, and set up mid to handle result */
>> @@ -1619,7 +1619,7 @@ cifs_async_readv(struct cifs_readdata *rdata)
>>       kref_get(&rdata->refcount);
>>       rc = cifs_call_async(tcon->ses->server, rdata->iov, 1,
>>                            cifs_readv_receive, cifs_readv_callback,
>> -                          rdata, false);
>> +                          rdata, 0);
>>
>>       if (rc == 0)
>>               cifs_stats_inc(&tcon->num_reads);
>> @@ -2010,7 +2010,7 @@ cifs_writev_callback(struct mid_q_entry *mid)
>>
>>       queue_work(cifsiod_wq, &wdata->work);
>>       DeleteMidQEntry(mid);
>> -     add_credits(tcon->ses->server, 1);
>> +     add_credits(tcon->ses->server, 1, 0);
>>  }
>>
>>  /* cifs_async_writev - send an async write, and set up mid to handle result */
>> @@ -2090,7 +2090,7 @@ cifs_async_writev(struct cifs_writedata *wdata)
>>
>>       kref_get(&wdata->refcount);
>>       rc = cifs_call_async(tcon->ses->server, iov, wdata->nr_pages + 1,
>> -                          NULL, cifs_writev_callback, wdata, false);
>> +                          NULL, cifs_writev_callback, wdata, 0);
>>
>>       if (rc == 0)
>>               cifs_stats_inc(&tcon->num_writes);
>> @@ -2268,7 +2268,7 @@ CIFSSMBLock(const int xid, struct cifs_tcon *tcon,
>>       LOCK_REQ *pSMB = NULL;
>>  /*   LOCK_RSP *pSMBr = NULL; */ /* No response data other than rc to parse */
>>       int bytes_returned;
>> -     int timeout = 0;
>> +     int flags = 0;
>>       __u16 count;
>>
>>       cFYI(1, "CIFSSMBLock timeout %d numLock %d", (int)waitFlag, numLock);
>> @@ -2278,10 +2278,11 @@ CIFSSMBLock(const int xid, struct cifs_tcon *tcon,
>>               return rc;
>>
>>       if (lockType == LOCKING_ANDX_OPLOCK_RELEASE) {
>> -             timeout = CIFS_ASYNC_OP; /* no response expected */
>> +             /* no response expected */
>> +             flags = CIFS_ASYNC_OP | CIFS_OBREAK_OP;
>>               pSMB->Timeout = 0;
>>       } else if (waitFlag) {
>> -             timeout = CIFS_BLOCKING_OP; /* blocking operation, no timeout */
>> +             flags = CIFS_BLOCKING_OP; /* blocking operation, no timeout */
>>               pSMB->Timeout = cpu_to_le32(-1);/* blocking - do not time out */
>>       } else {
>>               pSMB->Timeout = 0;
>> @@ -2314,7 +2315,7 @@ CIFSSMBLock(const int xid, struct cifs_tcon *tcon,
>>                       (struct smb_hdr *) pSMB, &bytes_returned);
>>               cifs_small_buf_release(pSMB);
>>       } else {
>> -             rc = SendReceiveNoRsp(xid, tcon->ses, (char *)pSMB, timeout);
>> +             rc = SendReceiveNoRsp(xid, tcon->ses, (char *)pSMB, flags);
>>               /* SMB buffer freed by function above */
>>       }
>>       cifs_stats_inc(&tcon->num_locks);
>> diff --git a/fs/cifs/smb1ops.c b/fs/cifs/smb1ops.c
>> index 28359e7..f4f8394 100644
>> --- a/fs/cifs/smb1ops.c
>> +++ b/fs/cifs/smb1ops.c
>> @@ -101,7 +101,8 @@ cifs_find_mid(struct TCP_Server_Info *server, char *buffer)
>>  }
>>
>>  static void
>> -cifs_add_credits(struct TCP_Server_Info *server, const unsigned int add)
>> +cifs_add_credits(struct TCP_Server_Info *server, const unsigned int add,
>> +              const int optype)
>>  {
>>       spin_lock(&server->req_lock);
>>       server->credits += add;
>> @@ -120,11 +121,17 @@ cifs_set_credits(struct TCP_Server_Info *server, const int val)
>>  }
>>
>>  static int *
>> -cifs_get_credits_field(struct TCP_Server_Info *server)
>> +cifs_get_credits_field(struct TCP_Server_Info *server, const int optype)
>>  {
>>       return &server->credits;
>>  }
>>
>> +static unsigned int
>> +cifs_get_credits(struct mid_q_entry *mid)
>> +{
>> +     return 1;
>> +}
>> +
>>  /*
>>   * Find a free multiplex id (SMB mid). Otherwise there could be
>>   * mid collisions which might cause problems, demultiplexing the
>> @@ -390,6 +397,7 @@ struct smb_version_operations smb1_operations = {
>>       .add_credits = cifs_add_credits,
>>       .set_credits = cifs_set_credits,
>>       .get_credits_field = cifs_get_credits_field,
>> +     .get_credits = cifs_get_credits,
>>       .get_next_mid = cifs_get_next_mid,
>>       .read_data_offset = cifs_read_data_offset,
>>       .read_data_length = cifs_read_data_length,
>> diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c
>> index 3097ee5..54cd8dd 100644
>> --- a/fs/cifs/transport.c
>> +++ b/fs/cifs/transport.c
>> @@ -254,13 +254,13 @@ smb_send(struct TCP_Server_Info *server, struct smb_hdr *smb_buffer,
>>  }
>>
>>  static int
>> -wait_for_free_credits(struct TCP_Server_Info *server, const int optype,
>> +wait_for_free_credits(struct TCP_Server_Info *server, const int long_op,
>>                     int *credits)
>>  {
>>       int rc;
>>
>>       spin_lock(&server->req_lock);
>> -     if (optype == CIFS_ASYNC_OP) {
>> +     if (long_op == CIFS_ASYNC_OP) {
>>               /* oplock breaks must not be held up */
>>               server->in_flight++;
>>               *credits -= 1;
>> @@ -290,7 +290,7 @@ wait_for_free_credits(struct TCP_Server_Info *server, const int optype,
>>                        */
>>
>>                       /* update # of requests on the wire to server */
>> -                     if (optype != CIFS_BLOCKING_OP) {
>> +                     if (long_op != CIFS_BLOCKING_OP) {
>>                               *credits -= 1;
>>                               server->in_flight++;
>>                       }
>
> Why rename the variable in this above function? "optype" seems like a better name...

That was done to make variable names the same: 'optype' is used to
indicate if it is echo, oplock or other operation, 'long_op' is used
to indicate if we need any special behavior (aysnc, blocking).

wait_for_free_request function (below) takes both 'optype' and
'long_op' and passes 'long_op' to wait_for_free_credits. It is cleaner
if wait_for_free_credits uses the same name - 'long_op'.

Of course, 'long_op' may be not so good name at all (I took it from
the old code) - suggestions are welcome.

>
>> @@ -302,10 +302,11 @@ wait_for_free_credits(struct TCP_Server_Info *server, const int optype,
>>  }
>>
>>  static int
>> -wait_for_free_request(struct TCP_Server_Info *server, const int optype)
>> +wait_for_free_request(struct TCP_Server_Info *server, const int long_op,
>> +                   const int optype)
>>  {
>> -     return wait_for_free_credits(server, optype,
>> -                                  server->ops->get_credits_field(server));
>> +     return wait_for_free_credits(server, long_op,
>> +                             server->ops->get_credits_field(server, optype));
>>  }
>>
>>  static int allocate_mid(struct cifs_ses *ses, struct smb_hdr *in_buf,
>> @@ -384,12 +385,15 @@ cifs_setup_async_request(struct TCP_Server_Info *server, struct kvec *iov,
>>  int
>>  cifs_call_async(struct TCP_Server_Info *server, struct kvec *iov,
>>               unsigned int nvec, mid_receive_t *receive,
>> -             mid_callback_t *callback, void *cbdata, bool ignore_pend)
>> +             mid_callback_t *callback, void *cbdata, const int flags)
>>  {
>> -     int rc;
>> +     int rc, long_op, optype;
>>       struct mid_q_entry *mid;
>>
>> -     rc = wait_for_free_request(server, ignore_pend ? CIFS_ASYNC_OP : 0);
>> +     long_op = flags & CIFS_TIMEOUT_MASK;
>> +     optype = flags & CIFS_OP_MASK;
>> +
>> +     rc = wait_for_free_request(server, long_op, optype);
>>       if (rc)
>>               return rc;
>>
>> @@ -397,7 +401,7 @@ cifs_call_async(struct TCP_Server_Info *server, struct kvec *iov,
>>       rc = cifs_setup_async_request(server, iov, nvec, &mid);
>>       if (rc) {
>>               mutex_unlock(&server->srv_mutex);
>> -             add_credits(server, 1);
>> +             add_credits(server, 1, optype);
>>               wake_up(&server->request_q);
>>               return rc;
>>       }
>> @@ -419,7 +423,7 @@ cifs_call_async(struct TCP_Server_Info *server, struct kvec *iov,
>>       return rc;
>>  out_err:
>>       delete_mid(mid);
>> -     add_credits(server, 1);
>> +     add_credits(server, 1, optype);
>>       wake_up(&server->request_q);
>>       return rc;
>>  }
>> @@ -539,11 +543,13 @@ SendReceive2(const unsigned int xid, struct cifs_ses *ses,
>>            const int flags)
>>  {
>>       int rc = 0;
>> -     int long_op;
>> +     int long_op, optype;
>>       struct mid_q_entry *midQ;
>>       char *buf = iov[0].iov_base;
>> +     unsigned int credits = 1;
>>
>>       long_op = flags & CIFS_TIMEOUT_MASK;
>> +     optype = flags & CIFS_OP_MASK;
>>
>>       *pRespBufType = CIFS_NO_BUFFER;  /* no response buf yet */
>>
>> @@ -564,7 +570,7 @@ SendReceive2(const unsigned int xid, struct cifs_ses *ses,
>>        * use ses->maxReq.
>>        */
>>
>> -     rc = wait_for_free_request(ses->server, long_op);
>> +     rc = wait_for_free_request(ses->server, optype, long_op);
>>       if (rc) {
>>               cifs_small_buf_release(buf);
>>               return rc;
>> @@ -583,7 +589,7 @@ SendReceive2(const unsigned int xid, struct cifs_ses *ses,
>>               mutex_unlock(&ses->server->srv_mutex);
>>               cifs_small_buf_release(buf);
>>               /* Update # of requests on wire to server */
>> -             add_credits(ses->server, 1);
>> +             add_credits(ses->server, 1, optype);
>>               return rc;
>>       }
>>
>> @@ -613,7 +619,7 @@ SendReceive2(const unsigned int xid, struct cifs_ses *ses,
>>                       midQ->callback = DeleteMidQEntry;
>>                       spin_unlock(&GlobalMid_Lock);
>>                       cifs_small_buf_release(buf);
>> -                     add_credits(ses->server, 1);
>> +                     add_credits(ses->server, 1, optype);
>>                       return rc;
>>               }
>>               spin_unlock(&GlobalMid_Lock);
>> @@ -623,7 +629,7 @@ SendReceive2(const unsigned int xid, struct cifs_ses *ses,
>>
>>       rc = cifs_sync_mid_result(midQ, ses->server);
>>       if (rc != 0) {
>> -             add_credits(ses->server, 1);
>> +             add_credits(ses->server, 1, optype);
>>               return rc;
>>       }
>>
>> @@ -641,6 +647,8 @@ SendReceive2(const unsigned int xid, struct cifs_ses *ses,
>>       else
>>               *pRespBufType = CIFS_SMALL_BUFFER;
>>
>> +     credits = ses->server->ops->get_credits(midQ);
>> +
>>       rc = ses->server->ops->check_receive(midQ, ses->server,
>>                                            flags & CIFS_LOG_ERROR);
>>
>> @@ -649,7 +657,7 @@ SendReceive2(const unsigned int xid, struct cifs_ses *ses,
>>               midQ->resp_buf = NULL;
>>  out:
>>       delete_mid(midQ);
>> -     add_credits(ses->server, 1);
>> +     add_credits(ses->server, credits, optype);
>>
>>       return rc;
>>  }
>> @@ -685,7 +693,7 @@ SendReceive(const unsigned int xid, struct cifs_ses *ses,
>>               return -EIO;
>>       }
>>
>> -     rc = wait_for_free_request(ses->server, long_op);
>> +     rc = wait_for_free_request(ses->server, 0, long_op);
>>       if (rc)
>>               return rc;
>>
>> @@ -699,7 +707,7 @@ SendReceive(const unsigned int xid, struct cifs_ses *ses,
>>       if (rc) {
>>               mutex_unlock(&ses->server->srv_mutex);
>>               /* Update # of requests on wire to server */
>> -             add_credits(ses->server, 1);
>> +             add_credits(ses->server, 1, 0);
>>               return rc;
>>       }
>>
>> @@ -731,7 +739,7 @@ SendReceive(const unsigned int xid, struct cifs_ses *ses,
>>                       /* no longer considered to be "in-flight" */
>>                       midQ->callback = DeleteMidQEntry;
>>                       spin_unlock(&GlobalMid_Lock);
>> -                     add_credits(ses->server, 1);
>> +                     add_credits(ses->server, 1, 0);
>>                       return rc;
>>               }
>>               spin_unlock(&GlobalMid_Lock);
>> @@ -739,7 +747,7 @@ SendReceive(const unsigned int xid, struct cifs_ses *ses,
>>
>>       rc = cifs_sync_mid_result(midQ, ses->server);
>>       if (rc != 0) {
>> -             add_credits(ses->server, 1);
>> +             add_credits(ses->server, 1, 0);
>>               return rc;
>>       }
>>
>> @@ -755,7 +763,7 @@ SendReceive(const unsigned int xid, struct cifs_ses *ses,
>>       rc = cifs_check_receive(midQ, ses->server, 0);
>>  out:
>>       delete_mid(midQ);
>> -     add_credits(ses->server, 1);
>> +     add_credits(ses->server, 1, 0);
>>
>>       return rc;
>>  }
>> @@ -820,7 +828,7 @@ SendReceiveBlockingLock(const unsigned int xid, struct cifs_tcon *tcon,
>>               return -EIO;
>>       }
>>
>> -     rc = wait_for_free_request(ses->server, CIFS_BLOCKING_OP);
>> +     rc = wait_for_free_request(ses->server, 0, CIFS_BLOCKING_OP);
>>       if (rc)
>>               return rc;
>>
>
> Other than the nit about renaming that variable, this looks reasonable,
> at least until we can come up with a better scheme to handle this...
>
> Acked-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>



-- 
Best regards,
Pavel Shilovsky.

Re: [PATCH v5 2/5] CIFS: Extend credit mechanism to process request type

[Jeff Layton]

On Tue, 19 Jun 2012 23:53:06 +0400
Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org> wrote:

> 2012/6/19 Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>:
> > On Sat,  9 Jun 2012 10:26:01 +0400
> > Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org> wrote:
> >
> >> Split all requests to echos, oplocks and others - each group uses
> >> its own credit slot. This is indicated by new flags
> >>
> >> CIFS_ECHO_OP and CIFS_OBREAK_OP
> >>
> >> that are not used now for CIFS. This change is required to support
> >> SMB2 protocol because of different processing of these commands.
> >>
> >> Signed-off-by: Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
> >> ---
> >>  fs/cifs/cifsglob.h  |   16 +++++++++++---
> >>  fs/cifs/cifsproto.h |    2 +-
> >>  fs/cifs/cifssmb.c   |   21 ++++++++++---------
> >>  fs/cifs/smb1ops.c   |   12 +++++++++-
> >>  fs/cifs/transport.c |   54 +++++++++++++++++++++++++++++---------------------
> >>  5 files changed, 65 insertions(+), 40 deletions(-)
> >>
> >> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> >> index 2aac4e5..844b77c 100644
> >> --- a/fs/cifs/cifsglob.h
> >> +++ b/fs/cifs/cifsglob.h
> >> @@ -171,9 +171,11 @@ struct smb_version_operations {
> >>       /* check response: verify signature, map error */
> >>       int (*check_receive)(struct mid_q_entry *, struct TCP_Server_Info *,
> >>                            bool);
> >> -     void (*add_credits)(struct TCP_Server_Info *, const unsigned int);
> >> +     void (*add_credits)(struct TCP_Server_Info *, const unsigned int,
> >> +                         const int);
> >>       void (*set_credits)(struct TCP_Server_Info *, const int);
> >> -     int * (*get_credits_field)(struct TCP_Server_Info *);
> >> +     int * (*get_credits_field)(struct TCP_Server_Info *, const int);
> >> +     unsigned int (*get_credits)(struct mid_q_entry *);
> >>       __u64 (*get_next_mid)(struct TCP_Server_Info *);
> >>       /* data offset from read response message */
> >>       unsigned int (*read_data_offset)(char *);
> >> @@ -392,9 +394,10 @@ has_credits(struct TCP_Server_Info *server, int *credits)
> >>  }
> >>
> >>  static inline void
> >> -add_credits(struct TCP_Server_Info *server, const unsigned int add)
> >> +add_credits(struct TCP_Server_Info *server, const unsigned int add,
> >> +         const int optype)
> >>  {
> >> -     server->ops->add_credits(server, add);
> >> +     server->ops->add_credits(server, add, optype);
> >>  }
> >>
> >>  static inline void
> >> @@ -957,6 +960,11 @@ static inline void free_dfs_info_array(struct dfs_info3_param *param,
> >>  #define   CIFS_LARGE_BUF_OP 0x020    /* large request buffer */
> >>  #define   CIFS_NO_RESP      0x040    /* no response buffer required */
> >>
> >> +/* Type of request operation */
> >> +#define   CIFS_ECHO_OP      0x080    /* echo request */
> >> +#define   CIFS_OBREAK_OP   0x0100    /* oplock break request */
> >> +#define   CIFS_OP_MASK     0x0180    /* mask request type */
> >> +
> >>  /* Security Flags: indicate type of session setup needed */
> >>  #define   CIFSSEC_MAY_SIGN   0x00001
> >>  #define   CIFSSEC_MAY_NTLM   0x00002
> >> diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
> >> index 0a6cbfe..8c1c7c1 100644
> >> --- a/fs/cifs/cifsproto.h
> >> +++ b/fs/cifs/cifsproto.h
> >> @@ -71,7 +71,7 @@ extern void DeleteMidQEntry(struct mid_q_entry *midEntry);
> >>  extern int cifs_call_async(struct TCP_Server_Info *server, struct kvec *iov,
> >>                          unsigned int nvec, mid_receive_t *receive,
> >>                          mid_callback_t *callback, void *cbdata,
> >> -                        bool ignore_pend);
> >> +                        const int flags);
> >>  extern int SendReceive(const unsigned int /* xid */ , struct cifs_ses *,
> >>                       struct smb_hdr * /* input */ ,
> >>                       struct smb_hdr * /* out */ ,
> >> diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
> >> index 5b40073..58f1ab5 100644
> >> --- a/fs/cifs/cifssmb.c
> >> +++ b/fs/cifs/cifssmb.c
> >> @@ -720,7 +720,7 @@ cifs_echo_callback(struct mid_q_entry *mid)
> >>       struct TCP_Server_Info *server = mid->callback_data;
> >>
> >>       DeleteMidQEntry(mid);
> >> -     add_credits(server, 1);
> >> +     add_credits(server, 1, CIFS_ECHO_OP);
> >>  }
> >>
> >>  int
> >> @@ -747,7 +747,7 @@ CIFSSMBEcho(struct TCP_Server_Info *server)
> >>       iov.iov_len = be32_to_cpu(smb->hdr.smb_buf_length) + 4;
> >>
> >>       rc = cifs_call_async(server, &iov, 1, NULL, cifs_echo_callback,
> >> -                          server, true);
> >> +                          server, CIFS_ASYNC_OP | CIFS_ECHO_OP);
> >>       if (rc)
> >>               cFYI(1, "Echo request failed: %d", rc);
> >>
> >> @@ -1563,7 +1563,7 @@ cifs_readv_callback(struct mid_q_entry *mid)
> >>
> >>       queue_work(cifsiod_wq, &rdata->work);
> >>       DeleteMidQEntry(mid);
> >> -     add_credits(server, 1);
> >> +     add_credits(server, 1, 0);
> >>  }
> >>
> >>  /* cifs_async_readv - send an async write, and set up mid to handle result */
> >> @@ -1619,7 +1619,7 @@ cifs_async_readv(struct cifs_readdata *rdata)
> >>       kref_get(&rdata->refcount);
> >>       rc = cifs_call_async(tcon->ses->server, rdata->iov, 1,
> >>                            cifs_readv_receive, cifs_readv_callback,
> >> -                          rdata, false);
> >> +                          rdata, 0);
> >>
> >>       if (rc == 0)
> >>               cifs_stats_inc(&tcon->num_reads);
> >> @@ -2010,7 +2010,7 @@ cifs_writev_callback(struct mid_q_entry *mid)
> >>
> >>       queue_work(cifsiod_wq, &wdata->work);
> >>       DeleteMidQEntry(mid);
> >> -     add_credits(tcon->ses->server, 1);
> >> +     add_credits(tcon->ses->server, 1, 0);
> >>  }
> >>
> >>  /* cifs_async_writev - send an async write, and set up mid to handle result */
> >> @@ -2090,7 +2090,7 @@ cifs_async_writev(struct cifs_writedata *wdata)
> >>
> >>       kref_get(&wdata->refcount);
> >>       rc = cifs_call_async(tcon->ses->server, iov, wdata->nr_pages + 1,
> >> -                          NULL, cifs_writev_callback, wdata, false);
> >> +                          NULL, cifs_writev_callback, wdata, 0);
> >>
> >>       if (rc == 0)
> >>               cifs_stats_inc(&tcon->num_writes);
> >> @@ -2268,7 +2268,7 @@ CIFSSMBLock(const int xid, struct cifs_tcon *tcon,
> >>       LOCK_REQ *pSMB = NULL;
> >>  /*   LOCK_RSP *pSMBr = NULL; */ /* No response data other than rc to parse */
> >>       int bytes_returned;
> >> -     int timeout = 0;
> >> +     int flags = 0;
> >>       __u16 count;
> >>
> >>       cFYI(1, "CIFSSMBLock timeout %d numLock %d", (int)waitFlag, numLock);
> >> @@ -2278,10 +2278,11 @@ CIFSSMBLock(const int xid, struct cifs_tcon *tcon,
> >>               return rc;
> >>
> >>       if (lockType == LOCKING_ANDX_OPLOCK_RELEASE) {
> >> -             timeout = CIFS_ASYNC_OP; /* no response expected */
> >> +             /* no response expected */
> >> +             flags = CIFS_ASYNC_OP | CIFS_OBREAK_OP;
> >>               pSMB->Timeout = 0;
> >>       } else if (waitFlag) {
> >> -             timeout = CIFS_BLOCKING_OP; /* blocking operation, no timeout */
> >> +             flags = CIFS_BLOCKING_OP; /* blocking operation, no timeout */
> >>               pSMB->Timeout = cpu_to_le32(-1);/* blocking - do not time out */
> >>       } else {
> >>               pSMB->Timeout = 0;
> >> @@ -2314,7 +2315,7 @@ CIFSSMBLock(const int xid, struct cifs_tcon *tcon,
> >>                       (struct smb_hdr *) pSMB, &bytes_returned);
> >>               cifs_small_buf_release(pSMB);
> >>       } else {
> >> -             rc = SendReceiveNoRsp(xid, tcon->ses, (char *)pSMB, timeout);
> >> +             rc = SendReceiveNoRsp(xid, tcon->ses, (char *)pSMB, flags);
> >>               /* SMB buffer freed by function above */
> >>       }
> >>       cifs_stats_inc(&tcon->num_locks);
> >> diff --git a/fs/cifs/smb1ops.c b/fs/cifs/smb1ops.c
> >> index 28359e7..f4f8394 100644
> >> --- a/fs/cifs/smb1ops.c
> >> +++ b/fs/cifs/smb1ops.c
> >> @@ -101,7 +101,8 @@ cifs_find_mid(struct TCP_Server_Info *server, char *buffer)
> >>  }
> >>
> >>  static void
> >> -cifs_add_credits(struct TCP_Server_Info *server, const unsigned int add)
> >> +cifs_add_credits(struct TCP_Server_Info *server, const unsigned int add,
> >> +              const int optype)
> >>  {
> >>       spin_lock(&server->req_lock);
> >>       server->credits += add;
> >> @@ -120,11 +121,17 @@ cifs_set_credits(struct TCP_Server_Info *server, const int val)
> >>  }
> >>
> >>  static int *
> >> -cifs_get_credits_field(struct TCP_Server_Info *server)
> >> +cifs_get_credits_field(struct TCP_Server_Info *server, const int optype)
> >>  {
> >>       return &server->credits;
> >>  }
> >>
> >> +static unsigned int
> >> +cifs_get_credits(struct mid_q_entry *mid)
> >> +{
> >> +     return 1;
> >> +}
> >> +
> >>  /*
> >>   * Find a free multiplex id (SMB mid). Otherwise there could be
> >>   * mid collisions which might cause problems, demultiplexing the
> >> @@ -390,6 +397,7 @@ struct smb_version_operations smb1_operations = {
> >>       .add_credits = cifs_add_credits,
> >>       .set_credits = cifs_set_credits,
> >>       .get_credits_field = cifs_get_credits_field,
> >> +     .get_credits = cifs_get_credits,
> >>       .get_next_mid = cifs_get_next_mid,
> >>       .read_data_offset = cifs_read_data_offset,
> >>       .read_data_length = cifs_read_data_length,
> >> diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c
> >> index 3097ee5..54cd8dd 100644
> >> --- a/fs/cifs/transport.c
> >> +++ b/fs/cifs/transport.c
> >> @@ -254,13 +254,13 @@ smb_send(struct TCP_Server_Info *server, struct smb_hdr *smb_buffer,
> >>  }
> >>
> >>  static int
> >> -wait_for_free_credits(struct TCP_Server_Info *server, const int optype,
> >> +wait_for_free_credits(struct TCP_Server_Info *server, const int long_op,
> >>                     int *credits)
> >>  {
> >>       int rc;
> >>
> >>       spin_lock(&server->req_lock);
> >> -     if (optype == CIFS_ASYNC_OP) {
> >> +     if (long_op == CIFS_ASYNC_OP) {
> >>               /* oplock breaks must not be held up */
> >>               server->in_flight++;
> >>               *credits -= 1;
> >> @@ -290,7 +290,7 @@ wait_for_free_credits(struct TCP_Server_Info *server, const int optype,
> >>                        */
> >>
> >>                       /* update # of requests on the wire to server */
> >> -                     if (optype != CIFS_BLOCKING_OP) {
> >> +                     if (long_op != CIFS_BLOCKING_OP) {
> >>                               *credits -= 1;
> >>                               server->in_flight++;
> >>                       }
> >
> > Why rename the variable in this above function? "optype" seems like a better name...
> 
> That was done to make variable names the same: 'optype' is used to
> indicate if it is echo, oplock or other operation, 'long_op' is used
> to indicate if we need any special behavior (aysnc, blocking).
> 
> wait_for_free_request function (below) takes both 'optype' and
> 'long_op' and passes 'long_op' to wait_for_free_credits. It is cleaner
> if wait_for_free_credits uses the same name - 'long_op'.
> 
> Of course, 'long_op' may be not so good name at all (I took it from
> the old code) - suggestions are welcome.
> 

Yeah, I think that's the confusion. "long_op" was from when those flags
were supposed to indicate the length of the operation so the code could
adjust the timeout. Maybe a better name is something generic like
"flags" ?

-- 
Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

Re: [PATCH v5 2/5] CIFS: Extend credit mechanism to process request type

[Steve French]

On Wed, Jun 20, 2012 at 12:00 AM, Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> On Tue, 19 Jun 2012 23:53:06 +0400
> Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org> wrote:
>
>> 2012/6/19 Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>:
>> > On Sat,  9 Jun 2012 10:26:01 +0400
>> > Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org> wrote:
>> >
>> >> Split all requests to echos, oplocks and others - each group uses
>> >> its own credit slot. This is indicated by new flags
>> >>
>> >> CIFS_ECHO_OP and CIFS_OBREAK_OP
>> >>
>> >> that are not used now for CIFS. This change is required to support
>> >> SMB2 protocol because of different processing of these commands.
>> >>
>> >> Signed-off-by: Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
>> >> ---
>> >>  fs/cifs/cifsglob.h  |   16 +++++++++++---
>> >>  fs/cifs/cifsproto.h |    2 +-
>> >>  fs/cifs/cifssmb.c   |   21 ++++++++++---------
>> >>  fs/cifs/smb1ops.c   |   12 +++++++++-
>> >>  fs/cifs/transport.c |   54 +++++++++++++++++++++++++++++---------------------
>> >>  5 files changed, 65 insertions(+), 40 deletions(-)
>> >>
>> >> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
>> >> index 2aac4e5..844b77c 100644
>> >> --- a/fs/cifs/cifsglob.h
>> >> +++ b/fs/cifs/cifsglob.h
>> >> @@ -171,9 +171,11 @@ struct smb_version_operations {
>> >>       /* check response: verify signature, map error */
>> >>       int (*check_receive)(struct mid_q_entry *, struct TCP_Server_Info *,
>> >>                            bool);
>> >> -     void (*add_credits)(struct TCP_Server_Info *, const unsigned int);
>> >> +     void (*add_credits)(struct TCP_Server_Info *, const unsigned int,
>> >> +                         const int);
>> >>       void (*set_credits)(struct TCP_Server_Info *, const int);
>> >> -     int * (*get_credits_field)(struct TCP_Server_Info *);
>> >> +     int * (*get_credits_field)(struct TCP_Server_Info *, const int);
>> >> +     unsigned int (*get_credits)(struct mid_q_entry *);
>> >>       __u64 (*get_next_mid)(struct TCP_Server_Info *);
>> >>       /* data offset from read response message */
>> >>       unsigned int (*read_data_offset)(char *);
>> >> @@ -392,9 +394,10 @@ has_credits(struct TCP_Server_Info *server, int *credits)
>> >>  }
>> >>
>> >>  static inline void
>> >> -add_credits(struct TCP_Server_Info *server, const unsigned int add)
>> >> +add_credits(struct TCP_Server_Info *server, const unsigned int add,
>> >> +         const int optype)
>> >>  {
>> >> -     server->ops->add_credits(server, add);
>> >> +     server->ops->add_credits(server, add, optype);
>> >>  }
>> >>
>> >>  static inline void
>> >> @@ -957,6 +960,11 @@ static inline void free_dfs_info_array(struct dfs_info3_param *param,
>> >>  #define   CIFS_LARGE_BUF_OP 0x020    /* large request buffer */
>> >>  #define   CIFS_NO_RESP      0x040    /* no response buffer required */
>> >>
>> >> +/* Type of request operation */
>> >> +#define   CIFS_ECHO_OP      0x080    /* echo request */
>> >> +#define   CIFS_OBREAK_OP   0x0100    /* oplock break request */
>> >> +#define   CIFS_OP_MASK     0x0180    /* mask request type */
>> >> +
>> >>  /* Security Flags: indicate type of session setup needed */
>> >>  #define   CIFSSEC_MAY_SIGN   0x00001
>> >>  #define   CIFSSEC_MAY_NTLM   0x00002
>> >> diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
>> >> index 0a6cbfe..8c1c7c1 100644
>> >> --- a/fs/cifs/cifsproto.h
>> >> +++ b/fs/cifs/cifsproto.h
>> >> @@ -71,7 +71,7 @@ extern void DeleteMidQEntry(struct mid_q_entry *midEntry);
>> >>  extern int cifs_call_async(struct TCP_Server_Info *server, struct kvec *iov,
>> >>                          unsigned int nvec, mid_receive_t *receive,
>> >>                          mid_callback_t *callback, void *cbdata,
>> >> -                        bool ignore_pend);
>> >> +                        const int flags);
>> >>  extern int SendReceive(const unsigned int /* xid */ , struct cifs_ses *,
>> >>                       struct smb_hdr * /* input */ ,
>> >>                       struct smb_hdr * /* out */ ,
>> >> diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
>> >> index 5b40073..58f1ab5 100644
>> >> --- a/fs/cifs/cifssmb.c
>> >> +++ b/fs/cifs/cifssmb.c
>> >> @@ -720,7 +720,7 @@ cifs_echo_callback(struct mid_q_entry *mid)
>> >>       struct TCP_Server_Info *server = mid->callback_data;
>> >>
>> >>       DeleteMidQEntry(mid);
>> >> -     add_credits(server, 1);
>> >> +     add_credits(server, 1, CIFS_ECHO_OP);
>> >>  }
>> >>
>> >>  int
>> >> @@ -747,7 +747,7 @@ CIFSSMBEcho(struct TCP_Server_Info *server)
>> >>       iov.iov_len = be32_to_cpu(smb->hdr.smb_buf_length) + 4;
>> >>
>> >>       rc = cifs_call_async(server, &iov, 1, NULL, cifs_echo_callback,
>> >> -                          server, true);
>> >> +                          server, CIFS_ASYNC_OP | CIFS_ECHO_OP);
>> >>       if (rc)
>> >>               cFYI(1, "Echo request failed: %d", rc);
>> >>
>> >> @@ -1563,7 +1563,7 @@ cifs_readv_callback(struct mid_q_entry *mid)
>> >>
>> >>       queue_work(cifsiod_wq, &rdata->work);
>> >>       DeleteMidQEntry(mid);
>> >> -     add_credits(server, 1);
>> >> +     add_credits(server, 1, 0);
>> >>  }
>> >>
>> >>  /* cifs_async_readv - send an async write, and set up mid to handle result */
>> >> @@ -1619,7 +1619,7 @@ cifs_async_readv(struct cifs_readdata *rdata)
>> >>       kref_get(&rdata->refcount);
>> >>       rc = cifs_call_async(tcon->ses->server, rdata->iov, 1,
>> >>                            cifs_readv_receive, cifs_readv_callback,
>> >> -                          rdata, false);
>> >> +                          rdata, 0);
>> >>
>> >>       if (rc == 0)
>> >>               cifs_stats_inc(&tcon->num_reads);
>> >> @@ -2010,7 +2010,7 @@ cifs_writev_callback(struct mid_q_entry *mid)
>> >>
>> >>       queue_work(cifsiod_wq, &wdata->work);
>> >>       DeleteMidQEntry(mid);
>> >> -     add_credits(tcon->ses->server, 1);
>> >> +     add_credits(tcon->ses->server, 1, 0);
>> >>  }
>> >>
>> >>  /* cifs_async_writev - send an async write, and set up mid to handle result */
>> >> @@ -2090,7 +2090,7 @@ cifs_async_writev(struct cifs_writedata *wdata)
>> >>
>> >>       kref_get(&wdata->refcount);
>> >>       rc = cifs_call_async(tcon->ses->server, iov, wdata->nr_pages + 1,
>> >> -                          NULL, cifs_writev_callback, wdata, false);
>> >> +                          NULL, cifs_writev_callback, wdata, 0);
>> >>
>> >>       if (rc == 0)
>> >>               cifs_stats_inc(&tcon->num_writes);
>> >> @@ -2268,7 +2268,7 @@ CIFSSMBLock(const int xid, struct cifs_tcon *tcon,
>> >>       LOCK_REQ *pSMB = NULL;
>> >>  /*   LOCK_RSP *pSMBr = NULL; */ /* No response data other than rc to parse */
>> >>       int bytes_returned;
>> >> -     int timeout = 0;
>> >> +     int flags = 0;
>> >>       __u16 count;
>> >>
>> >>       cFYI(1, "CIFSSMBLock timeout %d numLock %d", (int)waitFlag, numLock);
>> >> @@ -2278,10 +2278,11 @@ CIFSSMBLock(const int xid, struct cifs_tcon *tcon,
>> >>               return rc;
>> >>
>> >>       if (lockType == LOCKING_ANDX_OPLOCK_RELEASE) {
>> >> -             timeout = CIFS_ASYNC_OP; /* no response expected */
>> >> +             /* no response expected */
>> >> +             flags = CIFS_ASYNC_OP | CIFS_OBREAK_OP;
>> >>               pSMB->Timeout = 0;
>> >>       } else if (waitFlag) {
>> >> -             timeout = CIFS_BLOCKING_OP; /* blocking operation, no timeout */
>> >> +             flags = CIFS_BLOCKING_OP; /* blocking operation, no timeout */
>> >>               pSMB->Timeout = cpu_to_le32(-1);/* blocking - do not time out */
>> >>       } else {
>> >>               pSMB->Timeout = 0;
>> >> @@ -2314,7 +2315,7 @@ CIFSSMBLock(const int xid, struct cifs_tcon *tcon,
>> >>                       (struct smb_hdr *) pSMB, &bytes_returned);
>> >>               cifs_small_buf_release(pSMB);
>> >>       } else {
>> >> -             rc = SendReceiveNoRsp(xid, tcon->ses, (char *)pSMB, timeout);
>> >> +             rc = SendReceiveNoRsp(xid, tcon->ses, (char *)pSMB, flags);
>> >>               /* SMB buffer freed by function above */
>> >>       }
>> >>       cifs_stats_inc(&tcon->num_locks);
>> >> diff --git a/fs/cifs/smb1ops.c b/fs/cifs/smb1ops.c
>> >> index 28359e7..f4f8394 100644
>> >> --- a/fs/cifs/smb1ops.c
>> >> +++ b/fs/cifs/smb1ops.c
>> >> @@ -101,7 +101,8 @@ cifs_find_mid(struct TCP_Server_Info *server, char *buffer)
>> >>  }
>> >>
>> >>  static void
>> >> -cifs_add_credits(struct TCP_Server_Info *server, const unsigned int add)
>> >> +cifs_add_credits(struct TCP_Server_Info *server, const unsigned int add,
>> >> +              const int optype)
>> >>  {
>> >>       spin_lock(&server->req_lock);
>> >>       server->credits += add;
>> >> @@ -120,11 +121,17 @@ cifs_set_credits(struct TCP_Server_Info *server, const int val)
>> >>  }
>> >>
>> >>  static int *
>> >> -cifs_get_credits_field(struct TCP_Server_Info *server)
>> >> +cifs_get_credits_field(struct TCP_Server_Info *server, const int optype)
>> >>  {
>> >>       return &server->credits;
>> >>  }
>> >>
>> >> +static unsigned int
>> >> +cifs_get_credits(struct mid_q_entry *mid)
>> >> +{
>> >> +     return 1;
>> >> +}
>> >> +
>> >>  /*
>> >>   * Find a free multiplex id (SMB mid). Otherwise there could be
>> >>   * mid collisions which might cause problems, demultiplexing the
>> >> @@ -390,6 +397,7 @@ struct smb_version_operations smb1_operations = {
>> >>       .add_credits = cifs_add_credits,
>> >>       .set_credits = cifs_set_credits,
>> >>       .get_credits_field = cifs_get_credits_field,
>> >> +     .get_credits = cifs_get_credits,
>> >>       .get_next_mid = cifs_get_next_mid,
>> >>       .read_data_offset = cifs_read_data_offset,
>> >>       .read_data_length = cifs_read_data_length,
>> >> diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c
>> >> index 3097ee5..54cd8dd 100644
>> >> --- a/fs/cifs/transport.c
>> >> +++ b/fs/cifs/transport.c
>> >> @@ -254,13 +254,13 @@ smb_send(struct TCP_Server_Info *server, struct smb_hdr *smb_buffer,
>> >>  }
>> >>
>> >>  static int
>> >> -wait_for_free_credits(struct TCP_Server_Info *server, const int optype,
>> >> +wait_for_free_credits(struct TCP_Server_Info *server, const int long_op,
>> >>                     int *credits)
>> >>  {
>> >>       int rc;
>> >>
>> >>       spin_lock(&server->req_lock);
>> >> -     if (optype == CIFS_ASYNC_OP) {
>> >> +     if (long_op == CIFS_ASYNC_OP) {
>> >>               /* oplock breaks must not be held up */
>> >>               server->in_flight++;
>> >>               *credits -= 1;
>> >> @@ -290,7 +290,7 @@ wait_for_free_credits(struct TCP_Server_Info *server, const int optype,
>> >>                        */
>> >>
>> >>                       /* update # of requests on the wire to server */
>> >> -                     if (optype != CIFS_BLOCKING_OP) {
>> >> +                     if (long_op != CIFS_BLOCKING_OP) {
>> >>                               *credits -= 1;
>> >>                               server->in_flight++;
>> >>                       }
>> >
>> > Why rename the variable in this above function? "optype" seems like a better name...
>>
>> That was done to make variable names the same: 'optype' is used to
>> indicate if it is echo, oplock or other operation, 'long_op' is used
>> to indicate if we need any special behavior (aysnc, blocking).
>>
>> wait_for_free_request function (below) takes both 'optype' and
>> 'long_op' and passes 'long_op' to wait_for_free_credits. It is cleaner
>> if wait_for_free_credits uses the same name - 'long_op'.
>>
>> Of course, 'long_op' may be not so good name at all (I took it from
>> the old code) - suggestions are welcome.
>>
>
> Yeah, I think that's the confusion. "long_op" was from when those flags
> were supposed to indicate the length of the operation so the code could
> adjust the timeout. Maybe a better name is something generic like
> "flags" ?

"flags" is fine with me

-- 
Thanks,

Steve

Re: [PATCH v5 2/5] CIFS: Extend credit mechanism to process request type

[Pavel Shilovsky]

2012/6/20 Steve French <smfrench-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>:
> On Wed, Jun 20, 2012 at 12:00 AM, Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
>> On Tue, 19 Jun 2012 23:53:06 +0400
>> Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org> wrote:
>>
>>> 2012/6/19 Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>:
>>> > On Sat,  9 Jun 2012 10:26:01 +0400
>>> > Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org> wrote:
>>> >
>>> >> Split all requests to echos, oplocks and others - each group uses
>>> >> its own credit slot. This is indicated by new flags
>>> >>
>>> >> CIFS_ECHO_OP and CIFS_OBREAK_OP
>>> >>
>>> >> that are not used now for CIFS. This change is required to support
>>> >> SMB2 protocol because of different processing of these commands.
>>> >>
>>> >> Signed-off-by: Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
>>> >> ---
>>> >>  fs/cifs/cifsglob.h  |   16 +++++++++++---
>>> >>  fs/cifs/cifsproto.h |    2 +-
>>> >>  fs/cifs/cifssmb.c   |   21 ++++++++++---------
>>> >>  fs/cifs/smb1ops.c   |   12 +++++++++-
>>> >>  fs/cifs/transport.c |   54 +++++++++++++++++++++++++++++---------------------
>>> >>  5 files changed, 65 insertions(+), 40 deletions(-)
>>> >>
>>> >> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
>>> >> index 2aac4e5..844b77c 100644
>>> >> --- a/fs/cifs/cifsglob.h
>>> >> +++ b/fs/cifs/cifsglob.h
>>> >> @@ -171,9 +171,11 @@ struct smb_version_operations {
>>> >>       /* check response: verify signature, map error */
>>> >>       int (*check_receive)(struct mid_q_entry *, struct TCP_Server_Info *,
>>> >>                            bool);
>>> >> -     void (*add_credits)(struct TCP_Server_Info *, const unsigned int);
>>> >> +     void (*add_credits)(struct TCP_Server_Info *, const unsigned int,
>>> >> +                         const int);
>>> >>       void (*set_credits)(struct TCP_Server_Info *, const int);
>>> >> -     int * (*get_credits_field)(struct TCP_Server_Info *);
>>> >> +     int * (*get_credits_field)(struct TCP_Server_Info *, const int);
>>> >> +     unsigned int (*get_credits)(struct mid_q_entry *);
>>> >>       __u64 (*get_next_mid)(struct TCP_Server_Info *);
>>> >>       /* data offset from read response message */
>>> >>       unsigned int (*read_data_offset)(char *);
>>> >> @@ -392,9 +394,10 @@ has_credits(struct TCP_Server_Info *server, int *credits)
>>> >>  }
>>> >>
>>> >>  static inline void
>>> >> -add_credits(struct TCP_Server_Info *server, const unsigned int add)
>>> >> +add_credits(struct TCP_Server_Info *server, const unsigned int add,
>>> >> +         const int optype)
>>> >>  {
>>> >> -     server->ops->add_credits(server, add);
>>> >> +     server->ops->add_credits(server, add, optype);
>>> >>  }
>>> >>
>>> >>  static inline void
>>> >> @@ -957,6 +960,11 @@ static inline void free_dfs_info_array(struct dfs_info3_param *param,
>>> >>  #define   CIFS_LARGE_BUF_OP 0x020    /* large request buffer */
>>> >>  #define   CIFS_NO_RESP      0x040    /* no response buffer required */
>>> >>
>>> >> +/* Type of request operation */
>>> >> +#define   CIFS_ECHO_OP      0x080    /* echo request */
>>> >> +#define   CIFS_OBREAK_OP   0x0100    /* oplock break request */
>>> >> +#define   CIFS_OP_MASK     0x0180    /* mask request type */
>>> >> +
>>> >>  /* Security Flags: indicate type of session setup needed */
>>> >>  #define   CIFSSEC_MAY_SIGN   0x00001
>>> >>  #define   CIFSSEC_MAY_NTLM   0x00002
>>> >> diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
>>> >> index 0a6cbfe..8c1c7c1 100644
>>> >> --- a/fs/cifs/cifsproto.h
>>> >> +++ b/fs/cifs/cifsproto.h
>>> >> @@ -71,7 +71,7 @@ extern void DeleteMidQEntry(struct mid_q_entry *midEntry);
>>> >>  extern int cifs_call_async(struct TCP_Server_Info *server, struct kvec *iov,
>>> >>                          unsigned int nvec, mid_receive_t *receive,
>>> >>                          mid_callback_t *callback, void *cbdata,
>>> >> -                        bool ignore_pend);
>>> >> +                        const int flags);
>>> >>  extern int SendReceive(const unsigned int /* xid */ , struct cifs_ses *,
>>> >>                       struct smb_hdr * /* input */ ,
>>> >>                       struct smb_hdr * /* out */ ,
>>> >> diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
>>> >> index 5b40073..58f1ab5 100644
>>> >> --- a/fs/cifs/cifssmb.c
>>> >> +++ b/fs/cifs/cifssmb.c
>>> >> @@ -720,7 +720,7 @@ cifs_echo_callback(struct mid_q_entry *mid)
>>> >>       struct TCP_Server_Info *server = mid->callback_data;
>>> >>
>>> >>       DeleteMidQEntry(mid);
>>> >> -     add_credits(server, 1);
>>> >> +     add_credits(server, 1, CIFS_ECHO_OP);
>>> >>  }
>>> >>
>>> >>  int
>>> >> @@ -747,7 +747,7 @@ CIFSSMBEcho(struct TCP_Server_Info *server)
>>> >>       iov.iov_len = be32_to_cpu(smb->hdr.smb_buf_length) + 4;
>>> >>
>>> >>       rc = cifs_call_async(server, &iov, 1, NULL, cifs_echo_callback,
>>> >> -                          server, true);
>>> >> +                          server, CIFS_ASYNC_OP | CIFS_ECHO_OP);
>>> >>       if (rc)
>>> >>               cFYI(1, "Echo request failed: %d", rc);
>>> >>
>>> >> @@ -1563,7 +1563,7 @@ cifs_readv_callback(struct mid_q_entry *mid)
>>> >>
>>> >>       queue_work(cifsiod_wq, &rdata->work);
>>> >>       DeleteMidQEntry(mid);
>>> >> -     add_credits(server, 1);
>>> >> +     add_credits(server, 1, 0);
>>> >>  }
>>> >>
>>> >>  /* cifs_async_readv - send an async write, and set up mid to handle result */
>>> >> @@ -1619,7 +1619,7 @@ cifs_async_readv(struct cifs_readdata *rdata)
>>> >>       kref_get(&rdata->refcount);
>>> >>       rc = cifs_call_async(tcon->ses->server, rdata->iov, 1,
>>> >>                            cifs_readv_receive, cifs_readv_callback,
>>> >> -                          rdata, false);
>>> >> +                          rdata, 0);
>>> >>
>>> >>       if (rc == 0)
>>> >>               cifs_stats_inc(&tcon->num_reads);
>>> >> @@ -2010,7 +2010,7 @@ cifs_writev_callback(struct mid_q_entry *mid)
>>> >>
>>> >>       queue_work(cifsiod_wq, &wdata->work);
>>> >>       DeleteMidQEntry(mid);
>>> >> -     add_credits(tcon->ses->server, 1);
>>> >> +     add_credits(tcon->ses->server, 1, 0);
>>> >>  }
>>> >>
>>> >>  /* cifs_async_writev - send an async write, and set up mid to handle result */
>>> >> @@ -2090,7 +2090,7 @@ cifs_async_writev(struct cifs_writedata *wdata)
>>> >>
>>> >>       kref_get(&wdata->refcount);
>>> >>       rc = cifs_call_async(tcon->ses->server, iov, wdata->nr_pages + 1,
>>> >> -                          NULL, cifs_writev_callback, wdata, false);
>>> >> +                          NULL, cifs_writev_callback, wdata, 0);
>>> >>
>>> >>       if (rc == 0)
>>> >>               cifs_stats_inc(&tcon->num_writes);
>>> >> @@ -2268,7 +2268,7 @@ CIFSSMBLock(const int xid, struct cifs_tcon *tcon,
>>> >>       LOCK_REQ *pSMB = NULL;
>>> >>  /*   LOCK_RSP *pSMBr = NULL; */ /* No response data other than rc to parse */
>>> >>       int bytes_returned;
>>> >> -     int timeout = 0;
>>> >> +     int flags = 0;
>>> >>       __u16 count;
>>> >>
>>> >>       cFYI(1, "CIFSSMBLock timeout %d numLock %d", (int)waitFlag, numLock);
>>> >> @@ -2278,10 +2278,11 @@ CIFSSMBLock(const int xid, struct cifs_tcon *tcon,
>>> >>               return rc;
>>> >>
>>> >>       if (lockType == LOCKING_ANDX_OPLOCK_RELEASE) {
>>> >> -             timeout = CIFS_ASYNC_OP; /* no response expected */
>>> >> +             /* no response expected */
>>> >> +             flags = CIFS_ASYNC_OP | CIFS_OBREAK_OP;
>>> >>               pSMB->Timeout = 0;
>>> >>       } else if (waitFlag) {
>>> >> -             timeout = CIFS_BLOCKING_OP; /* blocking operation, no timeout */
>>> >> +             flags = CIFS_BLOCKING_OP; /* blocking operation, no timeout */
>>> >>               pSMB->Timeout = cpu_to_le32(-1);/* blocking - do not time out */
>>> >>       } else {
>>> >>               pSMB->Timeout = 0;
>>> >> @@ -2314,7 +2315,7 @@ CIFSSMBLock(const int xid, struct cifs_tcon *tcon,
>>> >>                       (struct smb_hdr *) pSMB, &bytes_returned);
>>> >>               cifs_small_buf_release(pSMB);
>>> >>       } else {
>>> >> -             rc = SendReceiveNoRsp(xid, tcon->ses, (char *)pSMB, timeout);
>>> >> +             rc = SendReceiveNoRsp(xid, tcon->ses, (char *)pSMB, flags);
>>> >>               /* SMB buffer freed by function above */
>>> >>       }
>>> >>       cifs_stats_inc(&tcon->num_locks);
>>> >> diff --git a/fs/cifs/smb1ops.c b/fs/cifs/smb1ops.c
>>> >> index 28359e7..f4f8394 100644
>>> >> --- a/fs/cifs/smb1ops.c
>>> >> +++ b/fs/cifs/smb1ops.c
>>> >> @@ -101,7 +101,8 @@ cifs_find_mid(struct TCP_Server_Info *server, char *buffer)
>>> >>  }
>>> >>
>>> >>  static void
>>> >> -cifs_add_credits(struct TCP_Server_Info *server, const unsigned int add)
>>> >> +cifs_add_credits(struct TCP_Server_Info *server, const unsigned int add,
>>> >> +              const int optype)
>>> >>  {
>>> >>       spin_lock(&server->req_lock);
>>> >>       server->credits += add;
>>> >> @@ -120,11 +121,17 @@ cifs_set_credits(struct TCP_Server_Info *server, const int val)
>>> >>  }
>>> >>
>>> >>  static int *
>>> >> -cifs_get_credits_field(struct TCP_Server_Info *server)
>>> >> +cifs_get_credits_field(struct TCP_Server_Info *server, const int optype)
>>> >>  {
>>> >>       return &server->credits;
>>> >>  }
>>> >>
>>> >> +static unsigned int
>>> >> +cifs_get_credits(struct mid_q_entry *mid)
>>> >> +{
>>> >> +     return 1;
>>> >> +}
>>> >> +
>>> >>  /*
>>> >>   * Find a free multiplex id (SMB mid). Otherwise there could be
>>> >>   * mid collisions which might cause problems, demultiplexing the
>>> >> @@ -390,6 +397,7 @@ struct smb_version_operations smb1_operations = {
>>> >>       .add_credits = cifs_add_credits,
>>> >>       .set_credits = cifs_set_credits,
>>> >>       .get_credits_field = cifs_get_credits_field,
>>> >> +     .get_credits = cifs_get_credits,
>>> >>       .get_next_mid = cifs_get_next_mid,
>>> >>       .read_data_offset = cifs_read_data_offset,
>>> >>       .read_data_length = cifs_read_data_length,
>>> >> diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c
>>> >> index 3097ee5..54cd8dd 100644
>>> >> --- a/fs/cifs/transport.c
>>> >> +++ b/fs/cifs/transport.c
>>> >> @@ -254,13 +254,13 @@ smb_send(struct TCP_Server_Info *server, struct smb_hdr *smb_buffer,
>>> >>  }
>>> >>
>>> >>  static int
>>> >> -wait_for_free_credits(struct TCP_Server_Info *server, const int optype,
>>> >> +wait_for_free_credits(struct TCP_Server_Info *server, const int long_op,
>>> >>                     int *credits)
>>> >>  {
>>> >>       int rc;
>>> >>
>>> >>       spin_lock(&server->req_lock);
>>> >> -     if (optype == CIFS_ASYNC_OP) {
>>> >> +     if (long_op == CIFS_ASYNC_OP) {
>>> >>               /* oplock breaks must not be held up */
>>> >>               server->in_flight++;
>>> >>               *credits -= 1;
>>> >> @@ -290,7 +290,7 @@ wait_for_free_credits(struct TCP_Server_Info *server, const int optype,
>>> >>                        */
>>> >>
>>> >>                       /* update # of requests on the wire to server */
>>> >> -                     if (optype != CIFS_BLOCKING_OP) {
>>> >> +                     if (long_op != CIFS_BLOCKING_OP) {
>>> >>                               *credits -= 1;
>>> >>                               server->in_flight++;
>>> >>                       }
>>> >
>>> > Why rename the variable in this above function? "optype" seems like a better name...
>>>
>>> That was done to make variable names the same: 'optype' is used to
>>> indicate if it is echo, oplock or other operation, 'long_op' is used
>>> to indicate if we need any special behavior (aysnc, blocking).
>>>
>>> wait_for_free_request function (below) takes both 'optype' and
>>> 'long_op' and passes 'long_op' to wait_for_free_credits. It is cleaner
>>> if wait_for_free_credits uses the same name - 'long_op'.
>>>
>>> Of course, 'long_op' may be not so good name at all (I took it from
>>> the old code) - suggestions are welcome.
>>>
>>
>> Yeah, I think that's the confusion. "long_op" was from when those flags
>> were supposed to indicate the length of the operation so the code could
>> adjust the timeout. Maybe a better name is something generic like
>> "flags" ?
>
> "flags" is fine with me

'flags' is suppose to indicate all request flags (credit slot,
timeout, etc). As soon as we mask CIFS_ASYNC_OP and CIFS_BLOCKING_OP
with CIFS_TIMEOUT_MASK macro we should use something close to it: e.g.
optimeout, timeout, etc.

I think we can rethink names of mask macro and variables further -
let's not stop on this.

-- 
Best regards,
Pavel Shilovsky.