[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v12 08/23] x86: refactor psr: L3 CAT: set value: implement framework.



>>> Yi Sun <yi.y.sun@xxxxxxxxxxxxxxx> 06/28/17 11:10 AM >>>
>On 17-06-28 01:14:59, Jan Beulich wrote:
>> >>> Yi Sun <yi.y.sun@xxxxxxxxxxxxxxx> 06/14/17 3:25 AM >>>
>> > @@ -537,7 +556,16 @@ int psr_get_val(struct domain *d, unsigned int socket,
>> >          return -ENOENT;
>> >      }
>> >  
>> > +    domain_lock(d);
>> > +    if ( !test_bit(d->domain_id, socket_info[socket].dom_set) )
>> > +    {
>> > +        d->arch.psr_cos_ids[socket] = 0;
>> > +        set_bit(d->domain_id, socket_info[socket].dom_set);
>> > +    }
>> 
>> Any reason not to use test_and_set_bit() here? I.e. is this on any hot path?
>> Or wait - I think it's even wrong to split the test from the set, as the lock
>> doesn't protect dom_set[].

With the last sentence here (which I had added after having written all of the
rest of the reply, I'm afraid I've managed to confuse you:

>>> 
>>Will change it to test_and_set_bit.
>...
>> > +    /*
>> > +     * Step 6:
>> > +     * Save the COS ID into current domain's psr_cos_ids[] so that we can 
>> > know
>> > +     * which COS the domain is using on the socket. One domain can only 
>> > use
>> > +     * one COS ID at same time on each socket.
>> > +     */
>> > +    domain_lock(d);
>> > +    d->arch.psr_cos_ids[socket] = cos;
>> > +    domain_unlock(d);
>> > +
>> > +    /*
>> > +     * Step 7:
>> > +     * Then, set the dom_set bit which corresponds to domain_id to mark 
>> > this
>> > +     * domain has been set and the COS ID of the domain is valid.
>> > +     */
>> > +    set_bit(d->domain_id, info->dom_set);
>> 
>> With the way things are being done above, doesn't this belong in the
>> domain_lock()-ed region?

I should have deleted this, since - as said above - the lock doesn't guard
against anything dom_set[]-wise. So ...

>Yes, should be. Thanks!

... I think you rather shouldn't do this. Instead you may want to consider 
whether
the other domain_lock()-ed regions couldn't be further shrunk.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.