Re: [take8 2/2] kevent: poll/select() notifications. Timer notifications.
From: Andrew Morton <hidden>
Date: 2006-08-12 08:39:15
Also in:
lkml
On Sat, 12 Aug 2006 12:18:35 +0400 Evgeniy Polyakov [off-list ref] wrote:
On Fri, Aug 11, 2006 at 08:45:31AM -0700, Andrew Morton (akpm@osdl.org) wrote:quoted
quoted
+static struct lock_class_key kevent_poll_key; + +void kevent_poll_reinit(struct file *file) +{ + lockdep_set_class(&file->st.lock, &kevent_poll_key); +}Why is this necessary?Locks for all storages are initialized in the same function, so lockdep thinks they are the same, so when later one lock is being held in proces context and other in BH or IRQ lockdep screams, so I reinitialize locks after spin_lock_init().
So why not simply run spin_lock_init() in the kevent_storage_init() caller? Does kevent_poll_reinit() have any callers?
quoted
quoted
+ st = (struct kevent_storage *)(t+1);It would be cleaner to create struct <something> { struct timer_list timer; struct kevent_storage storage; };
You missed this?
quoted
quoted
+ + kevent_storage_dequeue(st, k); + + kfree(t); + + return 0; +} + +static int kevent_timer_callback(struct kevent *k) +{ + struct kevent_storage *st = k->st; + struct timer_list *t = st->origin; + + if (!t) + return -ENODEV; + + k->event.ret_data[0] = (__u32)jiffies;What does this do? Does it expose jiffies to userspace? It truncates jiffies on 64-bit machines.It is a hint when timer was stopped.
What does that mean? What is it for? Does it expose jiffies to userspace? It truncates jiffies on 64-bit machines. Please respond to all review comments and questions.
quoted
quoted
+late_initcall(kevent_init_timer);module_init() would be more typical. If there was a reason for using late_initcall(), that reason should be commented.No, there are no reasons to use late_initcall() in any kevent initialization function, I do not use module_init() since kevent can not be modular. It can be replaced with pure __init function. Should it?
We use module_init() for non-modular modules all the time. Try doing grep module_init */*.c