On 16 September 2016 at 12:51, Peter Zijlstra peterz@infradead.org wrote:
On Mon, Sep 12, 2016 at 09:47:52AM +0200, Vincent Guittot wrote:
-dequeue task -put task -change the property -enqueue task -set task as current task
diff --git a/kernel/sched/core.c b/kernel/sched/core.c index 3e52d08..7a9c9b9 100644 --- a/kernel/sched/core.c +++ b/kernel/sched/core.c @@ -1105,10 +1105,10 @@ void do_set_cpus_allowed(struct task_struct *p, const struct cpumask *new_mask)
p->sched_class->set_cpus_allowed(p, new_mask);
if (running)p->sched_class->set_curr_task(rq); if (queued) enqueue_task(rq, p, ENQUEUE_RESTORE);
if (running)p->sched_class->set_curr_task(rq);}
/*
So one thing that I've wanted to do for a while, but never managed to come up with a sensible way to do is encapsulate this pattern.
The two options I came up with are:
#define FOO(p, stmt) ({ struct rq *rq = task_rq(p); bool queued = task_on_rq_queued(p); bool running = task_current(rq); int queue_flags = DEQUEUE_SAVE; /* also ENQUEUE_RESTORE */
if (queued) dequeue_task(rq, p, queue_flags); if (running) put_prev_task(rq, p); stmt; if (queued) enqueue_task(rq, p, queue_flags); if (running) set_curr_task(rq, p);})
and
void foo(struct task_struct *p, void (*func)(struct task_struct *, int *)) { struct rq *rq = task_rq(p); bool queued = task_on_rq_queued(p); bool running = task_current(rq); int queue_flags = DEQUEUE_SAVE; /* also ENQUEUE_RESTORE */
if (queued) dequeue_task(rq, p, queue_flags); if (running) put_prev_task(rq, p); func(p, &queue_flags); if (queued) enqueue_task(rq, p, queue_flags); if (running) set_curr_task(rq, p);}
Neither results in particularly pretty code. Although I suppose if I'd have to pick one I'd go for the macro variant.
Opinions? I'm fine with leaving the code as is, just wanted to throw this out there.
I'm not convinced by using such encapsulation as it adds the constraint of having a function to pass which is not always the case and it hides a bit whats happen to this function What about creating a task_FOO_save and a task_FOO_save macro ? something like
#define task_FOO_save(p, rq, flags) ({ bool queued = task_on_rq_queued(p); bool running = task_current(rq); int queue_flags = DEQUEUE_SAVE; /* also ENQUEUE_RESTORE */
if (queued) dequeue_task(rq, p, queue_flags); if (running) put_prev_task(rq, p); flags = queued | running << 1; })
#define task_FOO_restore(p, rq, flags) ({ bool queued = flags & 0x1; bool running = flags & 0x2; int queue_flags = ENQUEUE_RESTORE;
if (queued) enqueue_task(rq, p, queue_flags); if (running) set_curr_task(rq, p); })