* ARM: why smp_mb() is not needed in the "__mutex_fastpath_lock" and "__mutex_fastpath_unlock" functions
@ 2012-07-12 2:14 shan kang
2012-07-13 7:02 ` Li Haifeng
0 siblings, 1 reply; 5+ messages in thread
From: shan kang @ 2012-07-12 2:14 UTC (permalink / raw)
To: linux-arm-kernel
Hello,
I wonder why smp_mb() is not needed in the "__mutex_fastpath_lock"
and "__mutex_fastpath_unlock" functions which are located in the
"arch/arm/include/asm/mutex.h"?
I think "dmb" instruction is necessary there.
^ permalink raw reply [flat|nested] 5+ messages in thread
* ARM: why smp_mb() is not needed in the "__mutex_fastpath_lock" and "__mutex_fastpath_unlock" functions
2012-07-12 2:14 ARM: why smp_mb() is not needed in the "__mutex_fastpath_lock" and "__mutex_fastpath_unlock" functions shan kang
@ 2012-07-13 7:02 ` Li Haifeng
2012-07-13 9:10 ` shan kang
0 siblings, 1 reply; 5+ messages in thread
From: Li Haifeng @ 2012-07-13 7:02 UTC (permalink / raw)
To: linux-arm-kernel
Hi Shan,
2012/7/12 shan kang <kangshan0910@gmail.com>:
> Hello,
> I wonder why smp_mb() is not needed in the "__mutex_fastpath_lock"
> and "__mutex_fastpath_unlock" functions which are located in the
> "arch/arm/include/asm/mutex.h"?
> I think "dmb" instruction is necessary there.
Why necessary? Could you explain more detailed?
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 5+ messages in thread
* ARM: why smp_mb() is not needed in the "__mutex_fastpath_lock" and "__mutex_fastpath_unlock" functions
2012-07-13 7:02 ` Li Haifeng
@ 2012-07-13 9:10 ` shan kang
2012-07-13 9:42 ` Will Deacon
0 siblings, 1 reply; 5+ messages in thread
From: shan kang @ 2012-07-13 9:10 UTC (permalink / raw)
To: linux-arm-kernel
For example, in the following scenario, Process2 may get the wrong value;
Process1:
mutex_lock(&lock);
write data; (store operation)
mutex_unlock(&lock);
Process2:
mutex_lock(&lock);
read data; (load operation)
mutex_unlock(&lock);
Suppose Process1 gets the lock first, write data and unlock. If the
store operation completes very slowly, the load operation of the
Process2 will fail to get the new value.
Since there are no dmb instructions in the mutex_lock and
mutex_unlock, which doesn't make sure that after Process2 gets the
lock, the result of the Process1's store operation will be seen by the
Process2.
2012/7/13 Li Haifeng <omycle@gmail.com>:
> Hi Shan,
>
> 2012/7/12 shan kang <kangshan0910@gmail.com>:
>> Hello,
>> I wonder why smp_mb() is not needed in the "__mutex_fastpath_lock"
>> and "__mutex_fastpath_unlock" functions which are located in the
>> "arch/arm/include/asm/mutex.h"?
>> I think "dmb" instruction is necessary there.
>
> Why necessary? Could you explain more detailed?
>
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel at lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 5+ messages in thread
* ARM: why smp_mb() is not needed in the "__mutex_fastpath_lock" and "__mutex_fastpath_unlock" functions
2012-07-13 9:10 ` shan kang
@ 2012-07-13 9:42 ` Will Deacon
2012-07-13 10:04 ` shan kang
0 siblings, 1 reply; 5+ messages in thread
From: Will Deacon @ 2012-07-13 9:42 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Jul 13, 2012 at 10:10:52AM +0100, shan kang wrote:
> For example, in the following scenario, Process2 may get the wrong value;
> Process1:
> mutex_lock(&lock);
> write data; (store operation)
> mutex_unlock(&lock);
>
> Process2:
> mutex_lock(&lock);
> read data; (load operation)
> mutex_unlock(&lock);
Yes, it looks like we can screw things up in the uncontended case (where
nobody blocks on the mutex). We could add an smp_mb after the lock operation
and another one before the unlock, but I'm tempted just to use
asm-generic/mutex-dec.h instead. The latter approach will subtly change the
current behaviour, so I'll post a patch when I'm happy with it.
Curious: did you find this by inspection or did you observe it going wrong?
Cheers,
Will
^ permalink raw reply [flat|nested] 5+ messages in thread
* ARM: why smp_mb() is not needed in the "__mutex_fastpath_lock" and "__mutex_fastpath_unlock" functions
2012-07-13 9:42 ` Will Deacon
@ 2012-07-13 10:04 ` shan kang
0 siblings, 0 replies; 5+ messages in thread
From: shan kang @ 2012-07-13 10:04 UTC (permalink / raw)
To: linux-arm-kernel
Yes, a customer reported an issue to me.
After my investigation, I found the problem may be the lack of "dmb"
in mutex_lock and mutex_unlock functions.
Moreover, the issue could be resolved after the "dmb" was added.
2012/7/13 Will Deacon <will.deacon@arm.com>:
> On Fri, Jul 13, 2012 at 10:10:52AM +0100, shan kang wrote:
>> For example, in the following scenario, Process2 may get the wrong value;
>> Process1:
>> mutex_lock(&lock);
>> write data; (store operation)
>> mutex_unlock(&lock);
>>
>> Process2:
>> mutex_lock(&lock);
>> read data; (load operation)
>> mutex_unlock(&lock);
>
> Yes, it looks like we can screw things up in the uncontended case (where
> nobody blocks on the mutex). We could add an smp_mb after the lock operation
> and another one before the unlock, but I'm tempted just to use
> asm-generic/mutex-dec.h instead. The latter approach will subtly change the
> current behaviour, so I'll post a patch when I'm happy with it.
>
> Curious: did you find this by inspection or did you observe it going wrong?
>
> Cheers,
>
> Will
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2012-07-13 10:04 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-07-12 2:14 ARM: why smp_mb() is not needed in the "__mutex_fastpath_lock" and "__mutex_fastpath_unlock" functions shan kang
2012-07-13 7:02 ` Li Haifeng
2012-07-13 9:10 ` shan kang
2012-07-13 9:42 ` Will Deacon
2012-07-13 10:04 ` shan kang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox