linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] timens: show clock symbolic names in /proc/pid/timens_offsets
@ 2020-04-11  6:52 Andrei Vagin
  2020-04-11 10:32 ` Michael Kerrisk (man-pages)
  0 siblings, 1 reply; 12+ messages in thread
From: Andrei Vagin @ 2020-04-11  6:52 UTC (permalink / raw)
  To: Thomas Gleixner, Andrew Morton
  Cc: linux-api, linux-kernel, Andrei Vagin, Eric W . Biederman,
	Michael Kerrisk, Dmitry Safonov

Michael Kerrisk suggested to replace numeric clock IDs on symbolic
names.

Now the content of these files looks like this:
$ cat /proc/5362/timens_offsets
monotonic	   9504000	         0
boottime	   3456000	         0

For setting offsets, both representations of clocks can be used.

As for compatibility, it is acceptable to change things as long as
userspace doesn't care. The format of timens_offsets files is very
new and there are no userspace tools that rely on this format.

But three projects crun, util-linux and criu rely on the interface of
setting time offsets and this is why we need to continue supporting the
clock IDs in this case.

Fixes: 04a8682a71be ("fs/proc: Introduce /proc/pid/timens_offsets")
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Eric W. Biederman <ebiederm@xmission.com>
Cc: Michael Kerrisk <mtk.manpages@gmail.com>
Cc: Dmitry Safonov <0x7f454c46@gmail.com>
Suggested-by: Michael Kerrisk <mtk.manpages@gmail.com>
Signed-off-by: Andrei Vagin <avagin@gmail.com>
---
 fs/proc/base.c          | 14 +++++++++++++-
 kernel/time/namespace.c | 15 ++++++++++++++-
 2 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/fs/proc/base.c b/fs/proc/base.c
index 6042b646ab27..572898dd16a0 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -1573,6 +1573,7 @@ static ssize_t timens_offsets_write(struct file *file, const char __user *buf,
 	noffsets = 0;
 	for (pos = kbuf; pos; pos = next_line) {
 		struct proc_timens_offset *off = &offsets[noffsets];
+		char clock[10];
 		int err;
 
 		/* Find the end of line and ensure we don't look past it */
@@ -1584,10 +1585,21 @@ static ssize_t timens_offsets_write(struct file *file, const char __user *buf,
 				next_line = NULL;
 		}
 
-		err = sscanf(pos, "%u %lld %lu", &off->clockid,
+		err = sscanf(pos, "%9s %lld %lu", clock,
 				&off->val.tv_sec, &off->val.tv_nsec);
 		if (err != 3 || off->val.tv_nsec >= NSEC_PER_SEC)
 			goto out;
+
+		clock[sizeof(clock) - 1] = 0;
+		if (strcmp(clock, "monotonic") == 0 ||
+		    strcmp(clock, __stringify(CLOCK_MONOTONIC)) == 0)
+			off->clockid = CLOCK_MONOTONIC;
+		else if (strcmp(clock, "boottime") == 0 ||
+			 strcmp(clock, __stringify(CLOCK_BOOTTIME)) == 0)
+			off->clockid = CLOCK_BOOTTIME;
+		else
+			goto out;
+
 		noffsets++;
 		if (noffsets == ARRAY_SIZE(offsets)) {
 			if (next_line)
diff --git a/kernel/time/namespace.c b/kernel/time/namespace.c
index e6ba064ce773..8127d2647064 100644
--- a/kernel/time/namespace.c
+++ b/kernel/time/namespace.c
@@ -338,7 +338,20 @@ static struct user_namespace *timens_owner(struct ns_common *ns)
 
 static void show_offset(struct seq_file *m, int clockid, struct timespec64 *ts)
 {
-	seq_printf(m, "%d %lld %ld\n", clockid, ts->tv_sec, ts->tv_nsec);
+	char *clock;
+
+	switch (clockid) {
+	case CLOCK_BOOTTIME:
+		clock = "boottime";
+		break;
+	case CLOCK_MONOTONIC:
+		clock = "monotonic";
+		break;
+	default:
+		clock = "unknown";
+		break;
+	}
+	seq_printf(m, "%s\t%10lld\t%10ld\n", clock, ts->tv_sec, ts->tv_nsec);
 }
 
 void proc_timens_show_offsets(struct task_struct *p, struct seq_file *m)
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] timens: show clock symbolic names in /proc/pid/timens_offsets
  2020-04-11  6:52 [PATCH] timens: show clock symbolic names in /proc/pid/timens_offsets Andrei Vagin
@ 2020-04-11 10:32 ` Michael Kerrisk (man-pages)
  2020-04-11 10:36   ` Michael Kerrisk (man-pages)
  0 siblings, 1 reply; 12+ messages in thread
From: Michael Kerrisk (man-pages) @ 2020-04-11 10:32 UTC (permalink / raw)
  To: Andrei Vagin, Thomas Gleixner, Andrew Morton
  Cc: mtk.manpages, linux-api, linux-kernel, Eric W . Biederman,
	Dmitry Safonov

Hello Andrei,

On 4/11/20 8:52 AM, Andrei Vagin wrote:
> Michael Kerrisk suggested to replace numeric clock IDs on symbolic
> names.
> 
> Now the content of these files looks like this:
> $ cat /proc/5362/timens_offsets
> monotonic	   9504000	         0
> boottime	   3456000	         0
> 
> For setting offsets, both representations of clocks can be used.
> 
> As for compatibility, it is acceptable to change things as long as
> userspace doesn't care. The format of timens_offsets files is very
> new and there are no userspace tools that rely on this format.
> 
> But three projects crun, util-linux and criu rely on the interface of
> setting time offsets and this is why we need to continue supporting the
> clock IDs in this case.

Thanks very much for this patch, which I've tested. So:

Acked-by: Michael Kerrisk <mtk.manpages@gmail.com>
Tested-by: Michael Kerrisk <mtk.manpages@gmail.com>

But, I do have one small suggestion below.

> Fixes: 04a8682a71be ("fs/proc: Introduce /proc/pid/timens_offsets")
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Eric W. Biederman <ebiederm@xmission.com>
> Cc: Michael Kerrisk <mtk.manpages@gmail.com>
> Cc: Dmitry Safonov <0x7f454c46@gmail.com>
> Suggested-by: Michael Kerrisk <mtk.manpages@gmail.com>
> Signed-off-by: Andrei Vagin <avagin@gmail.com>
> ---
>  fs/proc/base.c          | 14 +++++++++++++-
>  kernel/time/namespace.c | 15 ++++++++++++++-
>  2 files changed, 27 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/proc/base.c b/fs/proc/base.c
> index 6042b646ab27..572898dd16a0 100644
> --- a/fs/proc/base.c
> +++ b/fs/proc/base.c
> @@ -1573,6 +1573,7 @@ static ssize_t timens_offsets_write(struct file *file, const char __user *buf,
>  	noffsets = 0;
>  	for (pos = kbuf; pos; pos = next_line) {
>  		struct proc_timens_offset *off = &offsets[noffsets];
> +		char clock[10];
>  		int err;
>  
>  		/* Find the end of line and ensure we don't look past it */
> @@ -1584,10 +1585,21 @@ static ssize_t timens_offsets_write(struct file *file, const char __user *buf,
>  				next_line = NULL;
>  		}
>  
> -		err = sscanf(pos, "%u %lld %lu", &off->clockid,
> +		err = sscanf(pos, "%9s %lld %lu", clock,
>  				&off->val.tv_sec, &off->val.tv_nsec);
>  		if (err != 3 || off->val.tv_nsec >= NSEC_PER_SEC)
>  			goto out;
> +
> +		clock[sizeof(clock) - 1] = 0;
> +		if (strcmp(clock, "monotonic") == 0 ||
> +		    strcmp(clock, __stringify(CLOCK_MONOTONIC)) == 0)
> +			off->clockid = CLOCK_MONOTONIC;
> +		else if (strcmp(clock, "boottime") == 0 ||
> +			 strcmp(clock, __stringify(CLOCK_BOOTTIME)) == 0)
> +			off->clockid = CLOCK_BOOTTIME;
> +		else
> +			goto out;
> +
>  		noffsets++;
>  		if (noffsets == ARRAY_SIZE(offsets)) {
>  			if (next_line)
> diff --git a/kernel/time/namespace.c b/kernel/time/namespace.c
> index e6ba064ce773..8127d2647064 100644
> --- a/kernel/time/namespace.c
> +++ b/kernel/time/namespace.c
> @@ -338,7 +338,20 @@ static struct user_namespace *timens_owner(struct ns_common *ns)
>  
>  static void show_offset(struct seq_file *m, int clockid, struct timespec64 *ts)
>  {
> -	seq_printf(m, "%d %lld %ld\n", clockid, ts->tv_sec, ts->tv_nsec);
> +	char *clock;
> +
> +	switch (clockid) {
> +	case CLOCK_BOOTTIME:
> +		clock = "boottime";
> +		break;
> +	case CLOCK_MONOTONIC:
> +		clock = "monotonic";
> +		break;
> +	default:
> +		clock = "unknown";
> +		break;
> +	}

As things stand, there is to my eye an excessive amount of white space
in the output produced by this line:

> +	seq_printf(m, "%s\t%10lld\t%10ld\n", clock, ts->tv_sec, ts->tv_nsec);

Can I suggest instead something like:

	seq_printf(m, "%-16s %10lld %9ld\n", clock, ts->tv_sec, ts->tv_nsec);

Thanks,

Michael

-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] timens: show clock symbolic names in /proc/pid/timens_offsets
  2020-04-11 10:32 ` Michael Kerrisk (man-pages)
