Re: [Xen-devel] [PATCH] Fix blkback/blktap sysfs read bug.

["Jan Beulich" <JBeulich@xxxxxxxxxx>]

>>> "Joe Jin" <joe.jin@xxxxxxxxxx> 20.01.10 03:06 >>>
>sysfs did not provide lock to handle this, not sure if developer
>think it is not necessary or they'd like to caller to handled it.

A lock is probably not the usual way to deal with this; ref-counting
would seem more common. Nevertheless I think adding a lock will
take care of the issue here.

>--- a/drivers/xen/blktap/xenbus.c      Fri Jan 08 13:07:17 2010 +0000
>+++ b/drivers/xen/blktap/xenbus.c      Wed Jan 20 10:00:53 2010 +0800
>...
>@@ -122,10 +123,15 @@
>                                  struct device_attribute *attr,       \
>                                  char *buf)                           \
>       {                                                               \
>+              ssize_t ret = -ENODEV;                                  \
>               struct xenbus_device *dev = to_xenbus_device(_dev);     \

The use of to_xenbus_device() here makes ...

>-              struct backend_info *be = dev->dev.driver_data;         \
>+              struct backend_info *be;                                \
>                                                                       \
>-              return sprintf(buf, format, ##args);                    \
>+              read_lock(&sysfs_read_lock);                            \
>+              if (dev && (be = dev->dev.driver_data) && be->blkif)    \

... the checking of dev here useless (and the blkback part of the patch
doesn't do the same).

>+                      ret = sprintf(buf, format, ##args);             \
>+              read_unlock(&sysfs_read_lock);                          \
>+              return ret;                                             \
>       }                                                               \
>       static DEVICE_ATTR(name, S_IRUGO, show_##name, NULL)
> 

And btw., in both cases with the lock added there's no need to check
both 'be' and 'be->blkif', since be->blkif can't be NULL when be is
non-NULL.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel