| Message ID | 20111230215024.GA5513@electric-eye.fr.zoreil.com |
|---|---|
| State | RFC, archived |
| Delegated to: | David Miller |
| Headers | show |
2011/12/30 Francois Romieu <romieu@fr.zoreil.com>: > Bjarke Istrup Pedersen <gurligebis@gentoo.org> : > [...] >> Just applied it to HEAD of net-next, and got the following compiler warning: >> drivers/net/ethernet/via/via-rhine.c:2182:13: warning: >> 'rhine_task_enable' defined but not used > > You are running with CONFIG_PM disabled and a down / up loop will not > work. The patch below should hide the latter, at least as long as the > relevant workqueue does not stall for too long... Extra cancel_work in > task_enable could work too but it will look strange. Applied the patch, testing later :) You are saying that with CONFIG_PM disabled, I wont be able to bring the interface down and up again? >> Just so you know that too :) > > Thanks for testing. > > diff --git a/drivers/net/ethernet/via/via-rhine.c b/drivers/net/ethernet/via/via-rhine.c > index c57e1da..89ced1b 100644 > --- a/drivers/net/ethernet/via/via-rhine.c > +++ b/drivers/net/ethernet/via/via-rhine.c > @@ -1536,6 +1536,7 @@ static int rhine_open(struct net_device *dev) > alloc_rbufs(dev); > alloc_tbufs(dev); > rhine_chip_reset(dev); > + rhine_task_enable(rp); > init_registers(dev); > if (debug > 2) > netdev_dbg(dev, "%s() Done - status %04x MII status: %04x\n", > -- > Ueimor > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Bjarke Istrup Pedersen <gurligebis@gentoo.org> : [...] > Applied the patch, testing later :) > You are saying that with CONFIG_PM disabled, I wont be able to bring > the interface down and up again? No. You would not see the message with CONFIG_PM but the down / up problem would still be there. The patch should fix the down / up problem. /away
2011/12/30 Francois Romieu <romieu@fr.zoreil.com>: > Bjarke Istrup Pedersen <gurligebis@gentoo.org> : > [...] >> Just applied it to HEAD of net-next, and got the following compiler warning: >> drivers/net/ethernet/via/via-rhine.c:2182:13: warning: >> 'rhine_task_enable' defined but not used > > You are running with CONFIG_PM disabled and a down / up loop will not > work. The patch below should hide the latter, at least as long as the > relevant workqueue does not stall for too long... Extra cancel_work in > task_enable could work too but it will look strange. > >> Just so you know that too :) Does not build with patch: drivers/net/ethernet/via/via-rhine.c: In function 'rhine_open': drivers/net/ethernet/via/via-rhine.c:1539:2: error: implicit declaration of function 'rhine_task_enable' drivers/net/ethernet/via/via-rhine.c: At top level: drivers/net/ethernet/via/via-rhine.c:2183:13: warning: conflicting types for 'rhine_task_enable' drivers/net/ethernet/via/via-rhine.c:2183:13: error: static declaration of 'rhine_task_enable' follows non-static declaration drivers/net/ethernet/via/via-rhine.c:1539:2: note: previous implicit declaration of 'rhine_task_enable' was here drivers/net/ethernet/via/via-rhine.c:2183:13: warning: 'rhine_task_enable' defined but not used Wouldn't it be an idea to move the declarations to a via-rhine.h file, and include it, to work around it, and seperate it like it should be? /Bjarke > Thanks for testing. > > diff --git a/drivers/net/ethernet/via/via-rhine.c b/drivers/net/ethernet/via/via-rhine.c > index c57e1da..89ced1b 100644 > --- a/drivers/net/ethernet/via/via-rhine.c > +++ b/drivers/net/ethernet/via/via-rhine.c > @@ -1536,6 +1536,7 @@ static int rhine_open(struct net_device *dev) > alloc_rbufs(dev); > alloc_tbufs(dev); > rhine_chip_reset(dev); > + rhine_task_enable(rp); > init_registers(dev); > if (debug > 2) > netdev_dbg(dev, "%s() Done - status %04x MII status: %04x\n", > -- > Ueimor > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Bjarke Istrup Pedersen <gurligebis@gentoo.org> : [...] > Does not build with patch: Neither did it here. My head is already elsewhere. [...] > Wouldn't it be an idea to move the declarations to a via-rhine.h file, > and include it, to work around it, and seperate it like it should be? Code could be correctly ordered too. Forward declare it. It's good enough in the meantime.
2011/12/30 Francois Romieu <romieu@fr.zoreil.com>: > Bjarke Istrup Pedersen <gurligebis@gentoo.org> : > [...] >> Does not build with patch: > > Neither did it here. My head is already elsewhere. > > [...] >> Wouldn't it be an idea to move the declarations to a via-rhine.h file, >> and include it, to work around it, and seperate it like it should be? > > Code could be correctly ordered too. Forward declare it. It's good enough > in the meantime. Forward declaring it makes it compile just fine. I'll try and do some stress test of it when the rest of the house goes to sleep, so I don't cut off the net :) /Bjarke > -- > Ueimor > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
2011/12/30 Bjarke Istrup Pedersen <gurligebis@gentoo.org>: > 2011/12/30 Francois Romieu <romieu@fr.zoreil.com>: >> Bjarke Istrup Pedersen <gurligebis@gentoo.org> : >> [...] >>> Does not build with patch: >> >> Neither did it here. My head is already elsewhere. >> >> [...] >>> Wouldn't it be an idea to move the declarations to a via-rhine.h file, >>> and include it, to work around it, and seperate it like it should be? >> >> Code could be correctly ordered too. Forward declare it. It's good enough >> in the meantime. > > Forward declaring it makes it compile just fine. > > I'll try and do some stress test of it when the rest of the house goes > to sleep, so I don't cut off the net :) > > /Bjarke Okay, this is beginning to look great. Just tried randomly up'ing/down'ing the interfaces on the router while there was alot of trafic going between them (a client downloading a knoppix live dvd using bittorrent) - survived without a single bump. Also tried connect a machine to one of the ports, and copying a large file across (something that before could make it lock up within 15 secords) - also worked without any problems - CPU usage around 15% (mostly "top" using that), so thats not bad at all. Is there any other testcases that I should try out? /Bjarke >> -- >> Ueimor >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html >> Please read the FAQ at http://www.tux.org/lkml/ -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Bjarke Istrup Pedersen <gurligebis@gentoo.org> : [...] > Also tried connect a machine to one of the ports, and copying a large > file across (something that before could make it lock up within 15 > secords) - also worked without any problems - CPU usage around 15% > (mostly "top" using that), so thats not bad at all. > > Is there any other testcases that I should try out? Same thing + random ethtool link management commands. Same thing + random cable unplug/plug. Same thing + pktgen facing the lan interface. Everything at the same time.
2011/12/31 Francois Romieu <romieu@fr.zoreil.com>: > Bjarke Istrup Pedersen <gurligebis@gentoo.org> : > [...] >> Also tried connect a machine to one of the ports, and copying a large >> file across (something that before could make it lock up within 15 >> secords) - also worked without any problems - CPU usage around 15% >> (mostly "top" using that), so thats not bad at all. >> >> Is there any other testcases that I should try out? > > Same thing + random ethtool link management commands. > Same thing + random cable unplug/plug. > Same thing + pktgen facing the lan interface. > > Everything at the same time. I tried messing around with plugging cables and playing with ethtool at the same time, while there was alot of trafic, which went fine :) Happy new year btw :) /Bjarke > -- > Ueimor > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
2011/12/31 Francois Romieu <romieu@fr.zoreil.com>: > Bjarke Istrup Pedersen <gurligebis@gentoo.org> : > [...] >> Also tried connect a machine to one of the ports, and copying a large >> file across (something that before could make it lock up within 15 >> secords) - also worked without any problems - CPU usage around 15% >> (mostly "top" using that), so thats not bad at all. >> >> Is there any other testcases that I should try out? > > Same thing + random ethtool link management commands. > Same thing + random cable unplug/plug. > Same thing + pktgen facing the lan interface. > > Everything at the same time. Btw. - I have split up the via-rhine.c file (with all the changes you have sent me) into via-rhine.c and via-rhine.h , like it should be :-) I'll send it shortly after this mail - could you incorporate it into your work? /Bjarke > -- > Ueimor > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
2011/12/31 Francois Romieu <romieu@fr.zoreil.com>: > Bjarke Istrup Pedersen <gurligebis@gentoo.org> : > [...] >> Also tried connect a machine to one of the ports, and copying a large >> file across (something that before could make it lock up within 15 >> secords) - also worked without any problems - CPU usage around 15% >> (mostly "top" using that), so thats not bad at all. >> >> Is there any other testcases that I should try out? > > Same thing + random ethtool link management commands. > Same thing + random cable unplug/plug. > Same thing + pktgen facing the lan interface. > > Everything at the same time. Hey, Is there anything else I should try out? Also, would it be possible to push these changes to mainline as another CONFIG option (not sure if thats possible), so more people can enable it and test it? :) /Bjarke > -- > Ueimor > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Bjarke Istrup Pedersen <gurligebis@gentoo.org> : [...] > Is there anything else I should try out? Your computer is supposed to be moderately underpowered. Find someone with an heavily overpowered one. > Also, would it be possible to push these changes to mainline as Probably. There has not been much objection nor comments so far. > another CONFIG option (not sure if thats possible), so more people can > enable it and test it? :) I'd rather not. It would not be terribly elegant and more CONFIG options would not imply more testing.
2012/1/3 Francois Romieu <romieu@fr.zoreil.com>: > Bjarke Istrup Pedersen <gurligebis@gentoo.org> : > [...] >> Is there anything else I should try out? > > Your computer is supposed to be moderately underpowered. Find someone > with an heavily overpowered one. Hmm, thats gonna be difficult, since it's an on-board chip in the box, so it's no possible to move it to something more powerful :) >> Also, would it be possible to push these changes to mainline as > > Probably. There has not been much objection nor comments so far. Nice, any objections David? :) >> another CONFIG option (not sure if thats possible), so more people can >> enable it and test it? :) > > I'd rather not. It would not be terribly elegant and more CONFIG options > would not imply more testing. I agree, but if we push these changes to mainline directly, there is no need - it was more as a way of getting it to mainline, while still keeping the old way around, if it was needed. /Bjarke > -- > Ueimor -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
2012/1/4 Bjarke Istrup Pedersen <gurligebis@gentoo.org>: > 2012/1/3 Francois Romieu <romieu@fr.zoreil.com>: >> Bjarke Istrup Pedersen <gurligebis@gentoo.org> : >> [...] >>> Is there anything else I should try out? >> >> Your computer is supposed to be moderately underpowered. Find someone >> with an heavily overpowered one. > > Hmm, thats gonna be difficult, since it's an on-board chip in the box, > so it's no possible to move it to something more powerful :) > >>> Also, would it be possible to push these changes to mainline as >> >> Probably. There has not been much objection nor comments so far. > > Nice, any objections David? :) Sorry for repeating myself David, but would it be possible to get this into net-next for 3.4 inclusion? (I'm thinking that now is the right time for it to have a complete release cycle in net-next before inclusion) :) /Bjarke >>> another CONFIG option (not sure if thats possible), so more people can >>> enable it and test it? :) >> >> I'd rather not. It would not be terribly elegant and more CONFIG options >> would not imply more testing. > > I agree, but if we push these changes to mainline directly, there is > no need - it was more as a way of getting it to mainline, while still > keeping the old way around, if it was needed. > > /Bjarke > >> -- >> Ueimor -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Bjarke Istrup Pedersen <gurligebis@gentoo.org> : [...] > Sorry for repeating myself David, but would it be possible to get this > into net-next for 3.4 inclusion ? You should not expect #net-next for 3.4 to open before the merge window for 3.3 is closed. With due consideration for his freedom of will and action, David can not take the current patchkit because he needs a formal submission of a complete set where #3 is fixed. Our last friday's exchange does not count as one. I understand it is important for you, it will be sent, ok ? For future reference, you can see what is going on here : http://patchwork.ozlabs.org/project/netdev/list/?state=*&q=via-rhine /me returns to paperwork...
2012/1/5 Francois Romieu <romieu@fr.zoreil.com>: > Bjarke Istrup Pedersen <gurligebis@gentoo.org> : > [...] >> Sorry for repeating myself David, but would it be possible to get this >> into net-next for 3.4 inclusion ? > > You should not expect #net-next for 3.4 to open before the merge window > for 3.3 is closed. Okay, I'm still pretty new to how the kernel development model works, and what opens when and for how long, but hey, you learn something everyday :) > With due consideration for his freedom of will and action, David can not > take the current patchkit because he needs a formal submission of a complete > set where #3 is fixed. Our last friday's exchange does not count as one. Sure, it wasn't meant like that (Sorry if it was written that way, can be difficult from time to time to get it across on paper) :) > I understand it is important for you, it will be sent, ok ? Yep :) /Bjarke > For future reference, you can see what is going on here : > > http://patchwork.ozlabs.org/project/netdev/list/?state=*&q=via-rhine > > /me returns to paperwork... > > -- > Ueimor > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Bjarke Istrup Pedersen <gurligebis@gentoo.org> Date: Thu, 5 Jan 2012 13:01:41 +0100 > Sorry for repeating myself David, but would it be possible to get this > into net-next for 3.4 inclusion? Francois will formally submit it to me when he feels it is appropriate, please be patient. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/ethernet/via/via-rhine.c b/drivers/net/ethernet/via/via-rhine.c index c57e1da..89ced1b 100644 --- a/drivers/net/ethernet/via/via-rhine.c +++ b/drivers/net/ethernet/via/via-rhine.c @@ -1536,6 +1536,7 @@ static int rhine_open(struct net_device *dev) alloc_rbufs(dev); alloc_tbufs(dev); rhine_chip_reset(dev); + rhine_task_enable(rp); init_registers(dev); if (debug > 2) netdev_dbg(dev, "%s() Done - status %04x MII status: %04x\n",