* [PATCH] Fix Tsi108 ethernet driver performance
@ 2007-07-13 20:00 Alexandre Bounine
2007-07-13 20:28 ` Scott Wood
0 siblings, 1 reply; 6+ messages in thread
From: Alexandre Bounine @ 2007-07-13 20:00 UTC (permalink / raw)
To: Zang Roy-r61911; +Cc: linuxppc-dev
From: Alexandre Bounine <alexandreb@tundra.com>
This patch improves performance of the Tsi108 Ethernet driver by
changing interrupt handling
for frame receive path. It reduces number of interrupts generated for
received frames and therefore lowers CPU utilization by the device
driver.
=20=20=20
Signed-off-by: Alexandre Bounine <alexandreb@tundra.com>
---
diff -Nurp linux-2.6.22-hpc2/drivers/net/tsi108_eth.c
linux-2.6.22-tun/drivers/net/tsi108_eth.c
--- linux-2.6.22-hpc2/drivers/net/tsi108_eth.c 2007-07-08
19:32:17.000000000 -0400
+++ linux-2.6.22-tun/drivers/net/tsi108_eth.c 2007-07-12
16:55:50.000000000 -0400
@@ -58,6 +58,7 @@
#define MII_READ_DELAY 10000 /* max link wait time in msec */
=20
#define TSI108_RXRING_LEN 256
+#define TSI108_RX_INT_FREQ 32
=20
/* NOTE: The driver currently does not support receiving packets
* larger than the buffer size, so don't decrease this (unless you
@@ -69,8 +70,10 @@
=20
#define TSI108_TX_INT_FREQ 64
=20
-/* Check the phy status every half a second. */
-#define CHECK_PHY_INTERVAL (HZ/2)
+/* Timer interval to check the RX queue status */
+#define CHECK_RX_INTERVAL (HZ/50)
+/* Timer interval to check the PHY status */
+#define CHECK_PHY_INTERVAL (CHECK_RX_INTERVAL * 10)
=20
static int tsi108_init_one(struct platform_device *pdev);
static int tsi108_ether_remove(struct platform_device *pdev);
@@ -104,6 +107,7 @@ struct tsi108_prv_data {
unsigned int txfree;
=20
unsigned int phy_ok; /* The PHY is currently powered
on. */
+ unsigned int phy_chk_count;
=20
/* PHY status (duplex is 1 for half, 2 for full,
* so that the default 0 indicates that neither has
@@ -823,7 +827,10 @@ static int tsi108_refill_rx(struct net_d
*/
=20
data->rxring[rx].blen =3D TSI108_RX_SKB_SIZE;
- data->rxring[rx].misc =3D TSI108_RX_OWN | TSI108_RX_INT;
+ if (rx % TSI108_RX_INT_FREQ)
+ data->rxring[rx].misc =3D TSI108_RX_OWN;
+ else
+ data->rxring[rx].misc =3D TSI108_RX_OWN |
TSI108_RX_INT;
=20
data->rxhead =3D (data->rxhead + 1) % TSI108_RXRING_LEN;
data->rxfree++;
@@ -973,24 +980,22 @@ static void tsi108_rx_int(struct net_dev
}
}
=20
-/* If the RX ring has run out of memory, try periodically
- * to allocate some more, as otherwise poll would never
- * get called (apart from the initial end-of-queue condition).
- *
- * This is called once per second (by default) from the thread.
+/* tsi108_check_rxring() is used to periodically check if the RX ring
has
+ * pending frames that have not been reported by RX interrupt (due to
interrupt
+ * moderation). It is called with CHECK_RX_INTERVAL timer interval.
*/
=20
static void tsi108_check_rxring(struct net_device *dev)
{
struct tsi108_prv_data *data =3D netdev_priv(dev);
+ unsigned int rx =3D data->rxtail;
=20
- /* A poll is scheduled, as opposed to caling tsi108_refill_rx
- * directly, so as to keep the receive path single-threaded
- * (and thus not needing a lock).
- */
-
- if (netif_running(dev) && data->rxfree < TSI108_RXRING_LEN / 4)
+ /* Schedule a poll (if required) */
+ if (netif_running(dev) &&
+ 0 =3D=3D (data->rxring[rx].misc & TSI108_RX_OWN)) {
+ data->rxpending =3D 1;
tsi108_rx_int(dev);
+ }
}
=20
static void tsi108_tx_int(struct net_device *dev)
@@ -1295,6 +1300,7 @@ static void tsi108_init_phy(struct net_d
=20
printk(KERN_DEBUG "PHY_STAT reg contains %08x\n", phyval);
data->phy_ok =3D 1;
+ data->phy_chk_count =3D 0;
data->init_media =3D 1;
spin_unlock_irqrestore(&phy_lock, flags);
}
@@ -1664,9 +1670,12 @@ static void tsi108_timed_checker(unsigne
struct net_device *dev =3D (struct net_device *)dev_ptr;
struct tsi108_prv_data *data =3D netdev_priv(dev);
=20
- tsi108_check_phy(dev);
+ /* We assume that RX queue is checked more often than PHY status
*/
+ if (data->phy_chk_count++ =3D=3D 0)
+ tsi108_check_phy(dev);
tsi108_check_rxring(dev);
- mod_timer(&data->timer, jiffies + CHECK_PHY_INTERVAL);
+ data->phy_chk_count %=3D (CHECK_PHY_INTERVAL/CHECK_RX_INTERVAL);
+ mod_timer(&data->timer, jiffies + CHECK_RX_INTERVAL);
}
=20
static int tsi108_ether_init(void)
---=0D
=0D
Important Notice: This message is intended for the use of the individual to=
whom it is addressed and may contain information which is privileged, conf=
idential and/or exempt from disclosure under applicable law. If the reader =
of this message is not the intended recipient, or is not the employee or ag=
ent responsible for delivering the message to the intended recipient, you a=
re hereby notified that any dissemination, distribution, or copying of this=
communication is strictly prohibited. If you have received this communicat=
ion in error, please notify the sender immediately by telephone or return e=
-mail and delete the original message from your systems. Thank you. =0D
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix Tsi108 ethernet driver performance
2007-07-13 20:00 Alexandre Bounine
@ 2007-07-13 20:28 ` Scott Wood
2007-07-13 20:32 ` Scott Wood
0 siblings, 1 reply; 6+ messages in thread
From: Scott Wood @ 2007-07-13 20:28 UTC (permalink / raw)
To: Alexandre Bounine; +Cc: linuxppc-dev
Alexandre Bounine wrote:
> -/* If the RX ring has run out of memory, try periodically
> - * to allocate some more, as otherwise poll would never
> - * get called (apart from the initial end-of-queue condition).
> - *
> - * This is called once per second (by default) from the thread.
> +/* tsi108_check_rxring() is used to periodically check if the RX ring
> has
> + * pending frames that have not been reported by RX interrupt (due to
> interrupt
> + * moderation). It is called with CHECK_RX_INTERVAL timer interval.
Wouldn't this cause latencies of up to a second in responding to
received packets? I'd think that would be considered excessive.
And shouldn't NAPI be reducing the RX interrupt load anyway?
-Scott
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix Tsi108 ethernet driver performance
2007-07-13 20:28 ` Scott Wood
@ 2007-07-13 20:32 ` Scott Wood
2007-07-16 17:25 ` Alexandre Bounine
0 siblings, 1 reply; 6+ messages in thread
From: Scott Wood @ 2007-07-13 20:32 UTC (permalink / raw)
To: Scott Wood; +Cc: Alexandre Bounine, linuxppc-dev
Scott Wood wrote:
> Wouldn't this cause latencies of up to a second in responding to
> received packets? I'd think that would be considered excessive.
>
> And shouldn't NAPI be reducing the RX interrupt load anyway?
Sorry, I missed that you reduced the interval to 20 ms. Still, a bit on
the high side, and a source of wasted CPU cycles when the network isn't
heavily loaded. How much of a difference in CPU usage did you see with
this patch?
-Scott
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] Fix Tsi108 ethernet driver performance
2007-07-13 20:32 ` Scott Wood
@ 2007-07-16 17:25 ` Alexandre Bounine
0 siblings, 0 replies; 6+ messages in thread
From: Alexandre Bounine @ 2007-07-16 17:25 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev
I agree that 20 ms is not the best option. But, as you mentioned as
well, shorter period may bring more wasted CPU cycles.
Using this approach to interrupt moderation I see performance
improvement using frame flow that generates about one interrupt per
frame. Below are CPU usage numbers reported by top for the current and
patched versions of the Tsi108_eth driver.=20=20=20=20=20=20=20
Current version:
Cpu(s): 0.0% us, 0.3% sy, 0.0% ni, 14.6% id, 0.0% wa, 14.6% hi, 70.4%
si
Patched version:
Cpu(s): 0.0% us, 0.0% sy, 0.0% ni, 35.2% id, 1.2% wa, 2.4% hi, 61.2% si
- Alex.
-----Original Message-----
From: Scott Wood [mailto:scottwood@freescale.com]=20
Sent: Friday, July 13, 2007 4:33 PM
To: Scott Wood
Cc: Alexandre Bounine; linuxppc-dev@ozlabs.org
Subject: Re: [PATCH] Fix Tsi108 ethernet driver performance
Scott Wood wrote:
> Wouldn't this cause latencies of up to a second in responding to=20
> received packets? I'd think that would be considered excessive.
>=20
> And shouldn't NAPI be reducing the RX interrupt load anyway?
Sorry, I missed that you reduced the interval to 20 ms. Still, a bit on
the high side, and a source of wasted CPU cycles when the network isn't=20
heavily loaded. How much of a difference in CPU usage did you see with=20
this patch?
-Scott
---=0D
=0D
Important Notice: This message is intended for the use of the individual to=
whom it is addressed and may contain information which is privileged, conf=
idential and/or exempt from disclosure under applicable law. If the reader =
of this message is not the intended recipient, or is not the employee or ag=
ent responsible for delivering the message to the intended recipient, you a=
re hereby notified that any dissemination, distribution, or copying of this=
communication is strictly prohibited. If you have received this communicat=
ion in error, please notify the sender immediately by telephone or return e=
-mail and delete the original message from your systems. Thank you. =0D
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] Fix Tsi108 ethernet driver performance
@ 2007-07-16 17:36 Alexandre Bounine
2007-07-16 22:27 ` Jeff Garzik
0 siblings, 1 reply; 6+ messages in thread
From: Alexandre Bounine @ 2007-07-16 17:36 UTC (permalink / raw)
To: jgarzik, netdev; +Cc: linuxppc-dev
From: Alexandre Bounine <alexandreb@tundra.com>
This patch improves performance of the Tsi108 Ethernet driver by
changing interrupt handling for frame receive path. It reduces number of
interrupts generated for received frames and therefore lowers CPU
utilization by the device driver.
=20=20=20
Signed-off-by: Alexandre Bounine <alexandreb@tundra.com>
---
diff -Nurp linux-2.6.22-hpc2/drivers/net/tsi108_eth.c
linux-2.6.22-tun/drivers/net/tsi108_eth.c
--- linux-2.6.22-hpc2/drivers/net/tsi108_eth.c 2007-07-08
19:32:17.000000000 -0400
+++ linux-2.6.22-tun/drivers/net/tsi108_eth.c 2007-07-12
16:55:50.000000000 -0400
@@ -58,6 +58,7 @@
#define MII_READ_DELAY 10000 /* max link wait time in msec */
=20
#define TSI108_RXRING_LEN 256
+#define TSI108_RX_INT_FREQ 32
=20
/* NOTE: The driver currently does not support receiving packets
* larger than the buffer size, so don't decrease this (unless you
@@ -69,8 +70,10 @@
=20
#define TSI108_TX_INT_FREQ 64
=20
-/* Check the phy status every half a second. */
-#define CHECK_PHY_INTERVAL (HZ/2)
+/* Timer interval to check the RX queue status */
+#define CHECK_RX_INTERVAL (HZ/50)
+/* Timer interval to check the PHY status */
+#define CHECK_PHY_INTERVAL (CHECK_RX_INTERVAL * 10)
=20
static int tsi108_init_one(struct platform_device *pdev);
static int tsi108_ether_remove(struct platform_device *pdev);
@@ -104,6 +107,7 @@ struct tsi108_prv_data {
unsigned int txfree;
=20
unsigned int phy_ok; /* The PHY is currently powered
on. */
+ unsigned int phy_chk_count;
=20
/* PHY status (duplex is 1 for half, 2 for full,
* so that the default 0 indicates that neither has
@@ -823,7 +827,10 @@ static int tsi108_refill_rx(struct net_d
*/
=20
data->rxring[rx].blen =3D TSI108_RX_SKB_SIZE;
- data->rxring[rx].misc =3D TSI108_RX_OWN | TSI108_RX_INT;
+ if (rx % TSI108_RX_INT_FREQ)
+ data->rxring[rx].misc =3D TSI108_RX_OWN;
+ else
+ data->rxring[rx].misc =3D TSI108_RX_OWN |
TSI108_RX_INT;
=20
data->rxhead =3D (data->rxhead + 1) % TSI108_RXRING_LEN;
data->rxfree++;
@@ -973,24 +980,22 @@ static void tsi108_rx_int(struct net_dev
}
}
=20
-/* If the RX ring has run out of memory, try periodically
- * to allocate some more, as otherwise poll would never
- * get called (apart from the initial end-of-queue condition).
- *
- * This is called once per second (by default) from the thread.
+/* tsi108_check_rxring() is used to periodically check if the RX ring
has
+ * pending frames that have not been reported by RX interrupt (due to
interrupt
+ * moderation). It is called with CHECK_RX_INTERVAL timer interval.
*/
=20
static void tsi108_check_rxring(struct net_device *dev)
{
struct tsi108_prv_data *data =3D netdev_priv(dev);
+ unsigned int rx =3D data->rxtail;
=20
- /* A poll is scheduled, as opposed to caling tsi108_refill_rx
- * directly, so as to keep the receive path single-threaded
- * (and thus not needing a lock).
- */
-
- if (netif_running(dev) && data->rxfree < TSI108_RXRING_LEN / 4)
+ /* Schedule a poll (if required) */
+ if (netif_running(dev) &&
+ 0 =3D=3D (data->rxring[rx].misc & TSI108_RX_OWN)) {
+ data->rxpending =3D 1;
tsi108_rx_int(dev);
+ }
}
=20
static void tsi108_tx_int(struct net_device *dev)
@@ -1295,6 +1300,7 @@ static void tsi108_init_phy(struct net_d
=20
printk(KERN_DEBUG "PHY_STAT reg contains %08x\n", phyval);
data->phy_ok =3D 1;
+ data->phy_chk_count =3D 0;
data->init_media =3D 1;
spin_unlock_irqrestore(&phy_lock, flags);
}
@@ -1664,9 +1670,12 @@ static void tsi108_timed_checker(unsigne
struct net_device *dev =3D (struct net_device *)dev_ptr;
struct tsi108_prv_data *data =3D netdev_priv(dev);
=20
- tsi108_check_phy(dev);
+ /* We assume that RX queue is checked more often than PHY status
*/
+ if (data->phy_chk_count++ =3D=3D 0)
+ tsi108_check_phy(dev);
tsi108_check_rxring(dev);
- mod_timer(&data->timer, jiffies + CHECK_PHY_INTERVAL);
+ data->phy_chk_count %=3D (CHECK_PHY_INTERVAL/CHECK_RX_INTERVAL);
+ mod_timer(&data->timer, jiffies + CHECK_RX_INTERVAL);
}
=20
static int tsi108_ether_init(void)
---=0D
=0D
Important Notice: This message is intended for the use of the individual to=
whom it is addressed and may contain information which is privileged, conf=
idential and/or exempt from disclosure under applicable law. If the reader =
of this message is not the intended recipient, or is not the employee or ag=
ent responsible for delivering the message to the intended recipient, you a=
re hereby notified that any dissemination, distribution, or copying of this=
communication is strictly prohibited. If you have received this communicat=
ion in error, please notify the sender immediately by telephone or return e=
-mail and delete the original message from your systems. Thank you. =0D
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix Tsi108 ethernet driver performance
2007-07-16 17:36 [PATCH] Fix Tsi108 ethernet driver performance Alexandre Bounine
@ 2007-07-16 22:27 ` Jeff Garzik
0 siblings, 0 replies; 6+ messages in thread
From: Jeff Garzik @ 2007-07-16 22:27 UTC (permalink / raw)
To: Alexandre Bounine; +Cc: netdev, linuxppc-dev
Alexandre Bounine wrote:
> From: Alexandre Bounine <alexandreb@tundra.com>
>
> This patch improves performance of the Tsi108 Ethernet driver by
> changing interrupt handling for frame receive path. It reduces number of
> interrupts generated for received frames and therefore lowers CPU
> utilization by the device driver.
>
> Signed-off-by: Alexandre Bounine <alexandreb@tundra.com>
Look into NAPI...
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2007-07-16 22:27 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-07-16 17:36 [PATCH] Fix Tsi108 ethernet driver performance Alexandre Bounine
2007-07-16 22:27 ` Jeff Garzik
-- strict thread matches above, loose matches on Subject: below --
2007-07-13 20:00 Alexandre Bounine
2007-07-13 20:28 ` Scott Wood
2007-07-13 20:32 ` Scott Wood
2007-07-16 17:25 ` Alexandre Bounine
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).