@ 2020-04-11 10:36   ` Michael Kerrisk (man-pages)
  2020-04-11 15:40     ` [PATCH v2] " Andrei Vagin
  0 siblings, 1 reply; 12+ messages in thread
From: Michael Kerrisk (man-pages) @ 2020-04-11 10:36 UTC (permalink / raw)
  To: Andrei Vagin, Thomas Gleixner, Andrew Morton
  Cc: mtk.manpages, linux-api, linux-kernel, Eric W . Biederman,
	Dmitry Safonov

> As things stand, there is to my eye an excessive amount of white space
> in the output produced by this line:
> 
>> +	seq_printf(m, "%s\t%10lld\t%10ld\n", clock, ts->tv_sec, ts->tv_nsec);
> 
> Can I suggest instead something like:
> 
> 	seq_printf(m, "%-16s %10lld %9ld\n", clock, ts->tv_sec, ts->tv_nsec);

Actually, how about even s/-16s/-10s/ for that first field.

Thanks,

Michael


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2] timens: show clock symbolic names in /proc/pid/timens_offsets
  2020-04-11 10:36   ` Michael Kerrisk (man-pages)
@ 2020-04-11 15:40     ` Andrei Vagin
  2020-04-12  5:51       ` Michael Kerrisk (man-pages)
                         ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Andrei Vagin @ 2020-04-11 15:40 UTC (permalink / raw)
  To: Thomas Gleixner, Andrew Morton, Michael Kerrisk
  Cc: linux-api, linux-kernel, Andrei Vagin, Eric W . Biederman,
	Dmitry Safonov

Michael Kerrisk suggested to replace numeric clock IDs on symbolic
names.

Now the content of these files looks like this:
$ cat /proc/774/timens_offsets
monotonic      864000         0
boottime      1728000         0

For setting offsets, both representations of clocks can be used.

As for compatibility, it is acceptable to change things as long as
userspace doesn't care. The format of timens_offsets files is very
new and there are no userspace tools that rely on this format.

But three projects crun, util-linux and criu rely on the interface of
setting time offsets and this is why we need to continue supporting the
clock IDs in this case.

Fixes: 04a8682a71be ("fs/proc: Introduce /proc/pid/timens_offsets")
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Eric W. Biederman <ebiederm@xmission.com>
Cc: Dmitry Safonov <0x7f454c46@gmail.com>
Acked-by: Michael Kerrisk <mtk.manpages@gmail.com>                                                                                                                                            
Tested-by: Michael Kerrisk <mtk.manpages@gmail.com> 
Suggested-by: Michael Kerrisk <mtk.manpages@gmail.com>
Signed-off-by: Andrei Vagin <avagin@gmail.com>
---

v2: use the more compact format of timens_offsets files.

 fs/proc/base.c          | 14 +++++++++++++-
 kernel/time/namespace.c | 15 ++++++++++++++-
 2 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/fs/proc/base.c b/fs/proc/base.c
index 6042b646ab27..572898dd16a0 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -1573,6 +1573,7 @@ static ssize_t timens_offsets_write(struct file *file, const char __user *buf,
 	noffsets = 0;
 	for (pos = kbuf; pos; pos = next_line) {
 		struct proc_timens_offset *off = &offsets[noffsets];
+		char clock[10];
 		int err;
 
 		/* Find the end of line and ensure we don't look past it */
@@ -1584,10 +1585,21 @@ static ssize_t timens_offsets_write(struct file *file, const char __user *buf,
 				next_line = NULL;
 		}
 
-		err = sscanf(pos, "%u %lld %lu", &off->clockid,
+		err = sscanf(pos, "%9s %lld %lu", clock,
 				&off->val.tv_sec, &off->val.tv_nsec);
 		if (err != 3 || off->val.tv_nsec >= NSEC_PER_SEC)
 			goto out;
+
+		clock[sizeof(clock) - 1] = 0;
+		if (strcmp(clock, "monotonic") == 0 ||
+		    strcmp(clock, __stringify(CLOCK_MONOTONIC)) == 0)
+			off->clockid = CLOCK_MONOTONIC;
+		else if (strcmp(clock, "boottime") == 0 ||
+			 strcmp(clock, __stringify(CLOCK_BOOTTIME)) == 0)
+			off->clockid = CLOCK_BOOTTIME;
+		else
+			goto out;
+
 		noffsets++;
 		if (noffsets == ARRAY_SIZE(offsets)) {
 			if (next_line)
diff --git a/kernel/time/namespace.c b/kernel/time/namespace.c
index e6ba064ce773..757581d64246 100644
--- a/kernel/time/namespace.c
+++ b/kernel/time/namespace.c
@@ -338,7 +338,20 @@ static struct user_namespace *timens_owner(struct ns_common *ns)
 
 static void show_offset(struct seq_file *m, int clockid, struct timespec64 *ts)
 {
-	seq_printf(m, "%d %lld %ld\n", clockid, ts->tv_sec, ts->tv_nsec);
+	char *clock;
+
+	switch (clockid) {
+	case CLOCK_BOOTTIME:
+		clock = "boottime";
+		break;
+	case CLOCK_MONOTONIC:
+		clock = "monotonic";
+		break;
+	default:
+		clock = "unknown";
+		break;
+	}
+	seq_printf(m, "%-10s %10lld %9ld\n", clock, ts->tv_sec, ts->tv_nsec);
 }
 
 void proc_timens_show_offsets(struct task_struct *p, struct seq_file *m)
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v2] timens: show clock symbolic names in /proc/pid/timens_offsets
  2020-04-11 15:40     ` [PATCH v2] " Andrei Vagin
