linux-kernel - Re: [PATCH v8 13/19] locking/rwsem: Make rwsem->owner an atomic_long

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: <20190604085238.GF3402@hirez.programming.kicks-ass.net>
Date:   Tue, 4 Jun 2019 10:52:38 +0200
From:   Peter Zijlstra <peterz@...radead.org>
To:     Waiman Long <longman@...hat.com>
Cc:     Ingo Molnar <mingo@...hat.com>, Will Deacon <will.deacon@....com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Borislav Petkov <bp@...en8.de>,
        "H. Peter Anvin" <hpa@...or.com>, linux-kernel@...r.kernel.org,
        x86@...nel.org, Davidlohr Bueso <dave@...olabs.net>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Tim Chen <tim.c.chen@...ux.intel.com>,
        huang ying <huang.ying.caritas@...il.com>
Subject: Re: [PATCH v8 13/19] locking/rwsem: Make rwsem->owner an
 atomic_long_t

On Mon, May 20, 2019 at 04:59:12PM -0400, Waiman Long wrote:
> +static inline struct task_struct *rwsem_read_owner(struct rw_semaphore *sem)
> +{
> +	return (struct task_struct *)(atomic_long_read(&sem->owner) &
> +					~RWSEM_OWNER_FLAGS_MASK);
> +}
> +
> +/*
> + * Return the real task structure pointer of the owner and the embedded
> + * flags in the owner. pflags must be non-NULL.
> + */
> +static inline struct task_struct *
> +rwsem_read_owner_flags(struct rw_semaphore *sem, long *pflags)
> +{
> +	long owner = atomic_long_read(&sem->owner);
> +
> +	*pflags = owner & RWSEM_OWNER_FLAGS_MASK;
> +	return (struct task_struct *)(owner & ~RWSEM_OWNER_FLAGS_MASK);
> +}

I got confused by the 'read' part in those nanes, I initially thought
they paired with rwsem_set_reader_owned().

So I've done 's/rwsem_read_owner/rwsem_owner/g on it.

--- a/kernel/locking/rwsem.c
+++ b/kernel/locking/rwsem.c
@@ -194,10 +194,10 @@ static inline void rwsem_clear_reader_ow
 /*
  * Return just the real task structure pointer of the owner
  */
-static inline struct task_struct *rwsem_read_owner(struct rw_semaphore *sem)
+static inline struct task_struct *rwsem_owner(struct rw_semaphore *sem)
 {
-	return (struct task_struct *)(atomic_long_read(&sem->owner) &
-					~RWSEM_OWNER_FLAGS_MASK);
+	return (struct task_struct *)
+		(atomic_long_read(&sem->owner) & ~RWSEM_OWNER_FLAGS_MASK);
 }
 
 /*
@@ -205,7 +205,7 @@ static inline struct task_struct *rwsem_
  * flags in the owner. pflags must be non-NULL.
  */
 static inline struct task_struct *
-rwsem_read_owner_flags(struct rw_semaphore *sem, long *pflags)
+rwsem_owner_flags(struct rw_semaphore *sem, long *pflags)
 {
 	long owner = atomic_long_read(&sem->owner);
 
@@ -561,7 +561,7 @@ static inline bool rwsem_can_spin_on_own
 
 	preempt_disable();
 	rcu_read_lock();
-	owner = rwsem_read_owner_flags(sem, &flags);
+	owner = rwsem_owner_flags(sem, &flags);
 	if ((flags & RWSEM_NONSPINNABLE) || (owner && !owner_on_cpu(owner)))
 		ret = false;
 	rcu_read_unlock();
@@ -590,8 +590,8 @@ enum owner_state {
 };
 #define OWNER_SPINNABLE		(OWNER_NULL | OWNER_WRITER)
 
-static inline enum owner_state rwsem_owner_state(struct task_struct *owner,
-						 long flags)
+static inline enum owner_state
+rwsem_owner_state(struct task_struct *owner, long flags)
 {
 	if (flags & RWSEM_NONSPINNABLE)
 		return OWNER_NONSPINNABLE;
@@ -608,7 +608,7 @@ static noinline enum owner_state rwsem_s
 	long flags, new_flags;
 	enum owner_state state;
 
-	owner = rwsem_read_owner_flags(sem, &flags);
+	owner = rwsem_owner_flags(sem, &flags);
 	state = rwsem_owner_state(owner, flags);
 	if (state != OWNER_WRITER)
 		return state;
@@ -620,7 +620,7 @@ static noinline enum owner_state rwsem_s
 			break;
 		}
 
-		new = rwsem_read_owner_flags(sem, &new_flags);
+		new = rwsem_owner_flags(sem, &new_flags);
 		if ((new != owner) || (new_flags != flags)) {
 			state = rwsem_owner_state(new, new_flags);
 			break;
@@ -1139,7 +1139,7 @@ static inline void __up_write(struct rw_
 	 * sem->owner may differ from current if the ownership is transferred
 	 * to an anonymous writer by setting the RWSEM_NONSPINNABLE bits.
 	 */
-	DEBUG_RWSEMS_WARN_ON((rwsem_read_owner(sem) != current) &&
+	DEBUG_RWSEMS_WARN_ON((rwsem_owner(sem) != current) &&
 			    !rwsem_test_oflags(sem, RWSEM_NONSPINNABLE), sem);
 	rwsem_clear_owner(sem);
 	tmp = atomic_long_fetch_add_release(-RWSEM_WRITER_LOCKED, &sem->count);
@@ -1161,7 +1161,7 @@ static inline void __downgrade_write(str
 	 * read-locked region is ok to be re-ordered into the
 	 * write side. As such, rely on RELEASE semantics.
 	 */
-	DEBUG_RWSEMS_WARN_ON(rwsem_read_owner(sem) != current, sem);
+	DEBUG_RWSEMS_WARN_ON(rwsem_owner(sem) != current, sem);
 	tmp = atomic_long_fetch_add_release(
 		-RWSEM_WRITER_LOCKED+RWSEM_READER_BIAS, &sem->count);
 	rwsem_set_reader_owned(sem);