| Message ID | 20180802102326.6859-1-ming.lei@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | blk-mq: fix updating tags depth | expand |
Tested by: Marco Patalano <mpatalan@redhat.com> On Thu, Aug 2, 2018 at 6:23 AM, Ming Lei <ming.lei@redhat.com> wrote: > The passed 'nr' from userspace represents the total depth, meantime > inside 'struct blk_mq_tags', 'nr_tags' stores the total tag depth, > and 'nr_reserved_tags' stores the reserved part. > > There are two issues in blk_mq_tag_update_depth() now: > > 1) for growing tags, we should have used the passed 'nr', and keep the > number of reserved tags not changed. > > 2) the passed 'nr' should have been used for checking against > 'tags->nr_tags', instead of number of the normal part. > > This patch fixes the above two cases, and avoids kernel crash caused > by wrong resizing sbitmap queue. > > Cc: Marco Patalano <mpatalan@redhat.com> > Cc: "Ewan D. Milne" <emilne@redhat.com> > Cc: Christoph Hellwig <hch@lst.de> > Cc: Bart Van Assche <bart.vanassche@sandisk.com> > Cc: Omar Sandoval <osandov@fb.com> > Signed-off-by: Ming Lei <ming.lei@redhat.com> > --- > block/blk-mq-tag.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/block/blk-mq-tag.c b/block/blk-mq-tag.c > index 09b2ee6694fb..c43b3398d7b4 100644 > --- a/block/blk-mq-tag.c > +++ b/block/blk-mq-tag.c > @@ -399,8 +399,6 @@ int blk_mq_tag_update_depth(struct blk_mq_hw_ctx *hctx, > if (tdepth <= tags->nr_reserved_tags) > return -EINVAL; > > - tdepth -= tags->nr_reserved_tags; > - > /* > * If we are allowed to grow beyond the original size, allocate > * a new set of tags before freeing the old one. > @@ -420,7 +418,8 @@ int blk_mq_tag_update_depth(struct blk_mq_hw_ctx *hctx, > if (tdepth > 16 * BLKDEV_MAX_RQ) > return -EINVAL; > > - new = blk_mq_alloc_rq_map(set, hctx->queue_num, tdepth, 0); > + new = blk_mq_alloc_rq_map(set, hctx->queue_num, tdepth, > + tags->nr_reserved_tags); > if (!new) > return -ENOMEM; > ret = blk_mq_alloc_rqs(set, new, hctx->queue_num, tdepth); > @@ -437,7 +436,8 @@ int blk_mq_tag_update_depth(struct blk_mq_hw_ctx *hctx, > * Don't need (or can't) update reserved tags here, they > * remain static and should never need resizing. > */ > - sbitmap_queue_resize(&tags->bitmap_tags, tdepth); > + sbitmap_queue_resize(&tags->bitmap_tags, > + tdepth - tags->nr_reserved_tags); > } > > return 0; > -- > 2.9.5 > > <div dir="ltr"><span style="font-size:12.8px;text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">Tested by: Marco Patalano <</span><a href="mailto:mpatalan@redhat.com" target="_blank" style="color:rgb(17,85,204);font-size:12.8px">mpatalan@redhat.com</a><span style="font-size:12.8px;text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">></span><br></div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Aug 2, 2018 at 6:23 AM, Ming Lei <span dir="ltr"><<a href="mailto:ming.lei@redhat.com" target="_blank">ming.lei@redhat.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">The passed 'nr' from userspace represents the total depth, meantime<br> inside 'struct blk_mq_tags', 'nr_tags' stores the total tag depth,<br> and 'nr_reserved_tags' stores the reserved part.<br> <br> There are two issues in blk_mq_tag_update_depth() now:<br> <br> 1) for growing tags, we should have used the passed 'nr', and keep the<br> number of reserved tags not changed.<br> <br> 2) the passed 'nr' should have been used for checking against<br> 'tags->nr_tags', instead of number of the normal part.<br> <br> This patch fixes the above two cases, and avoids kernel crash caused<br> by wrong resizing sbitmap queue.<br> <br> Cc: Marco Patalano <<a href="mailto:mpatalan@redhat.com">mpatalan@redhat.com</a>><br> Cc: "Ewan D. Milne" <<a href="mailto:emilne@redhat.com">emilne@redhat.com</a>><br> Cc: Christoph Hellwig <<a href="mailto:hch@lst.de">hch@lst.de</a>><br> Cc: Bart Van Assche <<a href="mailto:bart.vanassche@sandisk.com">bart.vanassche@sandisk.com</a>><br> Cc: Omar Sandoval <<a href="mailto:osandov@fb.com">osandov@fb.com</a>><br> Signed-off-by: Ming Lei <<a href="mailto:ming.lei@redhat.com">ming.lei@redhat.com</a>><br> ---<br> block/blk-mq-tag.c | 8 ++++----<br> 1 file changed, 4 insertions(+), 4 deletions(-)<br> <br> diff --git a/block/blk-mq-tag.c b/block/blk-mq-tag.c<br> index 09b2ee6694fb..c43b3398d7b4 100644<br> --- a/block/blk-mq-tag.c<br> +++ b/block/blk-mq-tag.c<br> @@ -399,8 +399,6 @@ int blk_mq_tag_update_depth(struct blk_mq_hw_ctx *hctx,<br> if (tdepth <= tags->nr_reserved_tags)<br> return -EINVAL;<br> <br> - tdepth -= tags->nr_reserved_tags;<br> -<br> /*<br> * If we are allowed to grow beyond the original size, allocate<br> * a new set of tags before freeing the old one.<br> @@ -420,7 +418,8 @@ int blk_mq_tag_update_depth(struct blk_mq_hw_ctx *hctx,<br> if (tdepth > 16 * BLKDEV_MAX_RQ)<br> return -EINVAL;<br> <br> - new = blk_mq_alloc_rq_map(set, hctx->queue_num, tdepth, 0);<br> + new = blk_mq_alloc_rq_map(set, hctx->queue_num, tdepth,<br> + tags->nr_reserved_tags);<br> if (!new)<br> return -ENOMEM;<br> ret = blk_mq_alloc_rqs(set, new, hctx->queue_num, tdepth);<br> @@ -437,7 +436,8 @@ int blk_mq_tag_update_depth(struct blk_mq_hw_ctx *hctx,<br> * Don't need (or can't) update reserved tags here, they<br> * remain static and should never need resizing.<br> */<br> - sbitmap_queue_resize(&tags-><wbr>bitmap_tags, tdepth);<br> + sbitmap_queue_resize(&tags-><wbr>bitmap_tags,<br> + tdepth - tags->nr_reserved_tags);<br> }<br> <br> return 0;<br> <span class="HOEnZb"><font color="#888888">-- <br> 2.9.5<br> <br> </font></span></blockquote></div><br></div>
On 8/2/18 4:23 AM, Ming Lei wrote: > The passed 'nr' from userspace represents the total depth, meantime > inside 'struct blk_mq_tags', 'nr_tags' stores the total tag depth, > and 'nr_reserved_tags' stores the reserved part. > > There are two issues in blk_mq_tag_update_depth() now: > > 1) for growing tags, we should have used the passed 'nr', and keep the > number of reserved tags not changed. > > 2) the passed 'nr' should have been used for checking against > 'tags->nr_tags', instead of number of the normal part. > > This patch fixes the above two cases, and avoids kernel crash caused > by wrong resizing sbitmap queue. Applied, thanks.
diff --git a/block/blk-mq-tag.c b/block/blk-mq-tag.c index 09b2ee6694fb..c43b3398d7b4 100644 --- a/block/blk-mq-tag.c +++ b/block/blk-mq-tag.c @@ -399,8 +399,6 @@ int blk_mq_tag_update_depth(struct blk_mq_hw_ctx *hctx, if (tdepth <= tags->nr_reserved_tags) return -EINVAL; - tdepth -= tags->nr_reserved_tags; - /* * If we are allowed to grow beyond the original size, allocate * a new set of tags before freeing the old one. @@ -420,7 +418,8 @@ int blk_mq_tag_update_depth(struct blk_mq_hw_ctx *hctx, if (tdepth > 16 * BLKDEV_MAX_RQ) return -EINVAL; - new = blk_mq_alloc_rq_map(set, hctx->queue_num, tdepth, 0); + new = blk_mq_alloc_rq_map(set, hctx->queue_num, tdepth, + tags->nr_reserved_tags); if (!new) return -ENOMEM; ret = blk_mq_alloc_rqs(set, new, hctx->queue_num, tdepth); @@ -437,7 +436,8 @@ int blk_mq_tag_update_depth(struct blk_mq_hw_ctx *hctx, * Don't need (or can't) update reserved tags here, they * remain static and should never need resizing. */ - sbitmap_queue_resize(&tags->bitmap_tags, tdepth); + sbitmap_queue_resize(&tags->bitmap_tags, + tdepth - tags->nr_reserved_tags); } return 0;
The passed 'nr' from userspace represents the total depth, meantime inside 'struct blk_mq_tags', 'nr_tags' stores the total tag depth, and 'nr_reserved_tags' stores the reserved part. There are two issues in blk_mq_tag_update_depth() now: 1) for growing tags, we should have used the passed 'nr', and keep the number of reserved tags not changed. 2) the passed 'nr' should have been used for checking against 'tags->nr_tags', instead of number of the normal part. This patch fixes the above two cases, and avoids kernel crash caused by wrong resizing sbitmap queue. Cc: Marco Patalano <mpatalan@redhat.com> Cc: "Ewan D. Milne" <emilne@redhat.com> Cc: Christoph Hellwig <hch@lst.de> Cc: Bart Van Assche <bart.vanassche@sandisk.com> Cc: Omar Sandoval <osandov@fb.com> Signed-off-by: Ming Lei <ming.lei@redhat.com> --- block/blk-mq-tag.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)