Skip to contentSkip to conversation

killable rwsems for v4.7

GIT PULL Ingo Molnar 5 participants 6 messages May 16, 2016 – May 17, 2016

Subsystems (8)
lore
Share & tools
lore
#0 lore raw

Linus,

Please pull the latest locking-rwsem-for-linus git tree from:

git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git locking-rwsem-for-linus

# HEAD: 4544ba8c6b1743499cabb682897a469911845f15 locking/rwsem: Fix comment on register clobbering

This tree, by Michal Hocko, implements down_write_killable(). The main usecase will be to update mm_sem usage sites to use this new API, to allow the mm-reaper introduced in:

aac453635549 mm, oom: introduce oom reaper

to tear down oom victim address spaces asynchronously with minimum latencies and without deadlock worries.

Thanks,

Ingo

------------------>
Borislav Petkov (1):
locking/rwsem: Fix comment on register clobbering

Michal Hocko (12):
locking/rwsem: Get rid of __down_write_nested()
locking/rwsem: Drop explicit memory barriers
locking/rwsem, xtensa: Drop superfluous arch specific implementation locking/rwsem, sh: Drop superfluous arch specific implementation locking/rwsem, sparc: Drop superfluous arch specific implementation locking/rwsem: Introduce basis for down_write_killable() locking/rwsem, alpha: Provide __down_write_killable()
locking/rwsem, ia64: Provide __down_write_killable()
locking/rwsem, s390: Provide __down_write_killable()
locking/rwsem, x86: Provide __down_write_killable()
locking/rwsem: Provide down_write_killable()
locking/rwsem, x86: Add frame annotation for call_rwsem_down_write_failed_killable()

Peter Zijlstra (1):
locking/rwsem: Fix down_write_killable()

 arch/alpha/include/asm/rwsem.h  |  18 +++++-
 arch/ia64/include/asm/rwsem.h   |  22 ++++++-
 arch/s390/include/asm/rwsem.h   |  18 ++++--
 arch/sh/include/asm/Kbuild      |   1 +
 arch/sh/include/asm/rwsem.h     | 132 ----------------------------------------
 arch/sparc/include/asm/Kbuild   |   1 +
 arch/sparc/include/asm/rwsem.h  | 124 -------------------------------------
 arch/x86/include/asm/rwsem.h    |  42 ++++++++-----
 arch/x86/lib/rwsem.S            |  16 ++++-
 arch/xtensa/include/asm/Kbuild  |   1 +
 arch/xtensa/include/asm/rwsem.h | 131 ---------------------------------------
 include/asm-generic/rwsem.h     |  13 +++-
 include/linux/lockdep.h         |  15 +++++
 include/linux/rwsem-spinlock.h  |   2 +-
 include/linux/rwsem.h           |   3 +
 kernel/locking/rwsem-spinlock.c |  19 +++++-
 kernel/locking/rwsem-xadd.c     |  38 ++++++++++--
 kernel/locking/rwsem.c          |  19 ++++++
 18 files changed, 189 insertions(+), 426 deletions(-)
 delete mode 100644 arch/sh/include/asm/rwsem.h
 delete mode 100644 arch/sparc/include/asm/rwsem.h
 delete mode 100644 arch/xtensa/include/asm/rwsem.h
arch/alpha/include/asm/rwsem.h +16 -2
index a83bbea62c67..0131a7058778 100644
@@ -63,7 +63,7 @@ static inline int __down_read_trylock(struct rw_semaphore *sem)
 	return res >= 0 ? 1 : 0;
 }
 
-static inline void __down_write(struct rw_semaphore *sem)
+static inline long ___down_write(struct rw_semaphore *sem)
 {
 	long oldcount;
 #ifndef	CONFIG_SMP
@@ -83,10 +83,24 @@ static inline void __down_write(struct rw_semaphore *sem)
 	:"=&r" (oldcount), "=m" (sem->count), "=&r" (temp)
 	:"Ir" (RWSEM_ACTIVE_WRITE_BIAS), "m" (sem->count) : "memory");
 #endif
-	if (unlikely(oldcount))
+	return oldcount;
+}
+
+static inline void __down_write(struct rw_semaphore *sem)
+{
+	if (unlikely(___down_write(sem)))
 		rwsem_down_write_failed(sem);
 }
 
