linux-kernel - Re: [kernel.org users] [KORG] Panics on master backend

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: <20110825135429.GA32048@redhat.com>
Date:	Thu, 25 Aug 2011 15:54:29 +0200
From:	Oleg Nesterov <oleg@...hat.com>
To:	Peter Zijlstra <peterz@...radead.org>
Cc:	Frank Rowand <frank.rowand@...sony.com>,
	linux-kernel <linux-kernel@...r.kernel.org>, users@...nel.org,
	hch <hch@...radead.org>, "yong.zhang0" <yong.zhang0@...il.com>,
	scameron@...rdog.cce.hp.com,
	"James E.J. Bottomley" <jejb@...isc-linux.org>,
	Jens Axboe <jaxboe@...ionio.com>,
	Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [kernel.org users] [KORG] Panics on master backend

On 08/25, Peter Zijlstra wrote:
>
> On Wed, 2011-08-24 at 18:08 +0200, Oleg Nesterov wrote:
> > >
> > >  static void ttwu_queue(struct task_struct *p, int cpu)
> > > @@ -2705,7 +2703,6 @@ try_to_wake_up(struct task_struct *p, unsigned int state, int wake_flags)
> > >  	 * this task as prev, wait until its done referencing the task.
> > >  	 */
> > >  	while (p->on_cpu) {
> > > -#ifdef __ARCH_WANT_INTERRUPTS_ON_CTXSW
> > >  		/*
> > >  		 * In case the architecture enables interrupts in
> > >  		 * context_switch(), we cannot busy wait, since that
> > > @@ -2713,11 +2710,11 @@ try_to_wake_up(struct task_struct *p, unsigned int state, int wake_flags)
> > >  		 * tries to wake up @prev. So bail and do a complete
> > >  		 * remote wakeup.
> > >  		 */
> > > -		if (ttwu_activate_remote(p, wake_flags))
> > > +		if (cpu == smp_processor_id() &&
> >
> > I think this needs "task_cpu(p) == smp_processor_id()". We can't trust
> > "cpu", task_cpu() was called before ->on_rq check.
>
> Isn't us holding ->pi_lock sufficient to stabilize task_cpu()? If its a
> running task the initial ->state check would have failed,

Of course it is not TASK_RUNNING, but it can be running or not.

> and thus its a
> proper wakeup when we get here and thus ->pi_lock is serializing things.

I am not sure. If ->on_rq is true, we need rq->lock. Say, pull_task() can
change its cpu.

> > 	--- x/kernel/sched.c
> > 	+++ x/kernel/sched.c
> > 	@@ -2694,10 +2694,11 @@ try_to_wake_up(struct task_struct *p, un
> > 			goto out;
> > 	 
> > 		success = 1; /* we're going to change ->state */
> > 	-	cpu = task_cpu(p);
> > 	 
> > 	-	if (p->on_rq && ttwu_remote(p, wake_flags))
> > 	+	if (p->on_rq && ttwu_remote(p, wake_flags)) {
> > 	+		cpu = task_cpu(p); /* for ttwu_stat() */
> > 			goto stat;
> > 	+	}
> > 	 
> > 	 #ifdef CONFIG_SMP
> > 		/*
>
> Would result in the same problem as below...

I see, thanks.

Yes, ttwu_queue(p, cpu) needs this task_cpu() without CONFIG_SMP.

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/