linux-kernel - Re: [PATCH 5/6] locking/rwsem: Unify wait loop

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <Y/uFn4WwZ8R3Dh4S@hirez.programming.kicks-ass.net>
Date:   Sun, 26 Feb 2023 17:15:27 +0100
From:   Peter Zijlstra <peterz@...radead.org>
To:     Waiman Long <longman@...hat.com>
Cc:     mingo@...hat.com, will@...nel.org, linux-kernel@...r.kernel.org,
        boqun.feng@...il.com
Subject: Re: [PATCH 5/6] locking/rwsem: Unify wait loop

On Thu, Feb 23, 2023 at 05:45:56PM -0500, Waiman Long wrote:

> Unfortunately, lockevent_inc() doesn't work with waiter_type() like that as
> the compilation will fail if CONFIG_LOCK_EVENT_COUNTS is enabled.  Could you
> include the attached patch in your series and make the following changes?

Yeah, robot told me; fixed it like so:

--- a/kernel/locking/rwsem.c
+++ b/kernel/locking/rwsem.c
@@ -995,13 +995,16 @@ static inline void rwsem_cond_wake_waite
 	rwsem_mark_wake(sem, wake_type, wake_q);
 }
 
-#define waiter_type(_waiter, _r, _w)	\
-	((_waiter)->type == RWSEM_WAITING_FOR_READ ? (_r) : (_w))
+#define lockevent_rw_inc(rd, evr, evw)	do {	\
+	lockevent_cond_inc(evr, (rd));		\
+	lockevent_cond_inc(evw, !(rd));		\
+} while (0)
 
 static __always_inline struct rw_semaphore *
-rwsem_waiter_wait(struct rw_semaphore *sem, struct rwsem_waiter *waiter, int state)
+rwsem_waiter_wait(struct rw_semaphore *sem, struct rwsem_waiter *waiter,
+		  int state, bool reader)
 {
-	trace_contention_begin(sem, waiter_type(waiter, LCB_F_READ, LCB_F_WRITE));
+	trace_contention_begin(sem, reader ? LCB_F_READ : LCB_F_WRITE);
 
 	/* wait to be given the lock */
 	for (;;) {
@@ -1019,18 +1022,19 @@ rwsem_waiter_wait(struct rw_semaphore *s
 			break;
 		}
 		schedule_preempt_disabled();
-		lockevent_inc(waiter_type(waiter, rwsem_sleep_reader, rwsem_sleep_writer));
+		lockevent_rw_inc(reader, rwsem_sleep_reader, rwsem_sleep_writer);
 	}
 
 	__set_current_state(TASK_RUNNING);
-	lockevent_inc(waiter_type(waiter, rwsem_rlock, rwsem_wlock));
+
+	lockevent_rw_inc(reader, rwsem_rlock, rwsem_wlock);
 	trace_contention_end(sem, 0);
 	return sem;
 
 out_nolock:
 	rwsem_del_wake_waiter(sem, waiter);
 	__set_current_state(TASK_RUNNING);
-	lockevent_inc(waiter_type(waiter, rwsem_rlock_fail, rwsem_wlock_fail));
+	lockevent_rw_inc(reader, rwem_rlock_fail, rwsem_wlock_fail);
 	trace_contention_end(sem, -EINTR);
 	return ERR_PTR(-EINTR);
 }
@@ -1112,7 +1116,7 @@ rwsem_down_read_slowpath(struct rw_semap
 	if (!wake_q_empty(&wake_q))
 		wake_up_q(&wake_q);
 
-	return rwsem_waiter_wait(sem, &waiter, state);
+	return rwsem_waiter_wait(sem, &waiter, state, true);
 }
 
 /*
@@ -1162,7 +1166,7 @@ rwsem_down_write_slowpath(struct rw_sema
 	}
 	raw_spin_unlock_irq(&sem->wait_lock);
 
-	return rwsem_waiter_wait(sem, &waiter, state);
+	return rwsem_waiter_wait(sem, &waiter, state, false);
 }
 
 /*