+static inline int __down_write_killable(struct rw_semaphore *sem)
+{
+	if (unlikely(___down_write(sem)))
+		if (IS_ERR(rwsem_down_write_failed_killable(sem)))
+			return -EINTR;
+
+	return 0;
+}
+
 /*
  * trylock for writing -- returns 1 if successful, 0 if contention
  */
arch/ia64/include/asm/rwsem.h +19 -3
index ce112472bdd6..8b23e070b844 100644
@@ -49,8 +49,8 @@ __down_read (struct rw_semaphore *sem)
 /*
  * lock for writing
  */
-static inline void
-__down_write (struct rw_semaphore *sem)
+static inline long
+___down_write (struct rw_semaphore *sem)
 {
 	long old, new;
 
@@ -59,10 +59,26 @@ __down_write (struct rw_semaphore *sem)
 		new = old + RWSEM_ACTIVE_WRITE_BIAS;
 	} while (cmpxchg_acq(&sem->count, old, new) != old);
 
-	if (old != 0)
+	return old;
+}
+
+static inline void
+__down_write (struct rw_semaphore *sem)
+{
+	if (___down_write(sem))
 		rwsem_down_write_failed(sem);
 }
 
+static inline int
+__down_write_killable (struct rw_semaphore *sem)
+{
+	if (___down_write(sem))
+		if (IS_ERR(rwsem_down_write_failed_killable(sem)))
+			return -EINTR;
+
+	return 0;
+}
+
 /*
  * unlock after reading
  */
arch/s390/include/asm/rwsem.h +14 -4
index fead491dfc28..c75e4471e618 100644
@@ -90,7 +90,7 @@ static inline int __down_read_trylock(struct rw_semaphore *sem)
 /*
  * lock for writing
  */
-static inline void __down_write_nested(struct rw_semaphore *sem, int subclass)
+static inline long ___down_write(struct rw_semaphore *sem)
 {
 	signed long old, new, tmp;
 
@@ -104,13 +104,23 @@ static inline void __down_write_nested(struct rw_semaphore *sem, int subclass)
 		: "=&d" (old), "=&d" (new), "=Q" (sem->count)
 		: "Q" (sem->count), "m" (tmp)
 		: "cc", "memory");
-	if (old != 0)
-		rwsem_down_write_failed(sem);
+
+	return old;
 }
 
 static inline void __down_write(struct rw_semaphore *sem)
 {
-	__down_write_nested(sem, 0);
+	if (___down_write(sem))
+		rwsem_down_write_failed(sem);
+}
+
+static inline int __down_write_killable(struct rw_semaphore *sem)
+{
+	if (___down_write(sem))
+		if (IS_ERR(rwsem_down_write_failed_killable(sem)))
+			return -EINTR;
+
+	return 0;
 }
 
 /*
arch/sh/include/asm/Kbuild +1 -0
index a319745a7b63..751c3373a92c 100644
@@ -26,6 +26,7 @@ generic-y += percpu.h
 generic-y += poll.h
 generic-y += preempt.h
 generic-y += resource.h
+generic-y += rwsem.h
 generic-y += sembuf.h
 generic-y += serial.h
 generic-y += shmbuf.h
arch/sh/include/asm/rwsem.h deleted +0 -132

132 lines not on this page · raw message

arch/sparc/include/asm/Kbuild +1 -0
index e928618838bc..6024c26c0585 100644
@@ -16,6 +16,7 @@ generic-y += mm-arch-hooks.h
 generic-y += module.h
 generic-y += mutex.h
 generic-y += preempt.h
+generic-y += rwsem.h
 generic-y += serial.h
 generic-y += trace_clock.h
 generic-y += types.h
arch/sparc/include/asm/rwsem.h deleted +0 -124

124 lines not on this page · raw message

arch/x86/include/asm/rwsem.h +26 -16

52 lines not on this page · raw message

arch/x86/lib/rwsem.S +14 -2
index be110efa0096..bf2c6074efd2 100644
@@ -29,8 +29,10 @@
  * there is contention on the semaphore.
  *
  * %eax contains the semaphore pointer on entry. Save the C-clobbered
- * registers (%eax, %edx and %ecx) except %eax whish is either a return
- * value or just clobbered..
+ * registers (%eax, %edx and %ecx) except %eax which is either a return
+ * value or just gets clobbered. Same is true for %edx so make sure GCC
+ * reloads it after the slow path, by making it hold a temporary, for
+ * example see ____down_write().
  */
 
 #define save_common_regs \
