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

["Metzger, Markus T" <markus.t.metzger@intel.com>]

>-----Original Message-----
>From: Oleg Nesterov [mailto:oleg@redhat.com]
>Sent: Wednesday, April 01, 2009 2:17 AM
>To: Metzger, Markus T


>> +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...

Hmmm, As far as I understand the code, rt-workqueues use a higher sched_class
and can thus not be preempted by normal threads. Non-rt workqueues
use the fair_sched_class. And schedule_work() uses a non-rt workqueue.

In practice, task is ptraced. It is either stopped or exiting.
I don't expect to loop very often.


>
>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).

I know. See the reply to patch 0; I tried to keep the patches small and focused
to simplify the review work and attract reviewers.

thanks and regards,
markus.

---------------------------------------------------------------------
Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen Germany
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Douglas Lusk, Peter Gleissner, Hannes Schwaderer
Registergericht: Muenchen HRB 47456 Ust.-IdNr.
VAT Registration No.: DE129385895
Citibank Frankfurt (BLZ 502 109 00) 600119052

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.