Re: [PATCH v12 2/3] KVM: s390: guest support for topology function

[Janosch Frank]

[...]
> > > > >     +/**
> > > > > + * kvm_s390_update_topology_change_report - update CPU topology change report
> > > > > + * @kvm: guest KVM description
> > > > > + * @val: set or clear the MTCR bit
> > > > > + *
> > > > > + * Updates the Multiprocessor Topology-Change-Report bit to signal
> > > > > + * the guest with a topology change.
> > > > > + * This is only relevant if the topology facility is present.
> > > > > + *
> > > > > + * The SCA version, bsca or esca, doesn't matter as offset is the same.
> > > > > + */
> > > > > +static void kvm_s390_update_topology_change_report(struct kvm *kvm, bool val)
> > > > > +{
> > > > > +    union sca_utility new, old;
> > > > > +    struct bsca_block *sca;
> > > > > +
> > > > > +    read_lock(&kvm->arch.sca_lock);
> > > > > +    do {
> > > > > +        sca = kvm->arch.sca;
> > > >
> > > > I find this assignment being in the loop unintuitive, but it should not make a difference.
> > >
> > > The price would be an ugly cast.
> >
> > I don't get what you mean. Nothing about the types changes if you move it before the loop.
>
> Yes right, did wrong understand.
> It is better before.
With the assignment moved one line up:
Reviewed-by: Janosch Frank <frankja@xxxxxxxxxxxxx>

> > > > > +        old = READ_ONCE(sca->utility);
> > > > > +        new = old;
> > > > > +        new.mtcr = val;
> > > > > +    } while (cmpxchg(&sca->utility.val, old.val, new.val) != old.val);
> > > > > +    read_unlock(&kvm->arch.sca_lock);
> > > > > +}
> > > > > +
> > > > [...]