All of lore.kernel.org
 help / color / mirror / Atom feed
From: James Hogan <james.hogan@imgtec.com>
To: Leonid Yegoshin <Leonid.Yegoshin@imgtec.com>,
	<linux-mips@linux-mips.org>, <benh@kernel.crashing.org>,
	<will.deacon@arm.com>, <linux-kernel@vger.kernel.org>,
	<ralf@linux-mips.org>, <markos.chandras@imgtec.com>,
	<macro@linux-mips.org>, <Steven.Hill@imgtec.com>,
	<alexander.h.duyck@redhat.com>, <davem@davemloft.net>
Subject: Re: [PATCH 3/3] MIPS: bugfix - replace smp_mb with release barrier function in unlocks
Date: Tue, 2 Jun 2015 12:42:40 +0100	[thread overview]
Message-ID: <556D96B0.3050409@imgtec.com> (raw)
In-Reply-To: <20150602000952.6668.82483.stgit@ubuntu-yegoshin>

[-- Attachment #1: Type: text/plain, Size: 1757 bytes --]

On 02/06/15 01:09, Leonid Yegoshin wrote:
> Repleace smp_mb() in arch_write_unlock() and __clear_bit_unlock() to

Replace.

> smp_mb__before_llsc() call which does "release" barrier functionality.
> 
> It seems like it was missed in commit f252ffd50c97dae87b45f1dbad24f71358ccfbd6
> during introduction of "acquire" and "release" semantics.
> 
> Signed-off-by: Leonid Yegoshin <Leonid.Yegoshin@imgtec.com>
> ---
>  arch/mips/include/asm/bitops.h   |    2 +-
>  arch/mips/include/asm/spinlock.h |    2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/mips/include/asm/bitops.h b/arch/mips/include/asm/bitops.h
> index 0cf29bd5dc5c..ce9666cf1499 100644
> --- a/arch/mips/include/asm/bitops.h
> +++ b/arch/mips/include/asm/bitops.h
> @@ -469,7 +469,7 @@ static inline int test_and_change_bit(unsigned long nr,
>   */
>  static inline void __clear_bit_unlock(unsigned long nr, volatile unsigned long *addr)
>  {
> -	smp_mb();
> +	smp_mb__before_llsc();
>  	__clear_bit(nr, addr);
>  }
>  
> diff --git a/arch/mips/include/asm/spinlock.h b/arch/mips/include/asm/spinlock.h
> index 1fca2e0793dc..7c7f3b2bd3de 100644
> --- a/arch/mips/include/asm/spinlock.h
> +++ b/arch/mips/include/asm/spinlock.h
> @@ -317,7 +317,7 @@ static inline void arch_write_lock(arch_rwlock_t *rw)
>  
>  static inline void arch_write_unlock(arch_rwlock_t *rw)
>  {
> -	smp_mb();
> +	smp_mb__before_llsc();

arch_write_unlock appears to just use sw, not sc, and __clear_bit
appears to be implemented in plain C, so is smp_mb__before_llsc() really
appropriate? Would smp_release() be more accurate/correct in both cases?

Cheers
James

>  
>  	__asm__ __volatile__(
>  	"				# arch_write_unlock	\n"
> 
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

WARNING: multiple messages have this Message-ID (diff)
From: James Hogan <james.hogan@imgtec.com>
To: Leonid Yegoshin <Leonid.Yegoshin@imgtec.com>,
	linux-mips@linux-mips.org, benh@kernel.crashing.org,
	will.deacon@arm.com, linux-kernel@vger.kernel.org,
	ralf@linux-mips.org, markos.chandras@imgtec.com,
	macro@linux-mips.org, Steven.Hill@imgtec.com,
	alexander.h.duyck@redhat.com, davem@davemloft.net
Subject: Re: [PATCH 3/3] MIPS: bugfix - replace smp_mb with release barrier function in unlocks
Date: Tue, 2 Jun 2015 12:42:40 +0100	[thread overview]
Message-ID: <556D96B0.3050409@imgtec.com> (raw)
Message-ID: <20150602114240.E9XkhpWtSw0iLXfCSAuFbJTebgYf_Q3tiW8rlYkmWVM@z> (raw)
In-Reply-To: <20150602000952.6668.82483.stgit@ubuntu-yegoshin>

[-- Attachment #1: Type: text/plain, Size: 1757 bytes --]

On 02/06/15 01:09, Leonid Yegoshin wrote:
> Repleace smp_mb() in arch_write_unlock() and __clear_bit_unlock() to

Replace.

> smp_mb__before_llsc() call which does "release" barrier functionality.
> 
> It seems like it was missed in commit f252ffd50c97dae87b45f1dbad24f71358ccfbd6
> during introduction of "acquire" and "release" semantics.
> 
> Signed-off-by: Leonid Yegoshin <Leonid.Yegoshin@imgtec.com>
> ---
>  arch/mips/include/asm/bitops.h   |    2 +-
>  arch/mips/include/asm/spinlock.h |    2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/mips/include/asm/bitops.h b/arch/mips/include/asm/bitops.h
> index 0cf29bd5dc5c..ce9666cf1499 100644
> --- a/arch/mips/include/asm/bitops.h
> +++ b/arch/mips/include/asm/bitops.h
> @@ -469,7 +469,7 @@ static inline int test_and_change_bit(unsigned long nr,
>   */
>  static inline void __clear_bit_unlock(unsigned long nr, volatile unsigned long *addr)
>  {
> -	smp_mb();
> +	smp_mb__before_llsc();
>  	__clear_bit(nr, addr);
>  }
>  
> diff --git a/arch/mips/include/asm/spinlock.h b/arch/mips/include/asm/spinlock.h
> index 1fca2e0793dc..7c7f3b2bd3de 100644
> --- a/arch/mips/include/asm/spinlock.h
> +++ b/arch/mips/include/asm/spinlock.h
> @@ -317,7 +317,7 @@ static inline void arch_write_lock(arch_rwlock_t *rw)
>  
>  static inline void arch_write_unlock(arch_rwlock_t *rw)
>  {
> -	smp_mb();
> +	smp_mb__before_llsc();

arch_write_unlock appears to just use sw, not sc, and __clear_bit
appears to be implemented in plain C, so is smp_mb__before_llsc() really
appropriate? Would smp_release() be more accurate/correct in both cases?

Cheers
James

>  
>  	__asm__ __volatile__(
>  	"				# arch_write_unlock	\n"
> 
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

  reply	other threads:[~2015-06-02 11:42 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-06-02  0:09 [PATCH 0/3] MIPS: SMP memory barriers: lightweight sync, acquire-release Leonid Yegoshin
2015-06-02  0:09 ` Leonid Yegoshin
2015-06-02  0:09 ` [PATCH 1/3] MIPS: R6: Use lightweight SYNC instruction in smp_* memory barriers Leonid Yegoshin
2015-06-02  0:09   ` Leonid Yegoshin
2015-06-02 10:08   ` Paul Burton
2015-06-02 10:08     ` Paul Burton
2015-06-02 12:12     ` Luc Van Oostenryck
2015-06-02 12:44       ` Ralf Baechle
2015-06-02 18:20       ` Leonid Yegoshin
2015-06-02 18:20         ` Leonid Yegoshin
2015-06-02 10:48   ` James Hogan
2015-06-02 10:48     ` James Hogan
2015-06-02 16:15     ` Maciej W. Rozycki
2015-06-02 23:56       ` David Daney
2015-06-03  1:56         ` Leonid Yegoshin
2015-06-03  1:56           ` Leonid Yegoshin
2015-06-05 13:10   ` Ralf Baechle
2015-06-05 21:18     ` Maciej W. Rozycki
2016-01-28  2:28     ` Joshua Kinard
2016-01-29 13:32       ` Maciej W. Rozycki
2016-01-29 13:32         ` Maciej W. Rozycki
2016-01-30 16:25         ` Joshua Kinard
2015-06-02  0:09 ` [PATCH 2/3] MIPS: enforce LL-SC loop enclosing with SYNC (ACQUIRE and RELEASE) Leonid Yegoshin
2015-06-02  0:09   ` Leonid Yegoshin
2015-06-02 11:39   ` James Hogan
2015-06-02 11:39     ` James Hogan
2015-06-02 18:43     ` Leonid Yegoshin
2015-06-02 18:43       ` Leonid Yegoshin
2015-06-02 18:53       ` Leonid Yegoshin
2015-06-02  0:09 ` [PATCH 3/3] MIPS: bugfix - replace smp_mb with release barrier function in unlocks Leonid Yegoshin
2015-06-02  0:09   ` Leonid Yegoshin
2015-06-02 11:42   ` James Hogan [this message]
2015-06-02 11:42     ` James Hogan
2015-06-02 13:22     ` Ralf Baechle
2015-06-02  8:41 ` [PATCH 0/3] MIPS: SMP memory barriers: lightweight sync, acquire-release Joshua Kinard
2015-06-02  9:59   ` Ralf Baechle
2015-06-02 18:59     ` Joshua Kinard
2015-06-02 19:19       ` Ralf Baechle

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=556D96B0.3050409@imgtec.com \
    --to=james.hogan@imgtec.com \
    --cc=Leonid.Yegoshin@imgtec.com \
    --cc=Steven.Hill@imgtec.com \
    --cc=alexander.h.duyck@redhat.com \
    --cc=benh@kernel.crashing.org \
    --cc=davem@davemloft.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mips@linux-mips.org \
    --cc=macro@linux-mips.org \
    --cc=markos.chandras@imgtec.com \
    --cc=ralf@linux-mips.org \
    --cc=will.deacon@arm.com \
    /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.