All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/4] cifs: print MIDs in decimal notation
@ 2021-03-08 15:00 Paulo Alcantara
  2021-03-08 15:00 ` [PATCH 2/4] cifs: change noisy error message to FYI Paulo Alcantara
                   ` (4 more replies)
  0 siblings, 5 replies; 16+ messages in thread
From: Paulo Alcantara @ 2021-03-08 15:00 UTC (permalink / raw)
  To: linux-cifs, smfrench; +Cc: Paulo Alcantara

The MIDs are mostly printed as decimal, so let's make it consistent.

Signed-off-by: Paulo Alcantara (SUSE) <pc@cjr.nz>
---
 fs/cifs/cifs_debug.c | 2 +-
 fs/cifs/connect.c    | 4 ++--
 fs/cifs/smb2misc.c   | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/fs/cifs/cifs_debug.c b/fs/cifs/cifs_debug.c
index 3aedc484e440..88a7958170ee 100644
--- a/fs/cifs/cifs_debug.c
+++ b/fs/cifs/cifs_debug.c
@@ -207,7 +207,7 @@ static int cifs_debug_files_proc_show(struct seq_file *m, void *v)
 						from_kuid(&init_user_ns, cfile->uid),
 						cfile->dentry);
 #ifdef CONFIG_CIFS_DEBUG2
