2009-03-24 07:17:45

by Hannes Reinecke

Subject: [PATCH] block: fix intermittent dm timeout based oops

Very rarely under stress testing of dm, oopses are occuring as
something tampers with an old stack frame. This has been traced back
to blk_abort_queue() leaving a timeout_list pointing to the stack.
The reason is that sometimes blk_abort_request() won't delete the
timer (if the request is marked as complete but before the timer has
been removed, a small race window). Fix this by splicing back from
the ususally empty list to the q->timeout_list.

Signed-off-by: Hannes Reinecke <[email protected]>
---
block/blk-timeout.c | 6 ++++++
1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/block/blk-timeout.c b/block/blk-timeout.c
index bbbdc4b..6213123 100644
--- a/block/blk-timeout.c
+++ b/block/blk-timeout.c
@@ -224,6 +224,12 @@ void blk_abort_queue(struct request_queue *q)
list_for_each_entry_safe(rq, tmp, &list, timeout_list)
blk_abort_request(rq);

+ /*
+ * Occasionally, blk_abort_request() will return without
+ * deleting the element from the list
+ */
+ list_splice(&list, &q->timeout_list);
+
spin_unlock_irqrestore(q->queue_lock, flags);

}
--
1.5.3.2

2009-04-03 14:33:34

by Christof Schmitt

Subject: Re: [PATCH] block: fix intermittent dm timeout based oops

On Tue, Mar 24, 2009 at 08:17:30AM +0100, Hannes Reinecke wrote:
> Very rarely under stress testing of dm, oopses are occuring as
> something tampers with an old stack frame. This has been traced back
> to blk_abort_queue() leaving a timeout_list pointing to the stack.
> The reason is that sometimes blk_abort_request() won't delete the
> timer (if the request is marked as complete but before the timer has
> been removed, a small race window). Fix this by splicing back from
> the ususally empty list to the q->timeout_list.
>
> Signed-off-by: Hannes Reinecke <[email protected]>
> ---
> block/blk-timeout.c | 6 ++++++
> 1 files changed, 6 insertions(+), 0 deletions(-)
>
> diff --git a/block/blk-timeout.c b/block/blk-timeout.c
> index bbbdc4b..6213123 100644
> --- a/block/blk-timeout.c
> +++ b/block/blk-timeout.c
> @@ -224,6 +224,12 @@ void blk_abort_queue(struct request_queue *q)
> list_for_each_entry_safe(rq, tmp, &list, timeout_list)
> blk_abort_request(rq);
>
> + /*
> + * Occasionally, blk_abort_request() will return without
> + * deleting the element from the list
> + */
> + list_splice(&list, &q->timeout_list);
> +
> spin_unlock_irqrestore(q->queue_lock, flags);
>
> }
> --
> 1.5.3.2

I just noticed that this fix is not upstream yet and i have seen test
cases hitting this problem.

Jens, are you going to included this patch, or should this go through
the SCSI tree?

--
Christof Schmitt

2009-04-03 14:35:59

by James Bottomley

Subject: Re: [PATCH] block: fix intermittent dm timeout based oops

On Fri, 2009-04-03 at 16:32 +0200, Christof Schmitt wrote:
> On Tue, Mar 24, 2009 at 08:17:30AM +0100, Hannes Reinecke wrote:
> > Very rarely under stress testing of dm, oopses are occuring as
> > something tampers with an old stack frame. This has been traced back
> > to blk_abort_queue() leaving a timeout_list pointing to the stack.
> > The reason is that sometimes blk_abort_request() won't delete the
> > timer (if the request is marked as complete but before the timer has
> > been removed, a small race window). Fix this by splicing back from
> > the ususally empty list to the q->timeout_list.
> >
> > Signed-off-by: Hannes Reinecke <[email protected]>
> > ---
> > block/blk-timeout.c | 6 ++++++
> > 1 files changed, 6 insertions(+), 0 deletions(-)
> >
> > diff --git a/block/blk-timeout.c b/block/blk-timeout.c
> > index bbbdc4b..6213123 100644
> > --- a/block/blk-timeout.c
> > +++ b/block/blk-timeout.c
> > @@ -224,6 +224,12 @@ void blk_abort_queue(struct request_queue *q)
> > list_for_each_entry_safe(rq, tmp, &list, timeout_list)
> > blk_abort_request(rq);
> >
> > + /*
> > + * Occasionally, blk_abort_request() will return without
> > + * deleting the element from the list
> > + */
> > + list_splice(&list, &q->timeout_list);
> > +
> > spin_unlock_irqrestore(q->queue_lock, flags);
> >
> > }
> > --
> > 1.5.3.2
>
> I just noticed that this fix is not upstream yet and i have seen test
> cases hitting this problem.
>
> Jens, are you going to included this patch, or should this go through
> the SCSI tree?

It's a block patch, so it goes through the block tree ... it also needs
backporting to stable.

James

2009-04-03 18:01:50

by Jens Axboe

Subject: Re: [PATCH] block: fix intermittent dm timeout based oops

