[PATCH] nvmet-tcp: use 'spin_lock_bh' for state_lock()

[PATCH] nvmet-tcp: use 'spin_lock_bh' for state_lock()

[Hannes Reinecke]

nvmet_tcp_schedule_release_queue() is called from socket state
change callbacks, which may be called from an softirq context.
So use 'spin_lock_bh' to avoid a spin lock warning.

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/nvme/target/tcp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/nvme/target/tcp.c b/drivers/nvme/target/tcp.c
index 868aa4de2e4c..f131b504aa01 100644
--- a/drivers/nvme/target/tcp.c
+++ b/drivers/nvme/target/tcp.c
@@ -1285,12 +1285,12 @@ static int nvmet_tcp_try_recv(struct nvmet_tcp_queue *queue,
 
 static void nvmet_tcp_schedule_release_queue(struct nvmet_tcp_queue *queue)
 {
-	spin_lock(&queue->state_lock);
+	spin_lock_bh(&queue->state_lock);
 	if (queue->state != NVMET_TCP_Q_DISCONNECTING) {
 		queue->state = NVMET_TCP_Q_DISCONNECTING;
 		queue_work(nvmet_wq, &queue->release_work);
 	}
-	spin_unlock(&queue->state_lock);
+	spin_unlock_bh(&queue->state_lock);
 }
 
 static inline void nvmet_tcp_arm_queue_deadline(struct nvmet_tcp_queue *queue)
-- 
2.35.3

Re: [PATCH] nvmet-tcp: use 'spin_lock_bh' for state_lock()

[Sagi Grimberg]

Patch looks good, but please phrase the bug in
the patch title instead of the solution.

> nvmet_tcp_schedule_release_queue() is called from socket state
> change callbacks, which may be called from an softirq context.
> So use 'spin_lock_bh' to avoid a spin lock warning.
> 
> Signed-off-by: Hannes Reinecke <hare@suse.de>

Please add a fixes tag.

[PATCH] nvmet-tcp: use 'spin_lock_bh' for state_lock()

[Hannes Reinecke]

nvmet_tcp_schedule_release_queue() is called from socket state
change callbacks, which may be called from an softirq context.
So use 'spin_lock_bh' to avoid a spin lock warning.

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/nvme/target/tcp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/nvme/target/tcp.c b/drivers/nvme/target/tcp.c
index 493645096eca..cd5ef64f1bc9 100644
--- a/drivers/nvme/target/tcp.c
+++ b/drivers/nvme/target/tcp.c
@@ -1379,7 +1379,7 @@ static void nvmet_tcp_release_queue(struct kref *kref)
 
 static void nvmet_tcp_schedule_release_queue(struct nvmet_tcp_queue *queue)
 {
-	spin_lock(&queue->state_lock);
+	spin_lock_bh(&queue->state_lock);
 	if (queue->state == NVMET_TCP_Q_TLS_HANDSHAKE) {
 		/* Socket closed during handshake */
 		tls_handshake_cancel(queue->sock->sk);
@@ -1388,7 +1388,7 @@ static void nvmet_tcp_schedule_release_queue(struct nvmet_tcp_queue *queue)
 		queue->state = NVMET_TCP_Q_DISCONNECTING;
 		kref_put(&queue->kref, nvmet_tcp_release_queue);
 	}
-	spin_unlock(&queue->state_lock);
+	spin_unlock_bh(&queue->state_lock);
 }
 
 static inline void nvmet_tcp_arm_queue_deadline(struct nvmet_tcp_queue *queue)
-- 
2.35.3

Re: [PATCH] nvmet-tcp: use 'spin_lock_bh' for state_lock()

[Keith Busch]

On Thu, Oct 12, 2023 at 02:59:54PM +0200, Hannes Reinecke wrote:
> nvmet_tcp_schedule_release_queue() is called from socket state
> change callbacks, which may be called from an softirq context.
> So use 'spin_lock_bh' to avoid a spin lock warning.

Doh, I missed you already sent this fix:

  http://lists.infradead.org/pipermail/linux-nvme/2023-August/041593.html

I can apply it on nvme-6.7, though this feels like a fix that can go in
now. Depending on which way we go, it will cause a merge conflict for
either stable or the next merge window. Do you have a preference? I'd
just do 6.7 if you don't care.

Re: [PATCH] nvmet-tcp: use 'spin_lock_bh' for state_lock()

[Hannes Reinecke]

On 10/12/23 17:09, Keith Busch wrote:
> On Thu, Oct 12, 2023 at 02:59:54PM +0200, Hannes Reinecke wrote:
>> nvmet_tcp_schedule_release_queue() is called from socket state
>> change callbacks, which may be called from an softirq context.
>> So use 'spin_lock_bh' to avoid a spin lock warning.
> 
> Doh, I missed you already sent this fix:
> 
>    http://lists.infradead.org/pipermail/linux-nvme/2023-August/041593.html
> 
> I can apply it on nvme-6.7, though this feels like a fix that can go in
> now. Depending on which way we go, it will cause a merge conflict for
> either stable or the next merge window. Do you have a preference? I'd
> just do 6.7 if you don't care.

Na, just apply it on top of the TLS patches. That should be fine.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                Kernel Storage Architect
hare@suse.de                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Ivo Totev, Andrew
Myers, Andrew McDonald, Martje Boudien Moerman

Re: [PATCH] nvmet-tcp: use 'spin_lock_bh' for state_lock()

[Keith Busch]

Applied to nvme-6.7, thanks!