diff mbox series

[SRU,Artful] blk-mq-tag: check for NULL rq when iterating tags

Message ID 20180119124312.16507-1-gpiccoli@canonical.com
State New
Headers show
Series [SRU,Artful] blk-mq-tag: check for NULL rq when iterating tags | expand

Commit Message

Guilherme G. Piccoli Jan. 19, 2018, 12:43 p.m. UTC
From: Jens Axboe <axboe@kernel.dk>

BugLink: https://bugs.launchpad.net/bugs/1744300

Since we introduced blk-mq-sched, the tags->rqs[] array has been
dynamically assigned. So we need to check for NULL when iterating,
since there's a window of time where the bit is set, but we haven't
dynamically assigned the tags->rqs[] array position yet.

This is perfectly safe, since the memory backing of the request is
never going away while the device is alive.

Reviewed-by: Bart Van Assche <bart.vanassche@wdc.com>
Reviewed-by: Omar Sandoval <osandov@fb.com>
Signed-off-by: Jens Axboe <axboe@kernel.dk>
(cherry-pick from commit 7f5562d5ecc44c757599b201df928ba52fa05047)
Signed-off-by: Guilherme G. Piccoli <gpiccoli@canonical.com>
---
 block/blk-mq-tag.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

Comments

Khalid Elmously Jan. 30, 2018, 7:39 a.m. UTC | #1
On 2018-01-19 10:43:12 , Guilherme G. Piccoli wrote:
> From: Jens Axboe <axboe@kernel.dk>
> 
> BugLink: https://bugs.launchpad.net/bugs/1744300
> 
> Since we introduced blk-mq-sched, the tags->rqs[] array has been
> dynamically assigned. So we need to check for NULL when iterating,
> since there's a window of time where the bit is set, but we haven't
> dynamically assigned the tags->rqs[] array position yet.
> 
> This is perfectly safe, since the memory backing of the request is
> never going away while the device is alive.
> 
> Reviewed-by: Bart Van Assche <bart.vanassche@wdc.com>
> Reviewed-by: Omar Sandoval <osandov@fb.com>
> Signed-off-by: Jens Axboe <axboe@kernel.dk>
> (cherry-pick from commit 7f5562d5ecc44c757599b201df928ba52fa05047)
> Signed-off-by: Guilherme G. Piccoli <gpiccoli@canonical.com>
> ---
>  block/blk-mq-tag.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/block/blk-mq-tag.c b/block/blk-mq-tag.c
> index d0be72ccb091..dc9e6dac5a2a 100644
> --- a/block/blk-mq-tag.c
> +++ b/block/blk-mq-tag.c
> @@ -214,7 +214,11 @@ static bool bt_iter(struct sbitmap *bitmap, unsigned int bitnr, void *data)
>  		bitnr += tags->nr_reserved_tags;
>  	rq = tags->rqs[bitnr];
>  
> -	if (rq->q == hctx->queue)
> +	/*
> +	 * We can hit rq == NULL here, because the tagging functions
> +	 * test and set the bit before assining ->rqs[].
> +	 */
> +	if (rq && rq->q == hctx->queue)
>  		iter_data->fn(hctx, rq, iter_data->data, reserved);
>  	return true;
>  }
> @@ -248,9 +252,15 @@ static bool bt_tags_iter(struct sbitmap *bitmap, unsigned int bitnr, void *data)
>  
>  	if (!reserved)
>  		bitnr += tags->nr_reserved_tags;
> +
> +	/*
> +	 * We can hit rq == NULL here, because the tagging functions
> +	 * test and set the bit before assining ->rqs[].
> +	 */
>  	rq = tags->rqs[bitnr];
> +	if (rq)
> +		iter_data->fn(rq, iter_data->data, reserved);
>  
> -	iter_data->fn(rq, iter_data->data, reserved);
>  	return true;
>  }
>

Acked-by: Khalid Elmously <khalid.elmously@canonical.com>
Kamal Mostafa Jan. 31, 2018, 7 p.m. UTC | #2
Acked-by: Kamal Mostafa <kamal@canonical.com>
Khalid Elmously Feb. 3, 2018, 1:52 a.m. UTC | #3
Applied to artful

