From: Halil Pasic <pasic@linux.ibm.com>
To: "Petr Tesařík" <ptesarik@suse.cz>
Cc: Christian Borntraeger <borntraeger@de.ibm.com>,
Janosch Frank <frankja@linux.ibm.com>,
David Hildenbrand <david@redhat.com>,
Cornelia Huck <cohuck@redhat.com>,
Claudio Imbrenda <imbrenda@linux.ibm.com>,
Heiko Carstens <hca@linux.ibm.com>,
Vasily Gorbik <gor@linux.ibm.com>,
kvm@vger.kernel.org, linux-s390@vger.kernel.org,
linux-kernel@vger.kernel.org, stable@vger.kernel.org,
Michael Mueller <mimu@linux.ibm.com>,
Halil Pasic <pasic@linux.ibm.com>
Subject: Re: [PATCH 1/1] KVM: s390: index kvm->arch.idle_mask by vcpu_idx
Date: Mon, 31 Jan 2022 13:24:59 +0100 [thread overview]
Message-ID: <20220131132459.125e560c.pasic@linux.ibm.com> (raw)
In-Reply-To: <76a1c7b0-a073-02bb-1612-a74ca97105ec@suse.cz>
On Mon, 31 Jan 2022 13:09:26 +0100
Petr Tesařík <ptesarik@suse.cz> wrote:
> Hi Halil,
>
> Dne 31. 01. 22 v 12:53 Halil Pasic napsal(a):
> > On Mon, 31 Jan 2022 11:13:18 +0100
> > Petr Tesařík <ptesarik@suse.cz> wrote:
> >
> >> Hi Halil,
> >>
> >> Dne 27. 08. 21 v 14:54 Halil Pasic napsal(a):
> >>> While in practice vcpu->vcpu_idx == vcpu->vcp_id is often true,
> >>> it may not always be, and we must not rely on this.
> >>>
> >>> Currently kvm->arch.idle_mask is indexed by vcpu_id, which implies
> >>> that code like
> >>> for_each_set_bit(vcpu_id, kvm->arch.idle_mask, online_vcpus) {
> >>> vcpu = kvm_get_vcpu(kvm, vcpu_id);
> >>> do_stuff(vcpu);
> >>> }
> >>> is not legit. The trouble is, we do actually use kvm->arch.idle_mask
> >>> like this. To fix this problem we have two options. Either use
> >>> kvm_get_vcpu_by_id(vcpu_id), which would loop to find the right vcpu_id,
> >>> or switch to indexing via vcpu_idx. The latter is preferable for obvious
> >>> reasons.
> >>
> >> I'm just backporting this fix to SLES 12 SP5, and I've noticed that
> >> there is still this code in __floating_irq_kick():
> >>
> >> /* find idle VCPUs first, then round robin */
> >> sigcpu = find_first_bit(fi->idle_mask, online_vcpus);
> >> /* ... round robin loop removed ...
> >> dst_vcpu = kvm_get_vcpu(kvm, sigcpu);
> >>
> >> It seems to me that this does exactly the thing that is not legit, but
> >> I'm no expert. Did I miss something?
> >>
> >
> > We made that legit by making the N-th bit in idle_mask correspond to the
> > vcpu whose vcpu_idx == N. The second argument of kvm_get_vcpu() is the
> > vcpu_idx. IMHO that ain't super-intuitive because it ain't spelled out.
> >
> > So before this was a mismatch (with a vcpu_id based bitmap we would have
> > to use kvm_get_vcpu_by_id()), and with this patch applied this code
> > becomes legit because both idle_mask and kvm_get_vcpu() operate with
> > vcpu_idx.
> >
> > Does that make sense?
>
> Yes!
>
> > I'm sorry the commit message did not convey this clearly enough...
>
> No, it's not your fault. I didn't pay enough attention to the change,
> and with vcpu_id and vcpu_idx being so similar I got confused.
No problem at all!
>
> In short, there's no bug now, indeed. Thanks for your patience.
>
Thank you for being mindful when backporting!
Regards,
Halil
prev parent reply other threads:[~2022-01-31 12:25 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-08-27 12:54 [PATCH 1/1] KVM: s390: index kvm->arch.idle_mask by vcpu_idx Halil Pasic
2021-08-27 13:42 ` Christian Borntraeger
2021-08-27 14:06 ` Claudio Imbrenda
2021-08-27 16:36 ` Christian Borntraeger
2021-08-27 21:23 ` Halil Pasic
2021-08-29 6:25 ` Christian Borntraeger
2021-08-27 21:19 ` Halil Pasic
2021-08-28 11:04 ` Michael Mueller
2022-01-31 10:13 ` Petr Tesařík
2022-01-31 11:53 ` Halil Pasic
2022-01-31 12:09 ` Petr Tesařík
2022-01-31 12:24 ` Halil Pasic [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20220131132459.125e560c.pasic@linux.ibm.com \
--to=pasic@linux.ibm.com \
--cc=borntraeger@de.ibm.com \
--cc=cohuck@redhat.com \
--cc=david@redhat.com \
--cc=frankja@linux.ibm.com \
--cc=gor@linux.ibm.com \
--cc=hca@linux.ibm.com \
--cc=imbrenda@linux.ibm.com \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-s390@vger.kernel.org \
--cc=mimu@linux.ibm.com \
--cc=ptesarik@suse.cz \
--cc=stable@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).