Messages in this thread | | | Date | Mon, 12 Sep 2005 08:32:07 -0700 | From | Paul Jackson <> | Subject | Re: [PATCH] cpuset semaphore depth check optimize |
| |
Linus wrote: > and then operate on _that_ level instead.
As I noted in my reply a few minutes ago, the one unusual rule that this scheme imposes is that all up's and down's on cpuset_sem must be done via the wrappers.
So I have continued to strive to have the lock and unlock calls have as literal substrings "up(&cpuset_sem)" and "down (&cpuset_sem)", such as with "cpuset_up(&cpuset_sem)" and "cpuset_down(&cpuset_sem)".
This serves as a clear visual reminder of this extra wrapper rule.
The usual "best practices" of: 1) consistent API's (referring to Nikita's suggestion that these routines have "void" arguments instead of "&cpuset_sem"), 2) encapsulating related data (your suggestion here), and 3) [in my inbox] Nikita's cpuset_lock/cpuset_unlock hiding, echoing an earlier suggestion of Linus's
are appropriate and desirable mechanisms for building clean abstraction layers.
I am more of a mind to code this as a thinly veiled hack for use just within cpuset.c, not another abstraction layer.
I can certainly code this as a proper layer, if you like. My intuition is that, in this case, doing so would slightly increase the mental load on the reader, not decrease it.
In actuality, I don't code for elegance so much as I code to minimize the time it takes the typical reader to -correctly- understand what's going on.
But if after all my eloquence of the last hour, Linus, Nikita and Andrew are all in agreement that cpuset_lock/cpuset_unlock with struct encapuslation of the 3 data items is preferrable, I'll gladly code that up. Well, actually, just a single clear "make it so" from Linus or Andrew would likely be sufficient.
-- I won't rest till it's the best ... Programmer, Linux Scalability Paul Jackson <pj@sgi.com> 1.925.600.0401 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |