On Mon, May 13, 2002, David Brownell <[EMAIL PROTECTED]> wrote:
> Simple fix: don't merge Johannes' changes.
That argument works both ways.
But since what we're doing is standard for the rest of the kernel, the
burden of proof is on you to convince everyone to change.
> > That's a device driver bug. If it's still hanging onto references and
> > the module is removed, it's a bug.
> >
> > That has nothing to do with the core, or HCD's.
>
> Johannes, your arguments have boiled down to not wanting
> the core to detect or prevent such oopses. I really don't
> understand why, since the cost to correctly written device
> drivers is zero and the _value_ of detecting/preventing such
> problems is significantly higher. (Measurable in years that
> such bugs have otherwise been creating random problems
> before someone finally figures out what's wrong. Consider
> cases like "printer" and "usbfs".)
Nope. That's not how the kernel works. We optimize for ease of coding,
maintainence and understanding.
Your patch doesn't do any of that.
> > NO, NO, NO. usb_free_dev() is called when the last reference is
> > decremented, which can be in ANY context!
> >
> > But it doesn't matter since usb_free_dev doesn't do any blocking.
>
> The HCD is most _certainly_ allowed to block when cleaning up
> device state. Every device management API I've had reason to
> look at in Linux guarantees that the the "clean up after this device"
> call can block.
You haven't been looking hard enough. Take a look at all of the
networking code.
Most of it runs in the interrupt handlers.
> I don't care at all about the memory management, except that as I
> said elsewhere, I can't see any case where the device memory
> should reasonably be kept around after the cleanup call is made.
Keep thinking then. Everything is easier. Much of what's in usb-ohci's
deallocate callback isn't needed anymore.
> > > If we add the BUG() mentioned above, how are things more fragile?
> > > Personally, they look more stable to me.
>
> Today (2.5.15) the BUG() test is being made. That's how I can
> say that they're not going to trigger such bugs, and how overall
> stability is better today.
That could be because you made the appropriate changes to work with your
version of reference counting.
That's not much of an argument for why it's superior.
> And the ONLY lack of stability is for folk using "uhci.c" ... or for
> new drivers that haven't gotten rid of disconnect() bugs yet. Which
> IMO is a good sign, particularly since the updates to "uhci.c" are
> so simple, and since the new device drivers will get nice clean
> BUG() reports if they screw up.
I wouldn't call it "so simple", but I wouldn't call it overly
complicated either.
It doesn't matter tho, since the reference counting patch I sent results
in less code that your version of reference counting.
That's one reason why it's superior.
> > Just code the drivers correctly.
> >
> > The kernel is not the place to coddle coders who cannot code correctly.
> > It's a dangerous place and it'll remain so for a long time.
>
> Likewise, the kernel is not the place to design-in fragility for the
> component interconnections. (Fragility like hiding errors.)
No. Removing extra debugging does not equate to designing in fragility.
If you're really that worried about it, I suggest you tackle the
networking code first, since infinitely more people use that than the
USB code.
> It's a place where good coding discipline -- like defining and preserving
> invariants of all kinds, like the BUG() tests you object to -- is well
> rewarded in terms of system stability.
You keep ignoring all of the other arguments.
Like I've said in the past, making things easier to debug is good, but
not when it's at the cost of simplicity and breaking other things.
Take a look at devio.c:proc_control and explain to me how you can
make that safe (which it isn't completely right now) with your method of
reference counting.
It's easy with the patch I sent to revert it to the 2.4 method of
reference counting:
--- linux-2.5.15/drivers/usb/core/devio.c.old Mon May 13 16:17:55 2002
+++ linux-2.5.15/drivers/usb/core/devio.c Mon May 13 16:22:25 2002
@@ -502,6 +502,7 @@
ret = -ENOMEM;
if (!(ps = kmalloc(sizeof(struct dev_state), GFP_KERNEL)))
goto out;
+ usb_inc_dev_use(dev);
ret = 0;
ps->dev = dev;
ps->file = file;
@@ -538,6 +539,7 @@
unlock_kernel();
destroy_all_async(ps);
kfree(ps);
+ usb_dec_dev_use(dev);
return 0;
}
You can't with your method because you erroneously assume that reference
counting MUST be tied to grabbing interfaces.
That's not the case when I just open a device and want to do control
transfers since it doesn't grab any interfaces.
JE
_______________________________________________________________
Have big pipes? SourceForge.net is looking for download mirrors. We supply
the hardware. You get the recognition. Email Us: [EMAIL PROTECTED]
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
