posix timers: Extend kernel API to report more info about timers (v2)
Subsystems (3)
cover posix timers: Extend kernel API to report more info about timers (v2)
Hi.
Here's another approach to address the problems with insufficient API of posix timers.
Currently kernel doesn't provide any API for getting information about what timers are currently created by process and in which state they are. Initially the proposal was to add a couple of system calls that would list timers owned by a task and provide info about sigevent configuration of particular timer.
Matthew pointed out, that fixing the API in _that_ way should better be left to POSIX people :) and we'd better go with new /proc files. So here's how the /proc file can look like to provide info about timers (patches 2 and 3).
And, yet again, I'd like to fix the way timer IDs are generated by replacing global IDR with hashtable and making IDs generation per-signal_struct (patch 1). A note: mentioned by Matthew important patches about timers IDR are "don't feed negative ID into idr_xxx engine". Since I remove the whole IDR stuff from timers, I do conflict with them, but this is easy to fix :)
Thanks,
Pavel
PATCH 1/3 posix timers: Allocate timer id per process (v2)
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);
}In this version the helpers from include/linux/hashtable.h are used instead of hand-made hash-table.
--- include/linux/posix-timers.h | 1 + include/linux/sched.h | 3 +- kernel/posix-timers.c | 108 +++++++++++++++++++++++++++--------------- 3 files changed, 72 insertions(+), 40 deletions(-)
include/linux/posix-timers.h +1 -0
@@ -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 */
include/linux/sched.h +2 -1
@@ -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;
kernel/posix-timers.c +69 -39
158 lines not on this page · raw message
Signature · 1 line
1.7.6.5
Re: [PATCH 1/3] posix timers: Allocate timer id per process (v2)
Pavel,
Quoted 14 lines · Pavel Emelyanov
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-EAGAINwill 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_MAXand then dropped to zero:
Quoted 7 lines
while(1) { id = timer_create(...); timer_delete(id); }In this version the helpers from
include/linux/hashtable.hare used instead of hand-made hash-table.
I think the patches are ready to go, though the changelog needs quite some care.
It's explaining what the patch does and not why. Can you please rework?
Thanks,
tglx
PATCH 2/3 posix-timers: Introduce /proc/<pid>/timers file
In reply to Pavel EmelyanovThis file is about to contain info about posix timers armed by a process. Since these timers are shared between threads, this file is present on tgid level only, no such thing in tid subdirs.
The file format is expected to be the "/proc/<pid>/smaps"-like, i.e. each timer will occupy seveal lines to allow for future extending.
Each new timer entry starts with the
ID: <number>
line which is added by this patch.
--- fs/proc/base.c | 83 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 83 insertions(+), 0 deletions(-)
fs/proc/base.c +83 -0
@@ -85,6 +85,7 @@#include <linux/fs_struct.h> #include <linux/slab.h> #include <linux/flex_array.h> +#include <linux/posix-timers.h> #ifdef CONFIG_HARDWALL #include <asm/hardwall.h> #endif
@@ -2012,6 +2013,85 @@ static const struct file_operations proc_map_files_operations = { .llseek = default_llseek, }; +struct timers_private { +struct pid *pid; +struct task_struct *task; +struct sighand_struct *sighand; +unsigned long flags; +}; + +static void *timers_start (struct seq_file *m,loff_t *pos) +{ +struct timers_private *tp = m->private; + + tp->task =get_pid_task (tp->pid,PIDTYPE_PID ); +if (!tp->task) +return ERR_PTR (-ESRCH ); + + tp->sighand =lock_task_sighand (tp->task, &tp->flags); +if (!tp->sighand) +return ERR_PTR (-ESRCH ); + +return seq_list_start (&tp->task->signal->posix_timers, *pos); +} + +static void *timers_next (struct seq_file *m,void *v,loff_t *pos) +{ +struct timers_private *tp = m->private; +return seq_list_next (v, &tp->task->signal->posix_timers, pos); +} + +static void timers_stop (struct seq_file *m,void *v) +{ +struct timers_private *tp = m->private; + +if (tp->sighand) { +unlock_task_sighand (tp->task, &tp->flags); + tp->sighand =NULL ; + } + +if (tp->task) { +put_task_struct (tp->task); + tp->task =NULL ; + } +} + +static int show_timer (struct seq_file *m,void *v) +{ +struct k_itimer *timer; + + timer = list_entry((struct list_head *)v, struct k_itimer, list); +seq_printf (m,"ID: %d\n" , timer->it_id); + +return 0 ; +} + +static const struct seq_operations proc_timers_seq_ops = { + .start = timers_start, + .next = timers_next, + .stop = timers_stop, + .show = show_timer, +}; + +static int proc_timers_open (struct inode *inode,struct file *file) +{ +struct timers_private *tp; + + tp =__seq_open_private (file, &proc_timers_seq_ops, +sizeof (struct timers_private )); +if (!tp) +return -ENOMEM ; + + tp->pid =proc_pid (inode); +return 0 ; +} + +static const struct file_operations proc_timers_operations = { + .open = proc_timers_open, + .read = seq_read, + .llseek = seq_lseek, + .release = seq_release_private, +};#endif /* CONFIG_CHECKPOINT_RESTORE */ static struct dentry *proc_pident_instantiate(struct inode *dir,
@@ -2582,6 +2662,9 @@ static const struct pid_entry tgid_base_stuff[] = { REG("gid_map", S_IRUGO|S_IWUSR, proc_gid_map_operations), REG("projid_map", S_IRUGO|S_IWUSR, proc_projid_map_operations),#endif +#ifdef CONFIG_CHECKPOINT_RESTORE + REG("timers", S_IRUGO, proc_timers_operations), +#endif }; static int proc_tgid_base_readdir(struct file * filp,
Signature · 1 line
1.7.6.5
PATCH 3/3 posix-timers: Show sigevent info in proc file
In reply to Pavel EmelyanovAfter the "ID:" line there go
1. "signal:" line, that shows signal number and sigval bits;
2. "notify:" line, that shows the timer notification method.
The timer entry would looke like this:
ID: 123
signal: 14/0000000000b005d0
notify: signal/pid.732
This information is enough to understand ho the timer_create() was called for each particular timer.
--- fs/proc/base.c | 17 +++++++++++++++++ 1 files changed, 17 insertions(+), 0 deletions(-)
fs/proc/base.c +17 -0
@@ -2017,6 +2017,7 @@ struct timers_private {struct pid *pid;struct task_struct *task;struct sighand_struct *sighand; +struct pid_namespace *ns;unsigned long flags; };
@@ -2059,9 +2060,24 @@ static void timers_stop(struct seq_file *m, void *v)static int show_timer (struct seq_file *m,void *v) {struct k_itimer *timer; +struct timers_private *tp = m->private; +int notify; +static char *nstr[] = { + [SIGEV_SIGNAL ] ="signal" , + [SIGEV_NONE ] ="none" , + [SIGEV_THREAD ] ="thread" , + }; timer = list_entry((struct list_head *)v, struct k_itimer, list); + notify = timer->it_sigev_notify; +seq_printf (m,"ID: %d\n" , timer->it_id); +seq_printf (m,"signal: %d/%p\n" , timer->sigq->info.si_signo, + timer->sigq->info.si_value.sival_ptr); +seq_printf (m,"notify: %s/%s.%d\n" , + nstr[notify & ~SIGEV_THREAD_ID ], + (notify &SIGEV_THREAD_ID ) ?"tid" :"pid" , +pid_nr_ns (timer->it_pid, tp->ns));return 0 ; }
@@ -2083,6 +2099,7 @@ static int proc_timers_open(struct inode *inode, struct file *file)return -ENOMEM ; tp->pid =proc_pid (inode); + tp->ns = inode->i_sb->s_fs_info;return 0 ; }
Signature · 1 line
1.7.6.5
Quoted 4 lines · Pavel Emelyanov
Hi.
Here's another approach to address the problems with insufficient API of posix timers.
Gentlemen,
If you don't mind, I'd like to remind you about this set :) If you could find some time in your schedule and share your thoughts about one, it would be appreciated!
Thank you.
Quoted 7 lines · Pavel Emelyanov
Quoted 4 lines · Pavel Emelyanov
Hi.
Here's another approach to address the problems with insufficient API of posix timers.
Gentlemen,
Sir,
that caught my attention. Will move it up on my todo list :)