Re: [PATCH] locking/percpu_rwsem: Rewrite to not use rwsem

Peter Zijlstra
2019-10-31T06:11:28Z

On Wed, Oct 30, 2019 at 06:52:31PM +0100, Peter Zijlstra wrote:
> +enum wake_state {
> + unknown = -1,
> + writer = 0,
> + reader = 1,
> +};
> +
> +/*
> + * percpu_rwsem_wake_function -- provide FIFO fair reader/writer wakeups
> + *
> + * As per percpu_rwsem_wait() all waiters are queued exclusive (tail/FIFO)
> + * without autoremove to preserve FIFO order.
> + */
> +static int percpu_rwsem_wake_function(struct wait_queue_entry *wq_entry,
> + unsigned int mode, int wake_flags,
> + void *key)
> +{
> + enum wake_state state = (wq_entry->flags & WQ_FLAG_CUSTOM) ? reader : writer;
> + enum wake_state *statep = key;
> +
> + if (*statep != unknown && (*statep == writer || state == writer))
> + return 1; /* stop; woken 1 writer or exhausted readers */
> +
> + if (default_wake_function(wq_entry, mode, wake_flags, NULL))
> + *statep = state;
> +
> + return 0; /* continue waking */
> +}
> +
> +#define percpu_rwsem_wait(sem, reader, cond) \
> +do { \
> + DEFINE_WAIT_FUNC(wq_entry, percpu_rwsem_wake_function); \
> + \
> + if (reader) \
> + wq_entry.flags |= WQ_FLAG_CUSTOM; \
> + \
> + add_wait_queue_exclusive(&(sem)->waiters, &wq_entry); \
> + for (;;) { \
> + set_current_state(TASK_UNINTERRUPTIBLE); \
> + if (cond) \
> + break; \
> + schedule(); \
> + } \
> + __set_current_state(TASK_RUNNING); \
> + remove_wait_queue(&(sem)->waiters, &wq_entry); \
> +} while (0)
> +
> +#define percpu_rwsem_wake(sem) \
> +do { \
> + enum wake_state ____state = unknown; \
> + __wake_up(&(sem)->waiters, TASK_NORMAL, 1, &____state); \
> +} while (0)

That isn't quite right, when it has readers and a pending writer, things
go sideways.

I'm going to have to poke a little more at this.