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

Groups > linux.kernel > #1397257

Re: [PATCH 2/4] locking/rwsem: Drop superfluous waiter refcount

Show all headers | View raw

On Mon, 09 May 2016, Peter Zijlstra wrote:

>> >So I think you're wrong here; imagine this:
>> >
>> >
>> >	rwsem_down_read_failed()			rwsem_wake()
>> >	  get_task_struct();
>> >	  raw_spin_lock_irq(&wait_lock);
>> >	  list_add_tail(&waiter.list, &wait_list);
>> >	  raw_spin_unlock_irq(&wait_lock);
>> >							  raw_spin_lock_irqsave(&wait_lock)
>> >							  __rwsem_do_wake()
>> >	  while (true) {
>> >	    set_task_state(tsk, TASK_UNINTERRUPTIBLE);
>> >							    waiter->task = NULL
>> >	    if (!waiter.task) // true
>> >	      break;
>> >
>> >	  __set_task_state(tsk, TASK_RUNNING);
>> >
>> >	do_exit();
>> >							    wake_up_process(tsk); /* BOOM */
>>
>> I may be missing something, but rwsem_down_read_failed() will not return until
>> after the wakeup is done by the rwsem_wake() thread.
>
>The above never gets to schedule(), and even if it did, a spurious
>wakeup could've happened, no?

Ah indeed, you are most certainly correct. For some reason  I was always
considering schedule() in the picture. Hmm I'll have to think about this
some more, but given the small chance of a waiter actually seeing the nil
task at the first iteration I'm wondering if we could just invert the code
and call schedule() before the task check. Saving the refcounts will serve
_all_ reader waiters otoh, but this would obviously need numbers...

Thanks,
Davidlohr

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

Thread

[PATCH -tip 0/4] locking/rwsem (xadd): Reader waiter optimizations Davidlohr Bueso <dave@stgolabs.net> - 2016-05-09 07:00 +0200
  [PATCH 4/4] locking/rwsem: Rework zeroing reader waiter->task Davidlohr Bueso <dave@stgolabs.net> - 2016-05-09 07:10 +0200
    Re: [PATCH 4/4] locking/rwsem: Rework zeroing reader waiter->task Peter Zijlstra <peterz@infradead.org> - 2016-05-09 10:00 +0200
  [PATCH 3/4] locking/rwsem: Enable lockless waiter wakeup(s) Davidlohr Bueso <dave@stgolabs.net> - 2016-05-09 07:10 +0200
    Re: [PATCH 3/4] locking/rwsem: Enable lockless waiter wakeup(s) Peter Zijlstra <peterz@infradead.org> - 2016-05-09 09:50 +0200
  [PATCH 2/4] locking/rwsem: Drop superfluous waiter refcount Davidlohr Bueso <dave@stgolabs.net> - 2016-05-09 07:10 +0200
    Re: [PATCH 2/4] locking/rwsem: Drop superfluous waiter refcount Peter Zijlstra <peterz@infradead.org> - 2016-05-09 09:40 +0200
      Re: [PATCH 2/4] locking/rwsem: Drop superfluous waiter refcount Davidlohr Bueso <dave@stgolabs.net> - 2016-05-09 18:00 +0200
        Re: [PATCH 2/4] locking/rwsem: Drop superfluous waiter refcount Peter Zijlstra <peterz@infradead.org> - 2016-05-09 18:20 +0200
          Re: [PATCH 2/4] locking/rwsem: Drop superfluous waiter refcount Davidlohr Bueso <dave@stgolabs.net> - 2016-05-09 21:00 +0200
            Re: [PATCH 2/4] locking/rwsem: Drop superfluous waiter refcount Peter Zijlstra <peterz@infradead.org> - 2016-05-09 21:10 +0200
  [PATCH 1/4] locking/rwsem: Avoid stale ->count for rwsem_down_write_failed() Davidlohr Bueso <dave@stgolabs.net> - 2016-05-09 07:10 +0200
    Re: [PATCH 1/4] locking/rwsem: Avoid stale ->count for  rwsem_down_write_failed() Peter Hurley <peter@hurleysoftware.com> - 2016-05-09 07:40 +0200
      Re: [PATCH 1/4] locking/rwsem: Avoid stale ->count for  rwsem_down_write_failed() Peter Zijlstra <peterz@infradead.org> - 2016-05-09 09:20 +0200
      Re: [PATCH 1/4] locking/rwsem: Avoid stale ->count for  rwsem_down_write_failed() Davidlohr Bueso <dave@stgolabs.net> - 2016-05-09 16:40 +0200
  Re: [PATCH -tip 0/4] locking/rwsem (xadd): Reader waiter optimizations Waiman Long <waiman.long@hpe.com> - 2016-05-10 03:40 +0200

csiph-web