Re: [patch 3/21] x86, bts: wait until traced task has been scheduled out

[Oleg Nesterov <oleg@redhat.com>]

On 03/31, Markus Metzger wrote:
>
> +static void wait_to_unschedule(struct task_struct *task)
> +{
> +	unsigned long nvcsw;
> +	unsigned long nivcsw;
> +
> +	if (!task)
> +		return;
> +
> +	if (task == current)
> +		return;
> +
> +	nvcsw  = task->nvcsw;
> +	nivcsw = task->nivcsw;
> +	for (;;) {
> +		if (!task_is_running(task))
> +			break;
> +		/*
> +		 * The switch count is incremented before the actual
> +		 * context switch. We thus wait for two switches to be
> +		 * sure at least one completed.
> +		 */
> +		if ((task->nvcsw - nvcsw) > 1)
> +			break;
> +		if ((task->nivcsw - nivcsw) > 1)
> +			break;
> +
> +		schedule();

schedule() is a nop here. We can wait unpredictably long...

Ingo, do have have any ideas to improve this helper?

Not that I really like it, but how about

	int force_unschedule(struct task_struct *p)
	{
		struct rq *rq;
		unsigned long flags;
		int running;

		rq = task_rq_lock(p, &flags);
		running = task_running(rq, p);
		task_rq_unlock(rq, &flags);

		if (running)
			wake_up_process(rq->migration_thread);

		return running;
	}

which should be used instead of task_is_running() ?


We can even do something like

	void wait_to_unschedule(struct task_struct *task)
	{
		struct migration_req req;

		rq = task_rq_lock(p, &task);
		running = task_running(rq, p);
		if (running) {
			// make sure __migrate_task() will do nothing
			req->dest_cpu = NR_CPUS + 1;
			init_completion(&req->done);
			list_add(&req->list, &rq->migration_queue);
		}
		task_rq_unlock(rq, &flags);

		if (running) {
			wake_up_process(rq->migration_thread);
			wait_for_completion(&req.done);
		}
	}

This way we don't poll, and we need only one helper.

(Can't resist, this patch is not bisect friendly, without the next patches
 wait_to_unschedule() is called under write_lock_irq, this is deadlockable).

But anyway, I think we can do this later.

Oleg.