Re: clone3: allow creation of time namespace with offset

On Wed, Mar 18, 2020 at 11:18:53AM +0100, Arnd Bergmann wrote:

On Tue, Mar 17, 2020 at 9:32 AM Adrian Reber [off-list ref] wrote:

quoted

This is an attempt to add time namespace support to clone3(). I am not
really sure which way clone3() should handle time namespaces. The time
namespace through /proc cannot be used with clone3() because the offsets
for the time namespace need to be written before a process has been
created in that time namespace. This means it is necessary to somehow
tell clone3() the offsets for the clocks.

The time namespace offers the possibility to set offsets for
CLOCK_MONOTONIC and CLOCK_BOOTTIME. My first approach was to extend
'struct clone_args` with '__aligned_u64 monotonic_offset' and
'__aligned_u64 boottime_offset'. The problem with this approach was that
it was not possible to set nanoseconds for the clocks in the time
namespace.

One of the motivations for clone3() with CLONE_NEWTIME was to enable
CRIU to restore a process in a time namespace with the corresponding
offsets. And although the nanosecond value can probably never be
restored to the same value it had during checkpointing, because the
clock keeps on running between CRIU pausing all processes and CRIU
actually reading the value of the clocks, the nanosecond value is still
necessary for CRIU to not restore a process where the clock jumps back
due to CRIU restoring it with a nanonsecond value that is too small.

Requiring nanoseconds as well as seconds for two clocks during clone3()
means that it would require 4 additional members to 'struct clone_args':

        __aligned_u64 tls;
        __aligned_u64 set_tid;
        __aligned_u64 set_tid_size;
+       __aligned_u64 boottime_offset_seconds;
+       __aligned_u64 boottime_offset_nanoseconds;
+       __aligned_u64 monotonic_offset_seconds;
+       __aligned_u64 monotonic_offset_nanoseconds;
 };

Wouldn't it be sufficient to have the two nanosecond values, rather
than both seconds and nanoseconds? With 64-bit nanoseconds
you can represent several hundred years, and these would
always start at zero during boot.

I like this. Just using nanoseconds will make it easier and should
indeed be enough.

Regardless of this, I think you need a signed offset, not unsigned.

Right, that was just a quick test at some point.

Christian and I have also been discussing this a bit and Christian
prefers a pointer to a struct. Maybe something like this:

        __aligned_u64 tls;
        __aligned_u64 set_tid;
        __aligned_u64 set_tid_size;
+       __aligned_u64 timens_offset;
 };

With Arnd's idea of only using nanoseconds, timens_offset would then
contain something like this:

struct timens_offset {
	__aligned_s64 monotonic_offset_ns;
	__aligned_s64 boottime_offset_ns;
};

I kind of prefer adding boottime and monotonic directly to struct clone_args

        __aligned_u64 tls;
        __aligned_u64 set_tid;
        __aligned_u64 set_tid_size;
+       __aligned_s64 monotonic_offset_ns;
+       __aligned_s64 boottime_offset_ns;
 };

But setting the time namespace offset is probably something which does
not happen very often while using clone3(), so maybe the pointer to a
struct approach is better.

I will resend the patches using the pointer to a struct approach if
there are no other ideas how to do this.

		Adrian