@@ -106,6 +108,16 @@ ENTRY(call_rwsem_down_write_failed)
 	ret
 ENDPROC(call_rwsem_down_write_failed)
 
+ENTRY(call_rwsem_down_write_failed_killable)
+	FRAME_BEGIN
+	save_common_regs
+	movq %rax,%rdi
+	call rwsem_down_write_failed_killable
+	restore_common_regs
+	FRAME_END
+	ret
+ENDPROC(call_rwsem_down_write_failed_killable)
+
 ENTRY(call_rwsem_wake)
 	FRAME_BEGIN
 	/* do nothing if still outstanding active readers */
arch/xtensa/include/asm/Kbuild +1 -0
index b56855a1382a..28cf4c5d65ef 100644
@@ -22,6 +22,7 @@ generic-y += mm-arch-hooks.h
 generic-y += percpu.h
 generic-y += preempt.h
 generic-y += resource.h
+generic-y += rwsem.h
 generic-y += sections.h
 generic-y += siginfo.h
 generic-y += statfs.h
arch/xtensa/include/asm/rwsem.h deleted +0 -131

131 lines not on this page · raw message

include/asm-generic/rwsem.h +10 -3
index d6d5dc98d7da..3fc94a046bf5 100644
@@ -53,7 +53,7 @@ static inline int __down_read_trylock(struct rw_semaphore *sem)
 /*
  * lock for writing
  */
-static inline void __down_write_nested(struct rw_semaphore *sem, int subclass)
+static inline void __down_write(struct rw_semaphore *sem)
 {
 	long tmp;
 
@@ -63,9 +63,16 @@ static inline void __down_write_nested(struct rw_semaphore *sem, int subclass)
 		rwsem_down_write_failed(sem);
 }
 
-static inline void __down_write(struct rw_semaphore *sem)
+static inline int __down_write_killable(struct rw_semaphore *sem)
 {
-	__down_write_nested(sem, 0);
+	long tmp;
+
+	tmp = atomic_long_add_return_acquire(RWSEM_ACTIVE_WRITE_BIAS,
+				     (atomic_long_t *)&sem->count);
+	if (unlikely(tmp != RWSEM_ACTIVE_WRITE_BIAS))
+		if (IS_ERR(rwsem_down_write_failed_killable(sem)))
+			return -EINTR;
+	return 0;
 }
 
 static inline int __down_write_trylock(struct rw_semaphore *sem)
include/linux/lockdep.h +15 -0
index d026b190c530..accfe56d8c51 100644
@@ -444,6 +444,18 @@ do {								\
 	lock_acquired(&(_lock)->dep_map, _RET_IP_);			\
 } while (0)
 
+#define LOCK_CONTENDED_RETURN(_lock, try, lock)			\
+({								\
+	int ____err = 0;					\
+	if (!try(_lock)) {					\
+		lock_contended(&(_lock)->dep_map, _RET_IP_);	\
+		____err = lock(_lock);				\
+	}							\
+	if (!____err)						\
+		lock_acquired(&(_lock)->dep_map, _RET_IP_);	\
+	____err;						\
+})
+
 #else /* CONFIG_LOCK_STAT */
 
 #define lock_contended(lockdep_map, ip) do {} while (0)
