From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752160AbdJFOkP (ORCPT ); Fri, 6 Oct 2017 10:40:15 -0400 Received: from mail-oi0-f49.google.com ([209.85.218.49]:53008 "EHLO mail-oi0-f49.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751532AbdJFOkN (ORCPT ); Fri, 6 Oct 2017 10:40:13 -0400 X-Google-Smtp-Source: AOwi7QDi802ElSKpuAd104iFkVnGJvxLHjZbFNETfXZdwDmswJOOiDBowt3sVpNI4psH3mCV57cyiJfPN3Wprn1r8QM= MIME-Version: 1.0 In-Reply-To: <1ab302d3-a7d6-fc70-1857-aec556d3f55c@redhat.com> References: <1507298492-8300-1-git-send-email-wanpeng.li@hotmail.com> <1ab302d3-a7d6-fc70-1857-aec556d3f55c@redhat.com> From: Wanpeng Li Date: Fri, 6 Oct 2017 22:40:12 +0800 Message-ID: Subject: Re: [PATCH v7] KVM: LAPIC: Apply change to TDCR right away to the timer To: Paolo Bonzini Cc: "linux-kernel@vger.kernel.org" , kvm , =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= , Wanpeng Li Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 2017-10-06 22:21 GMT+08:00 Paolo Bonzini : > On 06/10/2017 16:01, Wanpeng Li wrote: >> + if (!apic->lapic_timer.period) >> + return; >> + >> + now = ktime_get(); >> + remaining = ktime_sub(apic->lapic_timer.target_expiration, now); >> + if (ktime_to_ns(remaining) < 0) >> + remaining = 0; >> + delta = mod_64(ktime_to_ns(remaining), apic->lapic_timer.period); > > I think this shouldn't be happening. If it does, I'm not sure the mod > is the right thing to do, so I'd just use ktime_to_ns(remaining). > > So perhaps let's simplify all this to: > > ns_remaining_old = ktime_to_ns(remaining); > ns_remaining_new = mul_u64_u32_div(ns_remaining_old, > apic->divide_count, old_divisor); > > because below you're calling nsec_to_cycles but remaining is not expressed > in nanoseconds. > >> + if (!delta) >> + return; >> + >> + delta = delta * apic->divide_count / old_divisor; >> + >> + limit_periodic_timer_frequency(apic); > > This should be done before all the "if"s (which should not be there in v8, > but you should still call it before "now = ktime_get();"). Just sent out v8 to handle all the comments. Regards, Wanpeng Li > > Paolo > >> + apic->lapic_timer.tscdeadline += nsec_to_cycles(apic->vcpu, delta) - >> + nsec_to_cycles(apic->vcpu, remaining); >> + apic->lapic_timer.target_expiration = ktime_add_ns(now, delta); >> +} >