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


Groups > linux.kernel > #1398782

Re: [PATCH 03/11] locking, rwsem: introduce basis for down_write_killable

From Michal Hocko <mhocko@kernel.org>
Newsgroups linux.kernel
Subject Re: [PATCH 03/11] locking, rwsem: introduce basis for down_write_killable
Date 2016-05-11 10:30 +0200
Message-ID <rxxGP-4ls-53@gated-at.bofh.it> (permalink)
References (1 earlier) <rj57H-3qc-1@gated-at.bofh.it> <rxdoK-P9-21@gated-at.bofh.it> <rxeut-1FZ-1@gated-at.bofh.it> <rxf7b-2mv-15@gated-at.bofh.it> <rxwKJ-3ri-5@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On Wed 11-05-16 09:23:57, Michal Hocko wrote:
> On Tue 10-05-16 14:38:06, Peter Zijlstra wrote:
[...]
> > However, at the time I was thinking that if we have:
> > 
> > 	reader (owner)
> > 	writer (pending)
> > 	reader (blocked on writer)
> > 
> > and writer would get cancelled, the up_read() would do a wakeup and kick
> > the blocked reader.
> > 
> > But yes, immediately kicking further pending waiters might be better.
> 
> OK, that makes sense. We shouldn't be waiting for the first reader to
> do up_read.

I am still trying to wrap my head around the wake up logic (I managed to
forget all the juicy details). I still do not see the correctness
issue with the current code so the following should be "just an
optimization". But as I've said I might be missing something here.

Does the following look correct/reasonable? This is absolutely untested
and more for a discussion:
---
diff --git a/kernel/locking/rwsem-xadd.c b/kernel/locking/rwsem-xadd.c
index df4dcb883b50..726620c97366 100644
--- a/kernel/locking/rwsem-xadd.c
+++ b/kernel/locking/rwsem-xadd.c
@@ -516,7 +516,27 @@ EXPORT_SYMBOL(rwsem_down_write_failed);
 __visible struct rw_semaphore * __sched
 rwsem_down_write_failed_killable(struct rw_semaphore *sem)
 {
-	return __rwsem_down_write_failed_common(sem, TASK_KILLABLE);
+	struct rw_semaphore * ret;
+
+	ret = __rwsem_down_write_failed_common(sem, TASK_KILLABLE);
+	if (IS_ERR(ret)) {
+		long count;
+
+		raw_spin_lock_irq(&sem->wait_lock);
+		count = READ_ONCE(sem->count);
+
+		/*
+		 * If there were already threads queued before us and there are
+		 * no active writers, the lock must be read owned; so we try to
+		 * wake any read locks that were queued ahead of us so they
+		 * do not have to wait for up_read to wake up.
+		 */
+		if (count > RWSEM_WAITING_BIAS)
+			sem = __rwsem_do_wake(sem, RWSEM_WAKE_READERS);
+		raw_spin_unlock_irq(&sem->wait_lock);
+	}
+
+	return ret;
 }
 EXPORT_SYMBOL(rwsem_down_write_failed_killable);
 
-- 
Michal Hocko
SUSE Labs

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


Thread

Re: [PATCH 03/11] locking, rwsem: introduce basis for  down_write_killable Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> - 2016-05-10 12:50 +0200
  Re: [PATCH 03/11] locking, rwsem: introduce basis for  down_write_killable Michal Hocko <mhocko@kernel.org> - 2016-05-10 14:00 +0200
    Re: [PATCH 03/11] locking, rwsem: introduce basis for  down_write_killable Peter Zijlstra <peterz@infradead.org> - 2016-05-10 14:40 +0200
      Re: [PATCH 03/11] locking, rwsem: introduce basis for down_write_killable Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> - 2016-05-10 16:00 +0200
      Re: [PATCH 03/11] locking, rwsem: introduce basis for  down_write_killable Michal Hocko <mhocko@kernel.org> - 2016-05-11 09:30 +0200
        Re: [PATCH 03/11] locking, rwsem: introduce basis for  down_write_killable Michal Hocko <mhocko@kernel.org> - 2016-05-11 10:30 +0200
          Re: [PATCH 03/11] locking, rwsem: introduce basis for  down_write_killable Peter Zijlstra <peterz@infradead.org> - 2016-05-11 10:50 +0200
            Re: [PATCH 03/11] locking, rwsem: introduce basis for  down_write_killable Michal Hocko <mhocko@kernel.org> - 2016-05-11 11:10 +0200
              Re: [PATCH 03/11] locking, rwsem: introduce basis for  down_write_killable Peter Zijlstra <peterz@infradead.org> - 2016-05-11 11:20 +0200
                Re: [PATCH 03/11] locking, rwsem: introduce basis for  down_write_killable Michal Hocko <mhocko@kernel.org> - 2016-05-11 11:40 +0200
                Re: [PATCH 03/11] locking, rwsem: introduce basis for  down_write_killable Peter Zijlstra <peterz@infradead.org> - 2016-05-11 11:50 +0200
                Re: [PATCH 03/11] locking, rwsem: introduce basis for  down_write_killable Michal Hocko <mhocko@kernel.org> - 2016-05-11 16:00 +0200
                Re: [PATCH 03/11] locking, rwsem: introduce basis for  down_write_killable Michal Hocko <mhocko@kernel.org> - 2016-05-11 20:10 +0200
                [PATCH] locking, rwsem: Fix down_write_killable() Peter Zijlstra <peterz@infradead.org> - 2016-05-12 14:00 +0200
        Re: [PATCH 03/11] locking, rwsem: introduce basis for  down_write_killable Peter Zijlstra <peterz@infradead.org> - 2016-05-11 10:40 +0200
          Re: [PATCH 03/11] locking, rwsem: introduce basis for  down_write_killable Michal Hocko <mhocko@kernel.org> - 2016-05-11 11:10 +0200

csiph-web