Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 29 Apr 2003 14:56:43 -0400 (EDT)
From:      Andrew Gallatin <gallatin@cs.duke.edu>
To:        John Baldwin <jhb@FreeBSD.org>
Cc:        cvs-all@FreeBSD.org
Subject:   Re: cvs commit: src/sys/dev/fxp if_fxp.c if_fxpvar.h
Message-ID:  <16046.51947.425815.273156@grasshopper.cs.duke.edu>
In-Reply-To: <XFMail.20030429144430.jhb@FreeBSD.org>
References:  <16046.49785.35005.126940@grasshopper.cs.duke.edu> <XFMail.20030429144430.jhb@FreeBSD.org>

next in thread | previous in thread | raw e-mail | index | archive | help

John Baldwin writes:

 > 
 > First off, it's a gross API violation.  ithread's are supposed to be
 > transparent (except that you can use locks in your interrupt handlers).

The API seems designed for inefficiency, so it needs violating ;)

 > Secondly, you still have the same race as if you just removed the gone
 > condition.  We don't hold the ithread lock while executing handlers,
 > so there is nothing preventing the handler from being executed on another
 > CPU concurrently with your detach function.  In fact, it could easily
 > be blocked on the FXP lock.  You do your magic pointer foo, then unlock
 > the fxp.  The unlock releases the interrupt handler on another CPU
 > which happily executes _after_ the completion of bus_teardown_intr()

I assumed dropping the fxp lock would be enough to encourage any
pending handlers to finish.  Would simply tsleep()'ing for a second
work?

If needed we should violate the API even more to check to see if any
handlers are pending.  Anything is better than adding instructions to
the critical path.

We really need to think about efficiency.  Our 5.x performance sucks.
Really sucks.  We're being nickled and dimed to death by extra
instructions here, there, and everywhere.

Drew



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?16046.51947.425815.273156>