* [PATCH Resend 1/2] MIPS: io: Add barrier after register read in inX()
@ 2018-06-12 9:54 Huacai Chen
2018-06-12 9:54 ` [PATCH Resend 2/2] MIPS: mcs lock: implement arch_mcs_spin_lock_contended() for Loongson-3 Huacai Chen
0 siblings, 1 reply; 6+ messages in thread
From: Huacai Chen @ 2018-06-12 9:54 UTC (permalink / raw)
To: Ralf Baechle
Cc: Paul Burton, James Hogan, linux-mips, Fuxin Zhang, Zhangjin Wu,
Huacai Chen, Huacai Chen, stable
While a barrier is present in the outX() functions before the register
write, a similar barrier is missing in the inX() functions after the
register read. This could allow memory accesses following inX() to
observe stale data.
This patch is very similar to commit a1cc7034e33d12dc1 ("MIPS: io: Add
barrier after register read in readX()"). Because war_io_reorder_wmb()
is both used by writeX() and outX(), if readX() need a barrier then so
does inX().
Cc: stable@vger.kernel.org
Signed-off-by: Huacai Chen <chenhc@lemote.com>
---
arch/mips/include/asm/io.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/mips/include/asm/io.h b/arch/mips/include/asm/io.h
index a7d0b83..cea8ad8 100644
--- a/arch/mips/include/asm/io.h
+++ b/arch/mips/include/asm/io.h
@@ -414,6 +414,8 @@ static inline type pfx##in##bwlq##p(unsigned long port) \
__val = *__addr; \
slow; \
\
+ /* prevent prefetching of coherent DMA data prematurely */ \
+ rmb(); \
return pfx##ioswab##bwlq(__addr, __val); \
}
--
2.7.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH Resend 2/2] MIPS: mcs lock: implement arch_mcs_spin_lock_contended() for Loongson-3
2018-06-12 9:54 [PATCH Resend 1/2] MIPS: io: Add barrier after register read in inX() Huacai Chen
@ 2018-06-12 9:54 ` Huacai Chen
2018-06-12 18:40 ` Paul Burton
0 siblings, 1 reply; 6+ messages in thread
From: Huacai Chen @ 2018-06-12 9:54 UTC (permalink / raw)
To: Ralf Baechle
Cc: Paul Burton, James Hogan, linux-mips, Fuxin Zhang, Zhangjin Wu,
Huacai Chen, Huacai Chen
After commit 7f56b58a92aaf2c ("locking/mcs: Use smp_cond_load_acquire()
in MCS spin loop") Loongson-3 fails to boot. This is because Loongson-3
has SFB (Store Fill Buffer) and need a mb() after every READ_ONCE().
This patch introduce a MIPS-specific mcs_spinlock.h and revert to the
old implementation of arch_mcs_spin_lock_contended() for Loongson-3.
Signed-off-by: Huacai Chen <chenhc@lemote.com>
---
arch/mips/include/asm/Kbuild | 1 -
arch/mips/include/asm/mcs_spinlock.h | 14 ++++++++++++++
2 files changed, 14 insertions(+), 1 deletion(-)
create mode 100644 arch/mips/include/asm/mcs_spinlock.h
diff --git a/arch/mips/include/asm/Kbuild b/arch/mips/include/asm/Kbuild
index 45d541b..7076627 100644
--- a/arch/mips/include/asm/Kbuild
+++ b/arch/mips/include/asm/Kbuild
@@ -6,7 +6,6 @@ generic-y += emergency-restart.h
generic-y += export.h
generic-y += irq_work.h
generic-y += local64.h
-generic-y += mcs_spinlock.h
generic-y += mm-arch-hooks.h
generic-y += parport.h
generic-y += percpu.h
diff --git a/arch/mips/include/asm/mcs_spinlock.h b/arch/mips/include/asm/mcs_spinlock.h
new file mode 100644
index 0000000..063df4e
--- /dev/null
+++ b/arch/mips/include/asm/mcs_spinlock.h
@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __ASM_MCS_LOCK_H
+#define __ASM_MCS_LOCK_H
+
+/* Loongson-3 need a mb() after every READ_ONCE() */
+#if defined(CONFIG_CPU_LOONGSON3) && defined(CONFIG_SMP)
+#define arch_mcs_spin_lock_contended(l) \
+do { \
+ while (!(smp_load_acquire(l))) \
+ cpu_relax(); \
+} while (0)
+#endif /* CONFIG_CPU_LOONGSON3 && CONFIG_SMP */
+
+#endif /* __ASM_MCS_LOCK_H */
--
2.7.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH Resend 2/2] MIPS: mcs lock: implement arch_mcs_spin_lock_contended() for Loongson-3
@ 2018-06-12 18:40 ` Paul Burton
0 siblings, 0 replies; 6+ messages in thread
From: Paul Burton @ 2018-06-12 18:40 UTC (permalink / raw)
To: Huacai Chen
Cc: Ralf Baechle, James Hogan, linux-mips, Fuxin Zhang, Zhangjin Wu,
Huacai Chen
Hi Huacai,
On Tue, Jun 12, 2018 at 05:54:43PM +0800, Huacai Chen wrote:
> After commit 7f56b58a92aaf2c ("locking/mcs: Use smp_cond_load_acquire()
> in MCS spin loop") Loongson-3 fails to boot. This is because Loongson-3
> has SFB (Store Fill Buffer) and need a mb() after every READ_ONCE().
Why do you need that mb()?
Could you describe what is actually happening with the current code, in
order to explain what requires the mb() you mention here? What's being
held in the SFB & why is that problematic?
Most importantly if smp_cond_load_acquire() isn't working as expected on
Loongson CPUs then fixing that would be a much better path forwards than
trying to avoid using it.
Could it be that Loongson requires an implementation of
(smp_)read_barrier_depends()?
Additionally you have "Resend" in the subject of this email, but I don't
see any previous submissions of this patch - given that commit
7f56b58a92aaf2c ("locking/mcs: Use smp_cond_load_acquire() in MCS spin
loop") is very recent I doubt there were any. Please try to be factual,
and if you have 2 patches that are unrelated please send them separately
rather than as a series.
Thanks,
Paul
> This patch introduce a MIPS-specific mcs_spinlock.h and revert to the
> old implementation of arch_mcs_spin_lock_contended() for Loongson-3.
>
> Signed-off-by: Huacai Chen <chenhc@lemote.com>
> ---
> arch/mips/include/asm/Kbuild | 1 -
> arch/mips/include/asm/mcs_spinlock.h | 14 ++++++++++++++
> 2 files changed, 14 insertions(+), 1 deletion(-)
> create mode 100644 arch/mips/include/asm/mcs_spinlock.h
>
> diff --git a/arch/mips/include/asm/Kbuild b/arch/mips/include/asm/Kbuild
> index 45d541b..7076627 100644
> --- a/arch/mips/include/asm/Kbuild
> +++ b/arch/mips/include/asm/Kbuild
> @@ -6,7 +6,6 @@ generic-y += emergency-restart.h
> generic-y += export.h
> generic-y += irq_work.h
> generic-y += local64.h
> -generic-y += mcs_spinlock.h
> generic-y += mm-arch-hooks.h
> generic-y += parport.h
> generic-y += percpu.h
> diff --git a/arch/mips/include/asm/mcs_spinlock.h b/arch/mips/include/asm/mcs_spinlock.h
> new file mode 100644
> index 0000000..063df4e
> --- /dev/null
> +++ b/arch/mips/include/asm/mcs_spinlock.h
> @@ -0,0 +1,14 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef __ASM_MCS_LOCK_H
> +#define __ASM_MCS_LOCK_H
> +
> +/* Loongson-3 need a mb() after every READ_ONCE() */
> +#if defined(CONFIG_CPU_LOONGSON3) && defined(CONFIG_SMP)
> +#define arch_mcs_spin_lock_contended(l) \
> +do { \
> + while (!(smp_load_acquire(l))) \
> + cpu_relax(); \
> +} while (0)
> +#endif /* CONFIG_CPU_LOONGSON3 && CONFIG_SMP */
> +
> +#endif /* __ASM_MCS_LOCK_H */
> --
> 2.7.0
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH Resend 2/2] MIPS: mcs lock: implement arch_mcs_spin_lock_contended() for Loongson-3
@ 2018-06-12 18:40 ` Paul Burton
0 siblings, 0 replies; 6+ messages in thread
From: Paul Burton @ 2018-06-12 18:40 UTC (permalink / raw)
To: Huacai Chen
Cc: Ralf Baechle, James Hogan, linux-mips, Fuxin Zhang, Zhangjin Wu,
Huacai Chen
Hi Huacai,
On Tue, Jun 12, 2018 at 05:54:43PM +0800, Huacai Chen wrote:
> After commit 7f56b58a92aaf2c ("locking/mcs: Use smp_cond_load_acquire()
> in MCS spin loop") Loongson-3 fails to boot. This is because Loongson-3
> has SFB (Store Fill Buffer) and need a mb() after every READ_ONCE().
Why do you need that mb()?
Could you describe what is actually happening with the current code, in
order to explain what requires the mb() you mention here? What's being
held in the SFB & why is that problematic?
Most importantly if smp_cond_load_acquire() isn't working as expected on
Loongson CPUs then fixing that would be a much better path forwards than
trying to avoid using it.
Could it be that Loongson requires an implementation of
(smp_)read_barrier_depends()?
Additionally you have "Resend" in the subject of this email, but I don't
see any previous submissions of this patch - given that commit
7f56b58a92aaf2c ("locking/mcs: Use smp_cond_load_acquire() in MCS spin
loop") is very recent I doubt there were any. Please try to be factual,
and if you have 2 patches that are unrelated please send them separately
rather than as a series.
Thanks,
Paul
> This patch introduce a MIPS-specific mcs_spinlock.h and revert to the
> old implementation of arch_mcs_spin_lock_contended() for Loongson-3.
>
> Signed-off-by: Huacai Chen <chenhc@lemote.com>
> ---
> arch/mips/include/asm/Kbuild | 1 -
> arch/mips/include/asm/mcs_spinlock.h | 14 ++++++++++++++
> 2 files changed, 14 insertions(+), 1 deletion(-)
> create mode 100644 arch/mips/include/asm/mcs_spinlock.h
>
> diff --git a/arch/mips/include/asm/Kbuild b/arch/mips/include/asm/Kbuild
> index 45d541b..7076627 100644
> --- a/arch/mips/include/asm/Kbuild
> +++ b/arch/mips/include/asm/Kbuild
> @@ -6,7 +6,6 @@ generic-y += emergency-restart.h
> generic-y += export.h
> generic-y += irq_work.h
> generic-y += local64.h
> -generic-y += mcs_spinlock.h
> generic-y += mm-arch-hooks.h
> generic-y += parport.h
> generic-y += percpu.h
> diff --git a/arch/mips/include/asm/mcs_spinlock.h b/arch/mips/include/asm/mcs_spinlock.h
> new file mode 100644
> index 0000000..063df4e
> --- /dev/null
> +++ b/arch/mips/include/asm/mcs_spinlock.h
> @@ -0,0 +1,14 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef __ASM_MCS_LOCK_H
> +#define __ASM_MCS_LOCK_H
> +
> +/* Loongson-3 need a mb() after every READ_ONCE() */
> +#if defined(CONFIG_CPU_LOONGSON3) && defined(CONFIG_SMP)
> +#define arch_mcs_spin_lock_contended(l) \
> +do { \
> + while (!(smp_load_acquire(l))) \
> + cpu_relax(); \
> +} while (0)
> +#endif /* CONFIG_CPU_LOONGSON3 && CONFIG_SMP */
> +
> +#endif /* __ASM_MCS_LOCK_H */
> --
> 2.7.0
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH Resend 2/2] MIPS: mcs lock: implement arch_mcs_spin_lock_contended() for Loongson-3
2018-06-12 18:40 ` Paul Burton
(?)
@ 2018-06-13 5:55 ` Huacai Chen
2018-06-13 16:55 ` Paul Burton
-1 siblings, 1 reply; 6+ messages in thread
From: Huacai Chen @ 2018-06-13 5:55 UTC (permalink / raw)
To: Paul Burton
Cc: Ralf Baechle, James Hogan, Linux MIPS Mailing List, Fuxin Zhang,
Zhangjin Wu
Hi, Paul,
On Wed, Jun 13, 2018 at 2:40 AM, Paul Burton <paul.burton@mips.com> wrote:
> Hi Huacai,
>
> On Tue, Jun 12, 2018 at 05:54:43PM +0800, Huacai Chen wrote:
>> After commit 7f56b58a92aaf2c ("locking/mcs: Use smp_cond_load_acquire()
>> in MCS spin loop") Loongson-3 fails to boot. This is because Loongson-3
>> has SFB (Store Fill Buffer) and need a mb() after every READ_ONCE().
>
> Why do you need that mb()?
>
> Could you describe what is actually happening with the current code, in
> order to explain what requires the mb() you mention here? What's being
> held in the SFB & why is that problematic?
>
> Most importantly if smp_cond_load_acquire() isn't working as expected on
> Loongson CPUs then fixing that would be a much better path forwards than
> trying to avoid using it.
>
> Could it be that Loongson requires an implementation of
> (smp_)read_barrier_depends()?
Yes, we should fix smp_cond_load_acquire(), not
arch_mcs_spin_lock_contended(). Thank you very much.
>
> Additionally you have "Resend" in the subject of this email, but I don't
> see any previous submissions of this patch - given that commit
> 7f56b58a92aaf2c ("locking/mcs: Use smp_cond_load_acquire() in MCS spin
> loop") is very recent I doubt there were any. Please try to be factual,
> and if you have 2 patches that are unrelated please send them separately
> rather than as a series.
I send another patch in this series because it is so simple and should
be merged in the previous release, but it is ignored again and again.
In practise, my
single patch in linux-mips usually be ignored (even they are very
simple and well described)....
For example:
https://patchwork.linux-mips.org/patch/17723/
https://patchwork.linux-mips.org/patch/18587/
https://patchwork.linux-mips.org/patch/19184/
Some of them are promised by James to review, but still ignored until now.
Huacai
>
> Thanks,
> Paul
>
>> This patch introduce a MIPS-specific mcs_spinlock.h and revert to the
>> old implementation of arch_mcs_spin_lock_contended() for Loongson-3.
>>
>> Signed-off-by: Huacai Chen <chenhc@lemote.com>
>> ---
>> arch/mips/include/asm/Kbuild | 1 -
>> arch/mips/include/asm/mcs_spinlock.h | 14 ++++++++++++++
>> 2 files changed, 14 insertions(+), 1 deletion(-)
>> create mode 100644 arch/mips/include/asm/mcs_spinlock.h
>>
>> diff --git a/arch/mips/include/asm/Kbuild b/arch/mips/include/asm/Kbuild
>> index 45d541b..7076627 100644
>> --- a/arch/mips/include/asm/Kbuild
>> +++ b/arch/mips/include/asm/Kbuild
>> @@ -6,7 +6,6 @@ generic-y += emergency-restart.h
>> generic-y += export.h
>> generic-y += irq_work.h
>> generic-y += local64.h
>> -generic-y += mcs_spinlock.h
>> generic-y += mm-arch-hooks.h
>> generic-y += parport.h
>> generic-y += percpu.h
>> diff --git a/arch/mips/include/asm/mcs_spinlock.h b/arch/mips/include/asm/mcs_spinlock.h
>> new file mode 100644
>> index 0000000..063df4e
>> --- /dev/null
>> +++ b/arch/mips/include/asm/mcs_spinlock.h
>> @@ -0,0 +1,14 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +#ifndef __ASM_MCS_LOCK_H
>> +#define __ASM_MCS_LOCK_H
>> +
>> +/* Loongson-3 need a mb() after every READ_ONCE() */
>> +#if defined(CONFIG_CPU_LOONGSON3) && defined(CONFIG_SMP)
>> +#define arch_mcs_spin_lock_contended(l) \
>> +do { \
>> + while (!(smp_load_acquire(l))) \
>> + cpu_relax(); \
>> +} while (0)
>> +#endif /* CONFIG_CPU_LOONGSON3 && CONFIG_SMP */
>> +
>> +#endif /* __ASM_MCS_LOCK_H */
>> --
>> 2.7.0
>>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH Resend 2/2] MIPS: mcs lock: implement arch_mcs_spin_lock_contended() for Loongson-3
2018-06-13 5:55 ` Huacai Chen
@ 2018-06-13 16:55 ` Paul Burton
0 siblings, 0 replies; 6+ messages in thread
From: Paul Burton @ 2018-06-13 16:55 UTC (permalink / raw)
To: Huacai Chen
Cc: Ralf Baechle, James Hogan, Linux MIPS Mailing List, Fuxin Zhang,
Zhangjin Wu
Hi Huacai,
On Wed, Jun 13, 2018 at 01:55:17PM +0800, Huacai Chen wrote:
> > Additionally you have "Resend" in the subject of this email, but I don't
> > see any previous submissions of this patch - given that commit
> > 7f56b58a92aaf2c ("locking/mcs: Use smp_cond_load_acquire() in MCS spin
> > loop") is very recent I doubt there were any. Please try to be factual,
> > and if you have 2 patches that are unrelated please send them separately
> > rather than as a series.
> I send another patch in this series because it is so simple and should
> be merged in the previous release, but it is ignored again and again.
> In practise, my
> single patch in linux-mips usually be ignored (even they are very
> simple and well described)....
> For example:
> https://patchwork.linux-mips.org/patch/17723/
> https://patchwork.linux-mips.org/patch/18587/
> https://patchwork.linux-mips.org/patch/19184/
>
> Some of them are promised by James to review, but still ignored until now.
Both James & Ralf have been busy with things outside of kernel
development recently, so I'm sure it's just the case that they've not
had as much time to spend on maintenance as would have been ideal. I
know that has been frustrating, but the reasons for that haven't always
been controllable & I feel confident saying that it was nobody's intent
to ignore you or your contributions.
Going forwards I'm now listed in MAINTAINERS & after v4.18-rc1 is
released I intend to start merging patches ready for v4.19. I've started
looking though pending patches but haven't gotten as far as the ones you
linked yet.
A couple of those look like they could be candidates for a fixes pull in
the 4.18 cycle so I'll try to take a closer look at those sooner.
I'm still getting up to speed & there is a backlog of patches to review,
but please be assured that if your patch is still listed as "New" or
"Under Review" in patchwork then I intend to look at it, and there's no
need to resend it.
Thanks,
Paul
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-06-13 16:56 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-12 9:54 [PATCH Resend 1/2] MIPS: io: Add barrier after register read in inX() Huacai Chen
2018-06-12 9:54 ` [PATCH Resend 2/2] MIPS: mcs lock: implement arch_mcs_spin_lock_contended() for Loongson-3 Huacai Chen
2018-06-12 18:40 ` Paul Burton
2018-06-12 18:40 ` Paul Burton
2018-06-13 5:55 ` Huacai Chen
2018-06-13 16:55 ` Paul Burton
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.