Skip to content

Commit 8337b02

Browse files
Ming Leiaxboe
authored andcommitted
nbd: fix partial sending
nbd driver sends request header and payload with multiple call of sock_sendmsg, and partial sending can't be avoided. However, nbd driver returns BLK_STS_RESOURCE to block core in this situation. This way causes one issue: request->tag may change in the next run of nbd_queue_rq(), but the original old tag has been sent as part of header cookie, this way confuses nbd driver reply handling, since the real request can't be retrieved any more with the obsolete old tag. Fix it by retrying sending directly in per-socket work function, meantime return BLK_STS_OK to block layer core. Cc: vincent.chen@sifive.com Cc: Leon Schuermann <leon@is.currently.online> Cc: Bart Van Assche <bvanassche@acm.org> Reported-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Ming Lei <ming.lei@redhat.com> Tested-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Link: https://lore.kernel.org/r/20241029011941.153037-1-ming.lei@redhat.com Signed-off-by: Jens Axboe <axboe@kernel.dk>
1 parent 7c0be4e commit 8337b02

File tree

1 file changed

+85
-10
lines changed

1 file changed

+85
-10
lines changed

drivers/block/nbd.c

Lines changed: 85 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ struct nbd_sock {
6262
bool dead;
6363
int fallback_index;
6464
int cookie;
65+
struct work_struct work;
6566
};
6667

