Richard Weinberger | 12 Apr 2011 16:09
Picon

um: this_cpu_cmpxchg16b_emu

Hi,

This patch implements this_cpu_cmpxchg16b_emu() for UML.
As I'm not an amd64 assembly guru I'm not sure whether the assembly part is
correct.
Can someone please review it?

Especially I'm unsure which register have to be saved before each 
call to C functions.

Thanks,
//richard

From 202b0efe024d7c0500e7c11f0aa105f7a1fafb9b Mon Sep 17 00:00:00 2001
From: Richard Weinberger <richard <at> nod.at>
Date: Mon, 11 Apr 2011 19:22:38 +0200
Subject: [PATCH] um: Implement this_cpu_cmpxchg16b_emu()

Commit 8a5ec0ba "Lockless (and preemptless) fastpaths for slub"
makes use of this_cpu_cmpxchg_double() which needs
this_cpu_cmpxchg16b_emu() on x86_64.
User Mode Linux has to serve this function too.

Reported-by: Sergei Trofimovich <slyich <at> gmail.com>
Signed-off-by: Richard Weinberger <richard <at> nod.at>
---
 arch/um/sys-x86_64/Makefile            |    2 +-
 arch/um/sys-x86_64/cmpxchg16b_emu.S    |   72 ++++++++++++++++++++++++++++++++
 arch/um/sys-x86_64/cmpxchg16b_helper.c |   22 ++++++++++
 3 files changed, 95 insertions(+), 1 deletions(-)
(Continue reading)

Christoph Lameter | 12 Apr 2011 20:10
Picon

Re: um: this_cpu_cmpxchg16b_emu

On Tue, 12 Apr 2011, Richard Weinberger wrote:

> This patch implements this_cpu_cmpxchg16b_emu() for UML.

Is this really necessary? Just undefine CONFIG_CMPXCHG_LOCAL for UML and
the asm code will not be used.

Richard Weinberger | 12 Apr 2011 20:41
Picon

Re: um: this_cpu_cmpxchg16b_emu

Am Dienstag 12 April 2011, 20:10:37 schrieb Christoph Lameter:
> On Tue, 12 Apr 2011, Richard Weinberger wrote:
> > This patch implements this_cpu_cmpxchg16b_emu() for UML.
> 
> Is this really necessary? Just undefine CONFIG_CMPXCHG_LOCAL for UML and
> the asm code will not be used.

UML includes arch/x86/Kconfig.cpu which defines CONFIG_CMPXCHG_LOCAL automatically.
Just disabling CONFIG_CMPXCHG_LOCAL for UML is IMHO not very nice.
When chpxchg is available also UML should use it...

Thanks,
//richard
Tejun Heo | 12 Apr 2011 21:20

Re: um: this_cpu_cmpxchg16b_emu

On Tue, Apr 12, 2011 at 08:41:11PM +0200, Richard Weinberger wrote:
> Am Dienstag 12 April 2011, 20:10:37 schrieb Christoph Lameter:
> > On Tue, 12 Apr 2011, Richard Weinberger wrote:
> > > This patch implements this_cpu_cmpxchg16b_emu() for UML.
> > 
> > Is this really necessary? Just undefine CONFIG_CMPXCHG_LOCAL for UML and
> > the asm code will not be used.
> 
> UML includes arch/x86/Kconfig.cpu which defines CONFIG_CMPXCHG_LOCAL automatically.
> Just disabling CONFIG_CMPXCHG_LOCAL for UML is IMHO not very nice.
> When chpxchg is available also UML should use it...

Ugh... I'd really like to avoid things like this for UML.  Is there
any SLUB performance sensitive workload running on UML?  I've never
seen any UML in production environment.  Wouldn't it be better to keep
things simple?

Thanks.

--

-- 
tejun
Pekka Enberg | 12 Apr 2011 21:22
Gravatar

Re: um: this_cpu_cmpxchg16b_emu

On Tue, Apr 12, 2011 at 08:41:11PM +0200, Richard Weinberger wrote:
>> Am Dienstag 12 April 2011, 20:10:37 schrieb Christoph Lameter:
>> > On Tue, 12 Apr 2011, Richard Weinberger wrote:
>> > > This patch implements this_cpu_cmpxchg16b_emu() for UML.
>> >
>> > Is this really necessary? Just undefine CONFIG_CMPXCHG_LOCAL for UML and
>> > the asm code will not be used.
>>
>> UML includes arch/x86/Kconfig.cpu which defines CONFIG_CMPXCHG_LOCAL automatically.
>> Just disabling CONFIG_CMPXCHG_LOCAL for UML is IMHO not very nice.
>> When chpxchg is available also UML should use it...

On Tue, Apr 12, 2011 at 10:20 PM, Tejun Heo <tj <at> kernel.org> wrote:
> Ugh... I'd really like to avoid things like this for UML.  Is there
> any SLUB performance sensitive workload running on UML?  I've never
> seen any UML in production environment.  Wouldn't it be better to keep
> things simple?

Yes, it would be. :-)
Richard Weinberger | 12 Apr 2011 22:27
Picon

Re: um: this_cpu_cmpxchg16b_emu

Am Dienstag 12 April 2011, 21:22:47 schrieb Pekka Enberg:
> On Tue, Apr 12, 2011 at 08:41:11PM +0200, Richard Weinberger wrote:
> >> Am Dienstag 12 April 2011, 20:10:37 schrieb Christoph Lameter:
> >> > On Tue, 12 Apr 2011, Richard Weinberger wrote:
> >> > > This patch implements this_cpu_cmpxchg16b_emu() for UML.
> >> > 
> >> > Is this really necessary? Just undefine CONFIG_CMPXCHG_LOCAL for UML
> >> > and the asm code will not be used.
> >> 
> >> UML includes arch/x86/Kconfig.cpu which defines CONFIG_CMPXCHG_LOCAL
> >> automatically. Just disabling CONFIG_CMPXCHG_LOCAL for UML is IMHO not
> >> very nice. When chpxchg is available also UML should use it...
> 
> On Tue, Apr 12, 2011 at 10:20 PM, Tejun Heo <tj <at> kernel.org> wrote:
> > Ugh... I'd really like to avoid things like this for UML.  Is there
> > any SLUB performance sensitive workload running on UML?  I've never
> > seen any UML in production environment.  Wouldn't it be better to keep
> > things simple?
> 
> Yes, it would be. :-)

Okay. Then let's keep it simple. :-)
I'll disable CONFIG_CMPXCHG_LOCAL for UML.

Has someone looked at my this_cpu_cmpxchg16b_emu() implementation,
is it correct? Especially the call to C stuff.
I've tested it, it works fine.

Thanks,
//richard
(Continue reading)


Gmane