Re: [stffrdhrn:peterz-mbsync 1/2] arch/parisc/include/asm/spinlock.h:23:2: error: implicit declaration of function 'mb'; did you mean 'rmb'?

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



[added the parisc mailing list]

On Mon, 2018-04-09 at 10:05 +0200, Peter Zijlstra wrote:
> On Sat, Apr 07, 2018 at 06:22:37AM +0800, kbuild test robot wrote:
> > tree:   git://github.com/stffrdhrn/linux peterz-mbsync
> > head:   eb6885a5bcb07f0155f6ce6991894e7209cb2916
> > commit: 62b5969720267aa9e21cc4ea5cd22a2f4e8fd075 [1/2] asm-generic: 
> > Disallow no-op mb() for SMP systems
> > config: parisc-allmodconfig (attached as .config)
> > compiler: hppa-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
> > reproduce:
> >         wget https://raw.githubusercontent.com/intel/lkp-tests/mast
> > er/sbin/make.cross -O ~/bin/make.cross
> >         chmod +x ~/bin/make.cross
> >         git checkout 62b5969720267aa9e21cc4ea5cd22a2f4e8fd075
> >         # save the attached .config to linux build tree
> >         make.cross ARCH=parisc 
> > 
> > All errors (new ones prefixed by >>):
> > 
> >    In file included from arch/parisc/include/asm/atomic.h:22:0,
> >                     from include/linux/atomic.h:5,
> >                     from arch/parisc/include/asm/bitops.h:13,
> >                     from include/linux/bitops.h:38,
> >                     from include/linux/kernel.h:11,
> >                     from arch/parisc/include/asm/bug.h:5,
> >                     from include/linux/bug.h:5,
> >                     from include/linux/page-flags.h:10,
> >                     from kernel/bounds.c:10:
> >    arch/parisc/include/asm/spinlock.h: In function
> > 'arch_spin_lock_flags':
> > > > arch/parisc/include/asm/spinlock.h:23:2: error: implicit
> > > > declaration of function 'mb'; did you mean 'rmb'? [-
> > > > Werror=implicit-function-declaration]
> > 
> >      mb();
> >      ^~
> >      rmb
> 
> 
> Argh,.. so I did that in commit:
> 
>   93ea02bb8435 ("arch: Clean up asm/barrier.h implementations using
> asm-generic/barrier.h")
> 
> and failed to notice the whole SMP without mb thing.
> 
> Which would suggest we do something like the below;

patch below restoring mb() as a compiler barrier looks fine.

>  although then I read the docs here:
> 
>   https://parisc.wiki.kernel.org/images-parisc/7/73/Parisc2.0.pdf
> 
> I don't see the U-bit mentioned in the below blurb, and also, I see
> no mention on why SYNCDMA is not needed.

Um, well U is uncached and is used mostly for memory based IO regions. 
We also have the odd system that doesn't have the U bit, which is why
there's a network and a scsi driver with dma_cache_sync() in them.

The point here is that U being caching is nothing to do with ordering.

> A paranoid me would do mb() as SYNCDMA and __smp_mb() as SYNC, given
> that the manual also states it NO-OPs these instructions when not
> needed.

I think you misunderstand how ordering and caching work.  Ordering is
the province of barriers and we insert the ordering into the code
assuming we're in the CPU domain (usually everything as seen by the
cpus is fully cache coherent on parisc).  Caching is the province of
the coherence domains (usually CPU to bus) and we handle that in the
dma_ IO operations.

So we can quibble about whether we need a syncdma in our dma_
operations, we'd never need one in the execution barriers.

James

> James, Helge ?
> 
> 
> --- /dev/null
> +++ b/arch/parisc/include/asm/barrier.h
> @@ -0,0 +1,24 @@
> +#ifndef __PARISC_BARRIER_H
> +#define __PARISC_BARRIER_H
> +
> +/*
> + * PA-RISC architecture allows for weakly ordered memory accesses
> although
> + * none of the processors use it. There is a strong ordered bit that
> is
> + * set in the O-bit of the page directory entry. Operating systems
> that
> + * can not tolerate out of order accesses should set this bit when
> mapping
> + * pages. The O-bit of the PSW should also be set to 1 (I don't
> believe any
> + * of the processor implemented the PSW O-bit). The PCX-W ERS states
> that
> + * the TLB O-bit is not implemented so the page directory does not
> need to
> + * have the O-bit set when mapping pages (section 3.1). This section
> also
> + * states that the PSW Y, Z, G, and O bits are not implemented.
> + * So it looks like nothing needs to be done for parisc-linux (yet).
> + * (thanks to chada for the above comment -ggg)
> + *
> + * So PARISC really is Sequentially Consistent for SMP.
> + */
> +#define mb()		__asm__ __volatile__("":::"memory")	
> /* barrier() */
> +
> +#include <asm-generic/barrier.h>
> +
> +#endif /* __PARISC_BARRIER_H */
> +
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-parisc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux SoC]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux