All of lore.kernel.org
 help / color / mirror / Atom feed
From: vladimir.murzin@arm.com (Vladimir Murzin)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] [bugfix] replace unnessary ldax with common ldr
Date: Wed, 31 Aug 2016 14:30:40 +0100	[thread overview]
Message-ID: <57C6DC00.2050709@arm.com> (raw)
In-Reply-To: <20160830090721.GC5163@e104818-lin.cambridge.arm.com>

On 30/08/16 10:07, Catalin Marinas wrote:
> On Tue, Aug 30, 2016 at 02:35:31PM +0800, Kenneth Lee wrote:
>> (add comment for the previous mail, sorry for the duplication)
>>
>> There is no store_ex pairing with this load_ex. It is not necessary and
>> gave wrong hint to the cache system.
>>
>> Signed-off-by: Kenneth Lee <liguozhu@hisilicon.com>
>> ---
>>  arch/arm64/include/asm/spinlock.h | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/include/asm/spinlock.h b/arch/arm64/include/asm/spinlock.h
>> index c85e96d..3334c4f 100644
>> --- a/arch/arm64/include/asm/spinlock.h
>> +++ b/arch/arm64/include/asm/spinlock.h
>> @@ -63,7 +63,7 @@ static inline void arch_spin_lock(arch_spinlock_t *lock)
>>  	 */
>>  "	sevl\n"
>>  "2:	wfe\n"
>> -"	ldaxrh	%w2, %4\n"
>> +"	ldrh	%w2, %4\n"
>>  "	eor	%w1, %w2, %w0, lsr #16\n"
>>  "	cbnz	%w1, 2b\n"
>>  	/* We got the lock. Critical section starts here. */
> 
> This is needed because the arch_spin_unlock() code only uses an STLR
> without an explicit SEV (like we have on AArch32). An event is
> automatically generated when the exclusive monitor is cleared by STLR.
> But without setting it with a load exclusive in arch_spin_lock() (even
> though it does not acquire the lock), there won't be anything to clear,
> hence no event to be generated. In this case, the WFE would wait
> indefinitely.
> 

Maybe worth to add this as a comment, no?

Cheers
Vladimir

WARNING: multiple messages have this Message-ID (diff)
From: Vladimir Murzin <vladimir.murzin@arm.com>
To: Catalin Marinas <catalin.marinas@arm.com>,
	Kenneth Lee <liguozhu@hisilicon.com>
Cc: Will Deacon <will.deacon@arm.com>,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH] [bugfix] replace unnessary ldax with common ldr
Date: Wed, 31 Aug 2016 14:30:40 +0100	[thread overview]
Message-ID: <57C6DC00.2050709@arm.com> (raw)
In-Reply-To: <20160830090721.GC5163@e104818-lin.cambridge.arm.com>

On 30/08/16 10:07, Catalin Marinas wrote:
> On Tue, Aug 30, 2016 at 02:35:31PM +0800, Kenneth Lee wrote:
>> (add comment for the previous mail, sorry for the duplication)
>>
>> There is no store_ex pairing with this load_ex. It is not necessary and
>> gave wrong hint to the cache system.
>>
>> Signed-off-by: Kenneth Lee <liguozhu@hisilicon.com>
>> ---
>>  arch/arm64/include/asm/spinlock.h | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/include/asm/spinlock.h b/arch/arm64/include/asm/spinlock.h
>> index c85e96d..3334c4f 100644
>> --- a/arch/arm64/include/asm/spinlock.h
>> +++ b/arch/arm64/include/asm/spinlock.h
>> @@ -63,7 +63,7 @@ static inline void arch_spin_lock(arch_spinlock_t *lock)
>>  	 */
>>  "	sevl\n"
>>  "2:	wfe\n"
>> -"	ldaxrh	%w2, %4\n"
>> +"	ldrh	%w2, %4\n"
>>  "	eor	%w1, %w2, %w0, lsr #16\n"
>>  "	cbnz	%w1, 2b\n"
>>  	/* We got the lock. Critical section starts here. */
> 
> This is needed because the arch_spin_unlock() code only uses an STLR
> without an explicit SEV (like we have on AArch32). An event is
> automatically generated when the exclusive monitor is cleared by STLR.
> But without setting it with a load exclusive in arch_spin_lock() (even
> though it does not acquire the lock), there won't be anything to clear,
> hence no event to be generated. In this case, the WFE would wait
> indefinitely.
> 

Maybe worth to add this as a comment, no?

Cheers
Vladimir

  reply	other threads:[~2016-08-31 13:30 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-08-30  6:35 [PATCH] [bugfix] replace unnessary ldax with common ldr Kenneth Lee
2016-08-30  6:35 ` Kenneth Lee
2016-08-30  9:07 ` Catalin Marinas
2016-08-30  9:07   ` Catalin Marinas
2016-08-31 13:30   ` Vladimir Murzin [this message]
2016-08-31 13:30     ` Vladimir Murzin
2016-09-01 10:20     ` Catalin Marinas
2016-09-01 10:20       ` Catalin Marinas
2016-09-01  3:44   ` 答复: " Liguozhu (Kenneth)
  -- strict thread matches above, loose matches on Subject: below --
2016-08-30  4:13 Kenneth Lee
2016-08-30  4:13 ` Kenneth Lee

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=57C6DC00.2050709@arm.com \
    --to=vladimir.murzin@arm.com \
    --cc=linux-arm-kernel@lists.infradead.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.