6768
struct recv_thread_args {
@@ -141,6 +142,9 @@ struct nbd_device {
141142
*/
142143
#define NBD_CMD_INFLIGHT 2
143144

145+
/* Just part of request header or data payload is sent successfully */
146+
#define NBD_CMD_PARTIAL_SEND 3
147+
144148
struct nbd_cmd {
145149
struct nbd_device *nbd;
146150
struct mutex lock;
@@ -453,6 +457,12 @@ static enum blk_eh_timer_return nbd_xmit_timeout(struct request *req)
453457
if (!mutex_trylock(&cmd->lock))
454458
return BLK_EH_RESET_TIMER;
455459

460+
/* partial send is handled in nbd_sock's work function */
461+
if (test_bit(NBD_CMD_PARTIAL_SEND, &cmd->flags)) {
462+
mutex_unlock(&cmd->lock);
463+
return BLK_EH_RESET_TIMER;
464+
}
465+
456466
if (!test_bit(NBD_CMD_INFLIGHT, &cmd->flags)) {
457467
mutex_unlock(&cmd->lock);
458468
return BLK_EH_DONE;
@@ -601,6 +611,30 @@ static inline int was_interrupted(int result)
601611
return result == -ERESTARTSYS || result == -EINTR;
602612
}
603613

614+
/*
615+
* We've already sent header or part of data payload, have no choice but
616+
* to set pending and schedule it in work.
617+
*
618+
* And we have to return BLK_STS_OK to block core, otherwise this same
619+
* request may be re-dispatched with different tag, but our header has
620+
* been sent out with old tag, and this way does confuse reply handling.
621+
*/
622+
static void nbd_sched_pending_work(struct nbd_device *nbd,
623+
struct nbd_sock *nsock,
624+
struct nbd_cmd *cmd, int sent)
625+
{
626+
struct request *req = blk_mq_rq_from_pdu(cmd);
627+
628+
/* pending work should be scheduled only once */
629+
WARN_ON_ONCE(test_bit(NBD_CMD_PARTIAL_SEND, &cmd->flags));
630+
631+
nsock->pending = req;
632+
nsock->sent = sent;
633+
set_bit(NBD_CMD_PARTIAL_SEND, &cmd->flags);
634+
refcount_inc(&nbd->config_refs);
635+
schedule_work(&nsock->work);
636+
}
637+
604638
/*
605639
* Returns BLK_STS_RESOURCE if the caller should retry after a delay.
606640
* Returns BLK_STS_IOERR if sending failed.
@@ -686,8 +720,8 @@ static blk_status_t nbd_send_cmd(struct nbd_device *nbd, struct nbd_cmd *cmd,
686720
* completely done.
687721
*/
688722
if (sent) {
689-
nsock->pending = req;
690-
nsock->sent = sent;
723+
nbd_sched_pending_work(nbd, nsock, cmd, sent);
724+
return BLK_STS_OK;
691725
}
692726
set_bit(NBD_CMD_REQUEUED, &cmd->flags);
693727
return BLK_STS_RESOURCE;
@@ -724,14 +758,8 @@ static blk_status_t nbd_send_cmd(struct nbd_device *nbd, struct nbd_cmd *cmd,
724758
result = sock_xmit(nbd, index, 1, &from, flags, &sent);
725759
if (result < 0) {
726760
if (was_interrupted(result)) {
727-
/* We've already sent the header, we
728-
* have no choice but to set pending and
729-
* return BUSY.
730-
*/
731-
nsock->pending = req;
732-
nsock->sent = sent;
733-
set_bit(NBD_CMD_REQUEUED, &cmd->flags);
734-
return BLK_STS_RESOURCE;
761+
nbd_sched_pending_work(nbd, nsock, cmd, sent);
762+
return BLK_STS_OK;
735763
}
736764
dev_err(disk_to_dev(nbd->disk),
737765
"Send data failed (result %d)\n",
@@ -757,6 +785,14 @@ static blk_status_t nbd_send_cmd(struct nbd_device *nbd, struct nbd_cmd *cmd,
757785
return BLK_STS_OK;
758786

759787
requeue:
788+
/*
789+
* Can't requeue in case we are dealing with partial send
790+
*
791+
* We must run from pending work function.
792+
* */
793+
if (test_bit(NBD_CMD_PARTIAL_SEND, &cmd->flags))
794+
return BLK_STS_OK;
795+
760796
/* retry on a different socket */
761797
dev_err_ratelimited(disk_to_dev(nbd->disk),
762798
"Request send failed, requeueing\n");
@@ -765,6 +801,44 @@ static blk_status_t nbd_send_cmd(struct nbd_device *nbd, struct nbd_cmd *cmd,
765801
return BLK_STS_OK;
766802
}
767803

804+
/* handle partial sending */
805+
static void nbd_pending_cmd_work(struct work_struct *work)
806+
{
807+
struct nbd_sock *nsock = container_of(work, struct nbd_sock, work);
808+
struct request *req = nsock->pending;
809+
struct nbd_cmd *cmd = blk_mq_rq_to_pdu(req);
810+
struct nbd_device *nbd = cmd->nbd;
811+
unsigned long deadline = READ_ONCE(req->deadline);
812+
unsigned int wait_ms = 2;
813+
814+
mutex_lock(&cmd->lock);
815+
816+
WARN_ON_ONCE(test_bit(NBD_CMD_REQUEUED, &cmd->flags));
817+
if (WARN_ON_ONCE(!test_bit(NBD_CMD_PARTIAL_SEND, &cmd->flags)))
818+
goto out;
819+
820+
mutex_lock(&nsock->tx_lock);
821+
while (true) {
822+
nbd_send_cmd(nbd, cmd, cmd->index);
823+
if (!nsock->pending)
824+
break;
825+
826+
/* don't bother timeout handler for partial sending */
827+
if (READ_ONCE(jiffies) + msecs_to_jiffies(wait_ms) >= deadline) {
828+
cmd->status = BLK_STS_IOERR;
829+
blk_mq_complete_request(req);
830+
break;
831+
}
832+
msleep(wait_ms);
833+
wait_ms *= 2;
834+
}
835+
mutex_unlock(&nsock->tx_lock);
836+
clear_bit(NBD_CMD_PARTIAL_SEND, &cmd->flags);
837+
out:
838+
mutex_unlock(&cmd->lock);
839+
nbd_config_put(nbd);
840+
}
841+
768842
static int nbd_read_reply(struct nbd_device *nbd, struct socket *sock,
769843
struct nbd_reply *reply)
770844
{
@@ -1211,6 +1285,7 @@ static int nbd_add_socket(struct nbd_device *nbd, unsigned long arg,
12111285
nsock->pending = NULL;
12121286
nsock->sent = 0;
12131287
nsock->cookie = 0;
1288+
INIT_WORK(&nsock->work, nbd_pending_cmd_work);
12141289
socks[config->num_connections++] = nsock;
12151290
atomic_inc(&config->live_connections);
12161291
blk_mq_unfreeze_queue(nbd->disk->queue);

0 commit comments

Comments
 (0)