Skip to contentSkip to conversation

cifs: smbd: Make upper layer decide when to destroy the transport

PATCH Superseded Long Li 3 participants 12 messages May 7, 2018 – May 8, 2018

Subsystems (3)

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Long Li

author
#0 lore raw

PATCH 1/7 cifs: smbd: Make upper layer decide when to destroy the transport

From: Long Li <[email protected]>

On transport recoonect, upper layer CIFS code destroys the current transport and then recoonect. This code path is not used by SMBD, in that SMBD destroys its transport on RDMA disconnect notification independent of CIFS upper layer behavior.

This approach adds some costs to SMBD layer to handle transport shutdown and restart, and to deal with several racing conditions on reconnecting transport.

Re-work this code path by introducing a new smbd_destroy. This function is called form upper layer to ask SMBD to destroy the transport. SMBD will no longer need to destroy the transport by itself while worrying about data transfer is in progress. The upper layer guarantees the transport is locked.

---
 fs/cifs/connect.c   |   9 ++---
 fs/cifs/smbdirect.c | 114 +++++++++++++++++++++++++++++++++++++++++++---------
 fs/cifs/smbdirect.h |   3 +-
 3 files changed, 100 insertions(+), 26 deletions(-)
fs/cifs/connect.c +4 -5
index 83b0234..5db3e9d 100644
@@ -377,7 +377,8 @@ cifs_reconnect(struct TCP_Server_Info *server)
 			 server->ssocket->state, server->ssocket->flags);
 		sock_release(server->ssocket);
 		server->ssocket = NULL;
-	}
+	} else if (cifs_rdma_enabled(server))
+		smbd_destroy(server);
 	server->sequence_number = 0;
 	server->session_estab = false;
 	kfree(server->session_key.response);
@@ -710,10 +711,8 @@ static void clean_demultiplex_info(struct TCP_Server_Info *server)
 	wake_up_all(&server->request_q);
 	/* give those requests time to exit */
 	msleep(125);
