Re: [Qemu-devel] [PATCH 2/6] throttle: Add throttle group infrastructure

[Alberto Garcia <berto@igalia.com>]

On Tue, Mar 24, 2015 at 03:03:07PM +0000, Stefan Hajnoczi wrote:

> > +typedef struct ThrottleGroup {
> > +    char *name; /* This is constant during the lifetime of the group */
> 
> Is this also protected by throttle_groups_lock?
> 
> I guess throttle_groups_lock must be held in order to read this
> field - otherwise there is a risk that the object is freed unless
> you have already incremented the refcount.

The creation and destruction of ThrottleGroup objects are protected by
throttle_groups_lock. That includes handling the memory for that field
and its contents.

Once the ThrottleGroup is created the 'name' field doesn't need any
additional locking since it remains constant during the lifetime of
the group.

The only way to read it from the outside is throttle_group_get_name()
and that's safe (until you release the reference to the group, that
is).

> > +    QemuMutex lock;
> 
> What does this lock protect?  I guess 'ts', 'head', and 'tokens'
> fields.  Please document it.

I thought it was obvious in the code but since you ask I guess it's
not :) I can add an additional comment.

Berto