Re: [PATCH 2/2] [RT] hrtimer __run_hrtimer code cleanup

["John Kacur" <jkacur@gmail.com>]

On Thu, Aug 21, 2008 at 2:18 PM, Gilles Carry <glscarry@free.fr> 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@vger.kernel.org
> 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?
--
To unsubscribe from this list: send the line "unsubscribe linux-rt-users" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html