Messages in this thread Patch in this message | | | Date | Wed, 10 Sep 2008 10:18:36 +0800 | From | Lai Jiangshan <> | Subject | Re: [PATCH] cgroups: fix probable race with put_css_set[_taskexit] and find_css_set |
| |
Paul Menage wrote: > On Mon, Aug 18, 2008 at 11:29 PM, Lai Jiangshan <laijs@cn.fujitsu.com> wrote: > > 2) Use atomic_inc_not_zero() in find_existing_css_set(), to ensure > that we only return a referenced css, and remove the get_css_set() > call from find_css_set(). (Possibly wrapping this in a new > kref_get_not_zero() function) >
[CC: Greg Kroah-Hartman <greg@kroah.com>]
There are indeed several ways fix this race by Using the atomic-functions directly. I prefer the second one, i makes all code clearly. And put_css_set[_taskexit] do not need to be changed.
I don't think adding kref_get_not_zero() API is a good idea. It will bring kref APIs to a little chaos, kref_get_not_zero() is hard to be used, for this function needs a special lock held.
But I tried:
Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com> --- diff --git a/include/linux/kref.h b/include/linux/kref.h index 0cef6ba..400ffab 100644 --- a/include/linux/kref.h +++ b/include/linux/kref.h @@ -25,6 +25,7 @@ struct kref { void kref_set(struct kref *kref, int num); void kref_init(struct kref *kref); void kref_get(struct kref *kref); +int kref_get_not_zero(struct kref *kref); int kref_put(struct kref *kref, void (*release) (struct kref *kref)); #endif /* _KREF_H_ */ diff --git a/kernel/cgroup.c b/kernel/cgroup.c index 13932ab..0bbb98d 100644 --- a/kernel/cgroup.c +++ b/kernel/cgroup.c @@ -347,6 +347,8 @@ static struct css_set *find_existing_css_set( hlist_for_each_entry(cg, node, hhead, hlist) { if (!memcmp(template, cg->subsys, sizeof(cg->subsys))) { /* All subsystems matched */ + if (!kref_get_not_zero(&cg->ref)) + return NULL; return cg; } } @@ -410,8 +412,6 @@ static struct css_set *find_css_set( * the desired set */ read_lock(&css_set_lock); res = find_existing_css_set(oldcg, cgrp, template); - if (res) - get_css_set(res); read_unlock(&css_set_lock); if (res) diff --git a/lib/kref.c b/lib/kref.c index 9ecd6e8..b8c1ce6 100644 --- a/lib/kref.c +++ b/lib/kref.c @@ -46,6 +46,25 @@ void kref_get(struct kref *kref) } /** + * kref_get_not_zero - increment refcount for object if current refcount + * is not zero. + * @kref: object. + * + * Beware, the object maybe be being released, so we need a special lock held + * to ensure the object's refcount is remaining access. + * + * Return 0 if this refcount is 0, otherwise return 1. + */ +int kref_get_not_zero(struct kref *kref) +{ + if (atomic_inc_not_zero(&kref->refcount)) { + smp_mb__after_atomic_inc(); + return 1; + } + return 0; +} + +/** * kref_put - decrement refcount for object. * @kref: object. * @release: pointer to the function that will clean up the object when the
| |