[PATCH 2/2] netdev/fec: fix performance impact from mdio poll operation
Chase Douglas
chase.douglas at canonical.com
Thu Apr 8 20:38:03 UTC 2010
On Thu, Apr 8, 2010 at 2:44 AM, Bryan Wu <bryan.wu at canonical.com> wrote:
> BugLink: http://bugs.launchpad.net/bugs/546649
> BugLink: http://bugs.launchpad.net/bugs/457878
>
> After introducing phylib supporting, users experienced performace drop. That is
> because of the mdio polling operation of phylib. Use msleep to replace the busy
> waiting cpu_relax() and remove the warning message.
Is this performance drop in terms of overall system performance
because of the cpu_relax loop, or network/driver performance? I'm
assuming the former, but I want to make sure.
I was going to ask whether this was safe due to the potential for
msleep being run in an interrupt context. However, I found this
comment which calls the read() function that is reassuring:
http://lxr.linux.no/linux+v2.6.33/drivers/net/phy/mdio_bus.c#L201
Although things look ok to me, I don't have enough insight into this
driver to be able to give an Ack.
-- Chase
> Signed-off-by: Bryan Wu <bryan.wu at canonical.com>
> ---
> drivers/net/fec.c | 45 +++++++++++++++++++++------------------------
> 1 files changed, 21 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/net/fec.c b/drivers/net/fec.c
> index 53240d3..2280373 100644
> --- a/drivers/net/fec.c
> +++ b/drivers/net/fec.c
> @@ -216,7 +216,7 @@ static void fec_stop(struct net_device *dev);
> #define FEC_MMFR_TA (2 << 16)
> #define FEC_MMFR_DATA(v) (v & 0xffff)
>
> -#define FEC_MII_TIMEOUT 10000
> +#define FEC_MII_TIMEOUT 10
>
> /* Transmitter timeout */
> #define TX_TIMEOUT (2 * HZ)
> @@ -624,13 +624,29 @@ spin_unlock:
> /*
> * NOTE: a MII transaction is during around 25 us, so polling it...
> */
> -static int fec_enet_mdio_read(struct mii_bus *bus, int mii_id, int regnum)
> +static int fec_enet_mdio_poll(struct fec_enet_private *fep)
> {
> - struct fec_enet_private *fep = bus->priv;
> int timeout = FEC_MII_TIMEOUT;
>
> fep->mii_timeout = 0;
>
> + /* wait for end of transfer */
> + while (!(readl(fep->hwp + FEC_IEVENT) & FEC_ENET_MII)) {
> + msleep(1);
> + if (timeout-- < 0) {
> + fep->mii_timeout = 1;
> + break;
> + }
> + }
> +
> + return 0;
> +}
> +
> +static int fec_enet_mdio_read(struct mii_bus *bus, int mii_id, int regnum)
> +{
> + struct fec_enet_private *fep = bus->priv;
> +
> +
> /* clear MII end of transfer bit*/
> writel(FEC_ENET_MII, fep->hwp + FEC_IEVENT);
>
> @@ -639,15 +655,7 @@ static int fec_enet_mdio_read(struct mii_bus *bus, int mii_id, int regnum)
> FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(regnum) |
> FEC_MMFR_TA, fep->hwp + FEC_MII_DATA);
>
> - /* wait for end of transfer */
> - while (!(readl(fep->hwp + FEC_IEVENT) & FEC_ENET_MII)) {
> - cpu_relax();
> - if (timeout-- < 0) {
> - fep->mii_timeout = 1;
> - printk(KERN_ERR "FEC: MDIO read timeout\n");
> - return -ETIMEDOUT;
> - }
> - }
> + fec_enet_mdio_poll(fep);
>
> /* return value */
> return FEC_MMFR_DATA(readl(fep->hwp + FEC_MII_DATA));
> @@ -657,9 +665,6 @@ static int fec_enet_mdio_write(struct mii_bus *bus, int mii_id, int regnum,
> u16 value)
> {
> struct fec_enet_private *fep = bus->priv;
> - int timeout = FEC_MII_TIMEOUT;
> -
> - fep->mii_timeout = 0;
>
> /* clear MII end of transfer bit*/
> writel(FEC_ENET_MII, fep->hwp + FEC_IEVENT);
> @@ -670,15 +675,7 @@ static int fec_enet_mdio_write(struct mii_bus *bus, int mii_id, int regnum,
> FEC_MMFR_TA | FEC_MMFR_DATA(value),
> fep->hwp + FEC_MII_DATA);
>
> - /* wait for end of transfer */
> - while (!(readl(fep->hwp + FEC_IEVENT) & FEC_ENET_MII)) {
> - cpu_relax();
> - if (timeout-- < 0) {
> - fep->mii_timeout = 1;
> - printk(KERN_ERR "FEC: MDIO write timeout\n");
> - return -ETIMEDOUT;
> - }
> - }
> + fec_enet_mdio_poll(fep);
>
> return 0;
> }
> --
> 1.7.0.1
More information about the kernel-team
mailing list