[PATCH]&[Question] netdevice: Use netdev_priv()

[PATCH]&[Question] netdevice: Use netdev_priv()

[Wang Chen]

Dave

This mail is a question mail more than patch mail.
In your commit "netdev: netdev_priv() can now be sane again.",
you said netdev_priv() != netdev->priv, because that time,
netdev->priv = ((char *)dev +((sizeof(struct net_device) +(sizeof(struct net_device_subqueue) *
		(queue_count - 1)) + NETDEV_ALIGN_CONST) & ~NETDEV_ALIGN_CONST))
But now, after you do TX multiqueue improvement.
netdev->priv == netdev_priv() again.

That make me think, what the use of netdev->priv?
We have netdev_priv() and use it flexibly. Why don't kill netdev->priv?

---
>From be26ade29c6a3d0002492f7aab25258cb78ab597 Mon Sep 17 00:00:00 2001
From: Wang Chen <wangchen@cn.fujitsu.com>
Date: Fri, 1 Aug 2008 16:59:59 +0800
Subject: [PATCH] netdevice: Use netdev_priv()

dev->priv is now point to private data again.

Signed-off-by: Wang Chen <wangchen@cn.fujitsu.com>
---
 net/core/dev.c |    7 ++-----
 1 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index 63d6bcd..9b73624 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4255,11 +4255,8 @@ struct net_device *alloc_netdev_mq(int sizeof_priv, const char *name,
 	dev->num_tx_queues = queue_count;
 	dev->real_num_tx_queues = queue_count;
 
-	if (sizeof_priv) {
-		dev->priv = ((char *)dev +
-			     ((sizeof(struct net_device) + NETDEV_ALIGN_CONST)
-			      & ~NETDEV_ALIGN_CONST));
-	}
+	if (sizeof_priv)
+		dev->priv = netdev_priv(dev);
 
 	dev->gso_max_size = GSO_MAX_SIZE;
 
-- 
1.5.3.4

Re: [PATCH]&[Question] netdevice: Use netdev_priv()

[David Miller]

From: Wang Chen <wangchen@cn.fujitsu.com>
Date: Fri, 01 Aug 2008 17:50:15 +0800

> That make me think, what the use of netdev->priv?
> We have netdev_priv() and use it flexibly. Why don't kill netdev->priv?

Way too many old drivers still reference netdev->priv directly.

Once we convert all of them to netdev_priv(), yes we can kill
netdev->priv

Re: [PATCH]&[Question] netdevice: Use netdev_priv()

[Wang Chen]

David Miller said the following on 2008-8-1 18:08:
> From: Wang Chen <wangchen@cn.fujitsu.com>
> Date: Fri, 01 Aug 2008 17:50:15 +0800
> 
>> That make me think, what the use of netdev->priv?
>> We have netdev_priv() and use it flexibly. Why don't kill netdev->priv?
> 
> Way too many old drivers still reference netdev->priv directly.
> 
> Once we convert all of them to netdev_priv(), yes we can kill
> netdev->priv
> 

Thanks for the explanation.

Then next question comes.
We *have to* convert the drivers to use netdev_priv(), right?
Because, netdev->priv == netdev_priv() maybe change again in future,
just as before.

Re: [PATCH]&[Question] netdevice: Use netdev_priv()

[David Miller]

From: Wang Chen <wangchen@cn.fujitsu.com>
Date: Fri, 01 Aug 2008 18:16:32 +0800

> Then next question comes.
> We *have to* convert the drivers to use netdev_priv(), right?

No, we don't.

> Because, netdev->priv == netdev_priv() maybe change again in future,
> just as before.

We will work to prevent that.  I should not have allowed it to happen
in the first place. :-)

Re: [PATCH]&[Question] netdevice: Use netdev_priv()

[Wang Chen]

David Miller said the following on 2008-8-1 18:24:
> From: Wang Chen <wangchen@cn.fujitsu.com>
> Date: Fri, 01 Aug 2008 18:16:32 +0800
> 
>> Then next question comes.
>> We *have to* convert the drivers to use netdev_priv(), right?
> 
> No, we don't.
> 
>> Because, netdev->priv == netdev_priv() maybe change again in future,
>> just as before.
> 
> We will work to prevent that.  I should not have allowed it to happen
> in the first place. :-)
> 

How about add a comment to netdev->priv

void	*priv;	/* pointer to private data, referent me by netdev_priv()*/

Re: [PATCH]&[Question] netdevice: Use netdev_priv()

[David Miller]

From: Wang Chen <wangchen@cn.fujitsu.com>
Date: Fri, 01 Aug 2008 18:35:24 +0800

> How about add a comment to netdev->priv
> 
> void	*priv;	/* pointer to private data, referent me by netdev_priv()*/

Sure, but I'd much rather apply a patch that converts
all the direct references :-)

Re: [PATCH]&[Question] netdevice: Use netdev_priv()

[Wang Chen]

David Miller said the following on 2008-8-1 19:05:
> From: Wang Chen <wangchen@cn.fujitsu.com>
> Date: Fri, 01 Aug 2008 18:35:24 +0800
> 
>> How about add a comment to netdev->priv
>>
>> void	*priv;	/* pointer to private data, referent me by netdev_priv()*/
> 
> Sure, but I'd much rather apply a patch that converts
> all the direct references :-)

I just checked some drivers' source.
And, maybe some direct references of net_device->priv are needed.
For example:
init_one() of cxgb2 makes several net_device to share one private data.
For this purpose, direct reference is necessary.

Re: [PATCH]&[Question] netdevice: Use netdev_priv()

[David Miller]

From: Wang Chen <wangchen@cn.fujitsu.com>
Date: Tue, 05 Aug 2008 17:55:27 +0800

> init_one() of cxgb2 makes several net_device to share one private data.
> For this purpose, direct reference is necessary.

It should use netdev->ml_priv which is created for this purpose.

Every direct netdev->priv usage is a bug.

Re: [PATCH]&[Question] netdevice: Use netdev_priv()

[Divy Le Ray]

On Tuesday 05 August 2008 03:04:33 am David Miller wrote:
> From: Wang Chen <wangchen@cn.fujitsu.com>
> Date: Tue, 05 Aug 2008 17:55:27 +0800
>
> > init_one() of cxgb2 makes several net_device to share one private data.
> > For this purpose, direct reference is necessary.
>
> It should use netdev->ml_priv which is created for this purpose.
>
> Every direct netdev->priv usage is a bug.

cxgb3 was fixed in this regard, cxgb2 needs the same treatment ...

cheers,
Divy

Re: [PATCH]&[Question] netdevice: Use netdev_priv()

[Wang Chen]

Divy Le Ray said the following on 2008-8-6 1:51:
> On Tuesday 05 August 2008 03:04:33 am David Miller wrote:
>> From: Wang Chen <wangchen@cn.fujitsu.com>
>> Date: Tue, 05 Aug 2008 17:55:27 +0800
>>
>>> init_one() of cxgb2 makes several net_device to share one private data.
>>> For this purpose, direct reference is necessary.
>> It should use netdev->ml_priv which is created for this purpose.
>>
>> Every direct netdev->priv usage is a bug.
> 
> cxgb3 was fixed in this regard, cxgb2 needs the same treatment ...
> 

Thanks Dave and Divy.
I am making the big patch for fixing all direct reference of netdev->priv.