@ 2020-04-12  5:51       ` Michael Kerrisk (man-pages)
  2020-04-13 22:47         ` Andrew Morton
  2020-04-16  6:56       ` Andrei Vagin
  2020-04-16 10:15       ` [tip: timers/urgent] proc, time/namespace: Show " tip-bot2 for Andrei Vagin
  2 siblings, 1 reply; 12+ messages in thread
From: Michael Kerrisk (man-pages) @ 2020-04-12  5:51 UTC (permalink / raw)
  To: Andrei Vagin
  Cc: Thomas Gleixner, Andrew Morton, Linux API, lkml,
	Eric W . Biederman, Dmitry Safonov

Hi Andrei,

On Sat, 11 Apr 2020 at 17:40, Andrei Vagin <avagin@gmail.com> wrote:
>
> Michael Kerrisk suggested to replace numeric clock IDs on symbolic
> names.
>
> Now the content of these files looks like this:
> $ cat /proc/774/timens_offsets
> monotonic      864000         0
> boottime      1728000         0

Thanks.

Assuming no-one has objections to the patch, please do mark for stable@.

Cheers,

Michael

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2] timens: show clock symbolic names in /proc/pid/timens_offsets
  2020-04-12  5:51       ` Michael Kerrisk (man-pages)
@ 2020-04-13 22:47         ` Andrew Morton
  2020-04-14  0:04           ` Andrei Vagin
  2020-04-14  9:51           ` Michael Kerrisk (man-pages)
  0 siblings, 2 replies; 12+ messages in thread
