Re: [Xen-devel] [PATCH v5 07/13] x86: dynamically get/set CBM for a domain

[Chao Peng <chao.p.peng@xxxxxxxxxxxxxxx>]

On Mon, Apr 20, 2015 at 04:52:09PM +0100, Andrew Cooper wrote:
> On 17/04/15 15:33, Chao Peng wrote:
> > For CAT, COS is maintained in hypervisor only while CBM is exposed to
> > user space directly to allow getting/setting domain's cache capacity.
> > For each specified CBM, hypervisor will either use a existed COS which
> > has the same CBM or allocate a new one if the same CBM is not found. If
> > the allocation fails because of no enough COS available then error is
> > returned. The getting/setting are always operated on a specified socket.
> > For multiple sockets system, the interface may be called several times.
> >
> > Signed-off-by: Chao Peng <chao.p.peng@xxxxxxxxxxxxxxx>
> > ---
> > Changes in v5:
> > * Add spin_lock to protect cbm_map.
> > ---
> > +    for ( cos = 0; cos <= info->cos_max; cos++ )
> > +    {
> > +        /* If still not found, then keep unused one. */
> > +        if ( !find && cos != 0 && map[cos].ref == 0 )
> > +            find = map + cos;
> > +        else if ( map[cos].cbm == cbm )
> > +        {
> > +            if ( unlikely(cos == old_cos) )
> > +                return 0;
> > +            find = map + cos;
> > +            break;
> > +        }
> > +    }
> > +
> > +    /* If old cos is referred only by the domain, then use it. */
> > +    if ( !find && map[old_cos].ref == 1 )
> > +        find = map + old_cos;
> > +
> > +    if ( !find )
> > +        return -EUSERS;
> > +
> > +    cos = find - map;
> > +    if ( find->cbm != cbm )
> > +    {
> > +        ret = write_l3_cbm(socket, cos, cbm);
> > +        if ( ret )
> > +            return ret;
> > +        find->cbm = cbm;
> > +    }
> > +
> > +    spin_lock(&info->cbm_lock);
> > +    find->ref++;
> > +    map[old_cos].ref--;
> > +    spin_unlock(&info->cbm_lock);
> 
> The spinlock must cover read accesses as well, or old_cos is liable to
> be stale by this point.

You mean map[old_cos].ref and find->ref are stale, right? old_cos itself
seems not need to be protected.
> 
> It might be better to split into a rw_lock as it is read often but
> modifications should be very rare.

NP, thanks.

Chao

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