-					seq_printf(m, " 0x%llx\n", cfile->fid.mid);
+					seq_printf(m, " %llu\n", cfile->fid.mid);
 #else
 					seq_printf(m, "\n");
 #endif /* CIFS_DEBUG2 */
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 68642e3d4270..eec8a2052da2 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -741,7 +741,7 @@ static void clean_demultiplex_info(struct TCP_Server_Info *server)
 		spin_lock(&GlobalMid_Lock);
 		list_for_each_safe(tmp, tmp2, &server->pending_mid_q) {
 			mid_entry = list_entry(tmp, struct mid_q_entry, qhead);
-			cifs_dbg(FYI, "Clearing mid 0x%llx\n", mid_entry->mid);
+			cifs_dbg(FYI, "Clearing mid %llu\n", mid_entry->mid);
 			kref_get(&mid_entry->refcount);
 			mid_entry->mid_state = MID_SHUTDOWN;
 			list_move(&mid_entry->qhead, &dispose_list);
@@ -752,7 +752,7 @@ static void clean_demultiplex_info(struct TCP_Server_Info *server)
 		/* now walk dispose list and issue callbacks */
 		list_for_each_safe(tmp, tmp2, &dispose_list) {
 			mid_entry = list_entry(tmp, struct mid_q_entry, qhead);
-			cifs_dbg(FYI, "Callback mid 0x%llx\n", mid_entry->mid);
+			cifs_dbg(FYI, "Callback mid %llu\n", mid_entry->mid);
 			list_del_init(&mid_entry->qhead);
 			mid_entry->callback(mid_entry);
 			cifs_mid_q_entry_release(mid_entry);
diff --git a/fs/cifs/smb2misc.c b/fs/cifs/smb2misc.c
index 3ea3bda64083..0a55a77d94de 100644
--- a/fs/cifs/smb2misc.c
+++ b/fs/cifs/smb2misc.c
@@ -767,7 +767,7 @@ smb2_cancelled_close_fid(struct work_struct *work)
 	int rc;
 
 	if (cancelled->mid)
-		cifs_tcon_dbg(VFS, "Close unmatched open for MID:%llx\n",
+		cifs_tcon_dbg(VFS, "Close unmatched open for MID:%llu\n",
 			      cancelled->mid);
 	else
 		cifs_tcon_dbg(VFS, "Close interrupted close\n");
-- 
2.30.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 2/4] cifs: change noisy error message to FYI
  2021-03-08 15:00 [PATCH 1/4] cifs: print MIDs in decimal notation Paulo Alcantara
@ 2021-03-08 15:00 ` Paulo Alcantara
  2021-03-08 15:12   ` Aurélien Aptel
  2021-03-09  0:40   ` ronnie sahlberg
  2021-03-08 15:00 ` [PATCH 3/4] cifs: return proper error code in statfs(2) Paulo Alcantara
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 16+ messages in thread
From: Paulo Alcantara @ 2021-03-08 15:00 UTC (permalink / raw)
  To: linux-cifs, smfrench; +Cc: Paulo Alcantara

A customer has reported that their dmesg were being flooded by

  CIFS: VFS: \\server Cancelling wait for mid xxx cmd: a
  CIFS: VFS: \\server Cancelling wait for mid yyy cmd: b
  CIFS: VFS: \\server Cancelling wait for mid zzz cmd: c

because some processes that were performing statfs(2) on the share had
been interrupted due to their automount setup when certain users
logged in and out.

Change it to FYI as they should be mostly informative rather than
error messages.

Signed-off-by: Paulo Alcantara (SUSE) <pc@cjr.nz>
---
 fs/cifs/transport.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c
index 7bb1584b3724..f62f512e2cb1 100644
--- a/fs/cifs/transport.c
+++ b/fs/cifs/transport.c
@@ -1248,7 +1248,7 @@ compound_send_recv(const unsigned int xid, struct cifs_ses *ses,
 	}
 	if (rc != 0) {
 		for (; i < num_rqst; i++) {
-			cifs_server_dbg(VFS, "Cancelling wait for mid %llu cmd: %d\n",
+			cifs_server_dbg(FYI, "Cancelling wait for mid %llu cmd: %d\n",
 				 midQ[i]->mid, le16_to_cpu(midQ[i]->command));
 			send_cancel(server, &rqst[i], midQ[i]);
 			spin_lock(&GlobalMid_Lock);
-- 
2.30.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 3/4] cifs: return proper error code in statfs(2)
  2021-03-08 15:00 [PATCH 1/4] cifs: print MIDs in decimal notation Paulo Alcantara
  2021-03-08 15:00 ` [PATCH 2/4] cifs: change noisy error message to FYI Paulo Alcantara
@ 2021-03-08 15:00 ` Paulo Alcantara
  2021-03-08 15:13   ` Aurélien Aptel
                     ` (2 more replies)
  2021-03-08 15:00 ` [PATCH 4/4] cifs: do not send close in compound create+close requests Paulo Alcantara
                   ` (2 subsequent siblings)
  4 siblings, 3 replies; 16+ messages in thread
From: Paulo Alcantara @ 2021-03-08 15:00 UTC (permalink / raw)
  To: linux-cifs, smfrench; +Cc: Paulo Alcantara

In cifs_statfs(), if server->ops->queryfs is not NULL, then we should
use its return value rather than always returning 0.  Instead, use rc
variable as it is properly set to 0 in case there is no
server->ops->queryfs.

Signed-off-by: Paulo Alcantara (SUSE) <pc@cjr.nz>
---
 fs/cifs/cifsfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index d43e935d2df4..099ad9f3660b 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -290,7 +290,7 @@ cifs_statfs(struct dentry *dentry, struct kstatfs *buf)
 		rc = server->ops->queryfs(xid, tcon, cifs_sb, buf);
 
 	free_xid(xid);
-	return 0;
+	return rc;
 }
 
 static long cifs_fallocate(struct file *file, int mode, loff_t off, loff_t len)
-- 
2.30.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 4/4] cifs: do not send close in compound create+close requests
  2021-03-08 15:00 [PATCH 1/4] cifs: print MIDs in decimal notation Paulo Alcantara
  2021-03-08 15:00 ` [PATCH 2/4] cifs: change noisy error message to FYI Paulo Alcantara
  2021-03-08 15:00 ` [PATCH 3/4] cifs: return proper error code in statfs(2) Paulo Alcantara
@ 2021-03-08 15:00 ` Paulo Alcantara
  2021-03-08 15:29   ` Aurélien Aptel
                     ` (2 more replies)
  2021-03-08 15:10 ` [PATCH 1/4] cifs: print MIDs in decimal notation Aurélien Aptel
  2021-03-09  0:39 ` ronnie sahlberg
  4 siblings, 3 replies; 16+ messages in thread
From: Paulo Alcantara @ 2021-03-08 15:00 UTC (permalink / raw)
  To: linux-cifs, smfrench; +Cc: Paulo Alcantara

In case of interrupted syscalls, prevent sending CLOSE commands for
compound CREATE+CLOSE requests by introducing an
CIFS_CP_CREATE_CLOSE_OP flag to indicate lower layers that it should
not send a CLOSE command to the MIDs corresponding the compound
CREATE+CLOSE request.

A simple reproducer:

    #!/bin/bash

    mount //server/share /mnt -o username=foo,password=***
    tc qdisc add dev eth0 root netem delay 450ms
    stat -f /mnt &>/dev/null & pid=$!
    sleep 0.01
    kill $pid
    tc qdisc del dev eth0 root
    umount /mnt

Before patch:

    ...
    6 0.256893470 192.168.122.2 → 192.168.122.15 SMB2 402 Create Request File: ;GetInfo Request FS_INFO/FileFsFullSizeInformation;Close Request
    7 0.257144491 192.168.122.15 → 192.168.122.2 SMB2 498 Create Response File: ;GetInfo Response;Close Response
    9 0.260798209 192.168.122.2 → 192.168.122.15 SMB2 146 Close Request File:
   10 0.260841089 192.168.122.15 → 192.168.122.2 SMB2 130 Close Response, Error: STATUS_FILE_CLOSED

Signed-off-by: Paulo Alcantara (SUSE) <pc@cjr.nz>
---
 fs/cifs/cifsglob.h  | 19 ++++++++++---------
 fs/cifs/smb2inode.c |  1 +
 fs/cifs/smb2misc.c  |  8 ++++----
 fs/cifs/smb2ops.c   | 10 +++++-----
 fs/cifs/smb2proto.h |  3 +--
 fs/cifs/transport.c |  2 +-
 6 files changed, 22 insertions(+), 21 deletions(-)

diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index 3de3c5908a72..31fc8695abd6 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -257,7 +257,7 @@ struct smb_version_operations {
 	/* verify the message */
 	int (*check_message)(char *, unsigned int, struct TCP_Server_Info *);
 	bool (*is_oplock_break)(char *, struct TCP_Server_Info *);
-	int (*handle_cancelled_mid)(char *, struct TCP_Server_Info *);
+	int (*handle_cancelled_mid)(struct mid_q_entry *, struct TCP_Server_Info *);
 	void (*downgrade_oplock)(struct TCP_Server_Info *server,
 				 struct cifsInodeInfo *cinode, __u32 oplock,
 				 unsigned int epoch, bool *purge_cache);
@@ -1705,16 +1705,17 @@ static inline bool is_retryable_error(int error)
 #define   CIFS_NO_RSP_BUF   0x040    /* no response buffer required */
 
 /* Type of request operation */
-#define   CIFS_ECHO_OP      0x080    /* echo request */
-#define   CIFS_OBREAK_OP   0x0100    /* oplock break request */
-#define   CIFS_NEG_OP      0x0200    /* negotiate request */
+#define   CIFS_ECHO_OP            0x080  /* echo request */
+#define   CIFS_OBREAK_OP          0x0100 /* oplock break request */
+#define   CIFS_NEG_OP             0x0200 /* negotiate request */
+#define   CIFS_CP_CREATE_CLOSE_OP 0x0400 /* compound create+close request */
 /* Lower bitmask values are reserved by others below. */
-#define   CIFS_SESS_OP     0x2000    /* session setup request */
-#define   CIFS_OP_MASK     0x2380    /* mask request type */
+#define   CIFS_SESS_OP            0x2000 /* session setup request */
+#define   CIFS_OP_MASK            0x2780 /* mask request type */
 
-#define   CIFS_HAS_CREDITS 0x0400    /* already has credits */
-#define   CIFS_TRANSFORM_REQ 0x0800    /* transform request before sending */
-#define   CIFS_NO_SRV_RSP    0x1000    /* there is no server response */
+#define   CIFS_HAS_CREDITS        0x0400 /* already has credits */
+#define   CIFS_TRANSFORM_REQ      0x0800 /* transform request before sending */
+#define   CIFS_NO_SRV_RSP         0x1000 /* there is no server response */
 
 /* Security Flags: indicate type of session setup needed */
 #define   CIFSSEC_MAY_SIGN	0x00001
diff --git a/fs/cifs/smb2inode.c b/fs/cifs/smb2inode.c
index 1f900b81c34a..a718dc77e604 100644
--- a/fs/cifs/smb2inode.c
+++ b/fs/cifs/smb2inode.c
@@ -358,6 +358,7 @@ smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon,
 	if (cfile)
 		goto after_close;
 	/* Close */
+	flags |= CIFS_CP_CREATE_CLOSE_OP;
 	rqst[num_rqst].rq_iov = &vars->close_iov[0];
 	rqst[num_rqst].rq_nvec = 1;
 	rc = SMB2_close_init(tcon, server,
diff --git a/fs/cifs/smb2misc.c b/fs/cifs/smb2misc.c
index 0a55a77d94de..c99966121757 100644
--- a/fs/cifs/smb2misc.c
+++ b/fs/cifs/smb2misc.c
@@ -844,14 +844,14 @@ smb2_handle_cancelled_close(struct cifs_tcon *tcon, __u64 persistent_fid,
 }
 
 int
-smb2_handle_cancelled_mid(char *buffer, struct TCP_Server_Info *server)
+smb2_handle_cancelled_mid(struct mid_q_entry *mid, struct TCP_Server_Info *server)
 {
-	struct smb2_sync_hdr *sync_hdr = (struct smb2_sync_hdr *)buffer;
-	struct smb2_create_rsp *rsp = (struct smb2_create_rsp *)buffer;
+	struct smb2_sync_hdr *sync_hdr = mid->resp_buf;
+	struct smb2_create_rsp *rsp = mid->resp_buf;
 	struct cifs_tcon *tcon;
 	int rc;
 
-	if (sync_hdr->Command != SMB2_CREATE ||
+	if ((mid->optype & CIFS_CP_CREATE_CLOSE_OP) || sync_hdr->Command != SMB2_CREATE ||
 	    sync_hdr->Status != STATUS_SUCCESS)
 		return 0;
 
diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
index f5087295424c..9bae7e8deb09 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -1195,7 +1195,7 @@ smb2_set_ea(const unsigned int xid, struct cifs_tcon *tcon,
 	struct TCP_Server_Info *server = cifs_pick_channel(ses);
 	__le16 *utf16_path = NULL;
 	int ea_name_len = strlen(ea_name);
-	int flags = 0;
+	int flags = CIFS_CP_CREATE_CLOSE_OP;
 	int len;
 	struct smb_rqst rqst[3];
 	int resp_buftype[3];
@@ -1573,7 +1573,7 @@ smb2_ioctl_query_info(const unsigned int xid,
 	struct smb_query_info qi;
 	struct smb_query_info __user *pqi;
 	int rc = 0;
-	int flags = 0;
+	int flags = CIFS_CP_CREATE_CLOSE_OP;
 	struct smb2_query_info_rsp *qi_rsp = NULL;
 	struct smb2_ioctl_rsp *io_rsp = NULL;
 	void *buffer = NULL;
@@ -2577,7 +2577,7 @@ smb2_query_info_compound(const unsigned int xid, struct cifs_tcon *tcon,
 {
 	struct cifs_ses *ses = tcon->ses;
 	struct TCP_Server_Info *server = cifs_pick_channel(ses);
-	int flags = 0;
+	int flags = CIFS_CP_CREATE_CLOSE_OP;
 	struct smb_rqst rqst[3];
 	int resp_buftype[3];
 	struct kvec rsp_iov[3];
@@ -2975,7 +2975,7 @@ smb2_query_symlink(const unsigned int xid, struct cifs_tcon *tcon,
 	unsigned int sub_offset;
 	unsigned int print_len;
 	unsigned int print_offset;
-	int flags = 0;
+	int flags = CIFS_CP_CREATE_CLOSE_OP;
 	struct smb_rqst rqst[3];
 	int resp_buftype[3];
 	struct kvec rsp_iov[3];
@@ -3157,7 +3157,7 @@ smb2_query_reparse_tag(const unsigned int xid, struct cifs_tcon *tcon,
 	struct cifs_open_parms oparms;
 	struct cifs_fid fid;
 	struct TCP_Server_Info *server = cifs_pick_channel(tcon->ses);
-	int flags = 0;
+	int flags = CIFS_CP_CREATE_CLOSE_OP;
 	struct smb_rqst rqst[3];
 	int resp_buftype[3];
 	struct kvec rsp_iov[3];
diff --git a/fs/cifs/smb2proto.h b/fs/cifs/smb2proto.h
index 9565e27681a5..a2eb34a8d9c9 100644
--- a/fs/cifs/smb2proto.h
+++ b/fs/cifs/smb2proto.h
@@ -246,8 +246,7 @@ extern int SMB2_oplock_break(const unsigned int xid, struct cifs_tcon *tcon,
 extern int smb2_handle_cancelled_close(struct cifs_tcon *tcon,
 				       __u64 persistent_fid,
 				       __u64 volatile_fid);
-extern int smb2_handle_cancelled_mid(char *buffer,
-					struct TCP_Server_Info *server);
+extern int smb2_handle_cancelled_mid(struct mid_q_entry *mid, struct TCP_Server_Info *server);
 void smb2_cancelled_close_fid(struct work_struct *work);
 extern int SMB2_QFS_info(const unsigned int xid, struct cifs_tcon *tcon,
 			 u64 persistent_file_id, u64 volatile_file_id,
diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c
index f62f512e2cb1..9438a0c35473 100644
--- a/fs/cifs/transport.c
+++ b/fs/cifs/transport.c
@@ -109,7 +109,7 @@ static void _cifs_mid_q_entry_release(struct kref *refcount)
 	if (midEntry->resp_buf && (midEntry->mid_flags & MID_WAIT_CANCELLED) &&
 	    midEntry->mid_state == MID_RESPONSE_RECEIVED &&
 	    server->ops->handle_cancelled_mid)
-		server->ops->handle_cancelled_mid(midEntry->resp_buf, server);
+		server->ops->handle_cancelled_mid(midEntry, server);
 
 	midEntry->mid_state = MID_FREE;
 	atomic_dec(&midCount);
-- 
2.30.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/4] cifs: print MIDs in decimal notation
  2021-03-08 15:00 [PATCH 1/4] cifs: print MIDs in decimal notation Paulo Alcantara
                   ` (2 preceding siblings ...)
  2021-03-08 15:00 ` [PATCH 4/4] cifs: do not send close in compound create+close requests Paulo Alcantara
@ 2021-03-08 15:10 ` Aurélien Aptel
  2021-03-09  0:39 ` ronnie sahlberg
  4 siblings, 0 replies; 16+ messages in thread
From: Aurélien Aptel @ 2021-03-08 15:10 UTC (permalink / raw)
  To: Paulo Alcantara, linux-cifs, smfrench; +Cc: Paulo Alcantara

Paulo Alcantara <pc@cjr.nz> writes:
> The MIDs are mostly printed as decimal, so let's make it consistent.

Reviewed-by: Aurelien Aptel <aaptel@suse.com>

-- 
Aurélien Aptel / SUSE Labs Samba Team
GPG: 1839 CB5F 9F5B FB9B AA97  8C99 03C8 A49B 521B D5D3
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg, DE
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah HRB 247165 (AG München)


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/4] cifs: change noisy error message to FYI
  2021-03-08 15:00 ` [PATCH 2/4] cifs: change noisy error message to FYI Paulo Alcantara
@ 2021-03-08 15:12   ` Aurélien Aptel
  2021-03-09  0:40   ` ronnie sahlberg
  1 sibling, 0 replies; 16+ messages in thread
From: Aurélien Aptel @ 2021-03-08 15:12 UTC (permalink / raw)
  To: Paulo Alcantara, linux-cifs, smfrench; +Cc: Paulo Alcantara


Reviewed-by: Aurelien Aptel <aaptel@suse.com>

-- 
Aurélien Aptel / SUSE Labs Samba Team
GPG: 1839 CB5F 9F5B FB9B AA97  8C99 03C8 A49B 521B D5D3
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg, DE
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah HRB 247165 (AG München)


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/4] cifs: return proper error code in statfs(2)
  2021-03-08 15:00 ` [PATCH 3/4] cifs: return proper error code in statfs(2) Paulo Alcantara
@ 2021-03-08 15:13   ` Aurélien Aptel
  2021-03-08 21:08   ` Steve French
  2021-03-09  0:40   ` ronnie sahlberg
  2 siblings, 0 replies; 16+ messages in thread
From: Aurélien Aptel @ 2021-03-08 15:13 UTC (permalink / raw)
  To: Paulo Alcantara, linux-cifs, smfrench; +Cc: Paulo Alcantara


Reviewed-by: Aurelien Aptel <aaptel@suse.com>

-- 
Aurélien Aptel / SUSE Labs Samba Team
GPG: 1839 CB5F 9F5B FB9B AA97  8C99 03C8 A49B 521B D5D3
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg, DE
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah HRB 247165 (AG München)


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 4/4] cifs: do not send close in compound create+close requests
  2021-03-08 15:00 ` [PATCH 4/4] cifs: do not send close in compound create+close requests Paulo Alcantara
@ 2021-03-08 15:29   ` Aurélien Aptel
  2021-03-08 21:06   ` Steve French
  2021-03-09  0:43   ` ronnie sahlberg
  2 siblings, 0 replies; 16+ messages in thread
From: Aurélien Aptel @ 2021-03-08 15:29 UTC (permalink / raw)
  To: Paulo Alcantara, linux-cifs, smfrench; +Cc: Paulo Alcantara

Paulo Alcantara <pc@cjr.nz> writes:
> In case of interrupted syscalls, prevent sending CLOSE commands for
> compound CREATE+CLOSE requests by introducing an
> CIFS_CP_CREATE_CLOSE_OP flag to indicate lower layers that it should
> not send a CLOSE command to the MIDs corresponding the compound
> CREATE+CLOSE request.

Sounds and looks good to me, good catch!

Reviewed-by: Aurelien Aptel <aaptel@suse.com>

-- 
Aurélien Aptel / SUSE Labs Samba Team
GPG: 1839 CB5F 9F5B FB9B AA97  8C99 03C8 A49B 521B D5D3
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg, DE
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah HRB 247165 (AG München)


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 4/4] cifs: do not send close in compound create+close requests
  2021-03-08 15:00 ` [PATCH 4/4] cifs: do not send close in compound create+close requests Paulo Alcantara
  2021-03-08 15:29   ` Aurélien Aptel
@ 2021-03-08 21:06   ` Steve French
  2021-03-08 22:22     ` Paulo Alcantara
  2021-03-09  0:43   ` ronnie sahlberg
  2 siblings, 1 reply; 16+ messages in thread
From: Steve French @ 2021-03-08 21:06 UTC (permalink / raw)
  To: Paulo Alcantara; +Cc: CIFS

stable?

On Mon, Mar 8, 2021 at 9:01 AM Paulo Alcantara <pc@cjr.nz> wrote:
>
> In case of interrupted syscalls, prevent sending CLOSE commands for
> compound CREATE+CLOSE requests by introducing an
> CIFS_CP_CREATE_CLOSE_OP flag to indicate lower layers that it should
> not send a CLOSE command to the MIDs corresponding the compound
> CREATE+CLOSE request.
>
> A simple reproducer:
>
>     #!/bin/bash
>
>     mount //server/share /mnt -o username=foo,password=***
>     tc qdisc add dev eth0 root netem delay 450ms
>     stat -f /mnt &>/dev/null & pid=$!
>     sleep 0.01
>     kill $pid
>     tc qdisc del dev eth0 root
>     umount /mnt
>
> Before patch:
>
>     ...
>     6 0.256893470 192.168.122.2 → 192.168.122.15 SMB2 402 Create Request File: ;GetInfo Request FS_INFO/FileFsFullSizeInformation;Close Request
>     7 0.257144491 192.168.122.15 → 192.168.122.2 SMB2 498 Create Response File: ;GetInfo Response;Close Response
>     9 0.260798209 192.168.122.2 → 192.168.122.15 SMB2 146 Close Request File:
>    10 0.260841089 192.168.122.15 → 192.168.122.2 SMB2 130 Close Response, Error: STATUS_FILE_CLOSED
>
> Signed-off-by: Paulo Alcantara (SUSE) <pc@cjr.nz>
> ---
>  fs/cifs/cifsglob.h  | 19 ++++++++++---------
>  fs/cifs/smb2inode.c |  1 +
>  fs/cifs/smb2misc.c  |  8 ++++----
>  fs/cifs/smb2ops.c   | 10 +++++-----
>  fs/cifs/smb2proto.h |  3 +--
>  fs/cifs/transport.c |  2 +-
>  6 files changed, 22 insertions(+), 21 deletions(-)
>
> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> index 3de3c5908a72..31fc8695abd6 100644
> --- a/fs/cifs/cifsglob.h
> +++ b/fs/cifs/cifsglob.h
> @@ -257,7 +257,7 @@ struct smb_version_operations {
>         /* verify the message */
>         int (*check_message)(char *, unsigned int, struct TCP_Server_Info *);
>         bool (*is_oplock_break)(char *, struct TCP_Server_Info *);
> -       int (*handle_cancelled_mid)(char *, struct TCP_Server_Info *);
> +       int (*handle_cancelled_mid)(struct mid_q_entry *, struct TCP_Server_Info *);
>         void (*downgrade_oplock)(struct TCP_Server_Info *server,
>                                  struct cifsInodeInfo *cinode, __u32 oplock,
>                                  unsigned int epoch, bool *purge_cache);
> @@ -1705,16 +1705,17 @@ static inline bool is_retryable_error(int error)
>  #define   CIFS_NO_RSP_BUF   0x040    /* no response buffer required */
>
>  /* Type of request operation */
> -#define   CIFS_ECHO_OP      0x080    /* echo request */
> -#define   CIFS_OBREAK_OP   0x0100    /* oplock break request */
> -#define   CIFS_NEG_OP      0x0200    /* negotiate request */
> +#define   CIFS_ECHO_OP            0x080  /* echo request */
> +#define   CIFS_OBREAK_OP          0x0100 /* oplock break request */
> +#define   CIFS_NEG_OP             0x0200 /* negotiate request */
> +#define   CIFS_CP_CREATE_CLOSE_OP 0x0400 /* compound create+close request */
>  /* Lower bitmask values are reserved by others below. */
> -#define   CIFS_SESS_OP     0x2000    /* session setup request */
> -#define   CIFS_OP_MASK     0x2380    /* mask request type */
> +#define   CIFS_SESS_OP            0x2000 /* session setup request */
> +#define   CIFS_OP_MASK            0x2780 /* mask request type */
>
> -#define   CIFS_HAS_CREDITS 0x0400    /* already has credits */
> -#define   CIFS_TRANSFORM_REQ 0x0800    /* transform request before sending */
> -#define   CIFS_NO_SRV_RSP    0x1000    /* there is no server response */
> +#define   CIFS_HAS_CREDITS        0x0400 /* already has credits */
> +#define   CIFS_TRANSFORM_REQ      0x0800 /* transform request before sending */
> +#define   CIFS_NO_SRV_RSP         0x1000 /* there is no server response */
>
>  /* Security Flags: indicate type of session setup needed */
>  #define   CIFSSEC_MAY_SIGN     0x00001
> diff --git a/fs/cifs/smb2inode.c b/fs/cifs/smb2inode.c
> index 1f900b81c34a..a718dc77e604 100644
> --- a/fs/cifs/smb2inode.c
> +++ b/fs/cifs/smb2inode.c
> @@ -358,6 +358,7 @@ smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon,
>         if (cfile)
>                 goto after_close;
>         /* Close */
> +       flags |= CIFS_CP_CREATE_CLOSE_OP;
>         rqst[num_rqst].rq_iov = &vars->close_iov[0];
>         rqst[num_rqst].rq_nvec = 1;
>         rc = SMB2_close_init(tcon, server,
> diff --git a/fs/cifs/smb2misc.c b/fs/cifs/smb2misc.c
> index 0a55a77d94de..c99966121757 100644
> --- a/fs/cifs/smb2misc.c
> +++ b/fs/cifs/smb2misc.c
> @@ -844,14 +844,14 @@ smb2_handle_cancelled_close(struct cifs_tcon *tcon, __u64 persistent_fid,
>  }
>
>  int
> -smb2_handle_cancelled_mid(char *buffer, struct TCP_Server_Info *server)
> +smb2_handle_cancelled_mid(struct mid_q_entry *mid, struct TCP_Server_Info *server)
>  {
> -       struct smb2_sync_hdr *sync_hdr = (struct smb2_sync_hdr *)buffer;
> -       struct smb2_create_rsp *rsp = (struct smb2_create_rsp *)buffer;
> +       struct smb2_sync_hdr *sync_hdr = mid->resp_buf;
> +       struct smb2_create_rsp *rsp = mid->resp_buf;
>         struct cifs_tcon *tcon;
>         int rc;
>
> -       if (sync_hdr->Command != SMB2_CREATE ||
> +       if ((mid->optype & CIFS_CP_CREATE_CLOSE_OP) || sync_hdr->Command != SMB2_CREATE ||
>             sync_hdr->Status != STATUS_SUCCESS)
>                 return 0;
>
> diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
> index f5087295424c..9bae7e8deb09 100644
> --- a/fs/cifs/smb2ops.c
> +++ b/fs/cifs/smb2ops.c
> @@ -1195,7 +1195,7 @@ smb2_set_ea(const unsigned int xid, struct cifs_tcon *tcon,
>         struct TCP_Server_Info *server = cifs_pick_channel(ses);
>         __le16 *utf16_path = NULL;
>         int ea_name_len = strlen(ea_name);
> -       int flags = 0;
> +       int flags = CIFS_CP_CREATE_CLOSE_OP;
>         int len;
>         struct smb_rqst rqst[3];
>         int resp_buftype[3];
> @@ -1573,7 +1573,7 @@ smb2_ioctl_query_info(const unsigned int xid,
>         struct smb_query_info qi;
>         struct smb_query_info __user *pqi;
>         int rc = 0;
> -       int flags = 0;
> +       int flags = CIFS_CP_CREATE_CLOSE_OP;
>         struct smb2_query_info_rsp *qi_rsp = NULL;
>         struct smb2_ioctl_rsp *io_rsp = NULL;
>         void *buffer = NULL;
> @@ -2577,7 +2577,7 @@ smb2_query_info_compound(const unsigned int xid, struct cifs_tcon *tcon,
>  {
>         struct cifs_ses *ses = tcon->ses;
>         struct TCP_Server_Info *server = cifs_pick_channel(ses);
> -       int flags = 0;
> +       int flags = CIFS_CP_CREATE_CLOSE_OP;
>         struct smb_rqst rqst[3];
>         int resp_buftype[3];
>         struct kvec rsp_iov[3];
> @@ -2975,7 +2975,7 @@ smb2_query_symlink(const unsigned int xid, struct cifs_tcon *tcon,
>         unsigned int sub_offset;
>         unsigned int print_len;
>         unsigned int print_offset;
> -       int flags = 0;
> +       int flags = CIFS_CP_CREATE_CLOSE_OP;
>         struct smb_rqst rqst[3];
>         int resp_buftype[3];
>         struct kvec rsp_iov[3];
> @@ -3157,7 +3157,7 @@ smb2_query_reparse_tag(const unsigned int xid, struct cifs_tcon *tcon,
>         struct cifs_open_parms oparms;
>         struct cifs_fid fid;
>         struct TCP_Server_Info *server = cifs_pick_channel(tcon->ses);
> -       int flags = 0;
> +       int flags = CIFS_CP_CREATE_CLOSE_OP;
>         struct smb_rqst rqst[3];
>         int resp_buftype[3];
>         struct kvec rsp_iov[3];
> diff --git a/fs/cifs/smb2proto.h b/fs/cifs/smb2proto.h
> index 9565e27681a5..a2eb34a8d9c9 100644
> --- a/fs/cifs/smb2proto.h
> +++ b/fs/cifs/smb2proto.h
> @@ -246,8 +246,7 @@ extern int SMB2_oplock_break(const unsigned int xid, struct cifs_tcon *tcon,
>  extern int smb2_handle_cancelled_close(struct cifs_tcon *tcon,
>                                        __u64 persistent_fid,
>                                        __u64 volatile_fid);
> -extern int smb2_handle_cancelled_mid(char *buffer,
> -                                       struct TCP_Server_Info *server);
> +extern int smb2_handle_cancelled_mid(struct mid_q_entry *mid, struct TCP_Server_Info *server);
>  void smb2_cancelled_close_fid(struct work_struct *work);
>  extern int SMB2_QFS_info(const unsigned int xid, struct cifs_tcon *tcon,
>                          u64 persistent_file_id, u64 volatile_file_id,
> diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c
> index f62f512e2cb1..9438a0c35473 100644
> --- a/fs/cifs/transport.c
> +++ b/fs/cifs/transport.c
> @@ -109,7 +109,7 @@ static void _cifs_mid_q_entry_release(struct kref *refcount)
>         if (midEntry->resp_buf && (midEntry->mid_flags & MID_WAIT_CANCELLED) &&
>             midEntry->mid_state == MID_RESPONSE_RECEIVED &&
>             server->ops->handle_cancelled_mid)
> -               server->ops->handle_cancelled_mid(midEntry->resp_buf, server);
> +               server->ops->handle_cancelled_mid(midEntry, server);
>
>         midEntry->mid_state = MID_FREE;
>         atomic_dec(&midCount);
> --
> 2.30.1
>


-- 
Thanks,

Steve

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/4] cifs: return proper error code in statfs(2)
  2021-03-08 15:00 ` [PATCH 3/4] cifs: return proper error code in statfs(2) Paulo Alcantara
  2021-03-08 15:13   ` Aurélien Aptel
@ 2021-03-08 21:08   ` Steve French
  2021-03-08 22:21     ` Paulo Alcantara
  2021-03-09  0:40   ` ronnie sahlberg
  2 siblings, 1 reply; 16+ messages in thread
From: Steve French @ 2021-03-08 21:08 UTC (permalink / raw)
  To: Paulo Alcantara; +Cc: CIFS

cc:stable?

On Mon, Mar 8, 2021 at 9:01 AM Paulo Alcantara <pc@cjr.nz> wrote:
>
> In cifs_statfs(), if server->ops->queryfs is not NULL, then we should
> use its return value rather than always returning 0.  Instead, use rc
> variable as it is properly set to 0 in case there is no
> server->ops->queryfs.
>
> Signed-off-by: Paulo Alcantara (SUSE) <pc@cjr.nz>
> ---
>  fs/cifs/cifsfs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
> index d43e935d2df4..099ad9f3660b 100644
> --- a/fs/cifs/cifsfs.c
> +++ b/fs/cifs/cifsfs.c
> @@ -290,7 +290,7 @@ cifs_statfs(struct dentry *dentry, struct kstatfs *buf)
>                 rc = server->ops->queryfs(xid, tcon, cifs_sb, buf);
>
>         free_xid(xid);
> -       return 0;
> +       return rc;
>  }
>
>  static long cifs_fallocate(struct file *file, int mode, loff_t off, loff_t len)
> --
> 2.30.1
>


-- 
Thanks,

Steve

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/4] cifs: return proper error code in statfs(2)
  2021-03-08 21:08   ` Steve French
@ 2021-03-08 22:21     ` Paulo Alcantara
  0 siblings, 0 replies; 16+ messages in thread
