lkml.org 
[lkml]   [2021]   [Apr]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
SubjectRe: [Qestion] Is preempt_disable/enable needed in non-preemption code path
From
Date


On 4/16/21 12:18 AM, Xu, Yanfei wrote:
>
>
> On 4/15/21 11:43 PM, Paul E. McKenney wrote:
>> [Please note: This e-mail is from an EXTERNAL e-mail address]
>>
>> On Thu, Apr 15, 2021 at 11:04:05PM +0800, Xu, Yanfei wrote:
>>> Hi experts,
>>>
>>> I am learning rcu mechanism and its codes. When looking at the
>>> rcu_blocking_is_gp(), I found there is a pair preemption disable/enable
>>> operation in non-preemption code path. And it has been a long time. I
>>> can't
>>> understand why we need it? Is there some thing I missed? If not, can we
>>> remove the unnecessary operation like blow?
>>
>> Good point, you are right that preemption is disabled anyway in that
>> block
>> of code.  However, preempt_disable() and preempt_enable() also prevent
>> the
>> compiler from moving that READ_ONCE() around.  So my question to you is
>> whether it is safe to remove those statements entirely or whether they
>> should instead be replaced by barrier() or similar.
>
> Thanks for your reply! :)
>
> Yes, preempt_disable() and preempt_enable() defined in !preemption are
> barrier(). barrier can prevent from reordering that READ_ONCE(), but
> base on my current understanding, volatile in READ_ONCE can also tell
> the compiler not to reorder it. So, I think it's safe?
>
> Best regards,
> Yanfei

Hi Paul,
I objdump the function rcu_blocking_is_gp():

after dropping the barrier():
ffffffff81107c50 <rcu_blocking_is_gp>:
ffffffff81107c50: e8 7b 2a f5 ff callq ffffffff8105a6d0
<__fentry__>
ffffffff81107c55: 8b 05 41 fe 7c 01 mov
0x17cfe41(%rip),%eax # ffffffff828d7a9c <rcu_state+0x221c>
ffffffff81107c5b: 55 push %rbp
ffffffff81107c5c: 48 89 e5 mov %rsp,%rbp
ffffffff81107c5f: 5d pop %rbp
ffffffff81107c60: 83 f8 01 cmp $0x1,%eax
ffffffff81107c63: 0f 9e c0 setle %al
ffffffff81107c66: 0f b6 c0 movzbl %al,%eax
ffffffff81107c69: c3 retq
ffffffff81107c6a: 66 0f 1f 44 00 00 nopw 0x0(%rax,%rax,1)

the original codes:
ffffffff81107ba0 <rcu_blocking_is_gp>:
ffffffff81107ba0: e8 2b 2b f5 ff callq ffffffff8105a6d0
<__fentry__>
ffffffff81107ba5: 55 push %rbp
ffffffff81107ba6: 48 89 e5 mov %rsp,%rbp
ffffffff81107ba9: 8b 05 ed fe 7c 01 mov
0x17cfeed(%rip),%eax # ffffffff828d7a9c <rcu_state+0x221c>
ffffffff81107baf: 83 f8 01 cmp $0x1,%eax
ffffffff81107bb2: 5d pop %rbp
ffffffff81107bb3: 0f 9e c0 setle %al
ffffffff81107bb6: 0f b6 c0 movzbl %al,%eax
ffffffff81107bb9: c3 retq
ffffffff81107bba: 66 0f 1f 44 00 00 nopw 0x0(%rax,%rax,1)

umm... It did been reordered by compiler after dropping the barrier(),
however, I think the result will not be effected. Right?

Best regards,
Yanfei

>
>>
>>                                                          Thanx, Paul
>>
>>> diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
>>> index da6f5213fb74..c6d95a00715e 100644
>>> --- a/kernel/rcu/tree.c
>>> +++ b/kernel/rcu/tree.c
>>> @@ -3703,7 +3703,6 @@ static int rcu_blocking_is_gp(void)
>>>          if (IS_ENABLED(CONFIG_PREEMPTION))
>>>                  return rcu_scheduler_active == RCU_SCHEDULER_INACTIVE;
>>>          might_sleep();  /* Check for RCU read-side critical section. */
>>> -       preempt_disable();
>>>          /*
>>>           * If the rcu_state.n_online_cpus counter is equal to one,
>>>           * there is only one CPU, and that CPU sees all prior accesses
>>> @@ -3718,7 +3717,6 @@ static int rcu_blocking_is_gp(void)
>>>           * Those memory barriers are provided by CPU-hotplug code.
>>>           */
>>>          ret = READ_ONCE(rcu_state.n_online_cpus) <= 1;
>>> -       preempt_enable();
>>>          return ret;
>>>   }
>>>
>>>
>>>
>>> Best regards,
>>> Yanfei

\
 
 \ /
  Last update: 2021-04-15 19:03    [W:0.175 / U:0.112 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site