* [PATCH] block: fully initialize queue in blk_mq_init_allocated_queue
@ 2015-03-13 9:30 Sergey Senozhatsky
2015-03-13 12:24 ` Mike Snitzer
0 siblings, 1 reply; 4+ messages in thread
From: Sergey Senozhatsky @ 2015-03-13 9:30 UTC (permalink / raw)
To: Mike Snitzer, Ming Lei
Cc: Jens Axboe, linux-kernel, Sergey Senozhatsky, Sergey Senozhatsky
Commit c9e8c91f8a279b87eb0d94b037504ea9fc1bef7c
Author: Mike Snitzer <snitzer@redhat.com>
Date: Tue Mar 10 15:54:41 2015 -0400
blk-mq: fix use of incorrect goto label in blk_mq_init_queue error path
for some reason has moved queue allocation 'q = blk_alloc_queue_node()'
after 'percpu_ref_init(&q->mq_usage_counter...)', so we are doing percpu
init on something that is not a request_queue.
Further commit 716452cd27b145d611e4399e7cc35df6c943686e
Author: Mike Snitzer <snitzer@redhat.com>
Date: Tue Mar 10 17:20:20 2015 -0400
blk-mq: add blk_mq_init_allocated_queue and export
has introduced abother issue.
In blk_mq_init_queue() we allocate new request_queue:
uninit_q = blk_alloc_queue_node(GFP_KERNEL, set->numa_node);
if (!uninit_q)
return ERR_PTR(-ENOMEM);
and pass `uninit_q' as a 'request_queue *q' parameter to
blk_mq_init_allocated_queue():
q = blk_mq_init_allocated_queue(set, uninit_q);
blk_mq_init_allocated_queue(), however, firstly use passed `uninit_q' to init
its percpu, but then it allocates a new request_queue and returns it back,
not being properly initialized:
blk_mq_init_allocated_queue(struct blk_mq_tag_set *set, struct request_queue *q)
[..]
if (percpu_ref_init(&q->mq_usage_counter, blk_mq_usage_counter_release,
PERCPU_REF_INIT_ATOMIC, GFP_KERNEL))
goto err_hctxs;
q = blk_alloc_queue_node(GFP_KERNEL, set->numa_node);
if (!q)
goto err_hctxs;
[..]
return q;
Which eventually leads to different problems, including a NULL pointer
dereference.
Remove blk_alloc_queue_node() from blk_mq_init_allocated_queue() and use
supplied request_queue.
Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
---
block/blk-mq.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/block/blk-mq.c b/block/blk-mq.c
index b838dfc..59fa239 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -1955,10 +1955,6 @@ struct request_queue *blk_mq_init_allocated_queue(struct blk_mq_tag_set *set,
PERCPU_REF_INIT_ATOMIC, GFP_KERNEL))
goto err_hctxs;
- q = blk_alloc_queue_node(GFP_KERNEL, set->numa_node);
- if (!q)
- goto err_hctxs;
-
setup_timer(&q->timeout, blk_mq_rq_timer, (unsigned long) q);
blk_queue_rq_timeout(q, 30000);
--
2.3.2.223.g7a9409c
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: block: fully initialize queue in blk_mq_init_allocated_queue
2015-03-13 9:30 [PATCH] block: fully initialize queue in blk_mq_init_allocated_queue Sergey Senozhatsky
@ 2015-03-13 12:24 ` Mike Snitzer
2015-03-13 12:37 ` Sergey Senozhatsky
0 siblings, 1 reply; 4+ messages in thread
From: Mike Snitzer @ 2015-03-13 12:24 UTC (permalink / raw)
To: Sergey Senozhatsky; +Cc: Ming Lei, Jens Axboe, linux-kernel
On Fri, Mar 13 2015 at 5:30am -0400,
Sergey Senozhatsky <sergey.senozhatsky@gmail.com> wrote:
> Commit c9e8c91f8a279b87eb0d94b037504ea9fc1bef7c
> Author: Mike Snitzer <snitzer@redhat.com>
> Date: Tue Mar 10 15:54:41 2015 -0400
>
> blk-mq: fix use of incorrect goto label in blk_mq_init_queue error path
>
> for some reason has moved queue allocation 'q = blk_alloc_queue_node()'
> after 'percpu_ref_init(&q->mq_usage_counter...)', so we are doing percpu
> init on something that is not a request_queue.
>
> Further commit 716452cd27b145d611e4399e7cc35df6c943686e
> Author: Mike Snitzer <snitzer@redhat.com>
> Date: Tue Mar 10 17:20:20 2015 -0400
>
> blk-mq: add blk_mq_init_allocated_queue and export
>
> has introduced abother issue.
> In blk_mq_init_queue() we allocate new request_queue:
> ...
Thanks, but I already fixed these 2 patches and pushed them to
linux-dm.git's for-next branch last night (I also posted v3 of the
corresponding patches to LKML at that time).
Sorry to waste your time, I've learned my lesson:
Don't validate a tree works and then decide to hurridly rebase before
pushing to linux-next, to get a fix before a feature, without re-testing
the result (prior to rebase, patches 1 and 2 were inverted and I didn't
have the extra blk_mq_init_queue -- that snuck in with the rebase).
Hopefully linux-next was able to pick up my new 'for-next' and we don't
have an entire weekend of linux-next crashes due to my idiocy.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: block: fully initialize queue in blk_mq_init_allocated_queue
2015-03-13 12:24 ` Mike Snitzer
@ 2015-03-13 12:37 ` Sergey Senozhatsky
2015-03-13 14:17 ` next-20150313's block is broken, but fix available [was: Re: block: fully initialize queue in blk_mq_init_allocated_queue] Mike Snitzer
0 siblings, 1 reply; 4+ messages in thread
From: Sergey Senozhatsky @ 2015-03-13 12:37 UTC (permalink / raw)
To: Mike Snitzer; +Cc: Sergey Senozhatsky, Ming Lei, Jens Axboe, linux-kernel
On (03/13/15 08:24), Mike Snitzer wrote:
> Thanks, but I already fixed these 2 patches and pushed them to
> linux-dm.git's for-next branch last night (I also posted v3 of the
> corresponding patches to LKML at that time).
oh, I see. didn't know that.
> Sorry to waste your time, I've learned my lesson:
>
no problem.
> Hopefully linux-next was able to pick up my new 'for-next' and we don't
> have an entire weekend of linux-next crashes due to my idiocy.
nope, this is how I spot it -- next-20150313 doesn't boot.
thanks,
-ss
^ permalink raw reply [flat|nested] 4+ messages in thread
* next-20150313's block is broken, but fix available [was: Re: block: fully initialize queue in blk_mq_init_allocated_queue]
2015-03-13 12:37 ` Sergey Senozhatsky
@ 2015-03-13 14:17 ` Mike Snitzer
0 siblings, 0 replies; 4+ messages in thread
From: Mike Snitzer @ 2015-03-13 14:17 UTC (permalink / raw)
To: sfr, linux-next; +Cc: Ming Lei, Jens Axboe, linux-kernel, Sergey Senozhatsky
On Fri, Mar 13 2015 at 8:37am -0400,
Sergey Senozhatsky <sergey.senozhatsky@gmail.com> wrote:
> On (03/13/15 08:24), Mike Snitzer wrote:
> > Thanks, but I already fixed these 2 patches and pushed them to
> > linux-dm.git's for-next branch last night (I also posted v3 of the
> > corresponding patches to LKML at that time).
>
> oh, I see. didn't know that.
>
> > Sorry to waste your time, I've learned my lesson:
> >
>
> no problem.
>
> > Hopefully linux-next was able to pick up my new 'for-next' and we don't
> > have an entire weekend of linux-next crashes due to my idiocy.
>
> nope, this is how I spot it -- next-20150313 doesn't boot.
Hi Stephen,
Is there any opportunity to get the tree fixed by re-pulling
linux-dm.git's 'for-next'? I'd hate to kill linux-next productivity
this weekend! Sorry about this.
Please advise, thanks.
Mike
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-03-13 14:17 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-13 9:30 [PATCH] block: fully initialize queue in blk_mq_init_allocated_queue Sergey Senozhatsky
2015-03-13 12:24 ` Mike Snitzer
2015-03-13 12:37 ` Sergey Senozhatsky
2015-03-13 14:17 ` next-20150313's block is broken, but fix available [was: Re: block: fully initialize queue in blk_mq_init_allocated_queue] Mike Snitzer
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.