Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Subject: Re: [PATCH] locking/osq_lock: Fix false sharing of
 optimistic_spin_node in osq_lock
To:     Waiman Long <longman@redhat.com>, peterz@infradead.org,
        mingo@redhat.com, will@kernel.org, boqun.feng@gmail.com
Cc:     linux-kernel@vger.kernel.org, wangxiaohua@uniontech.com
References: <20230615012437.21087-1-guohui@uniontech.com>
 <9f5321f9-0b35-7f85-ee09-247d961e5a79@redhat.com>
 <b8ec5185-f167-0b11-b823-51ba4fc3ca4f@redhat.com>
From:   Guo Hui <guohui@uniontech.com>
Message-ID: <96DD7021A77A45D4+430029ca-38ee-204a-4ee5-a5246b99bb62@uniontech.com>
Date:   Thu, 15 Jun 2023 16:27:41 +0800
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.12.0
MIME-Version: 1.0
In-Reply-To: <b8ec5185-f167-0b11-b823-51ba4fc3ca4f@redhat.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 8bit
Content-Language: en-US
Feedback-ID: bizesmtp:uniontech.com:qybglogicsvrsz:qybglogicsvrsz4a-0
Precedence: bulk


On 6/15/23 10:57 AM, Waiman Long wrote:
>
> On 6/14/23 21:58, Waiman Long wrote:
>>> diff --git a/kernel/locking/osq_lock.c b/kernel/locking/osq_lock.c
>>> index d5610ad52b92..bdcd216b73c4 100644
>>> --- a/kernel/locking/osq_lock.c
>>> +++ b/kernel/locking/osq_lock.c
>>> @@ -24,7 +24,7 @@ static inline int encode_cpu(int cpu_nr)
>>>     static inline int node_cpu(struct optimistic_spin_node *node)
>>>   {
>>> -    return node->cpu - 1;
>>> +    return node->prev_cpu - 1;
>>>   }
>>
>> From the first glance, the patch looks OK. The only issue that I have 
>> so far is the "node_cpu" name which is misleading in this case. You 
>> should have renamed it to node_prev_cpu() instead as there is only 
>> one caller.
>
> One more thing. Unlike node->cpu, node->prev_cpu is subjected to 
> change. So you should have use READ_ONCE() to read it to match the 
> WRITE_ONCE() that modifies it.
>
> Thanks,
> Longman
>
>

Ok thanks, I got it.