On Fri, 2007-09-07 at 14:09 +0100, Paul Durrant wrote:
> On 07/09/2007, Sebastien Roy <[EMAIL PROTECTED]> wrote:
> > > FWIW, I find the current mac_register_t scheme ugly; I don't see why a
> > > statically defined mac_register_t cannot simply be passed into
> > > mac_register(). I don't see why we need separate allocator functions
> > > there.
> >
> > It's done this way so that the structure can be expanded without
> > affecting binary compatibility. We can add a new field to the structure,
> > and the system won't crash and burn when an existing driver registers.
> > The framework will allocate an appropriately sized structure, but the
> > older driver simply won't fill in the new fields it doesn't know about.
> >
>
> Agreed, but you can still do that by the driver exporting a version
> field in a statically defined structure. The framework just parses the
> versioned structure and copies relevant fields into the mac_impl_t. No
> need for the extra function calls.
>
> > > I also don't see that you can't break this race using reference
> > > counting. There's no need for mac_unregister() to actually free the
> > > mac_impl_t; it could just drop a ref. and the last man out frees.
> > > Let's try not to complicate the interface because of implementation
> > > artifacts.
> >
> > I don't believe a reference counting scheme makes a difference in the
> > scenario that Thiru described. He's describing a scenario where the
> > mac_impl_t is freed by mac_unregister() before another thread gets a
> > chance to examine that mac_impl_t to verify its contents (either for a
> > flag or a reference count, it doesn't matter, the end result is still the
> > same; boom!)
> >
>
> Ok. I misunderstood the argument.
>
> > IMO, this case is borderline a driver bug. The driver has called some
> > MAC function (which hasn't returned yet), and decides to call
> > mac_unregister(). mac_unregister() is documented to invalidate the MAC
> > handle, so drivers shouldn't knowingly call mac_unregister() while using
> > that handle, or use the handle during or after calls to mac_unregister().
> >
> > To me, this is a similar problem to calling free() on a buffer that an
> > application knows that it's still using... Synchronization on the
> > allocation and deallocation of that buffer can't be done by the memory
> > framework, it has to be done by the application. The framework can't
> > possibly know what the application is going to do with that buffer once
> > the free() function has been called.
> >
>
> Absolutely agree. If a driver is stupid enough to mac_unregister()
> whilst there are outstanding threads up-calling then it deserves all
> it gets!
Absolutely *disagree*.
Every driver does this.
The only way that a driver can find out if it really *should* cancel all
those up calling threads (some of which are asynchronous) is to either
a) call mac_unregister, but that has this race condition
b) first cancel those threads, but that may result in loss of data if
unregister is called while upstream providers are present, and creates a
new failure mode (what if you destroy a timeout, or a cyclic, but then
later on when the mac is found to be busy cannot *add* a new one)
c) use a lock. except that is verboten to hold a lock across
mac_unregister, so we wind up with some more complex lock, flag, and
condition variable scheme in the drivers. This adds a *lot* of
complexity to the drivers.
All of this is a royal PITA. For no good reason, except that the API
for the MAC was poorly designed in this regard. Attempting to tie mac
registeration/unregistration with allocation/destruction is the root
cause here, and it gains us nothing but massive headaches.
Can we please just fix the Nemo layer? (Note again, I've offered to
make the requisite changes *myself*.)
-- Garrett
>
> Paul
>
_______________________________________________
networking-discuss mailing list
[email protected]
