Re: [RFC] Make need_resched() return true when rcu_urgent_qs requested

[Christian Borntraeger <borntraeger@de.ibm.com>]

On 07/18/2018 10:17 PM, Paul E. McKenney wrote:
> On Wed, Jul 18, 2018 at 09:41:05PM +0200, David Woodhouse wrote:
>>
>>
>> On Wed, 2018-07-18 at 09:37 -0700, Paul E. McKenney wrote:
>>> On Wed, Jul 18, 2018 at 06:01:51PM +0200, David Woodhouse wrote:
>>>>
>>>> On Wed, 2018-07-18 at 08:36 -0700, Paul E. McKenney wrote:
>>>>>
>>>>> And I finally did get some near misses from an earlier commit, so we
>>>>> should consider your patch to be officially off the hook.
>>>>
>>>> Yay, I like it when it's not my fault. I'll redo it with the ifdef
>>>> CONFIG_NO_HZ_FULL.
>>>
>>> Hey, I didn't say it wasn't your fault, only that it -officially- wasn't
>>> your fault.  ;-)
>>
>> I can live with being innocent until proven guilty.
>>
>>>>
>>>> What should it do for the !CONFIG_NO_HZ_FULL case? The existing call in
>>>> guest_enter_irqoff() clearly wasn't actually doing the right thing
>>>> anyway, hence the need for the need_resched() patch in $SUBJECT... so
>>>> should I just leave it doing nothing in guest_enter_irqoff()? 
>>>
>>> One starting point would be the combination of your patch and my
>>> patch, with -rcu commit IDs and diff below.  But yes, it needs to be
>>> !CONFIG_NO_HZ_FULL.  And no, I am not at all confident that I actually
>>> found all the places needing change in the core code, so this needs some
>>> serious review both by the KVM guys and the NO_HZ_FULL guys.
>>
>> Right, that looks fairly much like the version I'd ended up with. So my
>> question was...
>>
>>> --- a/include/linux/context_tracking.h
>>> +++ b/include/linux/context_tracking.h
>>> @@ -118,12 +118,12 @@ static inline void guest_enter_irqoff(void)
>>>  	 * one time slice). Lets treat guest mode as quiescent state, just like
>>>  	 * we do with user-mode execution.
>>>  	 */
>>
>> ...if we change this to something like...
>>
>> #ifdef CONFIG_NO_HZ_FULL
>>> +	rcu_kvm_enter();
>> #else
>>> 	if (!context_tracking_cpu_is_enabled())
>>> 		rcu_virt_note_context_switch(smp_processor_id());
>> #endif
>>
>> ... do you actually want me to keep the #else case there? It blatantly
>> wasn't working anyway for us, perhaps because the condition was false?
>> That's why I started fixing need_resched() in the first place, and that
>> fix ought to cover whatever this call to rcu_virt_note_context_switch()
>> was supposed to be doing?
> 
> My thought would be something like this:
> 
> 	if (context_tracking_cpu_is_enabled())
> 		rcu_kvm_enter();
> 	else
> 		rcu_virt_note_context_switch(smp_processor_id());

In the past we needed that (when we introduced that). At least with every
host interrupt we called this making an rcu event at least every HZ.
Will the changes in need_resched make this part unnecessary?