From: Paulo Alcantara @ 2021-03-08 22:21 UTC (permalink / raw)
  To: Steve French; +Cc: CIFS

Steve French <smfrench@gmail.com> writes:

> cc:stable?

Yes, please.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 4/4] cifs: do not send close in compound create+close requests
  2021-03-08 21:06   ` Steve French
@ 2021-03-08 22:22     ` Paulo Alcantara
  0 siblings, 0 replies; 16+ messages in thread
From: Paulo Alcantara @ 2021-03-08 22:22 UTC (permalink / raw)
  To: Steve French; +Cc: CIFS

Steve French <smfrench@gmail.com> writes:

> stable?

Yes, please.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/4] cifs: print MIDs in decimal notation
  2021-03-08 15:00 [PATCH 1/4] cifs: print MIDs in decimal notation Paulo Alcantara
                   ` (3 preceding siblings ...)
  2021-03-08 15:10 ` [PATCH 1/4] cifs: print MIDs in decimal notation Aurélien Aptel
@ 2021-03-09  0:39 ` ronnie sahlberg
  4 siblings, 0 replies; 16+ messages in thread
From: ronnie sahlberg @ 2021-03-09  0:39 UTC (permalink / raw)
  To: Paulo Alcantara; +Cc: linux-cifs, Steve French

reviewed-by me

On Tue, Mar 9, 2021 at 1:02 AM Paulo Alcantara <pc@cjr.nz> wrote:
>
> The MIDs are mostly printed as decimal, so let's make it consistent.
>
> Signed-off-by: Paulo Alcantara (SUSE) <pc@cjr.nz>
> ---
>  fs/cifs/cifs_debug.c | 2 +-
>  fs/cifs/connect.c    | 4 ++--
>  fs/cifs/smb2misc.c   | 2 +-
>  3 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/fs/cifs/cifs_debug.c b/fs/cifs/cifs_debug.c
> index 3aedc484e440..88a7958170ee 100644
> --- a/fs/cifs/cifs_debug.c
> +++ b/fs/cifs/cifs_debug.c
> @@ -207,7 +207,7 @@ static int cifs_debug_files_proc_show(struct seq_file *m, void *v)
>                                                 from_kuid(&init_user_ns, cfile->uid),
>                                                 cfile->dentry);
>  #ifdef CONFIG_CIFS_DEBUG2
> -                                       seq_printf(m, " 0x%llx\n", cfile->fid.mid);
> +                                       seq_printf(m, " %llu\n", cfile->fid.mid);
>  #else
>                                         seq_printf(m, "\n");
>  #endif /* CIFS_DEBUG2 */
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index 68642e3d4270..eec8a2052da2 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -741,7 +741,7 @@ static void clean_demultiplex_info(struct TCP_Server_Info *server)
>                 spin_lock(&GlobalMid_Lock);
>                 list_for_each_safe(tmp, tmp2, &server->pending_mid_q) {
>                         mid_entry = list_entry(tmp, struct mid_q_entry, qhead);
> -                       cifs_dbg(FYI, "Clearing mid 0x%llx\n", mid_entry->mid);
> +                       cifs_dbg(FYI, "Clearing mid %llu\n", mid_entry->mid);
>                         kref_get(&mid_entry->refcount);
>                         mid_entry->mid_state = MID_SHUTDOWN;
>                         list_move(&mid_entry->qhead, &dispose_list);
> @@ -752,7 +752,7 @@ static void clean_demultiplex_info(struct TCP_Server_Info *server)
>                 /* now walk dispose list and issue callbacks */
>                 list_for_each_safe(tmp, tmp2, &dispose_list) {
>                         mid_entry = list_entry(tmp, struct mid_q_entry, qhead);
> -                       cifs_dbg(FYI, "Callback mid 0x%llx\n", mid_entry->mid);
> +                       cifs_dbg(FYI, "Callback mid %llu\n", mid_entry->mid);
>                         list_del_init(&mid_entry->qhead);
>                         mid_entry->callback(mid_entry);
>                         cifs_mid_q_entry_release(mid_entry);
> diff --git a/fs/cifs/smb2misc.c b/fs/cifs/smb2misc.c
> index 3ea3bda64083..0a55a77d94de 100644
> --- a/fs/cifs/smb2misc.c
> +++ b/fs/cifs/smb2misc.c
> @@ -767,7 +767,7 @@ smb2_cancelled_close_fid(struct work_struct *work)
>         int rc;
>
>         if (cancelled->mid)
> -               cifs_tcon_dbg(VFS, "Close unmatched open for MID:%llx\n",
> +               cifs_tcon_dbg(VFS, "Close unmatched open for MID:%llu\n",
>                               cancelled->mid);
>         else
>                 cifs_tcon_dbg(VFS, "Close interrupted close\n");
> --
> 2.30.1
>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/4] cifs: change noisy error message to FYI
  2021-03-08 15:00 ` [PATCH 2/4] cifs: change noisy error message to FYI Paulo Alcantara
  2021-03-08 15:12   ` Aurélien Aptel
@ 2021-03-09  0:40   ` ronnie sahlberg
  1 sibling, 0 replies; 16+ messages in thread
