20080110

Spot the Bug!

I was looking over some code for someone today. They were having problems with preemption in the write routine and couldn't understand why, since they said to take Giant for all devsw operations. In digging into the problem, I discovered the following code. Maybe you can spot the bug:

#ifdef D_NEED_GIANT
#define FOO_GIANT D_NEEDGIANT
#else
#define FOO_GIANT 0
#endif

(FOO_GIANT is used in the devsw data structure to paper over differences between 4.x and 6.x in this driver).

I think I know who did this change, but without cvs access to the code I can't be sure. Which is good, because I think it was me, but it might have been the person I was helping out. The change was made almost 4 years ago. This code has been in there that long and it wasn't until an especially demanding application came along on a faster CPU was it a problem.

Just goes to show you can never be too careful, or test too much...

7 comments:

Anonymous said...

For bonus points, there's the excellent grammar that goes with the D_NEEDGIANT definition in sys/conf.h!

Warner Losh said...

Please note: I'm rejecting any spoilers that are posted as comments.

Hint: One of these things is not like the other.

Anonymous said...

That's why it's often more safe to use this construction:

#ifndef D_NEEDGIANT
#define D_NEEDGIANT 0
#endif

Unknown said...

D_NEED_GIANT != D_NEEDGIANT

Warner Losh said...

I've gone ahead and let the answer appear in the comments... Mostly since Ed schouten's reply seemed to be the most obvious solution to this problem.

Joey Joe Joe Shabbadoo said...

"Me? I'm The Onion Rings."

Whats is that from?? Driving me crazy....please tell me!

Joey Joe Joe Shabbadoo said...

email the onion rings answer to:

oddlaww@yahoo.com

MUCH APPRECIATED!