[PATCH RESENT] dm: Check kthread_run's return value

[PATCH RESENT] dm: Check kthread_run's return value

[Minfei Huang]

kthread function is used to process kthread_work. And there is no return
value checking during create this thread. Add this checking to fix this
issue.

Signed-off-by: Minfei Huang <mnghuan@gmail.com>
---
 drivers/md/dm.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index 1b2f962..d68b9d2 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -2654,12 +2654,15 @@ struct queue_limits *dm_get_queue_limits(struct mapped_device *md)
 }
 EXPORT_SYMBOL_GPL(dm_get_queue_limits);
 
-static void dm_old_init_rq_based_worker_thread(struct mapped_device *md)
+static int dm_old_init_rq_based_worker_thread(struct mapped_device *md)
 {
 	/* Initialize the request-based DM worker thread */
 	init_kthread_worker(&md->kworker);
 	md->kworker_task = kthread_run(kthread_worker_fn, &md->kworker,
 				       "kdmwork-%s", dm_device_name(md));
+	if (IS_ERR(md->kworker_task))
+		return -ENOMEM;
+	return 0;
 }
 
 /*
@@ -2667,6 +2670,8 @@ static void dm_old_init_rq_based_worker_thread(struct mapped_device *md)
  */
 static int dm_old_init_request_queue(struct mapped_device *md)
 {
+	int ret;
+
 	/* Fully initialize the queue */
 	if (!blk_init_allocated_queue(md->queue, dm_request_fn, NULL))
 		return -EINVAL;
@@ -2678,7 +2683,9 @@ static int dm_old_init_request_queue(struct mapped_device *md)
 	blk_queue_softirq_done(md->queue, dm_softirq_done);
 	blk_queue_prep_rq(md->queue, dm_old_prep_fn);
 
-	dm_old_init_rq_based_worker_thread(md);
+	ret = dm_old_init_rq_based_worker_thread(md);
+	if (ret < 0)
+		return ret;
 
 	elv_register_queue(md->queue);
 
-- 
2.6.3

Re: [PATCH RESENT] dm: Check kthread_run's return value

[Mike Snitzer]

On Mon, Jul 04 2016 at 11:25am -0400,
Minfei Huang <mnghuan@gmail.com> wrote:

> kthread function is used to process kthread_work. And there is no return
> value checking during create this thread. Add this checking to fix this
> issue.
> 
> Signed-off-by: Minfei Huang <mnghuan@gmail.com>
> ---
>  drivers/md/dm.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/md/dm.c b/drivers/md/dm.c
> index 1b2f962..d68b9d2 100644
> --- a/drivers/md/dm.c
> +++ b/drivers/md/dm.c
> @@ -2654,12 +2654,15 @@ struct queue_limits *dm_get_queue_limits(struct mapped_device *md)
>  }
>  EXPORT_SYMBOL_GPL(dm_get_queue_limits);
>  
> -static void dm_old_init_rq_based_worker_thread(struct mapped_device *md)
> +static int dm_old_init_rq_based_worker_thread(struct mapped_device *md)
>  {
>  	/* Initialize the request-based DM worker thread */
>  	init_kthread_worker(&md->kworker);
>  	md->kworker_task = kthread_run(kthread_worker_fn, &md->kworker,
>  				       "kdmwork-%s", dm_device_name(md));
> +	if (IS_ERR(md->kworker_task))
> +		return -ENOMEM;
> +	return 0;
>  }
>  
>  /*
> @@ -2667,6 +2670,8 @@ static void dm_old_init_rq_based_worker_thread(struct mapped_device *md)
>   */
>  static int dm_old_init_request_queue(struct mapped_device *md)
>  {
> +	int ret;
> +
>  	/* Fully initialize the queue */
>  	if (!blk_init_allocated_queue(md->queue, dm_request_fn, NULL))
>  		return -EINVAL;
> @@ -2678,7 +2683,9 @@ static int dm_old_init_request_queue(struct mapped_device *md)
>  	blk_queue_softirq_done(md->queue, dm_softirq_done);
>  	blk_queue_prep_rq(md->queue, dm_old_prep_fn);
>  
> -	dm_old_init_rq_based_worker_thread(md);
> +	ret = dm_old_init_rq_based_worker_thread(md);
> +	if (ret < 0)
> +		return ret;
>  
>  	elv_register_queue(md->queue);
>  

This needed rebasing against linux-dm.git's 'for-next'.  I've now staged
this fix for 4.8 inclusion, see:
https://git.kernel.org/cgit/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=for-next&id=7193a9defcab6f3d3f1eb64c68bad7534e5a39ad

Re: [PATCH RESENT] dm: Check kthread_run's return value

[Minfei Huang]

On 07/06/16 at 09:16P, Mike Snitzer wrote:
> On Mon, Jul 04 2016 at 11:25am -0400,
> Minfei Huang <mnghuan@gmail.com> wrote:
> 
> > kthread function is used to process kthread_work. And there is no return
> > value checking during create this thread. Add this checking to fix this
> > issue.
> > 
> > Signed-off-by: Minfei Huang <mnghuan@gmail.com>
> This needed rebasing against linux-dm.git's 'for-next'.  I've now staged
> this fix for 4.8 inclusion, see:
> https://git.kernel.org/cgit/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=for-next&id=7193a9defcab6f3d3f1eb64c68bad7534e5a39ad

Seems that we should fix it in stable as well.

Thanks
Minfei

Re: [PATCH RESENT] dm: Check kthread_run's return value

[Mike Snitzer]

On Wed, Jul 06 2016 at  9:27am -0400,
Minfei Huang <mnghuan@gmail.com> wrote:

> On 07/06/16 at 09:16P, Mike Snitzer wrote:
> > On Mon, Jul 04 2016 at 11:25am -0400,
> > Minfei Huang <mnghuan@gmail.com> wrote:
> > 
> > > kthread function is used to process kthread_work. And there is no return
> > > value checking during create this thread. Add this checking to fix this
> > > issue.
> > > 
> > > Signed-off-by: Minfei Huang <mnghuan@gmail.com>
> > This needed rebasing against linux-dm.git's 'for-next'.  I've now staged
> > this fix for 4.8 inclusion, see:
> > https://git.kernel.org/cgit/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=for-next&id=7193a9defcab6f3d3f1eb64c68bad7534e5a39ad
> 
> Seems that we should fix it in stable as well.

Given the code movement it isn't easy to do (by simply adding a stable@
cc).  I've not seen a single report of an ignored kthread_run() failure
for multipath using .request_fn interface.