From: ronnie sahlberg @ 2021-03-09  0:40 UTC (permalink / raw)
  To: Paulo Alcantara; +Cc: linux-cifs, Steve French

reviewed-by me

On Tue, Mar 9, 2021 at 1:02 AM Paulo Alcantara <pc@cjr.nz> wrote:
>
> A customer has reported that their dmesg were being flooded by
>
>   CIFS: VFS: \\server Cancelling wait for mid xxx cmd: a
>   CIFS: VFS: \\server Cancelling wait for mid yyy cmd: b
>   CIFS: VFS: \\server Cancelling wait for mid zzz cmd: c
>
> because some processes that were performing statfs(2) on the share had
> been interrupted due to their automount setup when certain users
> logged in and out.
>
> Change it to FYI as they should be mostly informative rather than
> error messages.
>
> Signed-off-by: Paulo Alcantara (SUSE) <pc@cjr.nz>
> ---
>  fs/cifs/transport.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c
> index 7bb1584b3724..f62f512e2cb1 100644
> --- a/fs/cifs/transport.c
> +++ b/fs/cifs/transport.c
> @@ -1248,7 +1248,7 @@ compound_send_recv(const unsigned int xid, struct cifs_ses *ses,
>         }
>         if (rc != 0) {
>                 for (; i < num_rqst; i++) {
> -                       cifs_server_dbg(VFS, "Cancelling wait for mid %llu cmd: %d\n",
> +                       cifs_server_dbg(FYI, "Cancelling wait for mid %llu cmd: %d\n",
>                                  midQ[i]->mid, le16_to_cpu(midQ[i]->command));
>                         send_cancel(server, &rqst[i], midQ[i]);
>                         spin_lock(&GlobalMid_Lock);
> --
> 2.30.1
>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/4] cifs: return proper error code in statfs(2)
  2021-03-08 15:00 ` [PATCH 3/4] cifs: return proper error code in statfs(2) Paulo Alcantara
  2021-03-08 15:13   ` Aurélien Aptel
  2021-03-08 21:08   ` Steve French
@ 2021-03-09  0:40   ` ronnie sahlberg
  2 siblings, 0 replies; 16+ messages in thread
From: ronnie sahlberg @ 2021-03-09  0:40 UTC (permalink / raw)
  To: Paulo Alcantara; +Cc: linux-cifs, Steve French

reviewed-by me

On Tue, Mar 9, 2021 at 1:02 AM Paulo Alcantara <pc@cjr.nz> wrote:
>
> In cifs_statfs(), if server->ops->queryfs is not NULL, then we should
> use its return value rather than always returning 0.  Instead, use rc
> variable as it is properly set to 0 in case there is no
> server->ops->queryfs.
>
> Signed-off-by: Paulo Alcantara (SUSE) <pc@cjr.nz>
> ---
>  fs/cifs/cifsfs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
> index d43e935d2df4..099ad9f3660b 100644
> --- a/fs/cifs/cifsfs.c
> +++ b/fs/cifs/cifsfs.c
> @@ -290,7 +290,7 @@ cifs_statfs(struct dentry *dentry, struct kstatfs *buf)
>                 rc = server->ops->queryfs(xid, tcon, cifs_sb, buf);
>
>         free_xid(xid);
> -       return 0;
> +       return rc;
>  }
>
>  static long cifs_fallocate(struct file *file, int mode, loff_t off, loff_t len)
> --
> 2.30.1
>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 4/4] cifs: do not send close in compound create+close requests
  2021-03-08 15:00 ` [PATCH 4/4] cifs: do not send close in compound create+close requests Paulo Alcantara
  2021-03-08 15:29   ` Aurélien Aptel
  2021-03-08 21:06   ` Steve French
@ 2021-03-09  0:43   ` ronnie sahlberg
  2 siblings, 0 replies; 16+ messages in thread
