All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH linux-2.6.18-xen] fix xenbus_transaction_start() hang caused by double xenbus_transaction_end()
@ 2011-05-12 10:24 Laszlo Ersek
  2011-05-13  8:15 ` Jan Beulich
  0 siblings, 1 reply; 5+ messages in thread
From: Laszlo Ersek @ 2011-05-12 10:24 UTC (permalink / raw)
  To: xen-devel; +Cc: Ky Srinivasan

fix xenbus_transaction_start() hang caused by double xenbus_transaction_end()

vbd_resize() up_read()'s xs_state.suspend_mutex twice in a row via double
xenbus_transaction_end() calls. The next down_read() in
xenbus_transaction_start() (at eg. the next resize attempt) hangs.

See RHBZ#618317.

Thanks for considering.

Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 drivers/xen/blkback/vbd.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/xen/blkback/vbd.c b/drivers/xen/blkback/vbd.c
--- a/drivers/xen/blkback/vbd.c
+++ b/drivers/xen/blkback/vbd.c
@@ -156,6 +156,7 @@
 		goto again;
 	if (err)
 		printk(KERN_WARNING "Error ending transaction");
+	return;
 abort:
 	xenbus_transaction_end(xbt, 1);
 }

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH linux-2.6.18-xen] fix xenbus_transaction_start() hang caused by double xenbus_transaction_end()
  2011-05-12 10:24 [PATCH linux-2.6.18-xen] fix xenbus_transaction_start() hang caused by double xenbus_transaction_end() Laszlo Ersek
@ 2011-05-13  8:15 ` Jan Beulich
  2011-05-13  8:41   ` Ian Campbell
  2011-05-19  9:49   ` Jan Beulich
  0 siblings, 2 replies; 5+ messages in thread
From: Jan Beulich @ 2011-05-13  8:15 UTC (permalink / raw)
  To: Laszlo Ersek; +Cc: xen-devel

>>> On 12.05.11 at 12:24, Laszlo Ersek <lersek@redhat.com> wrote:
> fix xenbus_transaction_start() hang caused by double xenbus_transaction_end()
> 
> vbd_resize() up_read()'s xs_state.suspend_mutex twice in a row via double
> xenbus_transaction_end() calls. The next down_read() in
> xenbus_transaction_start() (at eg. the next resize attempt) hangs.
> 
> See RHBZ#618317.
> 
> Thanks for considering.
> 
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>

Acked-by: Jan Beulich <jbeulich@novell.com>

(I wonder how this ever passed any testing.)

The same would be needed for pv-ops too afaics.

Jan

> ---
>  drivers/xen/blkback/vbd.c |    1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/xen/blkback/vbd.c b/drivers/xen/blkback/vbd.c
> --- a/drivers/xen/blkback/vbd.c
> +++ b/drivers/xen/blkback/vbd.c
> @@ -156,6 +156,7 @@
>  		goto again;
>  	if (err)
>  		printk(KERN_WARNING "Error ending transaction");
> +	return;
>  abort:
>  	xenbus_transaction_end(xbt, 1);
>  }
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com 
> http://lists.xensource.com/xen-devel 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH linux-2.6.18-xen] fix xenbus_transaction_start() hang caused by double xenbus_transaction_end()
  2011-05-13  8:15 ` Jan Beulich
@ 2011-05-13  8:41   ` Ian Campbell
  2011-05-13 13:49     ` Konrad Rzeszutek Wilk
  2011-05-19  9:49   ` Jan Beulich
  1 sibling, 1 reply; 5+ messages in thread
From: Ian Campbell @ 2011-05-13  8:41 UTC (permalink / raw)
  To: Jan Beulich
  Cc: Jeremy Fitzhardinge, xen-devel, Laszlo Ersek, Konrad Rzeszutek Wilk

On Fri, 2011-05-13 at 09:15 +0100, Jan Beulich wrote:
> >>> On 12.05.11 at 12:24, Laszlo Ersek <lersek@redhat.com> wrote:
> > fix xenbus_transaction_start() hang caused by double xenbus_transaction_end()
> > 
> > vbd_resize() up_read()'s xs_state.suspend_mutex twice in a row via double
> > xenbus_transaction_end() calls. The next down_read() in
> > xenbus_transaction_start() (at eg. the next resize attempt) hangs.
> > 
> > See RHBZ#618317.
> > 
> > Thanks for considering.
> > 
> > Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> 
> Acked-by: Jan Beulich <jbeulich@novell.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>

> (I wonder how this ever passed any testing.)

Indeed.

