Re: Null scheduler and vwfi native problem

["Jürgen Groß" <jgross@suse.com>]

[-- Attachment #1.1.1: Type: text/plain, Size: 2611 bytes --]

On 03.02.21 12:20, Julien Grall wrote:
> Hi Juergen,
> 
> On 03/02/2021 11:00, Jürgen Groß wrote:
>> On 03.02.21 10:19, Julien Grall wrote:
>>> Hi,
>>>
>>> On 03/02/2021 07:31, Dario Faggioli wrote:
>>>> On Tue, 2021-02-02 at 15:23 +0000, Julien Grall wrote:
>>>>> In reality, it is probably still too early as a pCPU can be
>>>>> considered
>>>>> quiesced until a call to rcu_lock*() (such rcu_lock_domain()).
>>>>>
>>>> Well, yes, in theory, we could track down which is the first RCU read
>>>> side crit. section on this path, and put the call right before that (if
>>>> I understood what you mean).
>>>
>>> Oh, that's not what I meant. This will indeed be far more complex 
>>> than I originally had in mind.
>>>
>>> AFAIU, the RCU uses critical section to protect data. So the 
>>> "entering" could be used as "the pCPU is not quiesced" and "exiting" 
>>> could be used as "the pCPU is quiesced".
>>>
>>> The concern with my approach is we would need to make sure that Xen 
>>> correctly uses the rcu helpers. I know Juergen worked on that 
>>> recently, but I don't know whether this is fully complete.
>>
>> I think it is complete, but I can't be sure, of course.
>>
>> One bit missing (for catching some wrong uses of the helpers) is this
>> patch:
>>
>> https://lists.xen.org/archives/html/xen-devel/2020-03/msg01759.html
>>
>> I don't remember why it hasn't been taken, but I think there was a
>> specific reason for that.
> 
> Looking at v8 and the patch is suitably reviewed by Jan. So I am a bit 
> puzzled to why this wasn't committed... I had to go to v6 and notice the 
> following message:
> 
> "albeit to be honest I'm not fully convinced we need to go this far."
> 
> Was the implication that his reviewed-by was conditional to someone else 
> answering to the e-mail?

I have no record for that being the case.

Patches 1-3 of that series were needed for getting rid of
stop_machine_run() in rcu handling and to fix other problems. Patch 4
was adding some additional ASSERT()s for making sure no potential
deadlocks due to wrong rcu usage could creep in again.

Patch 5 was more a "nice to have" addition in order to avoid any
wrong usage of rcu which should have no real negative impact on the
system stability.

So I believe Jan as the committer didn't want to commit it himself, but
was fine with the overall idea and implementation.

I still think for code sanity it would be nice, but I was rather busy
with Xenstore and event channel security work at that time, so I didn't
urge anyone to take this patch.


Juergen

[-- Attachment #1.1.2: OpenPGP_0xB0DE9DD628BF132F.asc --]
[-- Type: application/pgp-keys, Size: 3135 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]