From: Hugo Lefeuvre <hle@owl.eu.com>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: "Greg Hartman" <ghartman@google.com>,
"Alistair Strachan" <astrachan@google.com>,
"Arve Hjønnevåg" <arve@android.com>,
"Todd Kjos" <tkjos@android.com>,
"Martijn Coenen" <maco@android.com>,
"Joel Fernandes" <joel@joelfernandes.org>,
"Christian Brauner" <christian@brauner.io>,
devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org,
"Hugo Lefeuvre" <hle@owl.eu.com>
Subject: [PATCH] staging/android: use multiple futex wait queues
Date: Thu, 14 Feb 2019 18:34:59 +0100 [thread overview]
Message-ID: <20190214173459.GA10737@behemoth.owl.eu.com.local> (raw)
Use multiple per-offset wait queues instead of one big wait queue per
region.
Signed-off-by: Hugo Lefeuvre <hle@owl.eu.com>
---
This patch is based on the simplify handle_vsoc_cond_wait patchset,
currently under review: https://lkml.org/lkml/2019/2/7/870
---
drivers/staging/android/TODO | 4 ---
drivers/staging/android/vsoc.c | 56 ++++++++++++++++++++++++++--------
2 files changed, 44 insertions(+), 16 deletions(-)
diff --git a/drivers/staging/android/TODO b/drivers/staging/android/TODO
index fbf015cc6d62..cd2af06341dc 100644
--- a/drivers/staging/android/TODO
+++ b/drivers/staging/android/TODO
@@ -12,10 +12,6 @@ ion/
- Better test framework (integration with VGEM was suggested)
vsoc.c, uapi/vsoc_shm.h
- - The current driver uses the same wait queue for all of the futexes in a
- region. This will cause false wakeups in regions with a large number of
- waiting threads. We should eventually use multiple queues and select the
- queue based on the region.
- Add debugfs support for examining the permissions of regions.
- Remove VSOC_WAIT_FOR_INCOMING_INTERRUPT ioctl. This functionality has been
superseded by the futex and is there for legacy reasons.
diff --git a/drivers/staging/android/vsoc.c b/drivers/staging/android/vsoc.c
index f2bb18158e5b..55b0ee03e7b8 100644
--- a/drivers/staging/android/vsoc.c
+++ b/drivers/staging/android/vsoc.c
@@ -31,6 +31,7 @@
#include <linux/interrupt.h>
#include <linux/cdev.h>
#include <linux/file.h>
+#include <linux/list.h>
#include "uapi/vsoc_shm.h"
#define VSOC_DEV_NAME "vsoc"
@@ -62,11 +63,18 @@ static const int MAX_REGISTER_BAR_LEN = 0x100;
*/
static const int SHARED_MEMORY_BAR = 2;
+struct vsoc_futex_wait_queue_t {
+ struct list_head list;
+ u32 offset;
+ wait_queue_head_t queue;
+};
+
struct vsoc_region_data {
char name[VSOC_DEVICE_NAME_SZ + 1];
wait_queue_head_t interrupt_wait_queue;
- /* TODO(b/73664181): Use multiple futex wait queues */
- wait_queue_head_t futex_wait_queue;
+ /* Per-offset futex wait queue. */
+ struct list_head futex_wait_queue_list;
+ spinlock_t futex_wait_queue_lock;
/* Flag indicating that an interrupt has been signalled by the host. */
atomic_t *incoming_signalled;
/* Flag indicating the guest has signalled the host. */
@@ -402,6 +410,7 @@ static int handle_vsoc_cond_wait(struct file *filp, struct vsoc_cond_wait *arg)
struct vsoc_region_data *data = vsoc_dev.regions_data + region_number;
int ret = 0;
struct vsoc_device_region *region_p = vsoc_region_from_filep(filp);
+ struct vsoc_futex_wait_queue_t *it, *wait_queue = NULL;
atomic_t *address = NULL;
ktime_t wake_time;
@@ -415,10 +424,27 @@ static int handle_vsoc_cond_wait(struct file *filp, struct vsoc_cond_wait *arg)
address = shm_off_to_virtual_addr(region_p->region_begin_offset +
arg->offset);
+ /* Find wait queue corresponding to offset or create it */
+ spin_lock(&data->futex_wait_queue_lock);
+ list_for_each_entry(it, &data->futex_wait_queue_list, list) {
+ if (wait_queue->offset == arg->offset) {
+ wait_queue = it;
+ break;
+ }
+ }
+
+ if (!wait_queue) {
+ wait_queue = kmalloc(sizeof(*wait_queue), GFP_KERNEL);
+ wait_queue->offset = arg->offset;
+ init_waitqueue_head(&wait_queue->queue);
+ list_add(&wait_queue->list, &data->futex_wait_queue_list);
+ }
+ spin_unlock(&data->futex_wait_queue_lock);
+
/* Ensure that the type of wait is valid */
switch (arg->wait_type) {
case VSOC_WAIT_IF_EQUAL:
- ret = wait_event_freezable(data->futex_wait_queue,
+ ret = wait_event_freezable(wait_queue->queue,
arg->wakes++ &&
atomic_read(address) != arg->value);
break;
@@ -426,7 +452,7 @@ static int handle_vsoc_cond_wait(struct file *filp, struct vsoc_cond_wait *arg)
if (arg->wake_time_nsec >= NSEC_PER_SEC)
return -EINVAL;
wake_time = ktime_set(arg->wake_time_sec, arg->wake_time_nsec);
- ret = wait_event_freezable_hrtimeout(data->futex_wait_queue,
+ ret = wait_event_freezable_hrtimeout(wait_queue->queue,
arg->wakes++ &&
atomic_read(address) != arg->value,
wake_time);
@@ -463,6 +489,7 @@ static int do_vsoc_cond_wake(struct file *filp, uint32_t offset)
struct vsoc_device_region *region_p = vsoc_region_from_filep(filp);
u32 region_number = iminor(file_inode(filp));
struct vsoc_region_data *data = vsoc_dev.regions_data + region_number;
+ struct vsoc_futex_wait_queue_t *wait_queue;
/* Ensure that the offset is aligned */
if (offset & (sizeof(uint32_t) - 1))
return -EADDRNOTAVAIL;
@@ -470,13 +497,17 @@ static int do_vsoc_cond_wake(struct file *filp, uint32_t offset)
if (((uint64_t)offset) + region_p->region_begin_offset +
sizeof(uint32_t) > region_p->region_end_offset)
return -E2BIG;
- /*
- * TODO(b/73664181): Use multiple futex wait queues.
- * We need to wake every sleeper when the condition changes. Typically
- * only a single thread will be waiting on the condition, but there
- * are exceptions. The worst case is about 10 threads.
- */
- wake_up_interruptible_all(&data->futex_wait_queue);
+ /* Only wake up tasks waiting for given offset */
+ spin_lock(&data->futex_wait_queue_lock);
+ list_for_each_entry(wait_queue, &data->futex_wait_queue_list, list) {
+ if (wait_queue->offset == offset) {
+ wake_up_interruptible_all(&wait_queue->queue);
+ list_del(&wait_queue->list);
+ kfree(wait_queue);
+ break;
+ }
+ }
+ spin_unlock(&data->futex_wait_queue_lock);
return 0;
}
@@ -866,7 +897,8 @@ static int vsoc_probe_device(struct pci_dev *pdev,
i, vsoc_dev.regions_data[i].name);
init_waitqueue_head
(&vsoc_dev.regions_data[i].interrupt_wait_queue);
- init_waitqueue_head(&vsoc_dev.regions_data[i].futex_wait_queue);
+ spin_lock_init(&vsoc_dev.regions_data[i].futex_wait_queue_lock);
+ INIT_LIST_HEAD(&vsoc_dev.regions_data[i].futex_wait_queue_list);
vsoc_dev.regions_data[i].incoming_signalled =
shm_off_to_virtual_addr(region->region_begin_offset) +
h_to_g_signal_table->interrupt_signalled_offset;
--
2.20.1
next reply other threads:[~2019-02-14 17:35 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-02-14 17:34 Hugo Lefeuvre [this message]
2019-02-14 17:50 ` [PATCH] staging/android: use multiple futex wait queues Greg Kroah-Hartman
2019-02-14 21:28 ` Hugo Lefeuvre
2019-02-16 20:46 ` Hugo Lefeuvre
2019-02-15 7:06 ` Dan Carpenter
2019-02-15 7:54 ` Hugo Lefeuvre
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190214173459.GA10737@behemoth.owl.eu.com.local \
--to=hle@owl.eu.com \
--cc=arve@android.com \
--cc=astrachan@google.com \
--cc=christian@brauner.io \
--cc=devel@driverdev.osuosl.org \
--cc=ghartman@google.com \
--cc=gregkh@linuxfoundation.org \
--cc=joel@joelfernandes.org \
--cc=linux-kernel@vger.kernel.org \
--cc=maco@android.com \
--cc=tkjos@android.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox