On 01/10/2016 06:18 AM, Michael S. Tsirkin wrote:> On mips dma_rmb, dma_wmb, smp_store_mb, read_barrier_depends, > smp_read_barrier_depends, smp_store_release and smp_load_acquire match > the asm-generic variants exactly. Drop the local definitions and pull in > asm-generic/barrier.h instead. >This statement doesn't fit MIPS barriers variations. Moreover, there is a reason to extend that even more specific, at least for smp_store_release and smp_load_acquire, look into http://patchwork.linux-mips.org/patch/10506/ - Leonid.
On Mon, Jan 11, 2016 at 05:14:14PM -0800, Leonid Yegoshin wrote:> On 01/10/2016 06:18 AM, Michael S. Tsirkin wrote: > >On mips dma_rmb, dma_wmb, smp_store_mb, read_barrier_depends, > >smp_read_barrier_depends, smp_store_release and smp_load_acquire match > >the asm-generic variants exactly. Drop the local definitions and pull in > >asm-generic/barrier.h instead. > > > This statement doesn't fit MIPS barriers variations. Moreover, there is a > reason to extend that even more specific, at least for smp_store_release and > smp_load_acquire, look into > > http://patchwork.linux-mips.org/patch/10506/ > > - Leonid.Fine, but it matches what current code is doing. Since that MIPS_LIGHTWEIGHT_SYNC patch didn't go into linux-next yet, do you see a problem reworking it on top of this patchset? -- MST
On Mon, Jan 11, 2016 at 05:14:14PM -0800, Leonid Yegoshin wrote:> This statement doesn't fit MIPS barriers variations. Moreover, there is a > reason to extend that even more specific, at least for smp_store_release and > smp_load_acquire, look into > > http://patchwork.linux-mips.org/patch/10506/Dude, that's one horrible patch. 1) you do not make such things selectable; either the hardware needs them or it doesn't. If it does you _must_ use them, however unlikely. 2) the changelog _completely_ fails to explain the sync 0x11 and sync 0x12 semantics nor does it provide a publicly accessible link to documentation that does. 3) it really should have explained what you did with smp_llsc_mb/smp_mb__before_llsc() in _detail_. And I agree that ideally it should be split into parts. Seriously, this is _NOT_ OK.
On Tue, Jan 12, 2016 at 10:43:36AM +0200, Michael S. Tsirkin wrote:> On Mon, Jan 11, 2016 at 05:14:14PM -0800, Leonid Yegoshin wrote: > > On 01/10/2016 06:18 AM, Michael S. Tsirkin wrote: > > >On mips dma_rmb, dma_wmb, smp_store_mb, read_barrier_depends, > > >smp_read_barrier_depends, smp_store_release and smp_load_acquire match > > >the asm-generic variants exactly. Drop the local definitions and pull in > > >asm-generic/barrier.h instead. > > > > > This statement doesn't fit MIPS barriers variations. Moreover, there is a > > reason to extend that even more specific, at least for smp_store_release and > > smp_load_acquire, look into > > > > http://patchwork.linux-mips.org/patch/10506/ > > > > - Leonid. > > Fine, but it matches what current code is doing. Since that > MIPS_LIGHTWEIGHT_SYNC patch didn't go into linux-next yet, do > you see a problem reworking it on top of this patchset?That patch is a complete doorstop atm. It needs a lot more work before it can go anywhere. Don't worry about it.
On Tue, Jan 12, 2016 at 10:27:11AM +0100, Peter Zijlstra wrote:> 2) the changelog _completely_ fails to explain the sync 0x11 and sync > 0x12 semantics nor does it provide a publicly accessible link to > documentation that does.Ralf pointed me at: https://imgtec.com/mips/architectures/mips64/> 3) it really should have explained what you did with > smp_llsc_mb/smp_mb__before_llsc() in _detail_.And reading the MIPS64 v6.04 instruction set manual, I think 0x11/0x12 are _NOT_ transitive and therefore cannot be used to implement the smp_mb__{before,after} stuff. That is, in MIPS speak, those SYNC types are Ordering Barriers, not Completion Barriers. They need not be globally performed.