From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jens Axboe Subject: Re: [PATCH 1/1] null_blk: fix handling of BLKPREP_DEFER case Date: Mon, 27 Apr 2015 11:36:23 -0600 Message-ID: <553E7397.2010208@kernel.dk> References: <553E70FA.1080302@kernel.dk> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="------------060004090101030101030102" To: Dmitry Krivenok , linux-fsdevel@vger.kernel.org Return-path: Received: from mail-ie0-f175.google.com ([209.85.223.175]:36140 "EHLO mail-ie0-f175.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932203AbbD0Rg1 (ORCPT ); Mon, 27 Apr 2015 13:36:27 -0400 Received: by iebrs15 with SMTP id rs15so132065555ieb.3 for ; Mon, 27 Apr 2015 10:36:26 -0700 (PDT) In-Reply-To: <553E70FA.1080302@kernel.dk> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: This is a multi-part message in MIME format. --------------060004090101030101030102 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit 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 --------------060004090101030101030102 Content-Type: text/x-patch; name="nullb-defer.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="nullb-defer.patch" 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, --------------060004090101030101030102--