Skip to contentSkip to conversation

fix xenbus_transaction_start() hang caused by double xenbus_transaction_end()

PATCH In review Laszlo Ersek 4 participants 5 messages May 12, 2011 – May 19, 2011 Xen hypervisor interface

lore
Share & tools
lore

Laszlo Ersek

author
#0 lore raw

PATCH 1/1 fix xenbus_transaction_start() hang caused by double xenbus_transaction_end()

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 <[email protected]>
---
 drivers/xen/blkback/vbd.c |    1 +
 1 file changed, 1 insertion(+)
drivers/xen/blkback/vbd.c +1 -0
@@ -156,6 +156,7 @@
 		goto again;
 	if (err)
 		printk(KERN_WARNING "Error ending transaction");
+	return;
 abort:
 	xenbus_transaction_end(xbt, 1);
 }
Acked-by on v1 1/1
#1 lore raw
Quoted 7 lines
Quoted 1 line
Quoted 1 line · Laszlo Ersek

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.

Quoted 5 lines

See RHBZ#618317.

Thanks for considering.

Signed-off-by Laszlo Ersek <[email protected]>
Acked-by Jan Beulich <[email protected]>

(I wonder how this ever passed any testing.)

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

Jan

Quoted 21 lines
---
 drivers/xen/blkback/vbd.c |    1 +
 1 file changed, 1 insertion(+)
drivers/xen/blkback/vbd.c +1 -0
@@ -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
[email protected]
http://lists.xensource.com/xen-devel

Acked-by
#2 lore raw
Quoted 7 lines · Jan Beulich
Quoted 7 lines
Quoted 1 line
Quoted 1 line · Laszlo Ersek

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.

Quoted 7 lines
Quoted 5 lines

See RHBZ#618317.

Thanks for considering.

Signed-off-by Laszlo Ersek <[email protected]>
Acked-by Jan Beulich <[email protected]>
Acked-by Ian Campbell <[email protected]>
Quoted 1 line

(I wonder how this ever passed any testing.)

Indeed.

Quoted 1 line

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

Agreed, maintainers CCd.

Ian.

Quoted 32 lines

Jan

Quoted 17 lines
---
 drivers/xen/blkback/vbd.c |    1 +
 1 file changed, 1 insertion(+)
drivers/xen/blkback/vbd.c +1 -0
@@ -156,6 +156,7 @@
 		goto again;
 	if (err)
 		printk(KERN_WARNING "Error ending transaction");
+	return;
 abort:
 	xenbus_transaction_end(xbt, 1);
 }
Quoted 4 lines

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

#3 lore raw
Quoted 5 lines
Quoted 3 lines
Quoted 2 lines

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

#4 lore raw
In reply to Jan Beulich
Quoted 10 lines
Quoted 10 lines
Quoted 2 lines · Jan Beulich
Quoted 1 line · Laszlo Ersek

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.

Quoted 7 lines
Quoted 3 lines

Thanks for considering.

Signed-off-by Laszlo Ersek <[email protected]>
Acked-by Jan Beulich <[email protected]>

(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

Quoted 33 lines

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

Jan

Quoted 17 lines
---
 drivers/xen/blkback/vbd.c |    1 +
 1 file changed, 1 insertion(+)
drivers/xen/blkback/vbd.c +1 -0
@@ -156,6 +156,7 @@
 		goto again;
 	if (err)
 		printk(KERN_WARNING "Error ending transaction");
+	return;
 abort:
 	xenbus_transaction_end(xbt, 1);
 }
Quoted 4 lines

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel