Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1535893

Re: [PATCH v2 2/3] locking/percpu-rwsem: Rework writer block/wake to not use wait-queues

From Peter Zijlstra <peterz@infradead.org>
Newsgroups linux.kernel
Subject Re: [PATCH v2 2/3] locking/percpu-rwsem: Rework writer block/wake to not use wait-queues
Date 2016-12-05 09:40 +0100
Message-ID <sKWYx-2e2-17@gated-at.bofh.it> (permalink)
References <sEWye-2tW-9@gated-at.bofh.it> <sEWye-2tW-29@gated-at.bofh.it> <sK85H-3yk-1@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On Fri, Dec 02, 2016 at 06:18:39PM -0800, Davidlohr Bueso wrote:
> @@ -102,8 +103,13 @@ void __percpu_up_read(struct percpu_rw_semaphore *sem)
> 	 */
> 	__this_cpu_dec(*sem->read_count);
> 
> +	rcu_read_lock();
> +	writer = rcu_dereference(sem->writer);

Don't think this is correct, I think Oleg suggested using
task_rcu_dereference(), which is a giant pile of magic.

The problem is that task_struct isn't RCU protected as such.

> +
> 	/* Prod writer to recheck readers_active */
> -	wake_up(&sem->writer);
> +	if (writer)
> +		wake_up_process(writer);
> +	rcu_read_unlock();
> }
> EXPORT_SYMBOL_GPL(__percpu_up_read);
> 
> @@ -159,8 +165,18 @@ void percpu_down_write(struct percpu_rw_semaphore *sem)
> 	 * will wait for them.
> 	 */
> 
> -	/* Wait for all now active readers to complete. */
> -	wait_event(sem->writer, readers_active_check(sem));
> +	WRITE_ONCE(sem->writer, current);

So this one matches rcu_dereference(), which is weird, because you now
have unmatched barriers.

> +	for (;;) {
> +		set_current_state(TASK_UNINTERRUPTIBLE);
> +
> +		if (readers_active_check(sem))
> +			break;
> +
> +		schedule();
> +	}
> +
> +	rcu_assign_pointer(sem->writer, NULL);

And this one does not, and the value being NULL this actually reverts to
WRITE_ONCE().

> +	__set_current_state(TASK_RUNNING);
> }
> EXPORT_SYMBOL_GPL(percpu_down_write);

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH v2 2/3] locking/percpu-rwsem: Rework writer block/wake to not  use wait-queues Davidlohr Bueso <dave@stgolabs.net> - 2016-12-03 03:20 +0100
  Re: [PATCH v2 2/3] locking/percpu-rwsem: Rework writer block/wake to  not use wait-queues Peter Zijlstra <peterz@infradead.org> - 2016-12-05 09:40 +0100
    Re: [PATCH v2 2/3] locking/percpu-rwsem: Rework writer block/wake         to not use wait-queues Oleg Nesterov <oleg@redhat.com> - 2016-12-05 12:30 +0100
      Re: [PATCH v2 2/3] locking/percpu-rwsem: Rework writer block/wake         to not use wait-queues Oleg Nesterov <oleg@redhat.com> - 2016-12-05 12:50 +0100
      Re: [PATCH v2 2/3] locking/percpu-rwsem: Rework writer block/wake to  not use wait-queues Davidlohr Bueso <dave@stgolabs.net> - 2016-12-05 18:40 +0100
    Re: [PATCH v2 2/3] locking/percpu-rwsem: Rework writer block/wake         to not use wait-queues Oleg Nesterov <oleg@redhat.com> - 2016-12-05 18:20 +0100
  Re: [PATCH v2 2/3] locking/percpu-rwsem: Rework writer block/wake         to not use wait-queues Oleg Nesterov <oleg@redhat.com> - 2016-12-05 18:20 +0100

csiph-web