All of lore.kernel.org
 help / color / mirror / Atom feed
From: Russell King - ARM Linux <linux@arm.linux.org.uk>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: "Wang, Yalin" <Yalin.Wang@sonymobile.com>,
	"'arnd@arndb.de'" <arnd@arndb.de>,
	"'linux-arch@vger.kernel.org'" <linux-arch@vger.kernel.org>,
	"'linux-kernel@vger.kernel.org'" <linux-kernel@vger.kernel.org>,
	"'linux-arm-kernel@lists.infradead.org'"
	<linux-arm-kernel@lists.infradead.org>
Subject: Re: [RFC] change non-atomic bitops method
Date: Mon, 2 Feb 2015 23:31:36 +0000	[thread overview]
Message-ID: <20150202233136.GL8656@n2100.arm.linux.org.uk> (raw)
In-Reply-To: <20150202152909.13bfd11f192fb0268b2ab4bf@linux-foundation.org>

On Mon, Feb 02, 2015 at 03:29:09PM -0800, Andrew Morton wrote:
> On Mon, 2 Feb 2015 11:55:03 +0800 "Wang, Yalin" <Yalin.Wang@sonymobile.com> wrote:
> 
> > This patch change non-atomic bitops,
> > add a if() condition to test it, before set/clear the bit.
> > so that we don't need dirty the cache line, if this bit
> > have been set or clear. On SMP system, dirty cache line will
> > need invalidate other processors cache line, this will have
> > some impact on SMP systems.
> > 
> > --- a/include/asm-generic/bitops/non-atomic.h
> > +++ b/include/asm-generic/bitops/non-atomic.h
> > @@ -17,7 +17,9 @@ static inline void __set_bit(int nr, volatile unsigned long *addr)
> >  	unsigned long mask = BIT_MASK(nr);
> >  	unsigned long *p = ((unsigned long *)addr) + BIT_WORD(nr);
> >  
> > -	*p  |= mask;
> > +	if ((*p & mask) == 0)
> > +		*p  |= mask;
> > +
> >  }
> 
> hm, maybe.
> 
> It will speed up set_bit on an already-set bit.  But it will slow down
> set_bit on a not-set bit.  And the latter case is presumably much, much
> more common.
> 
> How do we know the patch is a net performance gain?

Yes, we do need to know the performance impact of changes like this -
as Laura said in her reply already...

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

WARNING: multiple messages have this Message-ID (diff)
From: linux@arm.linux.org.uk (Russell King - ARM Linux)
To: linux-arm-kernel@lists.infradead.org
Subject: [RFC] change non-atomic bitops method
Date: Mon, 2 Feb 2015 23:31:36 +0000	[thread overview]
Message-ID: <20150202233136.GL8656@n2100.arm.linux.org.uk> (raw)
In-Reply-To: <20150202152909.13bfd11f192fb0268b2ab4bf@linux-foundation.org>

On Mon, Feb 02, 2015 at 03:29:09PM -0800, Andrew Morton wrote:
> On Mon, 2 Feb 2015 11:55:03 +0800 "Wang, Yalin" <Yalin.Wang@sonymobile.com> wrote:
> 
> > This patch change non-atomic bitops,
> > add a if() condition to test it, before set/clear the bit.
> > so that we don't need dirty the cache line, if this bit
> > have been set or clear. On SMP system, dirty cache line will
> > need invalidate other processors cache line, this will have
> > some impact on SMP systems.
> > 
> > --- a/include/asm-generic/bitops/non-atomic.h
> > +++ b/include/asm-generic/bitops/non-atomic.h
> > @@ -17,7 +17,9 @@ static inline void __set_bit(int nr, volatile unsigned long *addr)
> >  	unsigned long mask = BIT_MASK(nr);
> >  	unsigned long *p = ((unsigned long *)addr) + BIT_WORD(nr);
> >  
> > -	*p  |= mask;
> > +	if ((*p & mask) == 0)
> > +		*p  |= mask;
> > +
> >  }
> 
> hm, maybe.
> 
> It will speed up set_bit on an already-set bit.  But it will slow down
> set_bit on a not-set bit.  And the latter case is presumably much, much
> more common.
> 
> How do we know the patch is a net performance gain?

Yes, we do need to know the performance impact of changes like this -
as Laura said in her reply already...

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

  reply	other threads:[~2015-02-02 23:31 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-02-02  3:55 [RFC] change non-atomic bitops method Wang, Yalin
2015-02-02  3:55 ` Wang, Yalin
2015-02-02 18:53 ` Laura Abbott
2015-02-02 18:53   ` Laura Abbott
2015-02-02 19:31 ` Uwe Kleine-König
2015-02-02 19:31   ` Uwe Kleine-König
2015-02-03 15:14   ` David Howells
2015-02-03 15:14     ` David Howells
2015-02-03 15:14     ` David Howells
2015-02-03 19:10     ` Uwe Kleine-König
2015-02-03 19:10       ` Uwe Kleine-König
2015-02-02 23:29 ` Andrew Morton
2015-02-02 23:29   ` Andrew Morton
2015-02-02 23:31   ` Russell King - ARM Linux [this message]
2015-02-02 23:31     ` Russell King - ARM Linux
2015-02-03  1:17   ` Kirill A. Shutemov
2015-02-03  1:17     ` Kirill A. Shutemov
2015-02-03  2:13     ` Wang, Yalin
2015-02-03  2:13       ` Wang, Yalin
2015-02-03  5:42       ` Wang, Yalin
2015-02-03  5:42         ` Wang, Yalin
2015-02-03  6:38         ` Andrew Morton
2015-02-03  6:38           ` Andrew Morton
2015-02-03  7:03           ` Wang, Yalin
2015-02-03  7:03             ` Wang, Yalin
2015-02-03  8:42             ` Wang, Yalin
2015-02-03  8:42               ` Wang, Yalin
2015-02-03 10:59               ` Andrew Morton
2015-02-03 10:59                 ` Andrew Morton
2015-02-09  8:18                 ` Wang, Yalin
2015-02-09  8:18                   ` Wang, Yalin
2015-02-09 20:34                   ` Andrew Morton
2015-02-09 20:34                     ` Andrew Morton
2015-02-10  7:05                     ` Wang, Yalin
2015-02-10  7:05                       ` Wang, Yalin
2015-02-09 21:42                   ` Rasmus Villemoes
2015-02-09 21:42                     ` Rasmus Villemoes
2015-02-09 21:42                     ` Rasmus Villemoes
2015-02-03  8:40           ` David Miller
2015-02-03  8:40             ` David Miller
2015-02-03  8:48             ` Andrew Morton
2015-02-03  8:48               ` Andrew Morton
2015-02-03  9:34           ` Rasmus Villemoes
2015-02-03  9:34             ` Rasmus Villemoes
2015-02-03  9:34             ` Rasmus Villemoes
2015-02-03  9:41             ` Wang, Yalin
2015-02-03  9:41               ` Wang, Yalin
2015-02-03 10:39     ` Kirill A. Shutemov
2015-02-03 10:39       ` Kirill A. Shutemov

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=20150202233136.GL8656@n2100.arm.linux.org.uk \
    --to=linux@arm.linux.org.uk \
    --cc=Yalin.Wang@sonymobile.com \
    --cc=akpm@linux-foundation.org \
    --cc=arnd@arndb.de \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.