cvs commit: src/sys/dev/fxp if_fxp.c

[Maxime Henrion]

Nate Lawson wrote:
> On Fri, 11 Apr 2003, Maxime Henrion wrote:
> > Nate Lawson wrote:
> > > On Thu, 10 Apr 2003, Maxime Henrion wrote:
> > > >   Modified files:
> > > >     sys/dev/fxp          if_fxp.c 
> > > >   Log:
> > > >   - Clean up the fxp_release() and fxp_detach() functions.
> > > 
> > > There's a version of this in the diff I just posted to current at .
> > 
> > Feel free to commit it.
> 
> I'd like to keep it bundled with the MPSAFE changes so it will have to
> wait a little while for more testing.
> 
> > > I have been testing my diff by loading and unloading fxp while doing a
> > > large transfer and I cannot replicate this.  Are you sure it's not a local
> > > problem?  I have never had a deadlock or a crash and loading fxp again
> > > always works.
> > 
> > I have no idea, this is why I was saying it's maybe not fxp(4)'s fault
> > and it may indeed be a local problem.
> 
> It would help if others gave their insight but if loading/unloading work
> for me (and have been for months), it suggests it is local.  I don't have
> a problem with making the unmap conditional on the tag (first part of
> commit).
>  
> > > Um, fxp_detach() should not be called for any case where the device isn't
> > > alive.  fxp_detach should ONLY be called once attach has succeeded which
> > > by definition means the device is alive.  bus_child_present() is the
> > > bus-specific method to see that the hardware is actually there;
> > > device_is_alive only tells you that the device_t node is present in the
> > > tree.  fxp_release may be called in error cases.  Rather than working
> > > around your problem this way, please find what is calling fxp_detach when
> > > the device is not alive.
> > 
> > This way of doing things (ie: using device_is_alive()) is an almost
> > verbatim copy of what you did in every network driver in sys/pci/.
> > Now if it's wrong, feel free to change it, but I guess you'll have
> > to change all those drivers too then.
> 
> The reason why I did that for the other drivers is that there was not a
> separate "release" function which deallocated resources.  Instead, I had a
> common routine which did detach and release.  For the error case, you do
> need to check if the device was alive before calling *stop() on
> potentially non-working hardware.
> 
> But for fxp, this is done explicitly -- routines which can only be done on
> a "live" device go in fxp_detach() and those that need to be done in both
> the error and detach cases go in fxp_release().  Thus the second part of
> your commit is unnecessary.

Ok.  This is fixed now, as is fixed the hangs I was seeing, which I now
believe weren't caused by a local change.

Thanks,
Maxime