[PATCH 1/3] virtio: change to_vp_device to an inlined definition

[PATCH 1/3] virtio: change to_vp_device to an inlined definition

[Wanlong Gao]

to_vp_device is worth changing to inlined definition.

Signed-off-by: Wanlong Gao <gaowanlong@cn.fujitsu.com>
---
 drivers/virtio/virtio_pci.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c
index e3ecc94..7681fe3 100644
--- a/drivers/virtio/virtio_pci.c
+++ b/drivers/virtio/virtio_pci.c
@@ -98,11 +98,7 @@ static struct pci_device_id virtio_pci_id_table[] = {
 
 MODULE_DEVICE_TABLE(pci, virtio_pci_id_table);
 
-/* Convert a generic virtio device to our structure */
-static struct virtio_pci_device *to_vp_device(struct virtio_device *vdev)
-{
-	return container_of(vdev, struct virtio_pci_device, vdev);
-}
+#define to_vp_device(_vdev) container_of(_vdev, struct virtio_pci_device, vdev)
 
 /* virtio config->get_features() implementation */
 static u32 vp_get_features(struct virtio_device *vdev)
-- 
1.8.0

[PATCH 2/3] virtio: use dev_to_virtio wrapper in virtio

[Wanlong Gao]

Use dev_to_virtio wrapper in virtio to make code clearly.

Signed-off-by: Wanlong Gao <gaowanlong@cn.fujitsu.com>
---
 drivers/virtio/virtio.c | 19 +++++++++----------
 include/linux/virtio.h  |  2 +-
 2 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
index 1e8659c..1346ae8 100644
--- a/drivers/virtio/virtio.c
+++ b/drivers/virtio/virtio.c
@@ -10,33 +10,32 @@ static DEFINE_IDA(virtio_index_ida);
 static ssize_t device_show(struct device *_d,
 			   struct device_attribute *attr, char *buf)
 {
-	struct virtio_device *dev = container_of(_d,struct virtio_device,dev);
+	struct virtio_device *dev = dev_to_virtio(_d);
 	return sprintf(buf, "0x%04x\n", dev->id.device);
 }
 static ssize_t vendor_show(struct device *_d,
 			   struct device_attribute *attr, char *buf)
 {
-	struct virtio_device *dev = container_of(_d,struct virtio_device,dev);
+	struct virtio_device *dev = dev_to_virtio(_d);
 	return sprintf(buf, "0x%04x\n", dev->id.vendor);
 }
 static ssize_t status_show(struct device *_d,
 			   struct device_attribute *attr, char *buf)
 {
-	struct virtio_device *dev = container_of(_d,struct virtio_device,dev);
+	struct virtio_device *dev = dev_to_virtio(_d);
 	return sprintf(buf, "0x%08x\n", dev->config->get_status(dev));
 }
 static ssize_t modalias_show(struct device *_d,
 			     struct device_attribute *attr, char *buf)
 {
-	struct virtio_device *dev = container_of(_d,struct virtio_device,dev);
-
+	struct virtio_device *dev = dev_to_virtio(_d);
 	return sprintf(buf, "virtio:d%08Xv%08X\n",
 		       dev->id.device, dev->id.vendor);
 }
 static ssize_t features_show(struct device *_d,
 			     struct device_attribute *attr, char *buf)
 {
-	struct virtio_device *dev = container_of(_d, struct virtio_device, dev);
+	struct virtio_device *dev = dev_to_virtio(_d);
 	unsigned int i;
 	ssize_t len = 0;
 
@@ -71,7 +70,7 @@ static inline int virtio_id_match(const struct virtio_device *dev,
 static int virtio_dev_match(struct device *_dv, struct device_driver *_dr)
 {
 	unsigned int i;
-	struct virtio_device *dev = container_of(_dv,struct virtio_device,dev);
+	struct virtio_device *dev = dev_to_virtio(_dv);
 	const struct virtio_device_id *ids;
 
 	ids = container_of(_dr, struct virtio_driver, driver)->id_table;
@@ -83,7 +82,7 @@ static int virtio_dev_match(struct device *_dv, struct device_driver *_dr)
 
 static int virtio_uevent(struct device *_dv, struct kobj_uevent_env *env)
 {
-	struct virtio_device *dev = container_of(_dv,struct virtio_device,dev);
+	struct virtio_device *dev = dev_to_virtio(_dv);
 
 	return add_uevent_var(env, "MODALIAS=virtio:d%08Xv%08X",
 			      dev->id.device, dev->id.vendor);
@@ -111,7 +110,7 @@ EXPORT_SYMBOL_GPL(virtio_check_driver_offered_feature);
 static int virtio_dev_probe(struct device *_d)
 {
 	int err, i;
-	struct virtio_device *dev = container_of(_d,struct virtio_device,dev);
+	struct virtio_device *dev = dev_to_virtio(_d);
 	struct virtio_driver *drv = container_of(dev->dev.driver,
 						 struct virtio_driver, driver);
 	u32 device_features;
@@ -152,7 +151,7 @@ static int virtio_dev_probe(struct device *_d)
 
 static int virtio_dev_remove(struct device *_d)
 {
-	struct virtio_device *dev = container_of(_d,struct virtio_device,dev);
+	struct virtio_device *dev = dev_to_virtio(_d);
 	struct virtio_driver *drv = container_of(dev->dev.driver,
 						 struct virtio_driver, driver);
 
diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index 25fa1a6..30fc3c9 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -79,7 +79,7 @@ struct virtio_device {
 	void *priv;
 };
 
-#define dev_to_virtio(dev) container_of(dev, struct virtio_device, dev)
+#define dev_to_virtio(_dev) container_of(_dev, struct virtio_device, dev)
 int register_virtio_device(struct virtio_device *dev);
 void unregister_virtio_device(struct virtio_device *dev);
 
-- 
1.8.0

Re: [PATCH 2/3] virtio: use dev_to_virtio wrapper in virtio

[Michael S. Tsirkin]

On Wed, Dec 05, 2012 at 03:03:28PM +0800, Wanlong Gao wrote:
> Use dev_to_virtio wrapper in virtio to make code clearly.
> 
> Signed-off-by: Wanlong Gao <gaowanlong@cn.fujitsu.com>
> ---
> diff --git a/include/linux/virtio.h b/include/linux/virtio.h
> index 25fa1a6..30fc3c9 100644
> --- a/include/linux/virtio.h
> +++ b/include/linux/virtio.h
> @@ -79,7 +79,7 @@ struct virtio_device {
>  	void *priv;
>  };
>  
> -#define dev_to_virtio(dev) container_of(dev, struct virtio_device, dev)
> +#define dev_to_virtio(_dev) container_of(_dev, struct virtio_device, dev)
>  int register_virtio_device(struct virtio_device *dev);
>  void unregister_virtio_device(struct virtio_device *dev);


More importantly this would fix dev_to_virtio since ATM

dev_to_virtio(_d) resolves to
 container_of(_d, struct virtio_device, _d)

which is not what was intended.

However, I think this shows that using a macro here
is a mistake. Could you code this up with a static inline
function instead please?

>  
> -- 
> 1.8.0

Re: [PATCH 2/3] virtio: use dev_to_virtio wrapper in virtio

[Michael S. Tsirkin]

On Wed, Dec 05, 2012 at 01:16:06PM +0200, Michael S. Tsirkin wrote:
> On Wed, Dec 05, 2012 at 03:03:28PM +0800, Wanlong Gao wrote:
> > Use dev_to_virtio wrapper in virtio to make code clearly.
> > 
> > Signed-off-by: Wanlong Gao <gaowanlong@cn.fujitsu.com>
> > ---
> > diff --git a/include/linux/virtio.h b/include/linux/virtio.h
> > index 25fa1a6..30fc3c9 100644
> > --- a/include/linux/virtio.h
> > +++ b/include/linux/virtio.h
> > @@ -79,7 +79,7 @@ struct virtio_device {
> >  	void *priv;
> >  };
> >  
> > -#define dev_to_virtio(dev) container_of(dev, struct virtio_device, dev)
> > +#define dev_to_virtio(_dev) container_of(_dev, struct virtio_device, dev)
> >  int register_virtio_device(struct virtio_device *dev);
> >  void unregister_virtio_device(struct virtio_device *dev);
> 
> 
> More importantly this would fix dev_to_virtio since ATM
> 
> dev_to_virtio(_d) resolves to
>  container_of(_d, struct virtio_device, _d)
> 
> which is not what was intended.
> 
> However, I think this shows that using a macro here
> is a mistake. Could you code this up with a static inline
> function instead please?

And probably move it to virtio.c there seems to be no need
to keep it in virtio.h

> >  
> > -- 
> > 1.8.0

Re: [PATCH 2/3] virtio: use dev_to_virtio wrapper in virtio

[Wanlong Gao]

On 12/05/2012 07:16 PM, Michael S. Tsirkin wrote:
> On Wed, Dec 05, 2012 at 03:03:28PM +0800, Wanlong Gao wrote:
>> Use dev_to_virtio wrapper in virtio to make code clearly.
>>
>> Signed-off-by: Wanlong Gao <gaowanlong@cn.fujitsu.com>
>> ---
>> diff --git a/include/linux/virtio.h b/include/linux/virtio.h
>> index 25fa1a6..30fc3c9 100644
>> --- a/include/linux/virtio.h
>> +++ b/include/linux/virtio.h
>> @@ -79,7 +79,7 @@ struct virtio_device {
>>  	void *priv;
>>  };
>>  
>> -#define dev_to_virtio(dev) container_of(dev, struct virtio_device, dev)
>> +#define dev_to_virtio(_dev) container_of(_dev, struct virtio_device, dev)
>>  int register_virtio_device(struct virtio_device *dev);
>>  void unregister_virtio_device(struct virtio_device *dev);
> 
> 
> More importantly this would fix dev_to_virtio since ATM
> 
> dev_to_virtio(_d) resolves to
>  container_of(_d, struct virtio_device, _d)
> 
> which is not what was intended.

Yes, you are right, I not mentioned.

> 
> However, I think this shows that using a macro here
> is a mistake. Could you code this up with a static inline
> function instead please?

Sure.

Thanks,
Wanlong Gao

> 
>>  
>> -- 
>> 1.8.0
> 

[PATCH 3/3] virtio: add drv_to_virtio to make code clearly

[Wanlong Gao]

Add drv_to_virtio wrapper to get virtio_driver from device_driver.

Signed-off-by: Wanlong Gao <gaowanlong@cn.fujitsu.com>
---
 drivers/virtio/virtio.c | 11 ++++-------
 include/linux/virtio.h  |  1 +
 2 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
index 1346ae8..1c01ac3 100644
--- a/drivers/virtio/virtio.c
+++ b/drivers/virtio/virtio.c
@@ -73,7 +73,7 @@ static int virtio_dev_match(struct device *_dv, struct device_driver *_dr)
 	struct virtio_device *dev = dev_to_virtio(_dv);
 	const struct virtio_device_id *ids;
 
-	ids = container_of(_dr, struct virtio_driver, driver)->id_table;
+	ids = drv_to_virtio(_dr)->id_table;
 	for (i = 0; ids[i].device; i++)
 		if (virtio_id_match(dev, &ids[i]))
 			return 1;
@@ -97,8 +97,7 @@ void virtio_check_driver_offered_feature(const struct virtio_device *vdev,
 					 unsigned int fbit)
 {
 	unsigned int i;
-	struct virtio_driver *drv = container_of(vdev->dev.driver,
-						 struct virtio_driver, driver);
+	struct virtio_driver *drv = drv_to_virtio(vdev->dev.driver);
 
 	for (i = 0; i < drv->feature_table_size; i++)
 		if (drv->feature_table[i] == fbit)
@@ -111,8 +110,7 @@ static int virtio_dev_probe(struct device *_d)
 {
 	int err, i;
 	struct virtio_device *dev = dev_to_virtio(_d);
-	struct virtio_driver *drv = container_of(dev->dev.driver,
-						 struct virtio_driver, driver);
+	struct virtio_driver *drv = drv_to_virtio(dev->dev.driver);
 	u32 device_features;
 
 	/* We have a driver! */
@@ -152,8 +150,7 @@ static int virtio_dev_probe(struct device *_d)
 static int virtio_dev_remove(struct device *_d)
 {
 	struct virtio_device *dev = dev_to_virtio(_d);
-	struct virtio_driver *drv = container_of(dev->dev.driver,
-						 struct virtio_driver, driver);
+	struct virtio_driver *drv = drv_to_virtio(dev->dev.driver);
 
 	drv->remove(dev);
 
diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index 30fc3c9..8da5811 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -109,6 +109,7 @@ struct virtio_driver {
 #endif
 };
 
+#define drv_to_virtio(drv) container_of(drv, struct virtio_driver, driver)
 int register_virtio_driver(struct virtio_driver *drv);
 void unregister_virtio_driver(struct virtio_driver *drv);
 #endif /* _LINUX_VIRTIO_H */
-- 
1.8.0

Re: [PATCH 3/3] virtio: add drv_to_virtio to make code clearly

[Michael S. Tsirkin]

On Wed, Dec 05, 2012 at 03:03:29PM +0800, Wanlong Gao wrote:
> Add drv_to_virtio wrapper to get virtio_driver from device_driver.
> 
> Signed-off-by: Wanlong Gao <gaowanlong@cn.fujitsu.com>

IMHO a function would be slightly better.

> ---
>  drivers/virtio/virtio.c | 11 ++++-------
>  include/linux/virtio.h  |  1 +
>  2 files changed, 5 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
> index 1346ae8..1c01ac3 100644
> --- a/drivers/virtio/virtio.c
> +++ b/drivers/virtio/virtio.c
> @@ -73,7 +73,7 @@ static int virtio_dev_match(struct device *_dv, struct device_driver *_dr)
>  	struct virtio_device *dev = dev_to_virtio(_dv);
>  	const struct virtio_device_id *ids;
>  
> -	ids = container_of(_dr, struct virtio_driver, driver)->id_table;
> +	ids = drv_to_virtio(_dr)->id_table;
>  	for (i = 0; ids[i].device; i++)
>  		if (virtio_id_match(dev, &ids[i]))
>  			return 1;
> @@ -97,8 +97,7 @@ void virtio_check_driver_offered_feature(const struct virtio_device *vdev,
>  					 unsigned int fbit)
>  {
>  	unsigned int i;
> -	struct virtio_driver *drv = container_of(vdev->dev.driver,
> -						 struct virtio_driver, driver);
> +	struct virtio_driver *drv = drv_to_virtio(vdev->dev.driver);
>  
>  	for (i = 0; i < drv->feature_table_size; i++)
>  		if (drv->feature_table[i] == fbit)
> @@ -111,8 +110,7 @@ static int virtio_dev_probe(struct device *_d)
>  {
>  	int err, i;
>  	struct virtio_device *dev = dev_to_virtio(_d);
> -	struct virtio_driver *drv = container_of(dev->dev.driver,
> -						 struct virtio_driver, driver);
> +	struct virtio_driver *drv = drv_to_virtio(dev->dev.driver);
>  	u32 device_features;
>  
>  	/* We have a driver! */
> @@ -152,8 +150,7 @@ static int virtio_dev_probe(struct device *_d)
>  static int virtio_dev_remove(struct device *_d)
>  {
>  	struct virtio_device *dev = dev_to_virtio(_d);
> -	struct virtio_driver *drv = container_of(dev->dev.driver,
> -						 struct virtio_driver, driver);
> +	struct virtio_driver *drv = drv_to_virtio(dev->dev.driver);
>  
>  	drv->remove(dev);
>  
> diff --git a/include/linux/virtio.h b/include/linux/virtio.h
> index 30fc3c9..8da5811 100644
> --- a/include/linux/virtio.h
> +++ b/include/linux/virtio.h
> @@ -109,6 +109,7 @@ struct virtio_driver {
>  #endif
>  };
>  
> +#define drv_to_virtio(drv) container_of(drv, struct virtio_driver, driver)
>  int register_virtio_driver(struct virtio_driver *drv);
>  void unregister_virtio_driver(struct virtio_driver *drv);
>  #endif /* _LINUX_VIRTIO_H */
> -- 
> 1.8.0

Re: [PATCH 1/3] virtio: change to_vp_device to an inlined definition

[Michael S. Tsirkin]

On Wed, Dec 05, 2012 at 03:03:27PM +0800, Wanlong Gao wrote:
> to_vp_device is worth changing to inlined definition.
> 
> Signed-off-by: Wanlong Gao <gaowanlong@cn.fujitsu.com>

I don't think it's worth it.
The oldest gcc I happened to have around is 4.3.3
which is already smart enough to generate
identical code before and after the patch.

And a function is cleaner.

> ---
>  drivers/virtio/virtio_pci.c | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c
> index e3ecc94..7681fe3 100644
> --- a/drivers/virtio/virtio_pci.c
> +++ b/drivers/virtio/virtio_pci.c
> @@ -98,11 +98,7 @@ static struct pci_device_id virtio_pci_id_table[] = {
>  
>  MODULE_DEVICE_TABLE(pci, virtio_pci_id_table);
>  
> -/* Convert a generic virtio device to our structure */
> -static struct virtio_pci_device *to_vp_device(struct virtio_device *vdev)
> -{
> -	return container_of(vdev, struct virtio_pci_device, vdev);
> -}
> +#define to_vp_device(_vdev) container_of(_vdev, struct virtio_pci_device, vdev)
>  
>  /* virtio config->get_features() implementation */
>  static u32 vp_get_features(struct virtio_device *vdev)
> -- 
> 1.8.0

[PATCH V2 1/2] virtio: use dev_to_virtio wrapper in virtio

[Wanlong Gao]

Use dev_to_virtio wrapper in virtio to make code clearly.
Move dev_to_virtio from virtio.h to virtio.c.

Signed-off-by: Wanlong Gao <gaowanlong@cn.fujitsu.com>
---
 drivers/virtio/virtio.c | 24 ++++++++++++++----------
 include/linux/virtio.h  |  1 -
 2 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
index 1e8659c..ba24e87 100644
--- a/drivers/virtio/virtio.c
+++ b/drivers/virtio/virtio.c
@@ -7,36 +7,40 @@
 /* Unique numbering for virtio devices. */
 static DEFINE_IDA(virtio_index_ida);
 
+static inline struct virtio_device *dev_to_virtio(struct device *_dev)
+{
+	return container_of(_dev, struct virtio_device, dev);
+}
+
 static ssize_t device_show(struct device *_d,
 			   struct device_attribute *attr, char *buf)
 {
-	struct virtio_device *dev = container_of(_d,struct virtio_device,dev);
+	struct virtio_device *dev = dev_to_virtio(_d);
 	return sprintf(buf, "0x%04x\n", dev->id.device);
 }
 static ssize_t vendor_show(struct device *_d,
 			   struct device_attribute *attr, char *buf)
 {
-	struct virtio_device *dev = container_of(_d,struct virtio_device,dev);
+	struct virtio_device *dev = dev_to_virtio(_d);
 	return sprintf(buf, "0x%04x\n", dev->id.vendor);
 }
 static ssize_t status_show(struct device *_d,
 			   struct device_attribute *attr, char *buf)
 {
-	struct virtio_device *dev = container_of(_d,struct virtio_device,dev);
+	struct virtio_device *dev = dev_to_virtio(_d);
 	return sprintf(buf, "0x%08x\n", dev->config->get_status(dev));
 }
 static ssize_t modalias_show(struct device *_d,
 			     struct device_attribute *attr, char *buf)
 {
-	struct virtio_device *dev = container_of(_d,struct virtio_device,dev);
-
+	struct virtio_device *dev = dev_to_virtio(_d);
 	return sprintf(buf, "virtio:d%08Xv%08X\n",
 		       dev->id.device, dev->id.vendor);
 }
 static ssize_t features_show(struct device *_d,
 			     struct device_attribute *attr, char *buf)
 {
-	struct virtio_device *dev = container_of(_d, struct virtio_device, dev);
+	struct virtio_device *dev = dev_to_virtio(_d);
 	unsigned int i;
 	ssize_t len = 0;
 
@@ -71,7 +75,7 @@ static inline int virtio_id_match(const struct virtio_device *dev,
 static int virtio_dev_match(struct device *_dv, struct device_driver *_dr)
 {
 	unsigned int i;
-	struct virtio_device *dev = container_of(_dv,struct virtio_device,dev);
+	struct virtio_device *dev = dev_to_virtio(_dv);
 	const struct virtio_device_id *ids;
 
 	ids = container_of(_dr, struct virtio_driver, driver)->id_table;
@@ -83,7 +87,7 @@ static int virtio_dev_match(struct device *_dv, struct device_driver *_dr)
 
 static int virtio_uevent(struct device *_dv, struct kobj_uevent_env *env)
 {
-	struct virtio_device *dev = container_of(_dv,struct virtio_device,dev);
+	struct virtio_device *dev = dev_to_virtio(_dv);
 
 	return add_uevent_var(env, "MODALIAS=virtio:d%08Xv%08X",
 			      dev->id.device, dev->id.vendor);
@@ -111,7 +115,7 @@ EXPORT_SYMBOL_GPL(virtio_check_driver_offered_feature);
 static int virtio_dev_probe(struct device *_d)
 {
 	int err, i;
-	struct virtio_device *dev = container_of(_d,struct virtio_device,dev);
+	struct virtio_device *dev = dev_to_virtio(_d);
 	struct virtio_driver *drv = container_of(dev->dev.driver,
 						 struct virtio_driver, driver);
 	u32 device_features;
@@ -152,7 +156,7 @@ static int virtio_dev_probe(struct device *_d)
 
 static int virtio_dev_remove(struct device *_d)
 {
-	struct virtio_device *dev = container_of(_d,struct virtio_device,dev);
+	struct virtio_device *dev = dev_to_virtio(_d);
 	struct virtio_driver *drv = container_of(dev->dev.driver,
 						 struct virtio_driver, driver);
 
diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index 25fa1a6..4a8768f 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -79,7 +79,6 @@ struct virtio_device {
 	void *priv;
 };
 
-#define dev_to_virtio(dev) container_of(dev, struct virtio_device, dev)
 int register_virtio_device(struct virtio_device *dev);
 void unregister_virtio_device(struct virtio_device *dev);
 
-- 
1.8.0

[PATCH V2 2/2] virtio: add drv_to_virtio to make code clearly

[Wanlong Gao]

Add drv_to_virtio wrapper to get virtio_driver from device_driver.

Signed-off-by: Wanlong Gao <gaowanlong@cn.fujitsu.com>
---
 drivers/virtio/virtio.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
index ba24e87..43c1fb2 100644
--- a/drivers/virtio/virtio.c
+++ b/drivers/virtio/virtio.c
@@ -11,6 +11,10 @@ static inline struct virtio_device *dev_to_virtio(struct device *_dev)
 {
 	return container_of(_dev, struct virtio_device, dev);
 }
+static inline struct virtio_driver *drv_to_virtio(struct device_driver *drv)
+{
+	return container_of(drv, struct virtio_driver, driver);
+}
 
 static ssize_t device_show(struct device *_d,
 			   struct device_attribute *attr, char *buf)
@@ -78,7 +82,7 @@ static int virtio_dev_match(struct device *_dv, struct device_driver *_dr)
 	struct virtio_device *dev = dev_to_virtio(_dv);
 	const struct virtio_device_id *ids;
 
-	ids = container_of(_dr, struct virtio_driver, driver)->id_table;
+	ids = drv_to_virtio(_dr)->id_table;
 	for (i = 0; ids[i].device; i++)
 		if (virtio_id_match(dev, &ids[i]))
 			return 1;
@@ -102,8 +106,7 @@ void virtio_check_driver_offered_feature(const struct virtio_device *vdev,
 					 unsigned int fbit)
 {
 	unsigned int i;
-	struct virtio_driver *drv = container_of(vdev->dev.driver,
-						 struct virtio_driver, driver);
+	struct virtio_driver *drv = drv_to_virtio(vdev->dev.driver);
 
 	for (i = 0; i < drv->feature_table_size; i++)
 		if (drv->feature_table[i] == fbit)
@@ -116,8 +119,7 @@ static int virtio_dev_probe(struct device *_d)
 {
 	int err, i;
 	struct virtio_device *dev = dev_to_virtio(_d);
-	struct virtio_driver *drv = container_of(dev->dev.driver,
-						 struct virtio_driver, driver);
+	struct virtio_driver *drv = drv_to_virtio(dev->dev.driver);
 	u32 device_features;
 
 	/* We have a driver! */
@@ -157,8 +159,7 @@ static int virtio_dev_probe(struct device *_d)
 static int virtio_dev_remove(struct device *_d)
 {
 	struct virtio_device *dev = dev_to_virtio(_d);
-	struct virtio_driver *drv = container_of(dev->dev.driver,
-						 struct virtio_driver, driver);
+	struct virtio_driver *drv = drv_to_virtio(dev->dev.driver);
 
 	drv->remove(dev);
 
-- 
1.8.0

Re: [PATCH 1/3] virtio: change to_vp_device to an inlined definition

[Stephen Hemminger]

On Wed, 5 Dec 2012 15:03:27 +0800
Wanlong Gao <gaowanlong@cn.fujitsu.com> wrote:

> to_vp_device is worth changing to inlined definition.
> 
> Signed-off-by: Wanlong Gao <gaowanlong@cn.fujitsu.com>
> ---
>  drivers/virtio/virtio_pci.c | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c
> index e3ecc94..7681fe3 100644
> --- a/drivers/virtio/virtio_pci.c
> +++ b/drivers/virtio/virtio_pci.c
> @@ -98,11 +98,7 @@ static struct pci_device_id virtio_pci_id_table[] = {
>  
>  MODULE_DEVICE_TABLE(pci, virtio_pci_id_table);
>  
> -/* Convert a generic virtio device to our structure */
> -static struct virtio_pci_device *to_vp_device(struct virtio_device *vdev)
> -{
> -	return container_of(vdev, struct virtio_pci_device, vdev);
> -}
> +#define to_vp_device(_vdev) container_of(_vdev, struct virtio_pci_device, vdev)

Just mark the function as inline. A macro loses type checking.

Re: [PATCH 1/3] virtio: change to_vp_device to an inlined definition

[Rusty Russell]

Stephen Hemminger <shemminger@vyatta.com> writes:

> On Wed, 5 Dec 2012 15:03:27 +0800
> Wanlong Gao <gaowanlong@cn.fujitsu.com> wrote:
>
>> to_vp_device is worth changing to inlined definition.
>> 
>> Signed-off-by: Wanlong Gao <gaowanlong@cn.fujitsu.com>
>> ---
>>  drivers/virtio/virtio_pci.c | 6 +-----
>>  1 file changed, 1 insertion(+), 5 deletions(-)
>> 
>> diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c
>> index e3ecc94..7681fe3 100644
>> --- a/drivers/virtio/virtio_pci.c
>> +++ b/drivers/virtio/virtio_pci.c
>> @@ -98,11 +98,7 @@ static struct pci_device_id virtio_pci_id_table[] = {
>>  
>>  MODULE_DEVICE_TABLE(pci, virtio_pci_id_table);
>>  
>> -/* Convert a generic virtio device to our structure */
>> -static struct virtio_pci_device *to_vp_device(struct virtio_device *vdev)
>> -{
>> -	return container_of(vdev, struct virtio_pci_device, vdev);
>> -}
>> +#define to_vp_device(_vdev) container_of(_vdev, struct virtio_pci_device, vdev)
>
> Just mark the function as inline. A macro loses type checking.

No, don't.

Inline functions in C files are *wrong*: you lose the warning should it
ever become unused.  GCC's does a pretty good job these days: certainly
better than guessing.

(Yeah yeah, there are always exceptions.  I've used inline deliberately
to avoid a tangle of #ifdefs, and sometimes gcc really does screw up).

Thanks,
Rusty.

Re: [PATCH 1/3] virtio: change to_vp_device to an inlined definition

[Rusty Russell]

Wanlong Gao <gaowanlong@cn.fujitsu.com> writes:

> to_vp_device is worth changing to inlined definition.

Why?

Thanks,
Rusty.

Re: [PATCH 1/3] virtio: change to_vp_device to an inlined definition

[Wanlong Gao]

On 12/06/2012 07:21 AM, Rusty Russell wrote:
> Wanlong Gao <gaowanlong@cn.fujitsu.com> writes:
> 
>> to_vp_device is worth changing to inlined definition.
> 
> Why?

OK, I saw your comments, and I dropped this patch already.

Thanks,
Wanlong Gao

> 
> Thanks,
> Rusty.
>