confusing code....whats the point of this construct ?

[Jeff Haran]

-----Original Message-----
From: Valdis.Kletnieks at vt.edu [mailto:Valdis.Kletnieks at vt.edu] 
Sent: Wednesday, March 11, 2015 11:58 AM
To: Jeff Haran
Cc: Nicholas Mc Guire; Bj??rn Mork; kernelnewbies at kernelnewbies.org
Subject: Re: confusing code....whats the point of this construct ?

On Wed, 11 Mar 2015 18:37:32 -0000, Jeff Haran said:

> I don't understand the problem here. The caller passes in a condition 
> to be evaluated in a loop. Many times that condition is quite simple 
> (e.g. a counter being non-zero). If it was a function the caller would 
> have to pass in a pointer to a function that does the evaluation, as in:

>We do pointers to callback functions all the time.  We try *really* hard to avoid anonymous lambda functions (which is basically what we have here).
>
>The problem here is that there's about 3 zillion ways to screw up the inline version, starting with the compiler optimizing the control variable into a >hoisted load outside the loop and causing the test to always fail - note that the macro does *not* use any barriers or volatile or anything like that.

Dealing with those issues has to be a problem for the caller to solve in whatever condition code he decides to pass in, not the implementation of the macro itself. I agree with Nicholas M. that it would have been easier to read had the condition been packaged into an inline function, but if whoever writes that inline function gets the optimization/barriers wrong then its broken regardless of whether it's written inline like it is now or packaged into a separate inline function. As for volatile, I had thought that the usage of volatile in kernel code was generally discouraged.

This construct is all over the place in the kernel. Changing theses wait macros into functions would be a big lift.

Jeff Haran