Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
In-Reply-To: <20171116045407.GF14563@u40b0340c692b58f6553c.ant.amazon.com>
References: <1509999980-10828-1-git-send-email-eduval@amazon.com>
 <20171108173651.GA3664@flask> <20171109085547.GA5107@u40b0340c692b58f6553c.ant.amazon.com>
 <20171109141732.GA20859@flask> <20171116045407.GF14563@u40b0340c692b58f6553c.ant.amazon.com>
From:   Wanpeng Li <kernellwp@gmail.com>
Date:   Mon, 27 Nov 2017 08:43:39 +0800
Message-ID: <CANRm+CwY_fiDk95g9Jf9ty1cUDocE6=wE3Fi2v7F2JQ5VFvJqg@mail.gmail.com>
Subject: Re: [PATCHv3 1/1] locking/qspinlock/x86: Avoid test-and-set when
 PV_DEDICATED is set
To:     Eduardo Valentin <eduval@amazon.com>
Cc:     =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= <rkrcmar@redhat.com>,
        Paolo Bonzini <pbonzini@redhat.com>,
        Matt Wilson <msw@amazon.com>, Jonathan Corbet <corbet@lwn.net>,
        Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>,
        "H. Peter Anvin" <hpa@zytor.com>,
        "the arch/x86 maintainers" <x86@kernel.org>,
        Peter Zijlstra <peterz@infradead.org>,
        Waiman Long <longman@redhat.com>, kvm <kvm@vger.kernel.org>,
        linux-doc@vger.kernel.org,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "Jan H . Schoenherr" <jschoenh@amazon.de>,
        Anthony Liguori <aliguori@amazon.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi Eduardo,
2017-11-16 12:54 GMT+08:00 Eduardo Valentin <eduval@amazon.com>:
> Hey Radim,
>
> On Thu, Nov 09, 2017 at 03:17:33PM +0100, Radim Kr=C4=8Dm=C3=A1=C5=99 wro=
te:
>
> <cut>
>
>>
>> This is what I'm doubting, because the patch is adding about two
>> thousand cycles to every spinlock-taken path.
>> Doesn't this patch yield better results?
>>
>> diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
>> index 3df743b60c80..d9225e48c11a 100644
>> --- a/arch/x86/kernel/kvm.c
>> +++ b/arch/x86/kernel/kvm.c
>> @@ -676,6 +676,12 @@ void __init kvm_spinlock_init(void)
>>  {
>>       if (!kvm_para_available())
>>               return;
>> +
>> +     if (kvm_para_has_feature(KVM_FEATURE_PV_DEDICATED)) {
>> +             static_branch_disable(&virt_spin_lock_key);
>> +             return;
>> +     }
>> +
>
> Yes, the above suggestion is a much better approach. The code has probabl=
y changed from the time I wrote the first version. I will refresh with the =
above suggestion.

Do you mind to send a new version since the merge window is closed?

Regards,
Wanpeng Li

>
>
>>       /* Does host kernel support KVM_FEATURE_PV_UNHALT? */
>>       if (!kvm_para_has_feature(KVM_FEATURE_PV_UNHALT))
>>               return;
>>
>> >                                                              However, =
the key aspect
>> > here is this patch gives a way for the host to instruct the guest to u=
se qspinlock.
>> > Even with Longman's patch which allows guest to select the spinlock im=
plementation,
>> > there should still be the auto-select mode. In such mode, PV_DEDICATED=
 should
>> > allow the host to get the guest to use qspinlock, without, the guest w=
ill fallback
>> > to tas when PV_UNHALT =3D=3D 0.
>>
>> I agree that a flag can be useful for certains setups.
>
> Cool!
>
>>
>
> --
> All the best,
> Eduardo Valentin

From 1584205308566971919@xxx Thu Nov 16 07:05:14 +0000 2017
X-GM-THRID: 1583352031107436177
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread