[PATCH 0/3] posix timers: Extend kernel API to report more info about timers

[PATCH 0/3] posix timers: Extend kernel API to report more info about timers

[Pavel Emelyanov]

Hi.

I'm working on the checkpoint-restore project (http://criu.org), briefly
it's aim is to collect information about process' state and saving it so
that later it is possible to recreate the processes in the very same state
as they were, using the collected information.

One part of the task's state is the posix timers that this task has created.
Currently kernel doesn't provide any API for getting information about
what timers are currently created by process and in which state they are.
I'd like to extend the posix timers API to provide more information about
timers.

Another problem with timers is the timer ID. Currently IDs are generated
from global IDR and this makes it impossible to restore a timer from
the saved state in general, as the required ID may be already busy at the
time of restore.

That said, I propose to

1. Change the way timer IDs are generated. This was done some time ago, so
   I'm just re-sending this patch;
2. Add a system call that will list timer IDs created by the calling process;
3. Add a system call that will allow to get the sigevent information about
   particular timer in the sigaction-like manner.

This is actually an RFC to start discussion about how the described problems
can be addressed. Thus, if the approach with new system calls is not acceptable,
I'm OK to implement this in any other form.

Thanks,
Pavel

[PATCH 1/3] posix timers: Allocate timer id per process

[Pavel Emelyanov]

From: Stanislav Kinsbursky <skinsbursky@parallels.com>

Patch replaces global idr with global hash table for posix timers and
makes timer ids unique not globally, but per process. Next free timer id is
type of integer and stored on signal struct (posix_timer_id). If free timer
id reaches negative value on timer creation, it will be dropped to zero and
-EAGAIN will be returned to user.

Hash table has 512 slots.
Key is constructed as follows:
key = hash_32(hash_32(current->signal) ^ posix_timer_id));

Note: with this patch, id, returned to user, is not the minimal free
amymore. It means, that id, returned to user space in loop, listed below, will
be increasing on each iteration till INT_MAX and then dropped to zero:

while(1) {
	id = timer_create(...);
	timer_delete(id);
}

Signed-off-by: Stanislav Kinsbursky <skinsbursky@parallels.com>
Signed-off-by: Pavel Emelyanov <xemul@parallels.com>
---
 include/linux/posix-timers.h |    1 +
 include/linux/sched.h        |    3 +-
 kernel/posix-timers.c        |  114 +++++++++++++++++++++++++++--------------
 3 files changed, 78 insertions(+), 40 deletions(-)

