On Tuesday 23 March 2004 19:12, Pete Zaitcev wrote:
> On Tue, 23 Mar 2004 09:12:00 +0100
>
> Oliver Neukum <[EMAIL PROTECTED]> wrote:
> > Am Tuesday 23 March 2004 01:28 schrieb Pete Zaitcev:
> > > On Mon, 22 Mar 2004 22:21:14 +0100
> > >
> > > Oliver Neukum <[EMAIL PROTECTED]> wrote:
> > > > you are using an irqsave spinlock in a completion handler. Not
> > > > actually wrong but it tends to confuse people about the context the
> > > > function is executed under.
> > > >
> > > > priv = usb_get_serial_port_data(port);
> > > > - spin_lock_irqsave(&priv->lock, flags);
> > > > + spin_lock(&priv->lock);
> > > > priv->last_msr = data[BELKIN_SA_MSR_INDEX];
> > >
> > > I strongly disagree with this point of view. Keeping track of a
> > > context in which a function can be called places an unreasonable
> > > burden on a programmer and introduces extremely annoying code rot
> > > whenever code migrates contexts.
> >
> > You cannot efficiently write code that runs under several contexts.
> > Too many things mostly related to the ability to sleep depend on the
> > context. We continously get buggy drivers which sleep in interrupt
> > because people are not aware.
>
> What you say is true for any non-trivial code, especially if it does
> any memory allocations. We learned this at a high price with Youngdale's
> SCSI which basically made programmers to assume an interrupt context
> throughout the stack and thus precluded GFP_KERNEL completely.
Precisely. Block IO paths make things even worse. Exactly for this reason
I introduced a second argument to usb_submit_urb()
> However, a small spinlock bracket in a device driver is not the place
> to apply this wisdom, primarily because a well used paradigm there
> looks like this:
If this were random lines of code I would absolutely agree.
But this is code especially intended as an interrupt handler.
[..]
> Please consider that sleeping in an interrupt is catcheable by BUG(),
> but not closing interrupts when needed is not catcheable. Thus, I will
> happily trade you all such bugs for those where the code races
> undetected.
Well, there is might_sleep(), but generally you don't catch places that
might sleep, you just catch those which do, which can be very unlikely.
So this is not an either or question, especially as that code wouldn't
race, it would deadlock.
So I would say that usually better be safe than sorry and use _irqsave,
but not in a place _designed_ to be nothing but an irq handler and in
a small driver in a prolific category of devices which might well be copied
and lead people to conclude that it is safe to add a down() there.
Regards
Oliver
-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of
GenToo technologies. Learn everything from fundamentals to system
administration.http://ads.osdn.com/?ad_id=1470&alloc_id=3638&op=click
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