From: Andrew Morton @ 2020-04-13 22:47 UTC (permalink / raw)
  To: mtk.manpages
  Cc: Andrei Vagin, Thomas Gleixner, Linux API, lkml,
	Eric W . Biederman, Dmitry Safonov

On Sun, 12 Apr 2020 07:51:47 +0200 "Michael Kerrisk (man-pages)" <mtk.manpages@gmail.com> wrote:

> Hi Andrei,
> 
> On Sat, 11 Apr 2020 at 17:40, Andrei Vagin <avagin@gmail.com> wrote:
> >
> > Michael Kerrisk suggested to replace numeric clock IDs on symbolic
> > names.
> >
> > Now the content of these files looks like this:
> > $ cat /proc/774/timens_offsets
> > monotonic      864000         0
> > boottime      1728000         0
> 
> Thanks.
> 
> Assuming no-one has objections to the patch, please do mark for stable@.
> 

`grep -r timens_offsets Documentation' comes up blank.  Is
/proc/pid/timens_offsets documented anywhere?  If not, it should be! 
And this patch should update that documentation.

I assume the time namespace feature itself is documented under clone(2)?

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2] timens: show clock symbolic names in /proc/pid/timens_offsets
  2020-04-13 22:47         ` Andrew Morton
@ 2020-04-14  0:04           ` Andrei Vagin
  2020-04-14  9:51           ` Michael Kerrisk (man-pages)
  1 sibling, 0 replies; 12+ messages in thread
