evdev: Make open(2) and close(3) handlers sleepable.

At the beginning of evdev there was a LOR between hardware driver's and
evdev client list locks as they were taken in different order at
driver's interrupt and evdev open()/close() handlers.

The LOR was fixed with introduction of evdev_register_mtx() function
which allowed to use a hardware driver's lock as evdev client list lock.
While this works good with PS/2 and USB, this does not work with I2C.
Unlike PS/2 and USB, I2C open()/close() handlers do unbound sleeps
while waiting for I2C bus to release and while performing IO.
This change uses epoch(9) for traversing evdev client list in interrupt
handler to avoid the LOR thus making possible to convert evdev client
list lock to sleepable sx.

While here add brief locking protocol description.

Reviewed by:	markj
Differential revision:	https://reviews.freebsd.org/D27865
This commit is contained in:
Vladimir Kondratyev 2020-04-21 13:26:58 +03:00
parent 5af73ad51b
commit d276eae674
4 changed files with 156 additions and 67 deletions

View File

@ -32,6 +32,7 @@
#include <sys/param.h>
#include <sys/bitstring.h>
#include <sys/conf.h>
#include <sys/epoch.h>
#include <sys/filio.h>
#include <sys/fcntl.h>
#include <sys/kernel.h>
@ -126,7 +127,7 @@ evdev_open(struct cdev *dev, int oflags, int devtype, struct thread *td)
knlist_init_mtx(&client->ec_selp.si_note, &client->ec_buffer_mtx);
/* Avoid race with evdev_unregister */
EVDEV_LOCK(evdev);
EVDEV_LIST_LOCK(evdev);
if (dev->si_drv1 == NULL)
ret = ENODEV;
else
@ -134,13 +135,9 @@ evdev_open(struct cdev *dev, int oflags, int devtype, struct thread *td)
if (ret != 0)
evdev_revoke_client(client);
/*
* Unlock evdev here because non-sleepable lock held
* while calling devfs_set_cdevpriv upsets WITNESS
*/
EVDEV_UNLOCK(evdev);
EVDEV_LIST_UNLOCK(evdev);
if (!ret)
if (ret == 0)
ret = devfs_set_cdevpriv(client, evdev_dtor);
if (ret != 0) {
@ -156,11 +153,13 @@ evdev_dtor(void *data)
{
struct evdev_client *client = (struct evdev_client *)data;
EVDEV_LOCK(client->ec_evdev);
EVDEV_LIST_LOCK(client->ec_evdev);
if (!client->ec_revoked)
evdev_dispose_client(client->ec_evdev, client);
EVDEV_UNLOCK(client->ec_evdev);
EVDEV_LIST_UNLOCK(client->ec_evdev);
if (client->ec_evdev->ev_lock_type != EV_LOCK_MTX)
epoch_wait_preempt(INPUT_EPOCH);
knlist_clear(&client->ec_selp.si_note, 0);
seldrain(&client->ec_selp);
knlist_destroy(&client->ec_selp.si_note);
@ -547,12 +546,12 @@ evdev_ioctl(struct cdev *dev, u_long cmd, caddr_t data, int fflag,
if (*(int *)data != 0)
return (EINVAL);
EVDEV_LOCK(evdev);
EVDEV_LIST_LOCK(evdev);
if (dev->si_drv1 != NULL && !client->ec_revoked) {
evdev_dispose_client(evdev, client);
evdev_revoke_client(client);
}
EVDEV_UNLOCK(evdev);
EVDEV_LIST_UNLOCK(evdev);
return (0);
case EVIOCSCLOCKID:
@ -717,7 +716,7 @@ void
evdev_revoke_client(struct evdev_client *client)
{
EVDEV_LOCK_ASSERT(client->ec_evdev);
EVDEV_LIST_LOCK_ASSERT(client->ec_evdev);
client->ec_revoked = true;
}

View File

@ -31,12 +31,15 @@
#include <sys/param.h>
#include <sys/bitstring.h>
#include <sys/ck.h>
#include <sys/conf.h>
#include <sys/epoch.h>
#include <sys/kdb.h>
#include <sys/kernel.h>
#include <sys/malloc.h>
#include <sys/module.h>
#include <sys/proc.h>
#include <sys/sx.h>
#include <sys/sysctl.h>
#include <sys/systm.h>
@ -295,12 +298,14 @@ evdev_register_common(struct evdev_dev *evdev)
evdev->ev_shortname, evdev->ev_name, evdev->ev_serial);
/* Initialize internal structures */
LIST_INIT(&evdev->ev_clients);
CK_SLIST_INIT(&evdev->ev_clients);
sx_init(&evdev->ev_list_lock, "evsx");
if (evdev_event_supported(evdev, EV_REP) &&
bit_test(evdev->ev_flags, EVDEV_FLAG_SOFTREPEAT)) {
/* Initialize callout */
callout_init_mtx(&evdev->ev_rep_callout, evdev->ev_lock, 0);
callout_init_mtx(&evdev->ev_rep_callout,
evdev->ev_state_lock, 0);
if (evdev->ev_rep[REP_DELAY] == 0 &&
evdev->ev_rep[REP_PERIOD] == 0) {
@ -332,6 +337,8 @@ evdev_register_common(struct evdev_dev *evdev)
evdev_sysctl_create(evdev);
bail_out:
if (ret != 0)
sx_destroy(&evdev->ev_list_lock);
return (ret);
}
@ -340,8 +347,11 @@ evdev_register(struct evdev_dev *evdev)
{
int ret;
evdev->ev_lock_type = EV_LOCK_INTERNAL;
evdev->ev_lock = &evdev->ev_mtx;
if (bit_test(evdev->ev_flags, EVDEV_FLAG_EXT_EPOCH))
evdev->ev_lock_type = EV_LOCK_EXT_EPOCH;
else
evdev->ev_lock_type = EV_LOCK_INTERNAL;
evdev->ev_state_lock = &evdev->ev_mtx;
mtx_init(&evdev->ev_mtx, "evmtx", NULL, MTX_DEF);
ret = evdev_register_common(evdev);
@ -356,7 +366,7 @@ evdev_register_mtx(struct evdev_dev *evdev, struct mtx *mtx)
{
evdev->ev_lock_type = EV_LOCK_MTX;
evdev->ev_lock = mtx;
evdev->ev_state_lock = mtx;
return (evdev_register_common(evdev));
}
@ -370,22 +380,23 @@ evdev_unregister(struct evdev_dev *evdev)
sysctl_ctx_free(&evdev->ev_sysctl_ctx);
EVDEV_LOCK(evdev);
EVDEV_LIST_LOCK(evdev);
evdev->ev_cdev->si_drv1 = NULL;
/* Wake up sleepers */
LIST_FOREACH_SAFE(client, &evdev->ev_clients, ec_link, tmp) {
CK_SLIST_FOREACH_SAFE(client, &evdev->ev_clients, ec_link, tmp) {
evdev_revoke_client(client);
evdev_dispose_client(evdev, client);
EVDEV_CLIENT_LOCKQ(client);
evdev_notify_event(client);
EVDEV_CLIENT_UNLOCKQ(client);
}
EVDEV_UNLOCK(evdev);
EVDEV_LIST_UNLOCK(evdev);
/* destroy_dev can sleep so release lock */
/* release lock to avoid deadlock with evdev_dtor */
ret = evdev_cdev_destroy(evdev);
evdev->ev_cdev = NULL;
if (ret == 0 && evdev->ev_lock_type == EV_LOCK_INTERNAL)
sx_destroy(&evdev->ev_list_lock);
if (ret == 0 && evdev->ev_lock_type != EV_LOCK_MTX)
mtx_destroy(&evdev->ev_mtx);
evdev_free_absinfo(evdev->ev_absinfo);
@ -689,7 +700,7 @@ evdev_modify_event(struct evdev_dev *evdev, uint16_t type, uint16_t code,
} else {
/* Start/stop callout for evdev repeats */
if (bit_test(evdev->ev_key_states, code) == !*value &&
!LIST_EMPTY(&evdev->ev_clients)) {
!CK_SLIST_EMPTY(&evdev->ev_clients)) {
if (*value == KEY_EVENT_DOWN)
evdev_start_repeat(evdev, code);
else
@ -817,6 +828,7 @@ static void
evdev_propagate_event(struct evdev_dev *evdev, uint16_t type, uint16_t code,
int32_t value)
{
struct epoch_tracker et;
struct evdev_client *client;
debugf(evdev, "%s pushed event %d/%d/%d",
@ -825,7 +837,14 @@ evdev_propagate_event(struct evdev_dev *evdev, uint16_t type, uint16_t code,
EVDEV_LOCK_ASSERT(evdev);
/* Propagate event through all clients */
LIST_FOREACH(client, &evdev->ev_clients, ec_link) {
if (evdev->ev_lock_type == EV_LOCK_INTERNAL)
epoch_enter_preempt(INPUT_EPOCH, &et);
KASSERT(
evdev->ev_lock_type == EV_LOCK_MTX || in_epoch(INPUT_EPOCH) != 0,
("Input epoch has not been entered\n"));
CK_SLIST_FOREACH(client, &evdev->ev_clients, ec_link) {
if (evdev->ev_grabber != NULL && evdev->ev_grabber != client)
continue;
@ -835,6 +854,8 @@ evdev_propagate_event(struct evdev_dev *evdev, uint16_t type, uint16_t code,
evdev_notify_event(client);
EVDEV_CLIENT_UNLOCKQ(client);
}
if (evdev->ev_lock_type == EV_LOCK_INTERNAL)
epoch_exit_preempt(INPUT_EPOCH, &et);
evdev->ev_event_count++;
}
@ -932,6 +953,7 @@ int
evdev_inject_event(struct evdev_dev *evdev, uint16_t type, uint16_t code,
int32_t value)
{
struct epoch_tracker et;
int ret = 0;
switch (type) {
@ -962,11 +984,16 @@ evdev_inject_event(struct evdev_dev *evdev, uint16_t type, uint16_t code,
case EV_ABS:
case EV_SW:
push:
if (evdev->ev_lock_type != EV_LOCK_INTERNAL)
if (evdev->ev_lock_type == EV_LOCK_MTX)
EVDEV_LOCK(evdev);
else if (evdev->ev_lock_type == EV_LOCK_EXT_EPOCH)
epoch_enter_preempt(INPUT_EPOCH, &et);
ret = evdev_push_event(evdev, type, code, value);
if (evdev->ev_lock_type != EV_LOCK_INTERNAL)
if (evdev->ev_lock_type == EV_LOCK_MTX)
EVDEV_UNLOCK(evdev);
else if (evdev->ev_lock_type == EV_LOCK_EXT_EPOCH)
epoch_exit_preempt(INPUT_EPOCH, &et);
break;
default:
@ -983,16 +1010,16 @@ evdev_register_client(struct evdev_dev *evdev, struct evdev_client *client)
debugf(evdev, "adding new client for device %s", evdev->ev_shortname);
EVDEV_LOCK_ASSERT(evdev);
EVDEV_LIST_LOCK_ASSERT(evdev);
if (LIST_EMPTY(&evdev->ev_clients) && evdev->ev_methods != NULL &&
if (CK_SLIST_EMPTY(&evdev->ev_clients) && evdev->ev_methods != NULL &&
evdev->ev_methods->ev_open != NULL) {
debugf(evdev, "calling ev_open() on device %s",
evdev->ev_shortname);
ret = evdev->ev_methods->ev_open(evdev);
}
if (ret == 0)
LIST_INSERT_HEAD(&evdev->ev_clients, client, ec_link);
CK_SLIST_INSERT_HEAD(&evdev->ev_clients, client, ec_link);
return (ret);
}
@ -1001,18 +1028,27 @@ evdev_dispose_client(struct evdev_dev *evdev, struct evdev_client *client)
{
debugf(evdev, "removing client for device %s", evdev->ev_shortname);
EVDEV_LOCK_ASSERT(evdev);
EVDEV_LIST_LOCK_ASSERT(evdev);
LIST_REMOVE(client, ec_link);
if (LIST_EMPTY(&evdev->ev_clients)) {
CK_SLIST_REMOVE(&evdev->ev_clients, client, evdev_client, ec_link);
if (CK_SLIST_EMPTY(&evdev->ev_clients)) {
if (evdev->ev_methods != NULL &&
evdev->ev_methods->ev_close != NULL)
(void)evdev->ev_methods->ev_close(evdev);
if (evdev_event_supported(evdev, EV_REP) &&
bit_test(evdev->ev_flags, EVDEV_FLAG_SOFTREPEAT))
bit_test(evdev->ev_flags, EVDEV_FLAG_SOFTREPEAT)) {
if (evdev->ev_lock_type != EV_LOCK_MTX)
EVDEV_LOCK(evdev);
evdev_stop_repeat(evdev);
if (evdev->ev_lock_type != EV_LOCK_MTX)
EVDEV_UNLOCK(evdev);
}
}
if (evdev->ev_lock_type != EV_LOCK_MTX)
EVDEV_LOCK(evdev);
evdev_release_client(evdev, client);
if (evdev->ev_lock_type != EV_LOCK_MTX)
EVDEV_UNLOCK(evdev);
}
int
@ -1046,10 +1082,15 @@ evdev_release_client(struct evdev_dev *evdev, struct evdev_client *client)
static void
evdev_repeat_callout(void *arg)
{
struct epoch_tracker et;
struct evdev_dev *evdev = (struct evdev_dev *)arg;
if (evdev->ev_lock_type == EV_LOCK_EXT_EPOCH)
epoch_enter_preempt(INPUT_EPOCH, &et);
evdev_send_event(evdev, EV_KEY, evdev->ev_rep_key, KEY_EVENT_REPEAT);
evdev_send_event(evdev, EV_SYN, SYN_REPORT, 1);
if (evdev->ev_lock_type == EV_LOCK_EXT_EPOCH)
epoch_exit_preempt(INPUT_EPOCH, &et);
if (evdev->ev_rep[REP_PERIOD])
callout_reset(&evdev->ev_rep_callout,

View File

@ -30,6 +30,7 @@
#define _DEV_EVDEV_EVDEV_H
#include <sys/types.h>
#include <sys/epoch.h>
#include <sys/kbio.h>
#include <dev/evdev/input.h>
#include <dev/kbd/kbdreg.h>
@ -87,6 +88,8 @@ extern int evdev_sysmouse_t_axis;
* for MT protocol type B reports */
#define EVDEV_FLAG_MT_AUTOREL 0x02 /* Autorelease MT-slots not listed in
* current MT protocol type B report */
#define EVDEV_FLAG_EXT_EPOCH 0x03 /* evdev_push_* is allways called with
* input (global) epoch entered */
#define EVDEV_FLAG_MAX 0x1F
#define EVDEV_FLAG_CNT (EVDEV_FLAG_MAX + 1)

View File

@ -31,12 +31,15 @@
#define _DEV_EVDEV_EVDEV_PRIVATE_H
#include <sys/bitstring.h>
#include <sys/ck.h>
#include <sys/epoch.h>
#include <sys/kbio.h>
#include <sys/lock.h>
#include <sys/malloc.h>
#include <sys/mutex.h>
#include <sys/queue.h>
#include <sys/selinfo.h>
#include <sys/sx.h>
#include <sys/sysctl.h>
#include <dev/evdev/evdev.h>
@ -75,10 +78,33 @@ enum evdev_clock_id
EV_CLOCK_BOOTTIME /* monotonic, suspend-awared */
};
/*
* Locking.
*
* Internal evdev structures are protected with next locks:
* State lock (s) - Internal state. The data it protects is changed
* by incoming evdev events and some ioctls.
* Client list epoch (l) - Read access to client list.
* Client list lock (l) - Write access to client list.
* Client queue locks (q) - One lock per client to serialize access to data
* available through character device node.
*
* Depending on evdev_register_() suffix evdev can run in following modes:
* 1. Internal epoch. evdev_register(). All locks are internal.
* 2. External epoch. Evdev expects to be run under input epoch entered by
* parent driver. The mode is enabled with EVDEV_FLAG_EXT_EPOCH flag.
* 3. External mutex. evdev_register_mtx(). Evdev uses mutex provided by parent
* driver as both "State lock" and "Client list lock". This mode is
* deprecated as it causes ev_open and ev_close handlers to be called with
* parent driver mutex taken.
*/
#define INPUT_EPOCH global_epoch_preempt
enum evdev_lock_type
{
EV_LOCK_INTERNAL = 0, /* Internal evdev mutex */
EV_LOCK_INTERNAL = 0, /* Internal epoch */
EV_LOCK_MTX, /* Driver`s mutex */
EV_LOCK_EXT_EPOCH, /* External epoch */
};
struct evdev_dev
@ -89,10 +115,11 @@ struct evdev_dev
struct cdev * ev_cdev;
int ev_unit;
enum evdev_lock_type ev_lock_type;
struct mtx * ev_lock;
struct mtx ev_mtx;
struct mtx * ev_state_lock; /* State lock */
struct mtx ev_mtx; /* Internal state lock */
struct sx ev_list_lock; /* Client list lock */
struct input_id ev_id;
struct evdev_client * ev_grabber;
struct evdev_client * ev_grabber; /* (s) */
size_t ev_report_size;
/* Supported features: */
@ -105,31 +132,31 @@ struct evdev_dev
bitstr_t bit_decl(ev_led_flags, LED_CNT);
bitstr_t bit_decl(ev_snd_flags, SND_CNT);
bitstr_t bit_decl(ev_sw_flags, SW_CNT);
struct input_absinfo * ev_absinfo;
struct input_absinfo * ev_absinfo; /* (s) */
bitstr_t bit_decl(ev_flags, EVDEV_FLAG_CNT);
/* Repeat parameters & callout: */
int ev_rep[REP_CNT];
struct callout ev_rep_callout;
uint16_t ev_rep_key;
int ev_rep[REP_CNT]; /* (s) */
struct callout ev_rep_callout; /* (s) */
uint16_t ev_rep_key; /* (s) */
/* State: */
bitstr_t bit_decl(ev_key_states, KEY_CNT);
bitstr_t bit_decl(ev_led_states, LED_CNT);
bitstr_t bit_decl(ev_snd_states, SND_CNT);
bitstr_t bit_decl(ev_sw_states, SW_CNT);
bool ev_report_opened;
bitstr_t bit_decl(ev_key_states, KEY_CNT); /* (s) */
bitstr_t bit_decl(ev_led_states, LED_CNT); /* (s) */
bitstr_t bit_decl(ev_snd_states, SND_CNT); /* (s) */
bitstr_t bit_decl(ev_sw_states, SW_CNT); /* (s) */
bool ev_report_opened; /* (s) */
/* KDB state: */
bool ev_kdb_active;
bitstr_t bit_decl(ev_kdb_led_states, LED_CNT);
/* Multitouch protocol type B state: */
struct evdev_mt * ev_mt;
struct evdev_mt * ev_mt; /* (s) */
/* Counters: */
uint64_t ev_event_count;
uint64_t ev_report_count;
uint64_t ev_event_count; /* (s) */
uint64_t ev_report_count; /* (s) */
/* Parent driver callbacks: */
const struct evdev_methods * ev_methods;
@ -139,47 +166,66 @@ struct evdev_dev
struct sysctl_ctx_list ev_sysctl_ctx;
LIST_ENTRY(evdev_dev) ev_link;
LIST_HEAD(, evdev_client) ev_clients;
CK_SLIST_HEAD(, evdev_client) ev_clients; /* (l) */
};
#define SYSTEM_CONSOLE_LOCK &Giant
#define EVDEV_LOCK(evdev) mtx_lock((evdev)->ev_lock)
#define EVDEV_UNLOCK(evdev) mtx_unlock((evdev)->ev_lock)
#define EVDEV_LOCK(evdev) mtx_lock((evdev)->ev_state_lock)
#define EVDEV_UNLOCK(evdev) mtx_unlock((evdev)->ev_state_lock)
#define EVDEV_LOCK_ASSERT(evdev) do { \
if ((evdev)->ev_lock != SYSTEM_CONSOLE_LOCK) \
mtx_assert((evdev)->ev_lock, MA_OWNED); \
if ((evdev)->ev_state_lock != SYSTEM_CONSOLE_LOCK) \
mtx_assert((evdev)->ev_state_lock, MA_OWNED); \
} while (0)
#define EVDEV_ENTER(evdev) do { \
if ((evdev)->ev_lock_type == EV_LOCK_INTERNAL) \
if ((evdev)->ev_lock_type != EV_LOCK_MTX) \
EVDEV_LOCK(evdev); \
else \
EVDEV_LOCK_ASSERT(evdev); \
} while (0)
#define EVDEV_EXIT(evdev) do { \
if ((evdev)->ev_lock_type == EV_LOCK_INTERNAL) \
if ((evdev)->ev_lock_type != EV_LOCK_MTX) \
EVDEV_UNLOCK(evdev); \
} while (0)
#define EVDEV_LIST_LOCK(evdev) do { \
if ((evdev)->ev_lock_type == EV_LOCK_MTX) \
EVDEV_LOCK(evdev); \
else \
sx_xlock(&(evdev)->ev_list_lock); \
} while (0)
#define EVDEV_LIST_UNLOCK(evdev) do { \
if ((evdev)->ev_lock_type == EV_LOCK_MTX) \
EVDEV_UNLOCK(evdev); \
else \
sx_unlock(&(evdev)->ev_list_lock); \
} while (0)
#define EVDEV_LIST_LOCK_ASSERT(evdev) do { \
if ((evdev)->ev_lock_type == EV_LOCK_MTX) \
EVDEV_LOCK_ASSERT(evdev); \
else \
sx_assert(&(evdev)->ev_list_lock, MA_OWNED); \
} while (0)
struct evdev_client
{
struct evdev_dev * ec_evdev;
struct mtx ec_buffer_mtx;
struct mtx ec_buffer_mtx; /* Client queue lock */
size_t ec_buffer_size;
size_t ec_buffer_head;
size_t ec_buffer_tail;
size_t ec_buffer_ready;
size_t ec_buffer_head; /* (q) */
size_t ec_buffer_tail; /* (q) */
size_t ec_buffer_ready; /* (q) */
enum evdev_clock_id ec_clock_id;
struct selinfo ec_selp;
struct selinfo ec_selp; /* (q) */
struct sigio * ec_sigio;
bool ec_async;
bool ec_revoked;
bool ec_blocked;
bool ec_selected;
bool ec_async; /* (q) */
bool ec_revoked; /* (l) */
bool ec_blocked; /* (q) */
bool ec_selected; /* (q) */
LIST_ENTRY(evdev_client) ec_link;
CK_SLIST_ENTRY(evdev_client) ec_link; /* (l) */
struct input_event ec_buffer[];
struct input_event ec_buffer[]; /* (q) */
};
#define EVDEV_CLIENT_LOCKQ(client) mtx_lock(&(client)->ec_buffer_mtx)