Date: Fri, 30 Sep 2005 10:07:09 -0400 From: John Baldwin <jhb@FreeBSD.org> To: freebsd-arch@freebsd.org Cc: Gleb Smirnoff <glebius@freebsd.org>, net@freebsd.org Subject: Re: [REVIEW/TEST] polling(4) changes Message-ID: <200509301007.10494.jhb@FreeBSD.org> In-Reply-To: <20050930124000.GA45345@cell.sick.ru> References: <20050930124000.GA45345@cell.sick.ru>
next in thread | previous in thread | raw e-mail | index | archive | help
On Friday 30 September 2005 08:40 am, Gleb Smirnoff wrote: > [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 id= le > 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 we= re > 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 y= ou > say 'sysctl kern.polling.enable=3D1' or 'ifconfig fxp0 -polling', the pol= ling > is switched on/off not immediately but on next tick or next interrupt. Th= is > 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(). Anyo= ne > 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 lo= ck > 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() befo= re > obtaining driver lock. =46rom my limited experience with locking various NIC drivers, I like this= =20 change. I think it is much better to tweak the polling state in the ioctl(= )=20 handler rather than in the poll handler. =2D-=20 John Baldwin <jhb@FreeBSD.org> <>< http://www.FreeBSD.org/~jhb/ "Power Users Use the Power to Serve" =3D http://www.FreeBSD.org
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200509301007.10494.jhb>