On Sat, Aug 12, 2006 at 01:38:35AM -0700, Andrew Morton (akpm@osdl.org) wrote:

On Sat, 12 Aug 2006 12:18:35 +0400
Evgeniy Polyakov [off-list ref] wrote:

quoted

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?

I separated storage initialization into special function, although it is
quite simple, but it allows not to have a lot of duplicated steps in
each origin (inode, socket, file, AIO, network AIO, poll, timer and so
on). If later some members will be changed or added/removed there will
be no need to change them all instead change one function.

Does kevent_poll_reinit() have any callers?

Only poll nitialization.

quoted

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?

No problem, I will create such structure instead of pointer-based
scheme.
 

quoted

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?

My test code prints it to stdout and I can show a nice graph of how
precise timer is.

Does it expose jiffies to userspace?

It is a timestamp of fired condition (measured in jiffies), I can not
say if it exposes jiffies to userspace or not.
It is not a requirement to use that field, it can contain zero or any
other number, I placed jiffies there.

It truncates jiffies on 64-bit machines.

For the purpose of evnt timestamp it is more than enough.

Please respond to all review comments and questions.

Sorry if something got lost.

quoted

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

Ok, I will use it instead late_initcall().

-- 
	Evgeniy Polyakov