Re: [PATCH v3 2/3] fs/proc: apply the time namespace offset to /proc/stat btime

[Michael Weiß]

On 15.10.20 09:53, Andrei Vagin wrote:
> On Sun, Oct 11, 2020 at 04:59:23PM +0200, Michael Weiß wrote:
>> @@ -79,6 +80,20 @@ static u64 get_iowait_time(struct kernel_cpustat *kcs, int cpu)
>>  
>>  #endif
>>  
>> +static void get_boottime(struct timespec64 *ts)
>> +{
> 
>> +	ktime_t boottime;
>> +
>> +	/* get kernel internal system boot timestamp */
>> +	getboottime64(ts);
>> +
>> +	/* shift boot timestamp according to the timens offset */
>> +	boottime = timespec64_to_ktime(*ts);
>> +	boottime = timens_ktime_to_host(CLOCK_BOOTTIME, boottime);
> 
> timens_ktime_to_host is used to convert timens' time to host's time.
> Here it looks like we are using it in the opposite direction. I spent
> some time to figure out what is going on here. I think it worth to add a
> comment here.
> 
>> +
>> +	*ts = ktime_to_timespec64(boottime);
> 
> I don't like all these conversions back and forth. Maybe something like
> this will look better:
> 
> #ifdef CONFIG_TIME_NS
> 	if (current->nsproxy->time_ns != init_time_ns) {
> 		struct timens_offsets *ns_offsets;
> 		ns_offsets = &current->nsproxy->time_ns->offsets;
> 		ts = timespec64_sub(ts, timens_offsets->boottime);
> 	}
> #endif
> 

I agree to that, but maybe we then introduce another helper in
time_namespace.h analogues to timens_add_boottime() and handle
also the ifdef there:

static inline void timens_sub_boottime(struct timespec64 *ts)
{
	struct timens_offsets *ns_offsets = &current->nsproxy->time_ns->offsets;

	*ts = timespec64_sub(*ts, ns_offsets->boottime);
}

using this in proc/stat, it is obvious what is going on and we only need
to add one line of code there. Further, future use-cases which depend on
timens aware boottime would be more strait forward to be implement.

Cheers,
Michael