@@ -452,6 +464,9 @@ do {								\
 #define LOCK_CONTENDED(_lock, try, lock) \
 	lock(_lock)
 
+#define LOCK_CONTENDED_RETURN(_lock, try, lock) \
+	lock(_lock)
+
 #endif /* CONFIG_LOCK_STAT */
 
 #ifdef CONFIG_LOCKDEP
include/linux/rwsem-spinlock.h +1 -1
index 561e8615528d..ae0528b834cd 100644
@@ -34,7 +34,7 @@ struct rw_semaphore {
 extern void __down_read(struct rw_semaphore *sem);
 extern int __down_read_trylock(struct rw_semaphore *sem);
 extern void __down_write(struct rw_semaphore *sem);
-extern void __down_write_nested(struct rw_semaphore *sem, int subclass);
+extern int __must_check __down_write_killable(struct rw_semaphore *sem);
 extern int __down_write_trylock(struct rw_semaphore *sem);
 extern void __up_read(struct rw_semaphore *sem);
 extern void __up_write(struct rw_semaphore *sem);
include/linux/rwsem.h +3 -0
index 8f498cdde280..d1c12d160ace 100644
@@ -14,6 +14,7 @@
 #include <linux/list.h>
 #include <linux/spinlock.h>
 #include <linux/atomic.h>
+#include <linux/err.h>
 #ifdef CONFIG_RWSEM_SPIN_ON_OWNER
 #include <linux/osq_lock.h>
 #endif
@@ -43,6 +44,7 @@ struct rw_semaphore {
 
 extern struct rw_semaphore *rwsem_down_read_failed(struct rw_semaphore *sem);
 extern struct rw_semaphore *rwsem_down_write_failed(struct rw_semaphore *sem);
+extern struct rw_semaphore *rwsem_down_write_failed_killable(struct rw_semaphore *sem);
 extern struct rw_semaphore *rwsem_wake(struct rw_semaphore *);
 extern struct rw_semaphore *rwsem_downgrade_wake(struct rw_semaphore *sem);
 
@@ -116,6 +118,7 @@ extern int down_read_trylock(struct rw_semaphore *sem);
  * lock for writing
  */
 extern void down_write(struct rw_semaphore *sem);
+extern int __must_check down_write_killable(struct rw_semaphore *sem);
 
 /*
  * trylock for writing -- returns 1 if successful, 0 if contention
kernel/locking/rwsem-spinlock.c +16 -3
index 3a5048572065..1591f6b3539f 100644
@@ -191,11 +191,12 @@ int __down_read_trylock(struct rw_semaphore *sem)
 /*
  * get a write lock on the semaphore
  */
-void __sched __down_write_nested(struct rw_semaphore *sem, int subclass)
+int __sched __down_write_common(struct rw_semaphore *sem, int state)
 {
 	struct rwsem_waiter waiter;
 	struct task_struct *tsk;
 	unsigned long flags;
+	int ret = 0;
 
 	raw_spin_lock_irqsave(&sem->wait_lock, flags);
 
@@ -215,21 +216,33 @@ void __sched __down_write_nested(struct rw_semaphore *sem, int subclass)
 		 */
 		if (sem->count == 0)
 			break;
-		set_task_state(tsk, TASK_UNINTERRUPTIBLE);
+		if (signal_pending_state(state, current)) {
+			ret = -EINTR;
+			goto out;
+		}
+		set_task_state(tsk, state);
 		raw_spin_unlock_irqrestore(&sem->wait_lock, flags);
 		schedule();
 		raw_spin_lock_irqsave(&sem->wait_lock, flags);
 	}
 	/* got the lock */
 	sem->count = -1;
+out:
 	list_del(&waiter.list);
 
 	raw_spin_unlock_irqrestore(&sem->wait_lock, flags);
+
+	return ret;
 }
 
 void __sched __down_write(struct rw_semaphore *sem)
 {
-	__down_write_nested(sem, 0);
+	__down_write_common(sem, TASK_UNINTERRUPTIBLE);
+}
+
+int __sched __down_write_killable(struct rw_semaphore *sem)
+{
+	return __down_write_common(sem, TASK_KILLABLE);
 }
 
 /*
kernel/locking/rwsem-xadd.c +33 -5
index a4d4de05b2d1..09e30c6225e5 100644
@@ -433,12 +433,13 @@ static inline bool rwsem_has_spinner(struct rw_semaphore *sem)
 /*
  * Wait until we successfully acquire the write lock
  */
-__visible
-struct rw_semaphore __sched *rwsem_down_write_failed(struct rw_semaphore *sem)
+static inline struct rw_semaphore *
+__rwsem_down_write_failed_common(struct rw_semaphore *sem, int state)
 {
 	long count;
 	bool waiting = true; /* any queued threads before us */
 	struct rwsem_waiter waiter;
+	struct rw_semaphore *ret = sem;
 
 	/* undo write bias from down_write operation, stop active locking */
 	count = rwsem_atomic_update(-RWSEM_ACTIVE_WRITE_BIAS, sem);
@@ -478,7 +479,7 @@ struct rw_semaphore __sched *rwsem_down_write_failed(struct rw_semaphore *sem)
 		count = rwsem_atomic_update(RWSEM_WAITING_BIAS, sem);
 
 	/* wait until we successfully acquire the lock */
-	set_current_state(TASK_UNINTERRUPTIBLE);
+	set_current_state(state);
 	while (true) {
 		if (rwsem_try_write_lock(count, sem))
 			break;
@@ -486,21 +487,48 @@ struct rw_semaphore __sched *rwsem_down_write_failed(struct rw_semaphore *sem)
 
 		/* Block until there are no active lockers. */
 		do {
+			if (signal_pending_state(state, current))
+				goto out_nolock;
+
 			schedule();
-			set_current_state(TASK_UNINTERRUPTIBLE);
+			set_current_state(state);
 		} while ((count = sem->count) & RWSEM_ACTIVE_MASK);
 
 		raw_spin_lock_irq(&sem->wait_lock);
 	}
 	__set_current_state(TASK_RUNNING);
+	list_del(&waiter.list);
+	raw_spin_unlock_irq(&sem->wait_lock);
 
+	return ret;
+
+out_nolock:
+	__set_current_state(TASK_RUNNING);
+	raw_spin_lock_irq(&sem->wait_lock);
 	list_del(&waiter.list);
+	if (list_empty(&sem->wait_list))
+		rwsem_atomic_update(-RWSEM_WAITING_BIAS, sem);
+	else
+		__rwsem_do_wake(sem, RWSEM_WAKE_ANY);
 	raw_spin_unlock_irq(&sem->wait_lock);
 
-	return sem;
+	return ERR_PTR(-EINTR);
+}
+
+__visible struct rw_semaphore * __sched
+rwsem_down_write_failed(struct rw_semaphore *sem)
+{
+	return __rwsem_down_write_failed_common(sem, TASK_UNINTERRUPTIBLE);
 }
 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);
+}
+EXPORT_SYMBOL(rwsem_down_write_failed_killable);
+
 /*
  * handle waking up a waiter on the semaphore
  * - up_read/up_write has decremented the active part of count if we come here
kernel/locking/rwsem.c +19 -0
index 205be0ce34de..c817216c1615 100644
@@ -55,6 +55,25 @@ void __sched down_write(struct rw_semaphore *sem)
 EXPORT_SYMBOL(down_write);
 
 /*
+ * lock for writing
+ */
+int __sched down_write_killable(struct rw_semaphore *sem)
+{
+	might_sleep();
+	rwsem_acquire(&sem->dep_map, 0, 0, _RET_IP_);
+
+	if (LOCK_CONTENDED_RETURN(sem, __down_write_trylock, __down_write_killable)) {
+		rwsem_release(&sem->dep_map, 1, _RET_IP_);
+		return -EINTR;
+	}
+
+	rwsem_set_owner(sem);
+	return 0;
+}
+
+EXPORT_SYMBOL(down_write_killable);
+
+/*
  * trylock for writing -- returns 1 if successful, 0 if contention
  */
 int down_write_trylock(struct rw_semaphore *sem)
