Re: fib_frontend: Add network specific broadcasts, when it takes a sense
On Mon, Dec 12, 2016 at 9:30 AM, Jiri Benc <jb...@redhat.com> wrote: > On Fri, 9 Dec 2016 15:41:52 -0800, Brandon Philips wrote: >> The issue we have: when creating the VXLAN interface and assigning it >> an address we see a broadcast route being added by the Kernel. For >> example if we have 10.4.0.0/16 a broadcast route to 10.4.0.0 is >> created. This route is unwanted because we assign 10.4.0.0 to one of >> our VXLAN interfaces. > > Are you saying you're trying to assign the IP address 10.4.0.0/16 as a > unicast address to an interface? Then you'll run into way more problems > than the one you're describing. You can't have host part of the IP > address consisting of all zeros (or all ones). Just don't do it. Choose > a valid IP address instead. Yes, this is what we are doing; it is because of an upstream, to us, address assignment so I will figure it out upstream. Regardless, it is hard to find an RFC that says "simply don't do this because _". The closest I could find was RFC 922 after sending this which says: "There is probably no reason for such addresses to appear anywhere but as the source address of an ICMP Information". I will submit a patch that at least documents this RFC and quote. Thanks! Brandon
fib_frontend: Add network specific broadcasts, when it takes a sense
Hello- A number of us are working on an OSS overlay network system called flannel. It is used in a variety of Linux container systems and one of the backends is VXLAN. The issue we have: when creating the VXLAN interface and assigning it an address we see a broadcast route being added by the Kernel. For example if we have 10.4.0.0/16 a broadcast route to 10.4.0.0 is created. This route is unwanted because we assign 10.4.0.0 to one of our VXLAN interfaces. However, the Kernel interface bring-up comment reads: Add network specific broadcasts, when it takes a sense. The code is here: https://github.com/torvalds/linux/blob/master/net/ipv4/fib_frontend.c#L859-L872 Can someone explain why creation of the broadcast route is non-optional? Would a patch to make it optional be acceptable? Is it safe for us to simply delete the route? We have a patch that simply deletes the broadcast route after interface creation but don't know why the Kernel code "makes sense". You can read more information about the issue here: https://github.com/coreos/flannel/pull/569 Thank You, Brandon
fib_frontend: Add network specific broadcasts, when it takes a sense
Hello- A number of us are working on an OSS overlay network system called flannel. It is used in a variety of Linux container systems and one of the backends is VXLAN. The issue we have: when creating the VXLAN interface and assigning it an address we see a broadcast route being added by the Kernel. For example if we have 10.4.0.0/16 a broadcast route to 10.4.0.0 is created. This route is unwanted because we assign 10.4.0.0 to one of our VXLAN interfaces. However, the Kernel interface bring-up comment reads: Add network specific broadcasts, when it takes a sense. The code is here: https://github.com/torvalds/linux/blob/master/net/ipv4/fib_frontend.c#L859-L872 Can someone explain why creation of the broadcast route is non-optional? Would a patch to make it optional be acceptable? Is it safe for us to simply delete the route? We have a patch that simply deletes the broadcast route after interface creation but don't know why the Kernel code "makes sense". You can read more information about the issue here: https://github.com/coreos/flannel/pull/569 Thank You, Brandon
Re: ixgbe: ksoftirqd consumes 100% CPU w/ ~50 TCP conns
Hello Everyone- So we tracked it down to IOMMU causing CPU affinity getting broken[1]. Can we provide any further details or is this a known issue? Thank You, Brandon [1] https://github.com/coreos/bugs/issues/1275#issuecomment-219866601 On Tue, May 17, 2016 at 12:44 PM, Brandon Philips <bran...@ifup.co> wrote: > Hello ixgbe team- > > With Linux v4.6 and the ixgbe driver (details below) a user is reporting > ksoftirqd consuming 100% of the CPU on all cores after a moderate ~20-50 > number of TCP connections. They are unable to reproduce this issue with > Cisco hardware. > > With Kernel v3.19 they cannot reproduce[1] the issue. Disabling IOMMU > (intel_iommu=off) does "fix" the issue[2]. > > Thank You, > > Brandon > > [1] https://github.com/coreos/bugs/issues/1275#issuecomment-219157803 > [2] https://github.com/coreos/bugs/issues/1275#issuecomment-219819986 > > Intel Corporation 82599ES 10-Gigabit SFI/SFP+ Network Connection (rev 01) > ethtool -i eno1 > driver: ixgbe > version: 4.0.1-k > firmware-version: 0x84e0 > bus-info: :06:00.0 > supports-statistics: yes > supports-test: yes > supports-eeprom-access: yes > supports-register-dump: yes > supports-priv-flags: no > > CPU > Intel(R) Xeon(R) CPU E5-2690 v3 @ 2.60GHz
[PATCH] e1000e: Update e1000e driver to use devres
Conversion of e1000e probe() and remove() to devres.
Depends on [patch 1/4] Update net core to use devres.
Signed-off-by: Brandon Philips [EMAIL PROTECTED]
---
drivers/net/e1000e/netdev.c | 70 ++--
1 file changed, 17 insertions(+), 53 deletions(-)
Index: linux-netdev/drivers/net/e1000e/netdev.c
===
--- linux-netdev.orig/drivers/net/e1000e/netdev.c
+++ linux-netdev/drivers/net/e1000e/netdev.c
@@ -2516,6 +2516,7 @@ void e1000e_reinit_locked(struct e1000_a
static int __devinit e1000_sw_init(struct e1000_adapter *adapter)
{
struct e1000_hw *hw = adapter-hw;
+ struct pci_dev *pdev = adapter-pdev;
struct net_device *netdev = adapter-netdev;
adapter-rx_buffer_len = ETH_FRAME_LEN + VLAN_HLEN + ETH_FCS_LEN;
@@ -2523,11 +2524,13 @@ static int __devinit e1000_sw_init(struc
hw-mac.max_frame_size = netdev-mtu + ETH_HLEN + ETH_FCS_LEN;
hw-mac.min_frame_size = ETH_ZLEN + ETH_FCS_LEN;
- adapter-tx_ring = kzalloc(sizeof(struct e1000_ring), GFP_KERNEL);
+ adapter-tx_ring = devm_kzalloc(pdev-dev,
+ sizeof(struct e1000_ring), GFP_KERNEL);
if (!adapter-tx_ring)
goto err;
- adapter-rx_ring = kzalloc(sizeof(struct e1000_ring), GFP_KERNEL);
+ adapter-rx_ring = devm_kzalloc(pdev-dev,
+ sizeof(struct e1000_ring), GFP_KERNEL);
if (!adapter-rx_ring)
goto err;
@@ -2544,8 +2547,6 @@ static int __devinit e1000_sw_init(struc
err:
ndev_err(netdev, Unable to allocate memory for queues\n);
- kfree(adapter-rx_ring);
- kfree(adapter-tx_ring);
return -ENOMEM;
}
@@ -4016,15 +4017,13 @@ static int __devinit e1000_probe(struct
struct e1000_adapter *adapter;
struct e1000_hw *hw;
const struct e1000_info *ei = e1000_info_tbl[ent-driver_data];
- unsigned long mmio_start, mmio_len;
- unsigned long flash_start, flash_len;
static int cards_found;
int i, err, pci_using_dac;
u16 eeprom_data = 0;
u16 eeprom_apme_mask = E1000_EEPROM_APME;
- err = pci_enable_device(pdev);
+ err = pcim_enable_device(pdev);
if (err)
return err;
@@ -4042,21 +4041,20 @@ static int __devinit e1000_probe(struct
if (err) {
dev_err(pdev-dev, No usable DMA
configuration, aborting\n);
- goto err_dma;
+ return err;
}
}
}
err = pci_request_regions(pdev, e1000e_driver_name);
if (err)
- goto err_pci_reg;
+ return err;
pci_set_master(pdev);
- err = -ENOMEM;
- netdev = alloc_etherdev(sizeof(struct e1000_adapter));
+ netdev = devm_alloc_etherdev(pdev-dev, sizeof(struct e1000_adapter));
if (!netdev)
- goto err_alloc_etherdev;
+ return -ENOMEM;
SET_MODULE_OWNER(netdev);
SET_NETDEV_DEV(netdev, pdev-dev);
@@ -4073,21 +4071,16 @@ static int __devinit e1000_probe(struct
adapter-hw.mac.type = ei-mac;
adapter-msg_enable = (1 NETIF_MSG_DRV | NETIF_MSG_PROBE) - 1;
- mmio_start = pci_resource_start(pdev, 0);
- mmio_len = pci_resource_len(pdev, 0);
- err = -EIO;
- adapter-hw.hw_addr = ioremap(mmio_start, mmio_len);
+ adapter-hw.hw_addr = pcim_iomap(pdev, 0, 0);
if (!adapter-hw.hw_addr)
- goto err_ioremap;
+ return -EIO;
if ((adapter-flags FLAG_HAS_FLASH)
(pci_resource_flags(pdev, 1) IORESOURCE_MEM)) {
- flash_start = pci_resource_start(pdev, 1);
- flash_len = pci_resource_len(pdev, 1);
- adapter-hw.flash_address = ioremap(flash_start, flash_len);
+ adapter-hw.flash_address = pcim_iomap(pdev, 1, 0);
if (!adapter-hw.flash_address)
- goto err_flashmap;
+ return -EIO;
}
/* construct the net_device struct */
@@ -4112,17 +4105,15 @@ static int __devinit e1000_probe(struct
#endif
strncpy(netdev-name, pci_name(pdev), sizeof(netdev-name) - 1);
- netdev-mem_start = mmio_start;
- netdev-mem_end = mmio_start + mmio_len;
+ netdev-mem_start = pci_resource_start(pdev, 0);
+ netdev-mem_end = netdev-mem_start + pci_resource_len(pdev, 0);
adapter-bd_number = cards_found++;
/* setup adapter struct */
err = e1000_sw_init(adapter);
if (err)
- goto err_sw_init;
-
- err = -EIO;
+ return err;
memcpy(hw-mac.ops, ei-mac_ops, sizeof(hw-mac.ops));
memcpy(hw-nvm.ops, ei-nvm_ops, sizeof
Re: [patch 2/4] Update e100 driver to use devres.
On 20:34 Thu 16 Aug 2007, Tejun Heo wrote:
Brandon Philips wrote:
@@ -2554,8 +2547,10 @@ static int __devinit e100_probe(struct p
struct net_device *netdev;
struct nic *nic;
int err;
+ void __iomem **iomap;
+ int bar;
- if(!(netdev = alloc_etherdev(sizeof(struct nic {
+ if (!(netdev = devm_alloc_etherdev(pdev-dev, sizeof(struct nic {
if(((1 debug) - 1) NETIF_MSG_PROBE)
printk(KERN_ERR PFX Etherdev alloc failed, abort.\n);
return -ENOMEM;
@@ -2585,26 +2580,28 @@ static int __devinit e100_probe(struct p
nic-msg_enable = (1 debug) - 1;
pci_set_drvdata(pdev, netdev);
- if((err = pci_enable_device(pdev))) {
+ if ((err = pcim_enable_device(pdev))) {
DPRINTK(PROBE, ERR, Cannot enable PCI device, aborting.\n);
- goto err_out_free_dev;
+ return err;
}
if(!(pci_resource_flags(pdev, 0) IORESOURCE_MEM)) {
DPRINTK(PROBE, ERR, Cannot find proper PCI device
base address, aborting.\n);
err = -ENODEV;
- goto err_out_disable_pdev;
+ return err;
}
- if((err = pci_request_regions(pdev, DRV_NAME))) {
+ bar = use_io ? 1 : 0;
+ if((err = pcim_iomap_regions(pdev, 1 bar, DRV_NAME))) {
DPRINTK(PROBE, ERR, Cannot obtain PCI resources, aborting.\n);
- goto err_out_disable_pdev;
+ return err;
}
+ iomap = pcim_iomap_table(pdev)[bar];
Type mismatch. Didn't compiler warn about it?
s/iomap/nic-csr/
if((err = pci_set_dma_mask(pdev, DMA_32BIT_MASK))) {
DPRINTK(PROBE, ERR, No usable DMA configuration, aborting.\n);
- goto err_out_free_res;
+ return err;
}
SET_MODULE_OWNER(netdev);
@@ -2613,12 +2610,6 @@ static int __devinit e100_probe(struct p
if (use_io)
DPRINTK(PROBE, INFO, using i/o access mode\n);
- nic-csr = pci_iomap(pdev, (use_io ? 1 : 0), sizeof(struct csr));
- if(!nic-csr) {
- DPRINTK(PROBE, ERR, Cannot map device registers, aborting.\n);
- err = -ENOMEM;
- goto err_out_free_res;
- }
nic-csr initialization seems missing. Have you tested the patched code?
No I didn't test it; my e100 box gave up on me. I shouldn't have sent
this patch since I wasn't able to get it tested.
Thanks,
Brandon
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 4/4] Update e1000 driver to use devres.
On 01:38 Thu 16 Aug 2007, Waskiewicz Jr, Peter P wrote:
- err = -ENOMEM;
- netdev = alloc_etherdev(sizeof(struct e1000_adapter));
+ netdev = devm_alloc_etherdev(pdev-dev, sizeof(struct
+e1000_adapter));
if (!netdev)
- goto err_alloc_etherdev;
+ return -ENOMEM;
I'm a bit confused why you removed the goto's, and then removed all the
target unwinding code at the bottom of e1000_probe(). Those labels
clean up resources if something fails, like the err_sw_init label. I
don't see anything in the devres code that jumps out at me that explains
why we can do away with these cleanup routines. Thoughts?
Have you read Documentation/driver-model/devres.txt? That has a good
explanation. Here is a practical explanation on how it works too.
This is the output from a normal modprobe then rmmod of e1000 with
devres debugging on.
# modprobe
DEVRES ADD f7fd6cc0 pcim_release (8 bytes)
DEVRES ADD f7a80fe0 devm_free_netdev (4 bytes) # netdev **p for free_netdev
DEVRES ADD f7dbe780 pcim_iomap_release (24 bytes) # adapter-hw.hw_addr
DEVRES ADD f7dbe9c0 devm_kzalloc_release (40 bytes) # adapter-tx_ring
DEVRES ADD f7dbe8c0 devm_kzalloc_release (44 bytes) # adapter-rx_ring
# rmmod
DEVRES REL f7dbe8c0 devm_kzalloc_release (44 bytes) # adapter-tx_ring
DEVRES REL f7dbe9c0 devm_kzalloc_release (40 bytes) # adapter-rx_ring
DEVRES REL f7dbe780 pcim_iomap_release (24 bytes) # adapter-hw.hw_addr
DEVRES REL f7a80fe0 devm_free_netdev (4 bytes) # called free_netdev
DEVRES REL f7fd6cc0 pcim_release (8 bytes)
Now if I insert a return -ENOMEM right after allocating tx_ring:
--- a/drivers/net/e1000/e1000_main.c
+++ b/drivers/net/e1000/e1000_main.c
@@ -1356,6 +1356,8 @@ e1000_alloc_queues(struct e1000_adapter *adapter)
{
adapter-tx_ring = kcalloc(adapter-num_tx_queues,
sizeof(struct e1000_tx_ring),
GFP_KERNEL);
+
+ return -ENOMEM;
if (!adapter-tx_ring)
return -ENOMEM;
#insmod
DEVRES ADD f7a80e80 pcim_release (8 bytes)
DEVRES ADD f7a80ca0 devm_free_netdev (4 bytes)
DEVRES ADD eb7f0080 pcim_iomap_release (24 bytes)
DEVRES ADD eb7f devm_kzalloc_release (40 bytes)
e1000_sw_init: Unable to allocate memory for queues
DEVRES REL eb7f devm_kzalloc_release (40 bytes)
DEVRES REL eb7f0080 pcim_iomap_release (24 bytes)
DEVRES REL f7a80ca0 devm_free_netdev (4 bytes)
DEVRES REL f7a80e80 pcim_release (8 bytes)
ACPI: PCI interrupt for device :02:00.0 disabled
e1000: probe of :02:00.0 failed with error -12
Since we are returning an error from probe the driver core calls
devres_release_all(dev) which releases all of the resources in the right
order. See really_probe() in drivers/base/dd.c.
SIDE NOTE
-
I ran into a possible e1000 bug with the little -ENOMEM patch above both
with and without the devres patches. The driver seems to leave the
EEPROM in a bad state on error because I get this error after trying to
insert the module again:
e1000_probe: The EEPROM Checksum Is Not Valid
A power cycle but not a reboot fixes it.
Thanks,
Brandon
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch 0/4] Update network drivers to use devres
These patches update the network driver core, e100 and e1000 driver to use devres. Devres is a simple resource manager for device drivers, see Documentation/driver-model/devres.txt for more information. The use of devres will continue to be optional with this patch set and can be applied to drivers where it makes sense. This series includes all of the feedback from the final RFC. Thanks :) - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch 1/4] Update net core to use devres.
* netdev_pci_remove_one() can replace simple pci device remove
functions
* devm_alloc_netdev() is like alloc_netdev but allocates memory using devres.
Signed-off-by: Brandon Philips [EMAIL PROTECTED]
---
include/linux/etherdevice.h |5 ++
include/linux/netdevice.h |7 ++
net/core/dev.c | 105 +++-
net/ethernet/eth.c |8 +++
4 files changed, 115 insertions(+), 10 deletions(-)
Index: linux-2.6/include/linux/netdevice.h
===
--- linux-2.6.orig/include/linux/netdevice.h
+++ linux-2.6/include/linux/netdevice.h
@@ -656,6 +656,7 @@ extern int dev_queue_xmit(struct sk_buf
extern int register_netdevice(struct net_device *dev);
extern voidunregister_netdevice(struct net_device *dev);
extern voidfree_netdev(struct net_device *dev);
+extern voidnetdev_pci_remove_one(struct pci_dev *pdev);
extern voidsynchronize_net(void);
extern int register_netdevice_notifier(struct notifier_block *nb);
extern int unregister_netdevice_notifier(struct notifier_block
*nb);
@@ -1085,8 +1086,14 @@ extern void ether_setup(struct net_devi
extern struct net_device *alloc_netdev_mq(int sizeof_priv, const char *name,
void (*setup)(struct net_device *),
unsigned int queue_count);
+extern struct net_device *devm_alloc_netdev_mq(struct device *dev,
+ int sizeof_priv, const char *name,
+ void (*setup)(struct net_device *),
+ unsigned int queue_count);
#define alloc_netdev(sizeof_priv, name, setup) \
alloc_netdev_mq(sizeof_priv, name, setup, 1)
+#define devm_alloc_netdev(dev, sizeof_priv, name, setup) \
+ devm_alloc_netdev_mq(dev, sizeof_priv, name, setup, 1)
extern int register_netdev(struct net_device *dev);
extern voidunregister_netdev(struct net_device *dev);
/* Functions used for secondary unicast and multicast support */
Index: linux-2.6/net/core/dev.c
===
--- linux-2.6.orig/net/core/dev.c
+++ linux-2.6/net/core/dev.c
@@ -89,6 +89,7 @@
#include linux/interrupt.h
#include linux/if_ether.h
#include linux/netdevice.h
+#include linux/pci.h
#include linux/etherdevice.h
#include linux/notifier.h
#include linux/skbuff.h
@@ -3658,18 +3659,46 @@ static struct net_device_stats *internal
}
/**
- * alloc_netdev_mq - allocate network device
- * @sizeof_priv: size of private data to allocate space for
- * @name: device name format string
- * @setup: callback to initialize device
- * @queue_count: the number of subqueues to allocate
+ * devm_free_netdev - wrapper around free_netdev for devres
+ */
+static void devm_free_netdev(struct device *gendev, void *res)
+{
+ struct net_device **dev = (struct net_device **)res;
+ free_netdev(*dev);
+}
+
+/**
+ * register_netdev_devres - register netdev with a managed device
+ * @dev: devres managed device responsible for the memory
+ * @netdev:pointer to netdev to be managed
*
- * Allocates a struct net_device with private data area for driver use
- * and performs basic initialization. Also allocates subquue structs
- * for each queue on the device at the end of the netdevice.
+ * Registers @netdev to the device @dev and calls free_netdev automatically
+ * when the device disappears
*/
-struct net_device *alloc_netdev_mq(int sizeof_priv, const char *name,
- void (*setup)(struct net_device *), unsigned int queue_count)
+static void *register_netdev_devres(struct device *gendev,
+ struct net_device *dev)
+{
+ struct net_device **p;
+
+ p = devres_alloc(devm_free_netdev, sizeof(*p), GFP_KERNEL);
+
+ if (unlikely(!p))
+ return NULL;
+
+ *p = dev;
+ devres_add(gendev, p);
+
+ return dev;
+}
+
+/**
+ * __alloc_netdev_mq - does the work to allocate a network device
+ * @dev: devres managed device responsible for mem.
+ * NULL if unmanaged
+ */
+struct net_device *__alloc_netdev_mq(struct device *gendev, int sizeof_priv,
+ const char *name, void (*setup)(struct net_device *),
+ unsigned int queue_count)
{
void *p;
struct net_device *dev;
@@ -3706,8 +3735,44 @@ struct net_device *alloc_netdev_mq(int s
dev-get_stats = internal_stats;
setup(dev);
strcpy(dev-name, name);
+
+ /* If we are given a device then manage this netdev with devres */
+ if (gendev != NULL)
+ return register_netdev_devres(gendev, dev);
+
return dev
[patch 2/4] Update e100 driver to use devres.
devres manages device resources and is currently used by all libata low level
drivers. When devres is used in a driver the complexity of error handling in
the device probe and remove functions can be reduced.
In this case e100_free(), e100_remove() and all of the gotos in e100_probe have
been removed.
Signed-off-by: Brandon Philips [EMAIL PROTECTED]
---
drivers/net/e100.c | 78 +
1 file changed, 20 insertions(+), 58 deletions(-)
Index: linux-2.6/drivers/net/e100.c
===
--- linux-2.6.orig/drivers/net/e100.c
+++ linux-2.6/drivers/net/e100.c
@@ -2517,18 +2517,11 @@ static int e100_do_ioctl(struct net_devi
static int e100_alloc(struct nic *nic)
{
- nic-mem = pci_alloc_consistent(nic-pdev, sizeof(struct mem),
- nic-dma_addr);
- return nic-mem ? 0 : -ENOMEM;
-}
+ struct device *dev = nic-pdev-dev;
-static void e100_free(struct nic *nic)
-{
- if(nic-mem) {
- pci_free_consistent(nic-pdev, sizeof(struct mem),
- nic-mem, nic-dma_addr);
- nic-mem = NULL;
- }
+ nic-mem = dmam_alloc_coherent(dev, sizeof(struct mem),
+ nic-dma_addr, GFP_ATOMIC);
+ return nic-mem ? 0 : -ENOMEM;
}
static int e100_open(struct net_device *netdev)
@@ -2554,8 +2547,10 @@ static int __devinit e100_probe(struct p
struct net_device *netdev;
struct nic *nic;
int err;
+ void __iomem **iomap;
+ int bar;
- if(!(netdev = alloc_etherdev(sizeof(struct nic {
+ if (!(netdev = devm_alloc_etherdev(pdev-dev, sizeof(struct nic {
if(((1 debug) - 1) NETIF_MSG_PROBE)
printk(KERN_ERR PFX Etherdev alloc failed, abort.\n);
return -ENOMEM;
@@ -2585,26 +2580,28 @@ static int __devinit e100_probe(struct p
nic-msg_enable = (1 debug) - 1;
pci_set_drvdata(pdev, netdev);
- if((err = pci_enable_device(pdev))) {
+ if ((err = pcim_enable_device(pdev))) {
DPRINTK(PROBE, ERR, Cannot enable PCI device, aborting.\n);
- goto err_out_free_dev;
+ return err;
}
if(!(pci_resource_flags(pdev, 0) IORESOURCE_MEM)) {
DPRINTK(PROBE, ERR, Cannot find proper PCI device
base address, aborting.\n);
err = -ENODEV;
- goto err_out_disable_pdev;
+ return err;
}
- if((err = pci_request_regions(pdev, DRV_NAME))) {
+ bar = use_io ? 1 : 0;
+ if((err = pcim_iomap_regions(pdev, 1 bar, DRV_NAME))) {
DPRINTK(PROBE, ERR, Cannot obtain PCI resources, aborting.\n);
- goto err_out_disable_pdev;
+ return err;
}
+ iomap = pcim_iomap_table(pdev)[bar];
if((err = pci_set_dma_mask(pdev, DMA_32BIT_MASK))) {
DPRINTK(PROBE, ERR, No usable DMA configuration, aborting.\n);
- goto err_out_free_res;
+ return err;
}
SET_MODULE_OWNER(netdev);
@@ -2613,12 +2610,6 @@ static int __devinit e100_probe(struct p
if (use_io)
DPRINTK(PROBE, INFO, using i/o access mode\n);
- nic-csr = pci_iomap(pdev, (use_io ? 1 : 0), sizeof(struct csr));
- if(!nic-csr) {
- DPRINTK(PROBE, ERR, Cannot map device registers, aborting.\n);
- err = -ENOMEM;
- goto err_out_free_res;
- }
if(ent-driver_data)
nic-flags |= ich;
@@ -2650,11 +2641,11 @@ static int __devinit e100_probe(struct p
if((err = e100_alloc(nic))) {
DPRINTK(PROBE, ERR, Cannot alloc driver memory, aborting.\n);
- goto err_out_iounmap;
+ return err;
}
if((err = e100_eeprom_load(nic)))
- goto err_out_free;
+ return err;
e100_phy_init(nic);
@@ -2664,8 +2655,7 @@ static int __devinit e100_probe(struct p
if (!eeprom_bad_csum_allow) {
DPRINTK(PROBE, ERR, Invalid MAC address from
EEPROM, aborting.\n);
- err = -EAGAIN;
- goto err_out_free;
+ return -EAGAIN;
} else {
DPRINTK(PROBE, ERR, Invalid MAC address from EEPROM,
you MUST configure one.\n);
@@ -2685,7 +2675,7 @@ static int __devinit e100_probe(struct p
strcpy(netdev-name, eth%d);
if((err = register_netdev(netdev))) {
DPRINTK(PROBE, ERR, Cannot register net device, aborting.\n);
- goto err_out_free;
+ return err;
}
DPRINTK(PROBE, INFO, addr 0x%llx, irq %d,
@@ -2695,36 +2685,8 @@ static int __devinit e100_probe(struct p
[patch 3/4] Implement devm_kcalloc
devm_kcalloc is a simple wrapper around devm_kzalloc for arrays. This is
needed because kcalloc is often used in network devices.
Signed-off-by: Brandon Philips [EMAIL PROTECTED]
---
drivers/base/devres.c | 16
include/linux/device.h |2 ++
2 files changed, 18 insertions(+)
Index: linux-2.6/drivers/base/devres.c
===
--- linux-2.6.orig/drivers/base/devres.c
+++ linux-2.6/drivers/base/devres.c
@@ -630,6 +630,22 @@ void * devm_kzalloc(struct device *dev,
EXPORT_SYMBOL_GPL(devm_kzalloc);
/**
+ * devm_kcalloc - resource-managed kcalloc
+ * @dev: Device to allocate memory for
+ * @n: number of elements.
+ * @size: element size.
+ * @flags: the type of memory to allocate.
+ */
+void *devm_kcalloc(struct device *dev, size_t n, size_t size,
+ gfp_t flags)
+{
+ if (n != 0 size ULONG_MAX / n)
+ return NULL;
+ return devm_kzalloc(dev, n * size, flags);
+}
+EXPORT_SYMBOL_GPL(devm_kcalloc);
+
+/**
* devm_kfree - Resource-managed kfree
* @dev: Device this memory belongs to
* @p: Memory to free
Index: linux-2.6/include/linux/device.h
===
--- linux-2.6.orig/include/linux/device.h
+++ linux-2.6/include/linux/device.h
@@ -402,6 +402,8 @@ extern int devres_release_group(struct d
/* managed kzalloc/kfree for device drivers, no kmalloc, always use kzalloc */
extern void *devm_kzalloc(struct device *dev, size_t size, gfp_t gfp);
+extern void *devm_kcalloc(struct device *dev, size_t n, size_t size,
+ gfp_t flags);
extern void devm_kfree(struct device *dev, void *p);
struct device {
--
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch 4/4] Update e1000 driver to use devres.
Conversion of e1000 probe() and remove() to devres.
Signed-off-by: Brandon Philips [EMAIL PROTECTED]
---
drivers/net/e1000/e1000.h |1
drivers/net/e1000/e1000_main.c | 92 -
2 files changed, 28 insertions(+), 65 deletions(-)
Index: linux-2.6/drivers/net/e1000/e1000_main.c
===
--- linux-2.6.orig/drivers/net/e1000/e1000_main.c
+++ linux-2.6/drivers/net/e1000/e1000_main.c
@@ -860,15 +860,14 @@ e1000_probe(struct pci_dev *pdev,
{
struct net_device *netdev;
struct e1000_adapter *adapter;
- unsigned long mmio_start, mmio_len;
- unsigned long flash_start, flash_len;
+ unsigned long mmio_len, flash_len;
static int cards_found = 0;
static int global_quad_port_a = 0; /* global ksp3 port a indication */
int i, err, pci_using_dac;
uint16_t eeprom_data = 0;
uint16_t eeprom_apme_mask = E1000_EEPROM_APME;
- if ((err = pci_enable_device(pdev)))
+ if ((err = pcim_enable_device(pdev)))
return err;
if (!(err = pci_set_dma_mask(pdev, DMA_64BIT_MASK))
@@ -878,20 +877,19 @@ e1000_probe(struct pci_dev *pdev,
if ((err = pci_set_dma_mask(pdev, DMA_32BIT_MASK))
(err = pci_set_consistent_dma_mask(pdev, DMA_32BIT_MASK))) {
E1000_ERR(No usable DMA configuration, aborting\n);
- goto err_dma;
+ return err;
}
pci_using_dac = 0;
}
if ((err = pci_request_regions(pdev, e1000_driver_name)))
- goto err_pci_reg;
+ return err;
pci_set_master(pdev);
- err = -ENOMEM;
- netdev = alloc_etherdev(sizeof(struct e1000_adapter));
+ netdev = devm_alloc_etherdev(pdev-dev, sizeof(struct e1000_adapter));
if (!netdev)
- goto err_alloc_etherdev;
+ return -ENOMEM;
SET_MODULE_OWNER(netdev);
SET_NETDEV_DEV(netdev, pdev-dev);
@@ -903,13 +901,11 @@ e1000_probe(struct pci_dev *pdev,
adapter-hw.back = adapter;
adapter-msg_enable = (1 debug) - 1;
- mmio_start = pci_resource_start(pdev, BAR_0);
mmio_len = pci_resource_len(pdev, BAR_0);
- err = -EIO;
- adapter-hw.hw_addr = ioremap(mmio_start, mmio_len);
+ adapter-hw.hw_addr = pcim_iomap(pdev, BAR_0, mmio_len);
if (!adapter-hw.hw_addr)
- goto err_ioremap;
+ return -EIO;
for (i = BAR_1; i = BAR_5; i++) {
if (pci_resource_len(pdev, i) == 0)
@@ -943,8 +939,8 @@ e1000_probe(struct pci_dev *pdev,
#endif
strncpy(netdev-name, pci_name(pdev), sizeof(netdev-name) - 1);
- netdev-mem_start = mmio_start;
- netdev-mem_end = mmio_start + mmio_len;
+ netdev-mem_start = pci_resource_start(pdev, BAR_0);
+ netdev-mem_end = netdev-mem_start + mmio_len;
netdev-base_addr = adapter-hw.io_base;
adapter-bd_number = cards_found;
@@ -952,16 +948,15 @@ e1000_probe(struct pci_dev *pdev,
/* setup the private structure */
if ((err = e1000_sw_init(adapter)))
- goto err_sw_init;
+ return err;
err = -EIO;
/* Flash BAR mapping must happen after e1000_sw_init
* because it depends on mac_type */
if ((adapter-hw.mac_type == e1000_ich8lan)
(pci_resource_flags(pdev, 1) IORESOURCE_MEM)) {
- flash_start = pci_resource_start(pdev, 1);
flash_len = pci_resource_len(pdev, 1);
- adapter-hw.flash_address = ioremap(flash_start, flash_len);
+ adapter-hw.flash_address = pcim_iomap(pdev, 1, flash_len);
if (!adapter-hw.flash_address)
goto err_flashmap;
}
@@ -1163,29 +1158,11 @@ err_register:
err_eeprom:
if (!e1000_check_phy_reset_block(adapter-hw))
e1000_phy_hw_reset(adapter-hw);
-
- if (adapter-hw.flash_address)
- iounmap(adapter-hw.flash_address);
err_flashmap:
#ifdef CONFIG_E1000_NAPI
for (i = 0; i adapter-num_rx_queues; i++)
dev_put(adapter-polling_netdev[i]);
#endif
-
- kfree(adapter-tx_ring);
- kfree(adapter-rx_ring);
-#ifdef CONFIG_E1000_NAPI
- kfree(adapter-polling_netdev);
-#endif
-err_sw_init:
- iounmap(adapter-hw.hw_addr);
-err_ioremap:
- free_netdev(netdev);
-err_alloc_etherdev:
- pci_release_regions(pdev);
-err_pci_reg:
-err_dma:
- pci_disable_device(pdev);
return err;
}
@@ -1224,21 +1201,6 @@ e1000_remove(struct pci_dev *pdev)
if (!e1000_check_phy_reset_block(adapter-hw))
e1000_phy_hw_reset(adapter-hw);
-
- kfree(adapter-tx_ring);
- kfree(adapter-rx_ring);
-#ifdef CONFIG_E1000_NAPI
- kfree(adapter-polling_netdev);
-#endif
Re: [patch 1/5][RFC] NET: Change pci_enable_device topci_reenable_device to keep device enable balance
On 17:30 Wed 08 Aug 2007, Ramkrishna Vepa wrote:
Before slot_reset event is called io_error_detected could be called
(where pci_disable_device() is called), right?
Oops! Right, the documentation says .error_detected is _always_ called
before .slot_reset. So, this patch is not correct. Please don't merge
this.
From Documentation/pci-error-recovery.txt:
STEP 1: Notification
Platform calls the error_detected() callback on every instance of
every driver affected by the error.
...
If any driver requested a slot reset (by returning PCI_ERS_RESULT_NEED_RESET),
then recovery proceeds to STEP 4 (Slot Reset).
The pci_reenable_device() will call enable only if the device was
enabled before and would not be enabled if the device were disabled. Is
this the intended behavior?
Yes, you are right. And no it isn't.
Thanks,
Brandon
-Original Message-
From: [EMAIL PROTECTED]
[mailto:[EMAIL PROTECTED]
On Behalf Of Brandon Philips
Sent: Thursday, August 02, 2007 3:44 PM
To: netdev@vger.kernel.org
Cc: [EMAIL PROTECTED]; Brandon Philips
Subject: [patch 1/5][RFC] NET: Change pci_enable_device
topci_reenable_device to keep device enable balance
On a slot_reset event pci_disable_device() is never called so calling
pci_enable_device() will unbalance the enable count.
Signed-off-by: Brandon Philips [EMAIL PROTECTED]
---
drivers/net/e100.c |2 +-
drivers/net/e1000/e1000_main.c |2 +-
drivers/net/ixgb/ixgb_main.c |2 +-
drivers/net/s2io.c |2 +-
4 files changed, 4 insertions(+), 4 deletions(-)
Index: linux-2.6/drivers/net/e100.c
===
--- linux-2.6.orig/drivers/net/e100.c
+++ linux-2.6/drivers/net/e100.c
@@ -2828,7 +2828,7 @@ static pci_ers_result_t e100_io_slot_res
struct net_device *netdev = pci_get_drvdata(pdev);
struct nic *nic = netdev_priv(netdev);
- if (pci_enable_device(pdev)) {
+ if (pci_reenable_device(pdev)) {
printk(KERN_ERR e100: Cannot re-enable PCI device after
reset.\n);
return PCI_ERS_RESULT_DISCONNECT;
}
Index: linux-2.6/drivers/net/e1000/e1000_main.c
===
--- linux-2.6.orig/drivers/net/e1000/e1000_main.c
+++ linux-2.6/drivers/net/e1000/e1000_main.c
@@ -5270,7 +5270,7 @@ static pci_ers_result_t e1000_io_slot_re
struct net_device *netdev = pci_get_drvdata(pdev);
struct e1000_adapter *adapter = netdev-priv;
- if (pci_enable_device(pdev)) {
+ if (pci_reenable_device(pdev)) {
printk(KERN_ERR e1000: Cannot re-enable PCI device
after
reset.\n);
return PCI_ERS_RESULT_DISCONNECT;
}
Index: linux-2.6/drivers/net/ixgb/ixgb_main.c
===
--- linux-2.6.orig/drivers/net/ixgb/ixgb_main.c
+++ linux-2.6/drivers/net/ixgb/ixgb_main.c
@@ -2294,7 +2294,7 @@ static pci_ers_result_t ixgb_io_slot_res
struct net_device *netdev = pci_get_drvdata(pdev);
struct ixgb_adapter *adapter = netdev_priv(netdev);
- if(pci_enable_device(pdev)) {
+ if(pci_reenable_device(pdev)) {
DPRINTK(PROBE, ERR, Cannot re-enable PCI device after
reset.\n);
return PCI_ERS_RESULT_DISCONNECT;
}
Index: linux-2.6/drivers/net/s2io.c
===
--- linux-2.6.orig/drivers/net/s2io.c
+++ linux-2.6/drivers/net/s2io.c
@@ -7833,7 +7833,7 @@ static pci_ers_result_t s2io_io_slot_res
struct net_device *netdev = pci_get_drvdata(pdev);
struct s2io_nic *sp = netdev-priv;
- if (pci_enable_device(pdev)) {
+ if (pci_reenable_device(pdev)) {
printk(KERN_ERR s2io:
Cannot re-enable PCI device after reset.\n);
return PCI_ERS_RESULT_DISCONNECT;
--
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] NET: Change pci_enable_device to pci_reenable_device to keep device enable balance
I sent this last week as part of my devres patches but it is purely a
bug fix and can be merged now.
On a slot_reset event pci_disable_device() is never called so calling
pci_enable_device() will unbalance the enable count.
Signed-off-by: Brandon Philips [EMAIL PROTECTED]
Acked-by: Tejun Heo [EMAIL PROTECTED]
---
drivers/net/e100.c |2 +-
drivers/net/e1000/e1000_main.c |2 +-
drivers/net/ixgb/ixgb_main.c |2 +-
drivers/net/s2io.c |2 +-
4 files changed, 4 insertions(+), 4 deletions(-)
Index: linux-2.6/drivers/net/e100.c
===
--- linux-2.6.orig/drivers/net/e100.c
+++ linux-2.6/drivers/net/e100.c
@@ -2828,7 +2828,7 @@ static pci_ers_result_t e100_io_slot_res
struct net_device *netdev = pci_get_drvdata(pdev);
struct nic *nic = netdev_priv(netdev);
- if (pci_enable_device(pdev)) {
+ if (pci_reenable_device(pdev)) {
printk(KERN_ERR e100: Cannot re-enable PCI device after
reset.\n);
return PCI_ERS_RESULT_DISCONNECT;
}
Index: linux-2.6/drivers/net/e1000/e1000_main.c
===
--- linux-2.6.orig/drivers/net/e1000/e1000_main.c
+++ linux-2.6/drivers/net/e1000/e1000_main.c
@@ -5270,7 +5270,7 @@ static pci_ers_result_t e1000_io_slot_re
struct net_device *netdev = pci_get_drvdata(pdev);
struct e1000_adapter *adapter = netdev-priv;
- if (pci_enable_device(pdev)) {
+ if (pci_reenable_device(pdev)) {
printk(KERN_ERR e1000: Cannot re-enable PCI device after
reset.\n);
return PCI_ERS_RESULT_DISCONNECT;
}
Index: linux-2.6/drivers/net/ixgb/ixgb_main.c
===
--- linux-2.6.orig/drivers/net/ixgb/ixgb_main.c
+++ linux-2.6/drivers/net/ixgb/ixgb_main.c
@@ -2294,7 +2294,7 @@ static pci_ers_result_t ixgb_io_slot_res
struct net_device *netdev = pci_get_drvdata(pdev);
struct ixgb_adapter *adapter = netdev_priv(netdev);
- if(pci_enable_device(pdev)) {
+ if(pci_reenable_device(pdev)) {
DPRINTK(PROBE, ERR, Cannot re-enable PCI device after
reset.\n);
return PCI_ERS_RESULT_DISCONNECT;
}
Index: linux-2.6/drivers/net/s2io.c
===
--- linux-2.6.orig/drivers/net/s2io.c
+++ linux-2.6/drivers/net/s2io.c
@@ -7833,7 +7833,7 @@ static pci_ers_result_t s2io_io_slot_res
struct net_device *netdev = pci_get_drvdata(pdev);
struct s2io_nic *sp = netdev-priv;
- if (pci_enable_device(pdev)) {
+ if (pci_reenable_device(pdev)) {
printk(KERN_ERR s2io:
Cannot re-enable PCI device after reset.\n);
return PCI_ERS_RESULT_DISCONNECT;
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [E1000-devel] [PATCH] NET: Change pci_enable_device to pci_reenable_device to keep device enable balance
On 10:36 Wed 08 Aug 2007, Kok, Auke wrote:
Brandon Philips wrote:
I sent this last week as part of my devres patches but it is purely a
bug fix and can be merged now.
On a slot_reset event pci_disable_device() is never called so calling
pci_enable_device() will unbalance the enable count.
Signed-off-by: Brandon Philips [EMAIL PROTECTED]
Acked-by: Tejun Heo [EMAIL PROTECTED]
ACK all the parts except s2io. Of course, I haven't seen the
pci_reenable_device rename patch in Jeff's tree yet...
Actually, it is in Linus's tree on git.kernel.org.
---
drivers/net/e100.c |2 +-
drivers/net/e1000/e1000_main.c |2 +-
drivers/net/ixgb/ixgb_main.c |2 +-
drivers/net/s2io.c |2 +-
4 files changed, 4 insertions(+), 4 deletions(-)
Index: linux-2.6/drivers/net/e100.c
===
--- linux-2.6.orig/drivers/net/e100.c
+++ linux-2.6/drivers/net/e100.c
@@ -2828,7 +2828,7 @@ static pci_ers_result_t e100_io_slot_res
struct net_device *netdev = pci_get_drvdata(pdev);
struct nic *nic = netdev_priv(netdev);
- if (pci_enable_device(pdev)) {
+if (pci_reenable_device(pdev)) {
printk(KERN_ERR e100: Cannot re-enable PCI device after
reset.\n);
return PCI_ERS_RESULT_DISCONNECT;
}
Index: linux-2.6/drivers/net/e1000/e1000_main.c
===
--- linux-2.6.orig/drivers/net/e1000/e1000_main.c
+++ linux-2.6/drivers/net/e1000/e1000_main.c
@@ -5270,7 +5270,7 @@ static pci_ers_result_t e1000_io_slot_re
struct net_device *netdev = pci_get_drvdata(pdev);
struct e1000_adapter *adapter = netdev-priv;
- if (pci_enable_device(pdev)) {
+if (pci_reenable_device(pdev)) {
printk(KERN_ERR e1000: Cannot re-enable PCI device after
reset.\n);
return PCI_ERS_RESULT_DISCONNECT;
}
Index: linux-2.6/drivers/net/ixgb/ixgb_main.c
===
--- linux-2.6.orig/drivers/net/ixgb/ixgb_main.c
+++ linux-2.6/drivers/net/ixgb/ixgb_main.c
@@ -2294,7 +2294,7 @@ static pci_ers_result_t ixgb_io_slot_res
struct net_device *netdev = pci_get_drvdata(pdev);
struct ixgb_adapter *adapter = netdev_priv(netdev);
- if(pci_enable_device(pdev)) {
+if(pci_reenable_device(pdev)) {
DPRINTK(PROBE, ERR, Cannot re-enable PCI device after
reset.\n);
return PCI_ERS_RESULT_DISCONNECT;
}
Index: linux-2.6/drivers/net/s2io.c
===
--- linux-2.6.orig/drivers/net/s2io.c
+++ linux-2.6/drivers/net/s2io.c
@@ -7833,7 +7833,7 @@ static pci_ers_result_t s2io_io_slot_res
struct net_device *netdev = pci_get_drvdata(pdev);
struct s2io_nic *sp = netdev-priv;
- if (pci_enable_device(pdev)) {
+if (pci_reenable_device(pdev)) {
printk(KERN_ERR s2io:
Cannot re-enable PCI device after reset.\n);
return PCI_ERS_RESULT_DISCONNECT;
-
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems? Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now http://get.splunk.com/
___
E1000-devel mailing list
[EMAIL PROTECTED]
https://lists.sourceforge.net/lists/listinfo/e1000-devel
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 2/5][RFC] Update net core to use devres.
On 18:13 Fri 03 Aug 2007, Tejun Heo wrote:
+ p = devres_alloc(devm_free_netdev, 0, GFP_KERNEL);
s/0/sizeof(*p)/
Oops! It should have read like this:
+static void * register_netdev_devres(struct device *gendev,
+ struct net_device *dev)
+{
+ void *p;
+
+ /* 0 size because we don't need it. The net_device is already alloc'd
+* in alloc_netdev_mq. We can't use devm_kzalloc in alloc_netdev_mq
+* because a net_device cannot be free'd directly as it can be a
+* kobject. See free_netdev.
+*/
+ p = devres_alloc(devm_free_netdev, 0, GFP_KERNEL);
+
+ if (unlikely(!p))
+ return NULL;
+
+ devres_add(gendev, p);
+
+ return dev;
+}
I will send the full correct patch.
Thanks,
Brandon
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 0/5][RFC] Update network drivers to use devres
On 14:44 Fri 03 Aug 2007, Stephen Hemminger wrote:
On Fri, 03 Aug 2007 20:33:04 +0900 Tejun Heo [EMAIL PROTECTED] wrote:
Devres makes low level drivers simpler, easier to get right and
maintain. Writing new drivers becomes easier too. So, why not?
Network devices seem to work fine thanks, and the resource requirements
are different. If ain't broke, don't fix it.
Care to enlighten me on how the resource requirments are different
from ATA drivers?
I was thinking of the hot remove (no mod ref counts) and lingering
/sys open issues. ATA drivers use ref counts.
I guess the hot removing is done by severing netdev from the actual
device, right? I don't see how that affects usage of devres on network
drivers. Am I missing something?
The issue is that device may be removed at any time. So you can't rely
on module ref counts to save you. And netdevice structure must still
linger after module is removed, till dev ref count goes to zero.
These patches allow the net_device to linger. The code calls
free_netdev on device removal just as before.
This is how the net_device is handled on device removal by these
patches:
+static void devm_free_netdev(struct device *gendev, void *res)
+{
+ struct net_device *dev = dev_get_drvdata(gendev);
+ free_netdev(dev);
+}
On a separate note, can you explain lingering /sys open issue to me a
bit? With recent sysfs changes, sysfs nodes are disconnected
immediately on deletion. Would that make any difference to netdevs?
Examples are in Documentation/networking/netdevices.txt
Isn't this the same problem as above? The net_device structure must
stay around if there are still references to it and it does.
Or am I missing something?
Thanks,
Brandon
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch 5/5][RFC] Update e1000 driver to use devres.
Conversion of e1000 probe() and remove() to devres.
Signed-off-by: Brandon Philips [EMAIL PROTECTED]
---
drivers/net/e1000/e1000.h |1
drivers/net/e1000/e1000_main.c | 79 -
2 files changed, 26 insertions(+), 54 deletions(-)
Index: linux-2.6/drivers/net/e1000/e1000_main.c
===
--- linux-2.6.orig/drivers/net/e1000/e1000_main.c
+++ linux-2.6/drivers/net/e1000/e1000_main.c
@@ -868,7 +868,7 @@ e1000_probe(struct pci_dev *pdev,
int i, err, pci_using_dac;
uint16_t eeprom_data = 0;
uint16_t eeprom_apme_mask = E1000_EEPROM_APME;
- if ((err = pci_enable_device(pdev)))
+ if ((err = pcim_enable_device(pdev)))
return err;
if (!(err = pci_set_dma_mask(pdev, DMA_64BIT_MASK))
@@ -884,14 +884,14 @@ e1000_probe(struct pci_dev *pdev,
}
if ((err = pci_request_regions(pdev, e1000_driver_name)))
- goto err_pci_reg;
+ goto err_dma;
pci_set_master(pdev);
err = -ENOMEM;
- netdev = alloc_etherdev(sizeof(struct e1000_adapter));
+ netdev = devm_alloc_etherdev(pdev-dev, sizeof(struct e1000_adapter));
if (!netdev)
- goto err_alloc_etherdev;
+ goto err_dma;
SET_MODULE_OWNER(netdev);
SET_NETDEV_DEV(netdev, pdev-dev);
@@ -907,9 +907,9 @@ e1000_probe(struct pci_dev *pdev,
mmio_len = pci_resource_len(pdev, BAR_0);
err = -EIO;
- adapter-hw.hw_addr = ioremap(mmio_start, mmio_len);
+ adapter-hw.hw_addr = devm_ioremap(pdev-dev, mmio_start, mmio_len);
if (!adapter-hw.hw_addr)
- goto err_ioremap;
+ goto err_dma;
for (i = BAR_1; i = BAR_5; i++) {
if (pci_resource_len(pdev, i) == 0)
@@ -952,7 +952,7 @@ e1000_probe(struct pci_dev *pdev,
/* setup the private structure */
if ((err = e1000_sw_init(adapter)))
- goto err_sw_init;
+ goto err_dma;
err = -EIO;
/* Flash BAR mapping must happen after e1000_sw_init
@@ -961,7 +961,9 @@ e1000_probe(struct pci_dev *pdev,
(pci_resource_flags(pdev, 1) IORESOURCE_MEM)) {
flash_start = pci_resource_start(pdev, 1);
flash_len = pci_resource_len(pdev, 1);
- adapter-hw.flash_address = ioremap(flash_start, flash_len);
+ adapter-hw.flash_address = devm_ioremap(pdev-dev,
+ flash_start,
+ flash_len);
if (!adapter-hw.flash_address)
goto err_flashmap;
}
@@ -1163,27 +1165,11 @@ err_register:
err_eeprom:
if (!e1000_check_phy_reset_block(adapter-hw))
e1000_phy_hw_reset(adapter-hw);
-
- if (adapter-hw.flash_address)
- iounmap(adapter-hw.flash_address);
err_flashmap:
#ifdef CONFIG_E1000_NAPI
for (i = 0; i adapter-num_rx_queues; i++)
dev_put(adapter-polling_netdev[i]);
#endif
-
- kfree(adapter-tx_ring);
- kfree(adapter-rx_ring);
-#ifdef CONFIG_E1000_NAPI
- kfree(adapter-polling_netdev);
-#endif
-err_sw_init:
- iounmap(adapter-hw.hw_addr);
-err_ioremap:
- free_netdev(netdev);
-err_alloc_etherdev:
- pci_release_regions(pdev);
-err_pci_reg:
err_dma:
pci_disable_device(pdev);
return err;
@@ -1224,21 +1210,6 @@ e1000_remove(struct pci_dev *pdev)
if (!e1000_check_phy_reset_block(adapter-hw))
e1000_phy_hw_reset(adapter-hw);
-
- kfree(adapter-tx_ring);
- kfree(adapter-rx_ring);
-#ifdef CONFIG_E1000_NAPI
- kfree(adapter-polling_netdev);
-#endif
-
- iounmap(adapter-hw.hw_addr);
- if (adapter-hw.flash_address)
- iounmap(adapter-hw.flash_address);
- pci_release_regions(pdev);
-
- free_netdev(netdev);
-
- pci_disable_device(pdev);
}
/**
@@ -1350,27 +1321,27 @@ e1000_sw_init(struct e1000_adapter *adap
static int __devinit
e1000_alloc_queues(struct e1000_adapter *adapter)
{
- adapter-tx_ring = kcalloc(adapter-num_tx_queues,
- sizeof(struct e1000_tx_ring), GFP_KERNEL);
+ adapter-tx_ring = devm_kcalloc(adapter-pdev-dev,
+ adapter-num_tx_queues,
+ sizeof(struct e1000_tx_ring),
+ GFP_KERNEL);
if (!adapter-tx_ring)
return -ENOMEM;
- adapter-rx_ring = kcalloc(adapter-num_rx_queues,
- sizeof(struct e1000_rx_ring), GFP_KERNEL);
- if (!adapter-rx_ring) {
- kfree(adapter-tx_ring);
+ adapter-rx_ring = devm_kcalloc(adapter-pdev-dev,
+ adapter-num_rx_queues
[patch 2/5][RFC] Update net core to use devres.
* netdev_pci_remove_one() can replace simple pci device remove
functions
* devm_alloc_netdev() is like alloc_netdev but allocates memory using devres.
Signed-off-by: Brandon Philips [EMAIL PROTECTED]
---
include/linux/etherdevice.h |5 ++
include/linux/netdevice.h |7 ++
net/core/dev.c | 109 +++-
net/ethernet/eth.c |8 +++
4 files changed, 119 insertions(+), 10 deletions(-)
Index: linux-2.6/include/linux/netdevice.h
===
--- linux-2.6.orig/include/linux/netdevice.h
+++ linux-2.6/include/linux/netdevice.h
@@ -656,6 +656,7 @@ extern int dev_queue_xmit(struct sk_buf
extern int register_netdevice(struct net_device *dev);
extern voidunregister_netdevice(struct net_device *dev);
extern voidfree_netdev(struct net_device *dev);
+extern voidnetdev_pci_remove_one(struct pci_dev *pdev);
extern voidsynchronize_net(void);
extern int register_netdevice_notifier(struct notifier_block *nb);
extern int unregister_netdevice_notifier(struct notifier_block
*nb);
@@ -1085,8 +1086,14 @@ extern void ether_setup(struct net_devi
extern struct net_device *alloc_netdev_mq(int sizeof_priv, const char *name,
void (*setup)(struct net_device *),
unsigned int queue_count);
+extern struct net_device *devm_alloc_netdev_mq(struct device *dev,
+ int sizeof_priv, const char *name,
+ void (*setup)(struct net_device *),
+ unsigned int queue_count);
#define alloc_netdev(sizeof_priv, name, setup) \
alloc_netdev_mq(sizeof_priv, name, setup, 1)
+#define devm_alloc_netdev(dev, sizeof_priv, name, setup) \
+ devm_alloc_netdev_mq(dev, sizeof_priv, name, setup, 1)
extern int register_netdev(struct net_device *dev);
extern voidunregister_netdev(struct net_device *dev);
/* Functions used for secondary unicast and multicast support */
Index: linux-2.6/net/core/dev.c
===
--- linux-2.6.orig/net/core/dev.c
+++ linux-2.6/net/core/dev.c
@@ -89,6 +89,7 @@
#include linux/interrupt.h
#include linux/if_ether.h
#include linux/netdevice.h
+#include linux/pci.h
#include linux/etherdevice.h
#include linux/notifier.h
#include linux/skbuff.h
@@ -3658,18 +3659,51 @@ static struct net_device_stats *internal
}
/**
- * alloc_netdev_mq - allocate network device
- * @sizeof_priv: size of private data to allocate space for
- * @name: device name format string
- * @setup: callback to initialize device
- * @queue_count: the number of subqueues to allocate
+ * devm_free_netdev - wrapper around free_netdev for devres
+ */
+static void devm_free_netdev(struct device *gendev, void *res)
+{
+ struct net_device *dev = dev_get_drvdata(gendev);
+ free_netdev(dev);
+}
+
+/**
+ * register_netdev_devres - register netdev with a managed device
+ * @dev: devres managed device responsible for the memory
+ * @netdev:pointer to netdev to be managed
*
- * Allocates a struct net_device with private data area for driver use
- * and performs basic initialization. Also allocates subquue structs
- * for each queue on the device at the end of the netdevice.
+ * Registers @netdev to the device @dev and calls free_netdev automatically
when the
+ * device disappears
*/
-struct net_device *alloc_netdev_mq(int sizeof_priv, const char *name,
- void (*setup)(struct net_device *), unsigned int queue_count)
+static inline void * register_netdev_devres(struct device *gendev,
+ struct net_device *dev)
+{
+ struct net_device **p;
+
+ /* 0 size because we don't need it. The net_device is already alloc'd
+* in alloc_netdev_mq. We can't use devm_kzalloc in alloc_netdeev_mq
+* because a net_device cannot be free'd directly as it can be a
+* kobject. See free_netdev.
+*/
+ p = devres_alloc(devm_free_netdev, 0, GFP_KERNEL);
+
+ if (unlikely(!p))
+ return NULL;
+
+ *p = dev;
+ devres_add(gendev, p);
+
+ return dev;
+}
+
+/**
+ * __alloc_netdev_mq - does the work to allocate a network device
+ * @dev: devres managed device responsible for mem.
+ * NULL if unmanaged
+ */
+struct net_device *__alloc_netdev_mq(struct device *gendev, int sizeof_priv,
+ const char *name, void (*setup)(struct net_device *),
+ unsigned int queue_count)
{
void *p;
struct net_device *dev;
@@ -3706,8 +3740,43 @@ struct net_device *alloc_netdev_mq(int s
dev-get_stats
[patch 0/5][RFC] Update network drivers to use devres
This patch set adds support for devres in the net core and converts the e100 and e1000 drivers to devres. Devres is a simple resource manager for device drivers, see Documentation/driver-model/devres.txt for more information. The use of devres will remain optional for drivers with this patch set. Drivers can be converted when it makes sense. Builds on top of f0a664bbd1839fbe9f57564983f39bfc6c6f931d in Linus' tree which renames __pci_reenable_device() to pci_reenable_device() -- - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch 1/5][RFC] NET: Change pci_enable_device to pci_reenable_device to keep device enable balance
On a slot_reset event pci_disable_device() is never called so calling
pci_enable_device() will unbalance the enable count.
Signed-off-by: Brandon Philips [EMAIL PROTECTED]
---
drivers/net/e100.c |2 +-
drivers/net/e1000/e1000_main.c |2 +-
drivers/net/ixgb/ixgb_main.c |2 +-
drivers/net/s2io.c |2 +-
4 files changed, 4 insertions(+), 4 deletions(-)
Index: linux-2.6/drivers/net/e100.c
===
--- linux-2.6.orig/drivers/net/e100.c
+++ linux-2.6/drivers/net/e100.c
@@ -2828,7 +2828,7 @@ static pci_ers_result_t e100_io_slot_res
struct net_device *netdev = pci_get_drvdata(pdev);
struct nic *nic = netdev_priv(netdev);
- if (pci_enable_device(pdev)) {
+ if (pci_reenable_device(pdev)) {
printk(KERN_ERR e100: Cannot re-enable PCI device after
reset.\n);
return PCI_ERS_RESULT_DISCONNECT;
}
Index: linux-2.6/drivers/net/e1000/e1000_main.c
===
--- linux-2.6.orig/drivers/net/e1000/e1000_main.c
+++ linux-2.6/drivers/net/e1000/e1000_main.c
@@ -5270,7 +5270,7 @@ static pci_ers_result_t e1000_io_slot_re
struct net_device *netdev = pci_get_drvdata(pdev);
struct e1000_adapter *adapter = netdev-priv;
- if (pci_enable_device(pdev)) {
+ if (pci_reenable_device(pdev)) {
printk(KERN_ERR e1000: Cannot re-enable PCI device after
reset.\n);
return PCI_ERS_RESULT_DISCONNECT;
}
Index: linux-2.6/drivers/net/ixgb/ixgb_main.c
===
--- linux-2.6.orig/drivers/net/ixgb/ixgb_main.c
+++ linux-2.6/drivers/net/ixgb/ixgb_main.c
@@ -2294,7 +2294,7 @@ static pci_ers_result_t ixgb_io_slot_res
struct net_device *netdev = pci_get_drvdata(pdev);
struct ixgb_adapter *adapter = netdev_priv(netdev);
- if(pci_enable_device(pdev)) {
+ if(pci_reenable_device(pdev)) {
DPRINTK(PROBE, ERR, Cannot re-enable PCI device after
reset.\n);
return PCI_ERS_RESULT_DISCONNECT;
}
Index: linux-2.6/drivers/net/s2io.c
===
--- linux-2.6.orig/drivers/net/s2io.c
+++ linux-2.6/drivers/net/s2io.c
@@ -7833,7 +7833,7 @@ static pci_ers_result_t s2io_io_slot_res
struct net_device *netdev = pci_get_drvdata(pdev);
struct s2io_nic *sp = netdev-priv;
- if (pci_enable_device(pdev)) {
+ if (pci_reenable_device(pdev)) {
printk(KERN_ERR s2io:
Cannot re-enable PCI device after reset.\n);
return PCI_ERS_RESULT_DISCONNECT;
--
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch 3/5][RFC] Update e100 driver to use devres.
devres manages device resources and is currently used by all libata low level
drivers. It can greatly reduce the complexity of the error handling on probe
and the device removal functions.
For example the e100_free() function and all of the gotos in e100_probe have
been removed. Also, e100_remove() has been deleted and replaced with a much
simpler netdev_pci_remove_one() function that can handle PCI net devices that
don't require teardown besides resource deallocation.
Signed-off-by: Brandon Philips [EMAIL PROTECTED]
---
drivers/net/e100.c | 70 -
1 file changed, 17 insertions(+), 53 deletions(-)
Index: linux-2.6/drivers/net/e100.c
===
--- linux-2.6.orig/drivers/net/e100.c
+++ linux-2.6/drivers/net/e100.c
@@ -2517,18 +2517,11 @@ static int e100_do_ioctl(struct net_devi
static int e100_alloc(struct nic *nic)
{
- nic-mem = pci_alloc_consistent(nic-pdev, sizeof(struct mem),
- nic-dma_addr);
- return nic-mem ? 0 : -ENOMEM;
-}
+ struct device *dev = nic-pdev-dev;
-static void e100_free(struct nic *nic)
-{
- if(nic-mem) {
- pci_free_consistent(nic-pdev, sizeof(struct mem),
- nic-mem, nic-dma_addr);
- nic-mem = NULL;
- }
+ nic-mem = dmam_alloc_coherent(dev, sizeof(struct mem),
+ nic-dma_addr, GFP_ATOMIC);
+ return nic-mem ? 0 : -ENOMEM;
}
static int e100_open(struct net_device *netdev)
@@ -2555,7 +2548,7 @@ static int __devinit e100_probe(struct p
struct nic *nic;
int err;
- if(!(netdev = alloc_etherdev(sizeof(struct nic {
+ if (!(netdev = devm_alloc_etherdev(pdev-dev, sizeof(struct nic {
if(((1 debug) - 1) NETIF_MSG_PROBE)
printk(KERN_ERR PFX Etherdev alloc failed, abort.\n);
return -ENOMEM;
@@ -2585,26 +2578,26 @@ static int __devinit e100_probe(struct p
nic-msg_enable = (1 debug) - 1;
pci_set_drvdata(pdev, netdev);
- if((err = pci_enable_device(pdev))) {
+ if ((err = pcim_enable_device(pdev))) {
DPRINTK(PROBE, ERR, Cannot enable PCI device, aborting.\n);
- goto err_out_free_dev;
+ return err;
}
if(!(pci_resource_flags(pdev, 0) IORESOURCE_MEM)) {
DPRINTK(PROBE, ERR, Cannot find proper PCI device
base address, aborting.\n);
err = -ENODEV;
- goto err_out_disable_pdev;
+ return err;
}
if((err = pci_request_regions(pdev, DRV_NAME))) {
DPRINTK(PROBE, ERR, Cannot obtain PCI resources, aborting.\n);
- goto err_out_disable_pdev;
+ return err;
}
if((err = pci_set_dma_mask(pdev, DMA_32BIT_MASK))) {
DPRINTK(PROBE, ERR, No usable DMA configuration, aborting.\n);
- goto err_out_free_res;
+ return err;
}
SET_MODULE_OWNER(netdev);
@@ -2613,11 +2606,11 @@ static int __devinit e100_probe(struct p
if (use_io)
DPRINTK(PROBE, INFO, using i/o access mode\n);
- nic-csr = pci_iomap(pdev, (use_io ? 1 : 0), sizeof(struct csr));
+ nic-csr = pcim_iomap(pdev, (use_io ? 1 : 0), sizeof(struct csr));
if(!nic-csr) {
DPRINTK(PROBE, ERR, Cannot map device registers, aborting.\n);
err = -ENOMEM;
- goto err_out_free_res;
+ return err;
}
if(ent-driver_data)
@@ -2650,11 +2643,11 @@ static int __devinit e100_probe(struct p
if((err = e100_alloc(nic))) {
DPRINTK(PROBE, ERR, Cannot alloc driver memory, aborting.\n);
- goto err_out_iounmap;
+ return err;
}
if((err = e100_eeprom_load(nic)))
- goto err_out_free;
+ return err;
e100_phy_init(nic);
@@ -2664,8 +2657,7 @@ static int __devinit e100_probe(struct p
if (!eeprom_bad_csum_allow) {
DPRINTK(PROBE, ERR, Invalid MAC address from
EEPROM, aborting.\n);
- err = -EAGAIN;
- goto err_out_free;
+ return -EAGAIN;
} else {
DPRINTK(PROBE, ERR, Invalid MAC address from EEPROM,
you MUST configure one.\n);
@@ -2685,7 +2677,7 @@ static int __devinit e100_probe(struct p
strcpy(netdev-name, eth%d);
if((err = register_netdev(netdev))) {
DPRINTK(PROBE, ERR, Cannot register net device, aborting.\n);
- goto err_out_free;
+ return err;
}
DPRINTK(PROBE, INFO, addr 0x%llx, irq %d,
@@ -2695,36 +2687,8 @@ static int __devinit e100_probe(struct p
[patch 4/5][RFC] Implement devm_kcalloc
devm_kcalloc is a simple wrapper around devm_kzalloc for arrays. This is
needed because kcalloc is often used in network devices.
Signed-off-by: Brandon Philips [EMAIL PROTECTED]
---
drivers/base/devres.c | 16
include/linux/device.h |1 +
2 files changed, 17 insertions(+)
Index: linux-2.6/drivers/base/devres.c
===
--- linux-2.6.orig/drivers/base/devres.c
+++ linux-2.6/drivers/base/devres.c
@@ -630,6 +630,22 @@ void * devm_kzalloc(struct device *dev,
EXPORT_SYMBOL_GPL(devm_kzalloc);
/**
+ * devm_kcalloc - resource-managed kcalloc
+ * @dev: Device to allocate memory for
+ * @n: number of elements.
+ * @size: element size.
+ * @flags: the type of memory to allocate.
+ */
+inline void * devm_kcalloc(struct device * dev, size_t n, size_t size,
+ gfp_t flags)
+{
+if (n != 0 size ULONG_MAX / n)
+return NULL;
+return devm_kzalloc(dev, n * size, flags);
+}
+EXPORT_SYMBOL_GPL(devm_kcalloc);
+
+/**
* devm_kfree - Resource-managed kfree
* @dev: Device this memory belongs to
* @p: Memory to free
Index: linux-2.6/include/linux/device.h
===
--- linux-2.6.orig/include/linux/device.h
+++ linux-2.6/include/linux/device.h
@@ -402,6 +402,7 @@ extern int devres_release_group(struct d
/* managed kzalloc/kfree for device drivers, no kmalloc, always use kzalloc */
extern void *devm_kzalloc(struct device *dev, size_t size, gfp_t gfp);
+extern void *devm_kcalloc(struct device *dev, size_t n, size_t size, gfp_t
flags);
extern void devm_kfree(struct device *dev, void *p);
struct device {
--
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2][RFC] Update net core to use devres.
On 00:09 Wed 25 Jul 2007, Al Viro wrote: On Tue, Jul 24, 2007 at 11:51:34PM +0100, Al Viro wrote: On Tue, Jul 24, 2007 at 03:39:52PM -0700, [EMAIL PROTECTED] wrote: * netdev_pci_remove_one() can replace simple pci device remove functions * devm_alloc_netdev() is like alloc_netdev but allocates memory using devres. alloc_netdev() can be removed once all drivers use devres. E... What the hell for? To make sure that we have struct device for everything, whether we need it or not? Have you actually read through drivers/net? I mean, _really_ read through it, looking for ugly cases... I've done just that several times and I'm sorry, but I would classify that project as hopeless. It's way, _way_ more diverse than SATA... Actually, it's even worse - net_device itself simply *cannot* be dealt with that way. Its lifetime can indefinitely exceed that of underlying e.g. PCI device. Could you point me to an example you have in mind? I quickly searched through a handful of the PCI device drivers and couldn't find an example where the .remove function didn't do something to the tune of: unregister_netdev(dev); ... various un-allocs and cleanup ... free_netdev(dev) ... return; Thank You, Brandon - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] TCP YeAH selects TCP Vegas with Kconfig
I tried building TCP YeAH into my kernel with TCP Vegas as a module and the build failed because TCP YeAH depends on Vegas. This patch makes Kconfig aware of the YeAH dependency on Vegas. --- net/ipv4/Kconfig |1 + 1 file changed, 1 insertion(+) Index: linux-2.6/net/ipv4/Kconfig === --- linux-2.6.orig/net/ipv4/Kconfig +++ linux-2.6/net/ipv4/Kconfig @@ -577,6 +577,7 @@ config TCP_CONG_VENO config TCP_CONG_YEAH tristate YeAH TCP depends on EXPERIMENTAL + select TCP_CONG_VEGAS default n ---help--- YeAH-TCP is a sender-side high-speed enabled TCP congestion control - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
