* [PATCH 1/3] sched: fix confusing PFA_NO_NEW_PRIVS constant @ 2014-09-22 3:50 Zefan Li 2014-09-22 3:51 ` [PATCH 2/3] sched: add a macro to define bitops for task atomic flags Zefan Li ` (2 more replies) 0 siblings, 3 replies; 6+ messages in thread From: Zefan Li @ 2014-09-22 3:50 UTC (permalink / raw) To: Tejun Heo Cc: LKML, Cgroups, Ingo Molnar, Peter Zijlstra, keescook, Tetsuo Handa From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> Commit 1d4457f99928 ("sched: move no_new_privs into new atomic flags") defined PFA_NO_NEW_PRIVS as hexadecimal value, but it is confusing because it is used as bit number. Redefine it as decimal bit number. Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> Acked-by: Kees Cook <keescook@chromium.org> Signed-off-by: Zefan Li <lizefan@huawei.com> --- include/linux/sched.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/linux/sched.h b/include/linux/sched.h index 5c2c885..4557765 100644 --- a/include/linux/sched.h +++ b/include/linux/sched.h @@ -1957,7 +1957,7 @@ static inline void memalloc_noio_restore(unsigned int flags) } /* Per-process atomic flags. */ -#define PFA_NO_NEW_PRIVS 0x00000001 /* May not gain new privileges. */ +#define PFA_NO_NEW_PRIVS 0 /* May not gain new privileges. */ static inline bool task_no_new_privs(struct task_struct *p) { -- 1.8.0.2 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/3] sched: add a macro to define bitops for task atomic flags 2014-09-22 3:50 [PATCH 1/3] sched: fix confusing PFA_NO_NEW_PRIVS constant Zefan Li @ 2014-09-22 3:51 ` Zefan Li 2014-09-22 11:45 ` Peter Zijlstra 2014-09-22 3:52 ` [PATCH 3/3] cpuset: PF_SPREAD_PAGE and PF_SPREAD_SLAB should be " Zefan Li 2014-09-22 4:02 ` [PATCH 1/3] sched: fix confusing PFA_NO_NEW_PRIVS constant Tejun Heo 2 siblings, 1 reply; 6+ messages in thread From: Zefan Li @ 2014-09-22 3:51 UTC (permalink / raw) To: Tejun Heo Cc: LKML, Cgroups, Ingo Molnar, Peter Zijlstra, keescook, Tetsuo Handa This will simplify code when we add new flags. Signed-off-by: Zefan Li <lizefan@huawei.com> --- include/linux/sched.h | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/include/linux/sched.h b/include/linux/sched.h index 4557765..04a2ae2 100644 --- a/include/linux/sched.h +++ b/include/linux/sched.h @@ -1959,15 +1959,17 @@ static inline void memalloc_noio_restore(unsigned int flags) /* Per-process atomic flags. */ #define PFA_NO_NEW_PRIVS 0 /* May not gain new privileges. */ -static inline bool task_no_new_privs(struct task_struct *p) -{ - return test_bit(PFA_NO_NEW_PRIVS, &p->atomic_flags); -} - -static inline void task_set_no_new_privs(struct task_struct *p) -{ - set_bit(PFA_NO_NEW_PRIVS, &p->atomic_flags); -} +#define TASK_PFA_BITOPS(name, func) \ +static inline bool task_##func(struct task_struct *p) \ +{ return test_bit(PFA_##name, &p->atomic_flags); } \ + \ +static inline void task_set_##func(struct task_struct *p) \ +{ set_bit(PFA_##name, &p->atomic_flags); } \ + \ +static inline void task_clear_##func(struct task_struct *p) \ +{ clear_bit(PFA_##name, &p->atomic_flags); } + +TASK_PFA_BITOPS(NO_NEW_PRIVS, no_new_privs) /* * task->jobctl flags -- 1.8.0.2 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/3] sched: add a macro to define bitops for task atomic flags 2014-09-22 3:51 ` [PATCH 2/3] sched: add a macro to define bitops for task atomic flags Zefan Li @ 2014-09-22 11:45 ` Peter Zijlstra 0 siblings, 0 replies; 6+ messages in thread From: Peter Zijlstra @ 2014-09-22 11:45 UTC (permalink / raw) To: Zefan Li; +Cc: Tejun Heo, LKML, Cgroups, Ingo Molnar, keescook, Tetsuo Handa On Mon, Sep 22, 2014 at 11:51:48AM +0800, Zefan Li wrote: > This will simplify code when we add new flags. > > Signed-off-by: Zefan Li <lizefan@huawei.com> > --- > include/linux/sched.h | 20 +++++++++++--------- > 1 file changed, 11 insertions(+), 9 deletions(-) > > diff --git a/include/linux/sched.h b/include/linux/sched.h > index 4557765..04a2ae2 100644 > --- a/include/linux/sched.h > +++ b/include/linux/sched.h > @@ -1959,15 +1959,17 @@ static inline void memalloc_noio_restore(unsigned int flags) > /* Per-process atomic flags. */ > #define PFA_NO_NEW_PRIVS 0 /* May not gain new privileges. */ > > -static inline bool task_no_new_privs(struct task_struct *p) > -{ > - return test_bit(PFA_NO_NEW_PRIVS, &p->atomic_flags); > -} > - > -static inline void task_set_no_new_privs(struct task_struct *p) > -{ > - set_bit(PFA_NO_NEW_PRIVS, &p->atomic_flags); > -} > +#define TASK_PFA_BITOPS(name, func) \ > +static inline bool task_##func(struct task_struct *p) \ > +{ return test_bit(PFA_##name, &p->atomic_flags); } \ > + \ > +static inline void task_set_##func(struct task_struct *p) \ > +{ set_bit(PFA_##name, &p->atomic_flags); } \ > + \ > +static inline void task_clear_##func(struct task_struct *p) \ > +{ clear_bit(PFA_##name, &p->atomic_flags); } > + > +TASK_PFA_BITOPS(NO_NEW_PRIVS, no_new_privs) This could really do with a corresponding script/tags.sh update. --regex-c++='TASK_PFA_BITOPS\([^,]*,([^)]*)\)/task_\1/' --regex-c++='TASK_PFA_BITOPS\([^,]*,([^)]*)\)/task_set_\1/' --regex-c++='TASK_PFA_BITOPS\([^,]*,([^)]*)\)/task_clear_\1/' For exhuberant and matching bits for emacs. ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 3/3] cpuset: PF_SPREAD_PAGE and PF_SPREAD_SLAB should be atomic flags 2014-09-22 3:50 [PATCH 1/3] sched: fix confusing PFA_NO_NEW_PRIVS constant Zefan Li 2014-09-22 3:51 ` [PATCH 2/3] sched: add a macro to define bitops for task atomic flags Zefan Li @ 2014-09-22 3:52 ` Zefan Li 2014-09-22 4:02 ` [PATCH 1/3] sched: fix confusing PFA_NO_NEW_PRIVS constant Tejun Heo 2 siblings, 0 replies; 6+ messages in thread From: Zefan Li @ 2014-09-22 3:52 UTC (permalink / raw) To: Tejun Heo Cc: LKML, Cgroups, Ingo Molnar, Peter Zijlstra, keescook, Tetsuo Handa When we change cpuset.memory_spread_{page,slab}, cpuset will flip PF_SPREAD_{PAGE,SLAB} bit of tsk->flags for each task in that cpuset. This should be done using atomic bitops, but currently we don't, which is broken. Tetsuo reported a hard-to-reproduce kernel crash on RHEL6, which happend when one thread tried to clear PF_USED_MATH while at the same time another thread tried to flip PF_SPREAD_PAGE/PF_SPREAD_SLAB. They both operate on the same task. Here's the full report: https://lkml.org/lkml/2014/9/19/230 To fix this, we make PF_SPREAD_PAGE and PF_SPARED_SLAB atomic flags. Fixes: 950592f7b991 ("cpusets: update tasks' page/slab spread flags in time") Cc: <stable@vger.kernel.org> # 2.6.31+ Reported-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> Signed-off-by: Zefan Li <lizefan@huawei.com> --- include/linux/cpuset.h | 4 ++-- include/linux/sched.h | 6 ++++-- kernel/cpuset.c | 9 +++++---- 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/include/linux/cpuset.h b/include/linux/cpuset.h index 0d4e067..2f073db 100644 --- a/include/linux/cpuset.h +++ b/include/linux/cpuset.h @@ -94,12 +94,12 @@ extern int cpuset_slab_spread_node(void); static inline int cpuset_do_page_mem_spread(void) { - return current->flags & PF_SPREAD_PAGE; + return task_spread_page(current); } static inline int cpuset_do_slab_mem_spread(void) { - return current->flags & PF_SPREAD_SLAB; + return task_spread_slab(current); } extern int current_cpuset_is_being_rebound(void); diff --git a/include/linux/sched.h b/include/linux/sched.h index 04a2ae2..ee634d1 100644 --- a/include/linux/sched.h +++ b/include/linux/sched.h @@ -1903,8 +1903,6 @@ extern void thread_group_cputime_adjusted(struct task_struct *p, cputime_t *ut, #define PF_KTHREAD 0x00200000 /* I am a kernel thread */ #define PF_RANDOMIZE 0x00400000 /* randomize virtual address space */ #define PF_SWAPWRITE 0x00800000 /* Allowed to write to swap */ -#define PF_SPREAD_PAGE 0x01000000 /* Spread page cache over cpuset */ -#define PF_SPREAD_SLAB 0x02000000 /* Spread some slab caches over cpuset */ #define PF_NO_SETAFFINITY 0x04000000 /* Userland is not allowed to meddle with cpus_allowed */ #define PF_MCE_EARLY 0x08000000 /* Early kill for mce process policy */ #define PF_MUTEX_TESTER 0x20000000 /* Thread belongs to the rt mutex tester */ @@ -1958,6 +1956,8 @@ static inline void memalloc_noio_restore(unsigned int flags) /* Per-process atomic flags. */ #define PFA_NO_NEW_PRIVS 0 /* May not gain new privileges. */ +#define PFA_SPREAD_PAGE 1 /* Spread page cache over cpuset */ +#define PFA_SPREAD_SLAB 2 /* Spread some slab caches over cpuset */ #define TASK_PFA_BITOPS(name, func) \ static inline bool task_##func(struct task_struct *p) \ @@ -1970,6 +1970,8 @@ static inline void task_clear_##func(struct task_struct *p) \ { clear_bit(PFA_##name, &p->atomic_flags); } TASK_PFA_BITOPS(NO_NEW_PRIVS, no_new_privs) +TASK_PFA_BITOPS(SPREAD_PAGE, spread_page) +TASK_PFA_BITOPS(SPREAD_SLAB, spread_slab) /* * task->jobctl flags diff --git a/kernel/cpuset.c b/kernel/cpuset.c index a37f4ed..1f107c7 100644 --- a/kernel/cpuset.c +++ b/kernel/cpuset.c @@ -365,13 +365,14 @@ static void cpuset_update_task_spread_flag(struct cpuset *cs, struct task_struct *tsk) { if (is_spread_page(cs)) - tsk->flags |= PF_SPREAD_PAGE; + task_set_spread_page(tsk); else - tsk->flags &= ~PF_SPREAD_PAGE; + task_clear_spread_page(tsk); + if (is_spread_slab(cs)) - tsk->flags |= PF_SPREAD_SLAB; + task_set_spread_slab(tsk); else - tsk->flags &= ~PF_SPREAD_SLAB; + task_clear_spread_slab(tsk); } /* -- 1.8.0.2 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/3] sched: fix confusing PFA_NO_NEW_PRIVS constant 2014-09-22 3:50 [PATCH 1/3] sched: fix confusing PFA_NO_NEW_PRIVS constant Zefan Li 2014-09-22 3:51 ` [PATCH 2/3] sched: add a macro to define bitops for task atomic flags Zefan Li 2014-09-22 3:52 ` [PATCH 3/3] cpuset: PF_SPREAD_PAGE and PF_SPREAD_SLAB should be " Zefan Li @ 2014-09-22 4:02 ` Tejun Heo 2014-09-22 4:44 ` Zefan Li 2 siblings, 1 reply; 6+ messages in thread From: Tejun Heo @ 2014-09-22 4:02 UTC (permalink / raw) To: Zefan Li Cc: LKML, Cgroups, Ingo Molnar, Peter Zijlstra, keescook, Tetsuo Handa On Mon, Sep 22, 2014 at 11:50:46AM +0800, Zefan Li wrote: > From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> > > Commit 1d4457f99928 ("sched: move no_new_privs into new atomic flags") > defined PFA_NO_NEW_PRIVS as hexadecimal value, but it is confusing > because it is used as bit number. Redefine it as decimal bit number. It's also changing the bit position, which is fine but should be mentioned in the description. Thanks. -- tejun ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/3] sched: fix confusing PFA_NO_NEW_PRIVS constant 2014-09-22 4:02 ` [PATCH 1/3] sched: fix confusing PFA_NO_NEW_PRIVS constant Tejun Heo @ 2014-09-22 4:44 ` Zefan Li 0 siblings, 0 replies; 6+ messages in thread From: Zefan Li @ 2014-09-22 4:44 UTC (permalink / raw) To: Tejun Heo Cc: LKML, Cgroups, Ingo Molnar, Peter Zijlstra, keescook, Tetsuo Handa On 2014/9/22 12:02, Tejun Heo wrote: > On Mon, Sep 22, 2014 at 11:50:46AM +0800, Zefan Li wrote: >> From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> >> >> Commit 1d4457f99928 ("sched: move no_new_privs into new atomic flags") >> defined PFA_NO_NEW_PRIVS as hexadecimal value, but it is confusing >> because it is used as bit number. Redefine it as decimal bit number. > > It's also changing the bit position, which is fine but should be > mentioned in the description. > I've resent the patch with changelog updated. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-09-22 11:45 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-09-22 3:50 [PATCH 1/3] sched: fix confusing PFA_NO_NEW_PRIVS constant Zefan Li 2014-09-22 3:51 ` [PATCH 2/3] sched: add a macro to define bitops for task atomic flags Zefan Li 2014-09-22 11:45 ` Peter Zijlstra 2014-09-22 3:52 ` [PATCH 3/3] cpuset: PF_SPREAD_PAGE and PF_SPREAD_SLAB should be " Zefan Li 2014-09-22 4:02 ` [PATCH 1/3] sched: fix confusing PFA_NO_NEW_PRIVS constant Tejun Heo 2014-09-22 4:44 ` Zefan Li
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).