Am Freitag, 22. Juni 2007 schrieb Pete Zaitcev:
> > > > > static unsigned int usblp_poll(struct file *file, struct
> > > > > poll_table_struct *wait)
> > > > > {
> > > > > + int ret;
> > > > > + unsigned long flags;
> > > >
> > > > We are called with interrupts on.
> > >
> > > We discussed it a few times before. I guess my use of spin_lock in
> > > callbacks wasn't enough to appease you, eh?
> >
> > No, because here it bothers me for another reason.
> > It is just a few cycles. But it makes the code harder to maintain.
> > The next one to touch this will look at it, scratch his head and
> > use GFP_ATOMIC just in case or do some funny stuff just in
> > case somebody might call this function with a spinlock held.
>
> I'll have to think about this. The main issue is, the code travels
> contexts over time, and thus context-specific code like spin_lock_irq
> gets relocated into improper context. In this specific case, the
That is exactly what you need to prevent. If you try to make code that
will run under all contexts, you'll get code that usually works. And that
is code that is worse than code that never works.
I am sure that if your code is sufficiently complex, and these are the cases
that matter, you will overlook something, eg. a mutex_lock() four functions
down the callchain in a cleanup code for an error case. You are better
off with code that right away and obviously blows up if you change the
context it is called in. We need to _force_ people to think about context
and if they really need functions that can be called from many contexts,
it must be made sure that this code is audited and tested for that use.
Those who have to do that work will curse you for that, but you'll have
made sure that the new code works under all circumstances.
> methods affected are not deeply nested away from a known API and
> thus we do know that it will not happen... But it's only here.
> I am trying not to create the pattern of coding which you advocate.
>
> We keep fixing bugs where spin_lock_irqsave should've been used but
> wasn't. Here's how it all started:
> http://www.advogato.org/person/Zaitcev/diary.html?start=153
If I may quote you: "I place the blame squarely on premature optimization."
Which is exactly my point. Write code for the way it is used. Don't make
an attempt to cover all uses from the very start, only cross bridges that
must be crossed.
Regards
Oliver
-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
