[SRU][Xenial][PATCH 0/1] block: Fix a race between blk_cleanup_queue() and timeout handling

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

[SRU][Xenial][PATCH 0/1] block: Fix a race between blk_cleanup_queue() and timeout handling

Joseph Salisbury-3
BugLink: https://bugs.launchpad.net/bugs/1791790

== SRU Justification ==
The following commit was applied to Xenial and introduced this
regression:
287922eb0b18 ("block: defer timeouts to a workqueue")

This regression was introduced in mainline as of v4.5-rc1.  Bionc was
also affected by this regression, but it already go the fix when commit
4e9b6f20828a was applied to mainline in v4.15-rc1.

The regression caused a kernel hang because the HPSA driver has a tendency
to aggressively remove missing devices.

== Fix ==
4e9b6f20828a ("block: Fix a race between blk_cleanup_queue() and timeout handling")

== Regression Potential ==
Low.  This commit fixes a regression and has been cc'd to stable, so it
has had addition upstream review.  This commit is already applied to
Bionic and Cosmic.

== Test Case ==
A test kernel was built with this patch and tested by the original bug reporter.
The bug reporter states the test kernel resolved the bug.

Bart Van Assche (1):
  block: Fix a race between blk_cleanup_queue() and timeout handling

 block/blk-core.c    | 2 ++
 block/blk-timeout.c | 3 ---
 2 files changed, 2 insertions(+), 3 deletions(-)

--
2.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[SRU][Xenial][PATCH 1/1] block: Fix a race between blk_cleanup_queue() and timeout handling

Joseph Salisbury-3
From: Bart Van Assche <[hidden email]>

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

Make sure that if the timeout timer fires after a queue has been
marked "dying" that the affected requests are finished.

