On Tue, Mar 24, 2015 at 04:33:48PM +0100, Alberto Garcia wrote:
> 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).Right, the race condition is when the group is released. Looking at this again, the assumption isn't that throttle_groups_lock is held. The AioContext lock is held by throttle_group_get_name() users and that's why there is no race when releasing the reference. If someone adds a throttle_group_get_name() caller in the future without holding AioContext, then we'd be in trouble. That is why documenting the locking constraints is useful. Stefan
pgph8pXXwxUOR.pgp
Description: PGP signature
