Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 04 Mar 2001 15:37:40 -0700
From:      "Justin T. Gibbs" <gibbs@scsiguy.com>
To:        John Baldwin <jhb@FreeBSD.org>
Cc:        cvs-all@FreeBSD.org, cvs-committers@FreeBSD.org
Subject:   Re: cvs commit: src/sys/kern kern_intr.c src/sys/sys interrupt.h 
Message-ID:  <200103042237.f24MbfO84003@aslan.scsiguy.com>
In-Reply-To: Your message of "Sun, 04 Mar 2001 14:23:03 PST." <XFMail.010304142303.jhb@FreeBSD.org> 

next in thread | previous in thread | raw e-mail | index | archive | help
>
>I'd rather have the code not care either way to make it easier on driver
>writers if need be.  I do agree that drivers probably shouldn't be holding
>locks while removing handlers.  Note that pccard does make a lot of the cases
>more extreme as the hardware can leave at any time.

I don't think you can do this and still give the deregister method the
semantics that on return your handler is not and can not ever execute
again.


>>>This _still_ doesn't help if we are already executing the handler.
>> 
>> Sure it does.  Once the new handler is run, you are guaranteed that the old
>> handler
>> is not running and can cannot be run again.  If the handler is running, the
>> new handler
>> will not be run until the next time through the ihandler list.  The trick is
>> making
>> sure that the ithread loops at least one more time.  So, if you sleep until
>> the
>> your new handler is run, you are always safe.
>
>Erm, did you read what my code does?

We're talking past each other.  Forget what your code does for a moment.  Instead
of removing the entry directly, you simply change the handler to a function that
will wake you up after removing the entry.  The entry is then always removed from
the ithread context.

>If I have registerd the foo()
>function and the ithread blocks

It doesn't need to block for the race to occur.  I just need to deregister from
another CPU.

>while it is executing foo() and while it is blocked another thread removes
>foo() from the list, what possible good can changing the functino pointer
>do to change the fact that the ithread is executing foo()?

It seemed the easiest way to register state about who would be woken up as
the entry is really and truely killed off.  If you want to use a different
method, so be it, but whatever it is, it must allow mutiple, concurrent
deregistrations.

>Also, since I set it_need, I _do_ force
>the ithread to loop again to purge the item from the list.  I guess
>your new change to this is you want to sleep on the intrhand until the
>ithread runs to release it?  I don't see why that is needed.  Once the
>intrhand is dead, we don't dereference the function pointer again, so
>we aren't in danger of calling the function on a free'd softc, etc.

Rather you are in danger of freeing the softc just before the interrupt
handler is run.  This will always be the case in your scheme unless you
hold a lock across the call to the interrupt handler and that lock is
held by the interrupt deregistration method to close the race.  There
may also be cases in the interrupt handler where you release your
softc lock.  If, during that window, the detach method decides to
destroy your driver mutext or otherwise change the state of the softc
(hey, I've dregistered my interrupt handler, so who else could need
this any more?), you are hosed.

>The only bad case is if the function is already running, and changing
>the function pointer doesn't do anything to help with that.

Blocking is what fixes it and by storing *some state* in the handler
entry itself, you can handle more than one waiter for different
deregistrations at the same time.

>> How do you avoid the race when two threads attempt to add onto the list at
>> the same time?  A single atomic store doesn't help you unless you can safely
>> test and recover from a colision.
>
>The mutex protects against this.  The only unprotected case is the ithread
>walking the list while another thread is adding an item.

Okay.

--
Justin

To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe cvs-all" in the body of the message




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