Re: [PATCH RESENT] dm: Check kthread_run's return value

[Minfei Huang]

On 07/06/16 at 09:31P, Mike Snitzer wrote:
> On Wed, Jul 06 2016 at  9:27am -0400,
> Minfei Huang <mnghuan@gmail.com> wrote:
> 
> > On 07/06/16 at 09:16P, Mike Snitzer wrote:
> > > On Mon, Jul 04 2016 at 11:25am -0400,
> > > Minfei Huang <mnghuan@gmail.com> wrote:
> > > 
> > > > kthread function is used to process kthread_work. And there is no return
> > > > value checking during create this thread. Add this checking to fix this
> > > > issue.
> > > > 
> > > > Signed-off-by: Minfei Huang <mnghuan@gmail.com>
> > > This needed rebasing against linux-dm.git's 'for-next'.  I've now staged
> > > this fix for 4.8 inclusion, see:
> > > https://git.kernel.org/cgit/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=for-next&id=7193a9defcab6f3d3f1eb64c68bad7534e5a39ad
> > 
> > Seems that we should fix it in stable as well.
> 
> Given the code movement it isn't easy to do (by simply adding a stable@
> cc).  I've not seen a single report of an ignored kthread_run() failure
> for multipath using .request_fn interface.

Yep, this bug will be trigged in very restrict condition, also dm is
installed in boot time when there are a lot of memory avaiable.

Thanks
Minfei