linux-kernel - Re: [PATCH 3/3] wait_task_inactive: don't use the dummy version when !SMP && PREEMPT

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] [day] [month] [year] [list]

Date:	Thu, 31 Jul 2008 17:12:26 +0400
From:	Oleg Nesterov <oleg@...sign.ru>
To:	Dmitry Adamushko <dmitry.adamushko@...il.com>
Cc:	Linus Torvalds <torvalds@...ux-foundation.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Ingo Molnar <mingo@...e.hu>,
	Roland McGrath <roland@...hat.com>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/3] wait_task_inactive: don't use the dummy version when !SMP && PREEMPT

On 07/30, Dmitry Adamushko wrote:
>
> 2008/7/30 Linus Torvalds <torvalds@...ux-foundation.org>:
> >
> > Oh, and shouldn't it do a "yield()" instead of a cpu_relax() on UP?
>
> This part could have been skipped for UP. task_running(rq, p) just
> can't give 'true' for UP (otherwise it's a bug). The only relevant
> part is "on_rq = p->se.on_rq".

That was my understanding, thanks for your confirmation.

> (if [1], then I think a separate function for PREEMPT would look
> better, i.e. without parts with task_running())

in that case we can also eliminate task_rq_lock() afaics.

> e.g. consider this code from kthread_bind():
>
>         /* Must have done schedule() in kthread() before we set_task_cpu */
>         wait_task_inactive(k, 0);
>
>         set_task_cpu(k, cpu);
>         k->cpus_allowed = cpumask_of_cpu(cpu);
>         k->rt.nr_cpus_allowed = 1;
>         k->flags |= PF_THREAD_BOUND;
>
> set_task_cpu(k, cpu) is safe _only_ if 'k' is not on the run-queue
> (and can't be placed onto it behind our back -- heh, a bit subtle).
>
> Now, for !SMP + PREEMPT it's not a case. set_task_cpu() may be called
> while 'k' is still on the run-queue (more precisely, preempted in
> kthread() between complete(&create->started); and schedule();).
>
> Yes, set_task_cpu() is a "nop" for UP so that's ok in this particular
> case. But let's suppose, another use-case would be introduced with
> 'false' assumptions causing troubles for !SMP.

Completely agreed.


Currently the only user which can suffer on UP && PREEMPT is
task_current_syscall(). As Roland suggested we can fix it if we change
the !SMP version to return ->nivcsw + ->nvcsw. But personally I don't
like the fact that we will have the subltle difference in behaviour
depending on CONFIG_SMP. We have to wait for .on_rq == 0 on SMP, imho
it is better to do the same on UP even if none of the current callers
needs this.

That said, I think this problem is minor and I don't (and can't) have
the strong opinion on how to fix it. I'd better wait for the authoritative
verdict.

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/