* [PATCH RESEND] cgroups: fix incorrect using rcu_dereference() in cgroup_subsys_state()
@ 2011-01-04 8:18 Lai Jiangshan
2011-01-04 21:23 ` Paul E. McKenney
2011-01-05 1:31 ` Paul Menage
0 siblings, 2 replies; 3+ messages in thread
From: Lai Jiangshan @ 2011-01-04 8:18 UTC (permalink / raw)
To: Paul Menage, Paul E. McKenney, Li Zefan, LKML
From: Li Zefan <lizf@cn.fujitsu.com>
Date: Mon, 25 Aug 2008 11:05:28 +0800
(Original) Subject: [PATCH] cgroup: fix wrong rcu_dereference()
It is tsk->cgroups which is protected by RCU, not ->subsys[subsys_id].
laijs: updated it(the surrounding code have been changed since these two years).
Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
---
diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
index ed4ba11..a798814 100644
--- a/include/linux/cgroup.h
+++ b/include/linux/cgroup.h
@@ -535,10 +535,11 @@ static inline struct cgroup_subsys_state *cgroup_subsys_state(
* cgroup_subsys::attach() methods.
*/
#define task_subsys_state_check(task, subsys_id, __c) \
- rcu_dereference_check(task->cgroups->subsys[subsys_id], \
+ rcu_dereference_check(task->cgroups, \
rcu_read_lock_held() || \
lockdep_is_held(&task->alloc_lock) || \
- cgroup_lock_is_held() || (__c))
+ cgroup_lock_is_held() || \
+ (__c))->subsys[subsys_id]
static inline struct cgroup_subsys_state *
task_subsys_state(struct task_struct *task, int subsys_id)
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH RESEND] cgroups: fix incorrect using rcu_dereference() in cgroup_subsys_state()
2011-01-04 8:18 [PATCH RESEND] cgroups: fix incorrect using rcu_dereference() in cgroup_subsys_state() Lai Jiangshan
@ 2011-01-04 21:23 ` Paul E. McKenney
2011-01-05 1:31 ` Paul Menage
1 sibling, 0 replies; 3+ messages in thread
From: Paul E. McKenney @ 2011-01-04 21:23 UTC (permalink / raw)
To: Lai Jiangshan; +Cc: Paul Menage, Li Zefan, LKML
On Tue, Jan 04, 2011 at 04:18:06PM +0800, Lai Jiangshan wrote:
> From: Li Zefan <lizf@cn.fujitsu.com>
> Date: Mon, 25 Aug 2008 11:05:28 +0800
> (Original) Subject: [PATCH] cgroup: fix wrong rcu_dereference()
>
> It is tsk->cgroups which is protected by RCU, not ->subsys[subsys_id].
>
> laijs: updated it(the surrounding code have been changed since these two years).
This looks plausible to me, assuming that the cgroups guys are OK with
it.
One requested change: could you please delete the rcu_read_lock_held()?
This is now implied by rcu_dereference_check(). With that:
Acked-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
> Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
> ---
> diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
> index ed4ba11..a798814 100644
> --- a/include/linux/cgroup.h
> +++ b/include/linux/cgroup.h
> @@ -535,10 +535,11 @@ static inline struct cgroup_subsys_state *cgroup_subsys_state(
> * cgroup_subsys::attach() methods.
> */
> #define task_subsys_state_check(task, subsys_id, __c) \
> - rcu_dereference_check(task->cgroups->subsys[subsys_id], \
> + rcu_dereference_check(task->cgroups, \
> rcu_read_lock_held() || \
> lockdep_is_held(&task->alloc_lock) || \
> - cgroup_lock_is_held() || (__c))
> + cgroup_lock_is_held() || \
> + (__c))->subsys[subsys_id]
>
> static inline struct cgroup_subsys_state *
> task_subsys_state(struct task_struct *task, int subsys_id)
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH RESEND] cgroups: fix incorrect using rcu_dereference() in cgroup_subsys_state()
2011-01-04 8:18 [PATCH RESEND] cgroups: fix incorrect using rcu_dereference() in cgroup_subsys_state() Lai Jiangshan
2011-01-04 21:23 ` Paul E. McKenney
@ 2011-01-05 1:31 ` Paul Menage
1 sibling, 0 replies; 3+ messages in thread
From: Paul Menage @ 2011-01-05 1:31 UTC (permalink / raw)
To: Lai Jiangshan; +Cc: Paul E. McKenney, Li Zefan, LKML
On Tue, Jan 4, 2011 at 12:18 AM, Lai Jiangshan <laijs@cn.fujitsu.com> wrote:
> From: Li Zefan <lizf@cn.fujitsu.com>
> Date: Mon, 25 Aug 2008 11:05:28 +0800
> (Original) Subject: [PATCH] cgroup: fix wrong rcu_dereference()
>
> It is tsk->cgroups which is protected by RCU, not ->subsys[subsys_id].
>
> laijs: updated it(the surrounding code have been changed since these two years).
>
> Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
> Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
> ---
> diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
> index ed4ba11..a798814 100644
> --- a/include/linux/cgroup.h
> +++ b/include/linux/cgroup.h
> @@ -535,10 +535,11 @@ static inline struct cgroup_subsys_state *cgroup_subsys_state(
> * cgroup_subsys::attach() methods.
> */
> #define task_subsys_state_check(task, subsys_id, __c) \
> - rcu_dereference_check(task->cgroups->subsys[subsys_id], \
> + rcu_dereference_check(task->cgroups, \
> rcu_read_lock_held() || \
> lockdep_is_held(&task->alloc_lock) || \
> - cgroup_lock_is_held() || (__c))
> + cgroup_lock_is_held() || \
> + (__c))->subsys[subsys_id]
Can you reformat this a bit? It took a fair bit of staring to realize
that (__c))->subsys[subsys_id] wasn't trying to dereference __c.
Maybe make it:
#define task_subsys_state_check(task, subsys_id, __c) ({ \
struct css_set *_css = rcu_dereference_check(...); \
_css->subsys[subsys_id]; \
})
Paul
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2011-01-05 1:32 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-01-04 8:18 [PATCH RESEND] cgroups: fix incorrect using rcu_dereference() in cgroup_subsys_state() Lai Jiangshan
2011-01-04 21:23 ` Paul E. McKenney
2011-01-05 1:31 ` Paul Menage
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox