]> CyberLeo.Net >> Repos - FreeBSD/FreeBSD.git/commit
Avoid a race and a pessimization in bge_intr():
authorBruce Evans <bde@FreeBSD.org>
Wed, 20 Dec 2006 11:14:45 +0000 (11:14 +0000)
committerBruce Evans <bde@FreeBSD.org>
Wed, 20 Dec 2006 11:14:45 +0000 (11:14 +0000)
commitb848e03260f91336b2ee000521a6d30a6f6d5aeb
tree6027521f10e50dab59d90ae53da77e01bfe8a941
parentcc570216bb8985e0df63d8629db89c17e918113c
Avoid a race and a pessimization in bge_intr():

- moved the synchronizing bus read to after the bus write for the first
  interrupt ack so that it actually synchronizes everything necessary.

  We were acking not only the status update that triggered the interrupt
  together with any status updates that occurred before we got around
  to the bus write for the ack, but also any status updates that occur
  after we do the bus write but before the write reaches the device.
  The corresponding race for the second interrupt ack resulted in
  sometimes returning from the interrupt handler with acked but
  unserviced interrupt events.  Such events then remain unserviced
  until further events cause another interrupt or the watchdog times
  out.

  The race was often lost on my 5705, apparently since my 5705 has broken
  event coalescing which causes a status update for almost every packet,
  so another status update is quite likely to occur while the interrupt
  handler is running.  Watchdog timeouts weren't very noticeable,
  apparently because bge_txeof() has one of the usual bugs resetting the
  watchdog.

- don't disable device interrupts while bge_intr() is running.  Doing this
  just had the side effects of:
  - entering a device mode in which different coalescing parameters apply.
    Different coalescing parameters can be used to either inhibit or
    enhance the chance of getting another status update while in the
    interrupt handler.  This feature is useless with the current
    organization of the interrupt handler but might be useful with a
    taskqueue handler.
  - giving a race for ack+reenable/return.  This cannot be handled
    by simply rearranging the order of bus accesses like the race for
    ack+keepenable/entry.  It is necessary to sync the ack and then
    check for new events.
  - taking longer, especially with the extra code to avoid the race on
    ack+reenable/return.

Reviewed by: ru, gleb, scottl
sys/dev/bge/if_bge.c