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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20191031061118.GH5671@hirez.programming.kicks-ass.net>
Date:   Thu, 31 Oct 2019 07:11:18 +0100
From:   Peter Zijlstra <peterz@...radead.org>
To:     Oleg Nesterov <oleg@...hat.com>
Cc:     Will Deacon <will.deacon@...nel.org>,
        Ingo Molnar <mingo@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        linux-kernel@...r.kernel.org, bigeasy@...utronix.de,
        juri.lelli@...hat.com, williams@...hat.com, bristot@...hat.com,
        longman@...hat.com, dave@...olabs.net, jack@...e.com
Subject: Re: [PATCH] locking/percpu_rwsem: Rewrite to not use rwsem

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.