[REVIEW/TEST] polling(4) changes

From: Gleb Smirnoff (glebius_at_FreeBSD.org)
Date: 09/30/05

  • Next message: Poul-Henning Kamp: "Re: [REVIEW/TEST] polling(4) changes" 
    Date: Fri, 30 Sep 2005 16:40:00 +0400
To: arch@FreeBSD.org, net@FreeBSD.org

      [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(8)
      - 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

    _______________________________________________
    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"

  • Next message: Poul-Henning Kamp: "Re: [REVIEW/TEST] polling(4) changes"

    Relevant Pages

    • [REVIEW/TEST] polling(4) changes
      ... When Giant was removed from polling a problem was introduced. ... The idle poll thread can enter polling handler on ...
      (freebsd-net)
    • Re: [antinvidia@gmail.com: some questions about bge(4)]
      ... known bugs cause brgphy_serviceto often lose a packet. ... if the system can't actually poll at 1/HZ. ... since at least 1500 packets of buffering would be needed for polling ... Polling in idle can work better if the system is actually idle. ...
      (freebsd-net)
    • Re: [REVIEW/TEST] polling(4) changes
      ... > 1) When Giant was removed from polling a problem was introduced. ... The idle poll thread can enter polling handler on ... Why didn't this happen with Giant? ...
      (freebsd-arch)
    • Re: [REVIEW/TEST] polling(4) changes
      ... > 1) When Giant was removed from polling a problem was introduced. ... The idle poll thread can enter polling handler on ... Why didn't this happen with Giant? ...
      (freebsd-net)
    • Re: [antinvidia@gmail.com: some questions about bge(4)]
      ... known bugs cause brgphy_serviceto often lose a packet. ... But I've enabled device polling ... the lost poll will increase by at ... Polling in idle can work better if the system is actually idle. ...
      (freebsd-net)