On 2018-01-19 10:43:12 , Guilherme G. Piccoli wrote:
> From: Jens Axboe <axboe@kernel.dk>
> 
> BugLink: https://bugs.launchpad.net/bugs/1744300
> 
> Since we introduced blk-mq-sched, the tags->rqs[] array has been
> dynamically assigned. So we need to check for NULL when iterating,
> since there's a window of time where the bit is set, but we haven't
> dynamically assigned the tags->rqs[] array position yet.
> 
> This is perfectly safe, since the memory backing of the request is
> never going away while the device is alive.
> 
> Reviewed-by: Bart Van Assche <bart.vanassche@wdc.com>
> Reviewed-by: Omar Sandoval <osandov@fb.com>
> Signed-off-by: Jens Axboe <axboe@kernel.dk>
> (cherry-pick from commit 7f5562d5ecc44c757599b201df928ba52fa05047)
> Signed-off-by: Guilherme G. Piccoli <gpiccoli@canonical.com>
> ---
>  block/blk-mq-tag.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/block/blk-mq-tag.c b/block/blk-mq-tag.c
> index d0be72ccb091..dc9e6dac5a2a 100644
> --- a/block/blk-mq-tag.c
> +++ b/block/blk-mq-tag.c
> @@ -214,7 +214,11 @@ static bool bt_iter(struct sbitmap *bitmap, unsigned int bitnr, void *data)
>  		bitnr += tags->nr_reserved_tags;
>  	rq = tags->rqs[bitnr];
>  
> -	if (rq->q == hctx->queue)
> +	/*
> +	 * We can hit rq == NULL here, because the tagging functions
> +	 * test and set the bit before assining ->rqs[].
> +	 */
> +	if (rq && rq->q == hctx->queue)
>  		iter_data->fn(hctx, rq, iter_data->data, reserved);
>  	return true;
>  }
> @@ -248,9 +252,15 @@ static bool bt_tags_iter(struct sbitmap *bitmap, unsigned int bitnr, void *data)
>  
>  	if (!reserved)
>  		bitnr += tags->nr_reserved_tags;
> +
> +	/*
> +	 * We can hit rq == NULL here, because the tagging functions
> +	 * test and set the bit before assining ->rqs[].
> +	 */
>  	rq = tags->rqs[bitnr];
> +	if (rq)
> +		iter_data->fn(rq, iter_data->data, reserved);
>  
> -	iter_data->fn(rq, iter_data->data, reserved);
>  	return true;
>  }
>  
> -- 
> 2.15.1
> 
> 
> -- 
> kernel-team mailing list
> kernel-team@lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team
diff mbox series

Patch

diff --git a/block/blk-mq-tag.c b/block/blk-mq-tag.c
index d0be72ccb091..dc9e6dac5a2a 100644
--- a/block/blk-mq-tag.c
+++ b/block/blk-mq-tag.c
@@ -214,7 +214,11 @@  static bool bt_iter(struct sbitmap *bitmap, unsigned int bitnr, void *data)
 		bitnr += tags->nr_reserved_tags;
 	rq = tags->rqs[bitnr];
 
-	if (rq->q == hctx->queue)
+	/*
+	 * We can hit rq == NULL here, because the tagging functions
+	 * test and set the bit before assining ->rqs[].
+	 */
+	if (rq && rq->q == hctx->queue)
 		iter_data->fn(hctx, rq, iter_data->data, reserved);
 	return true;
 }
@@ -248,9 +252,15 @@  static bool bt_tags_iter(struct sbitmap *bitmap, unsigned int bitnr, void *data)
 
 	if (!reserved)
 		bitnr += tags->nr_reserved_tags;
+
+	/*
+	 * We can hit rq == NULL here, because the tagging functions
+	 * test and set the bit before assining ->rqs[].
+	 */
 	rq = tags->rqs[bitnr];
+	if (rq)
+		iter_data->fn(rq, iter_data->data, reserved);
 
-	iter_data->fn(rq, iter_data->data, reserved);
 	return true;
 }