Re: [PATCH 1/1] null_blk: fix handling of BLKPREP_DEFER case

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 04/27/2015 11:25 AM, Jens Axboe wrote:
On 04/19/2015 12:19 PM, Dmitry Krivenok wrote:
When we fail to allocate new cmd in null_rq_prep_fn we return
BLKPREP_DEFER
which is not handled properly. In single-queue mode of null_blk the
following
command hangs forever in io_schedule():
$ dd if=/dev/nullb0 of=/dev/null bs=8M count=5000 iflag=direct

The reason is that when 64 commands have been allocated, the 65th command
allocation will fail due to missing free tag. The request, however,
will be
kept in the queue which will never be started again (unless you run
another
command that does I/O to /dev/nullb0).

This small patch tries to solve the issue by stopping the queue when we
detect that all tags were exhausted and starting it again when we free
the tag.

I've verified that the command mentioned above doesn't hang anymore
and also
made sure that null_blk with my change survives fio-based stress tests.

You are right, legacy request_fn mode has a bug there. I'd get rid of
the no_cmds bool, though. If we fail allocating a command in alloc_cmd()
and we're in NULL_Q_RQ mode, stop the queue. In free_cmd(), again if
we're in NULL_Q_RQ_MODE, check the stopped flag and start the queue if
it is set.

Something like the attached, totally untested.

--
Jens Axboe

diff --git a/drivers/block/null_blk.c b/drivers/block/null_blk.c
index 65cd61a4145e..d7141d5ef4ff 100644
--- a/drivers/block/null_blk.c
+++ b/drivers/block/null_blk.c
@@ -24,6 +24,7 @@ struct nullb_queue {
 	wait_queue_head_t wait;
 	unsigned int queue_depth;
 
+	struct nullb *dev;
 	struct nullb_cmd *cmds;
 };
 
@@ -153,6 +154,13 @@ static void put_tag(struct nullb_queue *nq, unsigned int tag)
 
 	if (waitqueue_active(&nq->wait))
 		wake_up(&nq->wait);
+
+	if (queue_mode == NULL_Q_RQ) {
+		struct nullb *nullb = nq->dev;
+
+		if (blk_queue_stopped(nullb->q))
+			blk_start_queue(nullb->q);
+	}
 }
 
 static unsigned int get_tag(struct nullb_queue *nq)
@@ -196,7 +204,7 @@ static struct nullb_cmd *alloc_cmd(struct nullb_queue *nq, int can_wait)
 
 	cmd = __alloc_cmd(nq);
 	if (cmd || !can_wait)
-		return cmd;
+		goto out;
 
 	do {
 		prepare_to_wait(&nq->wait, &wait, TASK_UNINTERRUPTIBLE);
@@ -208,6 +216,11 @@ static struct nullb_cmd *alloc_cmd(struct nullb_queue *nq, int can_wait)
 	} while (1);
 
 	finish_wait(&nq->wait, &wait);
+out:
+	if (!cmd) {
+		BUG_ON(queue_mode != NULL_Q_RQ);
+		blk_stop_queue(nq->dev->q);
+	}
 	return cmd;
 }
 
@@ -372,6 +385,7 @@ static void null_init_queue(struct nullb *nullb, struct nullb_queue *nq)
 
 	init_waitqueue_head(&nq->wait);
 	nq->queue_depth = nullb->queue_depth;
+	nq->dev = nullb;
 }
 
 static int null_init_hctx(struct blk_mq_hw_ctx *hctx, void *data,

[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [Samba]     [Device Mapper]     [CEPH Development]
  Powered by Linux