|
|
|
|
|
|
| Author |
Message |
Gleb Smirnoff
*nix forums beginner
Joined: 30 Sep 2005
Posts: 27
|
 Posted: Fri Sep 30, 2005 10:40 am Post subject:
[REVIEW/TEST] polling(4) changes
|
|
|
[please, follow-up on net@ only]
Colleagues,
here are some patches for review.
Problems addressed:
1) When Giant was removed from polling a problem was introduced. The idle
poll feature was broken. The idle poll thread can enter polling handler on
one interface and put to sleep for a long time, until CPU resources found.
During this time no traffic is received on interface. Well, this is what
idle thread is supposed to do. Why didn't this happen with Giant? Because
idle poll entered poll handler holding Giant, and other threads (in particular
netisr poll) contested on Giant and propagated their priority to idle poll.
Well, this is a hack, but idle poll significantly improves polling performance
on an idle box, that's why I won't axe it but try to fix it.
To address the problem we need to use the same technique as before, but use
poll_mtx instead of Giant. However, this will resurrect LORs, that were fixed
with Giant removal. The alternative lock path happens, when driver decides
to deregister from polling itself. The LOR is fixed by further changes. See 3).
2) Drivers indicate their ability to do polling(4) with IFCAP_POLLING flag
int if_capabilites field. Setting the flag in if_capenable should register
interface with polling and disable interrupts. However, the if_flags is also
abused with IFF_POLLING flag. The aim is to remove IFF_POLLING flag.
3) The polling is switched on and off not functionally. That is, when you
say 'sysctl kern.polling.enable=1' or 'ifconfig fxp0 -polling', the polling
is switched on/off not immediately but on next tick or next interrupt. This
non-functional approach leads to a lot of ambiguouties in code, makes it
harder to understand and maintain. It also exposes race conditions.
The attached patch removes:
- IFF_POLLING flag.
- Use of if_flags, if_drv_flags, if_capenable from kern_poll.c.
All current accesses to these fields are not locked and polling shouldn't
look there.
- poll in trap feature. Sorry, we can't acquire mutexes in trap(). Anyone
used it, anyway?
- POLL_DEREGISTER command. No hacks. Everything is done functionally via
ioctl().
The new world order for driver is the following:
1) Declare IFCAP_POLLING in if_capabilities on attach. Do not touch if_capenable.
2) in ioctl method, in SIOCSIFCAP case the driver should:
- call ether_poll_[de]register
- if no error, set the IFCAP_POLLING flag in if_capenable
- obtain driver lock
- [dis/en]able interrupts
- drop driver lock
3) In poll method, check IFF_DRV_RUNNING flag after obtaining driver lock
4) In interrupt handler check IFCAP_POLLING flag in if_capenable. If present,
then return. This is important to protect from spurious interrupts.
5) In device detach method, call ether_poll_deregister() before obtaining
driver lock.
The new world order for user is the following:
- polling should be enabled and disabled with ifconfig(
- kern.polling.enable is now deprecated. It is kept for some compatibility. When
you set it to 1, polling is turned on for all capable interfaces. In case of 0
polling is turned off.
- no poll in trap
The attached patch touches only em(4) and fxp(4). I will write patches for all
other drivers ASAP. But I don't have all the hardware, so if you are using polling(4)
and you run FreeBSD 6 or 7, please help me with testing. ATM only em(4) driver patch
is tested.
--
Totus tuus, Glebius.
GLEBIUS-RIPN GLEB-RIPE |
|
| Back to top |
|
 |
Poul-Henning Kamp
*nix forums Guru
Joined: 21 Mar 2002
Posts: 436
|
| Posted: Fri Sep 30, 2005 10:43 am Post subject:
Re: [REVIEW/TEST] polling(4) changes
|
|
|
I still think we should stop having this network-centric view of
polling and implement _real_ *device* polling, so that other
device types can use it as well.
--
Poul-Henning Kamp | UNIX since Zilog Zeus 3.20
phk@FreeBSD.ORG | TCP/IP since RFC 956
FreeBSD committer | BSD since 4.3-tahoe
Never attribute to malice what can adequately be explained by incompetence.
_______________________________________________
freebsd-arch@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-arch
To unsubscribe, send any mail to "freebsd-arch-unsubscribe@freebsd.org" |
|
| Back to top |
|
|
Gleb Smirnoff
*nix forums beginner
Joined: 30 Sep 2005
Posts: 27
|
| Posted: Fri Sep 30, 2005 10:47 am Post subject:
Re: [REVIEW/TEST] polling(4) changes
|
|
|
On Fri, Sep 30, 2005 at 02:43:51PM +0200, Poul-Henning Kamp wrote:
P> I still think we should stop having this network-centric view of
P> polling and implement _real_ *device* polling, so that other
P> device types can use it as well.
I agree with both hands. My current work is aimed at RELENG_6 only.
btw, I've made a step in this direction already - kern_poll can now
forget about struct ifnet.
--
Totus tuus, Glebius.
GLEBIUS-RIPN GLEB-RIPE
_______________________________________________
freebsd-arch@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-arch
To unsubscribe, send any mail to "freebsd-arch-unsubscribe@freebsd.org" |
|
| Back to top |
|
|
Poul-Henning Kamp
*nix forums Guru
Joined: 21 Mar 2002
Posts: 436
|
| Posted: Fri Sep 30, 2005 10:57 am Post subject:
Re: [REVIEW/TEST] polling(4) changes
|
|
|
In message <20050930134526.R71864@fledge.watson.org>, Robert Watson writes:
| Quote: | On Fri, 30 Sep 2005, Poul-Henning Kamp wrote:
I still think we should stop having this network-centric view of polling
and implement _real_ *device* polling, so that other device types can
use it as well.
While I agree that we should offer polling to non-network device drivers
also, I think it's worth observing that the network awareness of our
current polling code has some interesting advantages. [...]
|
None of which could not be implemented on top of a general polling
facility, and some of which makes polling unusable with high end
networking hardware like a 8x1Gige card where all ports are handled
by the same interrupt.
Anyway, I just wanted to make the point, not start a long discussion.
--
Poul-Henning Kamp | UNIX since Zilog Zeus 3.20
phk@FreeBSD.ORG | TCP/IP since RFC 956
FreeBSD committer | BSD since 4.3-tahoe
Never attribute to malice what can adequately be explained by incompetence.
_______________________________________________
freebsd-arch@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-arch
To unsubscribe, send any mail to "freebsd-arch-unsubscribe@freebsd.org" |
|
| Back to top |
|
|
John Baldwin
*nix forums Guru Wannabe
Joined: 27 Mar 2002
Posts: 278
|
| Posted: Fri Sep 30, 2005 12:07 pm Post subject:
Re: [REVIEW/TEST] polling(4) changes
|
|
|
On Friday 30 September 2005 08:40 am, Gleb Smirnoff wrote:
| Quote: | [please, follow-up on net@ only]
Colleagues,
here are some patches for review.
Problems addressed:
1) When Giant was removed from polling a problem was introduced. The idle
poll feature was broken. The idle poll thread can enter polling handler on
one interface and put to sleep for a long time, until CPU resources found.
During this time no traffic is received on interface. Well, this is what
idle thread is supposed to do. Why didn't this happen with Giant? Because
idle poll entered poll handler holding Giant, and other threads (in
particular netisr poll) contested on Giant and propagated their priority to
idle poll. Well, this is a hack, but idle poll significantly improves
polling performance on an idle box, that's why I won't axe it but try to
fix it.
To address the problem we need to use the same technique as before, but
use poll_mtx instead of Giant. However, this will resurrect LORs, that were
fixed with Giant removal. The alternative lock path happens, when driver
decides to deregister from polling itself. The LOR is fixed by further
changes. See 3).
2) Drivers indicate their ability to do polling(4) with IFCAP_POLLING
flag int if_capabilites field. Setting the flag in if_capenable should
register interface with polling and disable interrupts. However, the
if_flags is also abused with IFF_POLLING flag. The aim is to remove
IFF_POLLING flag.
3) The polling is switched on and off not functionally. That is, when you
say 'sysctl kern.polling.enable=1' or 'ifconfig fxp0 -polling', the polling
is switched on/off not immediately but on next tick or next interrupt. This
non-functional approach leads to a lot of ambiguouties in code, makes it
harder to understand and maintain. It also exposes race conditions.
The attached patch removes:
- IFF_POLLING flag.
- Use of if_flags, if_drv_flags, if_capenable from kern_poll.c.
All current accesses to these fields are not locked and polling
shouldn't look there.
- poll in trap feature. Sorry, we can't acquire mutexes in trap(). Anyone
used it, anyway?
- POLL_DEREGISTER command. No hacks. Everything is done functionally via
ioctl().
The new world order for driver is the following:
1) Declare IFCAP_POLLING in if_capabilities on attach. Do not touch
if_capenable. 2) in ioctl method, in SIOCSIFCAP case the driver should:
- call ether_poll_[de]register
- if no error, set the IFCAP_POLLING flag in if_capenable
- obtain driver lock
- [dis/en]able interrupts
- drop driver lock
3) In poll method, check IFF_DRV_RUNNING flag after obtaining driver lock
4) In interrupt handler check IFCAP_POLLING flag in if_capenable. If
present, then return. This is important to protect from spurious
interrupts. 5) In device detach method, call ether_poll_deregister() before
obtaining driver lock.
|
From my limited experience with locking various NIC drivers, I like this
change. I think it is much better to tweak the polling state in the ioctl()
handler rather than in the poll handler.
--
John Baldwin <jhb@FreeBSD.org> <>< http://www.FreeBSD.org/~jhb/
"Power Users Use the Power to Serve" = http://www.FreeBSD.org
_______________________________________________
freebsd-net@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "freebsd-net-unsubscribe@freebsd.org" |
|
| Back to top |
|
|
Robert Watson
*nix forums Guru Wannabe
Joined: 22 Mar 2002
Posts: 218
|
| Posted: Fri Sep 30, 2005 12:47 pm Post subject:
Re: [REVIEW/TEST] polling(4) changes
|
|
|
On Fri, 30 Sep 2005, Poul-Henning Kamp wrote:
| Quote: | I still think we should stop having this network-centric view of polling
and implement _real_ *device* polling, so that other device types can
use it as well.
|
While I agree that we should offer polling to non-network device drivers
also, I think it's worth observing that the network awareness of our
current polling code has some interesting advantages. For one thing, the
framework itself is aware of the notion of batching and moderating the
workload as it is aware of the number of mbufs being processes, and knows
to bound the workload, etc. We'll need to revisit many of the ideas in
the current polling implementation (designed largely around 4.x operating
assumptions) anyway, but I think it's important we understand some of the
implicit design benefits that are present in the current system as well...
Robert N M Watson
_______________________________________________
freebsd-arch@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-arch
To unsubscribe, send any mail to "freebsd-arch-unsubscribe@freebsd.org" |
|
| Back to top |
|
|
Gleb Smirnoff
*nix forums beginner
Joined: 30 Sep 2005
Posts: 27
|
| Posted: Fri Sep 30, 2005 2:03 pm Post subject:
Re: [REVIEW/TEST] polling(4) changes
|
|
|
On Fri, Sep 30, 2005 at 04:40:00PM +0400, Gleb Smirnoff wrote:
T> [please, follow-up on net@ only]
T>
T> Colleagues,
T>
T> here are some patches for review.
I have some changes to patch after last compile, and haven't tested them
befire sending patch. Here is an updated one.
--
Totus tuus, Glebius.
GLEBIUS-RIPN GLEB-RIPE |
|
| Back to top |
|
|
Pawel Jakub Dawidek
*nix forums addict
Joined: 24 Jun 2003
Posts: 92
|
| Posted: Fri Sep 30, 2005 4:13 pm Post subject:
Re: [REVIEW/TEST] polling(4) changes
|
|
|
On Fri, Sep 30, 2005 at 08:03:02PM +0400, Gleb Smirnoff wrote:
+> On Fri, Sep 30, 2005 at 04:40:00PM +0400, Gleb Smirnoff wrote:
+> T> [please, follow-up on net@ only]
+> T>
+> T> Colleagues,
+> T>
+> T> here are some patches for review.
+>
+> I have some changes to patch after last compile, and haven't tested them
+> befire sending patch. Here is an updated one.
BTW. Not compiling in DEVICE_POLLING has any advantages except few bytes
smaller kernel?
I wonder if we could drop yet another kernel option and just set
kern.poll.enable to 0 by default.
If adding DEVICE_POLLING to the kernel doesn't cost additional locking, etc.
in network data flow paths (which could lead to performance impact in some
specific environments) can we just compile the code in always?
--
Pawel Jakub Dawidek http://www.wheel.pl
pjd@FreeBSD.org http://www.FreeBSD.org
FreeBSD committer Am I Evil? Yes, I Am! |
|
| Back to top |
|
|
Gleb Smirnoff
*nix forums beginner
Joined: 30 Sep 2005
Posts: 27
|
| Posted: Fri Sep 30, 2005 4:23 pm Post subject:
Re: [REVIEW/TEST] polling(4) changes
|
|
|
On Fri, Sep 30, 2005 at 08:13:22PM +0200, Pawel Jakub Dawidek wrote:
P> On Fri, Sep 30, 2005 at 08:03:02PM +0400, Gleb Smirnoff wrote:
P> +> On Fri, Sep 30, 2005 at 04:40:00PM +0400, Gleb Smirnoff wrote:
P> +> T> [please, follow-up on net@ only]
P> +> T>
P> +> T> Colleagues,
P> +> T>
P> +> T> here are some patches for review.
P> +>
P> +> I have some changes to patch after last compile, and haven't tested them
P> +> befire sending patch. Here is an updated one.
P>
P> BTW. Not compiling in DEVICE_POLLING has any advantages except few bytes
P> smaller kernel?
P> I wonder if we could drop yet another kernel option and just set
P> kern.poll.enable to 0 by default.
P> If adding DEVICE_POLLING to the kernel doesn't cost additional locking, etc.
P> in network data flow paths (which could lead to performance impact in some
P> specific environments) can we just compile the code in always?
It adds a stub function call every tick. The function returns almost
immediately if no interfaces do polling.
--
Totus tuus, Glebius.
GLEBIUS-RIPN GLEB-RIPE
_______________________________________________
freebsd-arch@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-arch
To unsubscribe, send any mail to "freebsd-arch-unsubscribe@freebsd.org" |
|
| Back to top |
|
|
Andre Oppermann
*nix forums addict
Joined: 21 Mar 2002
Posts: 55
|
| Posted: Fri Sep 30, 2005 4:29 pm Post subject:
Re: [REVIEW/TEST] polling(4) changes
|
|
|
Gleb Smirnoff wrote:
| Quote: |
On Fri, Sep 30, 2005 at 08:13:22PM +0200, Pawel Jakub Dawidek wrote:
P> On Fri, Sep 30, 2005 at 08:03:02PM +0400, Gleb Smirnoff wrote:
P> +> On Fri, Sep 30, 2005 at 04:40:00PM +0400, Gleb Smirnoff wrote:
P> +> T> [please, follow-up on net@ only]
P> +> T
P> +> T> Colleagues,
P> +> T
P> +> T> here are some patches for review.
P> +
P> +> I have some changes to patch after last compile, and haven't tested them
P> +> befire sending patch. Here is an updated one.
P
P> BTW. Not compiling in DEVICE_POLLING has any advantages except few bytes
P> smaller kernel?
P> I wonder if we could drop yet another kernel option and just set
P> kern.poll.enable to 0 by default.
P> If adding DEVICE_POLLING to the kernel doesn't cost additional locking, etc.
P> in network data flow paths (which could lead to performance impact in some
P> specific environments) can we just compile the code in always?
It adds a stub function call every tick. The function returns almost
immediately if no interfaces do polling.
|
If it does a FOREACH(interface) then it should stay as a kernel option.
--
Andre
_______________________________________________
freebsd-arch@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-arch
To unsubscribe, send any mail to "freebsd-arch-unsubscribe@freebsd.org" |
|
| Back to top |
|
|
Luigi Rizzo
*nix forums addict
Joined: 07 Apr 2002
Posts: 72
|
| Posted: Fri Sep 30, 2005 5:33 pm Post subject:
Re: [REVIEW/TEST] polling(4) changes
|
|
|
On Fri, Sep 30, 2005 at 08:29:43PM +0200, Andre Oppermann wrote:
....
| Quote: | It adds a stub function call every tick. The function returns almost
immediately if no interfaces do polling.
If it does a FOREACH(interface) then it should stay as a kernel option.
|
this wasn't the case when i first wrote it - the list of
interfaces actually using polling was stored into an array and
the count was in a variable, so the loop was something like
for (i=0; i < actively_polling_interfaces; i++)
foo[i]->poll()
so it's basically just an extra function call per tick if
no interfaces are doing polling.
cheers
luigi
_______________________________________________
freebsd-arch@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-arch
To unsubscribe, send any mail to "freebsd-arch-unsubscribe@freebsd.org" |
|
| Back to top |
|
|
Gleb Smirnoff
*nix forums beginner
Joined: 30 Sep 2005
Posts: 27
|
| Posted: Fri Sep 30, 2005 7:17 pm Post subject:
Re: [REVIEW/TEST] polling(4) changes
|
|
|
On Fri, Sep 30, 2005 at 08:29:43PM +0200, Andre Oppermann wrote:
A> > It adds a stub function call every tick. The function returns almost
A> > immediately if no interfaces do polling.
A>
A> If it does a FOREACH(interface) then it should stay as a kernel option.
It isn't. Just 'if (poll_handlers == 0) return;'.
Anyway, deoptionalizing polling would be a separate commit and
discussion.
--
Totus tuus, Glebius.
GLEBIUS-RIPN GLEB-RIPE
_______________________________________________
freebsd-arch@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-arch
To unsubscribe, send any mail to "freebsd-arch-unsubscribe@freebsd.org" |
|
| Back to top |
|
|
dima
*nix forums beginner
Joined: 11 Mar 2005
Posts: 7
|
| Posted: Thu Oct 06, 2005 2:17 pm Post subject:
Re[2]: [REVIEW/TEST] polling(4) changes
|
|
|
Seems to be a first considerable step regarding the ideas discussed in March 
But, my idea about the separate locking of each interface dissappeared from this implementation. mtx_poll is good to protect the pollrec array and other sensitive variables. But we could get advantage of SMP machines writing polling loops like this:
for( i = 0; i < poll_handlers; ++i ) {
mtx_lock( &iface_lock[i] );
pr[i].handler(pr[i].ifp, POLL_ONLY, count);
mtx_unlock( &iface_lock[i] );
}
_______________________________________________
freebsd-arch@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-arch
To unsubscribe, send any mail to "freebsd-arch-unsubscribe@freebsd.org" |
|
| Back to top |
|
|
Gleb Smirnoff
*nix forums beginner
Joined: 30 Sep 2005
Posts: 27
|
| Posted: Thu Oct 06, 2005 4:34 pm Post subject:
Re: [REVIEW/TEST] polling(4) changes
|
|
|
On Thu, Oct 06, 2005 at 08:17:17PM +0400, dima wrote:
d> Seems to be a first considerable step regarding the ideas discussed in March
d> But, my idea about the separate locking of each interface dissappeared from this implementation. mtx_poll is good to protect the pollrec array and other sensitive variables. But we could get advantage of SMP machines writing polling loops like this:
d>
d> for( i = 0; i < poll_handlers; ++i ) {
d> mtx_lock( &iface_lock[i] );
d> pr[i].handler(pr[i].ifp, POLL_ONLY, count);
d> mtx_unlock( &iface_lock[i] );
d> }
What is the benefit here? The driver must have its own lock.
--
Totus tuus, Glebius.
GLEBIUS-RIPN GLEB-RIPE
_______________________________________________
freebsd-arch@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-arch
To unsubscribe, send any mail to "freebsd-arch-unsubscribe@freebsd.org" |
|
| Back to top |
|
|
dima
*nix forums beginner
Joined: 11 Mar 2005
Posts: 7
|
| Posted: Fri Oct 07, 2005 7:28 am Post subject:
Re[2]: [REVIEW/TEST] polling(4) changes
|
|
|
| Quote: | d> Seems to be a first considerable step regarding the ideas discussed in March
d> But, my idea about the separate locking of each interface dissappeared from this implementation. mtx_poll is good to protect the pollrec array and other sensitive variables. But we could get advantage of SMP machines writing polling loops like this:
d
d> for( i = 0; i < poll_handlers; ++i ) {
d> mtx_lock( &iface_lock[i] );
d> pr[i].handler(pr[i].ifp, POLL_ONLY, count);
d> mtx_unlock( &iface_lock[i] );
d> }
What is the benefit here? The driver must have its own lock.
|
Well, consider the absense of the mtx_poll lock:
- mtx_lock( &mtx_poll );
for( i = 0; i < poll_handlers; ++i ) {
+ mtx_lock( &iface_lock[i] );
pr[i].handler( pr[i].ifp, POLL_ONLY, count );
+ mtx_unlock( &iface_lock[i] );
}
- mtx_unlock( &mtx_poll );
So, several kernel threads in an SMP machine can poll different interfaces simultaneously. And mtx_lock should only be used in ether_poll_[de]register().
_______________________________________________
freebsd-arch@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-arch
To unsubscribe, send any mail to "freebsd-arch-unsubscribe@freebsd.org" |
|
| Back to top |
|
|
Google
|
|
| Back to top |
|
|
|
|
The time now is Thu Jan 08, 2009 9:28 am | All times are GMT
|
|
McDonalds | Dutch Bodybuilding Forums | Credit Counseling | Fantasy | Debt Consolidation
|
|
Copyright © 2004-2005 DeniX Solutions SRL
|
|
|
|
Other DeniX Solutions sites:
Unix/Linux blog |
electronics forum |
medicine forum |
science forum |
|
|
Privacy Policy
|
Powered by phpBB © 2001, 2005 phpBB Group
|
|
FAQ
Search
Memberlist
Usergroups
Register
Profile
Log in to check your private messages
Log in
Forum index
»
*nix
»
BSD
»
FreeBSD
»
mail-lists
»
Architecture
