"Unless I'm missing something, your request for revert was pretty rude, technically incorrect and you also tried to circumvent the normal course of discussion."
"Unless I'm missing something, your request for revert was pretty rude, technically incorrect and you also tried to circumvent the normal course of discussion."
At first I thought the quote was from Con Kolivas, but then I saw it was from Ingo "Kudos to Con" Molnar. I find this statement hypocritical from someone who so brazenly plagarized Con's work, and circumvented all norms of collaboration. Sadly, with the help of his scheduler mafia friends, he seems to have gotten away with it.
If you had actually followed any of the debates you'd know that Con's RSDL and Ingo's CFS schedulers have nearly nothing in common technically. And yes he did give credit to Con Kolivas for coming up with the fairness aspect. So you kind of "forgot" to explain how exactly is this plagarism.
and circumvented all norms of collaboration
Again, groundless rants presented without any reasoning. Is using inspiration from someone else's code to improve your own "circumvention of all norms of collaboration"? Is it competing with someone who proved that the previous scheduler sucked? Is it being responsive to criticism from users? Is it being a more highly respected kernel hacker than your competition? Is it out-doing your competition in the end?
Or is it just that trolls are running out of arguments (if they had any in the first place)? You'd better start backing up your claims, or at least suggest what you're on about.
he seems to have gotten away with it
And the community got away with a better scheduler. This is the open source development model for you. When Con Kolivas says that he will code a kickass scheduler, nobody guarantees that his (however good) code will be merged.
Yes it is regrettable that Con left kernel development. However, the scheduler was not the only thing he had trouble merging into the mainline; it was simply the last one. I am not sure why his patches were not being favored, but if there's something I've learnt from previous controversies, unleashing a *users* mailing list in a battle against the LKML is not a good way to make friends with Linux developers.
What we really need is someone's impartial analysis of what went wrong, and what could be done differently the next time. With time this could be converted into a guide of "DOs and DON'Ts" for getting your patches merged, and how not to scare off newer contributors.
Check out the full paragraph... Ingo's rather concerned with the behavior of the person in question:
so unless i'm missing something, your request for revert was pretty
rude, technically incorrect and you also tried to circumvent the normal
course of discussion. I have no strong feelings either way technically
(the patch is borderline - we dont actively pass around const
task_struct pointers at the moment - but we could start doing so, if we
had the constification), but i do have strong feelings against the kind
of behavior you showed here.
That certainly does seem like a red flag. It's bad form to push a patch without consulting the active maintainers. It's worse form to push a patch that was turned down by the maintainers.
On the technical side, I'm with Ingo on the overall use of const to indicate whether a function and its callees modify the contents of a particular object in a called function. From the standpoint of "improving code generation," const isn't as useful as it once was, though: At one time, some compilers used const for similar purposes as C99's new restrict keyword. *sigh* The particular interpretation of the standard that allowed those optimizations is now considered invalid. (If you're curious, feel free to ask.)
Stating, though, as a matter of internal API that the caller is free to assume a given function doesn't modify a particular argument is important. And if someone tries to change that assumption, it can be worth having the compiler alert you to that sort of break in the API. For example, consider code like this:
struct whatsit *foo = next_struct_whatsit();
int local_variable = foo->field1 - foo->field2;
int flag;
/* bits of code here */
flag = compute_something(foo);
/* should 'local_variable' be considered valid here? */
If compute_something() takes a const struct whatsit*, it's very clear that the computation of flag can never modify foo->field1 or foo->field2, and therefore local_variable would have the same value if it were computed before the call or computed after. Furthermore, the compiler can choose to recompute it after the call if it results in better register allocation. Such code tranformations are obviously correct. The semantics of local_variable are very clear with respect to compute_something().
If compute_something() takes just struct whatsit*, one needs to know something more about the definition of that function and any other function it calls to know for sure that local_variable has a usable and meaningful value after the call. And, if compute_something does change either field in this situation, a person reading the code has to figure out if the caller really wants local_variable in terms of the old or new values of these fields. (There are cases when you want one or the other.) This increases the mental load on the programmer and the likelihood they misinterpret the code when viewing it in isolation.
I don't understand what you mean by "considered valid". local_variable is computed as the difference between two fields of fooat a particular point in time, and any assumption that the equality still holds after the call to compute_something would have to be up to the actual intent of the code the programmer has written, and the language rules have no business second-guessing this decision.
Optimization isn't about second guessing. Optimization is about producing more efficient code within the bounds of the semantics of the supplied program.
In this case, if the compiler can prove that compute_something(foo) cannot change the value of those two fields, it can move the computation of local_variable after the call (or even compute it more than once!) if that generates better overall code. Now, certain types of calls, such as those that take a lock, should be considered "barriers." But so-called "pure" functions and even nearly-pure functions don't need to be treated this way.
As for whether local_variable's value is useful after the call is a function of programmer's intent and the nature of compute_something(). If it's clear from the rest of the context that compute_something(foo) does not change the contents of foo, then references to local_variable after that call are unlikely to be suspicious. If compute_something(foo) might change the values of either field, though, then references to a value computed before the call at sites after the call are at least mildly suspicious without an explicit reason to use the value computed before the call.
If compute_something() takes a const struct whatsit*, it's very clear that the computation of flag can never modify foo->field1 or foo->field2
actually this can only be guaranteed if compute_something() takes const struct whatsit* _restrict - otherwise compute_something() could find an existing non-const pointer to the same object and modify it legally nevertheless (and all tasks are chained, so...)
You are correct, and so in the case of the compiler optimization I mentioned might happen, it's not legal in the general case for the compiler to do that unless it can see the body of the called function and determine if it can find some other path to the structure. Mea culpa. Another way to do this would be with function attributes, but that would be a GCC-specific extension. FWIW, some compilers do make certain assumptions regarding aliases and alternate paths to objects. Our compiler at work has a special flag to tell it that the case you mentioned might happen, if one path is through an argument and the other starts from a global or pointer returned from a different function. By default, it assumes (as I did in my post) that such aliasing doesn't happen by default.
As a matter of style and convention, though, I'd argue at the very least const clarifies the intent of the API and makes the code much easier to follow. I would also argue that "violating" the constness would be bad style.
To give a concrete example of what you're talking about, suppose struct whatsit is a doubly linked list, with two elements: struct whatsit *next, *prev;. I guess compute_something() could modify the passed in argument simply by referring to foo->next->prev assuming next is not NULL and the list is consistent. In this case, the non-const path to the structure came along with the structure itself.
... and I certainly use const in the same way; the point is: without further information about the function called (e.g. you precisely understand the semantic of the function and are aware of side-effects) you as a caller cannot rely on const meaning anything as a type qualifier for a function argument; therefore I am somewhat unhappy that a) specifying the intent "correctly" (through const/_restrict) is not well enough understood that anyone does it and b) the way chosen to express the intent does not really mean what everyone thinks it does.
That being said, the case of a function legally modifying a const qualified-object through a non-const alias and the modification not being changes to "private" stuff the caller is not supposed to be touching anyway (like linked list pointers) is certainly pretty esoteric.
What I would like to see however is a more general mechanism to annotate functions in a way that limit the scope of modifications; currently we have __attribute__((pure)) (which however is syntactically esoteric and noone uses it), but I would like to e.g. annotate a class function to specify that it only modifies members (or non-public members) of a class -- I think this would be helpful to both compiler and programmer and it could be formulated in a way that makes it a promising language guarantee instead of mere "programmer convention"
Hypocritical in the extreme
"Unless I'm missing something, your request for revert was pretty rude, technically incorrect and you also tried to circumvent the normal course of discussion."
At first I thought the quote was from Con Kolivas, but then I saw it was from Ingo "Kudos to Con" Molnar. I find this statement hypocritical from someone who so brazenly plagarized Con's work, and circumvented all norms of collaboration. Sadly, with the help of his scheduler mafia friends, he seems to have gotten away with it.
plagarized Con's work If you
If you had actually followed any of the debates you'd know that Con's RSDL and Ingo's CFS schedulers have nearly nothing in common technically. And yes he did give credit to Con Kolivas for coming up with the fairness aspect. So you kind of "forgot" to explain how exactly is this plagarism.
Again, groundless rants presented without any reasoning. Is using inspiration from someone else's code to improve your own "circumvention of all norms of collaboration"? Is it competing with someone who proved that the previous scheduler sucked? Is it being responsive to criticism from users? Is it being a more highly respected kernel hacker than your competition? Is it out-doing your competition in the end?
Or is it just that trolls are running out of arguments (if they had any in the first place)? You'd better start backing up your claims, or at least suggest what you're on about.
And the community got away with a better scheduler. This is the open source development model for you. When Con Kolivas says that he will code a kickass scheduler, nobody guarantees that his (however good) code will be merged.
Yes it is regrettable that Con left kernel development. However, the scheduler was not the only thing he had trouble merging into the mainline; it was simply the last one. I am not sure why his patches were not being favored, but if there's something I've learnt from previous controversies, unleashing a *users* mailing list in a battle against the LKML is not a good way to make friends with Linux developers.
What we really need is someone's impartial analysis of what went wrong, and what could be done differently the next time. With time this could be converted into a guide of "DOs and DON'Ts" for getting your patches merged, and how not to scare off newer contributors.
Wow, do these kernel guys
Wow, do these kernel guys have nothing better to do than argue about the use of const in their code?
It's the process...
Check out the full paragraph... Ingo's rather concerned with the behavior of the person in question:
That certainly does seem like a red flag. It's bad form to push a patch without consulting the active maintainers. It's worse form to push a patch that was turned down by the maintainers.
On the technical side, I'm with Ingo on the overall use of
constto indicate whether a function and its callees modify the contents of a particular object in a called function. From the standpoint of "improving code generation,"constisn't as useful as it once was, though: At one time, some compilers usedconstfor similar purposes as C99's newrestrictkeyword. *sigh* The particular interpretation of the standard that allowed those optimizations is now considered invalid. (If you're curious, feel free to ask.)Stating, though, as a matter of internal API that the caller is free to assume a given function doesn't modify a particular argument is important. And if someone tries to change that assumption, it can be worth having the compiler alert you to that sort of break in the API. For example, consider code like this:
struct whatsit *foo = next_struct_whatsit(); int local_variable = foo->field1 - foo->field2; int flag; /* bits of code here */ flag = compute_something(foo); /* should 'local_variable' be considered valid here? */If
compute_something()takes aconst struct whatsit*, it's very clear that the computation offlagcan never modifyfoo->field1orfoo->field2, and thereforelocal_variablewould have the same value if it were computed before the call or computed after. Furthermore, the compiler can choose to recompute it after the call if it results in better register allocation. Such code tranformations are obviously correct. The semantics oflocal_variableare very clear with respect tocompute_something().If
compute_something()takes juststruct whatsit*, one needs to know something more about the definition of that function and any other function it calls to know for sure thatlocal_variablehas a usable and meaningful value after the call. And, ifcompute_somethingdoes change either field in this situation, a person reading the code has to figure out if the caller really wantslocal_variablein terms of the old or new values of these fields. (There are cases when you want one or the other.) This increases the mental load on the programmer and the likelihood they misinterpret the code when viewing it in isolation.See? Little things like
constaren't so little.--
Program Intellivision and play Space Patrol!
What's "considered valid"?
I don't understand what you mean by "considered valid". local_variable is computed as the difference between two fields of foo at a particular point in time, and any assumption that the equality still holds after the call to compute_something would have to be up to the actual intent of the code the programmer has written, and the language rules have no business second-guessing this decision.
How about a simpler example?
Optimization isn't about second guessing. Optimization is about producing more efficient code within the bounds of the semantics of the supplied program.
In this case, if the compiler can prove that
compute_something(foo)cannot change the value of those two fields, it can move the computation oflocal_variableafter the call (or even compute it more than once!) if that generates better overall code. Now, certain types of calls, such as those that take a lock, should be considered "barriers." But so-called "pure" functions and even nearly-pure functions don't need to be treated this way.As for whether
local_variable's value is useful after the call is a function of programmer's intent and the nature ofcompute_something(). If it's clear from the rest of the context thatcompute_something(foo)does not change the contents offoo, then references tolocal_variableafter that call are unlikely to be suspicious. Ifcompute_something(foo)might change the values of either field, though, then references to a value computed before the call at sites after the call are at least mildly suspicious without an explicit reason to use the value computed before the call.--
Program Intellivision and play Space Patrol!
you need _restrict too
If compute_something() takes a const struct whatsit*, it's very clear that the computation of flag can never modify foo->field1 or foo->field2
actually this can only be guaranteed if compute_something() takes const struct whatsit* _restrict - otherwise compute_something() could find an existing non-const pointer to the same object and modify it legally nevertheless (and all tasks are chained, so...)
Good point
You are correct, and so in the case of the compiler optimization I mentioned might happen, it's not legal in the general case for the compiler to do that unless it can see the body of the called function and determine if it can find some other path to the structure. Mea culpa. Another way to do this would be with function attributes, but that would be a GCC-specific extension. FWIW, some compilers do make certain assumptions regarding aliases and alternate paths to objects. Our compiler at work has a special flag to tell it that the case you mentioned might happen, if one path is through an argument and the other starts from a global or pointer returned from a different function. By default, it assumes (as I did in my post) that such aliasing doesn't happen by default.
As a matter of style and convention, though, I'd argue at the very least
constclarifies the intent of the API and makes the code much easier to follow. I would also argue that "violating" the constness would be bad style.To give a concrete example of what you're talking about, suppose
struct whatsitis a doubly linked list, with two elements:struct whatsit *next, *prev;. I guesscompute_something()could modify the passed in argument simply by referring tofoo->next->prevassumingnextis notNULLand the list is consistent. In this case, the non-const path to the structure came along with the structure itself.--
Program Intellivision and play Space Patrol!
Annotating programmer intent is good...
... and I certainly use const in the same way; the point is: without further information about the function called (e.g. you precisely understand the semantic of the function and are aware of side-effects) you as a caller cannot rely on const meaning anything as a type qualifier for a function argument; therefore I am somewhat unhappy that a) specifying the intent "correctly" (through const/_restrict) is not well enough understood that anyone does it and b) the way chosen to express the intent does not really mean what everyone thinks it does.
That being said, the case of a function legally modifying a const qualified-object through a non-const alias and the modification not being changes to "private" stuff the caller is not supposed to be touching anyway (like linked list pointers) is certainly pretty esoteric.
What I would like to see however is a more general mechanism to annotate functions in a way that limit the scope of modifications; currently we have __attribute__((pure)) (which however is syntactically esoteric and noone uses it), but I would like to e.g. annotate a class function to specify that it only modifies members (or non-public members) of a class -- I think this would be helpful to both compiler and programmer and it could be formulated in a way that makes it a promising language guarantee instead of mere "programmer convention"
Wow, do these KernelTrap commentors
Wow, do these KernelTrap commentors have nothing better to do than to complain about what kernel developers decide to do with their time?