[PATCH 0/5] Move protocol specific transport code to ops struct

[PATCH 0/5] Move protocol specific transport code to ops struct

[Pavel Shilovsky]

Here is yet another patchset that is targeted to make existing cifs code
more protocol independent and let us add new protocols easily further.

It can be merge as soons as got reviewed and ok for 3.5.

Pavel Shilovsky (5):
  CIFS: Move protocol specific part from SendReceive2 to ops struct
  CIFS: Move header_size/max_header_size to ops structure
  CIFS: Move protocol specific part from cifs_readv_receive to ops
    struct
  CIFS: Move protocol specific demultiplex thread calls to ops struct
  CIFS: Move add/set_credits and get_credits_field to ops structure

 fs/cifs/cifsglob.h  |   45 +++++++++++++++++++----------
 fs/cifs/cifsproto.h |    5 +--
 fs/cifs/cifssmb.c   |   39 ++++++-------------------
 fs/cifs/connect.c   |   44 ++++++++--------------------
 fs/cifs/misc.c      |   19 ------------
 fs/cifs/smb1ops.c   |   78 +++++++++++++++++++++++++++++++++++++++++++++++++++
 fs/cifs/transport.c |   10 ++++--
 7 files changed, 138 insertions(+), 102 deletions(-)

[PATCH 1/5] CIFS: Move protocol specific part from SendReceive2 to ops struct

[Pavel Shilovsky]

Signed-off-by: Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>
---
 fs/cifs/cifsglob.h  |    5 +++++
 fs/cifs/cifsproto.h |    2 ++
 fs/cifs/smb1ops.c   |    3 +++
 fs/cifs/transport.c |    7 ++++---
 4 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index b6e97f8..ff06211 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -157,11 +157,16 @@ enum smb_version {
 struct mid_q_entry;
 struct TCP_Server_Info;
 struct cifsFileInfo;
+struct cifs_ses;
 
 struct smb_version_operations {
 	int (*send_cancel)(struct TCP_Server_Info *, void *,
 			   struct mid_q_entry *);
 	bool (*compare_fids)(struct cifsFileInfo *, struct cifsFileInfo *);
+	int (*setup_request)(struct cifs_ses *, struct kvec *, unsigned int,
+			     struct mid_q_entry **);
+	int (*check_receive)(struct mid_q_entry *, struct TCP_Server_Info *,
+			     bool);
 };
 
 struct smb_version_values {
diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
index 0a3fa96..57af642 100644
--- a/fs/cifs/cifsproto.h
+++ b/fs/cifs/cifsproto.h
@@ -78,6 +78,8 @@ extern int SendReceive(const unsigned int /* xid */ , struct cifs_ses *,
 			int * /* bytes returned */ , const int long_op);
 extern int SendReceiveNoRsp(const unsigned int xid, struct cifs_ses *ses,
 			    char *in_buf, int flags);
+extern int cifs_setup_request(struct cifs_ses *, struct kvec *, unsigned int,
+			      struct mid_q_entry **);
 extern int cifs_check_receive(struct mid_q_entry *mid,
 			struct TCP_Server_Info *server, bool log_error);
 extern int SendReceive2(const unsigned int /* xid */ , struct cifs_ses *,
diff --git a/fs/cifs/smb1ops.c b/fs/cifs/smb1ops.c
index ce27f86..3b7cf89 100644
--- a/fs/cifs/smb1ops.c
+++ b/fs/cifs/smb1ops.c
@@ -21,6 +21,7 @@
 #include "cifsproto.h"
 #include "cifs_debug.h"
 #include "cifspdu.h"
+#include "cifsproto.h"
 
 /*
  * An NT cancel request header looks just like the original request except:
@@ -69,6 +70,8 @@ cifs_compare_fids(struct cifsFileInfo *ob1, struct cifsFileInfo *ob2)
 struct smb_version_operations smb1_operations = {
 	.send_cancel = send_nt_cancel,
 	.compare_fids = cifs_compare_fids,
+	.setup_request = cifs_setup_request,
+	.check_receive = cifs_check_receive,
 };
 
 struct smb_version_values smb1_values = {
diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c
index 269a5a7..87bd998 100644
--- a/fs/cifs/transport.c
+++ b/fs/cifs/transport.c
@@ -514,7 +514,7 @@ cifs_check_receive(struct mid_q_entry *mid, struct TCP_Server_Info *server,
 	return map_smb_to_linux_error(mid->resp_buf, log_error);
 }
 
-static int
+int
 cifs_setup_request(struct cifs_ses *ses, struct kvec *iov,
 		   unsigned int nvec, struct mid_q_entry **ret_mid)
 {
@@ -577,7 +577,7 @@ SendReceive2(const unsigned int xid, struct cifs_ses *ses,
 
 	mutex_lock(&ses->server->srv_mutex);
 
-	rc = cifs_setup_request(ses, iov, n_vec, &midQ);
+	rc = ses->server->ops->setup_request(ses, iov, n_vec, &midQ);
 	if (rc) {
 		mutex_unlock(&ses->server->srv_mutex);
 		cifs_small_buf_release(buf);
@@ -640,7 +640,8 @@ SendReceive2(const unsigned int xid, struct cifs_ses *ses,
 	else
 		*pRespBufType = CIFS_SMALL_BUFFER;
 
-	rc = cifs_check_receive(midQ, ses->server, flags & CIFS_LOG_ERROR);
+	rc = ses->server->ops->check_receive(midQ, ses->server,
+					     flags & CIFS_LOG_ERROR);
 
 	/* mark it so buf will not be freed by delete_mid */
 	if ((flags & CIFS_NO_RESP) == 0)
-- 
1.7.1

Re: [PATCH 1/5] CIFS: Move protocol specific part from SendReceive2 to ops struct

[Shirish Pargaonkar]

On Thu, May 17, 2012 at 10:43 AM, Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org> wrote:
> Signed-off-by: Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>
> ---
>  fs/cifs/cifsglob.h  |    5 +++++
>  fs/cifs/cifsproto.h |    2 ++
>  fs/cifs/smb1ops.c   |    3 +++
>  fs/cifs/transport.c |    7 ++++---
>  4 files changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> index b6e97f8..ff06211 100644
> --- a/fs/cifs/cifsglob.h
> +++ b/fs/cifs/cifsglob.h
> @@ -157,11 +157,16 @@ enum smb_version {
>  struct mid_q_entry;
>  struct TCP_Server_Info;
>  struct cifsFileInfo;
> +struct cifs_ses;
>
>  struct smb_version_operations {
>        int (*send_cancel)(struct TCP_Server_Info *, void *,
>                           struct mid_q_entry *);
>        bool (*compare_fids)(struct cifsFileInfo *, struct cifsFileInfo *);
> +       int (*setup_request)(struct cifs_ses *, struct kvec *, unsigned int,
> +                            struct mid_q_entry **);
> +       int (*check_receive)(struct mid_q_entry *, struct TCP_Server_Info *,
> +                            bool);
>  };
>
>  struct smb_version_values {
> diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
> index 0a3fa96..57af642 100644
> --- a/fs/cifs/cifsproto.h
> +++ b/fs/cifs/cifsproto.h
> @@ -78,6 +78,8 @@ extern int SendReceive(const unsigned int /* xid */ , struct cifs_ses *,
>                        int * /* bytes returned */ , const int long_op);
>  extern int SendReceiveNoRsp(const unsigned int xid, struct cifs_ses *ses,
>                            char *in_buf, int flags);
> +extern int cifs_setup_request(struct cifs_ses *, struct kvec *, unsigned int,
> +                             struct mid_q_entry **);
>  extern int cifs_check_receive(struct mid_q_entry *mid,
>                        struct TCP_Server_Info *server, bool log_error);
>  extern int SendReceive2(const unsigned int /* xid */ , struct cifs_ses *,
> diff --git a/fs/cifs/smb1ops.c b/fs/cifs/smb1ops.c
> index ce27f86..3b7cf89 100644
> --- a/fs/cifs/smb1ops.c
> +++ b/fs/cifs/smb1ops.c
> @@ -21,6 +21,7 @@
>  #include "cifsproto.h"
>  #include "cifs_debug.h"
>  #include "cifspdu.h"
> +#include "cifsproto.h"
>
>  /*
>  * An NT cancel request header looks just like the original request except:
> @@ -69,6 +70,8 @@ cifs_compare_fids(struct cifsFileInfo *ob1, struct cifsFileInfo *ob2)
>  struct smb_version_operations smb1_operations = {
>        .send_cancel = send_nt_cancel,
>        .compare_fids = cifs_compare_fids,
> +       .setup_request = cifs_setup_request,
> +       .check_receive = cifs_check_receive,
>  };
>
>  struct smb_version_values smb1_values = {
> diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c
> index 269a5a7..87bd998 100644
> --- a/fs/cifs/transport.c
> +++ b/fs/cifs/transport.c
> @@ -514,7 +514,7 @@ cifs_check_receive(struct mid_q_entry *mid, struct TCP_Server_Info *server,
>        return map_smb_to_linux_error(mid->resp_buf, log_error);
>  }
>
> -static int
> +int
>  cifs_setup_request(struct cifs_ses *ses, struct kvec *iov,
>                   unsigned int nvec, struct mid_q_entry **ret_mid)
>  {
> @@ -577,7 +577,7 @@ SendReceive2(const unsigned int xid, struct cifs_ses *ses,
>
>        mutex_lock(&ses->server->srv_mutex);
>
> -       rc = cifs_setup_request(ses, iov, n_vec, &midQ);
> +       rc = ses->server->ops->setup_request(ses, iov, n_vec, &midQ);
>        if (rc) {
>                mutex_unlock(&ses->server->srv_mutex);
>                cifs_small_buf_release(buf);
> @@ -640,7 +640,8 @@ SendReceive2(const unsigned int xid, struct cifs_ses *ses,
>        else
>                *pRespBufType = CIFS_SMALL_BUFFER;
>
> -       rc = cifs_check_receive(midQ, ses->server, flags & CIFS_LOG_ERROR);
> +       rc = ses->server->ops->check_receive(midQ, ses->server,
> +                                            flags & CIFS_LOG_ERROR);
>
>        /* mark it so buf will not be freed by delete_mid */
>        if ((flags & CIFS_NO_RESP) == 0)
> --
> 1.7.1
>
> --
> 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

Looks correct.

Acked-by: Shirish Pargaonkar <shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Re: [PATCH 1/5] CIFS: Move protocol specific part from SendReceive2 to ops struct

[Jeff Layton]

On Thu, 17 May 2012 19:43:40 +0400
Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org> wrote:

> Signed-off-by: Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>
> ---
>  fs/cifs/cifsglob.h  |    5 +++++
>  fs/cifs/cifsproto.h |    2 ++
>  fs/cifs/smb1ops.c   |    3 +++
>  fs/cifs/transport.c |    7 ++++---
>  4 files changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> index b6e97f8..ff06211 100644
> --- a/fs/cifs/cifsglob.h
> +++ b/fs/cifs/cifsglob.h
> @@ -157,11 +157,16 @@ enum smb_version {
>  struct mid_q_entry;
>  struct TCP_Server_Info;
>  struct cifsFileInfo;
> +struct cifs_ses;
>  
>  struct smb_version_operations {
>  	int (*send_cancel)(struct TCP_Server_Info *, void *,
>  			   struct mid_q_entry *);
>  	bool (*compare_fids)(struct cifsFileInfo *, struct cifsFileInfo *);
> +	int (*setup_request)(struct cifs_ses *, struct kvec *, unsigned int,
> +			     struct mid_q_entry **);
> +	int (*check_receive)(struct mid_q_entry *, struct TCP_Server_Info *,
> +			     bool);
>  };
>  
>  struct smb_version_values {
> diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
> index 0a3fa96..57af642 100644
> --- a/fs/cifs/cifsproto.h
> +++ b/fs/cifs/cifsproto.h
> @@ -78,6 +78,8 @@ extern int SendReceive(const unsigned int /* xid */ , struct cifs_ses *,
>  			int * /* bytes returned */ , const int long_op);
>  extern int SendReceiveNoRsp(const unsigned int xid, struct cifs_ses *ses,
>  			    char *in_buf, int flags);
> +extern int cifs_setup_request(struct cifs_ses *, struct kvec *, unsigned int,
> +			      struct mid_q_entry **);
>  extern int cifs_check_receive(struct mid_q_entry *mid,
>  			struct TCP_Server_Info *server, bool log_error);
>  extern int SendReceive2(const unsigned int /* xid */ , struct cifs_ses *,
> diff --git a/fs/cifs/smb1ops.c b/fs/cifs/smb1ops.c
> index ce27f86..3b7cf89 100644
> --- a/fs/cifs/smb1ops.c
> +++ b/fs/cifs/smb1ops.c
> @@ -21,6 +21,7 @@
>  #include "cifsproto.h"
>  #include "cifs_debug.h"
>  #include "cifspdu.h"
> +#include "cifsproto.h"

	^^^^
Holy double-includes, batman! Please fix this before merging.

>  
>  /*
>   * An NT cancel request header looks just like the original request except:
> @@ -69,6 +70,8 @@ cifs_compare_fids(struct cifsFileInfo *ob1, struct cifsFileInfo *ob2)
>  struct smb_version_operations smb1_operations = {
>  	.send_cancel = send_nt_cancel,
>  	.compare_fids = cifs_compare_fids,
> +	.setup_request = cifs_setup_request,
> +	.check_receive = cifs_check_receive,
>  };
>  
>  struct smb_version_values smb1_values = {
> diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c
> index 269a5a7..87bd998 100644
> --- a/fs/cifs/transport.c
> +++ b/fs/cifs/transport.c
> @@ -514,7 +514,7 @@ cifs_check_receive(struct mid_q_entry *mid, struct TCP_Server_Info *server,
>  	return map_smb_to_linux_error(mid->resp_buf, log_error);
>  }
>  
> -static int
> +int
>  cifs_setup_request(struct cifs_ses *ses, struct kvec *iov,
>  		   unsigned int nvec, struct mid_q_entry **ret_mid)
>  {
> @@ -577,7 +577,7 @@ SendReceive2(const unsigned int xid, struct cifs_ses *ses,
>  
>  	mutex_lock(&ses->server->srv_mutex);
>  
> -	rc = cifs_setup_request(ses, iov, n_vec, &midQ);
> +	rc = ses->server->ops->setup_request(ses, iov, n_vec, &midQ);
>  	if (rc) {
>  		mutex_unlock(&ses->server->srv_mutex);
>  		cifs_small_buf_release(buf);
> @@ -640,7 +640,8 @@ SendReceive2(const unsigned int xid, struct cifs_ses *ses,
>  	else
>  		*pRespBufType = CIFS_SMALL_BUFFER;
>  
> -	rc = cifs_check_receive(midQ, ses->server, flags & CIFS_LOG_ERROR);
> +	rc = ses->server->ops->check_receive(midQ, ses->server,
> +					     flags & CIFS_LOG_ERROR);
>  
>  	/* mark it so buf will not be freed by delete_mid */
>  	if ((flags & CIFS_NO_RESP) == 0)

Looks fine otherwise -- if you just remove the unneeded #include
addition, you can add my:

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

[PATCH 2/5] CIFS: Move header_size/max_header_size to ops structure

[Pavel Shilovsky]

Signed-off-by: Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>
---
 fs/cifs/cifsglob.h |   17 +++++------------
 fs/cifs/cifssmb.c  |    7 ++++---
 fs/cifs/connect.c  |   17 +++++++++--------
 fs/cifs/smb1ops.c  |    2 ++
 4 files changed, 20 insertions(+), 23 deletions(-)

diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index ff06211..94657e2 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -175,8 +175,13 @@ struct smb_version_values {
 	__u32		exclusive_lock_type;
 	__u32		shared_lock_type;
 	__u32		unlock_lock_type;
+	size_t		header_size;
+	size_t		max_header_size;
 };
 
+#define HEADER_SIZE(server) (server->vals->header_size)
+#define MAX_HEADER_SIZE(server) (server->vals->max_header_size)
+
 struct smb_vol {
 	char *username;
 	char *password;
@@ -372,18 +377,6 @@ has_credits(struct TCP_Server_Info *server, int *credits)
 	return num > 0;
 }
 
-static inline size_t
-header_size(void)
-{
-	return sizeof(struct smb_hdr);
-}
-
-static inline size_t
-max_header_size(void)
-{
-	return MAX_CIFS_HDR_SIZE;
-}
-
 /*
  * Macros to allow the TCP_Server_Info->net field and related code to drop out
  * when CONFIG_NET_NS isn't set.
diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
index 3563c93..77463f7 100644
--- a/fs/cifs/cifssmb.c
+++ b/fs/cifs/cifssmb.c
@@ -1400,7 +1400,7 @@ cifs_readv_discard(struct TCP_Server_Info *server, struct mid_q_entry *mid)
 
 		length = cifs_read_from_socket(server, server->bigbuf,
 				min_t(unsigned int, remaining,
-					CIFSMaxBufSize + max_header_size()));
+				    CIFSMaxBufSize + MAX_HEADER_SIZE(server)));
 		if (length < 0)
 			return length;
 		server->total_read += length;
@@ -1449,9 +1449,10 @@ cifs_readv_receive(struct TCP_Server_Info *server, struct mid_q_entry *mid)
 	 * can if there's not enough data. At this point, we've read down to
 	 * the Mid.
 	 */
-	len = min_t(unsigned int, buflen, read_rsp_size()) - header_size() + 1;
+	len = min_t(unsigned int, buflen, read_rsp_size()) -
+							HEADER_SIZE(server) + 1;
 
-	rdata->iov[0].iov_base = buf + header_size() - 1;
+	rdata->iov[0].iov_base = buf + HEADER_SIZE(server) - 1;
 	rdata->iov[0].iov_len = len;
 
 	length = cifs_readv_from_socket(server, rdata->iov, 1, len);
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 5ac20fc..65ec6ef 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -568,7 +568,7 @@ allocate_buffers(struct TCP_Server_Info *server)
 		}
 	} else if (server->large_buf) {
 		/* we are reusing a dirty large buf, clear its start */
-		memset(server->bigbuf, 0, header_size());
+		memset(server->bigbuf, 0, HEADER_SIZE(server));
 	}
 
 	if (!server->smallbuf) {
@@ -582,7 +582,7 @@ allocate_buffers(struct TCP_Server_Info *server)
 		/* beginning of smb buffer is cleared in our buf_get */
 	} else {
 		/* if existing small buf clear beginning */
-		memset(server->smallbuf, 0, header_size());
+		memset(server->smallbuf, 0, HEADER_SIZE(server));
 	}
 
 	return true;
@@ -953,7 +953,7 @@ standard_receive3(struct TCP_Server_Info *server, struct mid_q_entry *mid)
 	unsigned int pdu_length = get_rfc1002_length(buf);
 
 	/* make sure this will fit in a large buffer */
-	if (pdu_length > CIFSMaxBufSize + max_header_size() - 4) {
+	if (pdu_length > CIFSMaxBufSize + MAX_HEADER_SIZE(server) - 4) {
 		cERROR(1, "SMB response too long (%u bytes)",
 			pdu_length);
 		cifs_reconnect(server);
@@ -969,8 +969,8 @@ standard_receive3(struct TCP_Server_Info *server, struct mid_q_entry *mid)
 	}
 
 	/* now read the rest */
-	length = cifs_read_from_socket(server, buf + header_size() - 1,
-				       pdu_length - header_size() + 1 + 4);
+	length = cifs_read_from_socket(server, buf + HEADER_SIZE(server) - 1,
+				pdu_length - HEADER_SIZE(server) + 1 + 4);
 	if (length < 0)
 		return length;
 	server->total_read += length;
@@ -1044,7 +1044,7 @@ cifs_demultiplex_thread(void *p)
 			continue;
 
 		/* make sure we have enough to get to the MID */
-		if (pdu_length < header_size() - 1 - 4) {
+		if (pdu_length < HEADER_SIZE(server) - 1 - 4) {
 			cERROR(1, "SMB response too short (%u bytes)",
 				pdu_length);
 			cifs_reconnect(server);
@@ -1054,7 +1054,7 @@ cifs_demultiplex_thread(void *p)
 
 		/* read down to the MID */
 		length = cifs_read_from_socket(server, buf + 4,
-					       header_size() - 1 - 4);
+					       HEADER_SIZE(server) - 1 - 4);
 		if (length < 0)
 			continue;
 		server->total_read += length;
@@ -1079,7 +1079,8 @@ cifs_demultiplex_thread(void *p)
 		} else if (!is_valid_oplock_break(buf, server)) {
 			cERROR(1, "No task to wake, unknown frame received! "
 				   "NumMids %d", atomic_read(&midCount));
-			cifs_dump_mem("Received Data is: ", buf, header_size());
+			cifs_dump_mem("Received Data is: ", buf,
+				      HEADER_SIZE(server));
 #ifdef CONFIG_CIFS_DEBUG2
 			cifs_dump_detail(buf);
 			cifs_dump_mids(server);
diff --git a/fs/cifs/smb1ops.c b/fs/cifs/smb1ops.c
index 3b7cf89..5dc365f 100644
--- a/fs/cifs/smb1ops.c
+++ b/fs/cifs/smb1ops.c
@@ -80,4 +80,6 @@ struct smb_version_values smb1_values = {
 	.exclusive_lock_type = 0,
 	.shared_lock_type = LOCKING_ANDX_SHARED_LOCK,
 	.unlock_lock_type = 0,
+	.header_size = sizeof(struct smb_hdr),
+	.max_header_size = MAX_CIFS_HDR_SIZE,
 };
-- 
1.7.1

Re: [PATCH 2/5] CIFS: Move header_size/max_header_size to ops structure

[Shirish Pargaonkar]

On Thu, May 17, 2012 at 10:43 AM, Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org> wrote:
> Signed-off-by: Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>
> ---
>  fs/cifs/cifsglob.h |   17 +++++------------
>  fs/cifs/cifssmb.c  |    7 ++++---
>  fs/cifs/connect.c  |   17 +++++++++--------
>  fs/cifs/smb1ops.c  |    2 ++
>  4 files changed, 20 insertions(+), 23 deletions(-)
>
> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> index ff06211..94657e2 100644
> --- a/fs/cifs/cifsglob.h
> +++ b/fs/cifs/cifsglob.h
> @@ -175,8 +175,13 @@ struct smb_version_values {
>        __u32           exclusive_lock_type;
>        __u32           shared_lock_type;
>        __u32           unlock_lock_type;
> +       size_t          header_size;
> +       size_t          max_header_size;
>  };
>
> +#define HEADER_SIZE(server) (server->vals->header_size)
> +#define MAX_HEADER_SIZE(server) (server->vals->max_header_size)
> +
>  struct smb_vol {
>        char *username;
>        char *password;
> @@ -372,18 +377,6 @@ has_credits(struct TCP_Server_Info *server, int *credits)
>        return num > 0;
>  }
>
> -static inline size_t
> -header_size(void)
> -{
> -       return sizeof(struct smb_hdr);
> -}
> -
> -static inline size_t
> -max_header_size(void)
> -{
> -       return MAX_CIFS_HDR_SIZE;
> -}
> -
>  /*
>  * Macros to allow the TCP_Server_Info->net field and related code to drop out
>  * when CONFIG_NET_NS isn't set.
> diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
> index 3563c93..77463f7 100644
> --- a/fs/cifs/cifssmb.c
> +++ b/fs/cifs/cifssmb.c
> @@ -1400,7 +1400,7 @@ cifs_readv_discard(struct TCP_Server_Info *server, struct mid_q_entry *mid)
>
>                length = cifs_read_from_socket(server, server->bigbuf,
>                                min_t(unsigned int, remaining,
> -                                       CIFSMaxBufSize + max_header_size()));
> +                                   CIFSMaxBufSize + MAX_HEADER_SIZE(server)));
>                if (length < 0)
>                        return length;
>                server->total_read += length;
> @@ -1449,9 +1449,10 @@ cifs_readv_receive(struct TCP_Server_Info *server, struct mid_q_entry *mid)
>         * can if there's not enough data. At this point, we've read down to
>         * the Mid.
>         */
> -       len = min_t(unsigned int, buflen, read_rsp_size()) - header_size() + 1;
> +       len = min_t(unsigned int, buflen, read_rsp_size()) -
> +                                                       HEADER_SIZE(server) + 1;
>
> -       rdata->iov[0].iov_base = buf + header_size() - 1;
> +       rdata->iov[0].iov_base = buf + HEADER_SIZE(server) - 1;
>        rdata->iov[0].iov_len = len;
>
>        length = cifs_readv_from_socket(server, rdata->iov, 1, len);
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index 5ac20fc..65ec6ef 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -568,7 +568,7 @@ allocate_buffers(struct TCP_Server_Info *server)
>                }
>        } else if (server->large_buf) {
>                /* we are reusing a dirty large buf, clear its start */
> -               memset(server->bigbuf, 0, header_size());
> +               memset(server->bigbuf, 0, HEADER_SIZE(server));
>        }
>
>        if (!server->smallbuf) {
> @@ -582,7 +582,7 @@ allocate_buffers(struct TCP_Server_Info *server)
>                /* beginning of smb buffer is cleared in our buf_get */
>        } else {
>                /* if existing small buf clear beginning */
> -               memset(server->smallbuf, 0, header_size());
> +               memset(server->smallbuf, 0, HEADER_SIZE(server));
>        }
>
>        return true;
> @@ -953,7 +953,7 @@ standard_receive3(struct TCP_Server_Info *server, struct mid_q_entry *mid)
>        unsigned int pdu_length = get_rfc1002_length(buf);
>
>        /* make sure this will fit in a large buffer */
> -       if (pdu_length > CIFSMaxBufSize + max_header_size() - 4) {
> +       if (pdu_length > CIFSMaxBufSize + MAX_HEADER_SIZE(server) - 4) {
>                cERROR(1, "SMB response too long (%u bytes)",
>                        pdu_length);
>                cifs_reconnect(server);
> @@ -969,8 +969,8 @@ standard_receive3(struct TCP_Server_Info *server, struct mid_q_entry *mid)
>        }
>
>        /* now read the rest */
> -       length = cifs_read_from_socket(server, buf + header_size() - 1,
> -                                      pdu_length - header_size() + 1 + 4);
> +       length = cifs_read_from_socket(server, buf + HEADER_SIZE(server) - 1,
> +                               pdu_length - HEADER_SIZE(server) + 1 + 4);
>        if (length < 0)
>                return length;
>        server->total_read += length;
> @@ -1044,7 +1044,7 @@ cifs_demultiplex_thread(void *p)
>                        continue;
>
>                /* make sure we have enough to get to the MID */
> -               if (pdu_length < header_size() - 1 - 4) {
> +               if (pdu_length < HEADER_SIZE(server) - 1 - 4) {
>                        cERROR(1, "SMB response too short (%u bytes)",
>                                pdu_length);
>                        cifs_reconnect(server);
> @@ -1054,7 +1054,7 @@ cifs_demultiplex_thread(void *p)
>
>                /* read down to the MID */
>                length = cifs_read_from_socket(server, buf + 4,
> -                                              header_size() - 1 - 4);
> +                                              HEADER_SIZE(server) - 1 - 4);
>                if (length < 0)
>                        continue;
>                server->total_read += length;
> @@ -1079,7 +1079,8 @@ cifs_demultiplex_thread(void *p)
>                } else if (!is_valid_oplock_break(buf, server)) {
>                        cERROR(1, "No task to wake, unknown frame received! "
>                                   "NumMids %d", atomic_read(&midCount));
> -                       cifs_dump_mem("Received Data is: ", buf, header_size());
> +                       cifs_dump_mem("Received Data is: ", buf,
> +                                     HEADER_SIZE(server));
>  #ifdef CONFIG_CIFS_DEBUG2
>                        cifs_dump_detail(buf);
>                        cifs_dump_mids(server);
> diff --git a/fs/cifs/smb1ops.c b/fs/cifs/smb1ops.c
> index 3b7cf89..5dc365f 100644
> --- a/fs/cifs/smb1ops.c
> +++ b/fs/cifs/smb1ops.c
> @@ -80,4 +80,6 @@ struct smb_version_values smb1_values = {
>        .exclusive_lock_type = 0,
>        .shared_lock_type = LOCKING_ANDX_SHARED_LOCK,
>        .unlock_lock_type = 0,
> +       .header_size = sizeof(struct smb_hdr),
> +       .max_header_size = MAX_CIFS_HDR_SIZE,
>  };
> --
> 1.7.1
>
> --
> 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

Looks correct.

Acked-by: Shirish Pargaonkar <shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Re: [PATCH 2/5] CIFS: Move header_size/max_header_size to ops structure

[Jeff Layton]

On Thu, 17 May 2012 19:43:41 +0400
Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org> wrote:

> Signed-off-by: Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>
> ---
>  fs/cifs/cifsglob.h |   17 +++++------------
>  fs/cifs/cifssmb.c  |    7 ++++---
>  fs/cifs/connect.c  |   17 +++++++++--------
>  fs/cifs/smb1ops.c  |    2 ++
>  4 files changed, 20 insertions(+), 23 deletions(-)
> 
> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> index ff06211..94657e2 100644
> --- a/fs/cifs/cifsglob.h
> +++ b/fs/cifs/cifsglob.h
> @@ -175,8 +175,13 @@ struct smb_version_values {
>  	__u32		exclusive_lock_type;
>  	__u32		shared_lock_type;
>  	__u32		unlock_lock_type;
> +	size_t		header_size;
> +	size_t		max_header_size;
>  };
>  
> +#define HEADER_SIZE(server) (server->vals->header_size)
> +#define MAX_HEADER_SIZE(server) (server->vals->max_header_size)
> +
>  struct smb_vol {
>  	char *username;
>  	char *password;
> @@ -372,18 +377,6 @@ has_credits(struct TCP_Server_Info *server, int *credits)
>  	return num > 0;
>  }
>  
> -static inline size_t
> -header_size(void)
> -{
> -	return sizeof(struct smb_hdr);
> -}
> -
> -static inline size_t
> -max_header_size(void)
> -{
> -	return MAX_CIFS_HDR_SIZE;
> -}
> -
>  /*
>   * Macros to allow the TCP_Server_Info->net field and related code to drop out
>   * when CONFIG_NET_NS isn't set.
> diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
> index 3563c93..77463f7 100644
> --- a/fs/cifs/cifssmb.c
> +++ b/fs/cifs/cifssmb.c
> @@ -1400,7 +1400,7 @@ cifs_readv_discard(struct TCP_Server_Info *server, struct mid_q_entry *mid)
>  
>  		length = cifs_read_from_socket(server, server->bigbuf,
>  				min_t(unsigned int, remaining,
> -					CIFSMaxBufSize + max_header_size()));
> +				    CIFSMaxBufSize + MAX_HEADER_SIZE(server)));
>  		if (length < 0)
>  			return length;
>  		server->total_read += length;
> @@ -1449,9 +1449,10 @@ cifs_readv_receive(struct TCP_Server_Info *server, struct mid_q_entry *mid)
>  	 * can if there's not enough data. At this point, we've read down to
>  	 * the Mid.
>  	 */
> -	len = min_t(unsigned int, buflen, read_rsp_size()) - header_size() + 1;
> +	len = min_t(unsigned int, buflen, read_rsp_size()) -
> +							HEADER_SIZE(server) + 1;
>  
> -	rdata->iov[0].iov_base = buf + header_size() - 1;
> +	rdata->iov[0].iov_base = buf + HEADER_SIZE(server) - 1;
>  	rdata->iov[0].iov_len = len;
>  
>  	length = cifs_readv_from_socket(server, rdata->iov, 1, len);
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index 5ac20fc..65ec6ef 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -568,7 +568,7 @@ allocate_buffers(struct TCP_Server_Info *server)
>  		}
>  	} else if (server->large_buf) {
>  		/* we are reusing a dirty large buf, clear its start */
> -		memset(server->bigbuf, 0, header_size());
> +		memset(server->bigbuf, 0, HEADER_SIZE(server));
>  	}
>  
>  	if (!server->smallbuf) {
> @@ -582,7 +582,7 @@ allocate_buffers(struct TCP_Server_Info *server)
>  		/* beginning of smb buffer is cleared in our buf_get */
>  	} else {
>  		/* if existing small buf clear beginning */
> -		memset(server->smallbuf, 0, header_size());
> +		memset(server->smallbuf, 0, HEADER_SIZE(server));
>  	}
>  
>  	return true;
> @@ -953,7 +953,7 @@ standard_receive3(struct TCP_Server_Info *server, struct mid_q_entry *mid)
>  	unsigned int pdu_length = get_rfc1002_length(buf);
>  
>  	/* make sure this will fit in a large buffer */
> -	if (pdu_length > CIFSMaxBufSize + max_header_size() - 4) {
> +	if (pdu_length > CIFSMaxBufSize + MAX_HEADER_SIZE(server) - 4) {
>  		cERROR(1, "SMB response too long (%u bytes)",
>  			pdu_length);
>  		cifs_reconnect(server);
> @@ -969,8 +969,8 @@ standard_receive3(struct TCP_Server_Info *server, struct mid_q_entry *mid)
>  	}
>  
>  	/* now read the rest */
> -	length = cifs_read_from_socket(server, buf + header_size() - 1,
> -				       pdu_length - header_size() + 1 + 4);
> +	length = cifs_read_from_socket(server, buf + HEADER_SIZE(server) - 1,
> +				pdu_length - HEADER_SIZE(server) + 1 + 4);
>  	if (length < 0)
>  		return length;
>  	server->total_read += length;
> @@ -1044,7 +1044,7 @@ cifs_demultiplex_thread(void *p)
>  			continue;
>  
>  		/* make sure we have enough to get to the MID */
> -		if (pdu_length < header_size() - 1 - 4) {
> +		if (pdu_length < HEADER_SIZE(server) - 1 - 4) {
>  			cERROR(1, "SMB response too short (%u bytes)",
>  				pdu_length);
>  			cifs_reconnect(server);
> @@ -1054,7 +1054,7 @@ cifs_demultiplex_thread(void *p)
>  
>  		/* read down to the MID */
>  		length = cifs_read_from_socket(server, buf + 4,
> -					       header_size() - 1 - 4);
> +					       HEADER_SIZE(server) - 1 - 4);
>  		if (length < 0)
>  			continue;
>  		server->total_read += length;
> @@ -1079,7 +1079,8 @@ cifs_demultiplex_thread(void *p)
>  		} else if (!is_valid_oplock_break(buf, server)) {
>  			cERROR(1, "No task to wake, unknown frame received! "
>  				   "NumMids %d", atomic_read(&midCount));
> -			cifs_dump_mem("Received Data is: ", buf, header_size());
> +			cifs_dump_mem("Received Data is: ", buf,
> +				      HEADER_SIZE(server));
>  #ifdef CONFIG_CIFS_DEBUG2
>  			cifs_dump_detail(buf);
>  			cifs_dump_mids(server);
> diff --git a/fs/cifs/smb1ops.c b/fs/cifs/smb1ops.c
> index 3b7cf89..5dc365f 100644
> --- a/fs/cifs/smb1ops.c
> +++ b/fs/cifs/smb1ops.c
> @@ -80,4 +80,6 @@ struct smb_version_values smb1_values = {
>  	.exclusive_lock_type = 0,
>  	.shared_lock_type = LOCKING_ANDX_SHARED_LOCK,
>  	.unlock_lock_type = 0,
> +	.header_size = sizeof(struct smb_hdr),
> +	.max_header_size = MAX_CIFS_HDR_SIZE,
>  };

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

[PATCH 3/5] CIFS: Move protocol specific part from cifs_readv_receive to ops struct

[Pavel Shilovsky]

Signed-off-by: Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>
---
 fs/cifs/cifsglob.h |    4 ++++
 fs/cifs/cifssmb.c  |   34 +++++++---------------------------
 fs/cifs/smb1ops.c  |   19 +++++++++++++++++++
 3 files changed, 30 insertions(+), 27 deletions(-)

diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index 94657e2..080dd86 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -167,6 +167,9 @@ struct smb_version_operations {
 			     struct mid_q_entry **);
 	int (*check_receive)(struct mid_q_entry *, struct TCP_Server_Info *,
 			     bool);
+	unsigned int (*read_data_offset)(char *);
+	unsigned int (*read_data_length)(char *);
+	int (*map_error)(char *, bool);
 };
 
 struct smb_version_values {
@@ -177,6 +180,7 @@ struct smb_version_values {
 	__u32		unlock_lock_type;
 	size_t		header_size;
 	size_t		max_header_size;
+	size_t		read_rsp_size;
 };
 
 #define HEADER_SIZE(server) (server->vals->header_size)
diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
index 77463f7..b1f3751 100644
--- a/fs/cifs/cifssmb.c
+++ b/fs/cifs/cifssmb.c
@@ -1411,27 +1411,6 @@ cifs_readv_discard(struct TCP_Server_Info *server, struct mid_q_entry *mid)
 	return 0;
 }
 
-static inline size_t
-read_rsp_size(void)
-{
-	return sizeof(READ_RSP);
-}
-
-static inline unsigned int
-read_data_offset(char *buf)
-{
-	READ_RSP *rsp = (READ_RSP *)buf;
-	return le16_to_cpu(rsp->DataOffset);
-}
-
-static inline unsigned int
-read_data_length(char *buf)
-{
-	READ_RSP *rsp = (READ_RSP *)buf;
-	return (le16_to_cpu(rsp->DataLengthHigh) << 16) +
-	       le16_to_cpu(rsp->DataLength);
-}
-
 static int
 cifs_readv_receive(struct TCP_Server_Info *server, struct mid_q_entry *mid)
 {
@@ -1449,7 +1428,7 @@ cifs_readv_receive(struct TCP_Server_Info *server, struct mid_q_entry *mid)
 	 * can if there's not enough data. At this point, we've read down to
 	 * the Mid.
 	 */
-	len = min_t(unsigned int, buflen, read_rsp_size()) -
+	len = min_t(unsigned int, buflen, server->vals->read_rsp_size) -
 							HEADER_SIZE(server) + 1;
 
 	rdata->iov[0].iov_base = buf + HEADER_SIZE(server) - 1;
@@ -1461,7 +1440,7 @@ cifs_readv_receive(struct TCP_Server_Info *server, struct mid_q_entry *mid)
 	server->total_read += length;
 
 	/* Was the SMB read successful? */
-	rdata->result = map_smb_to_linux_error(buf, false);
+	rdata->result = server->ops->map_error(buf, false);
 	if (rdata->result != 0) {
 		cFYI(1, "%s: server returned error %d", __func__,
 			rdata->result);
@@ -1469,14 +1448,15 @@ cifs_readv_receive(struct TCP_Server_Info *server, struct mid_q_entry *mid)
 	}
 
 	/* Is there enough to get to the rest of the READ_RSP header? */
-	if (server->total_read < read_rsp_size()) {
+	if (server->total_read < server->vals->read_rsp_size) {
 		cFYI(1, "%s: server returned short header. got=%u expected=%zu",
-			__func__, server->total_read, read_rsp_size());
+			__func__, server->total_read,
+			server->vals->read_rsp_size);
 		rdata->result = -EIO;
 		return cifs_readv_discard(server, mid);
 	}
 
-	data_offset = read_data_offset(buf) + 4;
+	data_offset = server->ops->read_data_offset(buf) + 4;
 	if (data_offset < server->total_read) {
 		/*
 		 * win2k8 sometimes sends an offset of 0 when the read
@@ -1515,7 +1495,7 @@ cifs_readv_receive(struct TCP_Server_Info *server, struct mid_q_entry *mid)
 		rdata->iov[0].iov_base, rdata->iov[0].iov_len);
 
 	/* how much data is in the response? */
-	data_len = read_data_length(buf);
+	data_len = server->ops->read_data_length(buf);
 	if (data_offset + data_len > buflen) {
 		/* data_len is corrupt -- discard frame */
 		rdata->result = -EIO;
diff --git a/fs/cifs/smb1ops.c b/fs/cifs/smb1ops.c
index 5dc365f..31a6111 100644
--- a/fs/cifs/smb1ops.c
+++ b/fs/cifs/smb1ops.c
@@ -67,11 +67,29 @@ cifs_compare_fids(struct cifsFileInfo *ob1, struct cifsFileInfo *ob2)
 	return ob1->netfid == ob2->netfid;
 }
 
+static unsigned int
+cifs_read_data_offset(char *buf)
+{
+	READ_RSP *rsp = (READ_RSP *)buf;
+	return le16_to_cpu(rsp->DataOffset);
+}
+
+static unsigned int
+cifs_read_data_length(char *buf)
+{
+	READ_RSP *rsp = (READ_RSP *)buf;
+	return (le16_to_cpu(rsp->DataLengthHigh) << 16) +
+	       le16_to_cpu(rsp->DataLength);
+}
+
 struct smb_version_operations smb1_operations = {
 	.send_cancel = send_nt_cancel,
 	.compare_fids = cifs_compare_fids,
 	.setup_request = cifs_setup_request,
 	.check_receive = cifs_check_receive,
+	.read_data_offset = cifs_read_data_offset,
+	.read_data_length = cifs_read_data_length,
+	.map_error = map_smb_to_linux_error,
 };
 
 struct smb_version_values smb1_values = {
@@ -82,4 +100,5 @@ struct smb_version_values smb1_values = {
 	.unlock_lock_type = 0,
 	.header_size = sizeof(struct smb_hdr),
 	.max_header_size = MAX_CIFS_HDR_SIZE,
+	.read_rsp_size = sizeof(READ_RSP),
 };
-- 
1.7.1

Re: [PATCH 3/5] CIFS: Move protocol specific part from cifs_readv_receive to ops struct

[Shirish Pargaonkar]

On Thu, May 17, 2012 at 10:43 AM, Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org> wrote:
> Signed-off-by: Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>
> ---
>  fs/cifs/cifsglob.h |    4 ++++
>  fs/cifs/cifssmb.c  |   34 +++++++---------------------------
>  fs/cifs/smb1ops.c  |   19 +++++++++++++++++++
>  3 files changed, 30 insertions(+), 27 deletions(-)
>
> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> index 94657e2..080dd86 100644
> --- a/fs/cifs/cifsglob.h
> +++ b/fs/cifs/cifsglob.h
> @@ -167,6 +167,9 @@ struct smb_version_operations {
>                             struct mid_q_entry **);
>        int (*check_receive)(struct mid_q_entry *, struct TCP_Server_Info *,
>                             bool);
> +       unsigned int (*read_data_offset)(char *);
> +       unsigned int (*read_data_length)(char *);
> +       int (*map_error)(char *, bool);
>  };
>
>  struct smb_version_values {
> @@ -177,6 +180,7 @@ struct smb_version_values {
>        __u32           unlock_lock_type;
>        size_t          header_size;
>        size_t          max_header_size;
> +       size_t          read_rsp_size;
>  };
>
>  #define HEADER_SIZE(server) (server->vals->header_size)
> diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
> index 77463f7..b1f3751 100644
> --- a/fs/cifs/cifssmb.c
> +++ b/fs/cifs/cifssmb.c
> @@ -1411,27 +1411,6 @@ cifs_readv_discard(struct TCP_Server_Info *server, struct mid_q_entry *mid)
>        return 0;
>  }
>
> -static inline size_t
> -read_rsp_size(void)
> -{
> -       return sizeof(READ_RSP);
> -}
> -
> -static inline unsigned int
> -read_data_offset(char *buf)
> -{
> -       READ_RSP *rsp = (READ_RSP *)buf;
> -       return le16_to_cpu(rsp->DataOffset);
> -}
> -
> -static inline unsigned int
> -read_data_length(char *buf)
> -{
> -       READ_RSP *rsp = (READ_RSP *)buf;
> -       return (le16_to_cpu(rsp->DataLengthHigh) << 16) +
> -              le16_to_cpu(rsp->DataLength);
> -}
> -
>  static int
>  cifs_readv_receive(struct TCP_Server_Info *server, struct mid_q_entry *mid)
>  {
> @@ -1449,7 +1428,7 @@ cifs_readv_receive(struct TCP_Server_Info *server, struct mid_q_entry *mid)
>         * can if there's not enough data. At this point, we've read down to
>         * the Mid.
>         */
> -       len = min_t(unsigned int, buflen, read_rsp_size()) -
> +       len = min_t(unsigned int, buflen, server->vals->read_rsp_size) -
>                                                        HEADER_SIZE(server) + 1;
>
>        rdata->iov[0].iov_base = buf + HEADER_SIZE(server) - 1;
> @@ -1461,7 +1440,7 @@ cifs_readv_receive(struct TCP_Server_Info *server, struct mid_q_entry *mid)
>        server->total_read += length;
>
>        /* Was the SMB read successful? */
> -       rdata->result = map_smb_to_linux_error(buf, false);
> +       rdata->result = server->ops->map_error(buf, false);
>        if (rdata->result != 0) {
>                cFYI(1, "%s: server returned error %d", __func__,
>                        rdata->result);
> @@ -1469,14 +1448,15 @@ cifs_readv_receive(struct TCP_Server_Info *server, struct mid_q_entry *mid)
>        }
>
>        /* Is there enough to get to the rest of the READ_RSP header? */
> -       if (server->total_read < read_rsp_size()) {
> +       if (server->total_read < server->vals->read_rsp_size) {
>                cFYI(1, "%s: server returned short header. got=%u expected=%zu",
> -                       __func__, server->total_read, read_rsp_size());
> +                       __func__, server->total_read,
> +                       server->vals->read_rsp_size);
>                rdata->result = -EIO;
>                return cifs_readv_discard(server, mid);
>        }
>
> -       data_offset = read_data_offset(buf) + 4;
> +       data_offset = server->ops->read_data_offset(buf) + 4;
>        if (data_offset < server->total_read) {
>                /*
>                 * win2k8 sometimes sends an offset of 0 when the read
> @@ -1515,7 +1495,7 @@ cifs_readv_receive(struct TCP_Server_Info *server, struct mid_q_entry *mid)
>                rdata->iov[0].iov_base, rdata->iov[0].iov_len);
>
>        /* how much data is in the response? */
> -       data_len = read_data_length(buf);
> +       data_len = server->ops->read_data_length(buf);
>        if (data_offset + data_len > buflen) {
>                /* data_len is corrupt -- discard frame */
>                rdata->result = -EIO;
> diff --git a/fs/cifs/smb1ops.c b/fs/cifs/smb1ops.c
> index 5dc365f..31a6111 100644
> --- a/fs/cifs/smb1ops.c
> +++ b/fs/cifs/smb1ops.c
> @@ -67,11 +67,29 @@ cifs_compare_fids(struct cifsFileInfo *ob1, struct cifsFileInfo *ob2)
>        return ob1->netfid == ob2->netfid;
>  }
>
> +static unsigned int
> +cifs_read_data_offset(char *buf)
> +{
> +       READ_RSP *rsp = (READ_RSP *)buf;
> +       return le16_to_cpu(rsp->DataOffset);
> +}
> +
> +static unsigned int
> +cifs_read_data_length(char *buf)
> +{
> +       READ_RSP *rsp = (READ_RSP *)buf;
> +       return (le16_to_cpu(rsp->DataLengthHigh) << 16) +
> +              le16_to_cpu(rsp->DataLength);
> +}
> +
>  struct smb_version_operations smb1_operations = {
>        .send_cancel = send_nt_cancel,
>        .compare_fids = cifs_compare_fids,
>        .setup_request = cifs_setup_request,
>        .check_receive = cifs_check_receive,
> +       .read_data_offset = cifs_read_data_offset,
> +       .read_data_length = cifs_read_data_length,
> +       .map_error = map_smb_to_linux_error,
>  };
>
>  struct smb_version_values smb1_values = {
> @@ -82,4 +100,5 @@ struct smb_version_values smb1_values = {
>        .unlock_lock_type = 0,
>        .header_size = sizeof(struct smb_hdr),
>        .max_header_size = MAX_CIFS_HDR_SIZE,
> +       .read_rsp_size = sizeof(READ_RSP),
>  };
> --
> 1.7.1
>
> --
> 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

Looks correct.

Acked-by: Shirish Pargaonkar <shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Re: [PATCH 3/5] CIFS: Move protocol specific part from cifs_readv_receive to ops struct

[Jeff Layton]

On Thu, 17 May 2012 19:43:42 +0400
Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org> wrote:

> Signed-off-by: Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>
> ---
>  fs/cifs/cifsglob.h |    4 ++++
>  fs/cifs/cifssmb.c  |   34 +++++++---------------------------
>  fs/cifs/smb1ops.c  |   19 +++++++++++++++++++
>  3 files changed, 30 insertions(+), 27 deletions(-)
> 
> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> index 94657e2..080dd86 100644
> --- a/fs/cifs/cifsglob.h
> +++ b/fs/cifs/cifsglob.h
> @@ -167,6 +167,9 @@ struct smb_version_operations {
>  			     struct mid_q_entry **);
>  	int (*check_receive)(struct mid_q_entry *, struct TCP_Server_Info *,
>  			     bool);
> +	unsigned int (*read_data_offset)(char *);
> +	unsigned int (*read_data_length)(char *);
> +	int (*map_error)(char *, bool);
>  };
>  

Minor comment -- it might be good to start adding some comments so that
we can keep track of what each of these functions does. This struct is
likely to become large over time and the logic for some of these things
will eventually be lost in antiquity.

>  struct smb_version_values {
> @@ -177,6 +180,7 @@ struct smb_version_values {
>  	__u32		unlock_lock_type;
>  	size_t		header_size;
>  	size_t		max_header_size;
> +	size_t		read_rsp_size;
>  };
>  
>  #define HEADER_SIZE(server) (server->vals->header_size)
> diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
> index 77463f7..b1f3751 100644
> --- a/fs/cifs/cifssmb.c
> +++ b/fs/cifs/cifssmb.c
> @@ -1411,27 +1411,6 @@ cifs_readv_discard(struct TCP_Server_Info *server, struct mid_q_entry *mid)
>  	return 0;
>  }
>  
> -static inline size_t
> -read_rsp_size(void)
> -{
> -	return sizeof(READ_RSP);
> -}
> -
> -static inline unsigned int
> -read_data_offset(char *buf)
> -{
> -	READ_RSP *rsp = (READ_RSP *)buf;
> -	return le16_to_cpu(rsp->DataOffset);
> -}
> -
> -static inline unsigned int
> -read_data_length(char *buf)
> -{
> -	READ_RSP *rsp = (READ_RSP *)buf;
> -	return (le16_to_cpu(rsp->DataLengthHigh) << 16) +
> -	       le16_to_cpu(rsp->DataLength);
> -}
> -
>  static int
>  cifs_readv_receive(struct TCP_Server_Info *server, struct mid_q_entry *mid)
>  {
> @@ -1449,7 +1428,7 @@ cifs_readv_receive(struct TCP_Server_Info *server, struct mid_q_entry *mid)
>  	 * can if there's not enough data. At this point, we've read down to
>  	 * the Mid.
>  	 */
> -	len = min_t(unsigned int, buflen, read_rsp_size()) -
> +	len = min_t(unsigned int, buflen, server->vals->read_rsp_size) -
>  							HEADER_SIZE(server) + 1;
>  
>  	rdata->iov[0].iov_base = buf + HEADER_SIZE(server) - 1;
> @@ -1461,7 +1440,7 @@ cifs_readv_receive(struct TCP_Server_Info *server, struct mid_q_entry *mid)
>  	server->total_read += length;
>  
>  	/* Was the SMB read successful? */
> -	rdata->result = map_smb_to_linux_error(buf, false);
> +	rdata->result = server->ops->map_error(buf, false);
>  	if (rdata->result != 0) {
>  		cFYI(1, "%s: server returned error %d", __func__,
>  			rdata->result);
> @@ -1469,14 +1448,15 @@ cifs_readv_receive(struct TCP_Server_Info *server, struct mid_q_entry *mid)
>  	}
>  
>  	/* Is there enough to get to the rest of the READ_RSP header? */
> -	if (server->total_read < read_rsp_size()) {
> +	if (server->total_read < server->vals->read_rsp_size) {
>  		cFYI(1, "%s: server returned short header. got=%u expected=%zu",
> -			__func__, server->total_read, read_rsp_size());
> +			__func__, server->total_read,
> +			server->vals->read_rsp_size);
>  		rdata->result = -EIO;
>  		return cifs_readv_discard(server, mid);
>  	}
>  
> -	data_offset = read_data_offset(buf) + 4;
> +	data_offset = server->ops->read_data_offset(buf) + 4;
>  	if (data_offset < server->total_read) {
>  		/*
>  		 * win2k8 sometimes sends an offset of 0 when the read
> @@ -1515,7 +1495,7 @@ cifs_readv_receive(struct TCP_Server_Info *server, struct mid_q_entry *mid)
>  		rdata->iov[0].iov_base, rdata->iov[0].iov_len);
>  
>  	/* how much data is in the response? */
> -	data_len = read_data_length(buf);
> +	data_len = server->ops->read_data_length(buf);
>  	if (data_offset + data_len > buflen) {
>  		/* data_len is corrupt -- discard frame */
>  		rdata->result = -EIO;
> diff --git a/fs/cifs/smb1ops.c b/fs/cifs/smb1ops.c
> index 5dc365f..31a6111 100644
> --- a/fs/cifs/smb1ops.c
> +++ b/fs/cifs/smb1ops.c
> @@ -67,11 +67,29 @@ cifs_compare_fids(struct cifsFileInfo *ob1, struct cifsFileInfo *ob2)
>  	return ob1->netfid == ob2->netfid;
>  }
>  
> +static unsigned int
> +cifs_read_data_offset(char *buf)
> +{
> +	READ_RSP *rsp = (READ_RSP *)buf;
> +	return le16_to_cpu(rsp->DataOffset);
> +}
> +
> +static unsigned int
> +cifs_read_data_length(char *buf)
> +{
> +	READ_RSP *rsp = (READ_RSP *)buf;
> +	return (le16_to_cpu(rsp->DataLengthHigh) << 16) +
> +	       le16_to_cpu(rsp->DataLength);
> +}
> +
>  struct smb_version_operations smb1_operations = {
>  	.send_cancel = send_nt_cancel,
>  	.compare_fids = cifs_compare_fids,
>  	.setup_request = cifs_setup_request,
>  	.check_receive = cifs_check_receive,
> +	.read_data_offset = cifs_read_data_offset,
> +	.read_data_length = cifs_read_data_length,
> +	.map_error = map_smb_to_linux_error,
>  };
>  
>  struct smb_version_values smb1_values = {
> @@ -82,4 +100,5 @@ struct smb_version_values smb1_values = {
>  	.unlock_lock_type = 0,
>  	.header_size = sizeof(struct smb_hdr),
>  	.max_header_size = MAX_CIFS_HDR_SIZE,
> +	.read_rsp_size = sizeof(READ_RSP),
>  };


Patch is otherwise fine though...

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

[PATCH 4/5] CIFS: Move protocol specific demultiplex thread calls to ops struct

[Pavel Shilovsky]

Signed-off-by: Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>
---
 fs/cifs/cifsglob.h |    6 ++++++
 fs/cifs/connect.c  |   27 ++++-----------------------
 fs/cifs/smb1ops.c  |   26 ++++++++++++++++++++++++++
 3 files changed, 36 insertions(+), 23 deletions(-)

diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index 080dd86..7b3e8fe 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -170,6 +170,12 @@ struct smb_version_operations {
 	unsigned int (*read_data_offset)(char *);
 	unsigned int (*read_data_length)(char *);
 	int (*map_error)(char *, bool);
+	struct mid_q_entry * (*find_mid)(struct TCP_Server_Info *, char *);
+#ifdef CONFIG_CIFS_DEBUG2
+	void (*dump_detail)(void *);
+#endif
+	int (*check_message)(char *, unsigned int);
+	bool (*is_oplock_break)(char *, struct TCP_Server_Info *);
 };
 
 struct smb_version_values {
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 65ec6ef..ce033d7 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -783,25 +783,6 @@ is_smb_response(struct TCP_Server_Info *server, unsigned char type)
 	return false;
 }
 
-static struct mid_q_entry *
-find_mid(struct TCP_Server_Info *server, char *buffer)
-{
-	struct smb_hdr *buf = (struct smb_hdr *)buffer;
-	struct mid_q_entry *mid;
-
-	spin_lock(&GlobalMid_Lock);
-	list_for_each_entry(mid, &server->pending_mid_q, qhead) {
-		if (mid->mid == buf->Mid &&
-		    mid->mid_state == MID_REQUEST_SUBMITTED &&
-		    le16_to_cpu(mid->command) == buf->Command) {
-			spin_unlock(&GlobalMid_Lock);
-			return mid;
-		}
-	}
-	spin_unlock(&GlobalMid_Lock);
-	return NULL;
-}
-
 void
 dequeue_mid(struct mid_q_entry *mid, bool malformed)
 {
@@ -986,7 +967,7 @@ standard_receive3(struct TCP_Server_Info *server, struct mid_q_entry *mid)
 	 * 48 bytes is enough to display the header and a little bit
 	 * into the payload for debugging purposes.
 	 */
-	length = checkSMB(buf, server->total_read);
+	length = server->ops->check_message(buf, server->total_read);
 	if (length != 0)
 		cifs_dump_mem("Bad SMB: ", buf,
 			min_t(unsigned int, server->total_read, 48));
@@ -1059,7 +1040,7 @@ cifs_demultiplex_thread(void *p)
 			continue;
 		server->total_read += length;
 
-		mid_entry = find_mid(server, buf);
+		mid_entry = server->ops->find_mid(server, buf);
 
 		if (!mid_entry || !mid_entry->receive)
 			length = standard_receive3(server, mid_entry);
@@ -1076,13 +1057,13 @@ cifs_demultiplex_thread(void *p)
 		if (mid_entry != NULL) {
 			if (!mid_entry->multiRsp || mid_entry->multiEnd)
 				mid_entry->callback(mid_entry);
-		} else if (!is_valid_oplock_break(buf, server)) {
+		} else if (!server->ops->is_oplock_break(buf, server)) {
 			cERROR(1, "No task to wake, unknown frame received! "
 				   "NumMids %d", atomic_read(&midCount));
 			cifs_dump_mem("Received Data is: ", buf,
 				      HEADER_SIZE(server));
 #ifdef CONFIG_CIFS_DEBUG2
-			cifs_dump_detail(buf);
+			server->ops->dump_detail(buf);
 			cifs_dump_mids(server);
 #endif /* CIFS_DEBUG2 */
 
diff --git a/fs/cifs/smb1ops.c b/fs/cifs/smb1ops.c
index 31a6111..8a0b35b 100644
--- a/fs/cifs/smb1ops.c
+++ b/fs/cifs/smb1ops.c
@@ -22,6 +22,7 @@
 #include "cifs_debug.h"
 #include "cifspdu.h"
 #include "cifsproto.h"
+#include "cifs_debug.h"
 
 /*
  * An NT cancel request header looks just like the original request except:
@@ -82,6 +83,25 @@ cifs_read_data_length(char *buf)
 	       le16_to_cpu(rsp->DataLength);
 }
 
+static struct mid_q_entry *
+cifs_find_mid(struct TCP_Server_Info *server, char *buffer)
+{
+	struct smb_hdr *buf = (struct smb_hdr *)buffer;
+	struct mid_q_entry *mid;
+
+	spin_lock(&GlobalMid_Lock);
+	list_for_each_entry(mid, &server->pending_mid_q, qhead) {
+		if (mid->mid == buf->Mid &&
+		    mid->mid_state == MID_REQUEST_SUBMITTED &&
+		    le16_to_cpu(mid->command) == buf->Command) {
+			spin_unlock(&GlobalMid_Lock);
+			return mid;
+		}
+	}
+	spin_unlock(&GlobalMid_Lock);
+	return NULL;
+}
+
 struct smb_version_operations smb1_operations = {
 	.send_cancel = send_nt_cancel,
 	.compare_fids = cifs_compare_fids,
@@ -90,6 +110,12 @@ struct smb_version_operations smb1_operations = {
 	.read_data_offset = cifs_read_data_offset,
 	.read_data_length = cifs_read_data_length,
 	.map_error = map_smb_to_linux_error,
+	.find_mid = cifs_find_mid,
+	.check_message = checkSMB,
+#ifdef CONFIG_CIFS_DEBUG2
+	.dump_detail = cifs_dump_detail,
+#endif
+	.is_oplock_break = is_valid_oplock_break,
 };
 
 struct smb_version_values smb1_values = {
-- 
1.7.1

Re: [PATCH 4/5] CIFS: Move protocol specific demultiplex thread calls to ops struct

[Shirish Pargaonkar]

On Thu, May 17, 2012 at 10:43 AM, Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org> wrote:
> Signed-off-by: Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>
> ---
>  fs/cifs/cifsglob.h |    6 ++++++
>  fs/cifs/connect.c  |   27 ++++-----------------------
>  fs/cifs/smb1ops.c  |   26 ++++++++++++++++++++++++++
>  3 files changed, 36 insertions(+), 23 deletions(-)
>
> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> index 080dd86..7b3e8fe 100644
> --- a/fs/cifs/cifsglob.h
> +++ b/fs/cifs/cifsglob.h
> @@ -170,6 +170,12 @@ struct smb_version_operations {
>        unsigned int (*read_data_offset)(char *);
>        unsigned int (*read_data_length)(char *);
>        int (*map_error)(char *, bool);
> +       struct mid_q_entry * (*find_mid)(struct TCP_Server_Info *, char *);
> +#ifdef CONFIG_CIFS_DEBUG2
> +       void (*dump_detail)(void *);
> +#endif
> +       int (*check_message)(char *, unsigned int);
> +       bool (*is_oplock_break)(char *, struct TCP_Server_Info *);
>  };
>
>  struct smb_version_values {
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index 65ec6ef..ce033d7 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -783,25 +783,6 @@ is_smb_response(struct TCP_Server_Info *server, unsigned char type)
>        return false;
>  }
>
> -static struct mid_q_entry *
> -find_mid(struct TCP_Server_Info *server, char *buffer)
> -{
> -       struct smb_hdr *buf = (struct smb_hdr *)buffer;
> -       struct mid_q_entry *mid;
> -
> -       spin_lock(&GlobalMid_Lock);
> -       list_for_each_entry(mid, &server->pending_mid_q, qhead) {
> -               if (mid->mid == buf->Mid &&
> -                   mid->mid_state == MID_REQUEST_SUBMITTED &&
> -                   le16_to_cpu(mid->command) == buf->Command) {
> -                       spin_unlock(&GlobalMid_Lock);
> -                       return mid;
> -               }
> -       }
> -       spin_unlock(&GlobalMid_Lock);
> -       return NULL;
> -}
> -
>  void
>  dequeue_mid(struct mid_q_entry *mid, bool malformed)
>  {
> @@ -986,7 +967,7 @@ standard_receive3(struct TCP_Server_Info *server, struct mid_q_entry *mid)
>         * 48 bytes is enough to display the header and a little bit
>         * into the payload for debugging purposes.
>         */
> -       length = checkSMB(buf, server->total_read);
> +       length = server->ops->check_message(buf, server->total_read);
>        if (length != 0)
>                cifs_dump_mem("Bad SMB: ", buf,
>                        min_t(unsigned int, server->total_read, 48));
> @@ -1059,7 +1040,7 @@ cifs_demultiplex_thread(void *p)
>                        continue;
>                server->total_read += length;
>
> -               mid_entry = find_mid(server, buf);
> +               mid_entry = server->ops->find_mid(server, buf);
>
>                if (!mid_entry || !mid_entry->receive)
>                        length = standard_receive3(server, mid_entry);
> @@ -1076,13 +1057,13 @@ cifs_demultiplex_thread(void *p)
>                if (mid_entry != NULL) {
>                        if (!mid_entry->multiRsp || mid_entry->multiEnd)
>                                mid_entry->callback(mid_entry);
> -               } else if (!is_valid_oplock_break(buf, server)) {
> +               } else if (!server->ops->is_oplock_break(buf, server)) {
>                        cERROR(1, "No task to wake, unknown frame received! "
>                                   "NumMids %d", atomic_read(&midCount));
>                        cifs_dump_mem("Received Data is: ", buf,
>                                      HEADER_SIZE(server));
>  #ifdef CONFIG_CIFS_DEBUG2
> -                       cifs_dump_detail(buf);
> +                       server->ops->dump_detail(buf);
>                        cifs_dump_mids(server);
>  #endif /* CIFS_DEBUG2 */
>
> diff --git a/fs/cifs/smb1ops.c b/fs/cifs/smb1ops.c
> index 31a6111..8a0b35b 100644
> --- a/fs/cifs/smb1ops.c
> +++ b/fs/cifs/smb1ops.c
> @@ -22,6 +22,7 @@
>  #include "cifs_debug.h"
>  #include "cifspdu.h"
>  #include "cifsproto.h"
> +#include "cifs_debug.h"
>
>  /*
>  * An NT cancel request header looks just like the original request except:
> @@ -82,6 +83,25 @@ cifs_read_data_length(char *buf)
>               le16_to_cpu(rsp->DataLength);
>  }
>
> +static struct mid_q_entry *
> +cifs_find_mid(struct TCP_Server_Info *server, char *buffer)
> +{
> +       struct smb_hdr *buf = (struct smb_hdr *)buffer;
> +       struct mid_q_entry *mid;
> +
> +       spin_lock(&GlobalMid_Lock);
> +       list_for_each_entry(mid, &server->pending_mid_q, qhead) {
> +               if (mid->mid == buf->Mid &&
> +                   mid->mid_state == MID_REQUEST_SUBMITTED &&
> +                   le16_to_cpu(mid->command) == buf->Command) {
> +                       spin_unlock(&GlobalMid_Lock);
> +                       return mid;
> +               }
> +       }
> +       spin_unlock(&GlobalMid_Lock);
> +       return NULL;
> +}
> +
>  struct smb_version_operations smb1_operations = {
>        .send_cancel = send_nt_cancel,
>        .compare_fids = cifs_compare_fids,
> @@ -90,6 +110,12 @@ struct smb_version_operations smb1_operations = {
>        .read_data_offset = cifs_read_data_offset,
>        .read_data_length = cifs_read_data_length,
>        .map_error = map_smb_to_linux_error,
> +       .find_mid = cifs_find_mid,
> +       .check_message = checkSMB,
> +#ifdef CONFIG_CIFS_DEBUG2
> +       .dump_detail = cifs_dump_detail,
> +#endif
> +       .is_oplock_break = is_valid_oplock_break,
>  };
>
>  struct smb_version_values smb1_values = {
> --
> 1.7.1
>
> --
> 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

Looks correct.

Acked-by: Shirish Pargaonkar <shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Re: [PATCH 4/5] CIFS: Move protocol specific demultiplex thread calls to ops struct

[Jeff Layton]

On Thu, 17 May 2012 19:43:43 +0400
Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org> wrote:

> Signed-off-by: Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>
> ---
>  fs/cifs/cifsglob.h |    6 ++++++
>  fs/cifs/connect.c  |   27 ++++-----------------------
>  fs/cifs/smb1ops.c  |   26 ++++++++++++++++++++++++++
>  3 files changed, 36 insertions(+), 23 deletions(-)
> 
> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> index 080dd86..7b3e8fe 100644
> --- a/fs/cifs/cifsglob.h
> +++ b/fs/cifs/cifsglob.h
> @@ -170,6 +170,12 @@ struct smb_version_operations {
>  	unsigned int (*read_data_offset)(char *);
>  	unsigned int (*read_data_length)(char *);
>  	int (*map_error)(char *, bool);
> +	struct mid_q_entry * (*find_mid)(struct TCP_Server_Info *, char *);
> +#ifdef CONFIG_CIFS_DEBUG2
> +	void (*dump_detail)(void *);
> +#endif
> +	int (*check_message)(char *, unsigned int);
> +	bool (*is_oplock_break)(char *, struct TCP_Server_Info *);
>  };
>  
>  struct smb_version_values {
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index 65ec6ef..ce033d7 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -783,25 +783,6 @@ is_smb_response(struct TCP_Server_Info *server, unsigned char type)
>  	return false;
>  }
>  
> -static struct mid_q_entry *
> -find_mid(struct TCP_Server_Info *server, char *buffer)
> -{
> -	struct smb_hdr *buf = (struct smb_hdr *)buffer;
> -	struct mid_q_entry *mid;
> -
> -	spin_lock(&GlobalMid_Lock);
> -	list_for_each_entry(mid, &server->pending_mid_q, qhead) {
> -		if (mid->mid == buf->Mid &&
> -		    mid->mid_state == MID_REQUEST_SUBMITTED &&
> -		    le16_to_cpu(mid->command) == buf->Command) {
> -			spin_unlock(&GlobalMid_Lock);
> -			return mid;
> -		}
> -	}
> -	spin_unlock(&GlobalMid_Lock);
> -	return NULL;
> -}
> -
>  void
>  dequeue_mid(struct mid_q_entry *mid, bool malformed)
>  {
> @@ -986,7 +967,7 @@ standard_receive3(struct TCP_Server_Info *server, struct mid_q_entry *mid)
>  	 * 48 bytes is enough to display the header and a little bit
>  	 * into the payload for debugging purposes.
>  	 */
> -	length = checkSMB(buf, server->total_read);
> +	length = server->ops->check_message(buf, server->total_read);
>  	if (length != 0)
>  		cifs_dump_mem("Bad SMB: ", buf,
>  			min_t(unsigned int, server->total_read, 48));
> @@ -1059,7 +1040,7 @@ cifs_demultiplex_thread(void *p)
>  			continue;
>  		server->total_read += length;
>  
> -		mid_entry = find_mid(server, buf);
> +		mid_entry = server->ops->find_mid(server, buf);
>  
>  		if (!mid_entry || !mid_entry->receive)
>  			length = standard_receive3(server, mid_entry);
> @@ -1076,13 +1057,13 @@ cifs_demultiplex_thread(void *p)
>  		if (mid_entry != NULL) {
>  			if (!mid_entry->multiRsp || mid_entry->multiEnd)
>  				mid_entry->callback(mid_entry);
> -		} else if (!is_valid_oplock_break(buf, server)) {
> +		} else if (!server->ops->is_oplock_break(buf, server)) {
>  			cERROR(1, "No task to wake, unknown frame received! "
>  				   "NumMids %d", atomic_read(&midCount));
>  			cifs_dump_mem("Received Data is: ", buf,
>  				      HEADER_SIZE(server));
>  #ifdef CONFIG_CIFS_DEBUG2
> -			cifs_dump_detail(buf);
> +			server->ops->dump_detail(buf);
>  			cifs_dump_mids(server);
>  #endif /* CIFS_DEBUG2 */
>  


Hmmm, might it be cleaner to unconditionally add the dump_detail
operation to smb_version_operations, and only set it when
CONFIG_CIFS_DEBUG2 is enabled?

With that, you could get rid of the #ifdef clutter in the code above...


> diff --git a/fs/cifs/smb1ops.c b/fs/cifs/smb1ops.c
> index 31a6111..8a0b35b 100644
> --- a/fs/cifs/smb1ops.c
> +++ b/fs/cifs/smb1ops.c
> @@ -22,6 +22,7 @@
>  #include "cifs_debug.h"
>  #include "cifspdu.h"
>  #include "cifsproto.h"
> +#include "cifs_debug.h"
>  
>  /*
>   * An NT cancel request header looks just like the original request except:
> @@ -82,6 +83,25 @@ cifs_read_data_length(char *buf)
>  	       le16_to_cpu(rsp->DataLength);
>  }
>  
> +static struct mid_q_entry *
> +cifs_find_mid(struct TCP_Server_Info *server, char *buffer)
> +{
> +	struct smb_hdr *buf = (struct smb_hdr *)buffer;
> +	struct mid_q_entry *mid;
> +
> +	spin_lock(&GlobalMid_Lock);
> +	list_for_each_entry(mid, &server->pending_mid_q, qhead) {
> +		if (mid->mid == buf->Mid &&
> +		    mid->mid_state == MID_REQUEST_SUBMITTED &&
> +		    le16_to_cpu(mid->command) == buf->Command) {
> +			spin_unlock(&GlobalMid_Lock);
> +			return mid;
> +		}
> +	}
> +	spin_unlock(&GlobalMid_Lock);
> +	return NULL;
> +}
> +
>  struct smb_version_operations smb1_operations = {
>  	.send_cancel = send_nt_cancel,
>  	.compare_fids = cifs_compare_fids,
> @@ -90,6 +110,12 @@ struct smb_version_operations smb1_operations = {
>  	.read_data_offset = cifs_read_data_offset,
>  	.read_data_length = cifs_read_data_length,
>  	.map_error = map_smb_to_linux_error,
> +	.find_mid = cifs_find_mid,
> +	.check_message = checkSMB,
> +#ifdef CONFIG_CIFS_DEBUG2
> +	.dump_detail = cifs_dump_detail,
> +#endif
> +	.is_oplock_break = is_valid_oplock_break,
>  };
>  
>  struct smb_version_values smb1_values = {


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

Re: [PATCH 4/5] CIFS: Move protocol specific demultiplex thread calls to ops struct

[Pavel Shilovsky]

2012/5/18 Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>:
> On Thu, 17 May 2012 19:43:43 +0400
> Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org> wrote:
>
>> Signed-off-by: Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>
>> ---
>>  fs/cifs/cifsglob.h |    6 ++++++
>>  fs/cifs/connect.c  |   27 ++++-----------------------
>>  fs/cifs/smb1ops.c  |   26 ++++++++++++++++++++++++++
>>  3 files changed, 36 insertions(+), 23 deletions(-)
>>
>> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
>> index 080dd86..7b3e8fe 100644
>> --- a/fs/cifs/cifsglob.h
>> +++ b/fs/cifs/cifsglob.h
>> @@ -170,6 +170,12 @@ struct smb_version_operations {
>>       unsigned int (*read_data_offset)(char *);
>>       unsigned int (*read_data_length)(char *);
>>       int (*map_error)(char *, bool);
>> +     struct mid_q_entry * (*find_mid)(struct TCP_Server_Info *, char *);
>> +#ifdef CONFIG_CIFS_DEBUG2
>> +     void (*dump_detail)(void *);
>> +#endif
>> +     int (*check_message)(char *, unsigned int);
>> +     bool (*is_oplock_break)(char *, struct TCP_Server_Info *);
>>  };
>>
>>  struct smb_version_values {
>> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
>> index 65ec6ef..ce033d7 100644
>> --- a/fs/cifs/connect.c
>> +++ b/fs/cifs/connect.c
>> @@ -783,25 +783,6 @@ is_smb_response(struct TCP_Server_Info *server, unsigned char type)
>>       return false;
>>  }
>>
>> -static struct mid_q_entry *
>> -find_mid(struct TCP_Server_Info *server, char *buffer)
>> -{
>> -     struct smb_hdr *buf = (struct smb_hdr *)buffer;
>> -     struct mid_q_entry *mid;
>> -
>> -     spin_lock(&GlobalMid_Lock);
>> -     list_for_each_entry(mid, &server->pending_mid_q, qhead) {
>> -             if (mid->mid == buf->Mid &&
>> -                 mid->mid_state == MID_REQUEST_SUBMITTED &&
>> -                 le16_to_cpu(mid->command) == buf->Command) {
>> -                     spin_unlock(&GlobalMid_Lock);
>> -                     return mid;
>> -             }
>> -     }
>> -     spin_unlock(&GlobalMid_Lock);
>> -     return NULL;
>> -}
>> -
>>  void
>>  dequeue_mid(struct mid_q_entry *mid, bool malformed)
>>  {
>> @@ -986,7 +967,7 @@ standard_receive3(struct TCP_Server_Info *server, struct mid_q_entry *mid)
>>        * 48 bytes is enough to display the header and a little bit
>>        * into the payload for debugging purposes.
>>        */
>> -     length = checkSMB(buf, server->total_read);
>> +     length = server->ops->check_message(buf, server->total_read);
>>       if (length != 0)
>>               cifs_dump_mem("Bad SMB: ", buf,
>>                       min_t(unsigned int, server->total_read, 48));
>> @@ -1059,7 +1040,7 @@ cifs_demultiplex_thread(void *p)
>>                       continue;
>>               server->total_read += length;
>>
>> -             mid_entry = find_mid(server, buf);
>> +             mid_entry = server->ops->find_mid(server, buf);
>>
>>               if (!mid_entry || !mid_entry->receive)
>>                       length = standard_receive3(server, mid_entry);
>> @@ -1076,13 +1057,13 @@ cifs_demultiplex_thread(void *p)
>>               if (mid_entry != NULL) {
>>                       if (!mid_entry->multiRsp || mid_entry->multiEnd)
>>                               mid_entry->callback(mid_entry);
>> -             } else if (!is_valid_oplock_break(buf, server)) {
>> +             } else if (!server->ops->is_oplock_break(buf, server)) {
>>                       cERROR(1, "No task to wake, unknown frame received! "
>>                                  "NumMids %d", atomic_read(&midCount));
>>                       cifs_dump_mem("Received Data is: ", buf,
>>                                     HEADER_SIZE(server));
>>  #ifdef CONFIG_CIFS_DEBUG2
>> -                     cifs_dump_detail(buf);
>> +                     server->ops->dump_detail(buf);
>>                       cifs_dump_mids(server);
>>  #endif /* CIFS_DEBUG2 */
>>
>
>
> Hmmm, might it be cleaner to unconditionally add the dump_detail
> operation to smb_version_operations, and only set it when
> CONFIG_CIFS_DEBUG2 is enabled?
>
> With that, you could get rid of the #ifdef clutter in the code above...

Ok. In this case we should slightly fix cifs_dump_mids that is between
#ifdef statement too.

>
>
>> diff --git a/fs/cifs/smb1ops.c b/fs/cifs/smb1ops.c
>> index 31a6111..8a0b35b 100644
>> --- a/fs/cifs/smb1ops.c
>> +++ b/fs/cifs/smb1ops.c
>> @@ -22,6 +22,7 @@
>>  #include "cifs_debug.h"
>>  #include "cifspdu.h"
>>  #include "cifsproto.h"
>> +#include "cifs_debug.h"
>>
>>  /*
>>   * An NT cancel request header looks just like the original request except:
>> @@ -82,6 +83,25 @@ cifs_read_data_length(char *buf)
>>              le16_to_cpu(rsp->DataLength);
>>  }
>>
>> +static struct mid_q_entry *
>> +cifs_find_mid(struct TCP_Server_Info *server, char *buffer)
>> +{
>> +     struct smb_hdr *buf = (struct smb_hdr *)buffer;
>> +     struct mid_q_entry *mid;
>> +
>> +     spin_lock(&GlobalMid_Lock);
>> +     list_for_each_entry(mid, &server->pending_mid_q, qhead) {
>> +             if (mid->mid == buf->Mid &&
>> +                 mid->mid_state == MID_REQUEST_SUBMITTED &&
>> +                 le16_to_cpu(mid->command) == buf->Command) {
>> +                     spin_unlock(&GlobalMid_Lock);
>> +                     return mid;
>> +             }
>> +     }
>> +     spin_unlock(&GlobalMid_Lock);
>> +     return NULL;
>> +}
>> +
>>  struct smb_version_operations smb1_operations = {
>>       .send_cancel = send_nt_cancel,
>>       .compare_fids = cifs_compare_fids,
>> @@ -90,6 +110,12 @@ struct smb_version_operations smb1_operations = {
>>       .read_data_offset = cifs_read_data_offset,
>>       .read_data_length = cifs_read_data_length,
>>       .map_error = map_smb_to_linux_error,
>> +     .find_mid = cifs_find_mid,
>> +     .check_message = checkSMB,
>> +#ifdef CONFIG_CIFS_DEBUG2
>> +     .dump_detail = cifs_dump_detail,
>> +#endif
>> +     .is_oplock_break = is_valid_oplock_break,
>>  };
>>
>>  struct smb_version_values smb1_values = {
>
>
> --
> Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>



-- 
Best regards,
Pavel Shilovsky.

[PATCH 5/5] CIFS: Move add/set_credits and get_credits_field to ops structure

[Pavel Shilovsky]

Signed-off-by: Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>
---
 fs/cifs/cifsglob.h  |   25 +++++++++++++++----------
 fs/cifs/cifsproto.h |    3 ---
 fs/cifs/misc.c      |   19 -------------------
 fs/cifs/smb1ops.c   |   28 ++++++++++++++++++++++++++++
 fs/cifs/transport.c |    3 ++-
 5 files changed, 45 insertions(+), 33 deletions(-)

diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index 7b3e8fe..d17db87 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -167,6 +167,9 @@ struct smb_version_operations {
 			     struct mid_q_entry **);
 	int (*check_receive)(struct mid_q_entry *, struct TCP_Server_Info *,
 			     bool);
+	void (*add_credits)(struct TCP_Server_Info *, const unsigned int);
+	void (*set_credits)(struct TCP_Server_Info *, const int);
+	int * (*get_credits_field)(struct TCP_Server_Info *);
 	unsigned int (*read_data_offset)(char *);
 	unsigned int (*read_data_length)(char *);
 	int (*map_error)(char *, bool);
@@ -367,16 +370,6 @@ in_flight(struct TCP_Server_Info *server)
 	return num;
 }
 
-static inline int*
-get_credits_field(struct TCP_Server_Info *server)
-{
-	/*
-	 * This will change to switch statement when we reserve slots for echos
-	 * and oplock breaks.
-	 */
-	return &server->credits;
-}
-
 static inline bool
 has_credits(struct TCP_Server_Info *server, int *credits)
 {
@@ -387,6 +380,18 @@ has_credits(struct TCP_Server_Info *server, int *credits)
 	return num > 0;
 }
 
+static inline void
+cifs_add_credits(struct TCP_Server_Info *server, const unsigned int add)
+{
+	server->ops->add_credits(server, add);
+}
+
+static inline void
+cifs_set_credits(struct TCP_Server_Info *server, const int val)
+{
+	server->ops->set_credits(server, val);
+}
+
 /*
  * Macros to allow the TCP_Server_Info->net field and related code to drop out
  * when CONFIG_NET_NS isn't set.
diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
index 57af642..5ec21ec 100644
--- a/fs/cifs/cifsproto.h
+++ b/fs/cifs/cifsproto.h
@@ -90,9 +90,6 @@ extern int SendReceiveBlockingLock(const unsigned int xid,
 			struct smb_hdr *in_buf ,
 			struct smb_hdr *out_buf,
 			int *bytes_returned);
-extern void cifs_add_credits(struct TCP_Server_Info *server,
-			     const unsigned int add);
-extern void cifs_set_credits(struct TCP_Server_Info *server, const int val);
 extern int checkSMB(char *buf, unsigned int length);
 extern bool is_valid_oplock_break(char *, struct TCP_Server_Info *);
 extern bool backup_cred(struct cifs_sb_info *);
diff --git a/fs/cifs/misc.c b/fs/cifs/misc.c
index d2bb1e7..e2552d2 100644
--- a/fs/cifs/misc.c
+++ b/fs/cifs/misc.c
@@ -653,22 +653,3 @@ backup_cred(struct cifs_sb_info *cifs_sb)
 
 	return false;
 }
-
-void
-cifs_add_credits(struct TCP_Server_Info *server, const unsigned int add)
-{
-	spin_lock(&server->req_lock);
-	server->credits += add;
-	server->in_flight--;
-	spin_unlock(&server->req_lock);
-	wake_up(&server->request_q);
-}
-
-void
-cifs_set_credits(struct TCP_Server_Info *server, const int val)
-{
-	spin_lock(&server->req_lock);
-	server->credits = val;
-	server->oplocks = val > 1 ? enable_oplocks : false;
-	spin_unlock(&server->req_lock);
-}
diff --git a/fs/cifs/smb1ops.c b/fs/cifs/smb1ops.c
index 8a0b35b..17a69ed 100644
--- a/fs/cifs/smb1ops.c
+++ b/fs/cifs/smb1ops.c
@@ -102,11 +102,39 @@ cifs_find_mid(struct TCP_Server_Info *server, char *buffer)
 	return NULL;
 }
 
+static void
+_cifs_add_credits(struct TCP_Server_Info *server, const unsigned int add)
+{
+	spin_lock(&server->req_lock);
+	server->credits += add;
+	server->in_flight--;
+	spin_unlock(&server->req_lock);
+	wake_up(&server->request_q);
+}
+
+static void
+_cifs_set_credits(struct TCP_Server_Info *server, const int val)
+{
+	spin_lock(&server->req_lock);
+	server->credits = val;
+	server->oplocks = val > 1 ? enable_oplocks : false;
+	spin_unlock(&server->req_lock);
+}
+
+static int *
+cifs_get_credits_field(struct TCP_Server_Info *server)
+{
+	return &server->credits;
+}
+
 struct smb_version_operations smb1_operations = {
 	.send_cancel = send_nt_cancel,
 	.compare_fids = cifs_compare_fids,
 	.setup_request = cifs_setup_request,
 	.check_receive = cifs_check_receive,
+	.add_credits = _cifs_add_credits,
+	.set_credits = _cifs_set_credits,
+	.get_credits_field = cifs_get_credits_field,
 	.read_data_offset = cifs_read_data_offset,
 	.read_data_length = cifs_read_data_length,
 	.map_error = map_smb_to_linux_error,
diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c
index 87bd998..64c35fd 100644
--- a/fs/cifs/transport.c
+++ b/fs/cifs/transport.c
@@ -304,7 +304,8 @@ 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)
 {
-	return wait_for_free_credits(server, optype, get_credits_field(server));
+	return wait_for_free_credits(server, optype,
+				     server->ops->get_credits_field(server));
 }
 
 static int allocate_mid(struct cifs_ses *ses, struct smb_hdr *in_buf,
-- 
1.7.1

Re: [PATCH 5/5] CIFS: Move add/set_credits and get_credits_field to ops structure

[Shirish Pargaonkar]

On Thu, May 17, 2012 at 10:43 AM, Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org> wrote:
> Signed-off-by: Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>
> ---
>  fs/cifs/cifsglob.h  |   25 +++++++++++++++----------
>  fs/cifs/cifsproto.h |    3 ---
>  fs/cifs/misc.c      |   19 -------------------
>  fs/cifs/smb1ops.c   |   28 ++++++++++++++++++++++++++++
>  fs/cifs/transport.c |    3 ++-
>  5 files changed, 45 insertions(+), 33 deletions(-)
>
> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> index 7b3e8fe..d17db87 100644
> --- a/fs/cifs/cifsglob.h
> +++ b/fs/cifs/cifsglob.h
> @@ -167,6 +167,9 @@ struct smb_version_operations {
>                             struct mid_q_entry **);
>        int (*check_receive)(struct mid_q_entry *, struct TCP_Server_Info *,
>                             bool);
> +       void (*add_credits)(struct TCP_Server_Info *, const unsigned int);
> +       void (*set_credits)(struct TCP_Server_Info *, const int);
> +       int * (*get_credits_field)(struct TCP_Server_Info *);
>        unsigned int (*read_data_offset)(char *);
>        unsigned int (*read_data_length)(char *);
>        int (*map_error)(char *, bool);
> @@ -367,16 +370,6 @@ in_flight(struct TCP_Server_Info *server)
>        return num;
>  }
>
> -static inline int*
> -get_credits_field(struct TCP_Server_Info *server)
> -{
> -       /*
> -        * This will change to switch statement when we reserve slots for echos
> -        * and oplock breaks.
> -        */
> -       return &server->credits;
> -}
> -
>  static inline bool
>  has_credits(struct TCP_Server_Info *server, int *credits)
>  {
> @@ -387,6 +380,18 @@ has_credits(struct TCP_Server_Info *server, int *credits)
>        return num > 0;
>  }
>
> +static inline void
> +cifs_add_credits(struct TCP_Server_Info *server, const unsigned int add)
> +{
> +       server->ops->add_credits(server, add);
> +}
> +
> +static inline void
> +cifs_set_credits(struct TCP_Server_Info *server, const int val)
> +{
> +       server->ops->set_credits(server, val);
> +}
> +
>  /*
>  * Macros to allow the TCP_Server_Info->net field and related code to drop out
>  * when CONFIG_NET_NS isn't set.
> diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
> index 57af642..5ec21ec 100644
> --- a/fs/cifs/cifsproto.h
> +++ b/fs/cifs/cifsproto.h
> @@ -90,9 +90,6 @@ extern int SendReceiveBlockingLock(const unsigned int xid,
>                        struct smb_hdr *in_buf ,
>                        struct smb_hdr *out_buf,
>                        int *bytes_returned);
> -extern void cifs_add_credits(struct TCP_Server_Info *server,
> -                            const unsigned int add);
> -extern void cifs_set_credits(struct TCP_Server_Info *server, const int val);
>  extern int checkSMB(char *buf, unsigned int length);
>  extern bool is_valid_oplock_break(char *, struct TCP_Server_Info *);
>  extern bool backup_cred(struct cifs_sb_info *);
> diff --git a/fs/cifs/misc.c b/fs/cifs/misc.c
> index d2bb1e7..e2552d2 100644
> --- a/fs/cifs/misc.c
> +++ b/fs/cifs/misc.c
> @@ -653,22 +653,3 @@ backup_cred(struct cifs_sb_info *cifs_sb)
>
>        return false;
>  }
> -
> -void
> -cifs_add_credits(struct TCP_Server_Info *server, const unsigned int add)
> -{
> -       spin_lock(&server->req_lock);
> -       server->credits += add;
> -       server->in_flight--;
> -       spin_unlock(&server->req_lock);
> -       wake_up(&server->request_q);
> -}
> -
> -void
> -cifs_set_credits(struct TCP_Server_Info *server, const int val)
> -{
> -       spin_lock(&server->req_lock);
> -       server->credits = val;
> -       server->oplocks = val > 1 ? enable_oplocks : false;
> -       spin_unlock(&server->req_lock);
> -}
> diff --git a/fs/cifs/smb1ops.c b/fs/cifs/smb1ops.c
> index 8a0b35b..17a69ed 100644
> --- a/fs/cifs/smb1ops.c
> +++ b/fs/cifs/smb1ops.c
> @@ -102,11 +102,39 @@ cifs_find_mid(struct TCP_Server_Info *server, char *buffer)
>        return NULL;
>  }
>
> +static void
> +_cifs_add_credits(struct TCP_Server_Info *server, const unsigned int add)
> +{
> +       spin_lock(&server->req_lock);
> +       server->credits += add;
> +       server->in_flight--;
> +       spin_unlock(&server->req_lock);
> +       wake_up(&server->request_q);
> +}
> +
> +static void
> +_cifs_set_credits(struct TCP_Server_Info *server, const int val)
> +{
> +       spin_lock(&server->req_lock);
> +       server->credits = val;
> +       server->oplocks = val > 1 ? enable_oplocks : false;
> +       spin_unlock(&server->req_lock);
> +}
> +
> +static int *
> +cifs_get_credits_field(struct TCP_Server_Info *server)
> +{
> +       return &server->credits;
> +}
> +
>  struct smb_version_operations smb1_operations = {
>        .send_cancel = send_nt_cancel,
>        .compare_fids = cifs_compare_fids,
>        .setup_request = cifs_setup_request,
>        .check_receive = cifs_check_receive,
> +       .add_credits = _cifs_add_credits,
> +       .set_credits = _cifs_set_credits,
> +       .get_credits_field = cifs_get_credits_field,
>        .read_data_offset = cifs_read_data_offset,
>        .read_data_length = cifs_read_data_length,
>        .map_error = map_smb_to_linux_error,
> diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c
> index 87bd998..64c35fd 100644
> --- a/fs/cifs/transport.c
> +++ b/fs/cifs/transport.c
> @@ -304,7 +304,8 @@ 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)
>  {
> -       return wait_for_free_credits(server, optype, get_credits_field(server));
> +       return wait_for_free_credits(server, optype,
> +                                    server->ops->get_credits_field(server));
>  }
>
>  static int allocate_mid(struct cifs_ses *ses, struct smb_hdr *in_buf,
> --
> 1.7.1
>
> --
> 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

Looks correct.

Acked-by: Shirish Pargaonkar <shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Re: [PATCH 5/5] CIFS: Move add/set_credits and get_credits_field to ops structure

[Jeff Layton]

On Thu, 17 May 2012 19:43:44 +0400
Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org> wrote:

> Signed-off-by: Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>
> ---
>  fs/cifs/cifsglob.h  |   25 +++++++++++++++----------
>  fs/cifs/cifsproto.h |    3 ---
>  fs/cifs/misc.c      |   19 -------------------
>  fs/cifs/smb1ops.c   |   28 ++++++++++++++++++++++++++++
>  fs/cifs/transport.c |    3 ++-
>  5 files changed, 45 insertions(+), 33 deletions(-)
> 
> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> index 7b3e8fe..d17db87 100644
> --- a/fs/cifs/cifsglob.h
> +++ b/fs/cifs/cifsglob.h
> @@ -167,6 +167,9 @@ struct smb_version_operations {
>  			     struct mid_q_entry **);
>  	int (*check_receive)(struct mid_q_entry *, struct TCP_Server_Info *,
>  			     bool);
> +	void (*add_credits)(struct TCP_Server_Info *, const unsigned int);
> +	void (*set_credits)(struct TCP_Server_Info *, const int);
> +	int * (*get_credits_field)(struct TCP_Server_Info *);
>  	unsigned int (*read_data_offset)(char *);
>  	unsigned int (*read_data_length)(char *);
>  	int (*map_error)(char *, bool);
> @@ -367,16 +370,6 @@ in_flight(struct TCP_Server_Info *server)
>  	return num;
>  }
>  
> -static inline int*
> -get_credits_field(struct TCP_Server_Info *server)
> -{
> -	/*
> -	 * This will change to switch statement when we reserve slots for echos
> -	 * and oplock breaks.
> -	 */
> -	return &server->credits;
> -}
> -
>  static inline bool
>  has_credits(struct TCP_Server_Info *server, int *credits)
>  {
> @@ -387,6 +380,18 @@ has_credits(struct TCP_Server_Info *server, int *credits)
>  	return num > 0;
>  }
>  
> +static inline void
> +cifs_add_credits(struct TCP_Server_Info *server, const unsigned int add)
> +{
> +	server->ops->add_credits(server, add);
> +}
> +
> +static inline void
> +cifs_set_credits(struct TCP_Server_Info *server, const int val)
> +{
> +	server->ops->set_credits(server, val);
> +}
> +

nit: maybe call these "add_credits and set_credits" and get rid of the
leading '_' on the SMB1 functions below?

>  /*
>   * Macros to allow the TCP_Server_Info->net field and related code to drop out
>   * when CONFIG_NET_NS isn't set.
> diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
> index 57af642..5ec21ec 100644
> --- a/fs/cifs/cifsproto.h
> +++ b/fs/cifs/cifsproto.h
> @@ -90,9 +90,6 @@ extern int SendReceiveBlockingLock(const unsigned int xid,
>  			struct smb_hdr *in_buf ,
>  			struct smb_hdr *out_buf,
>  			int *bytes_returned);
> -extern void cifs_add_credits(struct TCP_Server_Info *server,
> -			     const unsigned int add);
> -extern void cifs_set_credits(struct TCP_Server_Info *server, const int val);
>  extern int checkSMB(char *buf, unsigned int length);
>  extern bool is_valid_oplock_break(char *, struct TCP_Server_Info *);
>  extern bool backup_cred(struct cifs_sb_info *);
> diff --git a/fs/cifs/misc.c b/fs/cifs/misc.c
> index d2bb1e7..e2552d2 100644
> --- a/fs/cifs/misc.c
> +++ b/fs/cifs/misc.c
> @@ -653,22 +653,3 @@ backup_cred(struct cifs_sb_info *cifs_sb)
>  
>  	return false;
>  }
> -
> -void
> -cifs_add_credits(struct TCP_Server_Info *server, const unsigned int add)
> -{
> -	spin_lock(&server->req_lock);
> -	server->credits += add;
> -	server->in_flight--;
> -	spin_unlock(&server->req_lock);
> -	wake_up(&server->request_q);
> -}
> -
> -void
> -cifs_set_credits(struct TCP_Server_Info *server, const int val)
> -{
> -	spin_lock(&server->req_lock);
> -	server->credits = val;
> -	server->oplocks = val > 1 ? enable_oplocks : false;
> -	spin_unlock(&server->req_lock);
> -}
> diff --git a/fs/cifs/smb1ops.c b/fs/cifs/smb1ops.c
> index 8a0b35b..17a69ed 100644
> --- a/fs/cifs/smb1ops.c
> +++ b/fs/cifs/smb1ops.c
> @@ -102,11 +102,39 @@ cifs_find_mid(struct TCP_Server_Info *server, char *buffer)
>  	return NULL;
>  }
>  
> +static void
> +_cifs_add_credits(struct TCP_Server_Info *server, const unsigned int add)
> +{
> +	spin_lock(&server->req_lock);
> +	server->credits += add;
> +	server->in_flight--;
> +	spin_unlock(&server->req_lock);
> +	wake_up(&server->request_q);
> +}
> +
> +static void
> +_cifs_set_credits(struct TCP_Server_Info *server, const int val)
> +{
> +	spin_lock(&server->req_lock);
> +	server->credits = val;
> +	server->oplocks = val > 1 ? enable_oplocks : false;
> +	spin_unlock(&server->req_lock);
> +}
> +
> +static int *
> +cifs_get_credits_field(struct TCP_Server_Info *server)
> +{
> +	return &server->credits;
> +}
> +
>  struct smb_version_operations smb1_operations = {
>  	.send_cancel = send_nt_cancel,
>  	.compare_fids = cifs_compare_fids,
>  	.setup_request = cifs_setup_request,
>  	.check_receive = cifs_check_receive,
> +	.add_credits = _cifs_add_credits,
> +	.set_credits = _cifs_set_credits,
> +	.get_credits_field = cifs_get_credits_field,
>  	.read_data_offset = cifs_read_data_offset,
>  	.read_data_length = cifs_read_data_length,
>  	.map_error = map_smb_to_linux_error,
> diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c
> index 87bd998..64c35fd 100644
> --- a/fs/cifs/transport.c
> +++ b/fs/cifs/transport.c
> @@ -304,7 +304,8 @@ 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)
>  {
> -	return wait_for_free_credits(server, optype, get_credits_field(server));
> +	return wait_for_free_credits(server, optype,
> +				     server->ops->get_credits_field(server));
>  }
>  
>  static int allocate_mid(struct cifs_ses *ses, struct smb_hdr *in_buf,

Other than the minor nit above, this looks fine:

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

Re: [PATCH 5/5] CIFS: Move add/set_credits and get_credits_field to ops structure

[Pavel Shilovsky]

2012/5/18 Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>:
> On Thu, 17 May 2012 19:43:44 +0400
> Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org> wrote:
>
>> Signed-off-by: Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>
>> ---
>>  fs/cifs/cifsglob.h  |   25 +++++++++++++++----------
>>  fs/cifs/cifsproto.h |    3 ---
>>  fs/cifs/misc.c      |   19 -------------------
>>  fs/cifs/smb1ops.c   |   28 ++++++++++++++++++++++++++++
>>  fs/cifs/transport.c |    3 ++-
>>  5 files changed, 45 insertions(+), 33 deletions(-)
>>
>> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
>> index 7b3e8fe..d17db87 100644
>> --- a/fs/cifs/cifsglob.h
>> +++ b/fs/cifs/cifsglob.h
>> @@ -167,6 +167,9 @@ struct smb_version_operations {
>>                            struct mid_q_entry **);
>>       int (*check_receive)(struct mid_q_entry *, struct TCP_Server_Info *,
>>                            bool);
>> +     void (*add_credits)(struct TCP_Server_Info *, const unsigned int);
>> +     void (*set_credits)(struct TCP_Server_Info *, const int);
>> +     int * (*get_credits_field)(struct TCP_Server_Info *);
>>       unsigned int (*read_data_offset)(char *);
>>       unsigned int (*read_data_length)(char *);
>>       int (*map_error)(char *, bool);
>> @@ -367,16 +370,6 @@ in_flight(struct TCP_Server_Info *server)
>>       return num;
>>  }
>>
>> -static inline int*
>> -get_credits_field(struct TCP_Server_Info *server)
>> -{
>> -     /*
>> -      * This will change to switch statement when we reserve slots for echos
>> -      * and oplock breaks.
>> -      */
>> -     return &server->credits;
>> -}
>> -
>>  static inline bool
>>  has_credits(struct TCP_Server_Info *server, int *credits)
>>  {
>> @@ -387,6 +380,18 @@ has_credits(struct TCP_Server_Info *server, int *credits)
>>       return num > 0;
>>  }
>>
>> +static inline void
>> +cifs_add_credits(struct TCP_Server_Info *server, const unsigned int add)
>> +{
>> +     server->ops->add_credits(server, add);
>> +}
>> +
>> +static inline void
>> +cifs_set_credits(struct TCP_Server_Info *server, const int val)
>> +{
>> +     server->ops->set_credits(server, val);
>> +}
>> +
>
> nit: maybe call these "add_credits and set_credits" and get rid of the
> leading '_' on the SMB1 functions below?
>
>>  /*
>>   * Macros to allow the TCP_Server_Info->net field and related code to drop out
>>   * when CONFIG_NET_NS isn't set.
>> diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
>> index 57af642..5ec21ec 100644
>> --- a/fs/cifs/cifsproto.h
>> +++ b/fs/cifs/cifsproto.h
>> @@ -90,9 +90,6 @@ extern int SendReceiveBlockingLock(const unsigned int xid,
>>                       struct smb_hdr *in_buf ,
>>                       struct smb_hdr *out_buf,
>>                       int *bytes_returned);
>> -extern void cifs_add_credits(struct TCP_Server_Info *server,
>> -                          const unsigned int add);
>> -extern void cifs_set_credits(struct TCP_Server_Info *server, const int val);
>>  extern int checkSMB(char *buf, unsigned int length);
>>  extern bool is_valid_oplock_break(char *, struct TCP_Server_Info *);
>>  extern bool backup_cred(struct cifs_sb_info *);
>> diff --git a/fs/cifs/misc.c b/fs/cifs/misc.c
>> index d2bb1e7..e2552d2 100644
>> --- a/fs/cifs/misc.c
>> +++ b/fs/cifs/misc.c
>> @@ -653,22 +653,3 @@ backup_cred(struct cifs_sb_info *cifs_sb)
>>
>>       return false;
>>  }
>> -
>> -void
>> -cifs_add_credits(struct TCP_Server_Info *server, const unsigned int add)
>> -{
>> -     spin_lock(&server->req_lock);
>> -     server->credits += add;
>> -     server->in_flight--;
>> -     spin_unlock(&server->req_lock);
>> -     wake_up(&server->request_q);
>> -}
>> -
>> -void
>> -cifs_set_credits(struct TCP_Server_Info *server, const int val)
>> -{
>> -     spin_lock(&server->req_lock);
>> -     server->credits = val;
>> -     server->oplocks = val > 1 ? enable_oplocks : false;
>> -     spin_unlock(&server->req_lock);
>> -}
>> diff --git a/fs/cifs/smb1ops.c b/fs/cifs/smb1ops.c
>> index 8a0b35b..17a69ed 100644
>> --- a/fs/cifs/smb1ops.c
>> +++ b/fs/cifs/smb1ops.c
>> @@ -102,11 +102,39 @@ cifs_find_mid(struct TCP_Server_Info *server, char *buffer)
>>       return NULL;
>>  }
>>
>> +static void
>> +_cifs_add_credits(struct TCP_Server_Info *server, const unsigned int add)
>> +{
>> +     spin_lock(&server->req_lock);
>> +     server->credits += add;
>> +     server->in_flight--;
>> +     spin_unlock(&server->req_lock);
>> +     wake_up(&server->request_q);
>> +}
>> +
>> +static void
>> +_cifs_set_credits(struct TCP_Server_Info *server, const int val)
>> +{
>> +     spin_lock(&server->req_lock);
>> +     server->credits = val;
>> +     server->oplocks = val > 1 ? enable_oplocks : false;
>> +     spin_unlock(&server->req_lock);
>> +}
>> +
>> +static int *
>> +cifs_get_credits_field(struct TCP_Server_Info *server)
>> +{
>> +     return &server->credits;
>> +}
>> +
>>  struct smb_version_operations smb1_operations = {
>>       .send_cancel = send_nt_cancel,
>>       .compare_fids = cifs_compare_fids,
>>       .setup_request = cifs_setup_request,
>>       .check_receive = cifs_check_receive,
>> +     .add_credits = _cifs_add_credits,
>> +     .set_credits = _cifs_set_credits,
>> +     .get_credits_field = cifs_get_credits_field,
>>       .read_data_offset = cifs_read_data_offset,
>>       .read_data_length = cifs_read_data_length,
>>       .map_error = map_smb_to_linux_error,
>> diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c
>> index 87bd998..64c35fd 100644
>> --- a/fs/cifs/transport.c
>> +++ b/fs/cifs/transport.c
>> @@ -304,7 +304,8 @@ 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)
>>  {
>> -     return wait_for_free_credits(server, optype, get_credits_field(server));
>> +     return wait_for_free_credits(server, optype,
>> +                                  server->ops->get_credits_field(server));
>>  }
>>
>>  static int allocate_mid(struct cifs_ses *ses, struct smb_hdr *in_buf,
>
> Other than the minor nit above, this looks fine:
>
> Reviewed-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

I thought about it but decided to do it that way that lets us not
rename all caller places and makes the patch smaller. But of course I
don't object to rename it.

Steve, thoughts?

-- 
Best regards,
Pavel Shilovsky.

Re: [PATCH 5/5] CIFS: Move add/set_credits and get_credits_field to ops structure

[Steve French]

On Sat, May 19, 2012 at 12:47 AM, Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org> wrote:
> 2012/5/18 Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>:
>> On Thu, 17 May 2012 19:43:44 +0400
>> Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org> wrote:
>>
>>> Signed-off-by: Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>
>>> ---
>>>  fs/cifs/cifsglob.h  |   25 +++++++++++++++----------
>>>  fs/cifs/cifsproto.h |    3 ---
>>>  fs/cifs/misc.c      |   19 -------------------
>>>  fs/cifs/smb1ops.c   |   28 ++++++++++++++++++++++++++++
>>>  fs/cifs/transport.c |    3 ++-
>>>  5 files changed, 45 insertions(+), 33 deletions(-)
>>>
>>> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
>>> index 7b3e8fe..d17db87 100644
>>> --- a/fs/cifs/cifsglob.h
>>> +++ b/fs/cifs/cifsglob.h
>>> @@ -167,6 +167,9 @@ struct smb_version_operations {
>>>                            struct mid_q_entry **);
>>>       int (*check_receive)(struct mid_q_entry *, struct TCP_Server_Info *,
>>>                            bool);
>>> +     void (*add_credits)(struct TCP_Server_Info *, const unsigned int);
>>> +     void (*set_credits)(struct TCP_Server_Info *, const int);
>>> +     int * (*get_credits_field)(struct TCP_Server_Info *);
>>>       unsigned int (*read_data_offset)(char *);
>>>       unsigned int (*read_data_length)(char *);
>>>       int (*map_error)(char *, bool);
>>> @@ -367,16 +370,6 @@ in_flight(struct TCP_Server_Info *server)
>>>       return num;
>>>  }
>>>
>>> -static inline int*
>>> -get_credits_field(struct TCP_Server_Info *server)
>>> -{
>>> -     /*
>>> -      * This will change to switch statement when we reserve slots for echos
>>> -      * and oplock breaks.
>>> -      */
>>> -     return &server->credits;
>>> -}
>>> -
>>>  static inline bool
>>>  has_credits(struct TCP_Server_Info *server, int *credits)
>>>  {
>>> @@ -387,6 +380,18 @@ has_credits(struct TCP_Server_Info *server, int *credits)
>>>       return num > 0;
>>>  }
>>>
>>> +static inline void
>>> +cifs_add_credits(struct TCP_Server_Info *server, const unsigned int add)
>>> +{
>>> +     server->ops->add_credits(server, add);
>>> +}
>>> +
>>> +static inline void
>>> +cifs_set_credits(struct TCP_Server_Info *server, const int val)
>>> +{
>>> +     server->ops->set_credits(server, val);
>>> +}
>>> +
>>
>> nit: maybe call these "add_credits and set_credits" and get rid of the
>> leading '_' on the SMB1 functions below?
>>
>>>  /*
>>>   * Macros to allow the TCP_Server_Info->net field and related code to drop out
>>>   * when CONFIG_NET_NS isn't set.
>>> diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
>>> index 57af642..5ec21ec 100644
>>> --- a/fs/cifs/cifsproto.h
>>> +++ b/fs/cifs/cifsproto.h
>>> @@ -90,9 +90,6 @@ extern int SendReceiveBlockingLock(const unsigned int xid,
>>>                       struct smb_hdr *in_buf ,
>>>                       struct smb_hdr *out_buf,
>>>                       int *bytes_returned);
>>> -extern void cifs_add_credits(struct TCP_Server_Info *server,
>>> -                          const unsigned int add);
>>> -extern void cifs_set_credits(struct TCP_Server_Info *server, const int val);
>>>  extern int checkSMB(char *buf, unsigned int length);
>>>  extern bool is_valid_oplock_break(char *, struct TCP_Server_Info *);
>>>  extern bool backup_cred(struct cifs_sb_info *);
>>> diff --git a/fs/cifs/misc.c b/fs/cifs/misc.c
>>> index d2bb1e7..e2552d2 100644
>>> --- a/fs/cifs/misc.c
>>> +++ b/fs/cifs/misc.c
>>> @@ -653,22 +653,3 @@ backup_cred(struct cifs_sb_info *cifs_sb)
>>>
>>>       return false;
>>>  }
>>> -
>>> -void
>>> -cifs_add_credits(struct TCP_Server_Info *server, const unsigned int add)
>>> -{
>>> -     spin_lock(&server->req_lock);
>>> -     server->credits += add;
>>> -     server->in_flight--;
>>> -     spin_unlock(&server->req_lock);
>>> -     wake_up(&server->request_q);
>>> -}
>>> -
>>> -void
>>> -cifs_set_credits(struct TCP_Server_Info *server, const int val)
>>> -{
>>> -     spin_lock(&server->req_lock);
>>> -     server->credits = val;
>>> -     server->oplocks = val > 1 ? enable_oplocks : false;
>>> -     spin_unlock(&server->req_lock);
>>> -}
>>> diff --git a/fs/cifs/smb1ops.c b/fs/cifs/smb1ops.c
>>> index 8a0b35b..17a69ed 100644
>>> --- a/fs/cifs/smb1ops.c
>>> +++ b/fs/cifs/smb1ops.c
>>> @@ -102,11 +102,39 @@ cifs_find_mid(struct TCP_Server_Info *server, char *buffer)
>>>       return NULL;
>>>  }
>>>
>>> +static void
>>> +_cifs_add_credits(struct TCP_Server_Info *server, const unsigned int add)
>>> +{
>>> +     spin_lock(&server->req_lock);
>>> +     server->credits += add;
>>> +     server->in_flight--;
>>> +     spin_unlock(&server->req_lock);
>>> +     wake_up(&server->request_q);
>>> +}
>>> +
>>> +static void
>>> +_cifs_set_credits(struct TCP_Server_Info *server, const int val)
>>> +{
>>> +     spin_lock(&server->req_lock);
>>> +     server->credits = val;
>>> +     server->oplocks = val > 1 ? enable_oplocks : false;
>>> +     spin_unlock(&server->req_lock);
>>> +}
>>> +
>>> +static int *
>>> +cifs_get_credits_field(struct TCP_Server_Info *server)
>>> +{
>>> +     return &server->credits;
>>> +}
>>> +
>>>  struct smb_version_operations smb1_operations = {
>>>       .send_cancel = send_nt_cancel,
>>>       .compare_fids = cifs_compare_fids,
>>>       .setup_request = cifs_setup_request,
>>>       .check_receive = cifs_check_receive,
>>> +     .add_credits = _cifs_add_credits,
>>> +     .set_credits = _cifs_set_credits,
>>> +     .get_credits_field = cifs_get_credits_field,
>>>       .read_data_offset = cifs_read_data_offset,
>>>       .read_data_length = cifs_read_data_length,
>>>       .map_error = map_smb_to_linux_error,
>>> diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c
>>> index 87bd998..64c35fd 100644
>>> --- a/fs/cifs/transport.c
>>> +++ b/fs/cifs/transport.c
>>> @@ -304,7 +304,8 @@ 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)
>>>  {
>>> -     return wait_for_free_credits(server, optype, get_credits_field(server));
>>> +     return wait_for_free_credits(server, optype,
>>> +                                  server->ops->get_credits_field(server));
>>>  }
>>>
>>>  static int allocate_mid(struct cifs_ses *ses, struct smb_hdr *in_buf,
>>
>> Other than the minor nit above, this looks fine:
>>
>> Reviewed-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>
> I thought about it but decided to do it that way that lets us not
> rename all caller places and makes the patch smaller. But of course I
> don't object to rename it.
>
> Steve, thoughts?

getting rid of the leading underscore in the function name makes
sense, could do a followon
patch for this but it doesn't really matter to me either way.

-- 
Thanks,

Steve

Re: [PATCH 0/5] Move protocol specific transport code to ops struct

[Steve French]

These look reasonable to me.  Let me know if any objections.

On Thu, May 17, 2012 at 10:43 AM, Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org> wrote:
> Here is yet another patchset that is targeted to make existing cifs code
> more protocol independent and let us add new protocols easily further.
>
> It can be merge as soons as got reviewed and ok for 3.5.
>
> Pavel Shilovsky (5):
>  CIFS: Move protocol specific part from SendReceive2 to ops struct
>  CIFS: Move header_size/max_header_size to ops structure
>  CIFS: Move protocol specific part from cifs_readv_receive to ops
>    struct
>  CIFS: Move protocol specific demultiplex thread calls to ops struct
>  CIFS: Move add/set_credits and get_credits_field to ops structure
>
>  fs/cifs/cifsglob.h  |   45 +++++++++++++++++++----------
>  fs/cifs/cifsproto.h |    5 +--
>  fs/cifs/cifssmb.c   |   39 ++++++-------------------
>  fs/cifs/connect.c   |   44 ++++++++--------------------
>  fs/cifs/misc.c      |   19 ------------
>  fs/cifs/smb1ops.c   |   78 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  fs/cifs/transport.c |   10 ++++--
>  7 files changed, 138 insertions(+), 102 deletions(-)
>
> --
> 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