diff --git a/include/linux/posix-timers.h b/include/linux/posix-timers.h
index 042058f..60bac69 100644
--- a/include/linux/posix-timers.h
+++ b/include/linux/posix-timers.h
@@ -55,6 +55,7 @@ struct cpu_timer_list {
 /* POSIX.1b interval timer structure. */
 struct k_itimer {
 	struct list_head list;		/* free/ allocate list */
+	struct hlist_node t_hash;
 	spinlock_t it_lock;
 	clockid_t it_clock;		/* which timer type */
 	timer_t it_id;			/* timer id */
diff --git a/include/linux/sched.h b/include/linux/sched.h
index d211247..d0d9a8b 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -561,7 +561,8 @@ struct signal_struct {
 	unsigned int		has_child_subreaper:1;
 
 	/* POSIX.1b Interval Timers */
-	struct list_head posix_timers;
+	int			posix_timer_id;
+	struct list_head	posix_timers;
 
 	/* ITIMER_REAL timer for the process */
 	struct hrtimer real_timer;
diff --git a/kernel/posix-timers.c b/kernel/posix-timers.c
index 69185ae..9cfb86a 100644
--- a/kernel/posix-timers.c
+++ b/kernel/posix-timers.c
@@ -40,7 +40,7 @@
 #include <linux/list.h>
 #include <linux/init.h>
 #include <linux/compiler.h>
-#include <linux/idr.h>
+#include <linux/hash.h>
 #include <linux/posix-clock.h>
 #include <linux/posix-timers.h>
 #include <linux/syscalls.h>
@@ -49,29 +49,25 @@
 #include <linux/export.h>
 
 /*
- * Management arrays for POSIX timers.	 Timers are kept in slab memory
- * Timer ids are allocated by an external routine that keeps track of the
- * id and the timer.  The external interface is:
- *
- * void *idr_find(struct idr *idp, int id);           to find timer_id <id>
- * int idr_get_new(struct idr *idp, void *ptr);       to get a new id and
- *                                                    related it to <ptr>
- * void idr_remove(struct idr *idp, int id);          to release <id>
- * void idr_init(struct idr *idp);                    to initialize <idp>
- *                                                    which we supply.
- * The idr_get_new *may* call slab for more memory so it must not be
- * called under a spin lock.  Likewise idr_remore may release memory
- * (but it may be ok to do this under a lock...).
- * idr_find is just a memory look up and is quite fast.  A -1 return
- * indicates that the requested id does not exist.
+ * Management arrays for POSIX timers. Timers are now kept in static hash table
+ * with 512 entries.
+ * Timer ids are allocated by local routine, which selects proper hash head by
+ * key, constructed from current->signal address and per signal struct counter.
+ * This keeps timer ids unique per process, but now they can intersect between
+ * processes.
  */
 
 /*
  * Lets keep our timers in a slab cache :-)
  */
 static struct kmem_cache *posix_timers_cache;
-static struct idr posix_timers_id;
-static DEFINE_SPINLOCK(idr_lock);
+
+#define POSIX_TIMERS_HASH_BITS		9
+#define POSIX_TIMERS_HASH_SIZE		(1 << POSIX_TIMERS_HASH_BITS)
+
+/* Hash table is size of PAGE currently */
+static struct hlist_head posix_timers_hashtable[POSIX_TIMERS_HASH_SIZE];
+static DEFINE_SPINLOCK(hash_lock);
 
 /*
  * we assume that the new SIGEV_THREAD_ID shares no bits with the other
@@ -152,6 +148,57 @@ static struct k_itimer *__lock_timer(timer_t timer_id, unsigned long *flags);
 	__timr;								   \
 })
 
+static int hash(struct signal_struct *sig, unsigned int nr)
+{
+	return hash_32(hash32_ptr(sig) ^ nr, POSIX_TIMERS_HASH_BITS);
+}
+
+static struct k_itimer *__posix_timers_find(struct hlist_head *head,
+					    struct signal_struct *sig,
+					    timer_t id)
+{
+	struct hlist_node *node;
+	struct k_itimer *timer;
+
+	hlist_for_each_entry_rcu(timer, node, head, t_hash) {
+		if ((timer->it_signal == sig) && (timer->it_id == id))
+			return timer;
+	}
+	return NULL;
+}
+
+static struct k_itimer *posix_timer_by_id(timer_t id)
+{
+	struct signal_struct *sig = current->signal;
+	struct hlist_head *head = &posix_timers_hashtable[hash(sig, id)];
+
+	return __posix_timers_find(head, sig, id);
+}
+
+static int posix_timer_add(struct k_itimer *timer)
+{
+	struct signal_struct *sig = current->signal;
+	int first_free_id = sig->posix_timer_id;
+	struct hlist_head *head;
+	int ret = -ENOENT;
+
+	do {
+		spin_lock(&hash_lock);
+		head = &posix_timers_hashtable[hash(sig, sig->posix_timer_id)];
+		if (!__posix_timers_find(head, sig, sig->posix_timer_id)) {
+			hlist_add_head_rcu(&timer->t_hash, head);
+			ret = sig->posix_timer_id;
+		}
+		if (++sig->posix_timer_id < 0)
+			sig->posix_timer_id = 0;
+		if ((sig->posix_timer_id == first_free_id) && (ret == -ENOENT))
+			/* Loop over all possible ids completed */
+			ret = -EAGAIN;
+		spin_unlock(&hash_lock);
+	} while (ret == -ENOENT);
+	return ret;
+}
+
 static inline void unlock_timer(struct k_itimer *timr, unsigned long flags)
 {
 	spin_unlock_irqrestore(&timr->it_lock, flags);
@@ -271,6 +318,7 @@ static __init int init_posix_timers(void)
 		.timer_get	= common_timer_get,
 		.timer_del	= common_timer_del,
 	};
+	int i;
 
 	posix_timers_register_clock(CLOCK_REALTIME, &clock_realtime);
 	posix_timers_register_clock(CLOCK_MONOTONIC, &clock_monotonic);
@@ -282,7 +330,8 @@ static __init int init_posix_timers(void)
 	posix_timers_cache = kmem_cache_create("posix_timers_cache",
 					sizeof (struct k_itimer), 0, SLAB_PANIC,
 					NULL);
-	idr_init(&posix_timers_id);
+	for (i = 0; i < POSIX_TIMERS_HASH_SIZE; i++)
+		INIT_HLIST_HEAD(&posix_timers_hashtable[i]);
 	return 0;
 }
 
