Eeesh, two places to call mutex_exit(). I suppose a good compiler would recognize the common basic blocks and optimize them out, but that's still mildly ugly to look at. Still, that above code just rubbed me the wrong way, even though I KNOW there are other bits of Illumos that are like the above. I didn't block the reviewer, but I did write down what I thought it should look like:

That seems simpler. The operation is encapsulated in the mutex_{enter,exit} section, and there are no escape hatches save those in the while boolean. (It's the always-drop-the-mutex-upon-return that makes language constructs like monitors look appealing.)

I think I'm probably making a bigger deal out of this than I should, but the last code looks more readable to me.

One thing the reviewee suggested to me was that a for loop like before, but with breaks, would be equally clean w.r.t. only having one place to drop the mutex. I think the reviewee is right, and it allows for more sophisticated exits from a loop.

Some people would even use "goto fail" here, but we know what can happen when that goes wrong. :)