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

smb: server: make only use of wake_up[_all]() in transport_rdma.c 

wake_up_interruptible(() doesn't wake up tasks waiting with wait_event().

So we better wake_up[_all]() in order to wake up all tasks in order
to simplify the logic.

As we currently don't use any wait_event_*_exclusive() it
doesn't really matter if we use wake_up() or wake_up_all().
But in this patch I try to use wake_up() for expected situations
and wake_up_all() for situations of a broken connection.
So don't need to adjust things in future when we
may use wait_event_*_exclusive() in order to wake up
only one process that should make progress.

Changing the wait_event_*() code in order to keep
wait_event(), wait_event_interruptible() and
wait_event_interruptible_timeout() or
changing them to wait_event_killable(),
wait_event_killable_timeout(),
wait_event_killable_exclusive()
is something to think about in a future patch.

The goal here is to avoid that some tasks are not
woken and freeze forever.

Also note that this patch only changes the existing
wake_up*() calls. Adding more wake_up*() calls for
other wait queues is also deferred to a future patch.

Link: https://lore.kernel.org/linux-cifs/13851363-0dc9-465c-9ced-3ede4904eef0@samba.org/T/#t


Cc: Namjae Jeon <linkinjeon@kernel.org>
Cc: Steve French <smfrench@gmail.com>
Cc: Tom Talpey <tom@talpey.com>
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 98dc77b3

fs/smb/server/transport_rdma.c

+9 −9
Original line number Diff line number Diff line
@@ -357,7 +357,7 @@ static void free_transport(struct smb_direct_transport *t) 
	struct smbdirect_socket *sc = &t->socket;

	struct smbdirect_recv_io *recvmsg;



	wake_up_interruptible(&t->wait_send_credits);

	wake_up_all(&t->wait_send_credits);



	ksmbd_debug(RDMA, "wait for all send posted to IB to finish\n");

	wait_event(t->wait_send_pending,
@@ -524,7 +524,7 @@ static void recv_done(struct ib_cq *cq, struct ib_wc *wc) 
		sc->recv_io.reassembly.full_packet_received = true;

		sc->status = SMBDIRECT_SOCKET_CONNECTED;

		enqueue_reassembly(t, recvmsg, 0);

		wake_up_interruptible(&t->wait_status);

		wake_up(&t->wait_status);

		return;

	case SMBDIRECT_EXPECT_DATA_TRANSFER: {

		struct smbdirect_data_transfer *data_transfer =
@@ -587,14 +587,14 @@ static void recv_done(struct ib_cq *cq, struct ib_wc *wc) 
			queue_work(smb_direct_wq, &t->send_immediate_work);



		if (atomic_read(&t->send_credits) > 0)

			wake_up_interruptible(&t->wait_send_credits);

			wake_up(&t->wait_send_credits);



		if (is_receive_credit_post_required(receive_credits, avail_recvmsg_count))

			queue_work(smb_direct_wq, &t->post_recv_credits_work);



		if (data_length) {

			enqueue_reassembly(t, recvmsg, (int)data_length);

			wake_up_interruptible(&sc->recv_io.reassembly.wait_queue);

			wake_up(&sc->recv_io.reassembly.wait_queue);

		} else

			put_recvmsg(t, recvmsg);
@@ -1570,7 +1570,7 @@ static int smb_direct_cm_handler(struct rdma_cm_id *cm_id, 
	switch (event->event) {

	case RDMA_CM_EVENT_ESTABLISHED: {

		sc->status = SMBDIRECT_SOCKET_CONNECTED;

		wake_up_interruptible(&t->wait_status);

		wake_up(&t->wait_status);

		break;

	}

	case RDMA_CM_EVENT_DEVICE_REMOVAL:
@@ -1578,14 +1578,14 @@ static int smb_direct_cm_handler(struct rdma_cm_id *cm_id, 
		ib_drain_qp(sc->ib.qp);



		sc->status = SMBDIRECT_SOCKET_DISCONNECTED;

		wake_up_interruptible(&t->wait_status);

		wake_up_interruptible(&sc->recv_io.reassembly.wait_queue);

		wake_up(&t->wait_send_credits);

		wake_up_all(&t->wait_status);

		wake_up_all(&sc->recv_io.reassembly.wait_queue);

		wake_up_all(&t->wait_send_credits);

		break;

	}

	case RDMA_CM_EVENT_CONNECT_ERROR: {

		sc->status = SMBDIRECT_SOCKET_DISCONNECTED;

		wake_up_interruptible(&t->wait_status);

		wake_up_all(&t->wait_status);

		break;

	}

	default: