linux-kernel - Re: [PATCH 1/1] sched/cputime: do not decrease steal time after live migration on xen

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Wed, 11 Oct 2017 15:47:44 +0800
From:   Dongli Zhang <dongli.zhang@...cle.com>
To:     Rik van Riel <riel@...hat.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Stanislaw Gruszka <sgruszka@...hat.com>
Cc:     Ingo Molnar <mingo@...nel.org>,
        Wanpeng Li <wanpeng.li@...mail.com>,
        Xiaolong Ye <xiaolong.ye@...el.com>,
        Frederic Weisbecker <fweisbec@...il.com>,
        linux-kernel@...r.kernel.org, xen-devel@...ts.xen.org,
        mingo@...hat.com, dario.faggioli@...rix.com, bevan@...co.net,
        xen.list@...vel.fr, joao.m.martins@...cle.com
Subject: Re: [PATCH 1/1] sched/cputime: do not decrease steal time after live
 migration on xen

Hi Rik,

On 10/10/2017 10:01 PM, Rik van Riel wrote:
> On Tue, 2017-10-10 at 14:48 +0200, Peter Zijlstra wrote:
>> On Tue, Oct 10, 2017 at 02:42:01PM +0200, Stanislaw Gruszka wrote:
>>>>> +		u64 steal, steal_time;
>>>>> +		s64 steal_delta;
>>>>> +
>>>>> +		steal_time =
>>>>> paravirt_steal_clock(smp_processor_id());
>>>>> +		steal = steal_delta = steal_time - this_rq()-
>>>>>> prev_steal_time;
>>>>> +
>>>>> +		if (unlikely(steal_delta < 0)) {
>>>>> +			this_rq()->prev_steal_time =
>>>>> steal_time;
>>>
>>> I don't think setting prev_steal_time to smaller value is right
>>> thing to do. 
>>>
>>> Beside, I don't think we need to check for overflow condition for
>>> cputime variables (it will happen after 279 years :-). So instead
>>> of introducing signed steal_delta variable I would just add
>>> below check, which should be sufficient to fix the problem:
>>>
>>> 	if (unlikely(steal <= this_rq()->prev_steal_time))
>>> 		return 0;
>>
>> How about you just fix up paravirt_steal_time() on migration and not
>> muck with the users ?
> 
> Not just migration, either. CPU hotplug is another time to fix up
> the steal time.

I think this issue might be hit when we add and online vcpu after a very very
long time since boot (or the last time vcpu is offline). Please correct me if I
am wrong.

Thank you very much!

Dongli Zhang

>