Re: [PATCH v4 06/16] locking/rwsem: Code cleanup after files merging
On 04/16/2019 12:17 PM, Peter Zijlstra wrote:
> On Tue, Apr 16, 2019 at 06:01:13PM +0200, Peter Zijlstra wrote:
>> @@ -794,34 +770,38 @@ static inline int __down_read_trylock(st
>> */
>> static inline void __down_write(struct rw_semaphore *sem)
>> {
>> -if (unlikely(atomic_long_cmpxchg_acquire(>count, 0,
>> - RWSEM_WRITER_LOCKED)))
>> -rwsem_down_write_failed(sem);
>> +long tmp = RWSEM_UNLOCKED_VALUE;
>> +
>> +if (unlikely(atomic_long_try_cmpxchg_acquire(>count, ,
>> + RWSEM_WRITER_LOCKED)))
> !
>
>> +rwsem_down_write_slow(sem, TASK_UNINTERRUPTIBLE);
>> rwsem_set_owner(sem);
>> }
>>
>> static inline int __down_write_killable(struct rw_semaphore *sem)
>> {
>> -if (unlikely(atomic_long_cmpxchg_acquire(>count, 0,
>> - RWSEM_WRITER_LOCKED)))
>> -if (IS_ERR(rwsem_down_write_failed_killable(sem)))
>> +long tmp = RWSEM_UNLOCKED_VALUE;
>> +
>> +if (unlikely(atomic_long_try_cmpxchg_acquire(>count, ,
>> + RWSEM_WRITER_LOCKED))) {
> also !
>
>> +if (IS_ERR(rwsem_down_write_slow(sem, TASK_KILLABLE)))
>> return -EINTR;
>> +}
>> rwsem_set_owner(sem);
>> return 0;
>> }
> I'm having a great day it seems, it's like back in uni, trying to find
> all the missing - signs in this page-long DE.
I am really grateful that you have spent the time to review my rwsem
patches. It is also very intense for me in the last few days
concentrating on investigating the regression and finding useful fixes
that I don't have time to deal with other stuffs.
Cheers,
Longman
Re: [PATCH v4 06/16] locking/rwsem: Code cleanup after files merging
On Tue, Apr 16, 2019 at 06:01:13PM +0200, Peter Zijlstra wrote:
> @@ -794,34 +770,38 @@ static inline int __down_read_trylock(st
> */
> static inline void __down_write(struct rw_semaphore *sem)
> {
> - if (unlikely(atomic_long_cmpxchg_acquire(>count, 0,
> - RWSEM_WRITER_LOCKED)))
> - rwsem_down_write_failed(sem);
> + long tmp = RWSEM_UNLOCKED_VALUE;
> +
> + if (unlikely(atomic_long_try_cmpxchg_acquire(>count, ,
> + RWSEM_WRITER_LOCKED)))
!
> + rwsem_down_write_slow(sem, TASK_UNINTERRUPTIBLE);
> rwsem_set_owner(sem);
> }
>
> static inline int __down_write_killable(struct rw_semaphore *sem)
> {
> - if (unlikely(atomic_long_cmpxchg_acquire(>count, 0,
> - RWSEM_WRITER_LOCKED)))
> - if (IS_ERR(rwsem_down_write_failed_killable(sem)))
> + long tmp = RWSEM_UNLOCKED_VALUE;
> +
> + if (unlikely(atomic_long_try_cmpxchg_acquire(>count, ,
> + RWSEM_WRITER_LOCKED))) {
also !
> + if (IS_ERR(rwsem_down_write_slow(sem, TASK_KILLABLE)))
> return -EINTR;
> + }
> rwsem_set_owner(sem);
> return 0;
> }
I'm having a great day it seems, it's like back in uni, trying to find
all the missing - signs in this page-long DE.
Re: [PATCH v4 06/16] locking/rwsem: Code cleanup after files merging
More cleanups..
---
--- a/kernel/locking/rwsem.c
+++ b/kernel/locking/rwsem.c
@@ -303,7 +303,7 @@ static void __rwsem_mark_wake(struct rw_
list_del(>list);
/*
* Ensure calling get_task_struct() before setting the reader
-* waiter to nil such that rwsem_down_read_failed() cannot
+* waiter to nil such that rwsem_down_read_slow() cannot
* race with do_exit() by always holding a reference count
* to the task to wakeup.
*/
@@ -500,7 +500,7 @@ static bool rwsem_optimistic_spin(struct
* Wait for the read lock to be granted
*/
static inline struct rw_semaphore __sched *
-__rwsem_down_read_failed_common(struct rw_semaphore *sem, int state)
+rwsem_down_read_slow(struct rw_semaphore *sem, int state)
{
long count, adjustment = -RWSEM_READER_BIAS;
struct rwsem_waiter waiter;
@@ -572,23 +572,11 @@ __rwsem_down_read_failed_common(struct r
return ERR_PTR(-EINTR);
}
-static inline struct rw_semaphore * __sched
-rwsem_down_read_failed(struct rw_semaphore *sem)
-{
- return __rwsem_down_read_failed_common(sem, TASK_UNINTERRUPTIBLE);
-}
-
-static inline struct rw_semaphore * __sched
-rwsem_down_read_failed_killable(struct rw_semaphore *sem)
-{
- return __rwsem_down_read_failed_common(sem, TASK_KILLABLE);
-}
-
/*
* Wait until we successfully acquire the write lock
*/
static inline struct rw_semaphore *
-__rwsem_down_write_failed_common(struct rw_semaphore *sem, int state)
+rwsem_down_write_slow(struct rw_semaphore *sem, int state)
{
long count;
bool waiting = true; /* any queued threads before us */
@@ -689,18 +677,6 @@ __rwsem_down_write_failed_common(struct
return ERR_PTR(-EINTR);
}
-static inline struct rw_semaphore * __sched
-rwsem_down_write_failed(struct rw_semaphore *sem)
-{
- return __rwsem_down_write_failed_common(sem, TASK_UNINTERRUPTIBLE);
-}
-
-static inline struct rw_semaphore * __sched
-rwsem_down_write_failed_killable(struct rw_semaphore *sem)
-{
- return __rwsem_down_write_failed_common(sem, TASK_KILLABLE);
-}
-
/*
* handle waking up a waiter on the semaphore
* - up_read/up_write has decremented the active part of count if we come here
@@ -749,7 +725,7 @@ inline void __down_read(struct rw_semaph
{
if (unlikely(atomic_long_fetch_add_acquire(RWSEM_READER_BIAS,
>count) & RWSEM_READ_FAILED_MASK)) {
- rwsem_down_read_failed(sem);
+ rwsem_down_read_slow(sem, TASK_UNINTERRUPTIBLE);
DEBUG_RWSEMS_WARN_ON(!((unsigned long)sem->owner &
RWSEM_READER_OWNED), sem);
} else {
@@ -761,7 +737,7 @@ static inline int __down_read_killable(s
{
if (unlikely(atomic_long_fetch_add_acquire(RWSEM_READER_BIAS,
>count) & RWSEM_READ_FAILED_MASK)) {
- if (IS_ERR(rwsem_down_read_failed_killable(sem)))
+ if (IS_ERR(rwsem_down_read_slow(sem, TASK_KILLABLE)))
return -EINTR;
DEBUG_RWSEMS_WARN_ON(!((unsigned long)sem->owner &
RWSEM_READER_OWNED), sem);
@@ -794,34 +770,38 @@ static inline int __down_read_trylock(st
*/
static inline void __down_write(struct rw_semaphore *sem)
{
- if (unlikely(atomic_long_cmpxchg_acquire(>count, 0,
-RWSEM_WRITER_LOCKED)))
- rwsem_down_write_failed(sem);
+ long tmp = RWSEM_UNLOCKED_VALUE;
+
+ if (unlikely(atomic_long_try_cmpxchg_acquire(>count, ,
+RWSEM_WRITER_LOCKED)))
+ rwsem_down_write_slow(sem, TASK_UNINTERRUPTIBLE);
rwsem_set_owner(sem);
}
static inline int __down_write_killable(struct rw_semaphore *sem)
{
- if (unlikely(atomic_long_cmpxchg_acquire(>count, 0,
-RWSEM_WRITER_LOCKED)))
- if (IS_ERR(rwsem_down_write_failed_killable(sem)))
+ long tmp = RWSEM_UNLOCKED_VALUE;
+
+ if (unlikely(atomic_long_try_cmpxchg_acquire(>count, ,
+RWSEM_WRITER_LOCKED))) {
+ if (IS_ERR(rwsem_down_write_slow(sem, TASK_KILLABLE)))
return -EINTR;
+ }
rwsem_set_owner(sem);
return 0;
}
static inline int __down_write_trylock(struct rw_semaphore *sem)
{
- long tmp;
+ long tmp = RWSEM_UNLOCKED_VALUE;
lockevent_inc(rwsem_wtrylock);
- tmp = atomic_long_cmpxchg_acquire(>count, RWSEM_UNLOCKED_VALUE,
- RWSEM_WRITER_LOCKED);
- if (tmp == RWSEM_UNLOCKED_VALUE) {
- rwsem_set_owner(sem);
- return true;
- }
- return false;
+ if (!atomic_long_try_cmpxchg_acquire(>count, ,
+
[PATCH v4 06/16] locking/rwsem: Code cleanup after files merging
After merging all the relevant rwsem code into one single file, there
are a number of optimizations and cleanups that can be done:
1) Remove all the EXPORT_SYMBOL() calls for functions that are not
accessed elsewhere.
2) Remove all the __visible tags as none of the functions will be
called from assembly code anymore.
3) Make all the internal functions static.
4) Remove some unneeded blank lines.
That enables the compiler to do better optimization and reduce code
size. The text+data size of rwsem.o on an x86-64 machine was reduced
from 8945 bytes to 4651 bytes with this change.
Suggested-by: Peter Zijlstra
Signed-off-by: Waiman Long
---
kernel/locking/rwsem.c | 50 +-
1 file changed, 10 insertions(+), 40 deletions(-)
diff --git a/kernel/locking/rwsem.c b/kernel/locking/rwsem.c
index 5f06b0601eb6..c1a089ab19fd 100644
--- a/kernel/locking/rwsem.c
+++ b/kernel/locking/rwsem.c
@@ -207,7 +207,6 @@ void __init_rwsem(struct rw_semaphore *sem, const char
*name,
osq_lock_init(>osq);
#endif
}
-
EXPORT_SYMBOL(__init_rwsem);
enum rwsem_waiter_type {
@@ -575,19 +574,17 @@ __rwsem_down_read_failed_common(struct rw_semaphore *sem,
int state)
return ERR_PTR(-EINTR);
}
-__visible struct rw_semaphore * __sched
+static inline 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);
-__visible struct rw_semaphore * __sched
+static inline struct rw_semaphore * __sched
rwsem_down_read_failed_killable(struct rw_semaphore *sem)
{
return __rwsem_down_read_failed_common(sem, TASK_KILLABLE);
}
-EXPORT_SYMBOL(rwsem_down_read_failed_killable);
/*
* Wait until we successfully acquire the write lock
@@ -694,26 +691,23 @@ __rwsem_down_write_failed_common(struct rw_semaphore
*sem, int state)
return ERR_PTR(-EINTR);
}
-__visible struct rw_semaphore * __sched
+static inline 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
+static inline 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
*/
-__visible
-struct rw_semaphore *rwsem_wake(struct rw_semaphore *sem)
+static struct rw_semaphore *rwsem_wake(struct rw_semaphore *sem)
{
unsigned long flags;
DEFINE_WAKE_Q(wake_q);
@@ -728,15 +722,13 @@ struct rw_semaphore *rwsem_wake(struct rw_semaphore *sem)
return sem;
}
-EXPORT_SYMBOL(rwsem_wake);
/*
* downgrade a write lock into a read lock
* - caller incremented waiting part of count and discovered it still negative
* - just wake up any readers at the front of the queue
*/
-__visible
-struct rw_semaphore *rwsem_downgrade_wake(struct rw_semaphore *sem)
+static struct rw_semaphore *rwsem_downgrade_wake(struct rw_semaphore *sem)
{
unsigned long flags;
DEFINE_WAKE_Q(wake_q);
@@ -751,7 +743,6 @@ struct rw_semaphore *rwsem_downgrade_wake(struct
rw_semaphore *sem)
return sem;
}
-EXPORT_SYMBOL(rwsem_downgrade_wake);
/*
* lock for reading
@@ -895,7 +886,6 @@ void __sched down_read(struct rw_semaphore *sem)
LOCK_CONTENDED(sem, __down_read_trylock, __down_read);
}
-
EXPORT_SYMBOL(down_read);
int __sched down_read_killable(struct rw_semaphore *sem)
@@ -910,7 +900,6 @@ int __sched down_read_killable(struct rw_semaphore *sem)
return 0;
}
-
EXPORT_SYMBOL(down_read_killable);
/*
@@ -924,7 +913,6 @@ int down_read_trylock(struct rw_semaphore *sem)
rwsem_acquire_read(>dep_map, 0, 1, _RET_IP_);
return ret;
}
-
EXPORT_SYMBOL(down_read_trylock);
/*
@@ -934,10 +922,8 @@ void __sched down_write(struct rw_semaphore *sem)
{
might_sleep();
rwsem_acquire(>dep_map, 0, 0, _RET_IP_);
-
LOCK_CONTENDED(sem, __down_write_trylock, __down_write);
}
-
EXPORT_SYMBOL(down_write);
/*
@@ -948,14 +934,14 @@ int __sched down_write_killable(struct rw_semaphore *sem)
might_sleep();
rwsem_acquire(>dep_map, 0, 0, _RET_IP_);
- if (LOCK_CONTENDED_RETURN(sem, __down_write_trylock,
__down_write_killable)) {
+ if (LOCK_CONTENDED_RETURN(sem, __down_write_trylock,
+ __down_write_killable)) {
rwsem_release(>dep_map, 1, _RET_IP_);
return -EINTR;
}
return 0;
}
-
EXPORT_SYMBOL(down_write_killable);
/*
@@ -970,7 +956,6 @@ int down_write_trylock(struct rw_semaphore *sem)
return
