Skip to content

Commit

Permalink
relay: Use irq_work instead of plain timer for deferred wakeup
Browse files Browse the repository at this point in the history
Relay avoids calling wake_up_interruptible() for doing the wakeup of
readers/consumers, waiting for the generation of new data, from the
context of a process which produced the data.  This is apparently done to
prevent the possibility of a deadlock in case Scheduler itself is is
generating data for the relay, after acquiring rq->lock.

The following patch used a timer (to be scheduled at next jiffy), for
delegating the wakeup to another context.
	commit 7c9cb38
	Author: Tom Zanussi <[email protected]>
	Date:   Wed May 9 02:34:01 2007 -0700

	relay: use plain timer instead of delayed work

	relay doesn't need to use schedule_delayed_work() for waking readers
	when a simple timer will do.

Scheduling a plain timer, at next jiffies boundary, to do the wakeup
causes a significant wakeup latency for the Userspace client, which makes
relay less suitable for the high-frequency low-payload use cases where the
data gets generated at a very high rate, like multiple sub buffers getting
filled within a milli second.  Moreover the timer is re-scheduled on every
newly produced sub buffer so the timer keeps getting pushed out if sub
buffers are filled in a very quick succession (less than a jiffy gap
between filling of 2 sub buffers).  As a result relay runs out of sub
buffers to store the new data.

By using irq_work it is ensured that wakeup of userspace client, blocked
in the poll call, is done at earliest (through self IPI or next timer
tick) enabling it to always consume the data in time.  Also this makes
relay consistent with printk & ring buffers (trace), as they too use
irq_work for deferred wake up of readers.

[[email protected]: select CONFIG_IRQ_WORK]
 Link: http://lkml.kernel.org/r/[email protected]
[[email protected]: coding-style fixes]
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Peter Zijlstra <[email protected]>
Signed-off-by: Akash Goel <[email protected]>
Cc: Tom Zanussi <[email protected]>
Cc: Chris Wilson <[email protected]>
Cc: Tvrtko Ursulin <[email protected]>
Signed-off-by: Arnd Bergmann <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
Signed-off-by: Linus Torvalds <[email protected]>
  • Loading branch information
Peter Zijlstra authored and torvalds committed Oct 11, 2016
1 parent ab47deb commit 26b5679
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 11 deletions.
3 changes: 2 additions & 1 deletion include/linux/relay.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include <linux/timer.h>
#include <linux/wait.h>
#include <linux/list.h>
#include <linux/irq_work.h>
#include <linux/bug.h>
#include <linux/fs.h>
#include <linux/poll.h>
Expand All @@ -38,7 +39,7 @@ struct rchan_buf
size_t subbufs_consumed; /* count of sub-buffers consumed */
struct rchan *chan; /* associated channel */
wait_queue_head_t read_wait; /* reader wait queue */
struct timer_list timer; /* reader wake-up timer */
struct irq_work wakeup_work; /* reader wakeup */
struct dentry *dentry; /* channel file dentry */
struct kref kref; /* channel buffer refcount */
struct page **page_array; /* array of current buffer pages */
Expand Down
1 change: 1 addition & 0 deletions init/Kconfig
Original file line number Diff line number Diff line change
Expand Up @@ -1288,6 +1288,7 @@ config SYSFS_DEPRECATED_V2

config RELAY
bool "Kernel->user space relay support (formerly relayfs)"
select IRQ_WORK
help
This option enables support for relay interface support in
certain file systems (such as debugfs).
Expand Down
24 changes: 14 additions & 10 deletions kernel/relay.c
Original file line number Diff line number Diff line change
Expand Up @@ -328,13 +328,15 @@ static struct rchan_callbacks default_channel_callbacks = {

/**
* wakeup_readers - wake up readers waiting on a channel
* @data: contains the channel buffer
* @work: contains the channel buffer
*
* This is the timer function used to defer reader waking.
* This is the function used to defer reader waking
*/
static void wakeup_readers(unsigned long data)
static void wakeup_readers(struct irq_work *work)
{
struct rchan_buf *buf = (struct rchan_buf *)data;
struct rchan_buf *buf;

buf = container_of(work, struct rchan_buf, wakeup_work);
wake_up_interruptible(&buf->read_wait);
}

Expand All @@ -352,9 +354,10 @@ static void __relay_reset(struct rchan_buf *buf, unsigned int init)
if (init) {
init_waitqueue_head(&buf->read_wait);
kref_init(&buf->kref);
setup_timer(&buf->timer, wakeup_readers, (unsigned long)buf);
} else
del_timer_sync(&buf->timer);
init_irq_work(&buf->wakeup_work, wakeup_readers);
} else {
irq_work_sync(&buf->wakeup_work);
}

buf->subbufs_produced = 0;
buf->subbufs_consumed = 0;
Expand Down Expand Up @@ -487,7 +490,7 @@ static struct rchan_buf *relay_open_buf(struct rchan *chan, unsigned int cpu)
static void relay_close_buf(struct rchan_buf *buf)
{
buf->finalized = 1;
del_timer_sync(&buf->timer);
irq_work_sync(&buf->wakeup_work);
buf->chan->cb->remove_buf_file(buf->dentry);
kref_put(&buf->kref, relay_remove_buf);
}
Expand Down Expand Up @@ -754,14 +757,15 @@ size_t relay_switch_subbuf(struct rchan_buf *buf, size_t length)
buf->early_bytes += buf->chan->subbuf_size -
buf->padding[old_subbuf];
smp_mb();
if (waitqueue_active(&buf->read_wait))
if (waitqueue_active(&buf->read_wait)) {
/*
* Calling wake_up_interruptible() from here
* will deadlock if we happen to be logging
* from the scheduler (trying to re-grab
* rq->lock), so defer it.
*/
mod_timer(&buf->timer, jiffies + 1);
irq_work_queue(&buf->wakeup_work);
}
}

old = buf->data;
Expand Down

0 comments on commit 26b5679

Please sign in to comment.