[Devel] Re: [PATCH 1/2] c/r: compute hrtimer remaining time relative to ktime_begin

[Oren Laadan]

On 08/03/2010 02:40 AM, Matt Helsley wrote:
> On Mon, Aug 02, 2010 at 05:55:08PM -0400, Oren Laadan wrote:
>> The rule for time values in c/r is: all time measurements in
>> the checkpoint image are saved as relative values, using the
>> start-of-checkpoint as the reference point in time.
>>
>> This patch fixes how hrtimer delta is calculated to make it
>> really follow this rule.
>>
>> Signed-off-by: Oren Laadan <orenl at cs.columbia.edu>
>> ---
>>  kernel/signal.c |    3 ++-
>>  1 files changed, 2 insertions(+), 1 deletions(-)
>>
>> diff --git a/kernel/signal.c b/kernel/signal.c
>> index de76729..0ec4a46 100644
>> --- a/kernel/signal.c
>> +++ b/kernel/signal.c
>> @@ -3151,7 +3151,8 @@ static int checkpoint_signal(struct ckpt_ctx *ctx, struct task_struct *t)
>>  	/* real/virt/prof itimers */
>>  	if (hrtimer_active(&signal->real_timer)) {
>>  		/* For an active timer compute the time delta */
>> -		ktime_t delta = hrtimer_get_remaining(&signal->real_timer);
>> +		ktime_t delta = ktime_sub(ctx->ktime_begin,
>> +					  signal->real_timer._expires);
> 
> (Cc'ing Thomas -- perhaps he'd like to see something else..)
> 
> This doesn't look clean. Perhaps you should introduce a new hrtimer function
> rather than access signal->real_timer._expires directly. I'm thinking something
> like the following in include/linux/hrtimer.h:
> 
> /* after will usually be <= now */
> static inline ktime_t hrtimer_get_remaining_after(const struct hrtimer *timer,
> 						ktime_t after)
> {
>     return ktime_sub(timer->_expires, after);
> }
> 
> then in kernel/signal.c you'd replace the above patch hunk with:
> 
> 		ktime_t delta = hrtimer_get_remaining_after(&signal->real_timer,
> 							ctx->ktime_begin);
> 
> which is functionally the same but looks cleaner.

Makes sense. Did that, with s/get/expires/ in the choice of
name to match the existing name convention (no lock taken).

Thanks,

Oren.
_______________________________________________
Containers mailing list
Containers at lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers