From: "Xu, Yanfei" <yanfei.xu@windriver.com>
To: paulmck@kernel.org
Cc: rcu@vger.kernel.org, LKML <linux-kernel@vger.kernel.org>
Subject: Re: [Qestion] Is preempt_disable/enable needed in non-preemption code path
Date: Fri, 16 Apr 2021 18:51:10 +0800 [thread overview]
Message-ID: <3ef49985-68c9-277d-648c-53447ff602f4@windriver.com> (raw)
In-Reply-To: <20210415170748.GA4212@paulmck-ThinkPad-P17-Gen-1>
On 4/16/21 1:07 AM, Paul E. McKenney wrote:
> [Please note: This e-mail is from an EXTERNAL e-mail address]
>
> On Fri, Apr 16, 2021 at 12:18:42AM +0800, 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?
>
> Maybe.
>
> Please keep in mind that although the compiler is prohibited from
> reordering volatile accesses with each other, there is nothing stopping
> it from reordering volatile accesses with non-volatile accesses.
Thanks for your patient explanation!
I am trying to absorb what you said. Blow are my understanding:
1. "the compiler is prohibited from reordering volatile accesses with
each other" means these situations:
int a;
foo()
{
for(;;)
READ_ONCE(a);
}
or
int a,b;
foo()
{
int c,d;
c = READ_ONCE(a);
d = READ_ONCE(b);
}
2. "volatile accesses with non-volatile accesses" means d=b may happen
before c=READ_ONCE(a) :
int a;
foo()
{
int b = 2
int c,d;
c = READ_ONCE(a);
d = b;
}
if we want to keep the ordering of volatile access "c=READ_ONCE(a)" and
non-volatile access "d=b", we should use stronger barrier like barrier().
Hope I didn't misunderstand.
Back to rcu_blocking_is_gp(), I find this link today
https://www.spinics.net/lists/rcu/msg03985.html
With the content in this link, I still haven't got the meaning of these
two barrier(). I think I should learn knowledge about cpu-hotplug and
things which talked in the link first to make sure if I am missing
something, and then consult you. :)
Best regards,
Yanfei
>
> Thanx, Paul
>
>> 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
next prev parent reply other threads:[~2021-04-16 10:51 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-04-15 15:04 [Qestion] Is preempt_disable/enable needed in non-preemption code path Xu, Yanfei
2021-04-15 15:43 ` Paul E. McKenney
2021-04-15 16:18 ` Xu, Yanfei
2021-04-15 17:01 ` Xu, Yanfei
2021-04-15 17:15 ` Paul E. McKenney
2021-04-15 17:07 ` Paul E. McKenney
2021-04-16 10:51 ` Xu, Yanfei [this message]
2021-04-16 17:26 ` Paul E. McKenney
2021-04-19 4:58 ` Xu, Yanfei
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=3ef49985-68c9-277d-648c-53447ff602f4@windriver.com \
--to=yanfei.xu@windriver.com \
--cc=linux-kernel@vger.kernel.org \
--cc=paulmck@kernel.org \
--cc=rcu@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).