| Message ID | 1617052502-14181-3-git-send-email-boris.ostrovsky@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Performance regression due to XSA-336 | expand |
On Mon, Mar 29, 2021 at 05:15:02PM -0400, Boris Ostrovsky wrote: > Make both create_periodic_time() and pt_adjust_vcpu() call > write_{un}lock with similar arguments. > Might be worth adding that this is not a functional change? > Requested-by: Jan Beulich <jbeulich@suse.com> > Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com> Either way: Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> > --- > New patch. > > I ended up doing what Jan asked --- create_periodic_time() is also using different > start pointers in lock() and unlock(). Hm, I'm not sure I'm following, create_periodic_time uses 'v' in both write_{un}lock calls, which doesn't change across the function. Thanks, Roger.
On 30.03.2021 09:36, Roger Pau Monné wrote: > On Mon, Mar 29, 2021 at 05:15:02PM -0400, Boris Ostrovsky wrote: >> Make both create_periodic_time() and pt_adjust_vcpu() call >> write_{un}lock with similar arguments. This makes it sound like you adjust both functions, but really you bring the latter in line with the former. Would you mind me adjusting the wording along these lines while (and when) committing? > Might be worth adding that this is not a functional change? > >> Requested-by: Jan Beulich <jbeulich@suse.com> >> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com> > > Either way: > > Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> > >> --- >> New patch. >> >> I ended up doing what Jan asked --- create_periodic_time() is also using different >> start pointers in lock() and unlock(). > > Hm, I'm not sure I'm following, create_periodic_time uses 'v' in both > write_{un}lock calls, which doesn't change across the function. I guess Boris merely meant to express that there's already precedent? Jan
On 3/30/21 8:49 AM, Jan Beulich wrote: > On 30.03.2021 09:36, Roger Pau Monné wrote: >> On Mon, Mar 29, 2021 at 05:15:02PM -0400, Boris Ostrovsky wrote: >>> Make both create_periodic_time() and pt_adjust_vcpu() call >>> write_{un}lock with similar arguments. > This makes it sound like you adjust both functions, but really > you bring the latter in line with the former. Would you mind me > adjusting the wording along these lines while (and when) > committing? Yes, please. > >> Might be worth adding that this is not a functional change? >> >>> Requested-by: Jan Beulich <jbeulich@suse.com> >>> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com> >> Either way: >> >> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> >> >>> --- >>> New patch. >>> >>> I ended up doing what Jan asked --- create_periodic_time() is also using different >>> start pointers in lock() and unlock(). >> Hm, I'm not sure I'm following, create_periodic_time uses 'v' in both >> write_{un}lock calls, which doesn't change across the function. > I guess Boris merely meant to express that there's already precedent? Yes, that's what I wanted to say. But clearly not what I actually said. -boris
diff --git a/xen/arch/x86/hvm/vpt.c b/xen/arch/x86/hvm/vpt.c index 560fab9cfc60..4cc0a0848bd7 100644 --- a/xen/arch/x86/hvm/vpt.c +++ b/xen/arch/x86/hvm/vpt.c @@ -592,7 +592,7 @@ static void pt_adjust_vcpu(struct periodic_time *pt, struct vcpu *v) if ( pt->vcpu == NULL ) return; - write_lock(&pt->vcpu->domain->arch.hvm.pl_time->pt_migrate); + write_lock(&v->domain->arch.hvm.pl_time->pt_migrate); if ( pt->vcpu == v ) goto out; @@ -613,7 +613,7 @@ static void pt_adjust_vcpu(struct periodic_time *pt, struct vcpu *v) pt_vcpu_unlock(v); out: - write_unlock(&pt->vcpu->domain->arch.hvm.pl_time->pt_migrate); + write_unlock(&v->domain->arch.hvm.pl_time->pt_migrate); } void pt_adjust_global_vcpu_target(struct vcpu *v)
Make both create_periodic_time() and pt_adjust_vcpu() call write_{un}lock with similar arguments. Requested-by: Jan Beulich <jbeulich@suse.com> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com> --- New patch. I ended up doing what Jan asked --- create_periodic_time() is also using different start pointers in lock() and unlock(). xen/arch/x86/hvm/vpt.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)