From: Andrei Vagin @ 2020-04-14  0:04 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Michael Kerrisk, Thomas Gleixner, Linux API, lkml,
	Eric W . Biederman, Dmitry Safonov

On Mon, Apr 13, 2020 at 3:47 PM Andrew Morton <akpm@linux-foundation.org> wrote:
>
> On Sun, 12 Apr 2020 07:51:47 +0200 "Michael Kerrisk (man-pages)" <mtk.manpages@gmail.com> wrote:
>
> > Hi Andrei,
> >
> > On Sat, 11 Apr 2020 at 17:40, Andrei Vagin <avagin@gmail.com> wrote:
> > >
> > > Michael Kerrisk suggested to replace numeric clock IDs on symbolic
> > > names.
> > >
> > > Now the content of these files looks like this:
> > > $ cat /proc/774/timens_offsets
> > > monotonic      864000         0
> > > boottime      1728000         0
> >
> > Thanks.
> >
> > Assuming no-one has objections to the patch, please do mark for stable@.
> >
>
> `grep -r timens_offsets Documentation' comes up blank.  Is
> /proc/pid/timens_offsets documented anywhere?  If not, it should be!
> And this patch should update that documentation.
>
> I assume the time namespace feature itself is documented under clone(2)?

Thanks to Michael, we have the man page for time namespaces:
https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/tree/man7/time_namespaces.7

And it will be updated according with this change.

Thanks,
Andrei

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2] timens: show clock symbolic names in /proc/pid/timens_offsets
  2020-04-13 22:47         ` Andrew Morton
  2020-04-14  0:04           ` Andrei Vagin
@ 2020-04-14  9:51           ` Michael Kerrisk (man-pages)
  1 sibling, 0 replies; 12+ messages in thread
From: Michael Kerrisk (man-pages) @ 2020-04-14  9:51 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Andrei Vagin, Thomas Gleixner, Linux API, lkml,
	Eric W . Biederman, Dmitry Safonov

