lkml.org 
[lkml]   [2008]   [Sep]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [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




\
 
 \ /
  Last update: 2008-09-10 04:39    [W:0.062 / U:0.048 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site