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

["Jan Beulich" <JBeulich@novell.com>]

>>> "Joe Jin" <joe.jin@oracle.com> 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