* [PATCH/RFC] asm-generic/mutex-dec.h: add SMP support
@ 2011-05-30 3:19 Mike Frysinger
2011-05-30 7:00 ` Arnd Bergmann
2011-06-06 21:23 ` Andrew Morton
0 siblings, 2 replies; 6+ messages in thread
From: Mike Frysinger @ 2011-05-30 3:19 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: linux-kernel, Andrew Morton, linux-arch, uclinux-dist-devel
To make these guys work on SMP systems, we just need to sprinkle a few
barriers around.
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
note: this is what the Blackfin SMP port is using, but it doesn't seem
like other SMP ports are ... so I wonder if we're just trying too hard
and these barriers aren't actually necessary ?
include/asm-generic/mutex-dec.h | 8 +++++++-
1 files changed, 7 insertions(+), 1 deletions(-)
diff --git a/include/asm-generic/mutex-dec.h b/include/asm-generic/mutex-dec.h
index f104af7..e746c3c 100644
--- a/include/asm-generic/mutex-dec.h
+++ b/include/asm-generic/mutex-dec.h
@@ -22,6 +22,8 @@ __mutex_fastpath_lock(atomic_t *count, void (*fail_fn)(atomic_t *))
{
if (unlikely(atomic_dec_return(count) < 0))
fail_fn(count);
+ else
+ smp_mb();
}
/**
@@ -39,6 +41,7 @@ __mutex_fastpath_lock_retval(atomic_t *count, int (*fail_fn)(atomic_t *))
{
if (unlikely(atomic_dec_return(count) < 0))
return fail_fn(count);
+ smp_mb();
return 0;
}
@@ -58,6 +61,7 @@ __mutex_fastpath_lock_retval(atomic_t *count, int (*fail_fn)(atomic_t *))
static inline void
__mutex_fastpath_unlock(atomic_t *count, void (*fail_fn)(atomic_t *))
{
+ smp_mb();
if (unlikely(atomic_inc_return(count) <= 0))
fail_fn(count);
}
@@ -82,8 +86,10 @@ __mutex_fastpath_unlock(atomic_t *count, void (*fail_fn)(atomic_t *))
static inline int
__mutex_fastpath_trylock(atomic_t *count, int (*fail_fn)(atomic_t *))
{
- if (likely(atomic_cmpxchg(count, 1, 0) == 1))
+ if (likely(atomic_cmpxchg(count, 1, 0) == 1)) {
+ smp_mb();
return 1;
+ }
return 0;
}
--
1.7.5.rc3
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH/RFC] asm-generic/mutex-dec.h: add SMP support
2011-05-30 3:19 [PATCH/RFC] asm-generic/mutex-dec.h: add SMP support Mike Frysinger
@ 2011-05-30 7:00 ` Arnd Bergmann
2011-06-06 21:23 ` Andrew Morton
1 sibling, 0 replies; 6+ messages in thread
From: Arnd Bergmann @ 2011-05-30 7:00 UTC (permalink / raw)
To: Mike Frysinger
Cc: linux-kernel, Andrew Morton, linux-arch, uclinux-dist-devel
On Monday 30 May 2011 05:19:28 Mike Frysinger wrote:
> To make these guys work on SMP systems, we just need to sprinkle a few
> barriers around.
>
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> ---
> note: this is what the Blackfin SMP port is using, but it doesn't seem
> like other SMP ports are ... so I wonder if we're just trying too hard
> and these barriers aren't actually necessary ?
On some architectures, atomic instructions are implicit barriers. On
others, the cmpxchg() macro contains a barrier. I'm not sure if there
is other code relying on that.
Arnd
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH/RFC] asm-generic/mutex-dec.h: add SMP support
2011-05-30 3:19 [PATCH/RFC] asm-generic/mutex-dec.h: add SMP support Mike Frysinger
2011-05-30 7:00 ` Arnd Bergmann
@ 2011-06-06 21:23 ` Andrew Morton
2011-06-06 21:31 ` [uclinux-dist-devel] " Mike Frysinger
2011-06-06 21:32 ` Eric Dumazet
1 sibling, 2 replies; 6+ messages in thread
From: Andrew Morton @ 2011-06-06 21:23 UTC (permalink / raw)
To: Mike Frysinger
Cc: Arnd Bergmann, linux-kernel, linux-arch, uclinux-dist-devel, Nick Piggin
On Sun, 29 May 2011 23:19:28 -0400
Mike Frysinger <vapier@gentoo.org> wrote:
> To make these guys work on SMP systems, we just need to sprinkle a few
> barriers around.
>
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> ---
> note: this is what the Blackfin SMP port is using, but it doesn't seem
> like other SMP ports are ... so I wonder if we're just trying too hard
> and these barriers aren't actually necessary ?
>
> include/asm-generic/mutex-dec.h | 8 +++++++-
> 1 files changed, 7 insertions(+), 1 deletions(-)
>
> diff --git a/include/asm-generic/mutex-dec.h b/include/asm-generic/mutex-dec.h
> index f104af7..e746c3c 100644
> --- a/include/asm-generic/mutex-dec.h
> +++ b/include/asm-generic/mutex-dec.h
> @@ -22,6 +22,8 @@ __mutex_fastpath_lock(atomic_t *count, void (*fail_fn)(atomic_t *))
> {
> if (unlikely(atomic_dec_return(count) < 0))
> fail_fn(count);
> + else
> + smp_mb();
> }
>
> /**
> @@ -39,6 +41,7 @@ __mutex_fastpath_lock_retval(atomic_t *count, int (*fail_fn)(atomic_t *))
> {
> if (unlikely(atomic_dec_return(count) < 0))
> return fail_fn(count);
> + smp_mb();
> return 0;
> }
>
> @@ -58,6 +61,7 @@ __mutex_fastpath_lock_retval(atomic_t *count, int (*fail_fn)(atomic_t *))
> static inline void
> __mutex_fastpath_unlock(atomic_t *count, void (*fail_fn)(atomic_t *))
> {
> + smp_mb();
> if (unlikely(atomic_inc_return(count) <= 0))
> fail_fn(count);
> }
> @@ -82,8 +86,10 @@ __mutex_fastpath_unlock(atomic_t *count, void (*fail_fn)(atomic_t *))
> static inline int
> __mutex_fastpath_trylock(atomic_t *count, int (*fail_fn)(atomic_t *))
> {
> - if (likely(atomic_cmpxchg(count, 1, 0) == 1))
> + if (likely(atomic_cmpxchg(count, 1, 0) == 1)) {
> + smp_mb();
> return 1;
> + }
> return 0;
> }
This patch basically reverts Nick's a8ddac7e53e89cb ("mutex: speed up
generic mutex implementations"). What's up with that?
I could try to review this patch but I'm pathetic with barriers. Help.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [uclinux-dist-devel] [PATCH/RFC] asm-generic/mutex-dec.h: add SMP support
2011-06-06 21:23 ` Andrew Morton
@ 2011-06-06 21:31 ` Mike Frysinger
2011-06-06 22:33 ` Mike Frysinger
2011-06-06 21:32 ` Eric Dumazet
1 sibling, 1 reply; 6+ messages in thread
From: Mike Frysinger @ 2011-06-06 21:31 UTC (permalink / raw)
To: Andrew Morton
Cc: linux-arch, Piggin, Arnd Bergmann, linux-kernel, Nick,
uclinux-dist-devel
On Mon, Jun 6, 2011 at 17:23, Andrew Morton wrote:
> On Sun, 29 May 2011 23:19:28 -0400 Mike Frysinger wrote:
>> To make these guys work on SMP systems, we just need to sprinkle a few
>> barriers around.
>>
>> diff --git a/include/asm-generic/mutex-dec.h b/include/asm-generic/mutex-dec.h
>> index f104af7..e746c3c 100644
>> --- a/include/asm-generic/mutex-dec.h
>> +++ b/include/asm-generic/mutex-dec.h
>> @@ -22,6 +22,8 @@ __mutex_fastpath_lock(atomic_t *count, void (*fail_fn)(atomic_t *))
>> {
>> if (unlikely(atomic_dec_return(count) < 0))
>> fail_fn(count);
>> + else
>> + smp_mb();
>> }
>>
>> /**
>> @@ -39,6 +41,7 @@ __mutex_fastpath_lock_retval(atomic_t *count, int (*fail_fn)(atomic_t *))
>> {
>> if (unlikely(atomic_dec_return(count) < 0))
>> return fail_fn(count);
>> + smp_mb();
>> return 0;
>> }
>>
>> @@ -58,6 +61,7 @@ __mutex_fastpath_lock_retval(atomic_t *count, int (*fail_fn)(atomic_t *))
>> static inline void
>> __mutex_fastpath_unlock(atomic_t *count, void (*fail_fn)(atomic_t *))
>> {
>> + smp_mb();
>> if (unlikely(atomic_inc_return(count) <= 0))
>> fail_fn(count);
>> }
>> @@ -82,8 +86,10 @@ __mutex_fastpath_unlock(atomic_t *count, void (*fail_fn)(atomic_t *))
>> static inline int
>> __mutex_fastpath_trylock(atomic_t *count, int (*fail_fn)(atomic_t *))
>> {
>> - if (likely(atomic_cmpxchg(count, 1, 0) == 1))
>> + if (likely(atomic_cmpxchg(count, 1, 0) == 1)) {
>> + smp_mb();
>> return 1;
>> + }
>> return 0;
>> }
>
> This patch basically reverts Nick's a8ddac7e53e89cb ("mutex: speed up
> generic mutex implementations"). What's up with that?
>
> I could try to review this patch but I'm pathetic with barriers. Help.
thanks for that tip. i think we can chalk this patch up to the
origins of the Blackfin SMP port ... it branched this code before
Nick's patch, and never incorporated common changes back. so i'll
just drop it once i boot up a system to double check and convert the
Blackfin code over to the asm-generic version completely to avoid
future issues.
-mike
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [uclinux-dist-devel] [PATCH/RFC] asm-generic/mutex-dec.h: add SMP support
2011-06-06 21:31 ` [uclinux-dist-devel] " Mike Frysinger
@ 2011-06-06 22:33 ` Mike Frysinger
0 siblings, 0 replies; 6+ messages in thread
From: Mike Frysinger @ 2011-06-06 22:33 UTC (permalink / raw)
To: Andrew Morton
Cc: linux-arch, Piggin, Arnd Bergmann, linux-kernel, Nick,
uclinux-dist-devel
On Mon, Jun 6, 2011 at 17:31, Mike Frysinger wrote:
> On Mon, Jun 6, 2011 at 17:23, Andrew Morton wrote:
>> On Sun, 29 May 2011 23:19:28 -0400 Mike Frysinger wrote:
>>> To make these guys work on SMP systems, we just need to sprinkle a few
>>> barriers around.
>>>
>>> diff --git a/include/asm-generic/mutex-dec.h b/include/asm-generic/mutex-dec.h
>>> index f104af7..e746c3c 100644
>>> --- a/include/asm-generic/mutex-dec.h
>>> +++ b/include/asm-generic/mutex-dec.h
>>> @@ -22,6 +22,8 @@ __mutex_fastpath_lock(atomic_t *count, void (*fail_fn)(atomic_t *))
>>> {
>>> if (unlikely(atomic_dec_return(count) < 0))
>>> fail_fn(count);
>>> + else
>>> + smp_mb();
>>> }
>>>
>>> /**
>>> @@ -39,6 +41,7 @@ __mutex_fastpath_lock_retval(atomic_t *count, int (*fail_fn)(atomic_t *))
>>> {
>>> if (unlikely(atomic_dec_return(count) < 0))
>>> return fail_fn(count);
>>> + smp_mb();
>>> return 0;
>>> }
>>>
>>> @@ -58,6 +61,7 @@ __mutex_fastpath_lock_retval(atomic_t *count, int (*fail_fn)(atomic_t *))
>>> static inline void
>>> __mutex_fastpath_unlock(atomic_t *count, void (*fail_fn)(atomic_t *))
>>> {
>>> + smp_mb();
>>> if (unlikely(atomic_inc_return(count) <= 0))
>>> fail_fn(count);
>>> }
>>> @@ -82,8 +86,10 @@ __mutex_fastpath_unlock(atomic_t *count, void (*fail_fn)(atomic_t *))
>>> static inline int
>>> __mutex_fastpath_trylock(atomic_t *count, int (*fail_fn)(atomic_t *))
>>> {
>>> - if (likely(atomic_cmpxchg(count, 1, 0) == 1))
>>> + if (likely(atomic_cmpxchg(count, 1, 0) == 1)) {
>>> + smp_mb();
>>> return 1;
>>> + }
>>> return 0;
>>> }
>>
>> This patch basically reverts Nick's a8ddac7e53e89cb ("mutex: speed up
>> generic mutex implementations"). What's up with that?
>>
>> I could try to review this patch but I'm pathetic with barriers. Help.
>
> thanks for that tip. i think we can chalk this patch up to the
> origins of the Blackfin SMP port ... it branched this code before
> Nick's patch, and never incorporated common changes back. so i'll
> just drop it once i boot up a system to double check and convert the
> Blackfin code over to the asm-generic version completely to avoid
> future issues.
seems to be ok (and our core atomic's do include barriers themselves),
so let's just drop this patch on the floor and forget about it.
thanks all!
-mike
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH/RFC] asm-generic/mutex-dec.h: add SMP support
2011-06-06 21:23 ` Andrew Morton
2011-06-06 21:31 ` [uclinux-dist-devel] " Mike Frysinger
@ 2011-06-06 21:32 ` Eric Dumazet
1 sibling, 0 replies; 6+ messages in thread
From: Eric Dumazet @ 2011-06-06 21:32 UTC (permalink / raw)
To: Andrew Morton
Cc: Mike Frysinger, Arnd Bergmann, linux-kernel, linux-arch,
uclinux-dist-devel, Nick Piggin
Le lundi 06 juin 2011 à 14:23 -0700, Andrew Morton a écrit :
> On Sun, 29 May 2011 23:19:28 -0400
> Mike Frysinger <vapier@gentoo.org> wrote:
>
> > To make these guys work on SMP systems, we just need to sprinkle a few
> > barriers around.
> >
> > Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> > ---
> > note: this is what the Blackfin SMP port is using, but it doesn't seem
> > like other SMP ports are ... so I wonder if we're just trying too hard
> > and these barriers aren't actually necessary ?
> >
> > include/asm-generic/mutex-dec.h | 8 +++++++-
> > 1 files changed, 7 insertions(+), 1 deletions(-)
> >
> > diff --git a/include/asm-generic/mutex-dec.h b/include/asm-generic/mutex-dec.h
> > index f104af7..e746c3c 100644
> > --- a/include/asm-generic/mutex-dec.h
> > +++ b/include/asm-generic/mutex-dec.h
> > @@ -22,6 +22,8 @@ __mutex_fastpath_lock(atomic_t *count, void (*fail_fn)(atomic_t *))
> > {
> > if (unlikely(atomic_dec_return(count) < 0))
> > fail_fn(count);
Check Documentation/memory-barriers.txt, around line 1688
atomic_dec_return() implies a full memory barrier on each side of the
operation.
smp_mb() is therefore not needed here
> > + else
> > + smp_mb();
> > }
> >
> > /**
> > @@ -39,6 +41,7 @@ __mutex_fastpath_lock_retval(atomic_t *count, int (*fail_fn)(atomic_t *))
> > {
> > if (unlikely(atomic_dec_return(count) < 0))
> > return fail_fn(count);
> > + smp_mb();
atomic_dec_return() implies a full memory barrier.
smp_mb() is therefore not needed here
> > return 0;
> > }
> >
> > @@ -58,6 +61,7 @@ __mutex_fastpath_lock_retval(atomic_t *count, int (*fail_fn)(atomic_t *))
> > static inline void
> > __mutex_fastpath_unlock(atomic_t *count, void (*fail_fn)(atomic_t *))
> > {
> > + smp_mb();
atomic_inc_return() implies a full memory barrier.
smp_mb() is therefore not needed here
> > if (unlikely(atomic_inc_return(count) <= 0))
> > fail_fn(count);
> > }
> > @@ -82,8 +86,10 @@ __mutex_fastpath_unlock(atomic_t *count, void (*fail_fn)(atomic_t *))
> > static inline int
> > __mutex_fastpath_trylock(atomic_t *count, int (*fail_fn)(atomic_t *))
> > {
> > - if (likely(atomic_cmpxchg(count, 1, 0) == 1))
> > + if (likely(atomic_cmpxchg(count, 1, 0) == 1)) {
> > + smp_mb();
atomic_cmpxchg() implies a full memory barrier.
smp_mb() is therefore not needed here
> > return 1;
> > + }
> > return 0;
> > }
>
> This patch basically reverts Nick's a8ddac7e53e89cb ("mutex: speed up
> generic mutex implementations"). What's up with that?
>
> I could try to review this patch but I'm pathetic with barriers. Help.
Well, I really dont understand this patch, it makes no sense.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-06-06 22:34 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-05-30 3:19 [PATCH/RFC] asm-generic/mutex-dec.h: add SMP support Mike Frysinger
2011-05-30 7:00 ` Arnd Bergmann
2011-06-06 21:23 ` Andrew Morton
2011-06-06 21:31 ` [uclinux-dist-devel] " Mike Frysinger
2011-06-06 22:33 ` Mike Frysinger
2011-06-06 21:32 ` Eric Dumazet
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.