linux-kernel - Re: [PATCH] Fix WARN_ON / WARN_ON

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: <200610050417.39518.ajwade@cpe001346162bf9-cm0011ae8cd564.cpe.net.cable.rogers.com>
Date:	Thu, 5 Oct 2006 04:13:07 -0400
From:	Andrew James Wade <andrew.j.wade@...il.com>
To:	Andrew Morton <akpm@...l.org>
Cc:	andrew.j.wade@...il.com, tim.c.chen@...ux.intel.com,
	herbert@...dor.apana.org.au, linux-kernel@...r.kernel.org,
	leonid.i.ananiev@...el.com
Subject: Re: [PATCH] Fix WARN_ON / WARN_ON_ONCE regression

On Wednesday 04 October 2006 18:06, Andrew Morton wrote:
> On Wed, 4 Oct 2006 12:47:00 -0400
> Andrew James Wade <andrew.j.wade@...il.com> wrote:
> 
> > On Tuesday 03 October 2006 23:32, Andrew Morton wrote:
> > 
> > > It might help, but we still don't know what's going on (I think).
> > > 
> > > I mean, if cache misses against __warn_once were sufficiently high for it
> > > to affect performance, then __warn_once would be, err, in cache?
> > 
> > Yes, of course. I'm embarrassed.
> > 
> > I took a look at the generated code, and GCC is having difficulty
> > optimizing WARN_ON_ONCE. Here is the start of __local_bh_enable:
> > 
> > 00000130 <__local_bh_enable>:
> >  130:   83 ec 10                sub    $0x10,%esp
> >  133:   8b 15 04 00 00 00       mov    0x4,%edx         <-+
> >  139:   89 e0                   mov    %esp,%eax          |
> >  13b:   25 00 e0 ff ff          and    $0xffffe000,%eax   | !!!
> >  140:   8b 40 14                mov    0x14(%eax),%eax    |
> >  143:   25 00 00 ff 0f          and    $0xfff0000,%eax    |
> 
> This is the evaluation of in_irq(): calculate `current', grab
> current->thread_info->preempt_count.

Actually I was confusing "mov 0x4,%edx" with "mov $0x4,%edx". That
code's fine (albeit unlinked). There are stupid inefficiencies in some
of the other code, but nothing really stood out at me in
__local_bh_enable, _local_bh_enable, or local_bh_Enable. 

(from earlier)
> Perhaps the `static int __warn_once' is getting put in the same cacheline
> as some frequently-modified thing.

hmm:

00000460 l     O .data  00000044 task_exit_notifier
000004c0 l     O .data  0000002c task_free_notifier
000004ec l     O .data  00000004 warnlimit.15904
000004f0 l     O .data  00000004 firsttime.15774
000004f4 l     O .data  00000004 __warn_once.15180
000004f8 l     O .data  00000004 __warn_once.15174
000004fc l     O .data  00000004 __warn_once.15213
00000500 l     O .data  00000004 __warn_once.15207
00000504 l     O .data  00000004 __warn_once.15145
00000508 l     O .data  00000004 __warn_once.15309
0000050c l     O .data  00000004 __warn_once.15256
00000510 l     O .data  00000004 __warn_once.15250
000005a0 l     O .data  0000006c proc_iomem_operations
(extracted from objdump -t kernel/built-in.o)

warnlimit and firsttime are fine, and proc_iomem_operations is
presumably fine as well. But I'm not so sure task_free_notifier is
infrequently modified. But that's just my .config and I'm out of my depth.

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