| Message ID | 20210602133040.461908001@infradead.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | sched: Cleanup task_struct::state | expand |
----- On Jun 2, 2021, at 9:12 AM, Peter Zijlstra peterz@infradead.org wrote: > Remove yet another few p->state accesses. [...] > > --- a/include/linux/sched.h > +++ b/include/linux/sched.h > @@ -212,6 +212,8 @@ struct task_group; > > #endif > > +#define get_current_state() READ_ONCE(current->state) Why use a macro rather than a static inline here ? Thanks, Mathieu
On Wed, Jun 02, 2021 at 10:01:29AM -0400, Mathieu Desnoyers wrote: > ----- On Jun 2, 2021, at 9:12 AM, Peter Zijlstra peterz@infradead.org wrote: > > > Remove yet another few p->state accesses. > > [...] > > > > > --- a/include/linux/sched.h > > +++ b/include/linux/sched.h > > @@ -212,6 +212,8 @@ struct task_group; > > > > #endif > > > > +#define get_current_state() READ_ONCE(current->state) > > Why use a macro rather than a static inline here ? Mostly to be consistent, all that state stuff is macros. I suppose we could try and make them inlines at the end or so -- if the header maze allows.
On Wed, Jun 02, 2021 at 03:12:29PM +0200, Peter Zijlstra wrote: > Remove yet another few p->state accesses. > > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> > --- > block/blk-mq.c | 2 +- > include/linux/sched.h | 2 ++ > kernel/freezer.c | 2 +- > kernel/sched/core.c | 6 +++--- > 4 files changed, 7 insertions(+), 5 deletions(-) I think you can include kernel/kcsan/report.c here too. With that: Acked-by: Will Deacon <will@kernel.org> Will
--- a/block/blk-mq.c +++ b/block/blk-mq.c @@ -3891,7 +3891,7 @@ int blk_poll(struct request_queue *q, bl hctx->poll_considered++; - state = current->state; + state = get_current_state(); do { int ret; --- a/include/linux/sched.h +++ b/include/linux/sched.h @@ -212,6 +212,8 @@ struct task_group; #endif +#define get_current_state() READ_ONCE(current->state) + /* Task command name length: */ #define TASK_COMM_LEN 16 --- a/kernel/freezer.c +++ b/kernel/freezer.c @@ -58,7 +58,7 @@ bool __refrigerator(bool check_kthr_stop /* Hmm, should we be allowed to suspend when there are realtime processes around? */ bool was_frozen = false; - long save = current->state; + unsigned int save = get_current_state(); pr_debug("%s entered refrigerator\n", current->comm); --- a/kernel/sched/core.c +++ b/kernel/sched/core.c @@ -8273,15 +8273,15 @@ static inline int preempt_count_equals(i void __might_sleep(const char *file, int line, int preempt_offset) { + unsigned int state = get_current_state(); /* * Blocking primitives will set (and therefore destroy) current->state, * since we will exit with TASK_RUNNING make sure we enter with it, * otherwise we will destroy state. */ - WARN_ONCE(current->state != TASK_RUNNING && current->task_state_change, + WARN_ONCE(state != TASK_RUNNING && current->task_state_change, "do not call blocking ops when !TASK_RUNNING; " - "state=%lx set at [<%p>] %pS\n", - current->state, + "state=%x set at [<%p>] %pS\n", state, (void *)current->task_state_change, (void *)current->task_state_change);
Remove yet another few p->state accesses. Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> --- block/blk-mq.c | 2 +- include/linux/sched.h | 2 ++ kernel/freezer.c | 2 +- kernel/sched/core.c | 6 +++--- 4 files changed, 7 insertions(+), 5 deletions(-)