Matt Wilson
2012-Apr-20 19:21 UTC
[RFC] [PATCH] add locking in statistics sysfs show functions
The blkback code in the 2.6.18 tree has a patch to avoid a race condition in the statistics sysfs routines. Is it no longer needed? The following patch ports the change to 3.x. Matt Wilson (1): xen/blkback: add locking in statistics sysfs show functions drivers/block/xen-blkback/xenbus.c | 20 +++++++++++++++++--- 1 files changed, 17 insertions(+), 3 deletions(-) -- 1.7.2.5
Matt Wilson
2012-Apr-20 19:21 UTC
[RFC] [PATCH] xen/blkback: add locking in statistics sysfs show functions
This is a port of a patch in the XenoLinux 2.6.18 tree:
http://xenbits.xen.org/hg/linux-2.6.18-xen.hg/rev/f47c07325a56
Fix blkback sysfs race
Read blkback statistics info after remove vbd device(s) kernel
will crash.
Signed-off-by: Joe Jin <joe.jin@oracle.com>
Acked-by: Jan Beulich <jbeulich@novell.com>
[Ported to Linux 3.x]
Signed-off-by: Matt Wilson <msw@amazon.com>
---
drivers/block/xen-blkback/xenbus.c | 20 +++++++++++++++++---
1 files changed, 17 insertions(+), 3 deletions(-)
diff --git a/drivers/block/xen-blkback/xenbus.c
b/drivers/block/xen-blkback/xenbus.c
index da19506..74d4810 100644
--- a/drivers/block/xen-blkback/xenbus.c
+++ b/drivers/block/xen-blkback/xenbus.c
@@ -21,6 +21,8 @@
#include <xen/grant_table.h>
#include "common.h"
+static DEFINE_RWLOCK(sysfs_read_lock);
+
struct backend_info {
struct xenbus_device *dev;
struct xen_blkif *blkif;
@@ -225,10 +227,20 @@ int __init xen_blkif_interface_init(void)
struct device_attribute *attr, \
char *buf) \
{ \
- struct xenbus_device *dev = to_xenbus_device(_dev); \
- struct backend_info *be = dev_get_drvdata(&dev->dev); \
+ ssize_t ret = -ENODEV; \
+ struct xenbus_device *dev; \
+ struct backend_info *be; \
\
- return sprintf(buf, format, ##args); \
+ if (!get_device(_dev)) \
+ return ret; \
+ dev = to_xenbus_device(_dev); \
+ read_lock(&sysfs_read_lock); \
+ be = (struct backend_info *) dev_get_drvdata(&dev->dev);\
+ if (be != NULL) \
+ ret = sprintf(buf, format, ##args); \
+ read_unlock(&sysfs_read_lock); \
+ put_device(_dev); \
+ return ret; \
} \
static DEVICE_ATTR(name, S_IRUGO, show_##name, NULL)
@@ -353,6 +365,7 @@ static int xen_blkbk_remove(struct xenbus_device *dev)
DPRINTK("");
+ write_lock(&sysfs_read_lock);
if (be->major || be->minor)
xenvbd_sysfs_delif(dev);
@@ -371,6 +384,7 @@ static int xen_blkbk_remove(struct xenbus_device *dev)
kfree(be);
dev_set_drvdata(&dev->dev, NULL);
+ write_unlock(&sysfs_read_lock);
return 0;
}
--
1.7.2.5
Konrad Rzeszutek Wilk
2012-Apr-26 20:51 UTC
Re: [RFC] [PATCH] xen/blkback: add locking in statistics sysfs show functions
On Fri, Apr 20, 2012 at 07:21:13PM +0000, Matt Wilson wrote:> This is a port of a patch in the XenoLinux 2.6.18 tree: > http://xenbits.xen.org/hg/linux-2.6.18-xen.hg/rev/f47c07325a56 > > Fix blkback sysfs race > > Read blkback statistics info after remove vbd device(s) kernel > will crash.Looks pretty reasonable.> > Signed-off-by: Joe Jin <joe.jin@oracle.com> > Acked-by: Jan Beulich <jbeulich@novell.com> > [Ported to Linux 3.x] > Signed-off-by: Matt Wilson <msw@amazon.com> > --- > drivers/block/xen-blkback/xenbus.c | 20 +++++++++++++++++--- > 1 files changed, 17 insertions(+), 3 deletions(-) > > diff --git a/drivers/block/xen-blkback/xenbus.c b/drivers/block/xen-blkback/xenbus.c > index da19506..74d4810 100644 > --- a/drivers/block/xen-blkback/xenbus.c > +++ b/drivers/block/xen-blkback/xenbus.c > @@ -21,6 +21,8 @@ > #include <xen/grant_table.h> > #include "common.h" > > +static DEFINE_RWLOCK(sysfs_read_lock); > + > struct backend_info { > struct xenbus_device *dev; > struct xen_blkif *blkif; > @@ -225,10 +227,20 @@ int __init xen_blkif_interface_init(void) > struct device_attribute *attr, \ > char *buf) \ > { \ > - struct xenbus_device *dev = to_xenbus_device(_dev); \ > - struct backend_info *be = dev_get_drvdata(&dev->dev); \ > + ssize_t ret = -ENODEV; \ > + struct xenbus_device *dev; \ > + struct backend_info *be; \ > \ > - return sprintf(buf, format, ##args); \ > + if (!get_device(_dev)) \ > + return ret; \ > + dev = to_xenbus_device(_dev); \ > + read_lock(&sysfs_read_lock); \ > + be = (struct backend_info *) dev_get_drvdata(&dev->dev);\ > + if (be != NULL) \ > + ret = sprintf(buf, format, ##args); \ > + read_unlock(&sysfs_read_lock); \ > + put_device(_dev); \ > + return ret; \ > } \ > static DEVICE_ATTR(name, S_IRUGO, show_##name, NULL) > > @@ -353,6 +365,7 @@ static int xen_blkbk_remove(struct xenbus_device *dev) > > DPRINTK(""); > > + write_lock(&sysfs_read_lock); > if (be->major || be->minor) > xenvbd_sysfs_delif(dev); > > @@ -371,6 +384,7 @@ static int xen_blkbk_remove(struct xenbus_device *dev) > > kfree(be); > dev_set_drvdata(&dev->dev, NULL); > + write_unlock(&sysfs_read_lock); > return 0; > } > > -- > 1.7.2.5
Konrad Rzeszutek Wilk
2012-Apr-27 02:24 UTC
Re: [RFC] [PATCH] xen/blkback: add locking in statistics sysfs show functions
On Fri, Apr 20, 2012 at 07:21:13PM +0000, Matt Wilson wrote:> This is a port of a patch in the XenoLinux 2.6.18 tree: > http://xenbits.xen.org/hg/linux-2.6.18-xen.hg/rev/f47c07325a56 > > Fix blkback sysfs raceSo with this patch I keep on getting this: [ 128.274290] switch: port 2(vif1.0) entered disabled state [ 128.855442] BUG: sleeping function called from invalid context at /home/konrad/ssd/linux/kernel/mutex.c:85 [ 128.855628] in_atomic(): 1, irqs_disabled(): 0, pid: 46, name: xenwatch [ 128.855757] Pid: 46, comm: xenwatch Tainted: G O 3.4.0-rc4upstream-00103-g7e4325c-dirty #1 [ 128.855917] Call Trace: [ 128.856062] [<ffffffff810983ea>] __might_sleep+0xda/0x100 [ 128.856215] [<ffffffff815a9267>] mutex_lock+0x27/0x50 [ 128.856351] [<ffffffff811b3039>] sysfs_get_dirent+0x29/0x80 [ 128.856495] [<ffffffff811b4d7b>] sysfs_remove_group+0x2b/0x100 [ 128.856626] [<ffffffff81393a20>] xenvbd_sysfs_delif+0x20/0x50 [ 128.856756] [<ffffffff81394518>] xen_blkbk_remove+0xe8/0xf0 [ 128.856895] [<ffffffff8131c768>] xenbus_dev_remove+0x38/0x70 [ 128.857041] [<ffffffff81381601>] __device_release_driver+0x61/0xc0 [ 128.857177] [<ffffffff81381778>] device_release_driver+0x28/0x40 [ 128.857323] [<ffffffff81380896>] bus_remove_device+0x106/0x140 [ 128.857459] [<ffffffff8137e868>] device_del+0x118/0x1c0 [ 128.857580] [<ffffffff8137e921>] device_unregister+0x11/0x20 [ 128.857712] [<ffffffff8139376a>] frontend_changed+0xba/0x350 [ 128.857851] [<ffffffff810337f9>] ? xen_irq_enable_direct_reloc+0x4/0x4 [ 128.857982] [<ffffffff8109928d>] ? finish_task_switch+0x4d/0x100 [ 128.858106] [<ffffffff8131ca18>] xenbus_otherend_changed+0xa8/0xb0 [ 128.858236] [<ffffffff815ab629>] ? _raw_spin_unlock_irqrestore+0x19/0x30 [ 128.858368] [<ffffffff8131cc2b>] frontend_changed+0xb/0x10 [ 128.858522] [<ffffffff8131ad3b>] xenwatch_thread+0xcb/0x190 [ 128.858655] [<ffffffff8108ec40>] ? wake_up_bit+0x40/0x40 [ 128.858801] [<ffffffff8131ac70>] ? xenbus_thread+0x320/0x320 [ 128.858965] [<ffffffff8108e716>] kthread+0x96/0xa0 [ 128.859097] [<ffffffff815b3564>] kernel_thread_helper+0x4/0x10 [ 128.859235] [<ffffffff815abb78>] ? retint_restore_args+0x5/0x6 [ 128.859371] [<ffffffff815b3560>] ? gs_change+0x13/0x13 [ 128.859542] BUG: scheduling while atomic: xenwatch/46/0x00000002 whenever a guest is powered off.
Matt Wilson
2012-Apr-27 16:30 UTC
Re: [RFC] [PATCH] xen/blkback: add locking in statistics sysfs show functions
On Thu, Apr 26, 2012 at 07:24:56PM -0700, Konrad Rzeszutek Wilk wrote:> On Fri, Apr 20, 2012 at 07:21:13PM +0000, Matt Wilson wrote: > > This is a port of a patch in the XenoLinux 2.6.18 tree: > > http://xenbits.xen.org/hg/linux-2.6.18-xen.hg/rev/f47c07325a56 > > > > Fix blkback sysfs race > > So with this patch I keep on getting this: > > [ 128.274290] switch: port 2(vif1.0) entered disabled state...> [ 128.859542] BUG: scheduling while atomic: xenwatch/46/0x00000002 > > whenever a guest is powered off.Oops. I hadn''t had a chance to do testing of the patch, I just noticed that the upstream version didn''t have it. I''ll tweak it a bit, do some testing, and resubmit when it works. -- msw