On Fri, Apr 03 2009, Christof Schmitt wrote:
> On Tue, Mar 24, 2009 at 08:17:30AM +0100, Hannes Reinecke wrote:
> > Very rarely under stress testing of dm, oopses are occuring as
> > something tampers with an old stack frame. This has been traced back
> > to blk_abort_queue() leaving a timeout_list pointing to the stack.
> > The reason is that sometimes blk_abort_request() won't delete the
> > timer (if the request is marked as complete but before the timer has
> > been removed, a small race window). Fix this by splicing back from
> > the ususally empty list to the q->timeout_list.
> >
> > Signed-off-by: Hannes Reinecke <[email protected]>
> > ---
> > block/blk-timeout.c | 6 ++++++
> > 1 files changed, 6 insertions(+), 0 deletions(-)
> >
> > diff --git a/block/blk-timeout.c b/block/blk-timeout.c
> > index bbbdc4b..6213123 100644
> > --- a/block/blk-timeout.c
> > +++ b/block/blk-timeout.c
> > @@ -224,6 +224,12 @@ void blk_abort_queue(struct request_queue *q)
> > list_for_each_entry_safe(rq, tmp, &list, timeout_list)
> > blk_abort_request(rq);
> >
> > + /*
> > + * Occasionally, blk_abort_request() will return without
> > + * deleting the element from the list
> > + */
> > + list_splice(&list, &q->timeout_list);
> > +
> > spin_unlock_irqrestore(q->queue_lock, flags);
> >
> > }
> > --
> > 1.5.3.2
>
> I just noticed that this fix is not upstream yet and i have seen test
> cases hitting this problem.
>
> Jens, are you going to included this patch, or should this go through
> the SCSI tree?

I will include it, and CC stable as well.

--
Jens Axboe

2009-04-23 08:21:51

by Christof Schmitt

Subject: Re: [PATCH] block: fix intermittent dm timeout based oops

On Fri, Apr 03, 2009 at 08:01:06PM +0200, Jens Axboe wrote:
> On Fri, Apr 03 2009, Christof Schmitt wrote:
> > On Tue, Mar 24, 2009 at 08:17:30AM +0100, Hannes Reinecke wrote:
> > > Very rarely under stress testing of dm, oopses are occuring as
> > > something tampers with an old stack frame. This has been traced back
> > > to blk_abort_queue() leaving a timeout_list pointing to the stack.
> > > The reason is that sometimes blk_abort_request() won't delete the
> > > timer (if the request is marked as complete but before the timer has
> > > been removed, a small race window). Fix this by splicing back from
> > > the ususally empty list to the q->timeout_list.
> > >
> > > Signed-off-by: Hannes Reinecke <[email protected]>
> > > ---
> > > block/blk-timeout.c | 6 ++++++
> > > 1 files changed, 6 insertions(+), 0 deletions(-)
> > >
> > > diff --git a/block/blk-timeout.c b/block/blk-timeout.c
> > > index bbbdc4b..6213123 100644
> > > --- a/block/blk-timeout.c
> > > +++ b/block/blk-timeout.c
> > > @@ -224,6 +224,12 @@ void blk_abort_queue(struct request_queue *q)
> > > list_for_each_entry_safe(rq, tmp, &list, timeout_list)
> > > blk_abort_request(rq);
> > >
> > > + /*
> > > + * Occasionally, blk_abort_request() will return without
> > > + * deleting the element from the list
> > > + */
> > > + list_splice(&list, &q->timeout_list);
> > > +
> > > spin_unlock_irqrestore(q->queue_lock, flags);
> > >
> > > }
> > > --
> > > 1.5.3.2
> >
> > I just noticed that this fix is not upstream yet and i have seen test
> > cases hitting this problem.
> >
> > Jens, are you going to included this patch, or should this go through
> > the SCSI tree?
>
> I will include it, and CC stable as well.

Any update on this? 2.6.30-rc3 does not have the patch.

--
Christof Schmitt

2009-04-23 08:31:39

by Jens Axboe

Subject: Re: [PATCH] block: fix intermittent dm timeout based oops

On Thu, Apr 23 2009, Christof Schmitt wrote:
> On Fri, Apr 03, 2009 at 08:01:06PM +0200, Jens Axboe wrote:
> > On Fri, Apr 03 2009, Christof Schmitt wrote:
> > > On Tue, Mar 24, 2009 at 08:17:30AM +0100, Hannes Reinecke wrote:
> > > > Very rarely under stress testing of dm, oopses are occuring as
> > > > something tampers with an old stack frame. This has been traced back
> > > > to blk_abort_queue() leaving a timeout_list pointing to the stack.
> > > > The reason is that sometimes blk_abort_request() won't delete the
> > > > timer (if the request is marked as complete but before the timer has
> > > > been removed, a small race window). Fix this by splicing back from
> > > > the ususally empty list to the q->timeout_list.
> > > >
> > > > Signed-off-by: Hannes Reinecke <[email protected]>
> > > > ---
> > > > block/blk-timeout.c | 6 ++++++
> > > > 1 files changed, 6 insertions(+), 0 deletions(-)
> > > >
> > > > diff --git a/block/blk-timeout.c b/block/blk-timeout.c
> > > > index bbbdc4b..6213123 100644
> > > > --- a/block/blk-timeout.c
> > > > +++ b/block/blk-timeout.c
> > > > @@ -224,6 +224,12 @@ void blk_abort_queue(struct request_queue *q)
> > > > list_for_each_entry_safe(rq, tmp, &list, timeout_list)
> > > > blk_abort_request(rq);
> > > >
> > > > + /*
> > > > + * Occasionally, blk_abort_request() will return without
> > > > + * deleting the element from the list
> > > > + */
> > > > + list_splice(&list, &q->timeout_list);
> > > > +
> > > > spin_unlock_irqrestore(q->queue_lock, flags);
> > > >
> > > > }
> > > > --
> > > > 1.5.3.2
> > >
> > > I just noticed that this fix is not upstream yet and i have seen test
> > > cases hitting this problem.
> > >
> > > Jens, are you going to included this patch, or should this go through
> > > the SCSI tree?
> >
> > I will include it, and CC stable as well.
>
> Any update on this? 2.6.30-rc3 does not have the patch.

I'll be sure to include it today, I need to fix one more thing before
sending a new pull request.

--
Jens Axboe