#1 lore raw
Quoted 3 lines · Ingo Molnar

This tree, by Michal Hocko, implements down_write_killable(). The main usecase will be to update mm_sem usage sites to use this new API,

Hmm. Is somebody (Michal?) looking at down_read_killable() too?

The VFS layer will want it with the pending parallel lookup code - the inode semaphore is being converted to a rwsem, and there's a couple of "killable" users.. The first step actually just wants to the exclusive case (ie the write case that this adds), but I think the readdir code could really use a reading version too..

Linus

#2 lore raw
Quoted 6 lines · Linus Torvalds
Quoted 3 lines · Ingo Molnar

This tree, by Michal Hocko, implements down_write_killable(). The main usecase will be to update mm_sem usage sites to use this new API,

Hmm. Is somebody (Michal?) looking at down_read_killable() too?

I wasn't because I didn't need it for my oom_reaper use case.

Quoted 5 lines

The VFS layer will want it with the pending parallel lookup code - the inode semaphore is being converted to a rwsem, and there's a couple of "killable" users.. The first step actually just wants to the exclusive case (ie the write case that this adds), but I think the readdir code could really use a reading version too..

This is more a question for Al but I do not think adding killable read lock is a big deal. I can certainly help with it.

Signature · 2 lines
Michal Hocko
SUSE Labs
maintainer
#5 lore raw
In reply to Linus Torvalds
Quoted 5 lines · Linus Torvalds
Quoted 3 lines · Ingo Molnar

This tree, by Michal Hocko, implements down_write_killable(). The main usecase will be to update mm_sem usage sites to use this new API,

Quoted 7 lines

Hmm. Is somebody (Michal?) looking at down_read_killable() too?

The VFS layer will want it with the pending parallel lookup code - the inode semaphore is being converted to a rwsem, and there's a couple of "killable" users.. The first step actually just wants to the exclusive case (ie the write case that this adds), but I think the readdir code could really use a reading version too..

Something roughly like so.

Completely untested etc..

---
 kernel/locking/rwsem-spinlock.c | 28 +++++++++++++++++++++++-----
 kernel/locking/rwsem-xadd.c     | 28 +++++++++++++++++++++++++---
 2 files changed, 48 insertions(+), 8 deletions(-)
kernel/locking/rwsem-spinlock.c +23 -5
index 1591f6b3539f..d66325970248 100644
@@ -125,7 +125,7 @@ __rwsem_wake_one_writer(struct rw_semaphore *sem)
 /*
  * get a read lock on the semaphore
  */
-void __sched __down_read(struct rw_semaphore *sem)
+int __sched __down_read_common(struct rw_semaphore *sem, int state)
 {
 	struct rwsem_waiter waiter;
 	struct task_struct *tsk;
@@ -137,11 +137,11 @@ void __sched __down_read(struct rw_semaphore *sem)
 		/* granted */
 		sem->count++;
 		raw_spin_unlock_irqrestore(&sem->wait_lock, flags);
-		goto out;
+		return 0;
 	}
 
 	tsk = current;
-	set_task_state(tsk, TASK_UNINTERRUPTIBLE);
+	set_task_state(tsk, state);
 
 	/* set up my own style of waitqueue */
 	waiter.task = tsk;
@@ -155,6 +155,8 @@ void __sched __down_read(struct rw_semaphore *sem)
 
 	/* wait to be given the lock */
 	for (;;) {
+		if (signal_pending_state(state, current))
+			goto out_nolock;
 		if (!waiter.task)
 			break;
 		schedule();
@@ -162,8 +164,24 @@ void __sched __down_read(struct rw_semaphore *sem)
 	}
 
 	__set_task_state(tsk, TASK_RUNNING);
- out:
-	;
+	return 0;
+
+out_nolock:
+	raw_spin_lock_irqsave(&sem->wait_lock, flags);
+	if (waiter.task)
+		list_del(&waiter.list);
+	else
+		sem->count--;
+	if (!list_empty(&sem->wait_list))
+		__rwsem_do_wake(sem, 1);
+	raw_spin_unlock_irqrestore(&sem->wait_lock, flags);
+
+	return -EINTR;
+}
+
+void __sched __down_read(struct rw_semaphore *sem)
+{
+	__down_read_common(sem, TASK_UNINTERRUPTIBLE);
 }
 
 /*
kernel/locking/rwsem-xadd.c +25 -3
index 09e30c6225e5..4f7b1fb1f127 100644
@@ -210,8 +210,8 @@ __rwsem_do_wake(struct rw_semaphore *sem, enum rwsem_wake_type wake_type)
 /*
  * Wait for the read lock to be granted
  */