From: ronnie sahlberg @ 2021-03-09  0:43 UTC (permalink / raw)
  To: Paulo Alcantara; +Cc: linux-cifs, Steve French

reviewed-by me

On Tue, Mar 9, 2021 at 1:02 AM Paulo Alcantara <pc@cjr.nz> wrote:
>
> In case of interrupted syscalls, prevent sending CLOSE commands for
> compound CREATE+CLOSE requests by introducing an
> CIFS_CP_CREATE_CLOSE_OP flag to indicate lower layers that it should
> not send a CLOSE command to the MIDs corresponding the compound
> CREATE+CLOSE request.
>
> A simple reproducer:
>
>     #!/bin/bash
>
>     mount //server/share /mnt -o username=foo,password=***
>     tc qdisc add dev eth0 root netem delay 450ms
>     stat -f /mnt &>/dev/null & pid=$!
>     sleep 0.01
>     kill $pid
>     tc qdisc del dev eth0 root
>     umount /mnt
>
> Before patch:
>
>     ...
>     6 0.256893470 192.168.122.2 → 192.168.122.15 SMB2 402 Create Request File: ;GetInfo Request FS_INFO/FileFsFullSizeInformation;Close Request
>     7 0.257144491 192.168.122.15 → 192.168.122.2 SMB2 498 Create Response File: ;GetInfo Response;Close Response
>     9 0.260798209 192.168.122.2 → 192.168.122.15 SMB2 146 Close Request File:
>    10 0.260841089 192.168.122.15 → 192.168.122.2 SMB2 130 Close Response, Error: STATUS_FILE_CLOSED
>
> Signed-off-by: Paulo Alcantara (SUSE) <pc@cjr.nz>
> ---
>  fs/cifs/cifsglob.h  | 19 ++++++++++---------
>  fs/cifs/smb2inode.c |  1 +
>  fs/cifs/smb2misc.c  |  8 ++++----
>  fs/cifs/smb2ops.c   | 10 +++++-----
>  fs/cifs/smb2proto.h |  3 +--
>  fs/cifs/transport.c |  2 +-
>  6 files changed, 22 insertions(+), 21 deletions(-)
>
> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> index 3de3c5908a72..31fc8695abd6 100644
> --- a/fs/cifs/cifsglob.h
> +++ b/fs/cifs/cifsglob.h
> @@ -257,7 +257,7 @@ struct smb_version_operations {
>         /* verify the message */
>         int (*check_message)(char *, unsigned int, struct TCP_Server_Info *);
>         bool (*is_oplock_break)(char *, struct TCP_Server_Info *);
> -       int (*handle_cancelled_mid)(char *, struct TCP_Server_Info *);
> +       int (*handle_cancelled_mid)(struct mid_q_entry *, struct TCP_Server_Info *);
>         void (*downgrade_oplock)(struct TCP_Server_Info *server,
>                                  struct cifsInodeInfo *cinode, __u32 oplock,
>                                  unsigned int epoch, bool *purge_cache);
> @@ -1705,16 +1705,17 @@ static inline bool is_retryable_error(int error)
>  #define   CIFS_NO_RSP_BUF   0x040    /* no response buffer required */
>
>  /* Type of request operation */
> -#define   CIFS_ECHO_OP      0x080    /* echo request */
> -#define   CIFS_OBREAK_OP   0x0100    /* oplock break request */
> -#define   CIFS_NEG_OP      0x0200    /* negotiate request */
> +#define   CIFS_ECHO_OP            0x080  /* echo request */
> +#define   CIFS_OBREAK_OP          0x0100 /* oplock break request */
> +#define   CIFS_NEG_OP             0x0200 /* negotiate request */
> +#define   CIFS_CP_CREATE_CLOSE_OP 0x0400 /* compound create+close request */
>  /* Lower bitmask values are reserved by others below. */
> -#define   CIFS_SESS_OP     0x2000    /* session setup request */
> -#define   CIFS_OP_MASK     0x2380    /* mask request type */
> +#define   CIFS_SESS_OP            0x2000 /* session setup request */
> +#define   CIFS_OP_MASK            0x2780 /* mask request type */
>
> -#define   CIFS_HAS_CREDITS 0x0400    /* already has credits */
> -#define   CIFS_TRANSFORM_REQ 0x0800    /* transform request before sending */
> -#define   CIFS_NO_SRV_RSP    0x1000    /* there is no server response */
> +#define   CIFS_HAS_CREDITS        0x0400 /* already has credits */
> +#define   CIFS_TRANSFORM_REQ      0x0800 /* transform request before sending */
> +#define   CIFS_NO_SRV_RSP         0x1000 /* there is no server response */
>
>  /* Security Flags: indicate type of session setup needed */
>  #define   CIFSSEC_MAY_SIGN     0x00001
> diff --git a/fs/cifs/smb2inode.c b/fs/cifs/smb2inode.c
> index 1f900b81c34a..a718dc77e604 100644
> --- a/fs/cifs/smb2inode.c
> +++ b/fs/cifs/smb2inode.c
> @@ -358,6 +358,7 @@ smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon,
>         if (cfile)
>                 goto after_close;
>         /* Close */
> +       flags |= CIFS_CP_CREATE_CLOSE_OP;
>         rqst[num_rqst].rq_iov = &vars->close_iov[0];
>         rqst[num_rqst].rq_nvec = 1;
>         rc = SMB2_close_init(tcon, server,
> diff --git a/fs/cifs/smb2misc.c b/fs/cifs/smb2misc.c
> index 0a55a77d94de..c99966121757 100644
> --- a/fs/cifs/smb2misc.c
> +++ b/fs/cifs/smb2misc.c
> @@ -844,14 +844,14 @@ smb2_handle_cancelled_close(struct cifs_tcon *tcon, __u64 persistent_fid,
>  }
>
>  int
> -smb2_handle_cancelled_mid(char *buffer, struct TCP_Server_Info *server)
> +smb2_handle_cancelled_mid(struct mid_q_entry *mid, struct TCP_Server_Info *server)
>  {
> -       struct smb2_sync_hdr *sync_hdr = (struct smb2_sync_hdr *)buffer;
> -       struct smb2_create_rsp *rsp = (struct smb2_create_rsp *)buffer;
> +       struct smb2_sync_hdr *sync_hdr = mid->resp_buf;
> +       struct smb2_create_rsp *rsp = mid->resp_buf;
>         struct cifs_tcon *tcon;
>         int rc;
>
> -       if (sync_hdr->Command != SMB2_CREATE ||
> +       if ((mid->optype & CIFS_CP_CREATE_CLOSE_OP) || sync_hdr->Command != SMB2_CREATE ||
>             sync_hdr->Status != STATUS_SUCCESS)
>                 return 0;
>
> diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
> index f5087295424c..9bae7e8deb09 100644
> --- a/fs/cifs/smb2ops.c
> +++ b/fs/cifs/smb2ops.c
> @@ -1195,7 +1195,7 @@ smb2_set_ea(const unsigned int xid, struct cifs_tcon *tcon,
>         struct TCP_Server_Info *server = cifs_pick_channel(ses);
>         __le16 *utf16_path = NULL;
>         int ea_name_len = strlen(ea_name);
> -       int flags = 0;
> +       int flags = CIFS_CP_CREATE_CLOSE_OP;
>         int len;
>         struct smb_rqst rqst[3];
>         int resp_buftype[3];
> @@ -1573,7 +1573,7 @@ smb2_ioctl_query_info(const unsigned int xid,
>         struct smb_query_info qi;
>         struct smb_query_info __user *pqi;
>         int rc = 0;
> -       int flags = 0;
> +       int flags = CIFS_CP_CREATE_CLOSE_OP;
>         struct smb2_query_info_rsp *qi_rsp = NULL;
>         struct smb2_ioctl_rsp *io_rsp = NULL;
>         void *buffer = NULL;
> @@ -2577,7 +2577,7 @@ smb2_query_info_compound(const unsigned int xid, struct cifs_tcon *tcon,
>  {
>         struct cifs_ses *ses = tcon->ses;
>         struct TCP_Server_Info *server = cifs_pick_channel(ses);
> -       int flags = 0;
> +       int flags = CIFS_CP_CREATE_CLOSE_OP;
>         struct smb_rqst rqst[3];
>         int resp_buftype[3];
>         struct kvec rsp_iov[3];
> @@ -2975,7 +2975,7 @@ smb2_query_symlink(const unsigned int xid, struct cifs_tcon *tcon,
>         unsigned int sub_offset;
>         unsigned int print_len;
>         unsigned int print_offset;
> -       int flags = 0;
> +       int flags = CIFS_CP_CREATE_CLOSE_OP;
>         struct smb_rqst rqst[3];
>         int resp_buftype[3];
>         struct kvec rsp_iov[3];
> @@ -3157,7 +3157,7 @@ smb2_query_reparse_tag(const unsigned int xid, struct cifs_tcon *tcon,
>         struct cifs_open_parms oparms;
>         struct cifs_fid fid;
>         struct TCP_Server_Info *server = cifs_pick_channel(tcon->ses);
> -       int flags = 0;
> +       int flags = CIFS_CP_CREATE_CLOSE_OP;
>         struct smb_rqst rqst[3];
>         int resp_buftype[3];
>         struct kvec rsp_iov[3];
> diff --git a/fs/cifs/smb2proto.h b/fs/cifs/smb2proto.h
> index 9565e27681a5..a2eb34a8d9c9 100644
> --- a/fs/cifs/smb2proto.h
> +++ b/fs/cifs/smb2proto.h
> @@ -246,8 +246,7 @@ extern int SMB2_oplock_break(const unsigned int xid, struct cifs_tcon *tcon,
>  extern int smb2_handle_cancelled_close(struct cifs_tcon *tcon,
>                                        __u64 persistent_fid,
>                                        __u64 volatile_fid);
> -extern int smb2_handle_cancelled_mid(char *buffer,
> -                                       struct TCP_Server_Info *server);
> +extern int smb2_handle_cancelled_mid(struct mid_q_entry *mid, struct TCP_Server_Info *server);
>  void smb2_cancelled_close_fid(struct work_struct *work);
>  extern int SMB2_QFS_info(const unsigned int xid, struct cifs_tcon *tcon,
>                          u64 persistent_file_id, u64 volatile_file_id,
> diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c
> index f62f512e2cb1..9438a0c35473 100644
> --- a/fs/cifs/transport.c
> +++ b/fs/cifs/transport.c
> @@ -109,7 +109,7 @@ static void _cifs_mid_q_entry_release(struct kref *refcount)
>         if (midEntry->resp_buf && (midEntry->mid_flags & MID_WAIT_CANCELLED) &&
>             midEntry->mid_state == MID_RESPONSE_RECEIVED &&
>             server->ops->handle_cancelled_mid)
> -               server->ops->handle_cancelled_mid(midEntry->resp_buf, server);
> +               server->ops->handle_cancelled_mid(midEntry, server);
>
>         midEntry->mid_state = MID_FREE;
>         atomic_dec(&midCount);
> --
> 2.30.1
>

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2021-03-09  0:44 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-08 15:00 [PATCH 1/4] cifs: print MIDs in decimal notation Paulo Alcantara
2021-03-08 15:00 ` [PATCH 2/4] cifs: change noisy error message to FYI Paulo Alcantara
2021-03-08 15:12   ` Aurélien Aptel
2021-03-09  0:40   ` ronnie sahlberg
2021-03-08 15:00 ` [PATCH 3/4] cifs: return proper error code in statfs(2) Paulo Alcantara
2021-03-08 15:13   ` Aurélien Aptel
2021-03-08 21:08   ` Steve French
2021-03-08 22:21     ` Paulo Alcantara
2021-03-09  0:40   ` ronnie sahlberg
2021-03-08 15:00 ` [PATCH 4/4] cifs: do not send close in compound create+close requests Paulo Alcantara
2021-03-08 15:29   ` Aurélien Aptel
2021-03-08 21:06   ` Steve French
2021-03-08 22:22     ` Paulo Alcantara
2021-03-09  0:43   ` ronnie sahlberg
2021-03-08 15:10 ` [PATCH 1/4] cifs: print MIDs in decimal notation Aurélien Aptel
2021-03-09  0:39 ` ronnie sahlberg

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.