> The same would be needed for pv-ops too afaics.

Agreed, maintainers CCd.

Ian.

> 
> Jan
> 
> > ---
> >  drivers/xen/blkback/vbd.c |    1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/drivers/xen/blkback/vbd.c b/drivers/xen/blkback/vbd.c
> > --- a/drivers/xen/blkback/vbd.c
> > +++ b/drivers/xen/blkback/vbd.c
> > @@ -156,6 +156,7 @@
> >  		goto again;
> >  	if (err)
> >  		printk(KERN_WARNING "Error ending transaction");
> > +	return;
> >  abort:
> >  	xenbus_transaction_end(xbt, 1);
> >  }
> > 
> > 
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@lists.xensource.com 
> > http://lists.xensource.com/xen-devel 
> 
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH linux-2.6.18-xen] fix xenbus_transaction_start() hang caused by double xenbus_transaction_end()
  2011-05-13  8:41   ` Ian Campbell
@ 2011-05-13 13:49     ` Konrad Rzeszutek Wilk
  0 siblings, 0 replies; 5+ messages in thread
From: Konrad Rzeszutek Wilk @ 2011-05-13 13:49 UTC (permalink / raw)
  To: Ian Campbell; +Cc: Jeremy Fitzhardinge, xen-devel, Laszlo Ersek, Jan Beulich

> > > fix xenbus_transaction_start() hang caused by double xenbus_transaction_end()
> > > 
> > The same would be needed for pv-ops too afaics.
> 
> Agreed, maintainers CCd.

In for-jens/xen-blkback-v3.3

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH linux-2.6.18-xen] fix xenbus_transaction_start() hang caused by double xenbus_transaction_end()
  2011-05-13  8:15 ` Jan Beulich
  2011-05-13  8:41   ` Ian Campbell
@ 2011-05-19  9:49   ` Jan Beulich
  1 sibling, 0 replies; 5+ messages in thread
From: Jan Beulich @ 2011-05-19  9:49 UTC (permalink / raw)
  To: Laszlo Ersek; +Cc: xen-devel

>>> On 13.05.11 at 10:15, "Jan Beulich" <JBeulich@novell.com> wrote:
>>>> On 12.05.11 at 12:24, Laszlo Ersek <lersek@redhat.com> wrote:
>> fix xenbus_transaction_start() hang caused by double xenbus_transaction_end()
>> 
>> vbd_resize() up_read()'s xs_state.suspend_mutex twice in a row via double
>> xenbus_transaction_end() calls. The next down_read() in
>> xenbus_transaction_start() (at eg. the next resize attempt) hangs.
>> 
>> See RHBZ#618317.
>> 
>> Thanks for considering.
>> 
>> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> 
> Acked-by: Jan Beulich <jbeulich@novell.com>
> 
> (I wonder how this ever passed any testing.)

Hmm, I think if this really wasn't *that* bad. In particular I'm finding
that neither pv-ops nor the legacy tree has anything called
suspend_mutex in drivers/xen/xenbus/xenbus_xs.c, nor is there any
other mutex being up_read()-ed in transaction_end().

What got corrupted by the double call was transaction_count, which
would have had a negative effect at the next transaction_suspend()
(but not at any subsequent transaction_start()).

Jan

> The same would be needed for pv-ops too afaics.
> 
> Jan
> 
>> ---
>>  drivers/xen/blkback/vbd.c |    1 +
>>  1 file changed, 1 insertion(+)
>> 
>> diff --git a/drivers/xen/blkback/vbd.c b/drivers/xen/blkback/vbd.c
>> --- a/drivers/xen/blkback/vbd.c
>> +++ b/drivers/xen/blkback/vbd.c
>> @@ -156,6 +156,7 @@
>>  		goto again;
>>  	if (err)
>>  		printk(KERN_WARNING "Error ending transaction");
>> +	return;
>>  abort:
>>  	xenbus_transaction_end(xbt, 1);
>>  }
>> 
>> 
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xensource.com 
>> http://lists.xensource.com/xen-devel 
> 
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com 
> http://lists.xensource.com/xen-devel 

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2011-05-19  9:49 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-05-12 10:24 [PATCH linux-2.6.18-xen] fix xenbus_transaction_start() hang caused by double xenbus_transaction_end() Laszlo Ersek
2011-05-13  8:15 ` Jan Beulich
2011-05-13  8:41   ` Ian Campbell
2011-05-13 13:49     ` Konrad Rzeszutek Wilk
2011-05-19  9:49   ` Jan Beulich

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.