Reported-by: chenxiang (M) <[hidden email]>
Fixes: commit 287922eb0b18 ("block: defer timeouts to a workqueue")
Signed-off-by: Bart Van Assche <[hidden email]>
Tested-by: chenxiang (M) <[hidden email]>
Cc: Christoph Hellwig <[hidden email]>
Cc: Keith Busch <[hidden email]>
Cc: Hannes Reinecke <[hidden email]>
Cc: Ming Lei <[hidden email]>
Cc: Johannes Thumshirn <[hidden email]>
Cc: <[hidden email]>
Signed-off-by: Jens Axboe <[hidden email]>
(cherry picked from commit 4e9b6f20828ac880dbc1fa2fdbafae779473d1af)
Signed-off-by: Joseph Salisbury <[hidden email]>
---
 block/blk-core.c    | 2 ++
 block/blk-timeout.c | 3 ---
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index be184a8..13c1e5c 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -266,6 +266,7 @@ EXPORT_SYMBOL(blk_stop_queue);
 void blk_sync_queue(struct request_queue *q)
 {
  del_timer_sync(&q->timeout);
+ cancel_work_sync(&q->timeout_work);
 
  if (q->mq_ops) {
  struct blk_mq_hw_ctx *hctx;
@@ -701,6 +702,7 @@ struct request_queue *blk_alloc_queue_node(gfp_t gfp_mask, int node_id)
  setup_timer(&q->backing_dev_info->laptop_mode_wb_timer,
     laptop_mode_timer_fn, (unsigned long) q);
  setup_timer(&q->timeout, blk_rq_timed_out_timer, (unsigned long) q);
+ INIT_WORK(&q->timeout_work, NULL);
  INIT_LIST_HEAD(&q->queue_head);
  INIT_LIST_HEAD(&q->timeout_list);
  INIT_LIST_HEAD(&q->icq_list);
diff --git a/block/blk-timeout.c b/block/blk-timeout.c
index 7c176f6..aedd128 100644
--- a/block/blk-timeout.c
+++ b/block/blk-timeout.c
@@ -135,8 +135,6 @@ void blk_timeout_work(struct work_struct *work)
  struct request *rq, *tmp;
  int next_set = 0;
 
- if (blk_queue_enter(q, true))
- return;
  spin_lock_irqsave(q->queue_lock, flags);
 
  list_for_each_entry_safe(rq, tmp, &q->timeout_list, timeout_list)
@@ -146,7 +144,6 @@ void blk_timeout_work(struct work_struct *work)
  mod_timer(&q->timeout, round_jiffies_up(next));
 
  spin_unlock_irqrestore(q->queue_lock, flags);
- blk_queue_exit(q);
 }
 
 /**
--
2.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

ACK: [SRU][Xenial][PATCH 1/1] block: Fix a race between blk_cleanup_queue() and timeout handling

Stefan Bader-2
On 20.09.2018 12:01, Joseph Salisbury wrote:

> From: Bart Van Assche <[hidden email]>
>
> BugLink: https://bugs.launchpad.net/bugs/1791790
>
> Make sure that if the timeout timer fires after a queue has been
> marked "dying" that the affected requests are finished.
>
> Reported-by: chenxiang (M) <[hidden email]>
> Fixes: commit 287922eb0b18 ("block: defer timeouts to a workqueue")
> Signed-off-by: Bart Van Assche <[hidden email]>
> Tested-by: chenxiang (M) <[hidden email]>
> Cc: Christoph Hellwig <[hidden email]>
> Cc: Keith Busch <[hidden email]>
> Cc: Hannes Reinecke <[hidden email]>
> Cc: Ming Lei <[hidden email]>
> Cc: Johannes Thumshirn <[hidden email]>
> Cc: <[hidden email]>
> Signed-off-by: Jens Axboe <[hidden email]>
> (cherry picked from commit 4e9b6f20828ac880dbc1fa2fdbafae779473d1af)
> Signed-off-by: Joseph Salisbury <[hidden email]>
Acked-by: Stefan Bader <[hidden email]>

> ---
>  block/blk-core.c    | 2 ++
>  block/blk-timeout.c | 3 ---
>  2 files changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/block/blk-core.c b/block/blk-core.c
> index be184a8..13c1e5c 100644
> --- a/block/blk-core.c
> +++ b/block/blk-core.c
> @@ -266,6 +266,7 @@ EXPORT_SYMBOL(blk_stop_queue);
>  void blk_sync_queue(struct request_queue *q)
>  {
>   del_timer_sync(&q->timeout);
> + cancel_work_sync(&q->timeout_work);
>  
>   if (q->mq_ops) {
>   struct blk_mq_hw_ctx *hctx;
> @@ -701,6 +702,7 @@ struct request_queue *blk_alloc_queue_node(gfp_t gfp_mask, int node_id)
>   setup_timer(&q->backing_dev_info->laptop_mode_wb_timer,
>      laptop_mode_timer_fn, (unsigned long) q);
>   setup_timer(&q->timeout, blk_rq_timed_out_timer, (unsigned long) q);
> + INIT_WORK(&q->timeout_work, NULL);
>   INIT_LIST_HEAD(&q->queue_head);
>   INIT_LIST_HEAD(&q->timeout_list);
>   INIT_LIST_HEAD(&q->icq_list);
> diff --git a/block/blk-timeout.c b/block/blk-timeout.c
> index 7c176f6..aedd128 100644
> --- a/block/blk-timeout.c
> +++ b/block/blk-timeout.c
> @@ -135,8 +135,6 @@ void blk_timeout_work(struct work_struct *work)
>   struct request *rq, *tmp;
>   int next_set = 0;
>  
> - if (blk_queue_enter(q, true))
> - return;
>   spin_lock_irqsave(q->queue_lock, flags);
>  
>   list_for_each_entry_safe(rq, tmp, &q->timeout_list, timeout_list)
> @@ -146,7 +144,6 @@ void blk_timeout_work(struct work_struct *work)
>   mod_timer(&q->timeout, round_jiffies_up(next));
>  
>   spin_unlock_irqrestore(q->queue_lock, flags);
> - blk_queue_exit(q);
>  }
>  
>  /**
>


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

signature.asc (836 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

ACK: [SRU][Xenial][PATCH 1/1] block: Fix a race between blk_cleanup_queue() and timeout handling

Kleber Souza
In reply to this post by Joseph Salisbury-3
On 09/20/18 12:01, Joseph Salisbury wrote:

> From: Bart Van Assche <[hidden email]>
>
> BugLink: https://bugs.launchpad.net/bugs/1791790
>
> Make sure that if the timeout timer fires after a queue has been
> marked "dying" that the affected requests are finished.
>
> Reported-by: chenxiang (M) <[hidden email]>
> Fixes: commit 287922eb0b18 ("block: defer timeouts to a workqueue")
> Signed-off-by: Bart Van Assche <[hidden email]>
> Tested-by: chenxiang (M) <[hidden email]>
> Cc: Christoph Hellwig <[hidden email]>
> Cc: Keith Busch <[hidden email]>
> Cc: Hannes Reinecke <[hidden email]>
> Cc: Ming Lei <[hidden email]>
> Cc: Johannes Thumshirn <[hidden email]>
> Cc: <[hidden email]>
> Signed-off-by: Jens Axboe <[hidden email]>
> (cherry picked from commit 4e9b6f20828ac880dbc1fa2fdbafae779473d1af)
> Signed-off-by: Joseph Salisbury <[hidden email]>

Acked-by: Kleber Sacilotto de Souza <[hidden email]>

> ---
>  block/blk-core.c    | 2 ++
>  block/blk-timeout.c | 3 ---
>  2 files changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/block/blk-core.c b/block/blk-core.c
> index be184a8..13c1e5c 100644
> --- a/block/blk-core.c
> +++ b/block/blk-core.c
> @@ -266,6 +266,7 @@ EXPORT_SYMBOL(blk_stop_queue);
>  void blk_sync_queue(struct request_queue *q)
>  {
>   del_timer_sync(&q->timeout);
> + cancel_work_sync(&q->timeout_work);
>  
>   if (q->mq_ops) {
>   struct blk_mq_hw_ctx *hctx;
> @@ -701,6 +702,7 @@ struct request_queue *blk_alloc_queue_node(gfp_t gfp_mask, int node_id)
>   setup_timer(&q->backing_dev_info->laptop_mode_wb_timer,
>      laptop_mode_timer_fn, (unsigned long) q);
>   setup_timer(&q->timeout, blk_rq_timed_out_timer, (unsigned long) q);
> + INIT_WORK(&q->timeout_work, NULL);
>   INIT_LIST_HEAD(&q->queue_head);
>   INIT_LIST_HEAD(&q->timeout_list);
>   INIT_LIST_HEAD(&q->icq_list);
> diff --git a/block/blk-timeout.c b/block/blk-timeout.c
> index 7c176f6..aedd128 100644
> --- a/block/blk-timeout.c
> +++ b/block/blk-timeout.c
> @@ -135,8 +135,6 @@ void blk_timeout_work(struct work_struct *work)
>   struct request *rq, *tmp;
>   int next_set = 0;
>  
> - if (blk_queue_enter(q, true))
> - return;
>   spin_lock_irqsave(q->queue_lock, flags);
>  
>   list_for_each_entry_safe(rq, tmp, &q->timeout_list, timeout_list)
> @@ -146,7 +144,6 @@ void blk_timeout_work(struct work_struct *work)
>   mod_timer(&q->timeout, round_jiffies_up(next));
>  
>   spin_unlock_irqrestore(q->queue_lock, flags);
> - blk_queue_exit(q);
>  }
>  
>  /**
>


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

APPLIED: [SRU][Xenial][PATCH 0/1] block: Fix a race between blk_cleanup_queue() and timeout handling

Kleber Souza
In reply to this post by Joseph Salisbury-3
On 09/20/18 12:01, Joseph Salisbury wrote:

> BugLink: https://bugs.launchpad.net/bugs/1791790
>
> == SRU Justification ==
> The following commit was applied to Xenial and introduced this
> regression:
> 287922eb0b18 ("block: defer timeouts to a workqueue")
>
> This regression was introduced in mainline as of v4.5-rc1.  Bionc was
> also affected by this regression, but it already go the fix when commit
> 4e9b6f20828a was applied to mainline in v4.15-rc1.
>
> The regression caused a kernel hang because the HPSA driver has a tendency
> to aggressively remove missing devices.
>
> == Fix ==
> 4e9b6f20828a ("block: Fix a race between blk_cleanup_queue() and timeout handling")
>
> == Regression Potential ==
> Low.  This commit fixes a regression and has been cc'd to stable, so it
> has had addition upstream review.  This commit is already applied to
> Bionic and Cosmic.
>
> == Test Case ==
> A test kernel was built with this patch and tested by the original bug reporter.
> The bug reporter states the test kernel resolved the bug.
>
> Bart Van Assche (1):
>   block: Fix a race between blk_cleanup_queue() and timeout handling
>
>  block/blk-core.c    | 2 ++
>  block/blk-timeout.c | 3 ---
>  2 files changed, 2 insertions(+), 3 deletions(-)
>

Applied to xenial/master-next branch.

Thanks,
Kleber

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team