@@ -504,9 +553,9 @@ static void release_posix_timer(struct k_itimer *tmr, int it_id_set)
 {
 	if (it_id_set) {
 		unsigned long flags;
-		spin_lock_irqsave(&idr_lock, flags);
-		idr_remove(&posix_timers_id, tmr->it_id);
-		spin_unlock_irqrestore(&idr_lock, flags);
+		spin_lock_irqsave(&hash_lock, flags);
+		hlist_del_rcu(&tmr->t_hash);
+		spin_unlock_irqrestore(&hash_lock, flags);
 	}
 	put_pid(tmr->it_pid);
 	sigqueue_free(tmr->sigq);
@@ -552,22 +601,9 @@ SYSCALL_DEFINE3(timer_create, const clockid_t, which_clock,
 		return -EAGAIN;
 
 	spin_lock_init(&new_timer->it_lock);
- retry:
-	if (unlikely(!idr_pre_get(&posix_timers_id, GFP_KERNEL))) {
-		error = -EAGAIN;
-		goto out;
-	}
-	spin_lock_irq(&idr_lock);
-	error = idr_get_new(&posix_timers_id, new_timer, &new_timer_id);
-	spin_unlock_irq(&idr_lock);
-	if (error) {
-		if (error == -EAGAIN)
-			goto retry;
-		/*
-		 * Weird looking, but we return EAGAIN if the IDR is
-		 * full (proper POSIX return value for this)
-		 */
-		error = -EAGAIN;
+	new_timer_id = posix_timer_add(new_timer);
+	if (new_timer_id < 0) {
+		error = new_timer_id;
 		goto out;
 	}
 
@@ -640,7 +676,7 @@ static struct k_itimer *__lock_timer(timer_t timer_id, unsigned long *flags)
 	struct k_itimer *timr;
 
 	rcu_read_lock();
-	timr = idr_find(&posix_timers_id, (int)timer_id);
+	timr = posix_timer_by_id(timer_id);
 	if (timr) {
 		spin_lock_irqsave(&timr->it_lock, *flags);
 		if (timr->it_signal == current->signal) {
-- 
1.7.6.5

[PATCH 2/3] posix timers: Add syscall that lists timer IDs armed by process

[Pavel Emelyanov]

The sys_timer_list syscall accepts clock id, buffer size to store
the timers ids and the pointer to the buffer itself.

The number of timers of clockid type is returned and these timers'
ids are put into the provided buffer. If the buffer is not enough
for all timers, then only part of ids are put into it, but the
actual number of timers is reported anyway. If the buffer is bigger
than required, then its tail is left untouched and the number of
timers returned from the syscall denotes how much of ids are there.

Signed-off-by: Pavel Emelyanov <xemul@parallels.com>
---
 arch/x86/syscalls/syscall_64.tbl |    1 +
 include/linux/syscalls.h         |    2 +
 kernel/posix-timers.c            |   48 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 51 insertions(+), 0 deletions(-)

diff --git a/arch/x86/syscalls/syscall_64.tbl b/arch/x86/syscalls/syscall_64.tbl
index dc97328..72b6ee6 100644
--- a/arch/x86/syscalls/syscall_64.tbl
+++ b/arch/x86/syscalls/syscall_64.tbl
@@ -320,6 +320,7 @@
 311	64	process_vm_writev	sys_process_vm_writev
 312	common	kcmp			sys_kcmp
 313	common	finit_module		sys_finit_module
+314	common	timer_list		sys_timer_list
 
 #
 # x32-specific system call numbers start at 512 to avoid cache impact
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index 45e2db2..dc951da 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -319,6 +319,8 @@ asmlinkage long sys_timer_settime(timer_t timer_id, int flags,
 				const struct itimerspec __user *new_setting,
 				struct itimerspec __user *old_setting);
 asmlinkage long sys_timer_delete(timer_t timer_id);
+asmlinkage long sys_timer_list(const clockid_t clock_id, int nr,
+				timer_t __user *timer_ids);
 asmlinkage long sys_clock_settime(clockid_t which_clock,
 				const struct timespec __user *tp);
 asmlinkage long sys_clock_gettime(clockid_t which_clock,
diff --git a/kernel/posix-timers.c b/kernel/posix-timers.c
index 9cfb86a..46cee59 100644
--- a/kernel/posix-timers.c
+++ b/kernel/posix-timers.c
@@ -1103,3 +1103,51 @@ long clock_nanosleep_restart(struct restart_block *restart_block)
 
 	return kc->nsleep_restart(restart_block);
 }
+
+SYSCALL_DEFINE3(timer_list, const clockid_t, clock_id,
+		int, unr, timer_t __user *, timer_ids)
+{
+	struct k_itimer *timer;
+	timer_t *buf;
+	int nr = 0, buf_nr;
+
+	spin_lock_irq(&current->sighand->siglock);
+	list_for_each_entry(timer, &current->signal->posix_timers, list)
+		if (timer->it_clock == clock_id)
+			nr++;
+	spin_unlock_irq(&current->sighand->siglock);
+
+	if (!unr)
+		goto out;
+
+try_again:
+	buf_nr = min(nr, unr);
+	buf = kmalloc(buf_nr * sizeof(timer_t), GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	nr = 0;
+	spin_lock_irq(&current->sighand->siglock);
+	list_for_each_entry(timer, &current->signal->posix_timers, list) {
+		if (timer->it_clock != clock_id)
+			continue;
+
+		if (nr < buf_nr)
+			buf[nr] = timer->it_id;
+
+		nr++;
+	}
+	spin_unlock_irq(&current->sighand->siglock);
+
+	if (nr > buf_nr) {
+		kfree(buf);
+		goto try_again;
+	}
+
+	if (copy_to_user(timer_ids, buf, nr * sizeof(timer_t)))
+		unr = -EFAULT;
+
+	kfree(buf);
+out:
+	return nr;
+}
-- 
1.7.6.5

[PATCH 3/3] posix timers: Add syscall that works on timer sigevent

[Pavel Emelyanov]

The intention is to make a syscall, that works like sigaction
but on a posix timer. I.e. -- puts (if provided) new sigevent
on the timer and reports the previous value (if requested).

That said, the syscall accepts timer id to work on, a pointer
to the new sigevent (may be NULL, meaning that the existing
sigevent remains intact) and pointer to memory buffer where to
put the existing sigevent (may be NULL as well, sigevent is
not reported in this case).

For how I didn't implement the new sigevent assignment, as it
requires a little bit of trickery with timer. If the proposed
idea itself is OK useful I will implement this part as well in
the v2 series.

Signed-off-by: Pavel Emelyanov <xemul@parallels.com>
---
 arch/x86/syscalls/syscall_64.tbl |    1 +
 include/linux/syscalls.h         |    3 +++
 kernel/posix-timers.c            |   28 ++++++++++++++++++++++++++++
 3 files changed, 32 insertions(+), 0 deletions(-)

diff --git a/arch/x86/syscalls/syscall_64.tbl b/arch/x86/syscalls/syscall_64.tbl
index 72b6ee6..c2c1743 100644
--- a/arch/x86/syscalls/syscall_64.tbl
+++ b/arch/x86/syscalls/syscall_64.tbl
@@ -321,6 +321,7 @@
 312	common	kcmp			sys_kcmp
 313	common	finit_module		sys_finit_module
 314	common	timer_list		sys_timer_list
+315	64	timer_sigevent		sys_timer_sigevent
 
 #
 # x32-specific system call numbers start at 512 to avoid cache impact
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index dc951da..7d121c5 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -312,6 +312,9 @@ asmlinkage long sys_setitimer(int which,
 asmlinkage long sys_timer_create(clockid_t which_clock,
 				 struct sigevent __user *timer_event_spec,
 				 timer_t __user * created_timer_id);
+asmlinkage long sys_timer_sigevent(timer_t timer_id,
+				struct sigevent __user *new_event,
+				struct sigevent __user *old_event);
 asmlinkage long sys_timer_gettime(timer_t timer_id,
 				struct itimerspec __user *setting);
 asmlinkage long sys_timer_getoverrun(timer_t timer_id);
diff --git a/kernel/posix-timers.c b/kernel/posix-timers.c
index 46cee59..29de379 100644
--- a/kernel/posix-timers.c
+++ b/kernel/posix-timers.c
@@ -1151,3 +1151,31 @@ try_again:
 out:
 	return nr;
 }
+
+SYSCALL_DEFINE3(timer_sigevent, timer_t, timer_id, struct sigevent __user *, new_event,
+		struct sigevent __user *, old_event)
+{
+	struct k_itimer *timer;
+	unsigned long flags;
+	struct sigevent event;
+	int ret = 0;
+
+	if (new_event)
+		return -EINVAL;
+
+	timer = lock_timer(timer_id, &flags);
+	if (!timer)
+		return -EINVAL;
+
+	event.sigev_notify = timer->it_sigev_notify;
+	event.sigev_signo = timer->sigq->info.si_signo;
+	event.sigev_value = timer->sigq->info.si_value;
+	event.sigev_notify_thread_id = pid_vnr(timer->it_pid);
+
+	unlock_timer(timer, flags);
+
+	if (old_event && copy_to_user(old_event, &event, sizeof(event)))
+		ret = -EFAULT;
+
+	return ret;
+}
-- 
1.7.6.5

Re: [PATCH 1/3] posix timers: Allocate timer id per process

[Sasha Levin]

On Thu, Feb 14, 2013 at 11:19 AM, Pavel Emelyanov <xemul@parallels.com> wrote:
> From: Stanislav Kinsbursky <skinsbursky@parallels.com>
>
> Patch replaces global idr with global hash table for posix timers and
> makes timer ids unique not globally, but per process. Next free timer id is
> type of integer and stored on signal struct (posix_timer_id). If free timer
> id reaches negative value on timer creation, it will be dropped to zero and
> -EAGAIN will be returned to user.
>
> Hash table has 512 slots.
> Key is constructed as follows:
> key = hash_32(hash_32(current->signal) ^ posix_timer_id));
>
> Note: with this patch, id, returned to user, is not the minimal free
> amymore. It means, that id, returned to user space in loop, listed below, will
> be increasing on each iteration till INT_MAX and then dropped to zero:
>
> while(1) {
>         id = timer_create(...);
>         timer_delete(id);
> }
>
> Signed-off-by: Stanislav Kinsbursky <skinsbursky@parallels.com>
> Signed-off-by: Pavel Emelyanov <xemul@parallels.com>

Hi Pavel,

Why not use linux/hashtable.h?


Thanks,
Sasha

Re: [PATCH 1/3] posix timers: Allocate timer id per process

[Pavel Emelyanov]

On 02/15/2013 12:13 AM, Sasha Levin wrote:
> On Thu, Feb 14, 2013 at 11:19 AM, Pavel Emelyanov <xemul@parallels.com> wrote:
>> From: Stanislav Kinsbursky <skinsbursky@parallels.com>
>>
>> Patch replaces global idr with global hash table for posix timers and
>> makes timer ids unique not globally, but per process. Next free timer id is
>> type of integer and stored on signal struct (posix_timer_id). If free timer
>> id reaches negative value on timer creation, it will be dropped to zero and
>> -EAGAIN will be returned to user.
>>
>> Hash table has 512 slots.
>> Key is constructed as follows:
>> key = hash_32(hash_32(current->signal) ^ posix_timer_id));
>>
>> Note: with this patch, id, returned to user, is not the minimal free
>> amymore. It means, that id, returned to user space in loop, listed below, will
>> be increasing on each iteration till INT_MAX and then dropped to zero:
>>
>> while(1) {
>>         id = timer_create(...);
>>         timer_delete(id);
>> }
>>
>> Signed-off-by: Stanislav Kinsbursky <skinsbursky@parallels.com>
>> Signed-off-by: Pavel Emelyanov <xemul@parallels.com>
> 
> Hi Pavel,
> 
> Why not use linux/hashtable.h?

Simply because this patch was just picked from the previous discussions
as is :) I will tune it to use hashtable.h in the next iteration.

> 
> Thanks,
> Sasha
> .
> 

Re: [PATCH 3/3] posix timers: Add syscall that works on timer sigevent

[Jann Horn]

[-- Attachment #1: Type: text/plain, Size: 601 bytes --]

On Thu, Feb 14, 2013 at 08:19:45PM +0400, Pavel Emelyanov wrote:
> +	struct sigevent event;
[...]
> +	event.sigev_notify = timer->it_sigev_notify;
> +	event.sigev_signo = timer->sigq->info.si_signo;
> +	event.sigev_value = timer->sigq->info.si_value;
> +	event.sigev_notify_thread_id = pid_vnr(timer->it_pid);
[...]
> +	if (old_event && copy_to_user(old_event, &event, sizeof(event)))

Won't this leak uninitialized kernel stack data to userspace? As far as I can
see, the _sigev_un union is bigger than the _tid field in it, so the rest of
it will be copied over without initialization, right?

Jann

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 3/3] posix timers: Add syscall that works on timer sigevent

[Pavel Emelyanov]

On 02/17/2013 05:42 PM, Jann Horn wrote:
> On Thu, Feb 14, 2013 at 08:19:45PM +0400, Pavel Emelyanov wrote:
>> +	struct sigevent event;
> [...]
>> +	event.sigev_notify = timer->it_sigev_notify;
>> +	event.sigev_signo = timer->sigq->info.si_signo;
>> +	event.sigev_value = timer->sigq->info.si_value;
>> +	event.sigev_notify_thread_id = pid_vnr(timer->it_pid);
> [...]
>> +	if (old_event && copy_to_user(old_event, &event, sizeof(event)))
> 
> Won't this leak uninitialized kernel stack data to userspace? As far as I can
> see, the _sigev_un union is bigger than the _tid field in it, so the rest of
> it will be copied over without initialization, right?

Indeed :( Thanks for catching this, I will memezro the thing before copying
to the user-space.

> Jann
> 

Re: [PATCH 0/3] posix timers: Extend kernel API to report more info about timers

[Matthew Helsley]

On Thu, Feb 14, 2013 at 8:18 AM, Pavel Emelyanov <xemul@parallels.com> wrote:
> Hi.
>
> I'm working on the checkpoint-restore project (http://criu.org), briefly
> it's aim is to collect information about process' state and saving it so
> that later it is possible to recreate the processes in the very same state
> as they were, using the collected information.
>
> One part of the task's state is the posix timers that this task has created.
> Currently kernel doesn't provide any API for getting information about
> what timers are currently created by process and in which state they are.
> I'd like to extend the posix timers API to provide more information about
> timers.
>
> Another problem with timers is the timer ID. Currently IDs are generated
> from global IDR and this makes it impossible to restore a timer from
> the saved state in general, as the required ID may be already busy at the
> time of restore.
>
> That said, I propose to
>
> 1. Change the way timer IDs are generated. This was done some time ago, so
>    I'm just re-sending this patch;

Seems fine in principle. Aside: I noticed there were some
important-looking patches to the idr usage in timer id allocation
today...

> 2. Add a system call that will list timer IDs created by the calling process;

If timers were listed in /proc like fds then you wouldn't need this
syscall. If we keep adding new syscalls like this CRIU will be
needlessly x86-specific when it could have been written more portably.

> 3. Add a system call that will allow to get the sigevent information about
>    particular timer in the sigaction-like manner.

You mentioned "extending the POSIX timer API". Isn't that something
best left to standards bodies lest your changes conflict with theirs?
Again, if this were a /proc interface you wouldn't have that issue
(you'll have others ;)).

>
> This is actually an RFC to start discussion about how the described problems
> can be addressed. Thus, if the approach with new system calls is not acceptable,
> I'm OK to implement this in any other form.

My preference is for "other form" for the reasons above.

Cheers,
    -Matt Helsley

Re: [PATCH 0/3] posix timers: Extend kernel API to report more info about timers

[Pavel Emelyanov]

On 02/21/2013 05:21 AM, Matthew Helsley wrote:
> On Thu, Feb 14, 2013 at 8:18 AM, Pavel Emelyanov <xemul@parallels.com> wrote:
>> Hi.
>>
>> I'm working on the checkpoint-restore project (http://criu.org), briefly
>> it's aim is to collect information about process' state and saving it so
>> that later it is possible to recreate the processes in the very same state
>> as they were, using the collected information.
>>
>> One part of the task's state is the posix timers that this task has created.
>> Currently kernel doesn't provide any API for getting information about
>> what timers are currently created by process and in which state they are.
>> I'd like to extend the posix timers API to provide more information about
>> timers.
>>
>> Another problem with timers is the timer ID. Currently IDs are generated
>> from global IDR and this makes it impossible to restore a timer from
>> the saved state in general, as the required ID may be already busy at the
>> time of restore.
>>
>> That said, I propose to
>>
>> 1. Change the way timer IDs are generated. This was done some time ago, so
>>    I'm just re-sending this patch;
> 
> Seems fine in principle. Aside: I noticed there were some
> important-looking patches to the idr usage in timer id allocation
> today...

Hm, OK, will try to find one.

>> 2. Add a system call that will list timer IDs created by the calling process;
> 
> If timers were listed in /proc like fds then you wouldn't need this
> syscall. If we keep adding new syscalls like this CRIU will be
> needlessly x86-specific when it could have been written more portably.
> 
>> 3. Add a system call that will allow to get the sigevent information about
>>    particular timer in the sigaction-like manner.
> 
> You mentioned "extending the POSIX timer API". Isn't that something
> best left to standards bodies lest your changes conflict with theirs?
> Again, if this were a /proc interface you wouldn't have that issue
> (you'll have others ;)).
> 
>>
>> This is actually an RFC to start discussion about how the described problems
>> can be addressed. Thus, if the approach with new system calls is not acceptable,
>> I'm OK to implement this in any other form.
> 
> My preference is for "other form" for the reasons above.

No problem, proc is OK for me as well. I will look at what can be done here.

Thanks for the feedback!

> Cheers,
>     -Matt Helsley
> .
>