All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jens Axboe <axboe@kernel.dk>
To: Stephen Bates <sbates@raithlin.com>, Omar Sandoval <osandov@osandov.com>
Cc: "linux-block@vger.kernel.org" <linux-block@vger.kernel.org>,
	"linux-nvme@lists.infradead.org" <linux-nvme@lists.infradead.org>,
	"Damien.LeMoal@wdc.com" <Damien.LeMoal@wdc.com>,
	"sagi@grimberg.me" <sagi@grimberg.me>
Subject: Re: [PATCH v3 2/2] blk-mq: Add a polling specific stats function
Date: Thu, 20 Apr 2017 14:53:08 -0600	[thread overview]
Message-ID: <ca5bec05-ba3c-55db-6f3d-ef0965a79737@kernel.dk> (raw)
In-Reply-To: <D356CC80-F509-4427-B999-641636131E8D@raithlin.com>

On 04/20/2017 02:47 PM, Stephen  Bates wrote:
> 
>> I agree, it's fine as-is. We should queue it up for 4.12.
> 
> Great. I will get something based on Omar’s latest comments asap.
> 
>>> However right now I am stuck as I am seeing the kernel oops I reported
>>> before in testing of my latest patchset [1]. I will try and find some
>>> time to bisect that but it looks like it was introduced when the
>>> support for mq schedulers was added (on or around bd166ef18).
> 
>> Just replied to that one, let me know if the suggestion works.
> 
> That suggestion worked. Do you want me to send a patch for it?

Is the patch going to be different than the one I sent? Here it
is, with a comment added. Can I add you tested-by?

diff --git a/block/blk-mq.c b/block/blk-mq.c
index 572966f49596..c7836a1ded97 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -2928,8 +2928,17 @@ bool blk_mq_poll(struct request_queue *q, blk_qc_t cookie)
 	hctx = q->queue_hw_ctx[blk_qc_t_to_queue_num(cookie)];
 	if (!blk_qc_t_is_internal(cookie))
 		rq = blk_mq_tag_to_rq(hctx->tags, blk_qc_t_to_tag(cookie));
-	else
+	else {
 		rq = blk_mq_tag_to_rq(hctx->sched_tags, blk_qc_t_to_tag(cookie));
+		/*
+		 * With scheduling, if the request has completed, we'll
+		 * get a NULL return here, as we clear the sched tag when
+		 * that happens. The request still remains valid, like always,
+		 * so we should be safe with just the NULL check.
+		 */
+		if (!rq)
+			return false;
+	}
 
 	return __blk_mq_poll(hctx, rq);
 }

-- 
Jens Axboe

WARNING: multiple messages have this Message-ID (diff)
From: axboe@kernel.dk (Jens Axboe)
Subject: [PATCH v3 2/2] blk-mq: Add a polling specific stats function
Date: Thu, 20 Apr 2017 14:53:08 -0600	[thread overview]
Message-ID: <ca5bec05-ba3c-55db-6f3d-ef0965a79737@kernel.dk> (raw)
In-Reply-To: <D356CC80-F509-4427-B999-641636131E8D@raithlin.com>

On 04/20/2017 02:47 PM, Stephen  Bates wrote:
> 
>> I agree, it's fine as-is. We should queue it up for 4.12.
> 
> Great. I will get something based on Omar?s latest comments asap.
> 
>>> However right now I am stuck as I am seeing the kernel oops I reported
>>> before in testing of my latest patchset [1]. I will try and find some
>>> time to bisect that but it looks like it was introduced when the
>>> support for mq schedulers was added (on or around bd166ef18).
> 
>> Just replied to that one, let me know if the suggestion works.
> 
> That suggestion worked. Do you want me to send a patch for it?

Is the patch going to be different than the one I sent? Here it
is, with a comment added. Can I add you tested-by?

diff --git a/block/blk-mq.c b/block/blk-mq.c
index 572966f49596..c7836a1ded97 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -2928,8 +2928,17 @@ bool blk_mq_poll(struct request_queue *q, blk_qc_t cookie)
 	hctx = q->queue_hw_ctx[blk_qc_t_to_queue_num(cookie)];
 	if (!blk_qc_t_is_internal(cookie))
 		rq = blk_mq_tag_to_rq(hctx->tags, blk_qc_t_to_tag(cookie));
-	else
+	else {
 		rq = blk_mq_tag_to_rq(hctx->sched_tags, blk_qc_t_to_tag(cookie));
+		/*
+		 * With scheduling, if the request has completed, we'll
+		 * get a NULL return here, as we clear the sched tag when
+		 * that happens. The request still remains valid, like always,
+		 * so we should be safe with just the NULL check.
+		 */
+		if (!rq)
+			return false;
+	}
 
 	return __blk_mq_poll(hctx, rq);
 }

-- 
Jens Axboe

  reply	other threads:[~2017-04-20 20:53 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-04-07 12:24 [PATCH v3 0/2] blk-stat: Add ability to not bucket IO; improve IO polling sbates
2017-04-07 12:24 ` sbates
2017-04-07 12:24 ` [PATCH v3 1/2] blk-stat: convert blk-stat bucket callback to signed sbates
2017-04-07 12:24   ` sbates
2017-04-07 12:24 ` [PATCH v3 2/2] blk-mq: Add a polling specific stats function sbates
2017-04-07 12:24   ` sbates
2017-04-20 20:07   ` Omar Sandoval
2017-04-20 20:07     ` Omar Sandoval
2017-04-20 20:16     ` Jens Axboe
2017-04-20 20:16       ` Jens Axboe
2017-04-20 20:20       ` Omar Sandoval
2017-04-20 20:20         ` Omar Sandoval
2017-04-20 20:22         ` Jens Axboe
2017-04-20 20:22           ` Jens Axboe
2017-04-20 20:33           ` Stephen  Bates
2017-04-20 20:33             ` Stephen  Bates
2017-04-20 20:34             ` Jens Axboe
2017-04-20 20:34               ` Jens Axboe
2017-04-20 20:47               ` Stephen  Bates
2017-04-20 20:47                 ` Stephen  Bates
2017-04-20 20:53                 ` Jens Axboe [this message]
2017-04-20 20:53                   ` Jens Axboe
2017-04-20 21:08                   ` Stephen  Bates
2017-04-20 21:08                     ` Stephen  Bates
2017-04-20 21:14                     ` Jens Axboe
2017-04-20 21:14                       ` Jens Axboe
2017-04-20 21:41                       ` Stephen  Bates
2017-04-20 21:41                         ` Stephen  Bates
2017-04-20 21:42                         ` Jens Axboe
2017-04-20 21:42                           ` Jens Axboe
2017-04-20 21:45                           ` Stephen  Bates
2017-04-20 21:45                             ` Stephen  Bates
2017-04-20 22:05                           ` Stephen  Bates
2017-04-20 22:05                             ` Stephen  Bates
2017-04-20 22:06                             ` Jens Axboe
2017-04-20 22:06                               ` Jens Axboe

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ca5bec05-ba3c-55db-6f3d-ef0965a79737@kernel.dk \
    --to=axboe@kernel.dk \
    --cc=Damien.LeMoal@wdc.com \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-nvme@lists.infradead.org \
    --cc=osandov@osandov.com \
    --cc=sagi@grimberg.me \
    --cc=sbates@raithlin.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.