linux-kernel - Re: [PATCH v6 3/3] sched/rt: Check to push the task when changing its affinity

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20150420120650.338120f9@gandalf.local.home>
Date:	Mon, 20 Apr 2015 12:06:50 -0400
From:	Steven Rostedt <rostedt@...dmis.org>
To:	Xunlei Pang <xlpang@....com>
Cc:	linux-kernel@...r.kernel.org,
	Peter Zijlstra <peterz@...radead.org>,
	Juri Lelli <juri.lelli@...il.com>,
	Xunlei Pang <pang.xunlei@...aro.org>
Subject: Re: [PATCH v6 3/3] sched/rt: Check to push the task when changing
 its affinity

On Mon, 20 Apr 2015 16:22:48 +0800
Xunlei Pang <xlpang@....com> wrote:


> +	rq = task_rq(p);
> +
> +	if (new_weight > 1 &&
> +	    rt_task(rq->curr) &&
> +	    rq->rt.rt_nr_total > 1 &&
> +	    !test_tsk_need_resched(rq->curr)) {
> +		/*
> +		 * We own p->pi_lock and rq->lock. rq->lock might
> +		 * get released when doing direct pushing, however
> +		 * p->pi_lock is always held, so it's safe to assign
> +		 * new_mask and new_weight to p below.
> +		 */
> +		if (!task_running(rq, p)) {
> +			cpumask_copy(&p->cpus_allowed, new_mask);
> +			p->nr_cpus_allowed = new_weight;
> +			direct_push = 1;
> +		} else if (cpumask_test_cpu(task_cpu(p), new_mask)) {
> +			struct task_struct *next;
> +
> +			cpumask_copy(&p->cpus_allowed, new_mask);
> +			p->nr_cpus_allowed = new_weight;
> +			if (!cpupri_find(&rq->rd->cpupri, p, NULL))
> +				goto update;
> +
> +			/*
> +			 * At this point, current task gets migratable most
> +			 * likely due to the change of its affinity, let's
> +			 * figure out if we can migrate it.
> +			 *
> +			 * Can we find any task with the same priority as
> +			 * current? To accomplish this, firstly we requeue
> +			 * current to the tail and peek next, then restore
> +			 * current to the head.

"current"? Don't you mean 'p'?


> +			 */
> +			requeue_task_rt(rq, p, 0);
> +			next = peek_next_task_rt(rq);
> +			requeue_task_rt(rq, p, 1);

Actually, I'm totally confused by all this. Why are you looking at
next? The running task (current) should not be in the next_task_rt()
list to begin with. If p can not preempt current, and cpupri_find()
finds something for p to run on, then move it there, and be done with
it.

I also looked at set_cpus_allowed_ptr() and think we should probably
add a p->sched_class->pick_cpu() and do:

	if (p->sched_class->pick_cpu)
		dest_cpu = p->sched_class->pick_cpu(p, cpu_active_mask,
				new_mask);
	else
		dest_cpu = cpumask_any_and(cpu_active_mask, new_mask);

Hmm, maybe all this work should be done there, or have
do_set_cpus_allowed() return a value that states that the
sched_class->set_cpus_allowed() did all the work for us.

-- Steve

> +			if (next != p && next->prio == p->prio) {
> +				/*
> +				 * Target found, so let's reschedule to try
> +				 * and push current away.
> +				 */
> +				requeue_task_rt(rq, next, 1);
> +				preempt_push = 1;
> +			}
> +		}
> +	}
> +
> +update:
>  	/*
>  	 * Only update if the process changes its state from whether it
>  	 * can migrate or not.
>  	 */
> -	if ((p->nr_cpus_allowed > 1) == (weight > 1))
> -		return;
> -
> -	rq = task_rq(p);
> +	if ((old_weight > 1) == (new_weight > 1))
> +		goto out;
>  
>  	/*
>  	 * The process used to be able to migrate OR it can now migrate
>  	 */
> -	if (weight <= 1) {
> +	if (new_weight <= 1) {
>  		if (!task_current(rq, p))
>  			dequeue_pushable_task(rq, p);
>  		BUG_ON(!rq->rt.rt_nr_migratory);
> @@ -2129,6 +2184,12 @@ static void set_cpus_allowed_rt(struct task_struct *p,
>  	}
>  
>  	update_rt_migration(&rq->rt);
> +
> +out:
> +	if (direct_push)
> +		push_rt_tasks(rq);
> +	else if (preempt_push)
> +		resched_curr(rq);
>  }
>  
>  /* Assumes rq->lock is held */

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/