Commit dc10cf13 authored by Stefan Metzmacher's avatar Stefan Metzmacher Committed by Steve French
Browse files

smb: client: relax WARN_ON_ONCE(SMBDIRECT_SOCKET_*) checks in recv_done() and smbd_conn_upcall() 



sc->first_error might already be set and sc->status
is thus unexpected, so this should avoid the WARN[_ON]_ONCE()
if sc->first_error is already set and have a usable error path.

While there set sc->first_error as soon as possible.

This is done based on a problem seen in similar places on
the server. And there it was already very useful in order
to find the problem when we have a meaningful WARN_ONCE()
that prints details about the connection.

This is much more useful:

[  309.560973] expected[NEGOTIATE_NEEDED] != RDMA_CONNECT_RUNNING
first_error=0 local=192.168.0.200:445 remote=192.168.0.100:60445
[  309.561034] WARNING: CPU: 2 PID: 78 at transport_rdma.c:643
recv_done+0x2fa/0x3d0 [ksmbd]

than what we had before (only):

[  894.140316] WARNING: CPU: 1 PID: 116 at
fs/smb/server/transport_rdma.c:642 recv_done+0x308/0x360 [ksmbd]

Fixes: 58dfba8a ("smb: client/smbdirect: replace SMBDIRECT_SOCKET_CONNECTING with more detailed states")
Cc: Steve French <smfrench@gmail.com>
Cc: Tom Talpey <tom@talpey.com>
Cc: Long Li <longli@microsoft.com>
Cc: Namjae Jeon <linkinjeon@kernel.org>
Cc: Paulo Alcantara <pc@manguebit.org>
Cc: linux-cifs@vger.kernel.org
Cc: samba-technical@lists.samba.org
Signed-off-by: default avatarStefan Metzmacher <metze@samba.org>
Acked-by: default avatarNamjae Jeon <linkinjeon@kernel.org>
Signed-off-by: default avatarSteve French <stfrench@microsoft.com>
parent 425c3275

fs/smb/client/smbdirect.c

+15 −13
Original line number Diff line number Diff line
@@ -7,6 +7,7 @@ 
#include <linux/module.h>

#include <linux/highmem.h>

#include <linux/folio_queue.h>

#define __SMBDIRECT_SOCKET_DISCONNECT(__sc) smbd_disconnect_rdma_connection(__sc)

#include "../common/smbdirect/smbdirect_pdu.h"

#include "smbdirect.h"

