Le 21 août 08 à 15:03, John Kacur a écrit :
On Thu, Aug 21, 2008 at 2:18 PM, Gilles Carry <glscarry@xxxxxxx>
wrote:
Le 20 août 08 à 23:48, John Kacur a écrit :
I kind of find this confusing, because the return value is only
useful
in one specific instance. In other cases, the return value is thrown
away. People who are not aware of the history of this code and
examine
it later may ask why the return value is being ignored in some
cases.
John,
I don't see anything confusing here. It's similar to a function
where you
have plenty
of parameters and depending on the context, only a few are useful.
In the
present
case, the timers mode names (...NOSOFTIRQ) speak for themselves.
They don't
need
this return code.
As there is already plenty of explanations and comments within the
code,
people 'not aware' will do as everbody else do: decrypt and
understand. ;-)
Anyway, I don't mind adding comments at the head of the function.
My goal here is only to make the code more readable and easier to
maintain
by
factorization this is why I separated this patch from the fix.
If you look at the factorized code, you see the only difference is
the
raisesoftirq stuff.
I thought it was worth doing it.
Gilles.--
To unsubscribe from this list: send the line "unsubscribe linux-rt-
users" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
The goal of factorization is a good one of course. I also don't want
to argue too much about the implementation when you have done the
detective work to solve a hard problem, but I am just wondering if
there is a better way to do this - that would make the code even more
readable. As I go over this code again, I'm also thinking that it
conceptually doesn't really belong in the __run_timer function, even
if it works. If you look at the code before your changes, the function
__run_timer did just that, it ran a timer, but now it's manipulating
callback lists. Is there perhaps another way to common this code up
then?
True, __run_hrtimer does a bit more than it used to but if you consider
that this function as a method of an object of class_hrtimer, it is
exactly what it would do. Also, wakeup_thread simply puts a thread
back on a run queue which somewhat similar to put a timer in a
callback list and change its status.
But as I told you, this patch is only a proposal.
Gilles.--
To unsubscribe from this list: send the line "unsubscribe linux-rt-users" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
