linux-kernel - Re: [patch 04/41] cpu ops: Core piece for generic atomic per cpu operations

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: <Pine.LNX.4.64.0806092012240.13024@schroedinger.engr.sgi.com>
Date:	Mon, 9 Jun 2008 20:18:25 -0700 (PDT)
From:	Christoph Lameter <clameter@....com>
To:	Rusty Russell <rusty@...tcorp.com.au>
cc:	Mike Travis <travis@....com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	linux-arch@...r.kernel.org, linux-kernel@...r.kernel.org,
	David Miller <davem@...emloft.net>,
	Eric Dumazet <dada1@...mosbay.com>,
	Peter Zijlstra <peterz@...radead.org>
Subject: Re: [patch 04/41] cpu ops: Core piece for generic atomic per cpu
 operations

On Tue, 10 Jun 2008, Rusty Russell wrote:

> > Right that is what the cpu alloc patches do. So you could implement
> > cpu_local_inc on top of some of the cpu alloc patches.
> 
> Or you could just implement it today as a standalone patch.

You need at least the zero basing to enable the use of the segment 
register on x86_64.

> > But then the whole point of local_t is gone. Why not use atomic_t in the
> > first place?
> 
> Because some archs can do better.

The argument does not make any sense. First you want to use atomic_t then 
not?

> > > By definition if the caller cared, they would have had premption
> > > disabled.
> >
> > There are numerous instances where the caller does not care about
> > preemption. Its just important that one per cpu counter is increment in
> > the least intrusive way. See f.e. the VM event counters.
> 
> Yes, and that's exactly the point.  The VM event counters are exactly a case 
> where you should have used cpu_local_inc.

I tried it and did not give any benefit except first failing due to bugs 
because local_t did not disable preempt6... This led to Andi fixing 
local_t.

But with the preempt disabling I could not discern what the benefit 
would be.

CPU_INC does not require disabling of preempt and the cpu alloc patches 
shorten the code sequence to increment a VM counter significantly.

Here is the header from the patch. How would cpu_local_inc be able to do 
better unless you adopt this patchset and add a shim layer?

Subject: VM statistics: Use CPU ops

The use of CPU ops here avoids the offset calculations that we used to 
have to do with per cpu operations. The result of this patch is that event 
counters are coded with a single instruction the following way:

        incq   %gs:offset(%rip)

Without these patches this was:

        mov    %gs:0x8,%rdx
        mov    %eax,0x38(%rsp)
        mov    xxx(%rip),%eax
        mov    %eax,0x48(%rsp)
        mov    varoffset,%rax
        incq   0x110(%rax,%rdx,1)

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