#include "cifs_debug.h"
@@ -186,6 +187,9 @@ static void smbd_disconnect_rdma_work(struct work_struct *work) 
	struct smbdirect_socket *sc =

		container_of(work, struct smbdirect_socket, disconnect_work);



	if (sc->first_error == 0)

		sc->first_error = -ECONNABORTED;



	/*

	 * make sure this and other work is not queued again

	 * but here we don't block and avoid
@@ -197,9 +201,6 @@ static void smbd_disconnect_rdma_work(struct work_struct *work) 
	disable_work(&sc->idle.immediate_work);

	disable_delayed_work(&sc->idle.timer_work);



	if (sc->first_error == 0)

		sc->first_error = -ECONNABORTED;



	switch (sc->status) {

	case SMBDIRECT_SOCKET_NEGOTIATE_NEEDED:

	case SMBDIRECT_SOCKET_NEGOTIATE_RUNNING:
@@ -242,6 +243,9 @@ static void smbd_disconnect_rdma_work(struct work_struct *work) 


static void smbd_disconnect_rdma_connection(struct smbdirect_socket *sc)

{

	if (sc->first_error == 0)

		sc->first_error = -ECONNABORTED;



	/*

	 * make sure other work (than disconnect_work) is

	 * not queued again but here we don't block and avoid
@@ -252,9 +256,6 @@ static void smbd_disconnect_rdma_connection(struct smbdirect_socket *sc) 
	disable_work(&sc->idle.immediate_work);

	disable_delayed_work(&sc->idle.timer_work);



	if (sc->first_error == 0)

		sc->first_error = -ECONNABORTED;



	switch (sc->status) {

	case SMBDIRECT_SOCKET_RESOLVE_ADDR_FAILED:

	case SMBDIRECT_SOCKET_RESOLVE_ROUTE_FAILED:
@@ -322,27 +323,27 @@ static int smbd_conn_upcall( 


	switch (event->event) {

	case RDMA_CM_EVENT_ADDR_RESOLVED:

		WARN_ON_ONCE(sc->status != SMBDIRECT_SOCKET_RESOLVE_ADDR_RUNNING);

		if (SMBDIRECT_CHECK_STATUS_DISCONNECT(sc, SMBDIRECT_SOCKET_RESOLVE_ADDR_RUNNING))

			break;

		sc->status = SMBDIRECT_SOCKET_RESOLVE_ROUTE_NEEDED;

		wake_up(&sc->status_wait);

		break;



	case RDMA_CM_EVENT_ROUTE_RESOLVED:

		WARN_ON_ONCE(sc->status != SMBDIRECT_SOCKET_RESOLVE_ROUTE_RUNNING);

		if (SMBDIRECT_CHECK_STATUS_DISCONNECT(sc, SMBDIRECT_SOCKET_RESOLVE_ROUTE_RUNNING))

			break;

		sc->status = SMBDIRECT_SOCKET_RDMA_CONNECT_NEEDED;

		wake_up(&sc->status_wait);

		break;



	case RDMA_CM_EVENT_ADDR_ERROR:

		log_rdma_event(ERR, "connecting failed event=%s\n", event_name);

		WARN_ON_ONCE(sc->status != SMBDIRECT_SOCKET_RESOLVE_ADDR_RUNNING);

		sc->status = SMBDIRECT_SOCKET_RESOLVE_ADDR_FAILED;

		smbd_disconnect_rdma_work(&sc->disconnect_work);

		break;



	case RDMA_CM_EVENT_ROUTE_ERROR:

		log_rdma_event(ERR, "connecting failed event=%s\n", event_name);

		WARN_ON_ONCE(sc->status != SMBDIRECT_SOCKET_RESOLVE_ROUTE_RUNNING);

		sc->status = SMBDIRECT_SOCKET_RESOLVE_ROUTE_FAILED;

		smbd_disconnect_rdma_work(&sc->disconnect_work);

		break;
@@ -428,7 +429,8 @@ static int smbd_conn_upcall( 
					min_t(u8, sp->responder_resources,

					      peer_responder_resources);



		WARN_ON_ONCE(sc->status != SMBDIRECT_SOCKET_RDMA_CONNECT_RUNNING);

		if (SMBDIRECT_CHECK_STATUS_DISCONNECT(sc, SMBDIRECT_SOCKET_RDMA_CONNECT_RUNNING))

			break;

		sc->status = SMBDIRECT_SOCKET_NEGOTIATE_NEEDED;

		wake_up(&sc->status_wait);

		break;
@@ -437,7 +439,6 @@ static int smbd_conn_upcall( 
	case RDMA_CM_EVENT_UNREACHABLE:

	case RDMA_CM_EVENT_REJECTED:

		log_rdma_event(ERR, "connecting failed event=%s\n", event_name);

		WARN_ON_ONCE(sc->status != SMBDIRECT_SOCKET_RDMA_CONNECT_RUNNING);

		sc->status = SMBDIRECT_SOCKET_RDMA_CONNECT_FAILED;

		smbd_disconnect_rdma_work(&sc->disconnect_work);

		break;
@@ -699,7 +700,8 @@ static void recv_done(struct ib_cq *cq, struct ib_wc *wc) 
		negotiate_done =

			process_negotiation_response(response, wc->byte_len);

		put_receive_buffer(sc, response);

		WARN_ON_ONCE(sc->status != SMBDIRECT_SOCKET_NEGOTIATE_RUNNING);

		if (SMBDIRECT_CHECK_STATUS_WARN(sc, SMBDIRECT_SOCKET_NEGOTIATE_RUNNING))

			negotiate_done = false;

		if (!negotiate_done) {

			sc->status = SMBDIRECT_SOCKET_NEGOTIATE_FAILED;

			smbd_disconnect_rdma_connection(sc);