Netdev List
 help / color / mirror / Atom feed
* Re: troubles with r8169
From: Alistair John Strachan @ 2007-08-12 20:38 UTC (permalink / raw)
  To: Vadim Dyadkin; +Cc: Robert Hancock, netdev, linux-kernel
In-Reply-To: <46BF6853.4020800@lns.pnpi.spb.ru>

On Sunday 12 August 2007 21:06:43 Vadim Dyadkin wrote:
> Robert Hancock пишет:
> > This could well be a problem with the nvidia driver as it shares the
> > same IRQ. The first step would be to see if the problem still shows up
> > without the nvidia binary module loaded.
>
> Thank for your answer. This problem never shows up if I use only nvidia
> driver (the work without network), or if I use only r8169 (without
> x.org). If I use both of them I have the hang. Usually the hang appears
> if OpenGL is used or network rate is maximal. I tested this regimes many
> times before.

If you haven't already, I'd drop linux-bugs@nvidia.com an email to see if they 
have any insight. Not that this can't be a kernel bug, just that it's a bit 
difficult for kernel developers to debug when you're using a driver for which 
we don't have the sources.

As for changing the IRQ assignments, I don't have any immediate suggestions. I 
notice that this laptop has a dual core processor, so I'm guessing disabling 
APIC isn't an option. Have you tried that anyway, just to see if the IRQ 
assignment differs?

-- 
Cheers,
Alistair.

137/1 Warrender Park Road, Edinburgh, UK.

^ permalink raw reply

* Re: troubles with r8169
From: Vadim Dyadkin @ 2007-08-12 20:58 UTC (permalink / raw)
  To: Alistair John Strachan; +Cc: Robert Hancock, netdev, linux-kernel
In-Reply-To: <200708122138.58405.alistair@devzero.co.uk>

Alistair John Strachan пишет:
>> Thank for your answer. This problem never shows up if I use only nvidia
>> driver (the work without network), or if I use only r8169 (without
>> x.org). If I use both of them I have the hang. Usually the hang appears
>> if OpenGL is used or network rate is maximal. I tested this regimes many
>> times before.
>
> If you haven't already, I'd drop linux-bugs@nvidia.com an email to see if they 
> have any insight. 

I haven't done it yet. I'm going to do it.

> Not that this can't be a kernel bug, just that it's a bit 
> difficult for kernel developers to debug when you're using a driver for which 
> we don't have the sources.
>
> As for changing the IRQ assignments, I don't have any immediate suggestions. I 
> notice that this laptop has a dual core processor, so I'm guessing disabling 
> APIC isn't an option. Have you tried that anyway, just to see if the IRQ 
> assignment differs?

If I've correctly understood your suggestion I should load kernel with
'noapic' option. In that case the situation even worse:
******************************
           CPU0       CPU1
  0:     106543      11922    XT-PIC-XT        timer
  1:         53          8    XT-PIC-XT        i8042
  2:          0          0    XT-PIC-XT        cascade
  5:          0          0    XT-PIC-XT        sdhci:slot0
  7:      14250     126092    XT-PIC-XT        ehci_hcd:usb2
  8:          2          0    XT-PIC-XT        rtc
  9:       1168        190    XT-PIC-XT        acpi
 10:        338        181    XT-PIC-XT        HDA Intel
 11:       7167       1052    XT-PIC-XT        ohci_hcd:usb1, yenta,
eth0, nvidia
 12:        123          9    XT-PIC-XT        i8042
 14:         19         32    XT-PIC-XT        ide0
 15:      10936       1112    XT-PIC-XT        ide1
NMI:          0          0
LOC:      11921     106401
ERR:      41370
MIS:          0
*******************************
Now, pcmcia and usb 1.1 share the same IRQ.

Plus, dmesg says:
*********************************
irq 7: nobody cared (try booting with the "irqpoll" option)
 [<c013f9c4>] __report_bad_irq+0x24/0x80
 [<c013fc46>] note_interrupt+0x226/0x260
 [<f885d0bb>] usb_hcd_irq+0x2b/0x60 [usbcore]
 [<c013eed5>] handle_IRQ_event+0x25/0x50
 [<c01407f3>] handle_level_irq+0x93/0x100
 [<c010529b>] do_IRQ+0x3b/0x70
 [<c010293e>] sys_sigreturn+0xce/0xe0
 [<c010341b>] common_interrupt+0x23/0x28
 =======================
handlers:
[<f885d090>] (usb_hcd_irq+0x0/0x60 [usbcore])
Disabling IRQ #7
******************************

Vadim


^ permalink raw reply

* dm9000: add set_mac_address()
From: Michael Trimarchi @ 2007-08-12 19:27 UTC (permalink / raw)
  To: jgarzi; +Cc: netdev, trimarchi

Implement set_mac_address() for the dm9000 driver.  This allows changing
the mac address of the interface.

Signed-off-by: Michael Trimarchi <trimarchi@gandalf.sssup.it>
---

