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

Waiman Long
2019-04-19T19:22:52Z

On 04/19/2019 11:00 AM, Waiman Long wrote:
> On 04/19/2019 08:49 AM, Ingo Molnar wrote:
>> * Ingo Molnar <mingo@kernel.org> wrote:
>>
>>> * Waiman Long <longman@redhat.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;
}