-	if (cifs_rdma_enabled(server) && server->smbd_conn) {
-		smbd_destroy(server->smbd_conn);
-		server->smbd_conn = NULL;
-	}
+	if (cifs_rdma_enabled(server))
+		smbd_destroy(server);
 	if (server->ssocket) {
 		sock_release(server->ssocket);
 		server->ssocket = NULL;
fs/cifs/smbdirect.c +94 -20

161 lines not on this page · raw message

fs/cifs/smbdirect.h +2 -1
index f9038da..7849989 100644
@@ -71,6 +71,7 @@ struct smbd_connection {
 	struct completion ri_done;
 	wait_queue_head_t conn_wait;
 	wait_queue_head_t wait_destroy;
+	wait_queue_head_t disconn_wait;
 
 	struct completion negotiate_completion;
 	bool negotiate_done;
@@ -288,7 +289,7 @@ struct smbd_connection *smbd_get_connection(
 /* Reconnect SMBDirect session */
 int smbd_reconnect(struct TCP_Server_Info *server);
 /* Destroy SMBDirect session */
-void smbd_destroy(struct smbd_connection *info);
+void smbd_destroy(struct TCP_Server_Info *server);
 
 /* Interface for carrying upper layer I/O through send/recv */
 int smbd_recv(struct smbd_connection *info, struct msghdr *msg);
Signature · 1 line
2.7.4
author
#1 lore raw

PATCH 2/7 cifs: smbd: Don't destroy transport on RDMA disconnect

From: Long Li <[email protected]>

Now upper layer is handling the transport shutdown and reconnect, remove the code that handling transport shutdown on RDMA disconnect.

---
 fs/cifs/cifs_debug.c |   8 ++--
 fs/cifs/smbdirect.c  | 122 ++++-----------------------------------------------
 fs/cifs/smbdirect.h  |   9 ----
 3 files changed, 11 insertions(+), 128 deletions(-)
fs/cifs/cifs_debug.c +3 -5
index e6025e9..0337ee8 100644
@@ -214,12 +214,10 @@ static int cifs_debug_data_proc_show(struct seq_file *m, void *v)
 			atomic_read(&server->smbd_conn->send_credits),
 			atomic_read(&server->smbd_conn->receive_credits),
 			server->smbd_conn->receive_credit_target);
-		seq_printf(m, "\nPending send_pending: %x send_payload_pending:"
-			" %x smbd_send_pending: %x smbd_recv_pending: %x",
+		seq_printf(m, "\nPending send_pending: %x "
+			"send_payload_pending: %x",
 			atomic_read(&server->smbd_conn->send_pending),
-			atomic_read(&server->smbd_conn->send_payload_pending),
-			server->smbd_conn->smbd_send_pending,
-			server->smbd_conn->smbd_recv_pending);
+			atomic_read(&server->smbd_conn->send_payload_pending));
 		seq_printf(m, "\nReceive buffers count_receive_queue: %x "
 			"count_empty_packet_queue: %x",
 			server->smbd_conn->count_receive_queue,
fs/cifs/smbdirect.c +8 -114

190 lines not on this page · raw message

fs/cifs/smbdirect.h +0 -9
index 7849989..4563c0c 100644
@@ -70,13 +70,11 @@ struct smbd_connection {
 	int ri_rc;
 	struct completion ri_done;
 	wait_queue_head_t conn_wait;
-	wait_queue_head_t wait_destroy;
 	wait_queue_head_t disconn_wait;
 
 	struct completion negotiate_completion;
 	bool negotiate_done;
 
-	struct work_struct destroy_work;
 	struct work_struct disconnect_work;
 	struct work_struct recv_done_work;
 	struct work_struct post_send_credits_work;
@@ -124,13 +122,6 @@ struct smbd_connection {
 	wait_queue_head_t wait_for_mr_cleanup;
 
 	/* Activity accoutning */
-	/* Pending reqeusts issued from upper layer */
-	int smbd_send_pending;
-	wait_queue_head_t wait_smbd_send_pending;
-
-	int smbd_recv_pending;
-	wait_queue_head_t wait_smbd_recv_pending;
-
 	atomic_t send_pending;
 	wait_queue_head_t wait_send_pending;
 	atomic_t send_payload_pending;
Signature · 1 line
2.7.4
author
#2 lore raw

PATCH 3/7 cifs: smbd: Return EINTR when interrupted

In reply to Long Li

From: Long Li <[email protected]>

When packets are waiting for outbound I/O and interrupted, return the proper error code to user process.

---
 fs/cifs/smbdirect.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
fs/cifs/smbdirect.c +2 -2
index 79b025e..b8dd493 100644
@@ -1963,7 +1963,7 @@ static int smbd_recv_buf(struct smbd_connection *info, char *buf,
 			info->transport_status != SMBD_CONNECTED);
 	/* Don't return any data if interrupted */
 	if (rc)
-		return -ENODEV;
+		return rc;
 
 	if (info->transport_status != SMBD_CONNECTED) {
 		log_read(ERR, "disconnected\n");
@@ -1991,7 +1991,7 @@ static int smbd_recv_page(struct smbd_connection *info,
 		info->reassembly_data_length >= to_read ||
 			info->transport_status != SMBD_CONNECTED);
 	if (ret)
-		return 0;
+		return ret;
 
 	/* now we can read from reassembly queue and not sleep */
 	to_address = kmap_atomic(page);
Signature · 1 line
2.7.4
author
#3 lore raw

PATCH 4/7 cifs: smbd: Indicate to retry on transport sending failure

In reply to Long Li

From: Long Li <[email protected]>

Failure to send a packet doesn't mean it's a permanent failure, it can't be returned to user process. This I/O should be retried or failed based on server packet response and transport health. This logic is handled by the upper layer.

Give this decision to upper layer.

---
 fs/cifs/smbdirect.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
fs/cifs/smbdirect.c +3 -2
index b8dd493..74620f5 100644
@@ -851,7 +851,7 @@ static int smbd_create_header(struct smbd_connection *info,
 
 	if (info->transport_status != SMBD_CONNECTED) {
 		log_outgoing(ERR, "disconnected not sending\n");
-		return -ENOENT;
+		return -EAGAIN;
 	}
 	atomic_dec(&info->send_credits);
 
@@ -977,6 +977,7 @@ static int smbd_post_send(struct smbd_connection *info,
 				wake_up(&info->wait_send_pending);
 		}
 		smbd_disconnect_rdma_connection(info);
+		rc = -EAGAIN;
 	} else
 		/* Reset timer for idle connection after packet is sent */
 		mod_delayed_work(info->workqueue, &info->idle_timer_work,
@@ -2062,7 +2063,7 @@ int smbd_send(struct smbd_connection *info, struct smb_rqst *rqst)
 	int rc;
 
 	if (info->transport_status != SMBD_CONNECTED) {
-		rc = -ENODEV;
+		rc = -EAGAIN;
 		goto done;
 	}
 
Signature · 1 line
2.7.4
author
#4 lore raw

PATCH 5/7 cifs: smbd: Protect memory registration using RCU

In reply to Long Li

From: Long Li <[email protected]>

Unlike packet I/O sending path, the memory registration is not locked. They need to be protected when doing transport reconnect or shutdown.

It's not necessary to lock for memory deregistration, since at the time the transport is already destroyed so it's impossible to pick up the wrong transport.

---
 fs/cifs/smb2pdu.c   | 6 ++++++
 fs/cifs/smbdirect.c | 3 +++
 2 files changed, 9 insertions(+)
fs/cifs/smb2pdu.c +6 -0
index 8cd164e..09ca098 100644
@@ -2609,10 +2609,13 @@ smb2_new_read_req(void **buf, unsigned int *total_len,
 		bool need_invalidate =
 			io_parms->tcon->ses->server->dialect == SMB30_PROT_ID;
 
+		rcu_read_lock();
+		rcu_dereference(server->smbd_conn);
 		rdata->mr = smbd_register_mr(
 				server->smbd_conn, rdata->pages,
 				rdata->nr_pages, rdata->tailsz,
 				true, need_invalidate);
+		rcu_read_unlock();
 		if (!rdata->mr)
 			return -ENOBUFS;
 
@@ -2986,10 +2989,13 @@ smb2_async_writev(struct cifs_writedata *wdata,
 		struct smbd_buffer_descriptor_v1 *v1;
 		bool need_invalidate = server->dialect == SMB30_PROT_ID;
 
+		rcu_read_lock();
+		rcu_dereference(server->smbd_conn);
 		wdata->mr = smbd_register_mr(
 				server->smbd_conn, wdata->pages,
 				wdata->nr_pages, wdata->tailsz,
 				false, need_invalidate);
+		rcu_read_unlock();
 		if (!wdata->mr) {
 			rc = -ENOBUFS;
 			goto async_writev_out;
fs/cifs/smbdirect.c +3 -0
index 74620f5..8c46898 100644
@@ -1482,6 +1482,9 @@ void smbd_destroy(struct TCP_Server_Info *server)
 
 	info->transport_status = SMBD_DESTROYED;
 
+	rcu_assign_pointer(server->smbd_conn, NULL);
+	synchronize_rcu();
+
 	destroy_workqueue(info->workqueue);
 	kfree(info);
 }
Signature · 1 line
2.7.4
#5 lore raw

Re: [PATCH 5/7] cifs: smbd: Protect memory registration using RCU

Thank you for the patch! Perhaps something to improve:

Hi Long,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on cifs/for-next]
[also build test WARNING on v4.17-rc4 next-20180507]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Long-Li/cifs-smbd-Make-upper-layer-decide-when-to-destroy-the-transport/20180508-110150
base:   git://git.samba.org/sfrench/cifs-2.6.git for-next
reproduce:
        # apt-get install sparse
        make ARCH=x86_64 allmodconfig
        make C=1 CF=-D__CHECK_ENDIAN__

sparse warnings: (new ones prefixed by >>)

fs/cifs/smb2pdu.c:110:47: sparse: expression using sizeof(void) fs/cifs/smb2pdu.c:681:26: sparse: expression using sizeof(void)

>> fs/cifs/smb2pdu.c:2613:17: sparse: incompatible types in comparison expression (different address spaces)

fs/cifs/smb2pdu.c:2993:17: sparse: incompatible types in comparison expression (different address spaces) fs/cifs/smb2pdu.c:3251:23: sparse: expression using sizeof(void) fs/cifs/smb2pdu.c:3251:23: sparse: expression using sizeof(void) fs/cifs/smb2pdu.c:3252:23: sparse: expression using sizeof(void) fs/cifs/smb2pdu.c:3713:17: sparse: expression using sizeof(void) fs/cifs/smb2pdu.c:3713:17: sparse: expression using sizeof(void)

vim +2613 fs/cifs/smb2pdu.c

2565
2566 /*
2567 * To form a chain of read requests, any read requests after the first should
2568 * have the end_of_chain boolean set to true.
2569 */
2570 static int
2571 smb2_new_read_req(void **buf, unsigned int *total_len,
2572 struct cifs_io_parms *io_parms, struct cifs_readdata *rdata,
2573 unsigned int remaining_bytes, int request_type)
2574 {
2575 int rc = -EACCES;
2576 struct smb2_read_plain_req *req = NULL;
2577 struct smb2_sync_hdr *shdr;
2578 struct TCP_Server_Info *server;
2579
2580 rc = smb2_plain_req_init(SMB2_READ, io_parms->tcon, (void **) &req,
2581 total_len);
2582 if (rc)
2583 return rc;
2584
2585 server = io_parms->tcon->ses->server;
2586 if (server == NULL)
2587 return -ECONNABORTED;
2588
2589 shdr = &req->sync_hdr;
2590 shdr->ProcessId = cpu_to_le32(io_parms->pid);
2591
2592 req->PersistentFileId = io_parms->persistent_fid;
2593 req->VolatileFileId = io_parms->volatile_fid;
2594 req->ReadChannelInfoOffset = 0; /* reserved */
2595 req->ReadChannelInfoLength = 0; /* reserved */
2596 req->Channel = 0; /* reserved */
2597 req->MinimumCount = 0;
2598 req->Length = cpu_to_le32(io_parms->length);
2599 req->Offset = cpu_to_le64(io_parms->offset);
2600 #ifdef CONFIG_CIFS_SMB_DIRECT
2601 /*
2602 * If we want to do a RDMA write, fill in and append
2603 * smbd_buffer_descriptor_v1 to the end of read request
2604 */
2605 if (server->rdma && rdata && !server->sign &&
2606 rdata->bytes >= server->smbd_conn->rdma_readwrite_threshold) {
2607
2608 struct smbd_buffer_descriptor_v1 *v1;
2609 bool need_invalidate =
2610 io_parms->tcon->ses->server->dialect == SMB30_PROT_ID;
2611
2612 rcu_read_lock();

> 2613			rcu_dereference(server->smbd_conn);

2614 rdata->mr = smbd_register_mr(
2615 server->smbd_conn, rdata->pages,
2616 rdata->nr_pages, rdata->tailsz,
2617 true, need_invalidate);
2618 rcu_read_unlock();
2619 if (!rdata->mr)
2620 return -ENOBUFS;
2621
2622 req->Channel = SMB2_CHANNEL_RDMA_V1_INVALIDATE;
2623 if (need_invalidate)
2624 req->Channel = SMB2_CHANNEL_RDMA_V1;
2625 req->ReadChannelInfoOffset =
2626 cpu_to_le16(offsetof(struct smb2_read_plain_req, Buffer));
2627 req->ReadChannelInfoLength =
2628 cpu_to_le16(sizeof(struct smbd_buffer_descriptor_v1));
2629 v1 = (struct smbd_buffer_descriptor_v1 *) &req->Buffer[0];
2630 v1->offset = cpu_to_le64(rdata->mr->mr->iova);
2631 v1->token = cpu_to_le32(rdata->mr->mr->rkey);
2632 v1->length = cpu_to_le32(rdata->mr->mr->length);
2633
2634 *total_len += sizeof(*v1) - 1;
2635 }
2636 #endif
2637 if (request_type & CHAINED_REQUEST) {
2638 if (!(request_type & END_OF_CHAIN)) {
2639 /* next 8-byte aligned request */
2640 *total_len = DIV_ROUND_UP(*total_len, 8) * 8;
2641 shdr->NextCommand = cpu_to_le32(*total_len);
2642 } else /* END_OF_CHAIN */
2643 shdr->NextCommand = 0;
2644 if (request_type & RELATED_REQUEST) {
2645 shdr->Flags |= SMB2_FLAGS_RELATED_OPERATIONS;
2646 /*
2647 * Related requests use info from previous read request
2648 * in chain.
2649 */
2650 shdr->SessionId = 0xFFFFFFFF;
2651 shdr->TreeId = 0xFFFFFFFF;
2652 req->PersistentFileId = 0xFFFFFFFF;
2653 req->VolatileFileId = 0xFFFFFFFF;
2654 }
2655 }
2656 if (remaining_bytes > io_parms->length)
2657 req->RemainingBytes = cpu_to_le32(remaining_bytes);
2658 else
2659 req->RemainingBytes = 0;
2660
2661 *buf = req;
2662 return rc;
2663 }
2664

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation

#6 lore raw

RE: [PATCH 5/7] cifs: smbd: Protect memory registration using RCU

Hi Steve

Please drop this patch. I will find another solution.

The other patches in the series can be reviewed and applied.

Long

Quoted 171 lines

-----Original Message-----
From: kbuild test robot <[email protected]>
Sent: Monday, May 7, 2018 11:36 PM
To: Long Li <[email protected]>
Cc: [email protected]; Steve French <[email protected]>; linux- [email protected]; [email protected]; linux- [email protected]; [email protected]; Long Li <[email protected]>
Subject: Re: [PATCH 5/7] cifs: smbd: Protect memory registration using RCU

Hi Long,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on cifs/for-next] [also build test WARNING on v4.17-rc4 next-20180507] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:
https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub
.com%2F0day-ci%2Flinux%2Fcommits%2FLong-Li%2Fcifs-smbd-Make-upper- layer-decide-when-to-destroy-the-transport%2F20180508- 110150&data=02%7C01%7Clongli%40microsoft.com%7C56045b01aeae4e53b1 9408d5b4ae0502%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C6366 13581873868980&sdata=a1Ds3qI7sRrLCE77Eoasifonm1rUQYi7rdUEnLnGOB4% 3D&reserved=0
base: git://git.samba.org/sfrench/cifs-2.6.git for-next
reproduce:

        # apt-get install sparse
        make ARCH=x86_64 allmodconfig
        make C=1 CF=-D__CHECK_ENDIAN__

sparse warnings: (new ones prefixed by >>)

fs/cifs/smb2pdu.c:110:47: sparse: expression using sizeof(void) fs/cifs/smb2pdu.c:681:26: sparse: expression using sizeof(void)

Quoted 2 lines
Quoted 2 lines

fs/cifs/smb2pdu.c:2613:17: sparse: incompatible types in comparison expression (different address spaces)

fs/cifs/smb2pdu.c:2993:17: sparse: incompatible types in comparison
expression (different address spaces)
fs/cifs/smb2pdu.c:3251:23: sparse: expression using sizeof(void) fs/cifs/smb2pdu.c:3251:23: sparse: expression using sizeof(void) fs/cifs/smb2pdu.c:3252:23: sparse: expression using sizeof(void) fs/cifs/smb2pdu.c:3713:17: sparse: expression using sizeof(void) fs/cifs/smb2pdu.c:3713:17: sparse: expression using sizeof(void)

vim +2613 fs/cifs/smb2pdu.c

2565
2566 /*
2567 * To form a chain of read requests, any read requests after the first
should
2568 * have the end_of_chain boolean set to true.
2569 */
2570 static int
2571 smb2_new_read_req(void **buf, unsigned int *total_len,
2572 struct cifs_io_parms *io_parms, struct cifs_readdata *rdata,
2573 unsigned int remaining_bytes, int request_type)
2574 {
2575 int rc = -EACCES;
2576 struct smb2_read_plain_req *req = NULL;
2577 struct smb2_sync_hdr *shdr;
2578 struct TCP_Server_Info *server;
2579
2580 rc = smb2_plain_req_init(SMB2_READ, io_parms->tcon, (void
**) &req,
2581 total_len);
2582 if (rc)
2583 return rc;
2584
2585 server = io_parms->tcon->ses->server;
2586 if (server == NULL)
2587 return -ECONNABORTED;
2588
2589 shdr = &req->sync_hdr;
2590 shdr->ProcessId = cpu_to_le32(io_parms->pid);
2591
2592 req->PersistentFileId = io_parms->persistent_fid;
2593 req->VolatileFileId = io_parms->volatile_fid;
2594 req->ReadChannelInfoOffset = 0; /* reserved */
2595 req->ReadChannelInfoLength = 0; /* reserved */
2596 req->Channel = 0; /* reserved */
2597 req->MinimumCount = 0;
2598 req->Length = cpu_to_le32(io_parms->length);
2599 req->Offset = cpu_to_le64(io_parms->offset);
2600 #ifdef CONFIG_CIFS_SMB_DIRECT
2601 /*
2602 * If we want to do a RDMA write, fill in and append
2603 * smbd_buffer_descriptor_v1 to the end of read request
2604 */
2605 if (server->rdma && rdata && !server->sign &&
2606 rdata->bytes >= server->smbd_conn-

Quoted 1 line

rdma_readwrite_threshold) {

2607
2608 struct smbd_buffer_descriptor_v1 *v1;
2609 bool need_invalidate =
2610 io_parms->tcon->ses->server->dialect ==
SMB30_PROT_ID;
2611
2612 rcu_read_lock();

Quoted 1 line

2613 rcu_dereference(server->smbd_conn);

2614 rdata->mr = smbd_register_mr(
2615 server->smbd_conn, rdata->pages,
2616 rdata->nr_pages, rdata->tailsz,
2617 true, need_invalidate);
2618 rcu_read_unlock();
2619 if (!rdata->mr)
2620 return -ENOBUFS;
2621
2622 req->Channel =
SMB2_CHANNEL_RDMA_V1_INVALIDATE;
2623 if (need_invalidate)
2624 req->Channel = SMB2_CHANNEL_RDMA_V1;
2625 req->ReadChannelInfoOffset =
2626 cpu_to_le16(offsetof(struct
smb2_read_plain_req, Buffer));
2627 req->ReadChannelInfoLength =
2628 cpu_to_le16(sizeof(struct
smbd_buffer_descriptor_v1));
2629 v1 = (struct smbd_buffer_descriptor_v1 *) &req-

Quoted 1 line

Buffer[0];

2630 v1->offset = cpu_to_le64(rdata->mr->mr->iova);
2631 v1->token = cpu_to_le32(rdata->mr->mr->rkey);
2632 v1->length = cpu_to_le32(rdata->mr->mr->length);
2633
2634 *total_len += sizeof(*v1) - 1;
2635 }
2636 #endif
2637 if (request_type & CHAINED_REQUEST) {
2638 if (!(request_type & END_OF_CHAIN)) {
2639 /* next 8-byte aligned request */
2640 *total_len = DIV_ROUND_UP(*total_len, 8) *
8;
2641 shdr->NextCommand =
cpu_to_le32(*total_len);
2642 } else /* END_OF_CHAIN */
2643 shdr->NextCommand = 0;
2644 if (request_type & RELATED_REQUEST) {
2645 shdr->Flags |=
SMB2_FLAGS_RELATED_OPERATIONS;
2646 /*
2647 * Related requests use info from previous
read request
2648 * in chain.
2649 */
2650 shdr->SessionId = 0xFFFFFFFF;
2651 shdr->TreeId = 0xFFFFFFFF;
2652 req->PersistentFileId = 0xFFFFFFFF;
2653 req->VolatileFileId = 0xFFFFFFFF;
2654 }
2655 }
2656 if (remaining_bytes > io_parms->length)
2657 req->RemainingBytes =
cpu_to_le32(remaining_bytes);
2658 else
2659 req->RemainingBytes = 0;
2660
2661 *buf = req;
2662 return rc;
2663 }
2664

---
0-DAY kernel test infrastructure Open Source Technology Center
https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.01
.org%2Fpipermail%2Fkbuild- all&data=02%7C01%7Clongli%40microsoft.com%7C56045b01aeae4e53b19408 d5b4ae0502%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C63661358 1873868980&sdata=y3BYFeKEayhIfrs6hodamCPCyz8n3YQdN1yX3yXKs80%3D
&reserved=0 Intel Corporation

author
#7 lore raw

PATCH 6/7 cifs: smbd: Retry on memory registration failure

In reply to Long Li

From: Long Li <[email protected]>

Memory registration failure doesn't mean this I/O has failed, it means the transport is hitting I/O error or needs reconnect. This error is not from the server.

Indicate this error to upper layer, and let upper layer decide how to reconnect and proceed with this I/O.

---
 fs/cifs/smb2pdu.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
fs/cifs/smb2pdu.c +2 -2
index 09ca098..c72d1fa 100644
@@ -2617,7 +2617,7 @@ smb2_new_read_req(void **buf, unsigned int *total_len,
 				true, need_invalidate);
 		rcu_read_unlock();
 		if (!rdata->mr)
-			return -ENOBUFS;
+			return -EAGAIN;
 
 		req->Channel = SMB2_CHANNEL_RDMA_V1_INVALIDATE;
 		if (need_invalidate)
@@ -2997,7 +2997,7 @@ smb2_async_writev(struct cifs_writedata *wdata,
 				false, need_invalidate);
 		rcu_read_unlock();
 		if (!wdata->mr) {
-			rc = -ENOBUFS;
+			rc = -EAGAIN;
 			goto async_writev_out;
 		}
 		req->Length = 0;
Signature · 1 line
2.7.4
author
#8 lore raw

PATCH 7/7 cifs: Call MID callback before destroying transport

In reply to Long Li

From: Long Li <[email protected]>

When transport is being destroyed, it's possible that some processes may hold memory registrations that need to be deregistred.

Call them first so nobody is using transport resources, and it can be destroyed.

---
 fs/cifs/connect.c | 36 +++++++++++++++++++-----------------
 1 file changed, 19 insertions(+), 17 deletions(-)
fs/cifs/connect.c +19 -17
index 5db3e9d..7e7d40e 100644
@@ -369,22 +369,6 @@ cifs_reconnect(struct TCP_Server_Info *server)
 	/* do not want to be sending data on a socket we are freeing */
 	cifs_dbg(FYI, "%s: tearing down socket\n", __func__);
 	mutex_lock(&server->srv_mutex);
-	if (server->ssocket) {
-		cifs_dbg(FYI, "State: 0x%x Flags: 0x%lx\n",
-			 server->ssocket->state, server->ssocket->flags);
-		kernel_sock_shutdown(server->ssocket, SHUT_WR);
-		cifs_dbg(FYI, "Post shutdown state: 0x%x Flags: 0x%lx\n",
-			 server->ssocket->state, server->ssocket->flags);
-		sock_release(server->ssocket);
-		server->ssocket = NULL;
-	} else if (cifs_rdma_enabled(server))
-		smbd_destroy(server);
-	server->sequence_number = 0;
-	server->session_estab = false;
-	kfree(server->session_key.response);
-	server->session_key.response = NULL;
-	server->session_key.len = 0;
-	server->lstrp = jiffies;
 
 	/* mark submitted MIDs for retry and issue callback */
 	INIT_LIST_HEAD(&retry_list);
@@ -397,7 +381,6 @@ cifs_reconnect(struct TCP_Server_Info *server)
 		list_move(&mid_entry->qhead, &retry_list);
 	}
 	spin_unlock(&GlobalMid_Lock);
-	mutex_unlock(&server->srv_mutex);
 
 	cifs_dbg(FYI, "%s: issuing mid callbacks\n", __func__);
 	list_for_each_safe(tmp, tmp2, &retry_list) {
@@ -406,6 +389,25 @@ cifs_reconnect(struct TCP_Server_Info *server)
 		mid_entry->callback(mid_entry);
 	}
 
+	if (server->ssocket) {
+		cifs_dbg(FYI, "State: 0x%x Flags: 0x%lx\n",
+			 server->ssocket->state, server->ssocket->flags);
+		kernel_sock_shutdown(server->ssocket, SHUT_WR);
+		cifs_dbg(FYI, "Post shutdown state: 0x%x Flags: 0x%lx\n",
+			 server->ssocket->state, server->ssocket->flags);
+		sock_release(server->ssocket);
+		server->ssocket = NULL;
+	} else if (cifs_rdma_enabled(server))
+		smbd_destroy(server);
+	server->sequence_number = 0;
+	server->session_estab = false;
+	kfree(server->session_key.response);
+	server->session_key.response = NULL;
+	server->session_key.len = 0;
+	server->lstrp = jiffies;
+
+	mutex_unlock(&server->srv_mutex);
+
 	do {
 		try_to_freeze();
 
Signature · 1 line
2.7.4
#9 lore raw
In reply to Long Li
Thank you for the patch! Perhaps something to improve:

Hi Long,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on cifs/for-next]
[also build test WARNING on v4.17-rc4 next-20180507]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Long-Li/cifs-smbd-Make-upper-layer-decide-when-to-destroy-the-transport/20180508-110150
base:   git://git.samba.org/sfrench/cifs-2.6.git for-next
config: i386-randconfig-a1-05080831 (attached as .config)
compiler: gcc-4.9 (Debian 4.9.4-2) 4.9.4
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All warnings (new ones prefixed by >>):

fs//cifs/connect.c: In function 'cifs_reconnect':

>> fs//cifs/connect.c:381:16: warning: passing argument 1 of 'smbd_destroy' from incompatible pointer type
      smbd_destroy(server);
                   ^
   In file included from fs//cifs/connect.c:58:0:
   fs//cifs/smbdirect.h:334:20: note: expected 'struct smbd_connection *' but argument is of type 'struct TCP_Server_Info *'
    static inline void smbd_destroy(struct smbd_connection *info) {}
                       ^
   fs//cifs/connect.c: In function 'clean_demultiplex_info':
   fs//cifs/connect.c:715:16: warning: passing argument 1 of 'smbd_destroy' from incompatible pointer type
      smbd_destroy(server);
                   ^
   In file included from fs//cifs/connect.c:58:0:
   fs//cifs/smbdirect.h:334:20: note: expected 'struct smbd_connection *' but argument is of type 'struct TCP_Server_Info *'
    static inline void smbd_destroy(struct smbd_connection *info) {}
                       ^

vim +/smbd_destroy +381 fs//cifs/connect.c

312
313 static int ip_connect(struct TCP_Server_Info *server);
314 static int generic_ip_connect(struct TCP_Server_Info *server);
315 static void tlink_rb_insert(struct rb_root *root, struct tcon_link *new_tlink);
316 static void cifs_prune_tlinks(struct work_struct *work);
317 static int cifs_setup_volume_info(struct smb_vol *volume_info, char *mount_data,
318 const char *devname);
319
320 /*
321 * cifs tcp session reconnection
322 *
323 * mark tcp session as reconnecting so temporarily locked
324 * mark all smb sessions as reconnecting for tcp session
325 * reconnect tcp session
326 * wake up waiters on reconnection? - (not needed currently)
327 */
328 int
329 cifs_reconnect(struct TCP_Server_Info *server)
330 {
331 int rc = 0;
332 struct list_head *tmp, *tmp2;
333 struct cifs_ses *ses;
334 struct cifs_tcon *tcon;
335 struct mid_q_entry *mid_entry;
336 struct list_head retry_list;
337
338 spin_lock(&GlobalMid_Lock);
339 if (server->tcpStatus == CifsExiting) {
340 /* the demux thread will exit normally
341 next time through the loop */
342 spin_unlock(&GlobalMid_Lock);
343 return rc;
344 } else
345 server->tcpStatus = CifsNeedReconnect;
346 spin_unlock(&GlobalMid_Lock);
347 server->maxBuf = 0;
348 server->max_read = 0;
349
350 cifs_dbg(FYI, "Reconnecting tcp session\n");
351
352 /* before reconnecting the tcp session, mark the smb session (uid)
353 and the tid bad so they are not used until reconnected */
354 cifs_dbg(FYI, "%s: marking sessions and tcons for reconnect\n",
355 __func__);
356 spin_lock(&cifs_tcp_ses_lock);
357 list_for_each(tmp, &server->smb_ses_list) {
358 ses = list_entry(tmp, struct cifs_ses, smb_ses_list);
359 ses->need_reconnect = true;
360 list_for_each(tmp2, &ses->tcon_list) {
361 tcon = list_entry(tmp2, struct cifs_tcon, tcon_list);
362 tcon->need_reconnect = true;
363 }
364 if (ses->tcon_ipc)
365 ses->tcon_ipc->need_reconnect = true;
366 }
367 spin_unlock(&cifs_tcp_ses_lock);
368
369 /* do not want to be sending data on a socket we are freeing */
370 cifs_dbg(FYI, "%s: tearing down socket\n", __func__);
371 mutex_lock(&server->srv_mutex);
372 if (server->ssocket) {
373 cifs_dbg(FYI, "State: 0x%x Flags: 0x%lx\n",
374 server->ssocket->state, server->ssocket->flags);
375 kernel_sock_shutdown(server->ssocket, SHUT_WR);
376 cifs_dbg(FYI, "Post shutdown state: 0x%x Flags: 0x%lx\n",
377 server->ssocket->state, server->ssocket->flags);
378 sock_release(server->ssocket);
379 server->ssocket = NULL;
380 } else if (cifs_rdma_enabled(server))

 > 381			smbd_destroy(server);

382 server->sequence_number = 0;
383 server->session_estab = false;
384 kfree(server->session_key.response);
385 server->session_key.response = NULL;
386 server->session_key.len = 0;
387 server->lstrp = jiffies;
388
389 /* mark submitted MIDs for retry and issue callback */
390 INIT_LIST_HEAD(&retry_list);
391 cifs_dbg(FYI, "%s: moving mids to private list\n", __func__);
392 spin_lock(&GlobalMid_Lock);
393 list_for_each_safe(tmp, tmp2, &server->pending_mid_q) {
394 mid_entry = list_entry(tmp, struct mid_q_entry, qhead);
395 if (mid_entry->mid_state == MID_REQUEST_SUBMITTED)
396 mid_entry->mid_state = MID_RETRY_NEEDED;
397 list_move(&mid_entry->qhead, &retry_list);
398 }
399 spin_unlock(&GlobalMid_Lock);
400 mutex_unlock(&server->srv_mutex);
401
402 cifs_dbg(FYI, "%s: issuing mid callbacks\n", __func__);
403 list_for_each_safe(tmp, tmp2, &retry_list) {
404 mid_entry = list_entry(tmp, struct mid_q_entry, qhead);
405 list_del_init(&mid_entry->qhead);
406 mid_entry->callback(mid_entry);
407 }
408
409 do {
410 try_to_freeze();
411
412 /* we should try only the port we connected to before */
413 mutex_lock(&server->srv_mutex);
414 if (cifs_rdma_enabled(server))
415 rc = smbd_reconnect(server);
416 else
417 rc = generic_ip_connect(server);
418 if (rc) {
419 cifs_dbg(FYI, "reconnect error %d\n", rc);
420 mutex_unlock(&server->srv_mutex);
421 msleep(3000);
422 } else {
423 atomic_inc(&tcpSesReconnectCount);
424 spin_lock(&GlobalMid_Lock);
425 if (server->tcpStatus != CifsExiting)
426 server->tcpStatus = CifsNeedNegotiate;
427 spin_unlock(&GlobalMid_Lock);
428 mutex_unlock(&server->srv_mutex);
429 }
430 } while (server->tcpStatus == CifsNeedReconnect);
431
432 if (server->tcpStatus == CifsNeedNegotiate)
433 mod_delayed_work(cifsiod_wq, &server->echo, 0);
434
435 return rc;
436 }
437

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation

#10 lore raw
In reply to Long Li
Thank you for the patch! Yet something to improve:

Hi Long,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on cifs/for-next]
[also build test ERROR on v4.17-rc4 next-20180507]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Long-Li/cifs-smbd-Make-upper-layer-decide-when-to-destroy-the-transport/20180508-110150
base:   git://git.samba.org/sfrench/cifs-2.6.git for-next
config: i386-randconfig-x013-201818 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

fs//cifs/connect.c: In function 'cifs_reconnect':

>> fs//cifs/connect.c:381:16: error: passing argument 1 of 'smbd_destroy' from incompatible pointer type [-Werror=incompatible-pointer-types]
      smbd_destroy(server);
                   ^~~~~~
   In file included from fs//cifs/connect.c:58:0:
   fs//cifs/smbdirect.h:334:20: note: expected 'struct smbd_connection *' but argument is of type 'struct TCP_Server_Info *'
    static inline void smbd_destroy(struct smbd_connection *info) {}
                       ^~~~~~~~~~~~
   fs//cifs/connect.c: In function 'clean_demultiplex_info':
   fs//cifs/connect.c:715:16: error: passing argument 1 of 'smbd_destroy' from incompatible pointer type [-Werror=incompatible-pointer-types]
      smbd_destroy(server);
                   ^~~~~~
   In file included from fs//cifs/connect.c:58:0:
   fs//cifs/smbdirect.h:334:20: note: expected 'struct smbd_connection *' but argument is of type 'struct TCP_Server_Info *'
    static inline void smbd_destroy(struct smbd_connection *info) {}
                       ^~~~~~~~~~~~

Cyclomatic Complexity 5 include/linux/compiler.h:__read_once_size Cyclomatic Complexity 5 include/linux/compiler.h:__write_once_size Cyclomatic Complexity 1 include/linux/kasan-checks.h:kasan_check_read Cyclomatic Complexity 1 include/linux/kasan-checks.h:kasan_check_write Cyclomatic Complexity 2 arch/x86/include/asm/bitops.h:set_bit Cyclomatic Complexity 2 arch/x86/include/asm/bitops.h:clear_bit Cyclomatic Complexity 1 arch/x86/include/asm/bitops.h:test_and_set_bit Cyclomatic Complexity 1 arch/x86/include/asm/bitops.h:constant_test_bit Cyclomatic Complexity 1 arch/x86/include/asm/bitops.h:variable_test_bit Cyclomatic Complexity 1 arch/x86/include/asm/bitops.h:fls
Cyclomatic Complexity 1 include/uapi/linux/byteorder/little_endian.h:__le16_to_cpup Cyclomatic Complexity 1 include/linux/log2.h:__ilog2_u32
Cyclomatic Complexity 1 include/linux/list.h:INIT_LIST_HEAD
Cyclomatic Complexity 1 include/linux/list.h:__list_add_valid Cyclomatic Complexity 1 include/linux/list.h:__list_del_entry_valid Cyclomatic Complexity 2 include/linux/list.h:__list_add
Cyclomatic Complexity 1 include/linux/list.h:list_add
Cyclomatic Complexity 1 include/linux/list.h:__list_del
Cyclomatic Complexity 2 include/linux/list.h:__list_del_entry Cyclomatic Complexity 1 include/linux/list.h:list_del_init
Cyclomatic Complexity 1 include/linux/list.h:list_move
Cyclomatic Complexity 1 include/linux/list.h:list_empty
Cyclomatic Complexity 1 arch/x86/include/asm/current.h:get_current Cyclomatic Complexity 1 include/linux/string.h:strnlen
Cyclomatic Complexity 4 include/linux/string.h:strlen
Cyclomatic Complexity 6 include/linux/string.h:strlcpy
Cyclomatic Complexity 3 include/linux/string.h:memset
Cyclomatic Complexity 4 include/linux/string.h:memcpy
Cyclomatic Complexity 2 include/linux/string.h:strcpy
Cyclomatic Complexity 1 include/asm-generic/getorder.h:__get_order Cyclomatic Complexity 1 include/linux/err.h:ERR_PTR
Cyclomatic Complexity 1 include/linux/err.h:PTR_ERR
Cyclomatic Complexity 1 arch/x86/include/asm/atomic.h:arch_atomic_read Cyclomatic Complexity 1 arch/x86/include/asm/atomic.h:arch_atomic_inc Cyclomatic Complexity 1 arch/x86/include/asm/atomic.h:arch_atomic_dec_and_test Cyclomatic Complexity 1 arch/x86/include/asm/atomic.h:arch_atomic_add_return Cyclomatic Complexity 1 arch/x86/include/asm/atomic.h:arch_atomic_sub_return Cyclomatic Complexity 1 include/asm-generic/atomic-instrumented.h:atomic_read Cyclomatic Complexity 1 include/asm-generic/atomic-instrumented.h:atomic_inc Cyclomatic Complexity 1 include/asm-generic/atomic-instrumented.h:atomic_inc_return Cyclomatic Complexity 1 include/asm-generic/atomic-instrumented.h:atomic_dec_return Cyclomatic Complexity 1 include/asm-generic/atomic-instrumented.h:atomic_dec_and_test Cyclomatic Complexity 5 arch/x86/include/asm/preempt.h:__preempt_count_sub Cyclomatic Complexity 1 include/linux/spinlock.h:spinlock_check Cyclomatic Complexity 1 include/linux/spinlock.h:spin_lock
Cyclomatic Complexity 1 include/linux/spinlock.h:spin_unlock
Cyclomatic Complexity 1 include/linux/uidgid.h:__kuid_val
Cyclomatic Complexity 1 include/linux/uidgid.h:__kgid_val
Cyclomatic Complexity 1 include/linux/uidgid.h:uid_eq
Cyclomatic Complexity 1 include/linux/uidgid.h:gid_eq
Cyclomatic Complexity 1 include/linux/uidgid.h:uid_gt
Cyclomatic Complexity 1 include/linux/uidgid.h:uid_lt
Cyclomatic Complexity 1 include/linux/uidgid.h:uid_valid
Cyclomatic Complexity 1 include/linux/uidgid.h:gid_valid
Cyclomatic Complexity 1 include/linux/uidgid.h:make_kuid
Cyclomatic Complexity 1 include/linux/uidgid.h:make_kgid
Cyclomatic Complexity 1 include/linux/uidgid.h:from_kuid
Cyclomatic Complexity 1 include/linux/rbtree.h:rb_link_node
Cyclomatic Complexity 1 include/linux/debug_locks.h:debug_check_no_locks_held Cyclomatic Complexity 1 include/linux/workqueue.h:__init_work Cyclomatic Complexity 1 include/linux/uio.h:iov_iter_count
Cyclomatic Complexity 1 include/linux/socket.h:msg_data_left
Cyclomatic Complexity 1 include/linux/sched.h:task_pid_nr
Cyclomatic Complexity 1 include/linux/sched.h:task_thread_info Cyclomatic Complexity 1 include/linux/cred.h:current_user_ns
Cyclomatic Complexity 1 include/linux/kasan.h:kasan_kmalloc
Cyclomatic Complexity 28 include/linux/slab.h:kmalloc_index
Cyclomatic Complexity 1 include/linux/slab.h:kmem_cache_alloc_trace Cyclomatic Complexity 1 include/linux/slab.h:kmalloc_order_trace Cyclomatic Complexity 67 include/linux/slab.h:kmalloc_large
Cyclomatic Complexity 5 include/linux/slab.h:kmalloc
Cyclomatic Complexity 1 include/linux/slab.h:kzalloc
Cyclomatic Complexity 2 include/linux/ctype.h:__toupper
Cyclomatic Complexity 1 include/linux/utsname.h:utsname
Cyclomatic Complexity 1 include/net/net_namespace.h:get_net
Cyclomatic Complexity 1 include/net/net_namespace.h:put_net
Cyclomatic Complexity 1 include/net/net_namespace.h:net_eq
Cyclomatic Complexity 1 include/linux/module.h:__module_get
Cyclomatic Complexity 1 include/linux/module.h:module_put
Cyclomatic Complexity 1 include/keys/user-type.h:user_key_payload_locked Cyclomatic Complexity 1 include/net/ipv6.h:ipv6_addr_equal
Cyclomatic Complexity 1 include/linux/unaligned/access_ok.h:get_unaligned_le16 Cyclomatic Complexity 1 fs//cifs/cifspdu.h:BCC
Cyclomatic Complexity 1 fs//cifs/cifspdu.h:get_bcc
Cyclomatic Complexity 1 fs//cifs/cifsglob.h:set_credits
Cyclomatic Complexity 1 fs//cifs/cifsglob.h:get_next_mid

vim +/smbd_destroy +381 fs//cifs/connect.c

312
313 static int ip_connect(struct TCP_Server_Info *server);
314 static int generic_ip_connect(struct TCP_Server_Info *server);
315 static void tlink_rb_insert(struct rb_root *root, struct tcon_link *new_tlink);
316 static void cifs_prune_tlinks(struct work_struct *work);
317 static int cifs_setup_volume_info(struct smb_vol *volume_info, char *mount_data,
318 const char *devname);
319
320 /*
321 * cifs tcp session reconnection
322 *
323 * mark tcp session as reconnecting so temporarily locked
324 * mark all smb sessions as reconnecting for tcp session
325 * reconnect tcp session
326 * wake up waiters on reconnection? - (not needed currently)
327 */
328 int
329 cifs_reconnect(struct TCP_Server_Info *server)
330 {
331 int rc = 0;
332 struct list_head *tmp, *tmp2;
333 struct cifs_ses *ses;
334 struct cifs_tcon *tcon;
335 struct mid_q_entry *mid_entry;
336 struct list_head retry_list;
337
338 spin_lock(&GlobalMid_Lock);
339 if (server->tcpStatus == CifsExiting) {
340 /* the demux thread will exit normally
341 next time through the loop */
342 spin_unlock(&GlobalMid_Lock);
343 return rc;
344 } else
345 server->tcpStatus = CifsNeedReconnect;
346 spin_unlock(&GlobalMid_Lock);
347 server->maxBuf = 0;
348 server->max_read = 0;
349
350 cifs_dbg(FYI, "Reconnecting tcp session\n");
351
352 /* before reconnecting the tcp session, mark the smb session (uid)
353 and the tid bad so they are not used until reconnected */
354 cifs_dbg(FYI, "%s: marking sessions and tcons for reconnect\n",
355 __func__);
356 spin_lock(&cifs_tcp_ses_lock);
357 list_for_each(tmp, &server->smb_ses_list) {
358 ses = list_entry(tmp, struct cifs_ses, smb_ses_list);
359 ses->need_reconnect = true;
360 list_for_each(tmp2, &ses->tcon_list) {
361 tcon = list_entry(tmp2, struct cifs_tcon, tcon_list);
362 tcon->need_reconnect = true;
363 }
364 if (ses->tcon_ipc)
365 ses->tcon_ipc->need_reconnect = true;
366 }
367 spin_unlock(&cifs_tcp_ses_lock);
368
369 /* do not want to be sending data on a socket we are freeing */
370 cifs_dbg(FYI, "%s: tearing down socket\n", __func__);
371 mutex_lock(&server->srv_mutex);
372 if (server->ssocket) {
373 cifs_dbg(FYI, "State: 0x%x Flags: 0x%lx\n",
374 server->ssocket->state, server->ssocket->flags);
375 kernel_sock_shutdown(server->ssocket, SHUT_WR);
376 cifs_dbg(FYI, "Post shutdown state: 0x%x Flags: 0x%lx\n",
377 server->ssocket->state, server->ssocket->flags);
378 sock_release(server->ssocket);
379 server->ssocket = NULL;
380 } else if (cifs_rdma_enabled(server))

 > 381			smbd_destroy(server);

382 server->sequence_number = 0;
383 server->session_estab = false;
384 kfree(server->session_key.response);
385 server->session_key.response = NULL;
386 server->session_key.len = 0;
387 server->lstrp = jiffies;
388
389 /* mark submitted MIDs for retry and issue callback */
390 INIT_LIST_HEAD(&retry_list);
391 cifs_dbg(FYI, "%s: moving mids to private list\n", __func__);
392 spin_lock(&GlobalMid_Lock);
393 list_for_each_safe(tmp, tmp2, &server->pending_mid_q) {
394 mid_entry = list_entry(tmp, struct mid_q_entry, qhead);
395 if (mid_entry->mid_state == MID_REQUEST_SUBMITTED)
396 mid_entry->mid_state = MID_RETRY_NEEDED;
397 list_move(&mid_entry->qhead, &retry_list);
398 }
399 spin_unlock(&GlobalMid_Lock);
400 mutex_unlock(&server->srv_mutex);
401
402 cifs_dbg(FYI, "%s: issuing mid callbacks\n", __func__);
403 list_for_each_safe(tmp, tmp2, &retry_list) {
404 mid_entry = list_entry(tmp, struct mid_q_entry, qhead);
405 list_del_init(&mid_entry->qhead);
406 mid_entry->callback(mid_entry);
407 }
408
409 do {
410 try_to_freeze();
411
412 /* we should try only the port we connected to before */
413 mutex_lock(&server->srv_mutex);
414 if (cifs_rdma_enabled(server))
415 rc = smbd_reconnect(server);
416 else
417 rc = generic_ip_connect(server);
418 if (rc) {
419 cifs_dbg(FYI, "reconnect error %d\n", rc);
420 mutex_unlock(&server->srv_mutex);
421 msleep(3000);
422 } else {
423 atomic_inc(&tcpSesReconnectCount);
424 spin_lock(&GlobalMid_Lock);
425 if (server->tcpStatus != CifsExiting)
426 server->tcpStatus = CifsNeedNegotiate;
427 spin_unlock(&GlobalMid_Lock);
428 mutex_unlock(&server->srv_mutex);
429 }
430 } while (server->tcpStatus == CifsNeedReconnect);
431
432 if (server->tcpStatus == CifsNeedNegotiate)
433 mod_delayed_work(cifsiod_wq, &server->echo, 0);
434
435 return rc;
436 }
437

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation

#11 lore raw
Quoted 26 lines

Subject: Re: [PATCH 1/7] cifs: smbd: Make upper layer decide when to destroy the transport

Hi Long,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on cifs/for-next] [also build test ERROR on v4.17-rc4 next-20180507] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:
https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub
.com%2F0day-ci%2Flinux%2Fcommits%2FLong-Li%2Fcifs-smbd-Make-upper- layer-decide-when-to-destroy-the-transport%2F20180508- 110150&data=02%7C01%7Clongli%40microsoft.com%7C8eeef6813ee14ded2 dcc08d5b4a4113a%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636 613539125415461&sdata=6EzmQWCVBK9EESOC3UQwrObR9AL9W5u660M4k bDvoJw%3D&reserved=0
base: git://git.samba.org/sfrench/cifs-2.6.git for-next
config: i386-randconfig-x013-201818 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:

        # save the attached .config to linux build tree
        make ARCH=i386
Quoted 8 lines

All errors (new ones prefixed by >>):

fs//cifs/connect.c: In function 'cifs_reconnect':

Quoted 3 lines
Quoted 3 lines

fs//cifs/connect.c:381:16: error: passing argument 1 of 'smbd_destroy' from incompatible pointer type [-Werror=incompatible-pointer-types]

      smbd_destroy(server);
                   ^~~~~~

Thanks! I will send v2.

Quoted 268 lines

In file included from fs//cifs/connect.c:58:0:
fs//cifs/smbdirect.h:334:20: note: expected 'struct smbd_connection *' but
argument is of type 'struct TCP_Server_Info *'

    static inline void smbd_destroy(struct smbd_connection *info) {}
                       ^~~~~~~~~~~~
   fs//cifs/connect.c: In function 'clean_demultiplex_info':
   fs//cifs/connect.c:715:16: error: passing argument 1 of 'smbd_destroy' from
incompatible pointer type [-Werror=incompatible-pointer-types]
      smbd_destroy(server);
                   ^~~~~~
   In file included from fs//cifs/connect.c:58:0:
   fs//cifs/smbdirect.h:334:20: note: expected 'struct smbd_connection *' but
argument is of type 'struct TCP_Server_Info *'
    static inline void smbd_destroy(struct smbd_connection *info) {}
                       ^~~~~~~~~~~~

Cyclomatic Complexity 5 include/linux/compiler.h:__read_once_size Cyclomatic Complexity 5 include/linux/compiler.h:__write_once_size Cyclomatic Complexity 1 include/linux/kasan-checks.h:kasan_check_read Cyclomatic Complexity 1 include/linux/kasan-checks.h:kasan_check_write Cyclomatic Complexity 2 arch/x86/include/asm/bitops.h:set_bit Cyclomatic Complexity 2 arch/x86/include/asm/bitops.h:clear_bit Cyclomatic Complexity 1 arch/x86/include/asm/bitops.h:test_and_set_bit Cyclomatic Complexity 1 arch/x86/include/asm/bitops.h:constant_test_bit Cyclomatic Complexity 1 arch/x86/include/asm/bitops.h:variable_test_bit Cyclomatic Complexity 1 arch/x86/include/asm/bitops.h:fls Cyclomatic Complexity 1
include/uapi/linux/byteorder/little_endian.h:__le16_to_cpup
Cyclomatic Complexity 1 include/linux/log2.h:__ilog2_u32 Cyclomatic Complexity 1 include/linux/list.h:INIT_LIST_HEAD Cyclomatic Complexity 1 include/linux/list.h:__list_add_valid Cyclomatic Complexity 1 include/linux/list.h:__list_del_entry_valid Cyclomatic Complexity 2 include/linux/list.h:__list_add Cyclomatic Complexity 1 include/linux/list.h:list_add Cyclomatic Complexity 1 include/linux/list.h:__list_del Cyclomatic Complexity 2 include/linux/list.h:__list_del_entry Cyclomatic Complexity 1 include/linux/list.h:list_del_init Cyclomatic Complexity 1 include/linux/list.h:list_move Cyclomatic Complexity 1 include/linux/list.h:list_empty Cyclomatic Complexity 1 arch/x86/include/asm/current.h:get_current Cyclomatic Complexity 1 include/linux/string.h:strnlen Cyclomatic Complexity 4 include/linux/string.h:strlen Cyclomatic Complexity 6 include/linux/string.h:strlcpy Cyclomatic Complexity 3 include/linux/string.h:memset Cyclomatic Complexity 4 include/linux/string.h:memcpy Cyclomatic Complexity 2 include/linux/string.h:strcpy Cyclomatic Complexity 1 include/asm-generic/getorder.h:__get_order Cyclomatic Complexity 1 include/linux/err.h:ERR_PTR Cyclomatic Complexity 1 include/linux/err.h:PTR_ERR Cyclomatic Complexity 1 arch/x86/include/asm/atomic.h:arch_atomic_read Cyclomatic Complexity 1 arch/x86/include/asm/atomic.h:arch_atomic_inc Cyclomatic Complexity 1
arch/x86/include/asm/atomic.h:arch_atomic_dec_and_test
Cyclomatic Complexity 1
arch/x86/include/asm/atomic.h:arch_atomic_add_return
Cyclomatic Complexity 1
arch/x86/include/asm/atomic.h:arch_atomic_sub_return
Cyclomatic Complexity 1 include/asm-generic/atomic-
instrumented.h:atomic_read
Cyclomatic Complexity 1 include/asm-generic/atomic-
instrumented.h:atomic_inc
Cyclomatic Complexity 1 include/asm-generic/atomic-
instrumented.h:atomic_inc_return
Cyclomatic Complexity 1 include/asm-generic/atomic-
instrumented.h:atomic_dec_return
Cyclomatic Complexity 1 include/asm-generic/atomic-
instrumented.h:atomic_dec_and_test
Cyclomatic Complexity 5
arch/x86/include/asm/preempt.h:__preempt_count_sub
Cyclomatic Complexity 1 include/linux/spinlock.h:spinlock_check Cyclomatic Complexity 1 include/linux/spinlock.h:spin_lock Cyclomatic Complexity 1 include/linux/spinlock.h:spin_unlock Cyclomatic Complexity 1 include/linux/uidgid.h:__kuid_val Cyclomatic Complexity 1 include/linux/uidgid.h:__kgid_val Cyclomatic Complexity 1 include/linux/uidgid.h:uid_eq Cyclomatic Complexity 1 include/linux/uidgid.h:gid_eq Cyclomatic Complexity 1 include/linux/uidgid.h:uid_gt Cyclomatic Complexity 1 include/linux/uidgid.h:uid_lt Cyclomatic Complexity 1 include/linux/uidgid.h:uid_valid Cyclomatic Complexity 1 include/linux/uidgid.h:gid_valid Cyclomatic Complexity 1 include/linux/uidgid.h:make_kuid Cyclomatic Complexity 1 include/linux/uidgid.h:make_kgid Cyclomatic Complexity 1 include/linux/uidgid.h:from_kuid Cyclomatic Complexity 1 include/linux/rbtree.h:rb_link_node Cyclomatic Complexity 1
include/linux/debug_locks.h:debug_check_no_locks_held
Cyclomatic Complexity 1 include/linux/workqueue.h:__init_work Cyclomatic Complexity 1 include/linux/uio.h:iov_iter_count Cyclomatic Complexity 1 include/linux/socket.h:msg_data_left Cyclomatic Complexity 1 include/linux/sched.h:task_pid_nr Cyclomatic Complexity 1 include/linux/sched.h:task_thread_info Cyclomatic Complexity 1 include/linux/cred.h:current_user_ns Cyclomatic Complexity 1 include/linux/kasan.h:kasan_kmalloc Cyclomatic Complexity 28 include/linux/slab.h:kmalloc_index Cyclomatic Complexity 1 include/linux/slab.h:kmem_cache_alloc_trace Cyclomatic Complexity 1 include/linux/slab.h:kmalloc_order_trace Cyclomatic Complexity 67 include/linux/slab.h:kmalloc_large Cyclomatic Complexity 5 include/linux/slab.h:kmalloc Cyclomatic Complexity 1 include/linux/slab.h:kzalloc Cyclomatic Complexity 2 include/linux/ctype.h:__toupper Cyclomatic Complexity 1 include/linux/utsname.h:utsname Cyclomatic Complexity 1 include/net/net_namespace.h:get_net Cyclomatic Complexity 1 include/net/net_namespace.h:put_net Cyclomatic Complexity 1 include/net/net_namespace.h:net_eq Cyclomatic Complexity 1 include/linux/module.h:__module_get Cyclomatic Complexity 1 include/linux/module.h:module_put Cyclomatic Complexity 1 include/keys/user-
type.h:user_key_payload_locked
Cyclomatic Complexity 1 include/net/ipv6.h:ipv6_addr_equal Cyclomatic Complexity 1
include/linux/unaligned/access_ok.h:get_unaligned_le16
Cyclomatic Complexity 1 fs//cifs/cifspdu.h:BCC
Cyclomatic Complexity 1 fs//cifs/cifspdu.h:get_bcc Cyclomatic Complexity 1 fs//cifs/cifsglob.h:set_credits Cyclomatic Complexity 1 fs//cifs/cifsglob.h:get_next_mid

vim +/smbd_destroy +381 fs//cifs/connect.c

   312
   313	static int ip_connect(struct TCP_Server_Info *server);
   314	static int generic_ip_connect(struct TCP_Server_Info *server);
   315	static void tlink_rb_insert(struct rb_root *root, struct tcon_link

*new_tlink);
316 static void cifs_prune_tlinks(struct work_struct *work);
317 static int cifs_setup_volume_info(struct smb_vol *volume_info, char
*mount_data,

   318						const char *devname);
   319
   320	/*
   321	 * cifs tcp session reconnection
   322	 *
   323	 * mark tcp session as reconnecting so temporarily locked
   324	 * mark all smb sessions as reconnecting for tcp session
   325	 * reconnect tcp session
   326	 * wake up waiters on reconnection? - (not needed currently)
   327	 */
   328	int
   329	cifs_reconnect(struct TCP_Server_Info *server)
   330	{
   331		int rc = 0;
   332		struct list_head *tmp, *tmp2;
   333		struct cifs_ses *ses;
   334		struct cifs_tcon *tcon;
   335		struct mid_q_entry *mid_entry;
   336		struct list_head retry_list;
   337
   338		spin_lock(&GlobalMid_Lock);
   339		if (server->tcpStatus == CifsExiting) {
   340			/* the demux thread will exit normally
   341			next time through the loop */
   342			spin_unlock(&GlobalMid_Lock);
   343			return rc;
   344		} else
   345			server->tcpStatus = CifsNeedReconnect;
   346		spin_unlock(&GlobalMid_Lock);
   347		server->maxBuf = 0;
   348		server->max_read = 0;
   349
   350		cifs_dbg(FYI, "Reconnecting tcp session\n");
   351
   352		/* before reconnecting the tcp session, mark the smb

session (uid)
353 and the tid bad so they are not used until
reconnected */
354 cifs_dbg(FYI, "%s: marking sessions and tcons for
reconnect\n",

   355			 __func__);
   356		spin_lock(&cifs_tcp_ses_lock);
   357		list_for_each(tmp, &server->smb_ses_list) {
   358			ses = list_entry(tmp, struct cifs_ses, smb_ses_list);
   359			ses->need_reconnect = true;
   360			list_for_each(tmp2, &ses->tcon_list) {
   361				tcon = list_entry(tmp2, struct cifs_tcon,
tcon_list);
   362				tcon->need_reconnect = true;
   363			}
   364			if (ses->tcon_ipc)
   365				ses->tcon_ipc->need_reconnect = true;
   366		}
   367		spin_unlock(&cifs_tcp_ses_lock);
   368
   369		/* do not want to be sending data on a socket we are freeing
*/
   370		cifs_dbg(FYI, "%s: tearing down socket\n", __func__);
   371		mutex_lock(&server->srv_mutex);
   372		if (server->ssocket) {
   373			cifs_dbg(FYI, "State: 0x%x Flags: 0x%lx\n",
   374				 server->ssocket->state, server->ssocket-
Quoted 1 line

flags);

   375			kernel_sock_shutdown(server->ssocket, SHUT_WR);
   376			cifs_dbg(FYI, "Post shutdown state: 0x%x Flags:

0x%lx\n",
377 server->ssocket->state, server->ssocket-

Quoted 1 line

flags);

   378			sock_release(server->ssocket);
   379			server->ssocket = NULL;
   380		} else if (cifs_rdma_enabled(server))
Quoted 1 line

381 smbd_destroy(server);

   382		server->sequence_number = 0;
   383		server->session_estab = false;
   384		kfree(server->session_key.response);
   385		server->session_key.response = NULL;
   386		server->session_key.len = 0;
   387		server->lstrp = jiffies;
   388
   389		/* mark submitted MIDs for retry and issue callback */
   390		INIT_LIST_HEAD(&retry_list);
   391		cifs_dbg(FYI, "%s: moving mids to private list\n", __func__);
   392		spin_lock(&GlobalMid_Lock);
   393		list_for_each_safe(tmp, tmp2, &server->pending_mid_q) {
   394			mid_entry = list_entry(tmp, struct mid_q_entry,
qhead);
   395			if (mid_entry->mid_state ==
MID_REQUEST_SUBMITTED)
   396				mid_entry->mid_state =
MID_RETRY_NEEDED;
   397			list_move(&mid_entry->qhead, &retry_list);
   398		}
   399		spin_unlock(&GlobalMid_Lock);
   400		mutex_unlock(&server->srv_mutex);
   401
   402		cifs_dbg(FYI, "%s: issuing mid callbacks\n", __func__);
   403		list_for_each_safe(tmp, tmp2, &retry_list) {
   404			mid_entry = list_entry(tmp, struct mid_q_entry,
qhead);
   405			list_del_init(&mid_entry->qhead);
   406			mid_entry->callback(mid_entry);
   407		}
   408
   409		do {
   410			try_to_freeze();
   411
   412			/* we should try only the port we connected to
before */
   413			mutex_lock(&server->srv_mutex);
   414			if (cifs_rdma_enabled(server))
   415				rc = smbd_reconnect(server);
   416			else
   417				rc = generic_ip_connect(server);
   418			if (rc) {
   419				cifs_dbg(FYI, "reconnect error %d\n", rc);
   420				mutex_unlock(&server->srv_mutex);
   421				msleep(3000);
   422			} else {
   423				atomic_inc(&tcpSesReconnectCount);
   424				spin_lock(&GlobalMid_Lock);
   425				if (server->tcpStatus != CifsExiting)
   426					server->tcpStatus =
CifsNeedNegotiate;
   427				spin_unlock(&GlobalMid_Lock);
   428				mutex_unlock(&server->srv_mutex);
   429			}
   430		} while (server->tcpStatus == CifsNeedReconnect);
   431
   432		if (server->tcpStatus == CifsNeedNegotiate)
   433			mod_delayed_work(cifsiod_wq, &server->echo, 0);
   434
   435		return rc;
   436	}
   437

---
0-DAY kernel test infrastructure Open Source Technology Center
https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.01
.org%2Fpipermail%2Fkbuild- all&data=02%7C01%7Clongli%40microsoft.com%7C8eeef6813ee14ded2dcc08 d5b4a4113a%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C63661353 9125415461&sdata=OrOmZ1yCHuTbOlK8c6oBG9FpUbjBcQR5nGGa%2BntzwL
E%3D&reserved=0 Intel Corporation