| Message ID | 20200511150759.18766-6-brgl@bgdev.pl (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mediatek: add support for MediaTek Ethernet MAC | expand |
On Mon, May 11, 2020 at 05:07:50PM +0200, Bartosz Golaszewski wrote: > From: Bartosz Golaszewski <bgolaszewski@baylibre.com> > > Appropriate amount of extra memory for private data is allocated at > the end of struct net_device. We have a helper - netdev_priv() - that > returns its address but we don't have the reverse: a function which > given the address of the private data, returns the address of struct > net_device. > > This has caused many drivers to store the pointer to net_device in > the private data structure, which basically means storing the pointer > to a structure in this very structure. To some extent, that is the way it is done now. To do anything else just makes your driver different and so harder to maintain. Is 4/8 bytes for a pointer really worth being different? Andrew
From: Bartosz Golaszewski <brgl@bgdev.pl> Date: Mon, 11 May 2020 17:07:50 +0200 > From: Bartosz Golaszewski <bgolaszewski@baylibre.com> > > Appropriate amount of extra memory for private data is allocated at > the end of struct net_device. We have a helper - netdev_priv() - that > returns its address but we don't have the reverse: a function which > given the address of the private data, returns the address of struct > net_device. > > This has caused many drivers to store the pointer to net_device in > the private data structure, which basically means storing the pointer > to a structure in this very structure. > > This patch proposes to add priv_to_netdev() - a helper which converts > the address of the private data to the address of the associated > net_device. > > Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com> Sorry, please don't do this. We had this almost two decades ago and explicitly removed it intentionally. Store the back pointer in your software state just like everyone else does.
pon., 11 maj 2020 o 22:41 David Miller <davem@davemloft.net> napisał(a): > > From: Bartosz Golaszewski <brgl@bgdev.pl> > Date: Mon, 11 May 2020 17:07:50 +0200 > > > From: Bartosz Golaszewski <bgolaszewski@baylibre.com> > > > > Appropriate amount of extra memory for private data is allocated at > > the end of struct net_device. We have a helper - netdev_priv() - that > > returns its address but we don't have the reverse: a function which > > given the address of the private data, returns the address of struct > > net_device. > > > > This has caused many drivers to store the pointer to net_device in > > the private data structure, which basically means storing the pointer > > to a structure in this very structure. > > > > This patch proposes to add priv_to_netdev() - a helper which converts > > the address of the private data to the address of the associated > > net_device. > > > > Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com> > > Sorry, please don't do this. We had this almost two decades ago and > explicitly removed it intentionally. > > Store the back pointer in your software state just like everyone else > does. I will if you insist but would you mind sharing some details on why it was removed? To me it still makes more sense than storing the pointer to a structure in *that* structure. Bart
From: Bartosz Golaszewski <brgl@bgdev.pl> Date: Tue, 12 May 2020 08:04:39 +0200 > I will if you insist but would you mind sharing some details on why it > was removed? To me it still makes more sense than storing the pointer > to a structure in *that* structure. Flexibility in implementation of where the private data is located and how it is allocated. And yes, I do insist.
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h index 130a668049ab..933c6808a87f 100644 --- a/include/linux/netdevice.h +++ b/include/linux/netdevice.h @@ -2249,6 +2249,18 @@ static inline void *netdev_priv(const struct net_device *dev) return (char *)dev + ALIGN(sizeof(struct net_device), NETDEV_ALIGN); } +/** + * priv_to_netdev - get the net_device from private data + * @priv: net_device private data + * + * Get the address of network device associated with this private data. + */ +static inline struct net_device *priv_to_netdev(void *priv) +{ + priv = (char *)priv - ALIGN(sizeof(struct net_device), NETDEV_ALIGN); + return (struct net_device *)priv; +} + /* Set the sysfs physical device reference for the network logical device * if set prior to registration will cause a symlink during initialization. */