-__visible
-struct rw_semaphore __sched *rwsem_down_read_failed(struct rw_semaphore *sem)
+static inline struct rw_semaphore *
+rwsem_down_read_failed_common(struct rw_semaphore *sem, int state)
 {
 	long count, adjustment = -RWSEM_ACTIVE_READ_BIAS;
 	struct rwsem_waiter waiter;
@@ -244,7 +244,9 @@ struct rw_semaphore __sched *rwsem_down_read_failed(struct rw_semaphore *sem)
 
 	/* wait to be given the lock */
 	while (true) {
-		set_task_state(tsk, TASK_UNINTERRUPTIBLE);
+		set_task_state(tsk, state);
+		if (signal_pending_state(state, current))
+			goto out_nolock;
 		if (!waiter.task)
 			break;
 		schedule();
@@ -252,6 +254,26 @@ struct rw_semaphore __sched *rwsem_down_read_failed(struct rw_semaphore *sem)
 
 	__set_task_state(tsk, TASK_RUNNING);
 	return sem;
+
+out_nolock:
+	raw_spin_lock_irq(&sem->wait_lock);
+	if (waiter.task != NULL)
+		list_del(&waiter.list);
+	else
+		rwsem_atomic_update(-RWSEM_ACTIVE_READ_BIAS, sem);
+	if (list_empty(&sem->wait_list))
+		rwsem_atomic_update(-RWSEM_WAITING_BIAS, sem);
+	else
+		__rwsem_do_wake(sem, RWSEM_WAKE_ANY);
+	raw_spin_unlock_irq(&sem->wait_lock);
+
+	return ERR_PTR(-EINTR);
+}
+
+__visible struct rw_semaphore * __sched
+rwsem_down_read_failed(struct rw_semaphore *sem)
+{
+	return rwsem_down_read_failed_common(sem, TASK_UNINTERRUPTIBLE);
 }
 EXPORT_SYMBOL(rwsem_down_read_failed);
 
#3 lore raw
In reply to Michal Hocko
Quoted 10 lines · Michal Hocko
Quoted 6 lines · Linus Torvalds
Quoted 3 lines · Ingo Molnar

This tree, by Michal Hocko, implements down_write_killable(). The main usecase will be to update mm_sem usage sites to use this new API,

Hmm. Is somebody (Michal?) looking at down_read_killable() too?

I wasn't because I didn't need it for my oom_reaper use case.

Quoted 8 lines
Quoted 5 lines

The VFS layer will want it with the pending parallel lookup code - the inode semaphore is being converted to a rwsem, and there's a couple of "killable" users.. The first step actually just wants to the exclusive case (ie the write case that this adds), but I think the readdir code could really use a reading version too..

This is more a question for Al but I do not think adding killable read lock is a big deal. I can certainly help with it.

I'm not sure - killable write is needed in a bunch of places there (and the only reason it's not used in #work.lookups is to avoid even more merge headache; as soon as both are merged, I'll post a trivial followup switching half a dozen places to it), killable read... Do we really need it? The only plausible user right now (->i_rwsem one, that is) is parallel readdir. And I'm not convinced that we need to make that one killable. We can (down_read_killable seems to be easy to put together), but is it worth using it in that usecase?

IIRC, the original motivation had been "what if anything gets stuck on NFS server and we are left with unkillable processes on client sitting there in attempts to lock the directory". Seeing that lookups would be just as prone to getting stuck (and had been all along)... It doesn't look like there could be a deadlock scenario avoided by having that down_read (or mutex_lock) killable. I can't find the original thread, unfortunately; does anyone remember the details?

#4 lore raw
Quoted 9 lines · Al Viro

I'm not sure - killable write is needed in a bunch of places there (and the only reason it's not used in #work.lookups is to avoid even more merge headache; as soon as both are merged, I'll post a trivial followup switching half a dozen places to it), killable read... Do we really need it? The only plausible user right now (->i_rwsem one, that is) is parallel readdir. And I'm not convinced that we need to make that one killable. We can (down_read_killable seems to be easy to put together), but is it worth using it in that usecase?

It may not be a big deal.

Especially since I also expect to merge the "stop readdir in the middle if a signal is pending" patch that was buggy last time around but Ted has a fixed and extended version of it.

So together with the fact that most of the time we hopefully end up not blocking anyway (because taking the rwsem for writing is hopefully less common) I guess that the possible latency issue just isn't going to ever be noticeable.

So let's just see if we ever hit a situation where people end up even noticing the issue.

Linus