diff mbox

[v3,2/2] blk-mq: Add a polling specific stats function

Message ID ca5bec05-ba3c-55db-6f3d-ef0965a79737@kernel.dk (mailing list archive)
State New, archived
Headers show

Commit Message

Jens Axboe April 20, 2017, 8:53 p.m. UTC
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?

Comments

Stephen Bates April 20, 2017, 9:08 p.m. UTC | #1
> 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?


Yes you can add a Tested-By from me….

Tested-By: Stephen Bates <sbates@raithlin.com>
Jens Axboe April 20, 2017, 9:14 p.m. UTC | #2
On 04/20/2017 03:08 PM, Stephen  Bates wrote:
> 
>> 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?
> 
> Yes you can add a Tested-By from me….
> 
> Tested-By: Stephen Bates <sbates@raithlin.com>

Great thanks, pushed. I'll get this added for 4.11. Thanks for
the report!
Stephen Bates April 20, 2017, 9:41 p.m. UTC | #3
> Great thanks, pushed. I'll get this added for 4.11. Thanks for

> the report!


I see you applied my v3 series to the for-4.12/block branch. There is one issue there I need to fix. Can I send you a v4 a bit later today instead?
-- 
Jens Axboe
Jens Axboe April 20, 2017, 9:42 p.m. UTC | #4
On 04/20/2017 03:41 PM, Stephen  Bates wrote:
> 
>> Great thanks, pushed. I'll get this added for 4.11. Thanks for
>> the report!
> 
> I see you applied my v3 series to the for-4.12/block branch. There is
> one issue there I need to fix. Can I send you a v4 a bit later today
> instead?

You can, but it won't do much good since v3 is already applied. Any
further changes must be incremental.
Stephen Bates April 20, 2017, 9:45 p.m. UTC | #5
> You can, but it won't do much good since v3 is already applied. Any

> further changes must be incremental.


OK, I will send a small patch on top of what you just applied. Thanks!
Stephen Bates April 20, 2017, 10:05 p.m. UTC | #6
> You can, but it won't do much good since v3 is already applied. Any

> further changes must be incremental.


BTW getting a compile error from the Kyber code in for-4.12/block due to the fact we now return a signed from the bucket function…

batesste@ubuntu64-batesste:~/kernel/linux$ make -j 2 all
  CHK     include/config/kernel.release
  CHK     include/generated/uapi/linux/version.h
  CHK     include/generated/utsrelease.h
  CHK     include/generated/timeconst.h
  CHK     include/generated/bounds.h
  CHK     include/generated/asm-offsets.h
  CALL    scripts/checksyscalls.sh
  CHK     include/generated/compile.h
  CC      block/kyber-iosched.o
block/kyber-iosched.c: In function ‘kyber_queue_data_alloc’:
block/kyber-iosched.c:295:57: error: passing argument 2 of ‘blk_stat_alloc_callback’ from incompatible pointer type [-Werror=incompatible-pointer-types]
  kqd->cb = blk_stat_alloc_callback(kyber_stat_timer_fn, rq_sched_domain,
                                                         ^
In file included from block/blk-mq.h:4:0,
                 from block/blk.h:6,
                 from block/kyber-iosched.c:27:
block/blk-stat.h:138:1: note: expected ‘int (*)(const struct request *)’ but argument is of type ‘unsigned int (*)(const struct request *)’
 blk_stat_alloc_callback(void (*timer_fn)(struct blk_stat_callback *),
 ^
  CC      block/compat_ioctl.o
cc1: some warnings being treated as errors
Jens Axboe April 20, 2017, 10:06 p.m. UTC | #7
On 04/20/2017 04:05 PM, Stephen  Bates wrote:
> 
>> You can, but it won't do much good since v3 is already applied. Any
>> further changes must be incremental.
> 
> BTW getting a compile error from the Kyber code in for-4.12/block due to the fact we now return a signed from the bucket function…
> 
> batesste@ubuntu64-batesste:~/kernel/linux$ make -j 2 all
>   CHK     include/config/kernel.release
>   CHK     include/generated/uapi/linux/version.h
>   CHK     include/generated/utsrelease.h
>   CHK     include/generated/timeconst.h
>   CHK     include/generated/bounds.h
>   CHK     include/generated/asm-offsets.h
>   CALL    scripts/checksyscalls.sh
>   CHK     include/generated/compile.h
>   CC      block/kyber-iosched.o
> block/kyber-iosched.c: In function ‘kyber_queue_data_alloc’:
> block/kyber-iosched.c:295:57: error: passing argument 2 of ‘blk_stat_alloc_callback’ from incompatible pointer type [-Werror=incompatible-pointer-types]
>   kqd->cb = blk_stat_alloc_callback(kyber_stat_timer_fn, rq_sched_domain,
>                                                          ^
> In file included from block/blk-mq.h:4:0,
>                  from block/blk.h:6,
>                  from block/kyber-iosched.c:27:
> block/blk-stat.h:138:1: note: expected ‘int (*)(const struct request *)’ but argument is of type ‘unsigned int (*)(const struct request *)’
>  blk_stat_alloc_callback(void (*timer_fn)(struct blk_stat_callback *),
>  ^
>   CC      block/compat_ioctl.o
> cc1: some warnings being treated as errors

I did fix that one up, I did a ninja rebase, but apparently you pulled in the
few minutes it existed. So just update, and you should be fine.
diff mbox

Patch

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);
 }