linux-kernel - Re: [PATCH 1/4 v2] nmi: Provide the option to issue an NMI back trace to every cpu but current

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]

Message-ID: <20140421154746.GZ8488@redhat.com>
Date:	Mon, 21 Apr 2014 11:47:46 -0400
From:	Don Zickus <dzickus@...hat.com>
To:	Oleg Nesterov <oleg@...hat.com>
Cc:	Aaron Tomlin <atomlin@...hat.com>, mingo@...hat.com,
	mingo@...nel.com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/4 v2] nmi: Provide the option to issue an NMI back
 trace to every cpu but current

On Mon, Apr 21, 2014 at 03:41:54PM +0200, Oleg Nesterov wrote:
> On 04/21, Don Zickus wrote:
> >
> > On Tue, Apr 15, 2014 at 07:26:49PM +0200, Oleg Nesterov wrote:
> > > On 04/15, Oleg Nesterov wrote:
> > > >
> > > > Looking at https://lkml.org/lkml/2014/4/4/469... It seems that 2/4 can be
> > > > simplified, you can simply remove smp_processor_id() from backtrace_mask
> > > > if !include_self and use apic->send_IPI_mask(backtrace_mask). But this is
> > > > minor, I won't insist.
> > >
> > > And in fact, I do not understand why arch_trigger_all_cpu_backtrace() doesn't
> > > disable preemption. OK, probably we can simply ignore the race with cpu hotplug.
> > >
> > > But it seems that your patch makes the things worse. Lets look at, say,
> > > numachip_send_IPI_mask_allbutself(). The usage of smp_processor_id() is
> > > obviously racy but perhaps we do not care again. But we do not want a warning
> > > from debug_smp_processor_id().
> >
> > Good point.  I forgot that going from all cpus down to allbutself,
> > preemption now matters.
> 
> I am not sure it actually matters wrt "show other CPU's traces". If the preemption
> is possible then the caller can be preempted even before it sends ipi.
> 
> OTOH I think it does matter anyway, even without your patch, otherwise the usage
> of cpu_online_mask is racy and we can hit the "Wait for up to 10 seconds" case.

Hmm,  I understand what you are saying now.

> 
> Btw...
> 
> 	/* Wait for up to 10 seconds for all CPUs to do the backtrace */
> 	for (i = 0; i < 10 * 1000; i++) {
> 		if (cpumask_empty(to_cpumask(backtrace_mask)))
> 			break;
> 		mdelay(1);
> 	}
> 
> OK, but perhaps we should clear backtrace_mask if we return due to timeout.

I can look at that.

> 
> > does disabling preemption help in the cpu
> > hotplug case?
> 
> Yes. But I'd suggest to change your patch to use get_cpu() instead of
> preempt_disable/smp_processor_id.

ok.  Originally I was thinking of the remote hotplug cpu case, which
pre-emption won't block.  But forgot about the local cpu hotplug case.


> 
> And I think it would be better to not discuss this off-list, I added lkml.
> 
> Oleg.
> 

Thanks!

Cheers,
Don

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