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

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: <20090401001729.GC28228@redhat.com>
Date:	Wed, 1 Apr 2009 02:17:29 +0200
From:	Oleg Nesterov <oleg@...hat.com>
To:	Markus Metzger <markus.t.metzger@...el.com>
Cc:	linux-kernel@...r.kernel.org, mingo@...e.hu, tglx@...utronix.de,
	hpa@...or.com, markus.t.metzger@...il.com, roland@...hat.com,
	eranian@...glemail.com, juan.villacis@...el.com,
	ak@...ux.jf.intel.com
Subject: Re: [patch 3/21] x86, bts: wait until traced task has been
	scheduled out

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.

--
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/