On Tue, 14 Apr 2020 at 00:47, Andrew Morton <akpm@linux-foundation.org> wrote:
>
> On Sun, 12 Apr 2020 07:51:47 +0200 "Michael Kerrisk (man-pages)" <mtk.manpages@gmail.com> wrote:
>
> > Hi Andrei,
> >
> > On Sat, 11 Apr 2020 at 17:40, Andrei Vagin <avagin@gmail.com> wrote:
> > >
> > > Michael Kerrisk suggested to replace numeric clock IDs on symbolic
> > > names.
> > >
> > > Now the content of these files looks like this:
> > > $ cat /proc/774/timens_offsets
> > > monotonic      864000         0
> > > boottime      1728000         0
> >
> > Thanks.
> >
> > Assuming no-one has objections to the patch, please do mark for stable@.
> >
>
> `grep -r timens_offsets Documentation' comes up blank.  Is
> /proc/pid/timens_offsets documented anywhere?  If not, it should be!
> And this patch should update that documentation.
>
> I assume the time namespace feature itself is documented under clone(2)?

We're good, so far. There's time_namespaces(7) [1] and documentation
of CLONE_NEWTIME in unshare(2) [2].

CLONE_NEWTIME support for clone3() is still a work in progress [3].

Thanks,

Michael

[1] http://man7.org/linux/man-pages/man7/time_namespaces.7.html
[2] http://man7.org/linux/man-pages/man2/unshare.2.html
[3] https://lore.kernel.org/lkml/20200317083043.226593-1-areber@redhat.com/


--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2] timens: show clock symbolic names in /proc/pid/timens_offsets
  2020-04-11 15:40     ` [PATCH v2] " Andrei Vagin
  2020-04-12  5:51       ` Michael Kerrisk (man-pages)
@ 2020-04-16  6:56       ` Andrei Vagin
  2020-04-16  7:10         ` Michael Kerrisk (man-pages)
  2020-04-16  9:52         ` Thomas Gleixner
  2020-04-16 10:15       ` [tip: timers/urgent] proc, time/namespace: Show " tip-bot2 for Andrei Vagin
  2 siblings, 2 replies; 12+ messages in thread
From: Andrei Vagin @ 2020-04-16  6:56 UTC (permalink / raw)
  To: Thomas Gleixner, Andrew Morton, Michael Kerrisk
  Cc: linux-api, linux-kernel, Eric W . Biederman, Dmitry Safonov

On Sat, Apr 11, 2020 at 08:40:31AM -0700, Andrei Vagin wrote:
> Michael Kerrisk suggested to replace numeric clock IDs on symbolic
> names.
> 
> Now the content of these files looks like this:
> $ cat /proc/774/timens_offsets
> monotonic      864000         0
> boottime      1728000         0
> 
> For setting offsets, both representations of clocks can be used.
> 
> As for compatibility, it is acceptable to change things as long as
> userspace doesn't care. The format of timens_offsets files is very
> new and there are no userspace tools that rely on this format.
> 
> But three projects crun, util-linux and criu rely on the interface of
> setting time offsets and this is why we need to continue supporting the
> clock IDs in this case.
> 
> Fixes: 04a8682a71be ("fs/proc: Introduce /proc/pid/timens_offsets")
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Eric W. Biederman <ebiederm@xmission.com>
> Cc: Dmitry Safonov <0x7f454c46@gmail.com>
> Acked-by: Michael Kerrisk <mtk.manpages@gmail.com>                                                                                                                                            

Thomas and Andrew, could you merge this patch? I am sorry, I used the
wrong subsystem prefix. Let me know if I need to send the third version
of this patch.

Thanks,
Andrei

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2] timens: show clock symbolic names in /proc/pid/timens_offsets
  2020-04-16  6:56       ` Andrei Vagin
@ 2020-04-16  7:10         ` Michael Kerrisk (man-pages)
  2020-04-16  9:52         ` Thomas Gleixner
  1 sibling, 0 replies; 12+ messages in thread
From: Michael Kerrisk (man-pages) @ 2020-04-16  7:10 UTC (permalink / raw)
  To: Andrei Vagin
  Cc: Thomas Gleixner, Andrew Morton, Linux API, lkml,
	Eric W . Biederman, Dmitry Safonov

Hi Andrei,

On Thu, 16 Apr 2020 at 08:56, Andrei Vagin <avagin@gmail.com> wrote:
>
> On Sat, Apr 11, 2020 at 08:40:31AM -0700, Andrei Vagin wrote:
> > Michael Kerrisk suggested to replace numeric clock IDs on symbolic
> > names.
> >
> > Now the content of these files looks like this:
> > $ cat /proc/774/timens_offsets
> > monotonic      864000         0
> > boottime      1728000         0
> >
> > For setting offsets, both representations of clocks can be used.
> >
> > As for compatibility, it is acceptable to change things as long as
> > userspace doesn't care. The format of timens_offsets files is very
> > new and there are no userspace tools that rely on this format.
> >
> > But three projects crun, util-linux and criu rely on the interface of
> > setting time offsets and this is why we need to continue supporting the
> > clock IDs in this case.
> >
> > Fixes: 04a8682a71be ("fs/proc: Introduce /proc/pid/timens_offsets")
> > Cc: Thomas Gleixner <tglx@linutronix.de>
> > Cc: Andrew Morton <akpm@linux-foundation.org>
> > Cc: Eric W. Biederman <ebiederm@xmission.com>
> > Cc: Dmitry Safonov <0x7f454c46@gmail.com>
> > Acked-by: Michael Kerrisk <mtk.manpages@gmail.com>
>
> Thomas and Andrew, could you merge this patch? I am sorry, I used the
> wrong subsystem prefix. Let me know if I need to send the third version
> of this patch.

This patch should also be marked for stable@.

Thanks,

Michael

-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2] timens: show clock symbolic names in /proc/pid/timens_offsets
  2020-04-16  6:56       ` Andrei Vagin
  2020-04-16  7:10         ` Michael Kerrisk (man-pages)
@ 2020-04-16  9:52         ` Thomas Gleixner
  1 sibling, 0 replies; 12+ messages in thread
From: Thomas Gleixner @ 2020-04-16  9:52 UTC (permalink / raw)
  To: Andrei Vagin, Andrew Morton, Michael Kerrisk
  Cc: linux-api, linux-kernel, Eric W . Biederman, Dmitry Safonov

Andrei Vagin <avagin@gmail.com> writes:
> On Sat, Apr 11, 2020 at 08:40:31AM -0700, Andrei Vagin wrote:
>> Michael Kerrisk suggested to replace numeric clock IDs on symbolic
>> names.
>> 
>> Now the content of these files looks like this:
>> $ cat /proc/774/timens_offsets
>> monotonic      864000         0
>> boottime      1728000         0
>> 
>> For setting offsets, both representations of clocks can be used.
>> 
>> As for compatibility, it is acceptable to change things as long as
>> userspace doesn't care. The format of timens_offsets files is very
>> new and there are no userspace tools that rely on this format.
>> 
>> But three projects crun, util-linux and criu rely on the interface of
>> setting time offsets and this is why we need to continue supporting the
>> clock IDs in this case.
>> 
>> Fixes: 04a8682a71be ("fs/proc: Introduce /proc/pid/timens_offsets")
>> Cc: Thomas Gleixner <tglx@linutronix.de>
>> Cc: Andrew Morton <akpm@linux-foundation.org>
>> Cc: Eric W. Biederman <ebiederm@xmission.com>
>> Cc: Dmitry Safonov <0x7f454c46@gmail.com>
>> Acked-by: Michael Kerrisk <mtk.manpages@gmail.com>                                                                                                                                            
>
> Thomas and Andrew, could you merge this patch? I am sorry, I used the
> wrong subsystem prefix. Let me know if I need to send the third version
> of this patch.

Picking it up.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [tip: timers/urgent] proc, time/namespace: Show clock symbolic names in /proc/pid/timens_offsets
  2020-04-11 15:40     ` [PATCH v2] " Andrei Vagin
  2020-04-12  5:51       ` Michael Kerrisk (man-pages)
  2020-04-16  6:56       ` Andrei Vagin
@ 2020-04-16 10:15       ` tip-bot2 for Andrei Vagin
  2 siblings, 0 replies; 12+ messages in thread
From: tip-bot2 for Andrei Vagin @ 2020-04-16 10:15 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Michael Kerrisk, Andrei Vagin, Thomas Gleixner, Andrew Morton,
	Eric W. Biederman, Dmitry Safonov, stable, x86, LKML

The following commit has been merged into the timers/urgent branch of tip:

Commit-ID:     94d440d618467806009c8edc70b094d64e12ee5a
Gitweb:        https://git.kernel.org/tip/94d440d618467806009c8edc70b094d64e12ee5a
Author:        Andrei Vagin <avagin@gmail.com>
AuthorDate:    Sat, 11 Apr 2020 08:40:31 -07:00
Committer:     Thomas Gleixner <tglx@linutronix.de>
CommitterDate: Thu, 16 Apr 2020 12:10:54 +02:00

proc, time/namespace: Show clock symbolic names in /proc/pid/timens_offsets

Michael Kerrisk suggested to replace numeric clock IDs with symbolic names.

Now the content of these files looks like this:
$ cat /proc/774/timens_offsets
monotonic      864000         0
boottime      1728000         0

For setting offsets, both representations of clocks (numeric and symbolic)
can be used.

As for compatibility, it is acceptable to change things as long as
userspace doesn't care. The format of timens_offsets files is very new and
there are no userspace tools yet which rely on this format.

But three projects crun, util-linux and criu rely on the interface of
setting time offsets and this is why it's required to continue supporting
the numeric clock IDs on write.

Fixes: 04a8682a71be ("fs/proc: Introduce /proc/pid/timens_offsets")
Suggested-by: Michael Kerrisk <mtk.manpages@gmail.com>
Signed-off-by: Andrei Vagin <avagin@gmail.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Tested-by: Michael Kerrisk <mtk.manpages@gmail.com>
Acked-by: Michael Kerrisk <mtk.manpages@gmail.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Eric W. Biederman <ebiederm@xmission.com>
Cc: Dmitry Safonov <0x7f454c46@gmail.com>
Cc: stable@vger.kernel.org
Link: https://lkml.kernel.org/r/20200411154031.642557-1-avagin@gmail.com
---
 fs/proc/base.c          | 14 +++++++++++++-
 kernel/time/namespace.c | 15 ++++++++++++++-
 2 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/fs/proc/base.c b/fs/proc/base.c
index 6042b64..572898d 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -1573,6 +1573,7 @@ static ssize_t timens_offsets_write(struct file *file, const char __user *buf,
 	noffsets = 0;
 	for (pos = kbuf; pos; pos = next_line) {
 		struct proc_timens_offset *off = &offsets[noffsets];
+		char clock[10];
 		int err;
 
 		/* Find the end of line and ensure we don't look past it */
@@ -1584,10 +1585,21 @@ static ssize_t timens_offsets_write(struct file *file, const char __user *buf,
 				next_line = NULL;
 		}
 
-		err = sscanf(pos, "%u %lld %lu", &off->clockid,
+		err = sscanf(pos, "%9s %lld %lu", clock,
 				&off->val.tv_sec, &off->val.tv_nsec);
 		if (err != 3 || off->val.tv_nsec >= NSEC_PER_SEC)
 			goto out;
+
+		clock[sizeof(clock) - 1] = 0;
+		if (strcmp(clock, "monotonic") == 0 ||
+		    strcmp(clock, __stringify(CLOCK_MONOTONIC)) == 0)
+			off->clockid = CLOCK_MONOTONIC;
+		else if (strcmp(clock, "boottime") == 0 ||
+			 strcmp(clock, __stringify(CLOCK_BOOTTIME)) == 0)
+			off->clockid = CLOCK_BOOTTIME;
+		else
+			goto out;
+
 		noffsets++;
 		if (noffsets == ARRAY_SIZE(offsets)) {
 			if (next_line)
diff --git a/kernel/time/namespace.c b/kernel/time/namespace.c
index 3b30288..53bce34 100644
--- a/kernel/time/namespace.c
+++ b/kernel/time/namespace.c
@@ -338,7 +338,20 @@ static struct user_namespace *timens_owner(struct ns_common *ns)
 
 static void show_offset(struct seq_file *m, int clockid, struct timespec64 *ts)
 {
-	seq_printf(m, "%d %lld %ld\n", clockid, ts->tv_sec, ts->tv_nsec);
+	char *clock;
+
+	switch (clockid) {
+	case CLOCK_BOOTTIME:
+		clock = "boottime";
+		break;
+	case CLOCK_MONOTONIC:
+		clock = "monotonic";
+		break;
+	default:
+		clock = "unknown";
+		break;
+	}
+	seq_printf(m, "%-10s %10lld %9ld\n", clock, ts->tv_sec, ts->tv_nsec);
 }
 
 void proc_timens_show_offsets(struct task_struct *p, struct seq_file *m)

^ permalink raw reply related	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2020-04-16 10:17 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-11  6:52 [PATCH] timens: show clock symbolic names in /proc/pid/timens_offsets Andrei Vagin
2020-04-11 10:32 ` Michael Kerrisk (man-pages)
2020-04-11 10:36   ` Michael Kerrisk (man-pages)
2020-04-11 15:40     ` [PATCH v2] " Andrei Vagin
2020-04-12  5:51       ` Michael Kerrisk (man-pages)
2020-04-13 22:47         ` Andrew Morton
2020-04-14  0:04           ` Andrei Vagin
2020-04-14  9:51           ` Michael Kerrisk (man-pages)
2020-04-16  6:56       ` Andrei Vagin
2020-04-16  7:10         ` Michael Kerrisk (man-pages)
2020-04-16  9:52         ` Thomas Gleixner
2020-04-16 10:15       ` [tip: timers/urgent] proc, time/namespace: Show " tip-bot2 for Andrei Vagin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).