--- linux-2.6.22/drivers/net/dm9000.c.orig	2007-07-09 01:32:17.000000000 +0200
+++ linux-2.6.22/drivers/net/dm9000.c	2007-08-12 21:42:50.000000000 +0200
@@ -161,6 +161,8 @@ static irqreturn_t dm9000_interrupt(int,
 static int dm9000_phy_read(struct net_device *dev, int phyaddr_unsused, int reg);
 static void dm9000_phy_write(struct net_device *dev, int phyaddr_unused, int reg,
 			   int value);
+static int dm9000_eth_set_mac_address(struct net_device *dev, void *p);
+
 static u16 read_srom_word(board_info_t *, int);
 static void dm9000_rx(struct net_device *);
 static void dm9000_hash_table(struct net_device *);
@@ -546,6 +548,7 @@ dm9000_probe(struct platform_device *pde
 	ndev->stop		 = &dm9000_stop;
 	ndev->get_stats		 = &dm9000_get_stats;
 	ndev->set_multicast_list = &dm9000_hash_table;
+	ndev->set_mac_address	 = &dm9000_eth_set_mac_address;
 #ifdef CONFIG_NET_POLL_CONTROLLER
 	ndev->poll_controller	 = &dm9000_poll_controller;
 #endif
@@ -986,7 +989,6 @@ read_srom_word(board_info_t * db, int of
 	return (ior(db, DM9000_EPDRL) + (ior(db, DM9000_EPDRH) << 8));
 }
 
-#ifdef DM9000_PROGRAM_EEPROM
 /*
  * Write a word data to SROM
  */
@@ -1002,6 +1004,34 @@ write_srom_word(board_info_t * db, int o
 }
 
 /*
+ * dm9000_eth_set_mac_address
+ *
+ * Change the interface's mac address.
+ *
+ */
+static int 
+dm9000_eth_set_mac_address(struct net_device *dev, void *p)
+{
+	board_info_t *db = (board_info_t *) dev->priv;
+	struct sockaddr *addr = p;
+	int i;
+
+	if (!is_valid_ether_addr(addr->sa_data))
+		return -EADDRNOTAVAIL;
+
+	if (netif_running(dev))
+		return -EBUSY;
+
+	memcpy(dev->dev_addr, addr->sa_data, dev->addr_len);
+
+	for (i = 0; i < 3; i++)
+		write_srom_word(db, i, ((u16 *)(addr->sa_data))[i]);
+
+	return 0;
+}
+
+#ifdef DM9000_PROGRAM_EEPROM
+/*
  * Only for development:
  * Here we write static data to the eeprom in case
  * we don't have valid content on a new board

^ permalink raw reply

* Re: dm9000: add set_mac_address()
From: Michael Buesch @ 2007-08-12 21:38 UTC (permalink / raw)
  To: Michael Trimarchi; +Cc: jgarzi, netdev
In-Reply-To: <20070812192741.GB27490@gandalf.sssup.it>

On Sunday 12 August 2007 21:27:41 Michael Trimarchi wrote:
> Implement set_mac_address() for the dm9000 driver.  This allows changing
> the mac address of the interface.
> 
> Signed-off-by: Michael Trimarchi <trimarchi@gandalf.sssup.it>
> ---
> 
> --- linux-2.6.22/drivers/net/dm9000.c.orig	2007-07-09 01:32:17.000000000 +0200
> +++ linux-2.6.22/drivers/net/dm9000.c	2007-08-12 21:42:50.000000000 +0200
> @@ -161,6 +161,8 @@ static irqreturn_t dm9000_interrupt(int,
>  static int dm9000_phy_read(struct net_device *dev, int phyaddr_unsused, int reg);
>  static void dm9000_phy_write(struct net_device *dev, int phyaddr_unused, int reg,
>  			   int value);
> +static int dm9000_eth_set_mac_address(struct net_device *dev, void *p);
> +
>  static u16 read_srom_word(board_info_t *, int);
>  static void dm9000_rx(struct net_device *);
>  static void dm9000_hash_table(struct net_device *);
> @@ -546,6 +548,7 @@ dm9000_probe(struct platform_device *pde
>  	ndev->stop		 = &dm9000_stop;
>  	ndev->get_stats		 = &dm9000_get_stats;
>  	ndev->set_multicast_list = &dm9000_hash_table;
> +	ndev->set_mac_address	 = &dm9000_eth_set_mac_address;
>  #ifdef CONFIG_NET_POLL_CONTROLLER
>  	ndev->poll_controller	 = &dm9000_poll_controller;
>  #endif
> @@ -986,7 +989,6 @@ read_srom_word(board_info_t * db, int of
>  	return (ior(db, DM9000_EPDRL) + (ior(db, DM9000_EPDRH) << 8));
>  }
>  
> -#ifdef DM9000_PROGRAM_EEPROM
>  /*
>   * Write a word data to SROM
>   */
> @@ -1002,6 +1004,34 @@ write_srom_word(board_info_t * db, int o
>  }
>  
>  /*
> + * dm9000_eth_set_mac_address
> + *
> + * Change the interface's mac address.
> + *
> + */
> +static int 
> +dm9000_eth_set_mac_address(struct net_device *dev, void *p)
> +{
> +	board_info_t *db = (board_info_t *) dev->priv;
> +	struct sockaddr *addr = p;
> +	int i;
> +
> +	if (!is_valid_ether_addr(addr->sa_data))
> +		return -EADDRNOTAVAIL;
> +
> +	if (netif_running(dev))
> +		return -EBUSY;
> +
> +	memcpy(dev->dev_addr, addr->sa_data, dev->addr_len);
> +
> +	for (i = 0; i < 3; i++)
> +		write_srom_word(db, i, ((u16 *)(addr->sa_data))[i]);

That is broken on BigEndian architectures.

-- 
Greetings Michael.

^ permalink raw reply

* Re: dm9000: add set_mac_address()
From: Michael Buesch @ 2007-08-12 21:45 UTC (permalink / raw)
  To: Michael Trimarchi; +Cc: jgarzik, netdev
In-Reply-To: <200708122338.08523.mb@bu3sch.de>

On Sunday 12 August 2007 23:38:08 Michael Buesch wrote:
> On Sunday 12 August 2007 21:27:41 Michael Trimarchi wrote:
> > Implement set_mac_address() for the dm9000 driver.  This allows changing
> > the mac address of the interface.
> > 
> > Signed-off-by: Michael Trimarchi <trimarchi@gandalf.sssup.it>
> > ---
> > 
> > --- linux-2.6.22/drivers/net/dm9000.c.orig	2007-07-09 01:32:17.000000000 +0200
> > +++ linux-2.6.22/drivers/net/dm9000.c	2007-08-12 21:42:50.000000000 +0200
> > @@ -161,6 +161,8 @@ static irqreturn_t dm9000_interrupt(int,
> >  static int dm9000_phy_read(struct net_device *dev, int phyaddr_unsused, int reg);
> >  static void dm9000_phy_write(struct net_device *dev, int phyaddr_unused, int reg,
> >  			   int value);
> > +static int dm9000_eth_set_mac_address(struct net_device *dev, void *p);
> > +
> >  static u16 read_srom_word(board_info_t *, int);
> >  static void dm9000_rx(struct net_device *);
> >  static void dm9000_hash_table(struct net_device *);
> > @@ -546,6 +548,7 @@ dm9000_probe(struct platform_device *pde
> >  	ndev->stop		 = &dm9000_stop;
> >  	ndev->get_stats		 = &dm9000_get_stats;
> >  	ndev->set_multicast_list = &dm9000_hash_table;
> > +	ndev->set_mac_address	 = &dm9000_eth_set_mac_address;
> >  #ifdef CONFIG_NET_POLL_CONTROLLER
> >  	ndev->poll_controller	 = &dm9000_poll_controller;
> >  #endif
> > @@ -986,7 +989,6 @@ read_srom_word(board_info_t * db, int of
> >  	return (ior(db, DM9000_EPDRL) + (ior(db, DM9000_EPDRH) << 8));
> >  }
> >  
> > -#ifdef DM9000_PROGRAM_EEPROM
> >  /*
> >   * Write a word data to SROM
> >   */
> > @@ -1002,6 +1004,34 @@ write_srom_word(board_info_t * db, int o
> >  }
> >  
> >  /*
> > + * dm9000_eth_set_mac_address
> > + *
> > + * Change the interface's mac address.
> > + *
> > + */
> > +static int 
> > +dm9000_eth_set_mac_address(struct net_device *dev, void *p)
> > +{
> > +	board_info_t *db = (board_info_t *) dev->priv;
> > +	struct sockaddr *addr = p;
> > +	int i;
> > +
> > +	if (!is_valid_ether_addr(addr->sa_data))
> > +		return -EADDRNOTAVAIL;
> > +
> > +	if (netif_running(dev))
> > +		return -EBUSY;
> > +
> > +	memcpy(dev->dev_addr, addr->sa_data, dev->addr_len);
> > +
> > +	for (i = 0; i < 3; i++)
> > +		write_srom_word(db, i, ((u16 *)(addr->sa_data))[i]);
> 
> That is broken on BigEndian architectures.
> 

And is it really desired to change the mac in the ROM here?
I'd say this should only write the mac to the device registers
(filters and so on).

-- 
Greetings Michael.

^ permalink raw reply

* Re: dm9000: add set_mac_address()
From: michael trimarchi @ 2007-08-12 20:54 UTC (permalink / raw)
  To: Michael Buesch; +Cc: jgarzik, netdev
In-Reply-To: <200708122345.30386.mb@bu3sch.de>

On Sun, Aug 12, 2007 at 11:45:30PM +0200, Michael Buesch wrote:
> On Sunday 12 August 2007 23:38:08 Michael Buesch wrote:
> > On Sunday 12 August 2007 21:27:41 Michael Trimarchi wrote:
> > > Implement set_mac_address() for the dm9000 driver.  This allows changing
> > > the mac address of the interface.
[...]
> > 
> > That is broken on BigEndian architectures.
ok. I'll check it.
> > 
> 
> And is it really desired to change the mac in the ROM here?
> I'd say this should only write the mac to the device registers
> (filters and so on).
Many drivers do that in this way. You can change the MAC address for example
after flashing an arm board using ifconfig eth0 hw <mac address>. 
> 
> -- 
> Greetings Michael.
Cheers

^ permalink raw reply

* dm9000: add set_mac_address() v2
From: michael trimarchi @ 2007-08-12 21:20 UTC (permalink / raw)
  To: Michael Buesch; +Cc: jgarzi, netdev
In-Reply-To: <200708122338.08523.mb@bu3sch.de>

Implement set_mac_address() for the dm9000 driver.  This allows changing
the mac address of the interface. Fix BigEndian problem.
 
Signed-off-by: Michael Trimarchi <trimarchi@gandalf.sssup.it>
---

--- linux-2.6.22/drivers/net/dm9000.c.orig	2007-07-09 01:32:17.000000000 +0200
+++ linux-2.6.22/drivers/net/dm9000.c	2007-08-13 00:15:38.000000000 +0200
@@ -161,6 +161,8 @@ static irqreturn_t dm9000_interrupt(int,
 static int dm9000_phy_read(struct net_device *dev, int phyaddr_unsused, int reg);
 static void dm9000_phy_write(struct net_device *dev, int phyaddr_unused, int reg,
 			   int value);
+static int dm9000_eth_set_mac_address(struct net_device *dev, void *p);
+
 static u16 read_srom_word(board_info_t *, int);
 static void dm9000_rx(struct net_device *);
 static void dm9000_hash_table(struct net_device *);
@@ -546,6 +548,7 @@ dm9000_probe(struct platform_device *pde
 	ndev->stop		 = &dm9000_stop;
 	ndev->get_stats		 = &dm9000_get_stats;
 	ndev->set_multicast_list = &dm9000_hash_table;
+	ndev->set_mac_address	 = &dm9000_eth_set_mac_address;
 #ifdef CONFIG_NET_POLL_CONTROLLER
 	ndev->poll_controller	 = &dm9000_poll_controller;
 #endif
@@ -986,7 +989,6 @@ read_srom_word(board_info_t * db, int of
 	return (ior(db, DM9000_EPDRL) + (ior(db, DM9000_EPDRH) << 8));
 }
 
-#ifdef DM9000_PROGRAM_EEPROM
 /*
  * Write a word data to SROM
  */
@@ -1002,6 +1004,35 @@ write_srom_word(board_info_t * db, int o
 }
 
 /*
+ * dm9000_eth_set_mac_address
+ *
+ * Change the interface's mac address.
+ *
+ */
+static int 
+dm9000_eth_set_mac_address(struct net_device *dev, void *p)
+{
+	board_info_t *db = (board_info_t *) dev->priv;
+	struct sockaddr *addr = p;
+	int i;
+
+	if (!is_valid_ether_addr(addr->sa_data))
+		return -EADDRNOTAVAIL;
+
+	if (netif_running(dev))
+		return -EBUSY;
+
+	memcpy(dev->dev_addr, addr->sa_data, dev->addr_len);
+
+	for (i = 0; i < 3; i++)
+		write_srom_word(db, i,
+				cpu_to_le16(((u16 *) (addr->sa_data))[i]));
+
+	return 0;
+}
+
+#ifdef DM9000_PROGRAM_EEPROM
+/*
  * Only for development:
  * Here we write static data to the eeprom in case
  * we don't have valid content on a new board

^ permalink raw reply

* [PATCH 6/6][RESEND] Avoid possible NULL pointer deref in 3c359 driver
From: Jesper Juhl @ 2007-08-12 22:22 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Linux Kernel Mailing List, Mike Phillips, netdev, linux-tr, davem,
	Jesper Juhl
In-Reply-To: <200708130016.11281.jesper.juhl@gmail.com>

(Resending old patch originally submitted at 1/7-2007 02:19, 04-Aug-2007 20:31)

In xl_freemem(), if dev_if is NULL, the line
  struct xl_private *xl_priv =(struct xl_private *)dev->priv;
will cause a NULL pointer dereference. However, if we move
that assignment below the 'if' statement that tests for a NULL
'dev', then that NULL deref can never happen.
It never hurts to be safe :-)


Signed-off-by: Jesper Juhl <jesper.juhl@gmail.com>
---

diff --git a/drivers/net/tokenring/3c359.c b/drivers/net/tokenring/3c359.c
index e22a3f5..671f4da 100644
--- a/drivers/net/tokenring/3c359.c
+++ b/drivers/net/tokenring/3c359.c
@@ -1044,15 +1044,17 @@ static void xl_freemem(struct net_device *dev)
 static irqreturn_t xl_interrupt(int irq, void *dev_id) 
 {
 	struct net_device *dev = (struct net_device *)dev_id;
- 	struct xl_private *xl_priv =(struct xl_private *)dev->priv;
-	u8 __iomem * xl_mmio = xl_priv->xl_mmio ; 
-	u16 intstatus, macstatus  ;
+ 	struct xl_private *xl_priv;
+	u8 __iomem * xl_mmio; 
+	u16 intstatus, macstatus;
 
 	if (!dev) { 
-		printk(KERN_WARNING "Device structure dead, aaahhhh !\n") ;
+		printk(KERN_WARNING "3c359: Device structure dead, aaahhhh!\n");
 		return IRQ_NONE; 
 	}
 
+	xl_priv = (struct xl_private *)dev->priv;
+	xl_mmio = xl_priv->xl_mmio;
 	intstatus = readw(xl_mmio + MMIO_INTSTATUS) ;  
 
 	if (!(intstatus & 1)) /* We didn't generate the interrupt */




^ permalink raw reply related

* Re: dm9000: add set_mac_address() v2
From: Michael Buesch @ 2007-08-12 22:26 UTC (permalink / raw)
  To: michael trimarchi; +Cc: jgarzi, netdev
In-Reply-To: <20070812212009.GD27490@gandalf.sssup.it>

On Sunday 12 August 2007 23:20:09 michael trimarchi wrote:
> Implement set_mac_address() for the dm9000 driver.  This allows changing
> the mac address of the interface. Fix BigEndian problem.
>  
> Signed-off-by: Michael Trimarchi <trimarchi@gandalf.sssup.it>
> ---
> 
> --- linux-2.6.22/drivers/net/dm9000.c.orig	2007-07-09 01:32:17.000000000 +0200
> +++ linux-2.6.22/drivers/net/dm9000.c	2007-08-13 00:15:38.000000000 +0200
> @@ -161,6 +161,8 @@ static irqreturn_t dm9000_interrupt(int,
>  static int dm9000_phy_read(struct net_device *dev, int phyaddr_unsused, int reg);
>  static void dm9000_phy_write(struct net_device *dev, int phyaddr_unused, int reg,
>  			   int value);
> +static int dm9000_eth_set_mac_address(struct net_device *dev, void *p);
> +
>  static u16 read_srom_word(board_info_t *, int);
>  static void dm9000_rx(struct net_device *);
>  static void dm9000_hash_table(struct net_device *);
> @@ -546,6 +548,7 @@ dm9000_probe(struct platform_device *pde
>  	ndev->stop		 = &dm9000_stop;
>  	ndev->get_stats		 = &dm9000_get_stats;
>  	ndev->set_multicast_list = &dm9000_hash_table;
> +	ndev->set_mac_address	 = &dm9000_eth_set_mac_address;
>  #ifdef CONFIG_NET_POLL_CONTROLLER
>  	ndev->poll_controller	 = &dm9000_poll_controller;
>  #endif
> @@ -986,7 +989,6 @@ read_srom_word(board_info_t * db, int of
>  	return (ior(db, DM9000_EPDRL) + (ior(db, DM9000_EPDRH) << 8));
>  }
>  
> -#ifdef DM9000_PROGRAM_EEPROM
>  /*
>   * Write a word data to SROM
>   */
> @@ -1002,6 +1004,35 @@ write_srom_word(board_info_t * db, int o
>  }
>  
>  /*
> + * dm9000_eth_set_mac_address
> + *
> + * Change the interface's mac address.
> + *
> + */
> +static int 
> +dm9000_eth_set_mac_address(struct net_device *dev, void *p)
> +{
> +	board_info_t *db = (board_info_t *) dev->priv;
> +	struct sockaddr *addr = p;
> +	int i;
> +
> +	if (!is_valid_ether_addr(addr->sa_data))
> +		return -EADDRNOTAVAIL;
> +
> +	if (netif_running(dev))
> +		return -EBUSY;
> +
> +	memcpy(dev->dev_addr, addr->sa_data, dev->addr_len);
> +
> +	for (i = 0; i < 3; i++)
> +		write_srom_word(db, i,
> +				cpu_to_le16(((u16 *) (addr->sa_data))[i]));

Nope.

write_srom_word(db, i, le16_to_cpu(((__le16 *) (addr->sa_data))[i]));

-- 
Greetings Michael.

^ permalink raw reply

* Re: troubles with r8169
From: Francois Romieu @ 2007-08-12 22:32 UTC (permalink / raw)
  To: Vadim Dyadkin; +Cc: netdev, linux-kernel
In-Reply-To: <46BF5357.6080205@lns.pnpi.spb.ru>

Vadim Dyadkin <dyadkin@lns.pnpi.spb.ru> :
[...]
> I need help from developers, may be, because I have some troubles with
> r8169 card.

Which kernel do you use ?

-- 
Ueimor

^ permalink raw reply

* Re: [RFC: -mm patch] improve the SSB dependencies
From: Adrian Bunk @ 2007-08-12 22:44 UTC (permalink / raw)
  To: Michael Buesch
  Cc: Johannes Berg, John Linville, Andrew Morton, Linux Netdev List
In-Reply-To: <200708121400.28297.mb@bu3sch.de>

On Sun, Aug 12, 2007 at 02:00:26PM +0200, Michael Buesch wrote:
> Ok, I'll give it a try, with small modifications.

Thanks.

> On Sunday 12 August 2007, Adrian Bunk wrote:
> > Additional changes in this patch:
> > - small help text changes
> > - B44_PCI is no longer usr visible (automatically enabled when possible)
> 
> I think we want that to be selectable, as it's not needed
> on some embedded devices. And we need to save memory there.
>...

Makes sense, but then:

config B44_PCI
	bool "Broadcom 440x PCI device support" if EMBEDDED
	...
	default y
	...

I don't care about how many options we present if CONFIG_EMBEDDED=y, but 
for the normal CONFIG_EMBEDDED=n case we should not bother the user with 
this option.

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed


^ permalink raw reply

* Re: dm9000: add set_mac_address() v2
From: Michael Trimarchi @ 2007-08-12 22:45 UTC (permalink / raw)
  To: Michael Buesch; +Cc: jgarzik, netdev
In-Reply-To: <200708130026.07221.mb@bu3sch.de>


>> +		return -EBUSY;
>> +
>> +	memcpy(dev->dev_addr, addr->sa_data, dev->addr_len);
>> +
>> +	for (i = 0; i < 3; i++)
>> +		write_srom_word(db, i,
>> +				cpu_to_le16(((u16 *) (addr->sa_data))[i]));
>>     
>
> Nope.
>
> write_srom_word(db, i, le16_to_cpu(((__le16 *) (addr->sa_data))[i]));
>
>   
Are you sure? I don't know how the dm9000 reads the eeprom back.

regards
Michael


^ permalink raw reply

* Re: dm9000: add set_mac_address() v2
From: Michael Buesch @ 2007-08-12 22:58 UTC (permalink / raw)
  To: Michael Trimarchi; +Cc: jgarzik, netdev
In-Reply-To: <46BF8D7D.5070105@gandalf.sssup.it>

On Monday 13 August 2007 00:45:17 Michael Trimarchi wrote:
> 
> >> +		return -EBUSY;
> >> +
> >> +	memcpy(dev->dev_addr, addr->sa_data, dev->addr_len);
> >> +
> >> +	for (i = 0; i < 3; i++)
> >> +		write_srom_word(db, i,
> >> +				cpu_to_le16(((u16 *) (addr->sa_data))[i]));
> >>     
> >
> > Nope.
> >
> > write_srom_word(db, i, le16_to_cpu(((__le16 *) (addr->sa_data))[i]));
> >
> >   
> Are you sure?

Yes I am. cpu_to_le16 simply doesn't make any sense at all here,
while le16_to_cpu does make sense and is indeed correct.
Though they both generate the same asm code. Running sparse
also tells you more about this. ;)

-- 
Greetings Michael.

^ permalink raw reply

* Re: dm9000: add set_mac_address()
From: Jeff Garzik @ 2007-08-12 23:08 UTC (permalink / raw)
  To: Michael Trimarchi; +Cc: netdev
In-Reply-To: <20070812192741.GB27490@gandalf.sssup.it>

set_mac_address should not write to the SROM, as Michael noted.

The proper operations are:

     probe time:
	read MAC address from SROM

     dev open (interface up):
	write dev->dev_addr[] to RX filter (or identity) registers

EEPROM update support is available separately, via an ethtool sub-ioctl.

	Jeff




^ permalink raw reply

* Re: [1/1] Block device throttling [Re: Distributed storage.]
From: Daniel Phillips @ 2007-08-12 23:16 UTC (permalink / raw)
  To: Evgeniy Polyakov
  Cc: Jens Axboe, netdev, linux-kernel, linux-fsdevel, Peter Zijlstra
In-Reply-To: <20070808101708.GA23815@2ka.mipt.ru>

Hi Evgeniy,

Sorry for not getting back to you right away, I was on the road with 
limited email access.  Incidentally, the reason my mails to you keep 
bouncing is, your MTA is picky about my mailer's IP reversing to a real 
hostname.  I will take care of that pretty soon, but for now my direct 
mail to you is going to bounce and you will only see the lkml copy.

On Wednesday 08 August 2007 03:17, Evgeniy Polyakov wrote:
> This throttling mechanism allows to limit maximum amount of queued
> bios per physical device. By default it is turned off and old block
> layer behaviour with unlimited number of bios is used. When turned on
> (queue limit is set to something different than -1U via
> blk_set_queue_limit()), generic_make_request() will sleep until there
> is room in the queue. number of bios is increased in
> generic_make_request() and reduced either in bio_endio(), when bio is
> completely processed (bi_size is zero), and recharged from original
> queue when new device is assigned to bio via blk_set_bdev(). All
> oerations are not atomic, since we do not care about precise number
> of bios, but a fact, that we are close or close enough to the limit.
>
> Tested on distributed storage device - with limit of 2 bios it works
> slow :)

it seems to me you need:

-               if (q) {
+               if (q && q->bio_limit != -1) {

This patch is short and simple, and will throttle more accurately than 
the current simplistic per-request allocation limit.  However, it fails 
to throttle device mapper devices.  This is because no request is 
allocated by the device mapper queue method, instead the mapping call 
goes straight through to the mapping function.  If the mapping function 
allocates memory (typically the case) then this resource usage evades 
throttling and deadlock becomes a risk.

There are three obvious fixes:

   1) Implement bio throttling in each virtual block device
   2) Implement bio throttling generically in device mapper
   3) Implement bio throttling for all block devices

Number 1 is the approach we currently use in ddsnap, but it is ugly and 
repetitious.  Number 2 is a possibility, but I favor number 3 because 
it is a system-wide solution to a system-wide problem, does not need to 
be repeated for every block device that lacks a queue, heads in the 
direction of code subtraction, and allows system-wide reserve 
accounting. 

Your patch is close to the truth, but it needs to throttle at the top 
(virtual) end of each block device stack instead of the bottom 
(physical) end.  It does head in the direction of eliminating your own 
deadlock risk indeed, however there are block devices it does not 
cover.

Regards,

Daniel

^ permalink raw reply

* Re: Distributed storage.
From: Daniel Phillips @ 2007-08-12 23:36 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Evgeniy Polyakov, netdev, linux-kernel, linux-fsdevel,
	Peter Zijlstra
In-Reply-To: <20070807205538.GB5245@kernel.dk>

On Tuesday 07 August 2007 13:55, Jens Axboe wrote:
> I don't like structure bloat, but I do like nice design. Overloading
> is a necessary evil sometimes, though. Even today, there isn't enough
> room to hold bi_rw and bi_flags in the same variable on 32-bit archs,
> so that concern can be scratched. If you read bio.h, that much is
> obvious.

Sixteen bits in bi_rw are consumed by queue priority.  Is there a reason 
this lives in struct bio instead of struct request?

> If you check up on the iommu virtual merging, you'll understand the
> front and back size members. They may smell dubious to you, but
> please take the time to understand why it looks the way it does.

Virtual merging is only needed at the physical device, so why do these 
fields live in struct bio instead of struct request?

> Changing the number of bvecs is integral to how bio buildup current
> works.

Right, that is done by bi_vcnt.  I meant bi_max_vecs, which you can 
derive efficiently from BIO_POOL_IDX() provided the bio was allocated 
in the standard way.  This leaves a little bit of clean up to do for 
bios not allocated from a standard pool.

Incidentally, why does the bvl need to be memset to zero on allocation?  
bi_vcnt already tells you which bvecs are valid and the only field in a 
bvec that can reasonably default to zero is the offset, which ought to 
be set set every time a bvec is initialized anyway.

> > bi_destructor could be combined.  I don't see a lot of users of
> > bi_idx,
>
> bi_idx is integral to partial io completions.

Struct request has a remaining submission sector count so what does 
bi_idx do that is different?

> > that looks like a soft target.  See what happened to struct page
> > when a couple of folks got serious about attacking it, some really
> > deep hacks were done to pare off a few bytes here and there.  But
> > struct bio as a space waster is not nearly in the same ballpark.
>
> So show some concrete patches and examples, hand waving and
> assumptions is just a waste of everyones time.

Average struct bio memory footprint ranks near the bottom of the list of 
things that suck most about Linux storage.  At idle I see 8K in use 
(reserves); during updatedb it spikes occasionally to 50K; under a 
heavy  load generated by ddsnap on a storage box it sometimes goes to 
100K with bio throttling in place.  Really not moving the needle.

On the other hand, vm writeout deadlock ranks smack dab at the top of 
the list, so that is where the patching effort must go for the 
forseeable future.  Without bio throttling, the ddsnap load can go to 
24 MB for struct bio alone.  That definitely moves the needle.  in 
short, we save 3,200 times more memory by putting decent throttling in 
place than by saving an int in struct bio.

That said, I did a little analysis to get an idea of where the soft 
targets are in struct bio, and to get to know the bio layer a little 
better.  Maybe these few hints will get somebody interested enough to 
look further.

> > It would be interesting to see if bi_bdev could be made read only.
> > Generally, each stage in the block device stack knows what the next
> > stage is going to be, so why do we have to write that in the bio? 
> > For error reporting from interrupt context?  Anyway, if Evgeniy
> > wants to do the patch, I will happily unload the task of convincing
> > you that random fields are/are not needed in struct bio :-)
>
> It's a trade off, otherwise you'd have to pass the block device
> around a lot.

Which costs very little, probably less than trashing an extra field's 
worth of cache.

> And it's, again, a design issue. A bio contains 
> destination information, that means device/offset/size information.
> I'm all for shaving structure bytes where it matters, but not for the
> sake of sacrificing code stability or design. I consider struct bio
> quite lean and have worked hard to keep it that way. In fact, iirc,
> the only addition to struct bio since 2001 is the iommu front/back
> size members. And I resisted those for quite a while.

You did not comment on the one about putting the bio destructor in 
the ->endio handler, which looks dead simple.  The majority of cases 
just use the default endio handler and the default destructor.  Of the 
remaining cases, where a specialized destructor is needed, typically a 
specialized endio handler is too, so combining is free.  There are few 
if any cases where a new specialized endio handler would need to be 
written.

As far as code stability goes, current kernels are horribly unstable in 
a variety of contexts because of memory deadlock and slowdowns related 
to the attempt to fix the problem via dirty memory limits.  Accurate 
throttling of bio traffic is one of the two key requirements to fix 
this instability, the other other is accurate writeout path reserve 
management, which is only partially addressed by BIO_POOL.

Nice to see you jumping in Jens.  Now it is over to the other side of 
the thread where Evgeniy has posted a patch that a) grants your wish to 
add no new field in struct bio and b) does not fix the problem.

Regards,

Daniel

^ permalink raw reply

* Re: [RFD] Layering: Use-Case Composers (was: DRBD - what is it, anyways? [compare with e.g. NBD + MD raid])
From: Paul Clements @ 2007-08-13  1:41 UTC (permalink / raw)
  To: Jan Engelhardt, david, Al Boldi, linux-kernel, linux-fsdevel,
	netdev, linux-raid
In-Reply-To: <20070812174549.GA2915@teal.hq.k1024.org>

Iustin Pop wrote:
> On Sun, Aug 12, 2007 at 07:03:44PM +0200, Jan Engelhardt wrote:
>> On Aug 12 2007 09:39, david@lang.hm wrote:
>>> now, I am not an expert on either option, but three are a couple things that I
>>> would question about the DRDB+MD option
>>>
>>> 1. when the remote machine is down, how does MD deal with it for reads and
>>> writes?
>> I suppose it kicks the drive and you'd have to re-add it by hand unless done by
>> a cronjob.

Yes, and with a bitmap configured on the raid1, you just resync the 
blocks that have been written while the connection was down.


>>From my tests, since NBD doesn't have a timeout option, MD hangs in the
> write to that mirror indefinitely, somewhat like when dealing with a
> broken IDE driver/chipset/disk.

Well, if people would like to see a timeout option, I actually coded up 
a patch a couple of years ago to do just that, but I never got it into 
mainline because you can do almost as well by doing a check at 
user-level (I basically ping the nbd connection periodically and if it 
fails, I kill -9 the nbd-client).


>>> 2. MD over local drive will alternate reads between mirrors (or so I've been
>>> told), doing so over the network is wrong.
>> Certainly. In which case you set "write_mostly" (or even write_only, not sure
>> of its name) on the raid component that is nbd.
>>
>>> 3. when writing, will MD wait for the network I/O to get the data saved on the
>>> backup before returning from the syscall? or can it sync the data out lazily
>> Can't answer this one - ask Neil :)
> 
> MD has the write-mostly/write-behind options - which help in this case
> but only up to a certain amount.

You can configure write_behind (aka, asynchronous writes) to buffer as 
much data as you have RAM to hold. At a certain point, presumably, you'd 
want to just break the mirror and take the hit of doing a resync once 
your network leg falls too far behind.

--
Paul

^ permalink raw reply

* Re: [RFD] Layering: Use-Case Composers (was: DRBD - what is it, anyways? [compare with e.g. NBD + MD raid])
From: david @ 2007-08-13  3:21 UTC (permalink / raw)
  To: Paul Clements
  Cc: Jan Engelhardt, Al Boldi, linux-kernel, linux-fsdevel, netdev,
	linux-raid
In-Reply-To: <46BFB6BB.80406@steeleye.com>

per the message below MD (or DM) would need to be modified to work 
reasonably well with one of the disk components being over an unreliable 
link (like a network link)

are the MD/DM maintainers interested in extending their code in this 
direction? or would they prefer to keep it simpler by being able to 
continue to assume that the raid components are connected over a highly 
reliable connection?

if they are interested in adding (and maintaining) this functionality then 
there is a real possibility that NBD+MD/DM could eliminate the need for 
DRDB. however if they are not interested in adding all the code to deal 
with the network type issues, then the argument that DRDB should not be 
merged becouse you can do the same thing with MD/DM + NBD is invalid and 
can be dropped/ignored

David Lang

On Sun, 12 Aug 2007, Paul Clements wrote:

> Iustin Pop wrote:
>>  On Sun, Aug 12, 2007 at 07:03:44PM +0200, Jan Engelhardt wrote:
>> >  On Aug 12 2007 09:39, david@lang.hm wrote:
>> > >  now, I am not an expert on either option, but three are a couple 
>> > >  things that I
>> > >  would question about the DRDB+MD option
>> > > 
>> > >  1. when the remote machine is down, how does MD deal with it for reads 
>> > >  and
>> > >  writes?
>> >  I suppose it kicks the drive and you'd have to re-add it by hand unless 
>> >  done by
>> >  a cronjob.
>
> Yes, and with a bitmap configured on the raid1, you just resync the blocks 
> that have been written while the connection was down.
>
>
>> >From my tests, since NBD doesn't have a timeout option, MD hangs in the
>>  write to that mirror indefinitely, somewhat like when dealing with a
>>  broken IDE driver/chipset/disk.
>
> Well, if people would like to see a timeout option, I actually coded up a 
> patch a couple of years ago to do just that, but I never got it into mainline 
> because you can do almost as well by doing a check at user-level (I basically 
> ping the nbd connection periodically and if it fails, I kill -9 the 
> nbd-client).
>
>
>> > >  2. MD over local drive will alternate reads between mirrors (or so 
>> > >  I've been
>> > >  told), doing so over the network is wrong.
>> >  Certainly. In which case you set "write_mostly" (or even write_only, not 
>> >  sure
>> >  of its name) on the raid component that is nbd.
>> > 
>> > >  3. when writing, will MD wait for the network I/O to get the data 
>> > >  saved on the
>> > >  backup before returning from the syscall? or can it sync the data out 
>> > >  lazily
>> >  Can't answer this one - ask Neil :)
>>
>>  MD has the write-mostly/write-behind options - which help in this case
>>  but only up to a certain amount.
>
> You can configure write_behind (aka, asynchronous writes) to buffer as much 
> data as you have RAM to hold. At a certain point, presumably, you'd want to 
> just break the mirror and take the hit of doing a resync once your network 
> leg falls too far behind.
>
> --
> Paul
>

^ permalink raw reply

* Re: [PATCH 6/24] make atomic_read() behave consistently on frv
From: Herbert Xu @ 2007-08-13  5:15 UTC (permalink / raw)
  To: paulmck
  Cc: herbert, csnook, dhowells, linux-kernel, linux-arch, torvalds,
	netdev, akpm, ak, heiko.carstens, davem, schwidefsky, wensong,
	horms, wjiang, cfriesen, zlynx, rpjday, jesper.juhl
In-Reply-To: <20070811042943.GA13410@linux.vnet.ibm.com>

Paul E. McKenney <paulmck@linux.vnet.ibm.com> wrote:
> On Sat, Aug 11, 2007 at 08:54:46AM +0800, Herbert Xu wrote:
>> Chris Snook <csnook@redhat.com> wrote:
>> > 
>> > cpu_relax() contains a barrier, so it should do the right thing.  For 
>> > non-smp architectures, I'm concerned about interacting with interrupt 
>> > handlers.  Some drivers do use atomic_* operations.
>> 
>> What problems with interrupt handlers? Access to int/long must
>> be atomic or we're in big trouble anyway.
> 
> Reordering due to compiler optimizations.  CPU reordering does not
> affect interactions with interrupt handlers on a given CPU, but
> reordering due to compiler code-movement optimization does.  Since
> volatile can in some cases suppress code-movement optimizations,
> it can affect interactions with interrupt handlers.

If such reordering matters, then you should use one of the
*mb macros or barrier() rather than relying on possibly
hidden volatile cast.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: Block device throttling [Re: Distributed storage.]
From: Daniel Phillips @ 2007-08-13  5:22 UTC (permalink / raw)
  To: Evgeniy Polyakov
  Cc: Jens Axboe, netdev, linux-kernel, linux-fsdevel, Peter Zijlstra
In-Reply-To: <20070808095448.GA3440@2ka.mipt.ru>

On Wednesday 08 August 2007 02:54, Evgeniy Polyakov wrote:
> On Tue, Aug 07, 2007 at 10:55:38PM +0200, Jens Axboe 
(jens.axboe@oracle.com) wrote:
>
> So, what did we decide? To bloat bio a bit (add a queue pointer) or
> to use physical device limits? The latter requires to replace all
> occurence of bio->bi_bdev = something_new with blk_set_bdev(bio,
> somthing_new), where queue limits will be appropriately charged. So
> far I'm testing second case, but I only changed DST for testing, can
> change all other users if needed though.

Adding a queue pointer to struct bio and using physical device limits as 
in your posted patch both suffer from the same problem: you release the 
throttling on the previous queue when the bio moves to a new one, which 
is a bug because memory consumption on the previous queue then becomes 
unbounded, or limited only by the number of struct requests that can be 
allocated.  In other words, it reverts to the same situation we have 
now as soon as the IO stack has more than one queue.  (Just a shorter 
version of my previous post.)

We can solve this by having the bio only point at the queue to which it 
was originally submitted, since throttling the top level queue 
automatically throttles all queues lower down the stack.  Alternatively 
the bio can point at the block_device or straight at the 
backing_dev_info, which is the per-device structure it actually needs 
to touch.

Note!  There are two more issues I forgot to mention earlier.

1) One throttle count per submitted bio is too crude a measure.  A bio 
can carry as few as one page or as many as 256 pages.  If you take only 
one throttle count per bio and that data will be transferred over the 
network then you have to assume that (a little more than) 256 pages of 
sk_alloc reserve will be needed for every bio, resulting in a grossly 
over-provisioned reserve.  The precise reserve calculation we want to 
do is per-block device, and you will find hooks like this already 
living in backing_dev_info.  We need to place our own fn+data there to 
calculate the throttle draw for each bio.  Unthrottling gets trickier 
with variable size throttle draw.  In ddsnap, we simply write the 
amount we drew from the throttle into (the private data of) bio for use 
later by unthrottle, thus avoiding the issue that the bio fields we 
used to calculate might have changed during the lifetime of the bio.  
This would translate into one more per-bio field.



the throttling performs another function: keeping a reasonable amount of 
IO in flight for the device.  The definition of "reasonable" is 
complex.  For a hard disk it depends on the physical distance between 
sector addresses of the bios in flight.  In ddsnap we make a crude but 
workable approximation that 


 In general, a per block device 

The throttle count needs to cover 

Regards,

Daniel

^ permalink raw reply

* Re: Block device throttling [Re: Distributed storage.]
From: Daniel Phillips @ 2007-08-13  5:36 UTC (permalink / raw)
  To: Evgeniy Polyakov
  Cc: Jens Axboe, netdev, linux-kernel, linux-fsdevel, Peter Zijlstra
In-Reply-To: <20070808095448.GA3440@2ka.mipt.ru>

(previous incomplete message sent accidentally)

On Wednesday 08 August 2007 02:54, Evgeniy Polyakov wrote:
> On Tue, Aug 07, 2007 at 10:55:38PM +0200, Jens Axboe wrote:
>
> So, what did we decide? To bloat bio a bit (add a queue pointer) or
> to use physical device limits? The latter requires to replace all
> occurence of bio->bi_bdev = something_new with blk_set_bdev(bio,
> somthing_new), where queue limits will be appropriately charged. So
> far I'm testing second case, but I only changed DST for testing, can
> change all other users if needed though.

Adding a queue pointer to struct bio and using physical device limits as 
in your posted patch both suffer from the same problem: you release the 
throttling on the previous queue when the bio moves to a new one, which 
is a bug because memory consumption on the previous queue then becomes 
unbounded, or limited only by the number of struct requests that can be 
allocated.  In other words, it reverts to the same situation we have 
now as soon as the IO stack has more than one queue.  (Just a shorter 
version of my previous post.)

We can solve this by having the bio only point at the queue to which it 
was originally submitted, since throttling the top level queue 
automatically throttles all queues lower down the stack.  Alternatively 
the bio can point at the block_device or straight at the 
backing_dev_info, which is the per-device structure it actually needs 
to touch.

Note!  There are two more issues I forgot to mention earlier.

1) One throttle count per submitted bio is too crude a measure.  A bio 
can carry as few as one page or as many as 256 pages.  If you take only 
one throttle count per bio and that data will be transferred over the 
network then you have to assume that (a little more than) 256 pages of 
sk_alloc reserve will be needed for every bio, resulting in a grossly 
over-provisioned reserve.  The precise reserve calculation we want to 
do is per-block device, and you will find hooks like this already 
living in backing_dev_info.  We need to place our own fn+data there to 
calculate the throttle draw for each bio.  Unthrottling gets trickier 
with variable size throttle draw.  In ddsnap, we simply write the 
amount we drew from the throttle into (the private data of) bio for use 
later by unthrottle, thus avoiding the issue that the bio fields we 
used to calculate might have changed during the lifetime of the bio.  
This would translate into one more per-bio field.

2) Exposing the per-block device throttle limits via sysfs or similar is 
really not a good long term solution for system administration.  
Imagine our help text: "just keep trying smaller numbers until your 
system deadlocks".  We really need to figure this out internally and 
get it correct.  I can see putting in a temporary userspace interface 
just for experimentation, to help determine what really is safe, and 
what size the numbers should be to approach optimal throughput in a 
fully loaded memory state.

Regards,

Daniel


Regards,

Daniel

^ permalink raw reply

* RE: [PATCH 2.6.24]S2io: Default to IntA interrupt type when there are less than 4 CPUs in the system.
From: Ramkrishna Vepa @ 2007-08-13  5:59 UTC (permalink / raw)
  To: Andi Kleen; +Cc: Jeff Garzik, Rick Jones, Sivakumar Subramani, netdev, support
In-Reply-To: <p73sl6oishx.fsf@bingen.suse.de>

We will be releasing it to the mainline soon. The patches are getting
queued at our end for test and release. It should be out in a couple of
weeks.

Ram
> -----Original Message-----
> From: ak@suse.de [mailto:ak@suse.de] On Behalf Of Andi Kleen
> Sent: Sunday, August 12, 2007 10:16 AM
> To: Ramkrishna Vepa
> Cc: Jeff Garzik; Rick Jones; Sivakumar Subramani;
netdev@vger.kernel.org;
> support
> Subject: Re: [PATCH 2.6.24]S2io: Default to IntA interrupt type when
there
> are less than 4 CPUs in the system.
> 
> "Ramkrishna Vepa" <Ramkrishna.Vepa@neterion.com> writes:
> 
> > In one of the variations of this driver that is not
> > released to netdev, the received packets are steered to a channel
based
> > on hashing on a preconfigured criteria such as sockets on tcp_ipv4,
> > udp_ipv4, tcp_ipv6, udp_ipv6 or addresses in ipv4/6.
> 
> Why is it not released for mainline? It sounds interesting.
> 
> -Andi


^ permalink raw reply

* Re: [PATCH 6/24] make atomic_read() behave consistently on frv
From: Paul E. McKenney @ 2007-08-13  6:03 UTC (permalink / raw)
  To: Herbert Xu
  Cc: csnook, dhowells, linux-kernel, linux-arch, torvalds, netdev,
	akpm, ak, heiko.carstens, davem, schwidefsky, wensong, horms,
	wjiang, cfriesen, zlynx, rpjday, jesper.juhl
In-Reply-To: <E1IKSHQ-0007hg-00@gondolin.me.apana.org.au>

On Mon, Aug 13, 2007 at 01:15:52PM +0800, Herbert Xu wrote:
> Paul E. McKenney <paulmck@linux.vnet.ibm.com> wrote:
> > On Sat, Aug 11, 2007 at 08:54:46AM +0800, Herbert Xu wrote:
> >> Chris Snook <csnook@redhat.com> wrote:
> >> > 
> >> > cpu_relax() contains a barrier, so it should do the right thing.  For 
> >> > non-smp architectures, I'm concerned about interacting with interrupt 
> >> > handlers.  Some drivers do use atomic_* operations.
> >> 
> >> What problems with interrupt handlers? Access to int/long must
> >> be atomic or we're in big trouble anyway.
> > 
> > Reordering due to compiler optimizations.  CPU reordering does not
> > affect interactions with interrupt handlers on a given CPU, but
> > reordering due to compiler code-movement optimization does.  Since
> > volatile can in some cases suppress code-movement optimizations,
> > it can affect interactions with interrupt handlers.
> 
> If such reordering matters, then you should use one of the
> *mb macros or barrier() rather than relying on possibly
> hidden volatile cast.

If communicating among CPUs, sure.  However, when communicating between
mainline and interrupt/NMI handlers on the same CPU, the barrier() and
most expecially the *mb() macros are gross overkill.  So there really
truly is a place for volatile -- not a large place, to be sure, but a
place nonetheless.

						Thanx, Paul

^ permalink raw reply

* [PATCH] [3/2many] MAINTAINERS - 3C359 NETWORK DRIVER
From: joe @ 2007-08-13  6:15 UTC (permalink / raw)
  To: torvalds, netdev, mikep, linux-tr, linux-kernel, akpm

Add file pattern to MAINTAINER entry

Signed-off-by: Joe Perches <joe@perches.com>

diff --git a/MAINTAINERS b/MAINTAINERS
index 0d7f856..19d0c9e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -97,6 +97,7 @@ L:	netdev@vger.kernel.org
 L:	linux-tr@linuxtr.net
 W:	http://www.linuxtr.net
 S:	Maintained
+F:	drivers/net/tokenring/3c359*
 
 3C505 NETWORK DRIVER
 P:	Philip Blundell

^ permalink raw reply related

* [PATCH] [3/2many] MAINTAINERS - 3C359 NETWORK DRIVER
From: joe @ 2007-08-13  6:21 UTC (permalink / raw)
  To: torvalds, netdev, mikep, linux-kernel, akpm

Add file pattern to MAINTAINER entry

Signed-off-by: Joe Perches <joe@perches.com>

diff --git a/MAINTAINERS b/MAINTAINERS
index 0d7f856..f74f7d5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -94,9 +94,9 @@ F: Files and directories with wildcard patterns.
 P:	Mike Phillips
 M:	mikep@linuxtr.net
 L:	netdev@vger.kernel.org
-L:	linux-tr@linuxtr.net
 W:	http://www.linuxtr.net
 S:	Maintained
+F:	drivers/net/tokenring/3c359*
 
 3C505 NETWORK DRIVER
 P:	Philip Blundell

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox