[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 2/6] throttle: Add throttle group infrastructure
From: |
Alberto Garcia |
Subject: |
Re: [Qemu-devel] [PATCH 2/6] throttle: Add throttle group infrastructure |
Date: |
Wed, 25 Mar 2015 13:14:58 +0100 |
User-agent: |
Mutt/1.5.20 (2009-06-14) |
> > > > +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.
But that would only happen if you access bs->throttle_state without
holding bs's AioContext. I understand that it's implicit that you
should hold the context there.
Maybe I can update the throttle_group_* API to use BlockDriverState
in all cases instead of ThrottleState, it's probably more clear like
that.
Berto
[Qemu-devel] [PATCH 3/6] throttle: Add throttle group infrastructure tests, Alberto Garcia, 2015/03/10
[Qemu-devel] [PATCH 5/6] throttle: add the name of the ThrottleGroup to BlockDeviceInfo, Alberto Garcia, 2015/03/10
[Qemu-devel] [PATCH 1/6] throttle: Extract timers from ThrottleState into a separate structure, Alberto Garcia, 2015/03/10
[Qemu-devel] [PATCH 4/6] throttle: Add throttle group support, Alberto Garcia, 2015/03/10