kernfs_notify() poll latency

Hi everyone,

I believe kernfs_notify() poll latency can be improved if the poll notification is done from kernfs_notify() directly rather than scheduled work context.

I am sure there are good reasons why the fsnotify notification must be done from scheduled work context (an obvious one is that it needs to be able to sleep). But I don't see any reason why the poll notification could not be done from kernfs_notify(). If there is any, please point it out - I would highly appreciate it.

I came across this while working on a project that uses the sysfs GPIO interface to wake a (real time) user space project on GPIO interrupts. I know that interface is deprecated, but I still believe it's a valid scenario and may occur with other drivers as well (current or future).

The sysfs GPIO interface (drivers/gpio/gpiolib-sysfs.c) interrupt handler relies on kernfs_notify() (actually sysfs_notify_dirent(), but that's just an alias) to wake any thread that may be poll()ing on the interrupt. It is important to wake the thread as quickly as possible and going through the kernel worker to handle the scheduled work is much slower. Since the kernel worker runs with normal priority, this can even become a case of priority inversion. If a higher priority thread hogs the CPU, it may delay the kernel worker and in turn the thread that needs to be notified (which could be a real time thread).

Best regards,
Radu Rendec

Radu Rendec (1):
Improve kernfs_notify() poll notification latency

 fs/kernfs/file.c | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

2.17.2

kernfs_notify() does two notifications: poll and fsnotify. Originally, both notifications were done from scheduled work context and all that kernfs_notify() did was schedule the work.

This patch simply moves the poll notification from the scheduled work handler to kernfs_notify(). The fsnotify notification still needs to be done from scheduled work context because it can sleep (it needs to lock a mutex).

If the poll notification is time critical (the notified thread needs to wake as quickly as possible), it's better to do it from kernfs_notify() directly. One example is calling sysfs_notify_dirent() from a hardware interrupt handler to wake up a thread and handle the interrupt in user space.

---
 fs/kernfs/file.c | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

index dbf5bc250bfd..f8d5021a652e 100644
@@ -857,7 +857,6 @@ static __poll_t kernfs_fop_poll(struct file *filp, poll_table *wait)
 static void kernfs_notify_workfn(struct work_struct *work)
 {
 	struct kernfs_node *kn;
-	struct kernfs_open_node *on;
 	struct kernfs_super_info *info;
 repeat:
 	/* pop one off the notify_list */

@@ -871,17 +870,6 @@ static void kernfs_notify_workfn(struct work_struct *work)
 	kn->attr.notify_next = NULL;
 	spin_unlock_irq(&kernfs_notify_lock);
 
-	/* kick poll */
-	spin_lock_irq(&kernfs_open_node_lock);
-
-	on = kn->attr.open;
-	if (on) {
-		atomic_inc(&on->event);
-		wake_up_interruptible(&on->poll);
-	}
-
-	spin_unlock_irq(&kernfs_open_node_lock);
-
 	/* kick fsnotify */
 	mutex_lock(&kernfs_mutex);
 

@@ -934,10 +922,21 @@ void kernfs_notify(struct kernfs_node *kn)
 {
 	static DECLARE_WORK(kernfs_notify_work, kernfs_notify_workfn);
 	unsigned long flags;
+	struct kernfs_open_node *on;
 
 	if (WARN_ON(kernfs_type(kn) != KERNFS_FILE))
 		return;
 
+	/* kick poll immediately */
+	spin_lock_irqsave(&kernfs_open_node_lock, flags);
+	on = kn->attr.open;
+	if (on) {
+		atomic_inc(&on->event);
+		wake_up_interruptible(&on->poll);
+	}
+	spin_unlock_irqrestore(&kernfs_open_node_lock, flags);
+
+	/* schedule work to kick fsnotify */
 	spin_lock_irqsave(&kernfs_notify_lock, flags);
 	if (!kn->attr.notify_next) {
 		kernfs_get(kn);

2.17.2

Greg, can you please route this one?

Thanks.

tejun

Sure, I'll queue it up soon...

greg k-h