2019-05-14 04:03:42

by Long Li

Subject: [PATCH 1/2] cifs:smbd When reconnecting to server, call smbd_destroy() after all MIDs have been called

From: Long Li <[email protected]>

commit 214bab448476 ("cifs: Call MID callback before destroying transport")
assumes that the MID callback should not take srv_mutex, this may not always
be true. SMB Direct requires the MID callback completed before calling
transport so all pending memory registration can be freed. So restore the
orignal calling sequence so TCP transport will use the same code, but moving
smbd_destroy() after all MID has been called.

fixes: 214bab448476 ("cifs: Call MID callback before destroying transport")
Signed-off-by: Long Li <[email protected]>
---
fs/cifs/connect.c | 37 ++++++++++++++++++++-----------------
1 file changed, 20 insertions(+), 17 deletions(-)

diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 084756cfdaee..0b3ac8b76d18 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -528,6 +528,21 @@ 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;
+ }
+ 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);
@@ -540,6 +555,7 @@ 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) {
@@ -548,24 +564,11 @@ 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))
+ if (cifs_rdma_enabled(server)) {
+ mutex_lock(&server->srv_mutex);
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);
+ mutex_unlock(&server->srv_mutex);
+ }

do {
try_to_freeze();
--
2.17.1

2019-05-14 04:05:34

by Long Li

Subject: [PATCH 2/2] cifs:smbd Use the correct DMA direction when sending data

From: Long Li <[email protected]>

When sending data, use the DMA_TO_DEVICE to map buffers. Also log the number
of requests in a compounding request from upper layer.

Signed-off-by: Long Li <[email protected]>
---
fs/cifs/smbdirect.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/fs/cifs/smbdirect.c b/fs/cifs/smbdirect.c
index 251ef1223206..caac37b1de8c 100644
--- a/fs/cifs/smbdirect.c
+++ b/fs/cifs/smbdirect.c
@@ -903,7 +903,7 @@ static int smbd_create_header(struct smbd_connection *info,
request->sge[0].addr = ib_dma_map_single(info->id->device,
(void *)packet,
header_length,
- DMA_BIDIRECTIONAL);
+ DMA_TO_DEVICE);
if (ib_dma_mapping_error(info->id->device, request->sge[0].addr)) {
mempool_free(request, info->request_mempool);
rc = -EIO;
@@ -1005,7 +1005,7 @@ static int smbd_post_send_sgl(struct smbd_connection *info,
for_each_sg(sgl, sg, num_sgs, i) {
request->sge[i+1].addr =
ib_dma_map_page(info->id->device, sg_page(sg),
- sg->offset, sg->length, DMA_BIDIRECTIONAL);
+ sg->offset, sg->length, DMA_TO_DEVICE);
if (ib_dma_mapping_error(
info->id->device, request->sge[i+1].addr)) {
rc = -EIO;
@@ -2110,8 +2110,10 @@ int smbd_send(struct TCP_Server_Info *server,
goto done;
}

- rqst_idx = 0;
+ log_write(INFO, "num_rqst=%d total length=%u\n",
+ num_rqst, remaining_data_length);

+ rqst_idx = 0;
next_rqst:
rqst = &rqst_array[rqst_idx];
iov = rqst->rq_iov;
--
2.17.1

2019-05-14 21:14:22

by Pavel Shilovsky

Subject: Re: [PATCH 1/2] cifs:smbd When reconnecting to server, call smbd_destroy() after all MIDs have been called

пн, 13 мая 2019 г. в 21:02, <[email protected]>:
>
> From: Long Li <[email protected]>
>
> commit 214bab448476 ("cifs: Call MID callback before destroying transport")
> assumes that the MID callback should not take srv_mutex, this may not always
> be true. SMB Direct requires the MID callback completed before calling
> transport so all pending memory registration can be freed. So restore the
> orignal calling sequence so TCP transport will use the same code, but moving
> smbd_destroy() after all MID has been called.
>
> fixes: 214bab448476 ("cifs: Call MID callback before destroying transport")
> Signed-off-by: Long Li <[email protected]>
> ---
> fs/cifs/connect.c | 37 ++++++++++++++++++++-----------------
> 1 file changed, 20 insertions(+), 17 deletions(-)
>
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index 084756cfdaee..0b3ac8b76d18 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -528,6 +528,21 @@ 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;
> + }
> + 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);
> @@ -540,6 +555,7 @@ 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) {
> @@ -548,24 +564,11 @@ 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))
> + if (cifs_rdma_enabled(server)) {
> + mutex_lock(&server->srv_mutex);
> 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);
> + mutex_unlock(&server->srv_mutex);
> + }
>
> do {
> try_to_freeze();
> --
> 2.17.1
>

Thanks for quickly fixing it!

Reviewed-by: Pavel Shilovsky <[email protected]>

--
Best regards,
Pavel Shilovsky

2019-05-14 21:43:22

by Pavel Shilovsky

Subject: Re: [PATCH 2/2] cifs:smbd Use the correct DMA direction when sending data

пн, 13 мая 2019 г. в 21:02, <[email protected]>:
>
> From: Long Li <[email protected]>
>
> When sending data, use the DMA_TO_DEVICE to map buffers. Also log the number
> of requests in a compounding request from upper layer.
>
> Signed-off-by: Long Li <[email protected]>
> ---
> fs/cifs/smbdirect.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/fs/cifs/smbdirect.c b/fs/cifs/smbdirect.c
> index 251ef1223206..caac37b1de8c 100644
> --- a/fs/cifs/smbdirect.c
> +++ b/fs/cifs/smbdirect.c
> @@ -903,7 +903,7 @@ static int smbd_create_header(struct smbd_connection *info,
> request->sge[0].addr = ib_dma_map_single(info->id->device,
> (void *)packet,
> header_length,
> - DMA_BIDIRECTIONAL);
> + DMA_TO_DEVICE);
> if (ib_dma_mapping_error(info->id->device, request->sge[0].addr)) {
> mempool_free(request, info->request_mempool);
> rc = -EIO;
> @@ -1005,7 +1005,7 @@ static int smbd_post_send_sgl(struct smbd_connection *info,
> for_each_sg(sgl, sg, num_sgs, i) {
> request->sge[i+1].addr =
> ib_dma_map_page(info->id->device, sg_page(sg),
> - sg->offset, sg->length, DMA_BIDIRECTIONAL);
> + sg->offset, sg->length, DMA_TO_DEVICE);
> if (ib_dma_mapping_error(
> info->id->device, request->sge[i+1].addr)) {
> rc = -EIO;
> @@ -2110,8 +2110,10 @@ int smbd_send(struct TCP_Server_Info *server,
> goto done;
> }
>
> - rqst_idx = 0;
> + log_write(INFO, "num_rqst=%d total length=%u\n",
> + num_rqst, remaining_data_length);
>
> + rqst_idx = 0;
> next_rqst:
> rqst = &rqst_array[rqst_idx];
> iov = rqst->rq_iov;
> --
> 2.17.1
>

Acked-by: Pavel Shilovsky <[email protected]>

--
Best regards,
Pavel Shilovsky

2019-05-15 04:17:39

by Steve French

Subject: Re: [PATCH 2/2] cifs:smbd Use the correct DMA direction when sending data

merged both patches into cifs-2.6.git for-next

On Mon, May 13, 2019 at 11:02 PM <[email protected]> wrote:
>
> From: Long Li <[email protected]>
>
> When sending data, use the DMA_TO_DEVICE to map buffers. Also log the number
> of requests in a compounding request from upper layer.
>
> Signed-off-by: Long Li <[email protected]>
> ---
> fs/cifs/smbdirect.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/fs/cifs/smbdirect.c b/fs/cifs/smbdirect.c
> index 251ef1223206..caac37b1de8c 100644
> --- a/fs/cifs/smbdirect.c
> +++ b/fs/cifs/smbdirect.c
> @@ -903,7 +903,7 @@ static int smbd_create_header(struct smbd_connection *info,
> request->sge[0].addr = ib_dma_map_single(info->id->device,
> (void *)packet,
> header_length,
> - DMA_BIDIRECTIONAL);
> + DMA_TO_DEVICE);
> if (ib_dma_mapping_error(info->id->device, request->sge[0].addr)) {
> mempool_free(request, info->request_mempool);
> rc = -EIO;
> @@ -1005,7 +1005,7 @@ static int smbd_post_send_sgl(struct smbd_connection *info,
> for_each_sg(sgl, sg, num_sgs, i) {
> request->sge[i+1].addr =
> ib_dma_map_page(info->id->device, sg_page(sg),
> - sg->offset, sg->length, DMA_BIDIRECTIONAL);
> + sg->offset, sg->length, DMA_TO_DEVICE);
> if (ib_dma_mapping_error(
> info->id->device, request->sge[i+1].addr)) {
> rc = -EIO;
> @@ -2110,8 +2110,10 @@ int smbd_send(struct TCP_Server_Info *server,
> goto done;
> }
>
> - rqst_idx = 0;
> + log_write(INFO, "num_rqst=%d total length=%u\n",
> + num_rqst, remaining_data_length);
>
> + rqst_idx = 0;
> next_rqst:
> rqst = &rqst_array[rqst_idx];
> iov = rqst->rq_iov;
> --
> 2.17.1
>


--
Thanks,

Steve