LKML Archive on lore.kernel.org
 help / color / Atom feed
From: Vineet Gupta <Vineet.Gupta1@synopsys.com>
To: Peter Zijlstra <peterz@infradead.org>,
	Davidlohr Bueso <davidlohr@hp.com>
Cc: Ingo Molnar <mingo@kernel.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Heiko Carstens <heiko.carstens@de.ibm.com>,
	Aswin Chandramouleeswaran <aswin@hp.com>,
	<linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] arch,locking: Ciao arch_mutex_cpu_relax()
Date: Mon, 23 Jun 2014 12:43:46 +0530
Message-ID: <53A7D3AA.9020100@synopsys.com> (raw)
In-Reply-To: <20140623065423.GE19860@laptop.programming.kicks-ass.net>

Hi Peter,

On Monday 23 June 2014 12:24 PM, Peter Zijlstra wrote:
> On Fri, Jun 20, 2014 at 11:21:13AM -0700, Davidlohr Bueso wrote:
>> diff --git a/arch/arc/include/asm/processor.h b/arch/arc/include/asm/processor.h
>> index d99f9b3..8e1bf6b 100644
>> --- a/arch/arc/include/asm/processor.h
>> +++ b/arch/arc/include/asm/processor.h
>> @@ -62,6 +62,8 @@ unsigned long thread_saved_pc(struct task_struct *t);
>>  #define cpu_relax()	do { } while (0)
>>  #endif
>>  
>> +#define arch_cpu_relax() cpu_relax()
>> +
>>  #define copy_segments(tsk, mm)      do { } while (0)
>>  #define release_segments(mm)        do { } while (0)
> 
> I'm not at all sure that cpu_relax() definition ARC has is valid. We
> rely on cpu_relax() being at least a barrier() all over the place, and
> it doesn't need to be SMP only. You can have a UP wait loop waiting for
> an interrupt for example.
> 
> Vineet?

Over the years we've not had any trouble with !SMP cpu_relax() being a no-op (and
barrier version was only required when we hit a hard hang in our our initial SMP
code). UP busy wait looping would be frowned upon in general.

However what we have now is just a code optimization quirk for !SMP since a
compiler barrier will cause gcc to dump out and reload scratch regs - specially
for our deep reg file.

Here's what I get with current UP kernel switching to compiler barrier

./scripts/bloat-o-meter vmlinux-pre-cpu-relax  vmlinux | head
add/remove: 1/0 grow/shrink: 75/5 up/down: 1218/-32 (1186)
function                                     old     new   delta
path_init                                    708     826    +118
sys_semtimedop                              2540    2640    +100
...
__slab_alloc.isra.constprop                  564     560      -4
deactivate_slab                              886     878      -8

So it doesn't look too bad, although I've not run any performance tests. We can
switch UP to barrier if you feel it is needed semantically.

-Vineet



  reply index

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-06-20 18:21 Davidlohr Bueso
2014-06-23  6:54 ` Peter Zijlstra
2014-06-23  7:13   ` Vineet Gupta [this message]
2014-06-23 11:34     ` Peter Zijlstra
2014-06-23  6:58 ` Peter Zijlstra
2014-06-24 15:06   ` Davidlohr Bueso
2014-06-25  6:25     ` Heiko Carstens
2014-06-25 11:57       ` Linus Torvalds
2014-06-29 22:09         ` [PATCH v2] " Davidlohr Bueso
2014-08-05 13:04           ` Geert Uytterhoeven
2014-08-05 17:42             ` Davidlohr Bueso
2014-08-08  1:18               ` Davidlohr Bueso
2014-08-08 10:45                 ` Guenter Roeck
2014-08-15 19:34                 ` Guenter Roeck

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=53A7D3AA.9020100@synopsys.com \
    --to=vineet.gupta1@synopsys.com \
    --cc=aswin@hp.com \
    --cc=davidlohr@hp.com \
    --cc=heiko.carstens@de.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=peterz@infradead.org \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git
	git clone --mirror https://lore.kernel.org/lkml/8 lkml/git/8.git
	git clone --mirror https://lore.kernel.org/lkml/9 lkml/git/9.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org
	public-inbox-index lkml

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git