* Re: [Fastboot] Re: Re: tg3: issue for reboot/kexec
From: Khalid Aziz @ 2005-08-18 18:35 UTC (permalink / raw)
To: Eric W. Biederman; +Cc: linux-pm, Fastboot mailing list, netdev
In-Reply-To: <1124389821.2274.11.camel@lyra.fc.hp.com>
[-- Attachment #1: Type: text/plain, Size: 3728 bytes --]
On Thu, 2005-08-18 at 12:30 -0600, Khalid Aziz wrote:
> On Thu, 2005-06-30 at 17:52 -0600, Eric W. Biederman wrote:
> > Greg KH <greg@kroah.com> writes:
> >
> > > On Thu, Jun 30, 2005 at 05:21:45PM -0600, Eric W. Biederman wrote:
> > >> However I have gotten feedback a couple of times that
> > >> driver writers tend to prefer using reboot notifiers. In part
> > >> because shutdown functions don't exist for non-pci devices.
> > >
> > > That's a very lame excuse. All busses should have shutdown functions.
> > > And any device that is just bypassing all of the existing bus logic is
> > > still tying into the driver core directly (which is a bad thing by
> > > itself, but that's a different matter.) And there's a shutdown method
> > > there too.
> > >
> > > So there is no excuse to not use it. Please, if they complain, point
> > > them to me :)
> >
> > Ok.
> >
> > Then there is still my complaint and device_shutdown doesn't get called
> > on module removal which means it really doesn't get implemented. Perhaps
> > with kexec now being in the mainline kernel this will get better.
> >
> > Currently I have the following patch outstanding against the e1000
> > driver because on reboot on some boxes it card revisions
> > it places the card into a sleep state the driver initialization
> > routing cannot get the card out of.
> >
> > And yes the e1000 is bad and is using a reboot_notifier.
> >
> > Eric
> >
> > e1000_main.c | 2 +-
> > 1 files changed, 1 insertion(+), 1 deletion(-)
> >
> > diff -uNr linux-2.4.29-kexec-apic-virtwire-on-shutdownx86_64/drivers/net/e1000/e1000_main.c linux-2.4.29-e1000-no-poweroff-on-reboot/drivers/net/e1000/e1000_main.c
> > --- linux-2.4.29-kexec-apic-virtwire-on-shutdownx86_64/drivers/net/e1000/e1000_main.c Tue Feb 15 14:17:09 2005
> > +++ linux-2.4.29-e1000-no-poweroff-on-reboot/drivers/net/e1000/e1000_main.c Wed Feb 16 05:49:00 2005
> > @@ -2777,7 +2777,7 @@
> > case SYS_POWER_OFF:
> > while((pdev = pci_find_device(PCI_ANY_ID, PCI_ANY_ID, pdev))) {
> > if(pci_dev_driver(pdev) == &e1000_driver)
> > - e1000_suspend(pdev, 3);
> > + e1000_suspend(pdev, (event == SYS_DOWN)?0:3);
> > }
> > }
> > return NOTIFY_DONE;
>
> I have found that I can not walk reboot_notifier list in all cases
> before kexec'ing a new kernel. For instance, when handling INIT on ia64,
> we are running in interrupt context and atleast some of the reboot
> notifier callbacks call schedule(). Calling schedule() is not gonna work
> when we are running in interrupt context. I have the same concern for
> when panic gets called in interrupt context. So I added a shutdown
> function to e1000 driver instead. Patch is attached. This patch has
> worked for me.
>
> As soon as I have all the issues sorted out with kexec'ing on INIT on
> ia64, I will post a fully updated kexec patch for ia64. I now have kexec
> working solid on INIT with e1000 driver and it can handle multiple back
> to back INITs and come up in kexec'd kernel every time. I am now trying
> to sort some issues out with tg3 driver (another driver with no shutdown
> routine :(
>
Sorry, forgot to remove the last hunk that is already in 2.6.12. Updated
patch attached.
--
Khalid
====================================================================
Khalid Aziz Open Source and Linux Organization
(970)898-9214 Hewlett-Packard
khalid.aziz@hp.com Fort Collins, CO
"The Linux kernel is subject to relentless development"
- Alessandro Rubini
[-- Attachment #2: e1000_shutdown.patch --]
[-- Type: text/x-patch, Size: 3472 bytes --]
diff -urNp hpte-2.6/drivers/net/e1000/e1000_main.c hpte-2.6.init/drivers/net/e1000/e1000_main.c
--- hpte-2.6/drivers/net/e1000/e1000_main.c 2005-07-08 13:57:55.000000000 -0600
+++ hpte-2.6.init/drivers/net/e1000/e1000_main.c 2005-07-21 09:42:17.000000000 -0600
@@ -211,6 +211,7 @@ static void e1000_restore_vlan(struct e1
static int e1000_notify_reboot(struct notifier_block *, unsigned long event, void *ptr);
static int e1000_suspend(struct pci_dev *pdev, uint32_t state);
+static void e1000_shutdown(struct device *dev);
#ifdef CONFIG_PM
static int e1000_resume(struct pci_dev *pdev);
#endif
@@ -257,6 +258,8 @@ MODULE_PARM_DESC(debug, "Debug level (0=
* loaded. All it does is register with the PCI subsystem.
**/
+#define REBOOT_NOTIFIER 0
+
static int __init
e1000_init_module(void)
{
@@ -266,10 +269,15 @@ e1000_init_module(void)
printk(KERN_INFO "%s\n", e1000_copyright);
+#if (!REBOOT_NOTIFIER)
+ e1000_driver.driver.shutdown = e1000_shutdown;
+#endif
ret = pci_module_init(&e1000_driver);
+#if REBOOT_NOTIFIER
if(ret >= 0) {
register_reboot_notifier(&e1000_notifier_reboot);
}
+#endif
return ret;
}
@@ -285,7 +293,9 @@ module_init(e1000_init_module);
static void __exit
e1000_exit_module(void)
{
+#if REBOOT_NOTIFIER
unregister_reboot_notifier(&e1000_notifier_reboot);
+#endif
pci_unregister_driver(&e1000_driver);
}
@@ -3197,6 +3207,71 @@ e1000_suspend(struct pci_dev *pdev, uint
return 0;
}
+static void
+e1000_shutdown(struct device *dev)
+{
+ struct pci_dev *pdev = to_pci_dev(dev);
+ struct net_device *netdev = pci_get_drvdata(pdev);
+ struct e1000_adapter *adapter = netdev->priv;
+ struct e1000_hw *hw = &adapter->hw;
+ uint32_t ctrl;
+
+ netif_device_detach(netdev);
+
+ if(netif_running(netdev)) {
+ e1000_irq_disable(adapter);
+ del_timer(&adapter->tx_fifo_stall_timer);
+ del_timer(&adapter->watchdog_timer);
+ del_timer(&adapter->phy_info_timer);
+
+#ifdef CONFIG_E1000_NAPI
+ netif_poll_disable(netdev);
+#endif
+ adapter->link_speed = 0;
+ adapter->link_duplex = 0;
+ netif_carrier_off(netdev);
+ netif_stop_queue(netdev);
+ }
+
+ ctrl = E1000_READ_REG(hw, CTRL);
+
+ /* Must reset the PHY before resetting the MAC */
+ if((hw->mac_type == e1000_82541) || (hw->mac_type == e1000_82547)) {
+ E1000_WRITE_REG_IO(hw, CTRL, (ctrl | E1000_CTRL_PHY_RST));
+ mdelay(5);
+ }
+
+ /* Issue a global reset to the MAC. This will reset the chip's
+ * transmit, receive, DMA, and link units. It will not effect
+ * the current PCI configuration. The global reset bit is self-
+ * clearing, and should clear within a microsecond.
+ */
+ switch(hw->mac_type) {
+ case e1000_82544:
+ case e1000_82540:
+ case e1000_82545:
+ case e1000_82546:
+ case e1000_82541:
+ case e1000_82541_rev_2:
+ /* These controllers can't ack the 64-bit write when issuing the
+ * reset, so use IO-mapping as a workaround to issue the reset
+ */
+ E1000_WRITE_REG_IO(hw, CTRL, (ctrl | E1000_CTRL_RST));
+ break;
+ case e1000_82545_rev_3:
+ case e1000_82546_rev_3:
+ /* Reset is performed on a shadow of the control register */
+ E1000_WRITE_REG(hw, CTRL_DUP, (ctrl | E1000_CTRL_RST));
+ break;
+ default:
+ E1000_WRITE_REG(hw, CTRL, (ctrl | E1000_CTRL_RST));
+ break;
+ }
+
+ pci_disable_device(pdev);
+ pci_set_power_state(pdev, 0);
+}
+
#ifdef CONFIG_PM
static int
e1000_resume(struct pci_dev *pdev)
[-- Attachment #3: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply
* Re: [Fastboot] Re: Re: tg3: issue for reboot/kexec
From: Khalid Aziz @ 2005-08-18 18:30 UTC (permalink / raw)
To: Eric W. Biederman; +Cc: linux-pm, Fastboot mailing list, netdev
In-Reply-To: <m1vf3vjtbc.fsf@ebiederm.dsl.xmission.com>
[-- Attachment #1: Type: text/plain, Size: 3455 bytes --]
On Thu, 2005-06-30 at 17:52 -0600, Eric W. Biederman wrote:
> Greg KH <greg@kroah.com> writes:
>
> > On Thu, Jun 30, 2005 at 05:21:45PM -0600, Eric W. Biederman wrote:
> >> However I have gotten feedback a couple of times that
> >> driver writers tend to prefer using reboot notifiers. In part
> >> because shutdown functions don't exist for non-pci devices.
> >
> > That's a very lame excuse. All busses should have shutdown functions.
> > And any device that is just bypassing all of the existing bus logic is
> > still tying into the driver core directly (which is a bad thing by
> > itself, but that's a different matter.) And there's a shutdown method
> > there too.
> >
> > So there is no excuse to not use it. Please, if they complain, point
> > them to me :)
>
> Ok.
>
> Then there is still my complaint and device_shutdown doesn't get called
> on module removal which means it really doesn't get implemented. Perhaps
> with kexec now being in the mainline kernel this will get better.
>
> Currently I have the following patch outstanding against the e1000
> driver because on reboot on some boxes it card revisions
> it places the card into a sleep state the driver initialization
> routing cannot get the card out of.
>
> And yes the e1000 is bad and is using a reboot_notifier.
>
> Eric
>
> e1000_main.c | 2 +-
> 1 files changed, 1 insertion(+), 1 deletion(-)
>
> diff -uNr linux-2.4.29-kexec-apic-virtwire-on-shutdownx86_64/drivers/net/e1000/e1000_main.c linux-2.4.29-e1000-no-poweroff-on-reboot/drivers/net/e1000/e1000_main.c
> --- linux-2.4.29-kexec-apic-virtwire-on-shutdownx86_64/drivers/net/e1000/e1000_main.c Tue Feb 15 14:17:09 2005
> +++ linux-2.4.29-e1000-no-poweroff-on-reboot/drivers/net/e1000/e1000_main.c Wed Feb 16 05:49:00 2005
> @@ -2777,7 +2777,7 @@
> case SYS_POWER_OFF:
> while((pdev = pci_find_device(PCI_ANY_ID, PCI_ANY_ID, pdev))) {
> if(pci_dev_driver(pdev) == &e1000_driver)
> - e1000_suspend(pdev, 3);
> + e1000_suspend(pdev, (event == SYS_DOWN)?0:3);
> }
> }
> return NOTIFY_DONE;
I have found that I can not walk reboot_notifier list in all cases
before kexec'ing a new kernel. For instance, when handling INIT on ia64,
we are running in interrupt context and atleast some of the reboot
notifier callbacks call schedule(). Calling schedule() is not gonna work
when we are running in interrupt context. I have the same concern for
when panic gets called in interrupt context. So I added a shutdown
function to e1000 driver instead. Patch is attached. This patch has
worked for me.
As soon as I have all the issues sorted out with kexec'ing on INIT on
ia64, I will post a fully updated kexec patch for ia64. I now have kexec
working solid on INIT with e1000 driver and it can handle multiple back
to back INITs and come up in kexec'd kernel every time. I am now trying
to sort some issues out with tg3 driver (another driver with no shutdown
routine :(
--
Khalid
====================================================================
Khalid Aziz Open Source and Linux Organization
(970)898-9214 Hewlett-Packard
khalid.aziz@hp.com Fort Collins, CO
"The Linux kernel is subject to relentless development"
- Alessandro Rubini
[-- Attachment #2: e1000_shutdown.patch --]
[-- Type: text/x-patch, Size: 3834 bytes --]
diff -urNp hpte-2.6/drivers/net/e1000/e1000_main.c hpte-2.6.init/drivers/net/e1000/e1000_main.c
--- hpte-2.6/drivers/net/e1000/e1000_main.c 2005-07-08 13:57:55.000000000 -0600
+++ hpte-2.6.init/drivers/net/e1000/e1000_main.c 2005-07-21 09:42:17.000000000 -0600
@@ -211,6 +211,7 @@ static void e1000_restore_vlan(struct e1
static int e1000_notify_reboot(struct notifier_block *, unsigned long event, void *ptr);
static int e1000_suspend(struct pci_dev *pdev, uint32_t state);
+static void e1000_shutdown(struct device *dev);
#ifdef CONFIG_PM
static int e1000_resume(struct pci_dev *pdev);
#endif
@@ -257,6 +258,8 @@ MODULE_PARM_DESC(debug, "Debug level (0=
* loaded. All it does is register with the PCI subsystem.
**/
+#define REBOOT_NOTIFIER 0
+
static int __init
e1000_init_module(void)
{
@@ -266,10 +269,15 @@ e1000_init_module(void)
printk(KERN_INFO "%s\n", e1000_copyright);
+#if (!REBOOT_NOTIFIER)
+ e1000_driver.driver.shutdown = e1000_shutdown;
+#endif
ret = pci_module_init(&e1000_driver);
+#if REBOOT_NOTIFIER
if(ret >= 0) {
register_reboot_notifier(&e1000_notifier_reboot);
}
+#endif
return ret;
}
@@ -285,7 +293,9 @@ module_init(e1000_init_module);
static void __exit
e1000_exit_module(void)
{
+#if REBOOT_NOTIFIER
unregister_reboot_notifier(&e1000_notifier_reboot);
+#endif
pci_unregister_driver(&e1000_driver);
}
@@ -3197,6 +3207,71 @@ e1000_suspend(struct pci_dev *pdev, uint
return 0;
}
+static void
+e1000_shutdown(struct device *dev)
+{
+ struct pci_dev *pdev = to_pci_dev(dev);
+ struct net_device *netdev = pci_get_drvdata(pdev);
+ struct e1000_adapter *adapter = netdev->priv;
+ struct e1000_hw *hw = &adapter->hw;
+ uint32_t ctrl;
+
+ netif_device_detach(netdev);
+
+ if(netif_running(netdev)) {
+ e1000_irq_disable(adapter);
+ del_timer(&adapter->tx_fifo_stall_timer);
+ del_timer(&adapter->watchdog_timer);
+ del_timer(&adapter->phy_info_timer);
+
+#ifdef CONFIG_E1000_NAPI
+ netif_poll_disable(netdev);
+#endif
+ adapter->link_speed = 0;
+ adapter->link_duplex = 0;
+ netif_carrier_off(netdev);
+ netif_stop_queue(netdev);
+ }
+
+ ctrl = E1000_READ_REG(hw, CTRL);
+
+ /* Must reset the PHY before resetting the MAC */
+ if((hw->mac_type == e1000_82541) || (hw->mac_type == e1000_82547)) {
+ E1000_WRITE_REG_IO(hw, CTRL, (ctrl | E1000_CTRL_PHY_RST));
+ mdelay(5);
+ }
+
+ /* Issue a global reset to the MAC. This will reset the chip's
+ * transmit, receive, DMA, and link units. It will not effect
+ * the current PCI configuration. The global reset bit is self-
+ * clearing, and should clear within a microsecond.
+ */
+ switch(hw->mac_type) {
+ case e1000_82544:
+ case e1000_82540:
+ case e1000_82545:
+ case e1000_82546:
+ case e1000_82541:
+ case e1000_82541_rev_2:
+ /* These controllers can't ack the 64-bit write when issuing the
+ * reset, so use IO-mapping as a workaround to issue the reset
+ */
+ E1000_WRITE_REG_IO(hw, CTRL, (ctrl | E1000_CTRL_RST));
+ break;
+ case e1000_82545_rev_3:
+ case e1000_82546_rev_3:
+ /* Reset is performed on a shadow of the control register */
+ E1000_WRITE_REG(hw, CTRL_DUP, (ctrl | E1000_CTRL_RST));
+ break;
+ default:
+ E1000_WRITE_REG(hw, CTRL, (ctrl | E1000_CTRL_RST));
+ break;
+ }
+
+ pci_disable_device(pdev);
+ pci_set_power_state(pdev, 0);
+}
+
#ifdef CONFIG_PM
static int
e1000_resume(struct pci_dev *pdev)
@@ -3205,9 +3280,10 @@ e1000_resume(struct pci_dev *pdev)
struct e1000_adapter *adapter = netdev->priv;
uint32_t manc, ret;
- ret = pci_enable_device(pdev);
pci_set_power_state(pdev, 0);
pci_restore_state(pdev);
+ ret = pci_enable_device(pdev);
+ pci_set_master(pdev);
pci_enable_wake(pdev, 3, 0);
pci_enable_wake(pdev, 4, 0); /* 4 == D3 cold */
[-- Attachment #3: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply
* [PATCH 2/4] net: update the spider_net driver
From: Arnd Bergmann @ 2005-08-18 17:39 UTC (permalink / raw)
To: Jeff Garzik; +Cc: akpm, linuxppc64-dev, paulus, linux-kernel, netdev
In-Reply-To: <200508181931.43481.arnd@arndb.de>
This update fixes a few bugs in the spider_net driver, relative to
the version in 2.6.13-rc6-mm1. Please merge the updated driver in the
2.6.14 cycle.
- Prevent PCI posting problems by using synchronous register access
in critical places
- Check return value from firmware device tree functions
- fix device cleanup
From: Jens Osterkamp <osterkam@de.ibm.com>
Signed-off-by: Arnd Bergmann <arndb@de.ibm.com>
diff -u devel-akpm/drivers/net/spider_net.c linux-2.6.13-rc3/drivers/net/spider_net.c
--- devel-akpm/drivers/net/spider_net.c 2005-08-06 15:34:31.000000000 -0700
+++ linux-2.6.13-rc3/drivers/net/spider_net.c
@@ -109,6 +109,23 @@
}
/**
+ * spider_net_write_reg_sync - writes to an SMMIO register of a card
+ * @card: device structure
+ * @reg: register to write to
+ * @value: value to write into the specified SMMIO register
+ *
+ * Unlike spider_net_write_reg, this will also make sure the
+ * data arrives on the card by reading the reg again.
+ */
+static void
+spider_net_write_reg_sync(struct spider_net_card *card, u32 reg, u32 value)
+{
+ value = cpu_to_le32(value);
+ writel(value, card->regs + reg);
+ (void)readl(card->regs + reg);
+}
+
+/**
* spider_net_rx_irq_off - switch off rx irq on this spider card
* @card: device structure
*
@@ -123,7 +140,7 @@
spin_lock_irqsave(&card->intmask_lock, flags);
regvalue = spider_net_read_reg(card, SPIDER_NET_GHIINT0MSK);
regvalue &= ~SPIDER_NET_RXINT;
- spider_net_write_reg(card, SPIDER_NET_GHIINT0MSK, regvalue);
+ spider_net_write_reg_sync(card, SPIDER_NET_GHIINT0MSK, regvalue);
spin_unlock_irqrestore(&card->intmask_lock, flags);
}
@@ -196,7 +213,7 @@
spin_lock_irqsave(&card->intmask_lock, flags);
regvalue = spider_net_read_reg(card, SPIDER_NET_GHIINT0MSK);
regvalue |= SPIDER_NET_RXINT;
- spider_net_write_reg(card, SPIDER_NET_GHIINT0MSK, regvalue);
+ spider_net_write_reg_sync(card, SPIDER_NET_GHIINT0MSK, regvalue);
spin_unlock_irqrestore(&card->intmask_lock, flags);
}
@@ -215,7 +232,7 @@
spin_lock_irqsave(&card->intmask_lock, flags);
regvalue = spider_net_read_reg(card, SPIDER_NET_GHIINT0MSK);
regvalue &= ~SPIDER_NET_TXINT;
- spider_net_write_reg(card, SPIDER_NET_GHIINT0MSK, regvalue);
+ spider_net_write_reg_sync(card, SPIDER_NET_GHIINT0MSK, regvalue);
spin_unlock_irqrestore(&card->intmask_lock, flags);
}
@@ -234,7 +251,7 @@
spin_lock_irqsave(&card->intmask_lock, flags);
regvalue = spider_net_read_reg(card, SPIDER_NET_GHIINT0MSK);
regvalue |= SPIDER_NET_TXINT;
- spider_net_write_reg(card, SPIDER_NET_GHIINT0MSK, regvalue);
+ spider_net_write_reg_sync(card, SPIDER_NET_GHIINT0MSK, regvalue);
spin_unlock_irqrestore(&card->intmask_lock, flags);
}
@@ -813,6 +830,9 @@
spider_net_write_reg(card, SPIDER_NET_GHIINT1MSK, 0);
spider_net_write_reg(card, SPIDER_NET_GHIINT2MSK, 0);
+ /* free_irq(netdev->irq, netdev);*/
+ free_irq(to_pci_dev(netdev->class_dev.dev)->irq, netdev);
+
spider_net_write_reg(card, SPIDER_NET_GDTDMACCNTR,
SPIDER_NET_DMA_TX_FEND_VALUE);
@@ -822,10 +842,6 @@
/* release chains */
spider_net_release_tx_chain(card, 1);
- /* switch off card */
- spider_net_write_reg(card, SPIDER_NET_CKRCTRL,
- SPIDER_NET_CKRCTRL_STOP_VALUE);
-
spider_net_free_chain(card, &card->tx_chain);
spider_net_free_chain(card, &card->rx_chain);
@@ -1744,6 +1760,10 @@
goto register_int_failed;
spider_net_enable_card(card);
+
+ netif_start_queue(netdev);
+ netif_carrier_on(netdev);
+ netif_poll_enable(netdev);
return 0;
@@ -2045,7 +2065,12 @@
netdev->irq = card->pdev->irq;
dn = pci_device_to_OF_node(card->pdev);
+ if (!dn)
+ return -EIO;
+
mac = (u8 *)get_property(dn, "local-mac-address", NULL);
+ if (!mac)
+ return -EIO;
memcpy(addr.sa_data, mac, ETH_ALEN);
result = spider_net_set_mac(netdev, &addr);
@@ -2241,12 +2266,17 @@
wait_event(card->waitq,
atomic_read(&card->tx_timeout_task_counter) == 0);
-
+
unregister_netdev(netdev);
+
+ /* switch off card */
+ spider_net_write_reg(card, SPIDER_NET_CKRCTRL,
+ SPIDER_NET_CKRCTRL_STOP_VALUE);
+ spider_net_write_reg(card, SPIDER_NET_CKRCTRL,
+ SPIDER_NET_CKRCTRL_RUN_VALUE);
+
spider_net_undo_pci_setup(card);
free_netdev(netdev);
-
- free_irq(to_pci_dev(netdev->class_dev.dev)->irq, netdev);
}
static struct pci_driver spider_net_driver = {
^ permalink raw reply
* Re: OK if tg3_get_eeprom_hw_cfg() reads SRAM?
From: Grant Grundler @ 2005-08-18 17:03 UTC (permalink / raw)
To: Michael Chan; +Cc: davem, netdev
In-Reply-To: <B1508D50A0692F42B217C22C02D84972067F090E@NT-IRVA-0741.brcm.ad.broadcom.com>
On Thu, Aug 18, 2005 at 12:40:42AM -0700, Michael Chan wrote:
> > Is it OK for tg3_get_eeprom_hw_cfg() to read
> > NIC_SRAM_DATA_CFG?
>
> Yes, NIC_SRAM_DATA_CFG is in memory space (as opposed to
> register space) and we always use config. cycles to
> read/write memory space. Please see tg3_read/write_mem().
doh! of course...I've looked at tg3_read_mem() more than
a few times.
thanks,
grant
^ permalink raw reply
* Re: overflows in /proc/net/dev
From: Chris Wedgwood @ 2005-08-18 16:33 UTC (permalink / raw)
To: Sebastian Cla?en; +Cc: linux-kernel, netdev
In-Reply-To: <1124350090.29902.8.camel@basti79.freenet-ag.de>
On Thu, Aug 18, 2005 at 09:28:10AM +0200, Sebastian Cla?en wrote:
> in struct net_device_stats all members are defined as unsgined
> long. In time of gigabit ethernet this takes not long to overflow.
It should still take an appreciable amount of time surely? We can
detect those wraps in userspace and deal with it as needed.
> Are there any plans to change these coutners to unsigned long long?
It comes up from time to time (see below).
> I saw in ifconfig source code the byte and packet counters are
> already defined as unsigned long long.
ifconfig is userspace.
[...]
> struct net_device_stats
> {
> - unsigned long rx_packets; /* total packets received */
> - unsigned long tx_packets; /* total packets transmitted */
> - unsigned long rx_bytes; /* total bytes received */
> - unsigned long tx_bytes; /* total bytes transmitted */
> + unsigned long long rx_packets; /* total packets received */
> + unsigned long long tx_packets; /* total packets transmitted */
> + unsigned long long rx_bytes; /* total bytes received */
> + unsigned long long tx_bytes; /* total bytes transmitted */
I thought the concensurs here was that because doing reliable atomic
updates of 64-bit values isn't possible on some (most?) 32-bit
architectures so we need additional locking to make this work which is
undesirable? (It might even be a FAQ by now as this comes up fairly
often).
^ permalink raw reply
* RTM_F_NOTIFY
From: Tomáš Macek @ 2005-08-18 11:23 UTC (permalink / raw)
To: netdev
In the manual page is written that "RTM_F_NOTIFY if the route changes, notify the user via rtnetlink". Can you give me some information about how this 'notifying' works? How can I be notified? Will I be notified about one certain route change or only generaly about that route table changed?
I was looking for in the source of net-tools 1.60 and libnl and on google, but without success. Can you give me some link or info?
Any help appreciated!
^ permalink raw reply
* [PATCH] Put the very large file_ra_state outside of 'struct file'
From: Eric Dumazet @ 2005-08-18 9:05 UTC (permalink / raw)
To: Eric Dumazet
Cc: Andi Kleen, Benjamin LaHaise, David S. Miller, netdev,
linux-kernel
In-Reply-To: <4303D90E.2030103@cosmosbay.com>
[-- Attachment #1: Type: text/plain, Size: 409 bytes --]
[PATCH]
* struct file cleanup : the very large file_ra_state is now allocated only on demand, using a dedicated "filp_ra_cache" slab.
machines handling lot of sockets or pipes can save about 80 bytes (or 40 bytes on 32bits platforms) per file.
* private_data : The field is moved close to f_count and f_op fields to speedup sockfd_lookups
Thank you
Signed-off-by: Eric Dumazet <dada1@cosmosbay.com>
[-- Attachment #2: readahead.patch --]
[-- Type: text/plain, Size: 10565 bytes --]
--- linux-2.6.13-rc6/include/linux/fs.h 2005-08-07 20:18:56.000000000 +0200
+++ linux-2.6.13-rc6-ed/include/linux/fs.h 2005-08-18 10:30:35.000000000 +0200
@@ -586,20 +586,18 @@
struct dentry *f_dentry;
struct vfsmount *f_vfsmnt;
struct file_operations *f_op;
+ void *private_data;
atomic_t f_count;
unsigned int f_flags;
mode_t f_mode;
loff_t f_pos;
struct fown_struct f_owner;
unsigned int f_uid, f_gid;
- struct file_ra_state f_ra;
size_t f_maxcount;
unsigned long f_version;
void *f_security;
- /* needed for tty driver, and maybe others */
- void *private_data;
#ifdef CONFIG_EPOLL
/* Used by fs/eventpoll.c to link all the hooks to this file */
@@ -1480,7 +1478,10 @@
__insert_inode_hash(inode, inode->i_ino);
}
-extern struct file * get_empty_filp(void);
+#define FILP_NO_READ_AHEAD 0
+#define FILP_WITH_READ_AHEAD 1
+extern struct file * get_empty_filp(int need_read_ahead);
+
extern void file_move(struct file *f, struct list_head *list);
extern void file_kill(struct file *f);
struct bio;
@@ -1514,8 +1515,7 @@
extern void do_generic_mapping_read(struct address_space *mapping,
struct file_ra_state *, struct file *,
loff_t *, read_descriptor_t *, read_actor_t);
-extern void
-file_ra_state_init(struct file_ra_state *ra, struct address_space *mapping);
+extern void file_ra_state_init(struct file *);
extern ssize_t generic_file_direct_IO(int rw, struct kiocb *iocb,
const struct iovec *iov, loff_t offset, unsigned long nr_segs);
extern ssize_t generic_file_readv(struct file *filp, const struct iovec *iov,
@@ -1545,12 +1545,16 @@
}
#endif
+static inline struct file_ra_state *get_ra_state(struct file *f)
+{
+ return (struct file_ra_state *)(f+1);
+}
static inline void do_generic_file_read(struct file * filp, loff_t *ppos,
read_descriptor_t * desc,
read_actor_t actor)
{
do_generic_mapping_read(filp->f_mapping,
- &filp->f_ra,
+ get_ra_state(filp),
filp,
ppos,
desc,
--- linux-2.6.13-rc6/include/linux/slab.h 2005-08-07 20:18:56.000000000 +0200
+++ linux-2.6.13-rc6-ed/include/linux/slab.h 2005-08-18 09:46:40.000000000 +0200
@@ -125,6 +125,7 @@
extern kmem_cache_t *names_cachep;
extern kmem_cache_t *files_cachep;
extern kmem_cache_t *filp_cachep;
+extern kmem_cache_t *filp_ra_cachep;
extern kmem_cache_t *fs_cachep;
extern kmem_cache_t *signal_cachep;
extern kmem_cache_t *sighand_cachep;
--- linux-2.6.13-rc6/mm/readahead.c 2005-08-07 20:18:56.000000000 +0200
+++ linux-2.6.13-rc6-ed/mm/readahead.c 2005-08-18 10:41:13.000000000 +0200
@@ -33,9 +33,10 @@
* memset *ra to zero.
*/
void
-file_ra_state_init(struct file_ra_state *ra, struct address_space *mapping)
+file_ra_state_init(struct file *file)
{
- ra->ra_pages = mapping->backing_dev_info->ra_pages;
+ struct file_ra_state *ra = get_ra_state(file);
+ ra->ra_pages = file->f_mapping->host->i_mapping->backing_dev_info->ra_pages;
ra->prev_page = -1;
}
--- linux-2.6.13-rc6/mm/filemap.c 2005-08-07 20:18:56.000000000 +0200
+++ linux-2.6.13-rc6-ed/mm/filemap.c 2005-08-18 10:46:53.000000000 +0200
@@ -1187,7 +1187,7 @@
int error;
struct file *file = area->vm_file;
struct address_space *mapping = file->f_mapping;
- struct file_ra_state *ra = &file->f_ra;
+ struct file_ra_state *ra = get_ra_state(file);
struct inode *inode = mapping->host;
struct page *page;
unsigned long size, pgoff;
@@ -1243,7 +1243,7 @@
inc_page_state(pgmajfault);
}
did_readaround = 1;
- ra_pages = max_sane_readahead(file->f_ra.ra_pages);
+ ra_pages = max_sane_readahead(ra->ra_pages);
if (ra_pages) {
pgoff_t start = 0;
--- linux-2.6.13-rc6/mm/fadvise.c 2005-08-07 20:18:56.000000000 +0200
+++ linux-2.6.13-rc6-ed/mm/fadvise.c 2005-08-18 10:44:16.000000000 +0200
@@ -28,6 +28,7 @@
struct file *file = fget(fd);
struct address_space *mapping;
struct backing_dev_info *bdi;
+ struct file_ra_state *ra;
loff_t endbyte;
pgoff_t start_index;
pgoff_t end_index;
@@ -54,15 +55,20 @@
bdi = mapping->backing_dev_info;
+ /*
+ * FIXME : Are we sure here this 'file' has ra_state available ?
+ */
+ ra = get_ra_state(file);
+
switch (advice) {
case POSIX_FADV_NORMAL:
- file->f_ra.ra_pages = bdi->ra_pages;
+ ra->ra_pages = bdi->ra_pages;
break;
case POSIX_FADV_RANDOM:
- file->f_ra.ra_pages = 0;
+ ra->ra_pages = 0;
break;
case POSIX_FADV_SEQUENTIAL:
- file->f_ra.ra_pages = bdi->ra_pages * 2;
+ ra->ra_pages = bdi->ra_pages * 2;
break;
case POSIX_FADV_WILLNEED:
case POSIX_FADV_NOREUSE:
--- linux-2.6.13-rc6/fs/file_table.c 2005-08-07 20:18:56.000000000 +0200
+++ linux-2.6.13-rc6-ed/fs/file_table.c 2005-08-18 10:46:53.000000000 +0200
@@ -53,16 +53,20 @@
spin_unlock_irqrestore(&filp_count_lock, flags);
}
+/*
+ * a file can belong to different kmem_cache (filp_cache or filp_ra_cache),
+ * so let kfree() find it
+ */
static inline void file_free(struct file *f)
{
- kmem_cache_free(filp_cachep, f);
+ kfree(f);
}
/* Find an unused file structure and return a pointer to it.
* Returns NULL, if there are no more free file structures or
* we run out of memory.
*/
-struct file *get_empty_filp(void)
+struct file *get_empty_filp(int need_read_ahead)
{
static int old_max;
struct file * f;
@@ -74,11 +78,11 @@
!capable(CAP_SYS_ADMIN))
goto over;
- f = kmem_cache_alloc(filp_cachep, GFP_KERNEL);
+ f = kmem_cache_alloc(need_read_ahead ? filp_ra_cachep : filp_cachep, GFP_KERNEL);
if (f == NULL)
goto fail;
- memset(f, 0, sizeof(*f));
+ memset(f, 0, need_read_ahead ? sizeof(struct file) + sizeof(struct file_ra_state) : sizeof(struct file));
if (security_file_alloc(f))
goto fail_sec;
--- linux-2.6.13-rc6/fs/open.c 2005-08-07 20:18:56.000000000 +0200
+++ linux-2.6.13-rc6-ed/fs/open.c 2005-08-18 10:30:35.000000000 +0200
@@ -778,7 +778,7 @@
int error;
error = -ENFILE;
- f = get_empty_filp();
+ f = get_empty_filp(FILP_WITH_READ_AHEAD);
if (!f)
goto cleanup_dentry;
f->f_flags = flags;
@@ -804,7 +804,7 @@
}
f->f_flags &= ~(O_CREAT | O_EXCL | O_NOCTTY | O_TRUNC);
- file_ra_state_init(&f->f_ra, f->f_mapping->host->i_mapping);
+ file_ra_state_init(f);
/* NB: we're sure to have correct a_ops only after f_op->open */
if (f->f_flags & O_DIRECT) {
--- linux-2.6.13-rc6/fs/dcache.c 2005-08-07 20:18:56.000000000 +0200
+++ linux-2.6.13-rc6-ed/fs/dcache.c 2005-08-18 09:48:41.000000000 +0200
@@ -1703,8 +1703,10 @@
/* SLAB cache for __getname() consumers */
kmem_cache_t *names_cachep;
-/* SLAB cache for file structures */
+/* SLAB cache for file structures without read ahead state data */
kmem_cache_t *filp_cachep;
+/* SLAB cache for file structures with read ahead state data */
+kmem_cache_t *filp_ra_cachep;
EXPORT_SYMBOL(d_genocide);
@@ -1733,6 +1735,9 @@
filp_cachep = kmem_cache_create("filp", sizeof(struct file), 0,
SLAB_HWCACHE_ALIGN|SLAB_PANIC, filp_ctor, filp_dtor);
+ filp_ra_cachep = kmem_cache_create("filp_ra", sizeof(struct file) + sizeof(struct file_ra_state), 0,
+ SLAB_HWCACHE_ALIGN|SLAB_PANIC, filp_ctor, filp_dtor);
+
dcache_init(mempages);
inode_init(mempages);
files_init(mempages);
--- linux-2.6.13-rc6/net/socket.c 2005-08-07 20:18:56.000000000 +0200
+++ linux-2.6.13-rc6-ed/net/socket.c 2005-08-18 10:30:35.000000000 +0200
@@ -375,7 +375,7 @@
fd = get_unused_fd();
if (fd >= 0) {
- struct file *file = get_empty_filp();
+ struct file *file = get_empty_filp(FILP_NO_READ_AHEAD);
if (!file) {
put_unused_fd(fd);
--- linux-2.6.13-rc6/fs/eventpoll.c 2005-08-07 20:18:56.000000000 +0200
+++ linux-2.6.13-rc6-ed/fs/eventpoll.c 2005-08-18 10:30:35.000000000 +0200
@@ -717,7 +717,7 @@
/* Get an ready to use file */
error = -ENFILE;
- file = get_empty_filp();
+ file = get_empty_filp(FILP_NO_READ_AHEAD);
if (!file)
goto eexit_1;
--- linux-2.6.13-rc6/kernel/futex.c 2005-08-07 20:18:56.000000000 +0200
+++ linux-2.6.13-rc6-ed/kernel/futex.c 2005-08-18 10:30:35.000000000 +0200
@@ -661,7 +661,7 @@
ret = get_unused_fd();
if (ret < 0)
goto out;
- filp = get_empty_filp();
+ filp = get_empty_filp(FILP_NO_READ_AHEAD);
if (!filp) {
put_unused_fd(ret);
ret = -ENFILE;
--- linux-2.6.13-rc6/mm/shmem.c 2005-08-07 20:18:56.000000000 +0200
+++ linux-2.6.13-rc6-ed/mm/shmem.c 2005-08-18 10:30:35.000000000 +0200
@@ -2272,7 +2272,7 @@
goto put_memory;
error = -ENFILE;
- file = get_empty_filp();
+ file = get_empty_filp(FILP_WITH_READ_AHEAD);
if (!file)
goto put_dentry;
--- linux-2.6.13-rc6/fs/pipe.c 2005-08-07 20:18:56.000000000 +0200
+++ linux-2.6.13-rc6-ed/fs/pipe.c 2005-08-18 10:30:35.000000000 +0200
@@ -723,11 +723,11 @@
int i,j;
error = -ENFILE;
- f1 = get_empty_filp();
+ f1 = get_empty_filp(FILP_NO_READ_AHEAD);
if (!f1)
goto no_files;
- f2 = get_empty_filp();
+ f2 = get_empty_filp(FILP_NO_READ_AHEAD);
if (!f2)
goto close_f1;
--- linux-2.6.13-rc6/fs/inotify.c 2005-08-07 20:18:56.000000000 +0200
+++ linux-2.6.13-rc6-ed/fs/inotify.c 2005-08-18 10:30:35.000000000 +0200
@@ -865,7 +865,7 @@
if (fd < 0)
return fd;
- filp = get_empty_filp();
+ filp = get_empty_filp(FILP_NO_READ_AHEAD);
if (!filp) {
ret = -ENFILE;
goto out_put_fd;
--- linux-2.6.13-rc6/fs/hugetlbfs/inode.c 2005-08-07 20:18:56.000000000 +0200
+++ linux-2.6.13-rc6-ed/fs/hugetlbfs/inode.c 2005-08-18 10:30:35.000000000 +0200
@@ -785,7 +785,7 @@
goto out_shm_unlock;
error = -ENFILE;
- file = get_empty_filp();
+ file = get_empty_filp(FILP_WITH_READ_AHEAD);
if (!file)
goto out_dentry;
--- linux-2.6.13-rc6/arch/ia64/kernel/perfmon.c 2005-08-07 20:18:56.000000000 +0200
+++ linux-2.6.13-rc6-ed/arch/ia64/kernel/perfmon.c 2005-08-18 10:36:28.000000000 +0200
@@ -2157,7 +2157,7 @@
ret = -ENFILE;
- file = get_empty_filp();
+ file = get_empty_filp(FILP_WITH_READ_AHEAD);
if (!file) goto out;
/*
--- linux-2.6.13-rc6/drivers/infiniband/core/uverbs_main.c 2005-08-07 20:18:56.000000000 +0200
+++ linux-2.6.13-rc6-ed/drivers/infiniband/core/uverbs_main.c 2005-08-18 10:30:35.000000000 +0200
@@ -367,7 +367,7 @@
if (file->fd < 0)
return file->fd;
- filp = get_empty_filp();
+ filp = get_empty_filp(FILP_NO_READ_AHEAD);
if (!filp) {
put_unused_fd(file->fd);
return -ENFILE;
--- linux-2.6.13-rc6/mm/tiny-shmem.c 2005-08-07 20:18:56.000000000 +0200
+++ linux-2.6.13-rc6-ed/mm/tiny-shmem.c 2005-08-18 10:30:35.000000000 +0200
@@ -68,7 +68,7 @@
goto put_memory;
error = -ENFILE;
- file = get_empty_filp();
+ file = get_empty_filp(FILP_WITH_READ_AHEAD);
if (!file)
goto put_dentry;
^ permalink raw reply
* Re: [BUG] tg3 v3.26 patch and "FIBRE" partno(A7109-6)
From: Michael Chan @ 2005-08-18 8:22 UTC (permalink / raw)
To: Grant Grundler, davem; +Cc: netdev
Grant Grundler wrote:
> Dave, Michael,
> I was looking at a new problem Matthew Wilcox reported:
> tg3 networking failed on rx8620 IOX Core LAN
>
> He was testing 2.6.13-rc6 on an HP rx8620 (ia64).
> The NIC gets no link when "ifconfig up" and ethtool says:
> Supported ports: [ FIBRE ]
>
> when it should say "[ MII ]".
>
> I worked backwards and found v3.25 is the last version that
> ethtool reports "MII" and gets a link at 100BT (FDx and HDx).
I stared at the old and new tg3 code for a while and I think
I know what's going on in your case.
The boot code must be incorrectly reporting serdes in
NIC_SRAM_DATA_CFG. With older tg3, it always reads the PHY
ID if ASF is not enabled and if the PHY ID is valid, it will
ignore the serdes bit in NIC_SRAM_DATA_CFG and determine
whether it's copper/serdes based on the PHY ID.
In the new tg3, because the code is slightly different with
the new tg3_get_eeprom_hw_cfg(), it will always set the
TG3_FLG2_PHY_SERDES flag if the serdes bit in
NIC_SRAM_DATA_CFG is set.
So the problem is caused by wrong eeprom serdes configuration
and a slight change in the driver code. The old driver works
because the PHY ID overrides it. The new driver trusts the
eeprom's serdes configuration bit.
I will look at the code some more tomorrow to see if there
is an easy way to fix it in the new tg3.
Thanks.
^ permalink raw reply
* RE: OK if tg3_get_eeprom_hw_cfg() reads SRAM?
From: Michael Chan @ 2005-08-18 7:40 UTC (permalink / raw)
To: Grant Grundler, davem, mchan; +Cc: netdev
Grant Grundler wrote:
> The comment before tg3_get_eeprom_hw_cfg() suggests tg3
> should ONLY be touching PCI cfg space until the NIC gets
> to D0 power state.
The comment is correct. In D3 power state, the chip will
not respond to MMIO. When I did that patch, I made very
sure that only config. cycles were used before switching
to D0.
> But tg3_get_eeprom_hw_cfg() is called before
> tg3_set_power_state(tp,0).
> Is it OK for tg3_get_eeprom_hw_cfg() to read
> NIC_SRAM_DATA_CFG?
Yes, NIC_SRAM_DATA_CFG is in memory space (as opposed to
register space) and we always use config. cycles to
read/write memory space. Please see tg3_read/write_mem().
^ permalink raw reply
* Re: [PATCH] struct file cleanup : the very large file_ra_state is now allocated only on demand.
From: David S. Miller @ 2005-08-18 7:18 UTC (permalink / raw)
To: dada1; +Cc: ak, bcrl, netdev, linux-kernel
In-Reply-To: <43043567.3040204@cosmosbay.com>
From: Eric Dumazet <dada1@cosmosbay.com>
Date: Thu, 18 Aug 2005 09:14:47 +0200
> After reading your suggestions, I understand we still need two slabs.
> One (filp_cachep) without the readahead data, the other one
> (filp_ra_cachep) with it.
Correct.
> static inline struct file_ra_state *get_ra_state(struct file *f)
> {
> #ifdef CONFIG_DEBUG_READAHEAD
> BUG_ON(filp_ra_cachep != GET_PAGE_CACHE(virt_to_page(f)));
> #endif
> return (struct file_ra_state *)(f+1);
> }
Or, instead of mucking with SLAB internals, just put something like
a "filp_ra_present" debugging binary state into filp while it gets
debugged.
^ permalink raw reply
* Re: [PATCH] struct file cleanup : the very large file_ra_state is now allocated only on demand.
From: Eric Dumazet @ 2005-08-18 7:14 UTC (permalink / raw)
To: David S. Miller; +Cc: ak, bcrl, netdev, linux-kernel
In-Reply-To: <20050817.194315.111196480.davem@davemloft.net>
David S. Miller a écrit :
> From: Andi Kleen <ak@suse.de>
> Date: Thu, 18 Aug 2005 03:05:25 +0200
>
>
>>I would just set the ra pointer to a single global structure if the
>>allocation fails. Then you can avoid all the other checks. It will
>>slow down things and trash some state, but not fail and nobody
>>should expect good performance after out of memory anyways. The only
>>check still needed would be on freeing.
>
>
> I would think twice about that due to repeatability concerns. Yes, we
> should care less when memory is so low, but if we can avoid this kind
> of scenerio easily we should.
>
> Having said that, I would like to recommend looking into a scheme
> where the path leading to the filp allocation states whether the
> read-ahead bits are needed or not. This has two benefits:
>
> 1) Repeatability, and error signalling at the correct place
> should the memory allocation fail.
>
> 2) We can avoid the pointer dereference overhead. The read-ahead
> state is always at (filp + 1). Macro'ized or static inline
> function'ized interfaces for this access can make it look
> clean and perhaps even implement debugging of the case where
> we try to get at the read-ahead state for a non-read-ahead
> filp.
>
> I do really think that would be a better approach. A quick glance
> shows that it should be easy to propagate the "need_read_ahead"
> state, just by passing a boolean to get_unused_fd() via
> sock_map_fd().
Thanks David and Andi
Hum... get_unused_fd() allocates a file descriptor, not a 'struct file *'
Maybe you meant get_empty_filp(void), that we might change to get_empty_filp(int need_read_ahead)
After reading your suggestions, I understand we still need two slabs.
One (filp_cachep) without the readahead data, the other one (filp_ra_cachep) with it.
static inline struct file_ra_state *get_ra_state(struct file *f)
{
#ifdef CONFIG_DEBUG_READAHEAD
BUG_ON(filp_ra_cachep != GET_PAGE_CACHE(virt_to_page(f)));
#endif
return (struct file_ra_state *)(f+1);
}
struct file *get_empty_filp(int need_read_ahead)
{
struct file *f;
...
f = kmem_cache_alloc(need_read_ahead ? filp_ra_cachep : filp_cachep, GFP_KERNEL);
if (f == NULL)
goto fail;
memset(f, 0, need_read_ahead ? sizeof(struct file) : sizeof(struct file) + sizeof(struct file_ra_state));
...
}
file_free() then call kfree() that should find the right kmem_cache_t *
static inline void file_free(struct file *f)
{
kfree(f);
}
I will provide a new version of the patch in a later mail
Eric
^ permalink raw reply
* Re: [PATCH] struct file cleanup : the very large file_ra_state is now allocated only on demand.
From: Eric Dumazet @ 2005-08-18 6:51 UTC (permalink / raw)
To: Coywolf Qi Hunt
Cc: Andi Kleen, Benjamin LaHaise, David S. Miller, netdev,
linux-kernel
In-Reply-To: <2cd57c90050817183942b217fa@mail.gmail.com>
Coywolf Qi Hunt a écrit :
> On 8/18/05, Eric Dumazet <dada1@cosmosbay.com> wrote:
>
>>Andi Kleen a écrit :
>>
>>
>>>>(because of the insane struct file_ra_state f_ra. I wish this structure
>>>>were dynamically allocated only for files that really use it)
>>>
>>>
>>>How about you submit a patch for that instead?
>>>
>>>-Andi
>>
>>OK, could you please comment this patch ?
>>
>>The problem of dynamically allocating the readahead state data is that the allocation can fail and should not be fatal.
>>I made some choices that might be not good.
>>
>>I also chose not to align "file_ra" slab on SLAB_HWCACHE_ALIGN because the object size is 10*sizeof(long), so alignment would loose
>>6*sizeof(long) bytes for each object.
>>
>>
>>[PATCH]
>>
>>* struct file cleanup : the very large file_ra_state is now allocated only on demand, using a dedicated "file_ra" slab.
>> 64bits machines handling lot of sockets can save about 72 bytes per file.
>>* private_data : The field is moved close to f_count and f_op fields to speedup sockfd_lookups
>
>
> Why not keep the comment or fix it?
>
You mean the comment in include/linux/fs.h : /* needed for tty driver, and maybe others */ ?
I think this comment is outdated, since nearly every 'struct file' user store something of his own in this place, not only 'tty drivers'
As no other fields has comment, why should we keep this outdated comment ?
Eric
^ permalink raw reply
* [BUG] tg3 v3.26 patch and "FIBRE" partno(A7109-6)
From: Grant Grundler @ 2005-08-18 6:35 UTC (permalink / raw)
To: davem, mchan; +Cc: netdev, Grant Grundler
Dave, Michael,
I was looking at a new problem Matthew Wilcox reported:
tg3 networking failed on rx8620 IOX Core LAN
He was testing 2.6.13-rc6 on an HP rx8620 (ia64).
The NIC gets no link when "ifconfig up" and ethtool says:
Supported ports: [ FIBRE ]
when it should say "[ MII ]".
I worked backwards and found v3.25 is the last version that ethtool
reports "MII" and gets a link at 100BT (FDx and HDx).
We can't use 1000BT because of the brain damaged "bootcode"
on the "IOX Core LAN" fix wasn't committed until tg3 v3.30. :^(
Console output from modprobe and b57diag output are appended below.
This looks like further brain damage in the bootcode since both
3.25 and 3.26 read "0x25" (0x20 == FIBRE) from NIC_SRAM_DATA_CFG.
(ie nic_cfg == 0x25). But I don't understand why this commit causes
ethtool to report "FIBRE" when the previous code didn't:
"[TG3]: Split tg3_phy_probe into 2 functions"
http://www.kernel.org/git/?p=linux/kernel/git/davem/net-2.6.git;a=commit;h=7d0c41ef89dad9008edf1c3c0022721ebad39999
This commit causes tg3 to read NIC_SRAM_DATA_CFG *before* setting
the power state (see tg3_get_invariants()). And tg3_set_power_state()
wants to know if the phy is SERDES (or not). The above patch sets
TG3_FLG2_PHY_SERDES before calling tg3_set_power_state() since nic_cfg
has 0x20 bit set.
Calling tg3_get_eeprom_hw_cfg() *after* tg3_set_power_state() didn't change
anything visible. ethtool still reported FIBRE port.
I need to stare at the code some more tomorrow again to understand
why the older code happened to work despite the value of nic_cfg.
Or is it obvious to one of you?
I'm also hoping someone has a better idea how to fix this than to add
a hack based on subsystem id.
I expect HP would be willing to roll the "bootcode" for this NIC
since it's clearly broken. This would be the easiest solution.
But I've not seen a recipe that will let an HP Integrity customer
update the bcm5701 chip bootcoode. Broadcom and HP have been looking
at this since at least March, 2005 when I raised the previous bootcode
problem. I'll poke folks about this again but I'm not optimistic. *sigh*
thanks,
grant
Console output seems the same for both:
temp:/usr/src/linux-2.6.12# modprobe tg3_325-orig
tg3.c:v3.25 (March 24, 2005)
GSI 28 (level, low) -> CPU 2 (0x0800) vector 83
ACPI: PCI Interrupt 0000:00:01.0[A] -> GSI 28 (level, low) -> IRQ 83
tg3: nic_cfg 0x25
eth0: Tigon3 [partno(A7109-6) rev 0105 PHY(5701)] (PCI:33MHz:64-bit) 10/100/1000BaseT Ethernet 00:30:6e:49:42:ca
eth0: RXcsums[1] LinkChgREG[0] MIirq[0] ASF[0] Split[0] WireSpeed[1] TSOcap[0]
...
temp:/usr/src/linux-2.6.12# modprobe tg3-326
tg3.c:v3.26 (April 24, 2005)
GSI 28 (level, low) -> CPU 0 (0x0000) vector 83
ACPI: PCI Interrupt 0000:00:01.0[A] -> GSI 28 (level, low) -> IRQ 83
eth0: Tigon3 [partno(A7109-6) rev 0105 PHY(5701)] (PCI:33MHz:64-bit) 10/100/1000
BaseT Ethernet 00:30:6e:49:42:ca
eth0: RXcsums[1] LinkChgREG[0] MIirq[0] ASF[0] Split[0] WireSpeed[1] TSOcap[0]
...
b57diag output for the offending card:
C Brd:Rv Bus PCI Spd Base Irq EEP MAC Fmw Configuration
- ------- ------- --- --- ---- -- ---- ------------ ----------- --------------
0 5701:A3 00:01:0 64 33 C000 181 64k 00306E4942CA 5701-v2.17 auto
1 5701:A3 00:01:0 64 33 C000 125 64k 00306E49327F 5701-v2.17 auto
0:>secfg
Reading current NVRAM ... OK
Validating content...
** Error: unknow field 00 found
Using Defualt VPD value, press any key to continue... (paused)
1. MAC Address : 00:30:6e:49:42:ca
2. Power Dissipated (D3:D2:D1:D0) : 10:0:0:100
3. Power Consumed (D3:D2:D1:D0) : 10:0:0:100
4. Vendor ID : 14E4
5. Vendor Device ID : 1645
6. Subsystem Vendor ID : 103C
7. Subsystem Device ID : 1300
...
10. Magic Packet WoL { Enable(1), Disable(2) } : Disable
11. Product Name : A7109A COREIO10/100/1GBT ethernet controller
12. Part Number : BCM95700A6
13. Engineering Change : 106679-15
14. Serial Number : 0123456789
15. Manufacturing ID : 14e4
16. Asset Tag :
17. Part Revision : A3
18. Voltage { 1.3V(0), 1.8V(1) } : 1.8V
19. Force PCI Mode { Enable(1),Disable(2) } : Disable
21. Led Mode { TripleLink(1), Link/Speed(2) } : Triple Link
...
0:>seread 0x100-0x200
Current Mode: Legacy SEEPROM, Auto
000100: 822f0041 37313039 4120434f 5245494f 31302f31 30302f31 47425420 65746865
000120: 726e6574 20636f6e 74726f6c 6c657220 20209020 00504e07 41373130 392d3630
000140: 30303120 20534e41 35363230 31383734 3139304e 04313465 34525624 4c000000
000160: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
000180: 91990059 410b5859 5a303132 33343536 3752576b 00000000 00000000 00000000
0001a0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
0001c0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
0001e0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000078
000200: 0e000003
^ permalink raw reply
* OK if tg3_get_eeprom_hw_cfg() reads SRAM?
From: Grant Grundler @ 2005-08-18 6:34 UTC (permalink / raw)
To: davem, mchan; +Cc: netdev, Grant Grundler
Michael,
tg3_get_eeprom_hw_cfg() was added in this patch:
http://www.kernel.org/git/?p=linux/kernel/git/davem/net-2.6.git;a=commit;h=7d0c41ef89dad9008edf1c3c0022721ebad39999
The comment before tg3_get_eeprom_hw_cfg() suggests tg3 should
ONLY be touching PCI cfg space until the NIC gets to D0 power state.
But tg3_get_eeprom_hw_cfg() is called before tg3_set_power_state(tp,0).
Is it OK for tg3_get_eeprom_hw_cfg() to read NIC_SRAM_DATA_CFG?
thanks,
grant
^ permalink raw reply
* Re: [PATCH] struct file cleanup : the very large file_ra_state is now allocated only on demand.
From: Nick Piggin @ 2005-08-18 3:00 UTC (permalink / raw)
To: Andi Kleen
Cc: Eric Dumazet, Benjamin LaHaise, David S. Miller, netdev,
linux-kernel
In-Reply-To: <20050818025727.GY3996@wotan.suse.de>
Andi Kleen wrote:
>>You don't want to always have bad performance though, so you
>>could attempt to allocate if either the pointer is null _or_ it
>>points to the global structure?
>>
>
>Remember it's after a GFP_KERNEL OOM. If that fails most likely
>you have deadlocked somewhere else already because Linux's handling
>of that is so bad. Suboptimal readahead somewhere is the smallest
>of your problems.
>
>
True. And in practice it may not even be able to happen at the
moment if the page allocator still doesn't fail small order allocs.
But I guess the dream one day is to robustly handle any OOM :\
Send instant messages to your online friends http://au.messenger.yahoo.com
^ permalink raw reply
* Re: [PATCH] struct file cleanup : the very large file_ra_state is now allocated only on demand.
From: Andi Kleen @ 2005-08-18 2:57 UTC (permalink / raw)
To: Nick Piggin
Cc: Andi Kleen, Eric Dumazet, Benjamin LaHaise, David S. Miller,
netdev, linux-kernel
In-Reply-To: <4303F7E8.5030705@yahoo.com.au>
> You don't want to always have bad performance though, so you
> could attempt to allocate if either the pointer is null _or_ it
> points to the global structure?
Remember it's after a GFP_KERNEL OOM. If that fails most likely
you have deadlocked somewhere else already because Linux's handling
of that is so bad. Suboptimal readahead somewhere is the smallest
of your problems.
-Andi
^ permalink raw reply
* Re: [PATCH] struct file cleanup : the very large file_ra_state is now allocated only on demand.
From: Nick Piggin @ 2005-08-18 2:52 UTC (permalink / raw)
To: Andi Kleen
Cc: Eric Dumazet, Benjamin LaHaise, David S. Miller, netdev,
linux-kernel
In-Reply-To: <20050818010524.GW3996@wotan.suse.de>
Andi Kleen wrote:
>
>I would just set the ra pointer to a single global structure if the allocation
>fails. Then you can avoid all the other checks. It will slow down
>things and trash some state, but not fail and nobody should expect good
>performance after out of memory anyways. The only check still
>needed would be on freeing.
>
>
You don't want to always have bad performance though, so you
could attempt to allocate if either the pointer is null _or_ it
points to the global structure?
Send instant messages to your online friends http://au.messenger.yahoo.com
^ permalink raw reply
* Re: [PATCH] struct file cleanup : the very large file_ra_state is now allocated only on demand.
From: David S. Miller @ 2005-08-18 2:43 UTC (permalink / raw)
To: ak; +Cc: dada1, bcrl, netdev, linux-kernel
In-Reply-To: <20050818010524.GW3996@wotan.suse.de>
From: Andi Kleen <ak@suse.de>
Date: Thu, 18 Aug 2005 03:05:25 +0200
> I would just set the ra pointer to a single global structure if the
> allocation fails. Then you can avoid all the other checks. It will
> slow down things and trash some state, but not fail and nobody
> should expect good performance after out of memory anyways. The only
> check still needed would be on freeing.
I would think twice about that due to repeatability concerns. Yes, we
should care less when memory is so low, but if we can avoid this kind
of scenerio easily we should.
Having said that, I would like to recommend looking into a scheme
where the path leading to the filp allocation states whether the
read-ahead bits are needed or not. This has two benefits:
1) Repeatability, and error signalling at the correct place
should the memory allocation fail.
2) We can avoid the pointer dereference overhead. The read-ahead
state is always at (filp + 1). Macro'ized or static inline
function'ized interfaces for this access can make it look
clean and perhaps even implement debugging of the case where
we try to get at the read-ahead state for a non-read-ahead
filp.
I do really think that would be a better approach. A quick glance
shows that it should be easy to propagate the "need_read_ahead"
state, just by passing a boolean to get_unused_fd() via
sock_map_fd().
^ permalink raw reply
* Re: [patch 2.6.13-rc6] net/802/tr: use interrupt-safe locking
From: John W. Linville @ 2005-08-18 1:55 UTC (permalink / raw)
To: Jay Vosburgh; +Cc: netdev, linux-kernel
In-Reply-To: <200508172148.j7HLmPAt005857@death.nxdomain.ibm.com>
On Wed, Aug 17, 2005 at 02:48:25PM -0700, Jay Vosburgh wrote:
> John W. Linville <linville@tuxdriver.com> wrote:
> >Signed-off-by: Jay Vosburg <foobar@us.ibm.com>
>
> Pretty close.
>
> Signed-off-by: Jay Vosburgh <fubar@us.ibm.com>
Ooops...sorry! Tired, sloppy typing... :-(
> I believe that I originally wrote and posted this patch in the
> appended message; I recall posting it a few times in various places.
That is my understanding as well, which is why I put (an unfortunately
incorrect) S-o-b line for you ahead of mine. Should I have done
something else?
Thanks,
John
--
John W. Linville
linville@tuxdriver.com
^ permalink raw reply
* Re: [PATCH] struct file cleanup : the very large file_ra_state is now allocated only on demand.
From: Coywolf Qi Hunt @ 2005-08-18 1:39 UTC (permalink / raw)
To: Eric Dumazet
Cc: Andi Kleen, Benjamin LaHaise, David S. Miller, netdev,
linux-kernel
In-Reply-To: <4303D90E.2030103@cosmosbay.com>
On 8/18/05, Eric Dumazet <dada1@cosmosbay.com> wrote:
> Andi Kleen a écrit :
>
> >
> >>(because of the insane struct file_ra_state f_ra. I wish this structure
> >>were dynamically allocated only for files that really use it)
> >
> >
> > How about you submit a patch for that instead?
> >
> > -Andi
>
> OK, could you please comment this patch ?
>
> The problem of dynamically allocating the readahead state data is that the allocation can fail and should not be fatal.
> I made some choices that might be not good.
>
> I also chose not to align "file_ra" slab on SLAB_HWCACHE_ALIGN because the object size is 10*sizeof(long), so alignment would loose
> 6*sizeof(long) bytes for each object.
>
>
> [PATCH]
>
> * struct file cleanup : the very large file_ra_state is now allocated only on demand, using a dedicated "file_ra" slab.
> 64bits machines handling lot of sockets can save about 72 bytes per file.
> * private_data : The field is moved close to f_count and f_op fields to speedup sockfd_lookups
Why not keep the comment or fix it?
--
Coywolf Qi Hunt
http://ahbl.org/~coywolf/
^ permalink raw reply
* Re: [PATCH] struct file cleanup : the very large file_ra_state is now allocated only on demand.
From: Andi Kleen @ 2005-08-18 1:05 UTC (permalink / raw)
To: Eric Dumazet
Cc: Andi Kleen, Benjamin LaHaise, David S. Miller, netdev,
linux-kernel
In-Reply-To: <4303D90E.2030103@cosmosbay.com>
On Thu, Aug 18, 2005 at 02:40:46AM +0200, Eric Dumazet wrote:
> Andi Kleen a ?crit :
>
> >
> >>(because of the insane struct file_ra_state f_ra. I wish this structure
> >>were dynamically allocated only for files that really use it)
> >
> >
> >How about you submit a patch for that instead?
> >
> >-Andi
>
> OK, could you please comment this patch ?
I would just set the ra pointer to a single global structure if the allocation
fails. Then you can avoid all the other checks. It will slow down
things and trash some state, but not fail and nobody should expect good
performance after out of memory anyways. The only check still
needed would be on freeing.
-Andi
^ permalink raw reply
* [PATCH] struct file cleanup : the very large file_ra_state is now allocated only on demand.
From: Eric Dumazet @ 2005-08-18 0:40 UTC (permalink / raw)
To: Andi Kleen; +Cc: Benjamin LaHaise, David S. Miller, netdev, linux-kernel
In-Reply-To: <20050817215357.GU3996@wotan.suse.de>
[-- Attachment #1: Type: text/plain, Size: 977 bytes --]
Andi Kleen a écrit :
>
>>(because of the insane struct file_ra_state f_ra. I wish this structure
>>were dynamically allocated only for files that really use it)
>
>
> How about you submit a patch for that instead?
>
> -Andi
OK, could you please comment this patch ?
The problem of dynamically allocating the readahead state data is that the allocation can fail and should not be fatal.
I made some choices that might be not good.
I also chose not to align "file_ra" slab on SLAB_HWCACHE_ALIGN because the object size is 10*sizeof(long), so alignment would loose
6*sizeof(long) bytes for each object.
[PATCH]
* struct file cleanup : the very large file_ra_state is now allocated only on demand, using a dedicated "file_ra" slab.
64bits machines handling lot of sockets can save about 72 bytes per file.
* private_data : The field is moved close to f_count and f_op fields to speedup sockfd_lookups
Thank you
Signed-off-by: Eric Dumazet <dada1@cosmosbay.com>
[-- Attachment #2: readahead.patch --]
[-- Type: text/plain, Size: 7652 bytes --]
--- linux-2.6.13-rc6/include/linux/fs.h 2005-08-07 20:18:56.000000000 +0200
+++ linux-2.6.13-rc6-ed/include/linux/fs.h 2005-08-18 01:33:04.000000000 +0200
@@ -586,20 +586,19 @@
struct dentry *f_dentry;
struct vfsmount *f_vfsmnt;
struct file_operations *f_op;
+ void *private_data;
atomic_t f_count;
unsigned int f_flags;
mode_t f_mode;
loff_t f_pos;
struct fown_struct f_owner;
unsigned int f_uid, f_gid;
- struct file_ra_state f_ra;
+ struct file_ra_state *f_rap;
size_t f_maxcount;
unsigned long f_version;
void *f_security;
- /* needed for tty driver, and maybe others */
- void *private_data;
#ifdef CONFIG_EPOLL
/* Used by fs/eventpoll.c to link all the hooks to this file */
@@ -1514,8 +1513,7 @@
extern void do_generic_mapping_read(struct address_space *mapping,
struct file_ra_state *, struct file *,
loff_t *, read_descriptor_t *, read_actor_t);
-extern void
-file_ra_state_init(struct file_ra_state *ra, struct address_space *mapping);
+extern struct file_ra_state *file_ra_state_init(struct file *);
extern ssize_t generic_file_direct_IO(int rw, struct kiocb *iocb,
const struct iovec *iov, loff_t offset, unsigned long nr_segs);
extern ssize_t generic_file_readv(struct file *filp, const struct iovec *iov,
@@ -1549,8 +1547,10 @@
read_descriptor_t * desc,
read_actor_t actor)
{
+ if (filp->f_rap == NULL)
+ file_ra_state_init(filp);
do_generic_mapping_read(filp->f_mapping,
- &filp->f_ra,
+ filp->f_rap,
filp,
ppos,
desc,
--- linux-2.6.13-rc6/include/linux/slab.h 2005-08-07 20:18:56.000000000 +0200
+++ linux-2.6.13-rc6-ed/include/linux/slab.h 2005-08-18 00:37:59.000000000 +0200
@@ -125,6 +125,7 @@
extern kmem_cache_t *names_cachep;
extern kmem_cache_t *files_cachep;
extern kmem_cache_t *filp_cachep;
+extern kmem_cache_t *ra_cachep;
extern kmem_cache_t *fs_cachep;
extern kmem_cache_t *signal_cachep;
extern kmem_cache_t *sighand_cachep;
--- linux-2.6.13-rc6/mm/readahead.c 2005-08-07 20:18:56.000000000 +0200
+++ linux-2.6.13-rc6-ed/mm/readahead.c 2005-08-18 01:14:11.000000000 +0200
@@ -29,14 +29,20 @@
EXPORT_SYMBOL_GPL(default_backing_dev_info);
/*
- * Initialise a struct file's readahead state. Assumes that the caller has
- * memset *ra to zero.
+ * Initialise a struct file's readahead state.
*/
-void
-file_ra_state_init(struct file_ra_state *ra, struct address_space *mapping)
+struct file_ra_state *
+file_ra_state_init(struct file *file)
{
- ra->ra_pages = mapping->backing_dev_info->ra_pages;
- ra->prev_page = -1;
+ struct file_ra_state *ra = kmem_cache_alloc(ra_cachep, GFP_KERNEL);
+
+ if (ra != NULL) {
+ file->f_rap = ra;
+ memset(ra, 0, sizeof(*ra));
+ ra->ra_pages = file->f_mapping->host->i_mapping->backing_dev_info->ra_pages;
+ ra->prev_page = -1;
+ }
+ return ra;
}
/*
--- linux-2.6.13-rc6/mm/filemap.c 2005-08-07 20:18:56.000000000 +0200
+++ linux-2.6.13-rc6-ed/mm/filemap.c 2005-08-18 01:33:58.000000000 +0200
@@ -711,7 +711,7 @@
* NULL.
*/
void do_generic_mapping_read(struct address_space *mapping,
- struct file_ra_state *_ra,
+ struct file_ra_state *_rap,
struct file *filp,
loff_t *ppos,
read_descriptor_t *desc,
@@ -727,8 +727,15 @@
loff_t isize;
struct page *cached_page;
int error;
- struct file_ra_state ra = *_ra;
+ struct file_ra_state ra;
+ if (likely(_rap != NULL))
+ ra = *_rap;
+ else {
+ memset(&ra, 0, sizeof(ra));
+ ra.prev_page = -1;
+ ra.ra_pages = default_backing_dev_info.ra_pages;
+ }
cached_page = NULL;
index = *ppos >> PAGE_CACHE_SHIFT;
next_index = index;
@@ -908,7 +915,8 @@
}
out:
- *_ra = ra;
+ if (_rap != NULL)
+ *_rap = ra;
*ppos = ((loff_t) index << PAGE_CACHE_SHIFT) + offset;
if (cached_page)
@@ -1187,12 +1195,15 @@
int error;
struct file *file = area->vm_file;
struct address_space *mapping = file->f_mapping;
- struct file_ra_state *ra = &file->f_ra;
+ struct file_ra_state *ra = file->f_rap;
struct inode *inode = mapping->host;
struct page *page;
unsigned long size, pgoff;
int did_readaround = 0, majmin = VM_FAULT_MINOR;
+ if (ra == NULL)
+ ra = file_ra_state_init(file);
+
pgoff = ((address-area->vm_start) >> PAGE_CACHE_SHIFT) + area->vm_pgoff;
retry_all:
@@ -1225,14 +1236,16 @@
handle_ra_miss(mapping, ra, pgoff);
goto no_cached_page;
}
- ra->mmap_miss++;
+ if (ra != NULL) {
+ ra->mmap_miss++;
- /*
- * Do we miss much more than hit in this file? If so,
- * stop bothering with read-ahead. It will only hurt.
- */
- if (ra->mmap_miss > ra->mmap_hit + MMAP_LOTSAMISS)
- goto no_cached_page;
+ /*
+ * Do we miss much more than hit in this file? If so,
+ * stop bothering with read-ahead. It will only hurt.
+ */
+ if (ra->mmap_miss > ra->mmap_hit + MMAP_LOTSAMISS)
+ goto no_cached_page;
+ }
/*
* To keep the pgmajfault counter straight, we need to
@@ -1243,7 +1256,7 @@
inc_page_state(pgmajfault);
}
did_readaround = 1;
- ra_pages = max_sane_readahead(file->f_ra.ra_pages);
+ ra_pages = max_sane_readahead(ra != NULL ? ra->ra_pages : default_backing_dev_info.ra_pages);
if (ra_pages) {
pgoff_t start = 0;
@@ -1256,7 +1269,7 @@
goto no_cached_page;
}
- if (!did_readaround)
+ if (!did_readaround && ra != NULL)
ra->mmap_hit++;
/*
--- linux-2.6.13-rc6/mm/fadvise.c 2005-08-07 20:18:56.000000000 +0200
+++ linux-2.6.13-rc6-ed/mm/fadvise.c 2005-08-18 01:05:15.000000000 +0200
@@ -28,6 +28,7 @@
struct file *file = fget(fd);
struct address_space *mapping;
struct backing_dev_info *bdi;
+ struct file_ra_state *ra;
loff_t endbyte;
pgoff_t start_index;
pgoff_t end_index;
@@ -54,15 +55,20 @@
bdi = mapping->backing_dev_info;
+ if ((ra = file->f_rap) == NULL)
+ ra = file_ra_state_init(file);
switch (advice) {
case POSIX_FADV_NORMAL:
- file->f_ra.ra_pages = bdi->ra_pages;
+ if (ra != NULL)
+ ra->ra_pages = bdi->ra_pages;
break;
case POSIX_FADV_RANDOM:
- file->f_ra.ra_pages = 0;
+ if (ra != NULL)
+ ra->ra_pages = 0;
break;
case POSIX_FADV_SEQUENTIAL:
- file->f_ra.ra_pages = bdi->ra_pages * 2;
+ if (ra != NULL)
+ ra->ra_pages = bdi->ra_pages * 2;
break;
case POSIX_FADV_WILLNEED:
case POSIX_FADV_NOREUSE:
--- linux-2.6.13-rc6/fs/open.c 2005-08-07 20:18:56.000000000 +0200
+++ linux-2.6.13-rc6-ed/fs/open.c 2005-08-18 01:05:15.000000000 +0200
@@ -804,7 +804,7 @@
}
f->f_flags &= ~(O_CREAT | O_EXCL | O_NOCTTY | O_TRUNC);
- file_ra_state_init(&f->f_ra, f->f_mapping->host->i_mapping);
+ f->f_rap = NULL;
/* NB: we're sure to have correct a_ops only after f_op->open */
if (f->f_flags & O_DIRECT) {
--- linux-2.6.13-rc6/fs/file_table.c 2005-08-07 20:18:56.000000000 +0200
+++ linux-2.6.13-rc6-ed/fs/file_table.c 2005-08-18 00:37:59.000000000 +0200
@@ -55,6 +55,8 @@
static inline void file_free(struct file *f)
{
+ if (f->f_rap != NULL)
+ kmem_cache_free(ra_cachep, f->f_rap);
kmem_cache_free(filp_cachep, f);
}
--- linux-2.6.13-rc6/fs/dcache.c 2005-08-07 20:18:56.000000000 +0200
+++ linux-2.6.13-rc6-ed/fs/dcache.c 2005-08-18 02:22:56.000000000 +0200
@@ -1705,6 +1705,8 @@
/* SLAB cache for file structures */
kmem_cache_t *filp_cachep;
+/* SLAB cache for ra structures */
+kmem_cache_t *ra_cachep;
EXPORT_SYMBOL(d_genocide);
@@ -1733,6 +1735,9 @@
filp_cachep = kmem_cache_create("filp", sizeof(struct file), 0,
SLAB_HWCACHE_ALIGN|SLAB_PANIC, filp_ctor, filp_dtor);
+ ra_cachep = kmem_cache_create("file_ra", sizeof(struct file_ra_state), 0,
+ SLAB_PANIC, NULL, NULL);
+
dcache_init(mempages);
inode_init(mempages);
files_init(mempages);
^ permalink raw reply
* Re: [patch 2.6.13-rc6] net/802/tr: use interrupt-safe locking
From: David S. Miller @ 2005-08-17 22:00 UTC (permalink / raw)
To: linville; +Cc: netdev, linux-tr, mikep, jgarzik, fubar, linux-kernel
In-Reply-To: <20050817204959.GA20186@tuxdriver.com>
From: "John W. Linville" <linville@tuxdriver.com>
Date: Wed, 17 Aug 2005 16:49:59 -0400
> Change operations on rif_lock from spin_{un}lock_bh to
> spin_{un}lock_irq{save,restore} equivalents. Some of the
> rif_lock critical sections are called from interrupt context via
> tr_type_trans->tr_add_rif_info. The TR NIC drivers call tr_type_trans
> from their packet receive handlers.
Applied, I'll try to get this into 2.6.13, but it may have
to wait for 2.6.14
^ permalink raw reply
* Re: [patch 2.6.13-rc6] net/802/tr: use interrupt-safe locking
From: Jay Vosburgh @ 2005-08-17 21:48 UTC (permalink / raw)
To: netdev, linux-tr, mikep, jgarzik, davem, linux-kernel
In-Reply-To: <20050817204959.GA20186@tuxdriver.com>
John W. Linville <linville@tuxdriver.com> wrote:
>Change operations on rif_lock from spin_{un}lock_bh to
>spin_{un}lock_irq{save,restore} equivalents. Some of the
>rif_lock critical sections are called from interrupt context via
>tr_type_trans->tr_add_rif_info. The TR NIC drivers call tr_type_trans
>from their packet receive handlers.
>
>Signed-off-by: Jay Vosburg <foobar@us.ibm.com>
Pretty close.
Signed-off-by: Jay Vosburgh <fubar@us.ibm.com>
>It is my understanding that this same patch has been submitted multiple
>times in the past. Some of those submissions were around a year ago,
>but it does not seem to have been committed.
I believe that I originally wrote and posted this patch in the
appended message; I recall posting it a few times in various places.
>FWIW, this patch is currently being carried in the Fedora and RHEL
>kernels. It certainly looks like it is necessary to me. Can we get
>some movement on this?
It's in the SuSE kernel as well.
-J
---
-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com
To: Paul Mackerras <paulus@samba.org>
Cc: linux-tr@linuxtr.net, netdev@oss.sgi.com
Subject: Re: spin_lock_bh() called in irq handler
Date: Wed, 28 Apr 2004 10:55:33 -0700
From: Jay Vosburgh <fubar@us.ibm.com>
>I had a look and found that all of the token-ring drivers call
>tr_type_trans() at interrupt level. That seems perfectly reasonable
>to me. To fix the bug, it seems to me that there are two options:
>either move the tr_add_rif_info() call elsewhere (but I have no idea
>where) or else use spin_lock_irqsave instead of spin_lock_bh.
>
>Which is the more appropriate fix?
I'm guessing spin_lock_irqsave; would the following be
appropriate? I'm not absolutely sure about using spin_(un)lock_irq in
rif_seq_start/stop, but it'd be complicated to deal with the flags in
that case.
I've built this and given it some basic testing, but not really
hammered on it. The system doesn't panic when I cat /proc/net/tr_rif,
which is a good sign.
-J
---
-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com
diff -urN linux-2.6.5-orig/net/802/tr.c linux-2.6.5/net/802/tr.c
--- linux-2.6.5-orig/net/802/tr.c 2004-04-28 10:02:12.000000000 -0700
+++ linux-2.6.5/net/802/tr.c 2004-04-28 10:15:47.000000000 -0700
@@ -250,10 +250,11 @@
unsigned int hash;
struct rif_cache_s *entry;
unsigned char *olddata;
+ unsigned long flags;
static const unsigned char mcast_func_addr[]
= {0xC0,0x00,0x00,0x04,0x00,0x00};
- spin_lock_bh(&rif_lock);
+ spin_lock_irqsave(&rif_lock, flags);
/*
* Broadcasts are single route as stated in RFC 1042
@@ -322,7 +323,7 @@
else
slack = 18 - ((ntohs(trh->rcf) & TR_RCF_LEN_MASK)>>8);
olddata = skb->data;
- spin_unlock_bh(&rif_lock);
+ spin_unlock_irqrestore(&rif_lock, flags);
skb_pull(skb, slack);
memmove(skb->data, olddata, sizeof(struct trh_hdr) - slack);
@@ -336,10 +337,11 @@
static void tr_add_rif_info(struct trh_hdr *trh, struct net_device *dev)
{
unsigned int hash, rii_p = 0;
+ unsigned long flags;
struct rif_cache_s *entry;
- spin_lock_bh(&rif_lock);
+ spin_lock_irqsave(&rif_lock, flags);
/*
* Firstly see if the entry exists
@@ -377,7 +379,7 @@
if(!entry)
{
printk(KERN_DEBUG "tr.c: Couldn't malloc rif cache entry !\n");
- spin_unlock_bh(&rif_lock);
+ spin_unlock_irqrestore(&rif_lock, flags);
return;
}
@@ -419,7 +421,7 @@
}
entry->last_used=jiffies;
}
- spin_unlock_bh(&rif_lock);
+ spin_unlock_irqrestore(&rif_lock, flags);
}
/*
@@ -429,9 +431,9 @@
static void rif_check_expire(unsigned long dummy)
{
int i;
- unsigned long next_interval = jiffies + sysctl_tr_rif_timeout/2;
+ unsigned long flags, next_interval = jiffies + sysctl_tr_rif_timeout/2;
- spin_lock_bh(&rif_lock);
+ spin_lock_irqsave(&rif_lock, flags);
for(i =0; i < RIF_TABLE_SIZE; i++) {
struct rif_cache_s *entry, **pentry;
@@ -453,7 +455,7 @@
}
}
- spin_unlock_bh(&rif_lock);
+ spin_unlock_irqrestore(&rif_lock, flags);
mod_timer(&rif_timer, next_interval);
@@ -484,7 +486,7 @@
static void *rif_seq_start(struct seq_file *seq, loff_t *pos)
{
- spin_lock_bh(&rif_lock);
+ spin_lock_irq(&rif_lock);
return *pos ? rif_get_idx(*pos - 1) : SEQ_START_TOKEN;
}
@@ -515,7 +517,7 @@
static void rif_seq_stop(struct seq_file *seq, void *v)
{
- spin_unlock_bh(&rif_lock);
+ spin_unlock_irq(&rif_lock);
}
static int rif_seq_show(struct seq_file *seq, void *v)
-J
---
-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com
^ permalink raw reply
* [patch 2.6.13-rc6] net/802/tr: use interrupt-safe locking
From: John W. Linville @ 2005-08-17 20:49 UTC (permalink / raw)
To: netdev, linux-tr, mikep; +Cc: jgarzik, davem, fubar, linux-kernel
Change operations on rif_lock from spin_{un}lock_bh to
spin_{un}lock_irq{save,restore} equivalents. Some of the
rif_lock critical sections are called from interrupt context via
tr_type_trans->tr_add_rif_info. The TR NIC drivers call tr_type_trans
from their packet receive handlers.
Signed-off-by: Jay Vosburg <foobar@us.ibm.com>
Signed-off-by: John W. Linville <linville@tuxdriver.com>
---
It is my understanding that this same patch has been submitted multiple
times in the past. Some of those submissions were around a year ago,
but it does not seem to have been committed.
FWIW, this patch is currently being carried in the Fedora and RHEL
kernels. It certainly looks like it is necessary to me. Can we get
some movement on this?
net/802/tr.c | 22 ++++++++++++----------
1 files changed, 12 insertions(+), 10 deletions(-)
diff --git a/net/802/tr.c b/net/802/tr.c
--- a/net/802/tr.c
+++ b/net/802/tr.c
@@ -251,10 +251,11 @@ void tr_source_route(struct sk_buff *skb
unsigned int hash;
struct rif_cache *entry;
unsigned char *olddata;
+ unsigned long flags;
static const unsigned char mcast_func_addr[]
= {0xC0,0x00,0x00,0x04,0x00,0x00};
- spin_lock_bh(&rif_lock);
+ spin_lock_irqsave(&rif_lock, flags);
/*
* Broadcasts are single route as stated in RFC 1042
@@ -323,7 +324,7 @@ printk("source routing for %02X:%02X:%02
else
slack = 18 - ((ntohs(trh->rcf) & TR_RCF_LEN_MASK)>>8);
olddata = skb->data;
- spin_unlock_bh(&rif_lock);
+ spin_unlock_irqrestore(&rif_lock, flags);
skb_pull(skb, slack);
memmove(skb->data, olddata, sizeof(struct trh_hdr) - slack);
@@ -337,10 +338,11 @@ printk("source routing for %02X:%02X:%02
static void tr_add_rif_info(struct trh_hdr *trh, struct net_device *dev)
{
unsigned int hash, rii_p = 0;
+ unsigned long flags;
struct rif_cache *entry;
- spin_lock_bh(&rif_lock);
+ spin_lock_irqsave(&rif_lock, flags);
/*
* Firstly see if the entry exists
@@ -378,7 +380,7 @@ printk("adding rif_entry: addr:%02X:%02X
if(!entry)
{
printk(KERN_DEBUG "tr.c: Couldn't malloc rif cache entry !\n");
- spin_unlock_bh(&rif_lock);
+ spin_unlock_irqrestore(&rif_lock, flags);
return;
}
@@ -420,7 +422,7 @@ printk("updating rif_entry: addr:%02X:%0
}
entry->last_used=jiffies;
}
- spin_unlock_bh(&rif_lock);
+ spin_unlock_irqrestore(&rif_lock, flags);
}
/*
@@ -430,9 +432,9 @@ printk("updating rif_entry: addr:%02X:%0
static void rif_check_expire(unsigned long dummy)
{
int i;
- unsigned long next_interval = jiffies + sysctl_tr_rif_timeout/2;
+ unsigned long flags, next_interval = jiffies + sysctl_tr_rif_timeout/2;
- spin_lock_bh(&rif_lock);
+ spin_lock_irqsave(&rif_lock, flags);
for(i =0; i < RIF_TABLE_SIZE; i++) {
struct rif_cache *entry, **pentry;
@@ -454,7 +456,7 @@ static void rif_check_expire(unsigned lo
}
}
- spin_unlock_bh(&rif_lock);
+ spin_unlock_irqrestore(&rif_lock, flags);
mod_timer(&rif_timer, next_interval);
@@ -485,7 +487,7 @@ static struct rif_cache *rif_get_idx(lof
static void *rif_seq_start(struct seq_file *seq, loff_t *pos)
{
- spin_lock_bh(&rif_lock);
+ spin_lock_irq(&rif_lock);
return *pos ? rif_get_idx(*pos - 1) : SEQ_START_TOKEN;
}
@@ -516,7 +518,7 @@ static void *rif_seq_next(struct seq_fil
static void rif_seq_stop(struct seq_file *seq, void *v)
{
- spin_unlock_bh(&rif_lock);
+ spin_unlock_irq(&rif_lock);
}
static int rif_seq_show(struct seq_file *seq, void *v)
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox