lkml.org 
[lkml]   [2022]   [Sep]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v5 06/18] rcu: Introduce call_rcu_lazy() API implementation
From
Hi Frederick,

On 9/5/2022 8:59 AM, Frederic Weisbecker wrote:
>>> Also that's a subtle change which purpose isn't explained. It means that
>>> rcu_barrier_entrain() used to wait for the bypass timer in the worst case
>>> but now we force rcuog into it immediately. Should that be a separate change?
>> It could be split, but it is laziness that amplifies the issue so I thought of
>> keeping it in the same patch. I don't mind one way or the other.
> Ok then lets keep it here but please add a comment for the reason to
> force wake here.

Ok will do, thanks.

>>>> + case RCU_NOCB_WAKE_BYPASS:
>>>> + mod_jif = 2;
>>>> + break;
>>>> +
>>>> + case RCU_NOCB_WAKE:
>>>> + case RCU_NOCB_WAKE_FORCE:
>>>> + // For these, make it wake up the soonest if we
>>>> + // were in a bypass or lazy sleep before.
>>>> if (rdp_gp->nocb_defer_wakeup < RCU_NOCB_WAKE)
>>>> - mod_timer(&rdp_gp->nocb_timer, jiffies + 1);
>>>> - if (rdp_gp->nocb_defer_wakeup < waketype)
>>>> - WRITE_ONCE(rdp_gp->nocb_defer_wakeup, waketype);
>>>> + mod_jif = 1;
>>>> + break;
>>>> }
>>>>
>>>> + if (mod_jif)
>>>> + mod_timer(&rdp_gp->nocb_timer, jiffies + mod_jif);
>>>> +
>>>> + if (rdp_gp->nocb_defer_wakeup < waketype)
>>>> + WRITE_ONCE(rdp_gp->nocb_defer_wakeup, waketype);
>>> So RCU_NOCB_WAKE_BYPASS and RCU_NOCB_WAKE_LAZY don't override the timer state
>>> anymore? Looks like something is missing.
>> My goal was to make sure that NOCB_WAKE_LAZY wake keeps the timer lazy. If I
>> don't do this, then when CPU enters idle, it will immediately do a wake up via
>> this call:
>>
>> rcu_nocb_need_deferred_wakeup(rdp_gp, RCU_NOCB_WAKE)
> But if the timer is in RCU_NOCB_WAKE_LAZY mode, that shouldn't be a problem.
>
>> That was almost always causing lazy CBs to be non-lazy thus negating all the
>> benefits.
>>
>> Note that bypass will also have same issue where the bypass CB will not wait for
>> intended bypass duration. To make it consistent with lazy, I made bypass also
>> not override nocb_defer_wakeup.
> I'm surprised because rcu_nocb_flush_deferred_wakeup() should only do the wake up
> if the timer is RCU_NOCB_WAKE or RCU_NOCB_WAKE_FORCE. Or is that code buggy
> somehow?
> Actually your change is modifying the timer delay without changing the timer
> mode, which may shortcut rcu_nocb_flush_deferred_wakeup() check and actually
> make it perform early upon idle loop entry.
>
> Or am I missing something?
>

You could very well have a point and I am not sure now (I happen to 'forget' the
issue once the code was working). I distinctly remember not being able to be
lazy without doing this. Maybe there is some other path. I am kicking myself for
not commenting in the code or change log enough about the issue.

I will test again sync'ing the lazy timer and the ->nocb_defer_wakeup field
properly and see if I can trigger the issue.

Thanks!

- Joel







\
 
 \ /
  Last update: 2022-09-05 22:19    [W:0.135 / U:0.448 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site