linux-kernel - Re: [PATCH v5 00/18] locking/rwsem: Rwsem rearchitecture part 2

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

Date:   Fri, 19 Apr 2019 12:56:22 -0400
From:   Waiman Long <longman@...hat.com>
To:     Ingo Molnar <mingo@...nel.org>
Cc:     Peter Zijlstra <peterz@...radead.org>,
        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 v5 00/18] locking/rwsem: Rwsem rearchitecture part 2

On 04/19/2019 11:00 AM, Waiman Long wrote:
> On 04/19/2019 08:49 AM, Ingo Molnar wrote:
>> * Ingo Molnar <mingo@...nel.org> wrote:
>>
>>> * Waiman Long <longman@...hat.com> wrote:
>>>
>>>> On 04/18/2019 07:46 PM, Waiman Long wrote:
>>>>>  v5:
>>>>>   - Drop v4 patch 1 as it is merged into tip's locking/core branch.
>>>>>   - Integrate the 2 followup patches into the series. The first
>>>>>     follow-up patch is broken into 2 pieces. The first piece comes in
>>>>>     before the "Enable readers spinning on writer" and the 2nd piece
>>>>>     is merged into the "Enable time-based spinning on reader-owned
>>>>>     rwsem" patch. The 2nd followup patch is added after that.
>>>>>   - Add a new patch to make all wake_up_q() calls after dropping
>>>>>     wait_lock as suggested by PeterZ.
>>>>>   - Incorporate numerouos suggestions by PeterZ and Davidlohr.
>>>> This patchset is still being reviewed by Peter . The purpose of this
>>>> series is mainly to sync up the version that Peter has and the ones that
>>>> I am working on incorporating his feedback. Further changes may still be
>>>> needed.
>>>>
>>>> I run an overall performance test on this new patchset and present the
>>>> data in this cover letter. However, I haven't run performance tests for
>>>> individual patches. So the performance data listed in some of the
>>>> patches may be stale.
>>> Just for those who'd like to follow the scope of changes, find below the 
>>> v4->v5 interdiff. v5 is now included in tip:WIP.locking/core, and also 
>>> merged into tip:master. (But not propagated towards linux-next yet.)
>> Hm, I'm experiencing early boot hangs with v5, on defconfig-ish x86-64 
>> kernels:
>>
>> [    0.153940] rcu: Hierarchical RCU implementation.
>> [    0.154289] rcu: 	RCU restricting CPUs from NR_CPUS=128 to nr_cpu_ids=17.
>> [    0.154829] rcu: RCU calculated value of scheduler-enlistment delay is 100 jiffies.
>> [    0.155390] rcu: Adjusting geometry for rcu_fanout_leaf=16, nr_cpu_ids=17
>>
>>
>> I bisected it back to the v5 version of this patch:
>>
>>   2fd5f60fa4c3: locking/rwsem: Merge owner into count on x86-64
>>
>> I'm moving -tip back to -v4 meanwhile.
>>
>> Thanks,
>>
>> 	Ingo
> Sorry about that. Will look into that problem.
>
> -Longman
>
Yes, there is a bug in that patch. The following change should fix it:

diff --git a/kernel/locking/rwsem.c b/kernel/locking/rwsem.c
index 19d8fbd50d17..857dff330f9b 100644
--- a/kernel/locking/rwsem.c
+++ b/kernel/locking/rwsem.c
@@ -198,9 +198,15 @@
 /*
  * Task structure pointer compression (64-bit only):
  * (owner - PAGE_OFFSET) >> (L1_CACHE_SHIFT - 2)
+ *
+ * However, init_task may lie outside of the linearly mapped physical
+ * to virtual memory range and so has to be handled separately.
  */
 static inline unsigned long rwsem_owner_count(struct task_struct *owner)
 {
+       if (unlikely(owner == &init_task))
+               return RWSEM_WRITER_MASK;
+
        return ((unsigned long)owner - PAGE_OFFSET) >> (L1_CACHE_SHIFT - 2);
 }

@@ -208,6 +214,9 @@ static inline unsigned long rwsem_count_owner(long
count)
 {
        unsigned long writer = (unsigned long)count & RWSEM_WRITER_MASK;

+       if (unlikely(writer == RWSEM_WRITER_MASK))
+               return (unsigned long)&init_task;
+
        return writer ? (writer << (L1_CACHE_SHIFT - 2)) + PAGE_OFFSET : 0;
 }