Re: [PATCH RFC 3/4] barriers: convert a control to a data dependency

2019-01-07 Thread Paul E. McKenney 
On Mon, Jan 07, 2019 at 08:36:36AM -0500, Michael S. Tsirkin wrote:
> On Mon, Jan 07, 2019 at 10:46:10AM +0100, Peter Zijlstra wrote:
> > On Sun, Jan 06, 2019 at 11:23:07PM -0500, Michael S. Tsirkin wrote:
> > > On Mon, Jan 07, 2019 at 11:58:23AM +0800, Jason Wang wrote:
> > > > On 2019/1/3 上午4:57, Michael S. Tsirkin wrote:
> > 
> > > > > +#if defined(COMPILER_HAS_OPTIMIZER_HIDE_VAR) && \
> > > > > + !defined(ARCH_NEEDS_READ_BARRIER_DEPENDS)
> > > > > +
> > > > > +#define dependent_ptr_mb(ptr, val) ({
> > > > > \
> > > > > + long dependent_ptr_mb_val = (long)(val);
> > > > > \
> > > > > + long dependent_ptr_mb_ptr = (long)(ptr) - dependent_ptr_mb_val; 
> > > > > \
> > > > > + 
> > > > > \
> > > > > + BUILD_BUG_ON(sizeof(val) > sizeof(long));   
> > > > > \
> > > > > + OPTIMIZER_HIDE_VAR(dependent_ptr_mb_val);   
> > > > > \
> > > > > + (typeof(ptr))(dependent_ptr_mb_ptr + dependent_ptr_mb_val); 
> > > > > \
> > > > > +})
> > > > > +
> > > > > +#else
> > > > > +
> > > > > +#define dependent_ptr_mb(ptr, val) ({ mb(); (ptr); })
> > > > 
> > > > 
> > > > So for the example of patch 4, we'd better fall back to rmb() or need a
> > > > dependent_ptr_rmb()?
> > > > 
> > > > Thanks
> > > 
> > > You mean for strongly ordered architectures like Intel?
> > > Yes, maybe it makes sense to have dependent_ptr_smp_rmb,
> > > dependent_ptr_dma_rmb and dependent_ptr_virt_rmb.
> > > 
> > > mb variant is unused right now so I'll remove it.
> > 
> > How about naming the thing: dependent_ptr() ? That is without any (r)mb
> > implications at all. The address dependency is strictly weaker than an
> > rmb in that it will only order the two loads in qestion and not, like
> > rmb, any prior to any later load.
> 
> So I'm fine with this as it's enough for virtio, but I would like to point 
> out two things:
> 
> 1. E.g. on x86 both SMP and DMA variants can be NOPs but
> the madatory one can't, so assuming we do not want
> it to be stronger than rmp then either we want
> smp_dependent_ptr(), dma_dependent_ptr(), dependent_ptr()
> or we just will specify that dependent_ptr() works for
> both DMA and SMP.
> 
> 2. Down the road, someone might want to order a store after a load.
> Address dependency does that for us too. Assuming we make
> dependent_ptr a NOP on x86, we will want an mb variant
> which isn't a NOP on x86. Will we want to rename
> dependent_ptr to dependent_ptr_rmb at that point?

But x86 preserves store-after-load orderings anyway, and even Alpha
respects ordering from loads to dependent stores.  So what am I missing
here?

Thanx, Paul

Re: [PATCH 1/2] locking/xchg/alpha: Use smp_mb() in place of __ASM__MB

2018-02-22 Thread Paul E. McKenney 
On Thu, Feb 22, 2018 at 10:24:29AM +0100, Andrea Parri wrote:
> Replace each occurrence of __ASM__MB with a (trailing) smp_mb() in
> xchg(), cmpxchg(), and remove the now unused __ASM__MB definitions;
> this improves readability, with no additional synchronization cost.
> 
> Suggested-by: Will Deacon <will.dea...@arm.com>
> Signed-off-by: Andrea Parri <parri.and...@gmail.com>

I am a bit confused by the use of out-of-line branches to do a backwards
branch, but those were in place to start with.  Maybe the point is to
defeat backwards-branch prediction or some such.

Regardless...

Acked-by: Paul E. McKenney <paul...@linux.vnet.ibm.com>

> Cc: Peter Zijlstra <pet...@infradead.org>
> Cc: Paul E. McKenney <paul...@linux.vnet.ibm.com>
> Cc: Alan Stern <st...@rowland.harvard.edu>
> Cc: Ivan Kokshaysky <i...@jurassic.park.msu.ru>
> Cc: Matt Turner <matts...@gmail.com>
> Cc: Richard Henderson <r...@twiddle.net>
> Cc: linux-alpha@vger.kernel.org
> Cc: linux-ker...@vger.kernel.org
> ---
>  arch/alpha/include/asm/cmpxchg.h |  6 --
>  arch/alpha/include/asm/xchg.h| 16 
>  2 files changed, 8 insertions(+), 14 deletions(-)
> 
> diff --git a/arch/alpha/include/asm/cmpxchg.h 
> b/arch/alpha/include/asm/cmpxchg.h
> index 46ebf14aed4e5..8a2b331e43feb 100644
> --- a/arch/alpha/include/asm/cmpxchg.h
> +++ b/arch/alpha/include/asm/cmpxchg.h
> @@ -6,7 +6,6 @@
>   * Atomic exchange routines.
>   */
> 
> -#define __ASM__MB
>  #define xchg(type, args...)  __xchg ## type ## _local(args)
>  #define cmpxchg(type, args...)   __cmpxchg ## type ## _local(args)
>  #include 
> @@ -33,10 +32,6 @@
>   cmpxchg_local((ptr), (o), (n)); \
>  })
> 
> -#ifdef CONFIG_SMP
> -#undef __ASM__MB
> -#define __ASM__MB"\tmb\n"
> -#endif
>  #undef xchg
>  #undef cmpxchg
>  #define xchg(type, args...)  __xchg ##type(args)
> @@ -64,7 +59,6 @@
>   cmpxchg((ptr), (o), (n));   \
>  })
> 
> -#undef __ASM__MB
>  #undef cmpxchg
> 
>  #endif /* _ALPHA_CMPXCHG_H */
> diff --git a/arch/alpha/include/asm/xchg.h b/arch/alpha/include/asm/xchg.h
> index e2660866ce972..e1facf6fc2446 100644
> --- a/arch/alpha/include/asm/xchg.h
> +++ b/arch/alpha/include/asm/xchg.h
> @@ -28,12 +28,12 @@ xchg(_u8, volatile char *m, unsigned long val)
>   "   or  %1,%2,%2\n"
>   "   stq_c   %2,0(%3)\n"
>   "   beq %2,2f\n"
> - __ASM__MB
>   ".subsection 2\n"
>   "2: br  1b\n"
>   ".previous"
>   : "=" (ret), "=" (val), "=" (tmp), "=" (addr64)
>   : "r" ((long)m), "1" (val) : "memory");
> + smp_mb();
> 
>   return ret;
>  }
> @@ -52,12 +52,12 @@ xchg(_u16, volatile short *m, unsigned long val)
>   "   or  %1,%2,%2\n"
>   "   stq_c   %2,0(%3)\n"
>   "   beq %2,2f\n"
> - __ASM__MB
>   ".subsection 2\n"
>   "2: br  1b\n"
>   ".previous"
>   : "=" (ret), "=" (val), "=" (tmp), "=" (addr64)
>   : "r" ((long)m), "1" (val) : "memory");
> + smp_mb();
> 
>   return ret;
>  }
> @@ -72,12 +72,12 @@ xchg(_u32, volatile int *m, unsigned long val)
>   "   bis $31,%3,%1\n"
>   "   stl_c %1,%2\n"
>   "   beq %1,2f\n"
> - __ASM__MB
>   ".subsection 2\n"
>   "2: br 1b\n"
>   ".previous"
>   : "=" (val), "=" (dummy), "=m" (*m)
>   : "rI" (val), "m" (*m) : "memory");
> + smp_mb();
> 
>   return val;
>  }
> @@ -92,12 +92,12 @@ xchg(_u64, volatile long *m, unsigned long val)
>   "   bis $31,%3,%1\n"
>   "   stq_c %1,%2\n"
>   "   beq %1,2f\n"
> - __ASM__MB
>   ".subsection 2\n"
>   "2: br 1b\n"
>   ".previous"
>   : "=" (val), "=" (dummy), "=m" (*m)
>   : "rI" (val), "m" (*m) : "memory");
> + smp_mb();
> 
>   return val;
>  }
> @@ -150,12 +150,12 @@ cmpxchg(_u8, volatile char *m, unsigned char old, 
> unsigned char new)
>   "   stq_c

Re: [PATCH] xchg/alpha: Add unconditional memory barrier to cmpxchg

2018-02-20 Thread Paul E. McKenney 
On Tue, Feb 20, 2018 at 07:45:56PM +0100, Andrea Parri wrote:
> Continuing along with the fight against smp_read_barrier_depends() [1]
> (or rather, against its improper use), add an unconditional barrier to
> cmpxchg.  This guarantees that dependency ordering is preserved when a
> dependency is headed by an unsuccessful cmpxchg.  As it turns out, the
> change could enable further simplification of LKMM as proposed in [2].
> 
> [1] https://marc.info/?l=linux-kernel=150884953419377=2
> https://marc.info/?l=linux-kernel=150884946319353=2
> https://marc.info/?l=linux-kernel=151215810824468=2
> https://marc.info/?l=linux-kernel=151215816324484=2
> 
> [2] https://marc.info/?l=linux-kernel=151881978314872=2
> 
> Signed-off-by: Andrea Parri <parri.and...@gmail.com>
> Acked-by: Peter Zijlstra <pet...@infradead.org>
> Cc: Will Deacon <will.dea...@arm.com>
> Cc: "Paul E. McKenney" <paul...@linux.vnet.ibm.com>

Acked-by: "Paul E. McKenney" <paul...@linux.vnet.ibm.com>

> Cc: Alan Stern <st...@rowland.harvard.edu>
> Cc: Richard Henderson <r...@twiddle.net>
> Cc: Ivan Kokshaysky <i...@jurassic.park.msu.ru>
> Cc: Matt Turner <matts...@gmail.com>
> Cc: linux-alpha@vger.kernel.org
> Cc: linux-ker...@vger.kernel.org
> ---
>  arch/alpha/include/asm/xchg.h | 15 +++
>  1 file changed, 7 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/alpha/include/asm/xchg.h b/arch/alpha/include/asm/xchg.h
> index 68dfb3cb71454..e2660866ce972 100644
> --- a/arch/alpha/include/asm/xchg.h
> +++ b/arch/alpha/include/asm/xchg.h
> @@ -128,10 +128,9 @@ xchg(, volatile void *ptr, unsigned long x, int size)
>   * store NEW in MEM.  Return the initial value in MEM.  Success is
>   * indicated by comparing RETURN with OLD.
>   *
> - * The memory barrier should be placed in SMP only when we actually
> - * make the change. If we don't change anything (so if the returned
> - * prev is equal to old) then we aren't acquiring anything new and
> - * we don't need any memory barrier as far I can tell.
> + * The memory barrier is placed in SMP unconditionally, in order to
> + * guarantee that dependency ordering is preserved when a dependency
> + * is headed by an unsuccessful operation.
>   */
> 
>  static inline unsigned long
> @@ -150,8 +149,8 @@ cmpxchg(_u8, volatile char *m, unsigned char old, 
> unsigned char new)
>   "   or  %1,%2,%2\n"
>   "   stq_c   %2,0(%4)\n"
>   "   beq %2,3f\n"
> - __ASM__MB
>   "2:\n"
> + __ASM__MB
>   ".subsection 2\n"
>   "3: br  1b\n"
>   ".previous"
> @@ -177,8 +176,8 @@ cmpxchg(_u16, volatile short *m, unsigned short old, 
> unsigned short new)
>   "   or  %1,%2,%2\n"
>   "   stq_c   %2,0(%4)\n"
>   "   beq %2,3f\n"
> - __ASM__MB
>   "2:\n"
> + __ASM__MB
>   ".subsection 2\n"
>   "3: br  1b\n"
>   ".previous"
> @@ -200,8 +199,8 @@ cmpxchg(_u32, volatile int *m, int old, int new)
>   "   mov %4,%1\n"
>   "   stl_c %1,%2\n"
>   "   beq %1,3f\n"
> - __ASM__MB
>   "2:\n"
> + __ASM__MB
>   ".subsection 2\n"
>   "3: br 1b\n"
>   ".previous"
> @@ -223,8 +222,8 @@ cmpxchg(_u64, volatile long *m, unsigned long old, 
> unsigned long new)
>   "   mov %4,%1\n"
>   "   stq_c %1,%2\n"
>   "   beq %1,3f\n"
> - __ASM__MB
>   "2:\n"
> + __ASM__MB
>   ".subsection 2\n"
>   "3: br 1b\n"
>   ".previous"
> -- 
> 2.7.4
> 

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

[tip:core/rcu] drivers/infiniband: Remove now-redundant smp_read_barrier_depends()

2018-01-03 Thread tip-bot for Paul E. McKenney 
Commit-ID:  adf90eb49055636fc35aede54174456ac3520f27
Gitweb: https://git.kernel.org/tip/adf90eb49055636fc35aede54174456ac3520f27
Author: Paul E. McKenney <paul...@linux.vnet.ibm.com>
AuthorDate: Mon, 27 Nov 2017 09:04:22 -0800
Committer:  Paul E. McKenney <paul...@linux.vnet.ibm.com>
CommitDate: Tue, 5 Dec 2017 11:56:54 -0800

drivers/infiniband: Remove now-redundant smp_read_barrier_depends()

The smp_read_barrier_depends() does nothing at all except on DEC Alpha,
and no current DEC Alpha systems use Infiniband:

lkml.kernel.org/r/20171023085921.jwbntptn6ictbnvj@tower

This commit therefore makes Infiniband depend on !ALPHA and removes
the now-ineffective invocations of smp_read_barrier_depends() from
the InfiniBand driver.

Please note that this patch should not be construed as my saying that
InfiniBand's memory ordering is correct, but rather that this patch does
not in any way affect InfiniBand's correctness.  In other words, the
result of applying this patch is bug-for-bug compatible with the original.

Signed-off-by: Paul E. McKenney <paul...@linux.vnet.ibm.com>
Cc: Doug Ledford <dledf...@redhat.com>
Cc: Richard Henderson <r...@twiddle.net>
Cc: Ivan Kokshaysky <i...@jurassic.park.msu.ru>
Cc: Matt Turner <matts...@gmail.com>
Cc: Michael Cree <mc...@orcon.net.nz>
Cc: Andrea Parri <parri.and...@gmail.com>
Cc: <linux-r...@vger.kernel.org>
Cc: <linux-alpha@vger.kernel.org>
[ paulmck: Removed drivers/dma/ioat/dma.c per Jason Gunthorpe's feedback. ]
Acked-by: Jason Gunthorpe <j...@mellanox.com>
---
 drivers/infiniband/Kconfig  | 1 +
 drivers/infiniband/hw/hfi1/rc.c | 4 
 drivers/infiniband/hw/hfi1/ruc.c| 1 -
 drivers/infiniband/hw/hfi1/sdma.c   | 1 -
 drivers/infiniband/hw/hfi1/uc.c | 2 --
 drivers/infiniband/hw/hfi1/ud.c | 2 --
 drivers/infiniband/hw/qib/qib_rc.c  | 3 ---
 drivers/infiniband/hw/qib/qib_ruc.c | 1 -
 drivers/infiniband/hw/qib/qib_uc.c  | 2 --
 drivers/infiniband/hw/qib/qib_ud.c  | 2 --
 drivers/infiniband/sw/rdmavt/qp.c   | 1 -
 11 files changed, 1 insertion(+), 19 deletions(-)

diff --git a/drivers/infiniband/Kconfig b/drivers/infiniband/Kconfig
index 98ac46e..3bb6e35 100644
--- a/drivers/infiniband/Kconfig
+++ b/drivers/infiniband/Kconfig
@@ -4,6 +4,7 @@ menuconfig INFINIBAND
depends on NET
depends on INET
depends on m || IPV6 != m
+   depends on !ALPHA
select IRQ_POLL
---help---
  Core support for InfiniBand (IB).  Make sure to also select
diff --git a/drivers/infiniband/hw/hfi1/rc.c b/drivers/infiniband/hw/hfi1/rc.c
index fd01a76..f527bcd 100644
--- a/drivers/infiniband/hw/hfi1/rc.c
+++ b/drivers/infiniband/hw/hfi1/rc.c
@@ -302,7 +302,6 @@ int hfi1_make_rc_req(struct rvt_qp *qp, struct 
hfi1_pkt_state *ps)
if (!(ib_rvt_state_ops[qp->state] & RVT_FLUSH_SEND))
goto bail;
/* We are in the error state, flush the work request. */
-   smp_read_barrier_depends(); /* see post_one_send() */
if (qp->s_last == READ_ONCE(qp->s_head))
goto bail;
/* If DMAs are in progress, we can't flush immediately. */
@@ -346,7 +345,6 @@ int hfi1_make_rc_req(struct rvt_qp *qp, struct 
hfi1_pkt_state *ps)
newreq = 0;
if (qp->s_cur == qp->s_tail) {
/* Check if send work queue is empty. */
-   smp_read_barrier_depends(); /* see post_one_send() */
if (qp->s_tail == READ_ONCE(qp->s_head)) {
clear_ahg(qp);
goto bail;
@@ -900,7 +898,6 @@ void hfi1_send_rc_ack(struct hfi1_ctxtdata *rcd,
}
 
/* Ensure s_rdma_ack_cnt changes are committed */
-   smp_read_barrier_depends();
if (qp->s_rdma_ack_cnt) {
hfi1_queue_rc_ack(qp, is_fecn);
return;
@@ -1562,7 +1559,6 @@ static void rc_rcv_resp(struct hfi1_packet *packet)
trace_hfi1_ack(qp, psn);
 
/* Ignore invalid responses. */
-   smp_read_barrier_depends(); /* see post_one_send */
if (cmp_psn(psn, READ_ONCE(qp->s_next_psn)) >= 0)
goto ack_done;
 
diff --git a/drivers/infiniband/hw/hfi1/ruc.c b/drivers/infiniband/hw/hfi1/ruc.c
index 2c7fc6e..13b9947 100644
--- a/drivers/infiniband/hw/hfi1/ruc.c
+++ b/drivers/infiniband/hw/hfi1/ruc.c
@@ -362,7 +362,6 @@ static void ruc_loopback(struct rvt_qp *sqp)
sqp->s_flags |= RVT_S_BUSY;
 
 again:
-   smp_read_barrier_depends(); /* see post_one_send() */
if (sqp->s_last == READ_ONCE(sqp->s_head))
goto clr_busy;
wqe = rvt_get_swqe_ptr(sqp, sqp->s_last);
diff --git a/drivers/infiniband/hw/hfi1/sdma.c 
b/drivers/infiniband/hw/hfi1/sdma.c
index 31c8f89..61c130d 100644
--- a/drivers/infiniband/hw/hfi1/sdm

Re: [PATCH tip/core/rcu 16/21] drivers/infiniband: Remove now-redundant smp_read_barrier_depends()

2017-12-05 Thread Paul E. McKenney 
On Tue, Dec 05, 2017 at 08:08:32AM -0700, Jason Gunthorpe wrote:
> > commit c389c98ec5f4a7aa4c36853e89801eb5ea81870e
> > Author: Paul E. McKenney <paul...@linux.vnet.ibm.com>
> > Date:   Mon Nov 27 09:04:22 2017 -0800
> > 
> > drivers/infiniband: Remove now-redundant smp_read_barrier_depends()
> > 
> > The smp_read_barrier_depends() does nothing at all except on DEC Alpha,
> > and no current DEC Alpha systems use Infiniband:
> > 
> > lkml.kernel.org/r/20171023085921.jwbntptn6ictbnvj@tower
> > 
> > This commit therefore makes Infiniband depend on !ALPHA and removes
> > the now-ineffective invocations of smp_read_barrier_depends() from
> > the InfiniBand driver.
> > 
> > Please note that this patch should not be construed as my saying that
> > InfiniBand's memory ordering is correct, but rather that this patch does
> > not in any way affect InfiniBand's correctness.  In other words, the
> >     result of applying this patch is bug-for-bug compatible with the 
> > original.
> > 
> > Signed-off-by: Paul E. McKenney <paul...@linux.vnet.ibm.com>
> > Cc: Doug Ledford <dledf...@redhat.com>
> > Cc: Jason Gunthorpe <j...@mellanox.com>
> > Cc: Richard Henderson <r...@twiddle.net>
> > Cc: Ivan Kokshaysky <i...@jurassic.park.msu.ru>
> > Cc: Matt Turner <matts...@gmail.com>
> > Cc: Michael Cree <mc...@orcon.net.nz>
> > Cc: Andrea Parri <parri.and...@gmail.com>
> > Cc: <linux-r...@vger.kernel.org>
> > Cc: <linux-alpha@vger.kernel.org>
> > [ paulmck: Removed drivers/dma/ioat/dma.c per Jason Gunthorpe's 
> > feedback. ]
> 
> Let me know if you want this patch to flow through the rdma tree..
> 
> Acked-by: Jason Gunthorpe <j...@mellanox.com>

I applied your ack, thank you!

Your choice as to what tree it flows through.  By default, and if all
goes well, I will push this series through -rcu and -tip during the next
merge window.

Thanx, Paul

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

Re: [PATCH tip/core/rcu 16/21] drivers/infiniband: Remove now-redundant smp_read_barrier_depends()

2017-12-01 Thread Paul E. McKenney 
On Fri, Dec 01, 2017 at 05:11:09PM -0700, Jason Gunthorpe wrote:
> On Fri, Dec 01, 2017 at 11:51:11AM -0800, Paul E. McKenney wrote:
> > The smp_read_barrier_depends() does nothing at all except on DEC Alpha,
> > and no current DEC Alpha systems use Infiniband:
> > 
> > lkml.kernel.org/r/20171023085921.jwbntptn6ictbnvj@tower
> 
> I understand DEC Alpha has PCI, and we continue to support several
> RDMA devices that used the PCI bus.
> 
> However, the hif1, rdma vt and qib drivers modified in this patch are
> PCI-Express ONLY. So I think this idea is fine, but would revise the
> commit message to focus on PCI-E not Infiniband.

Hmmm...  It is not just the commit log, but also the Kconfig change.
I am not as worried as I might be about the few museum-piece DEC
Alpha systems suddenly sporting new RDMA PCI devices, but I am worried
about getting a more complex change right.

And if someone really does add a PCI RDMA device to their DEC Alpha,
and this patch causes them trouble, we can work out what to do about
it when and if that happens.

> >  drivers/dma/ioat/dma.c  | 2 --
> >  drivers/infiniband/Kconfig  | 1 +
> 
> Did you mean to have ioat/dma.c in this patch?

Indeed I did not, thank you for catching this!  Please see below for
updated ioat-free patch.

Thanx, Paul

----

commit c389c98ec5f4a7aa4c36853e89801eb5ea81870e
Author: Paul E. McKenney <paul...@linux.vnet.ibm.com>
Date:   Mon Nov 27 09:04:22 2017 -0800

drivers/infiniband: Remove now-redundant smp_read_barrier_depends()

The smp_read_barrier_depends() does nothing at all except on DEC Alpha,
and no current DEC Alpha systems use Infiniband:

lkml.kernel.org/r/20171023085921.jwbntptn6ictbnvj@tower

This commit therefore makes Infiniband depend on !ALPHA and removes
the now-ineffective invocations of smp_read_barrier_depends() from
the InfiniBand driver.

Please note that this patch should not be construed as my saying that
InfiniBand's memory ordering is correct, but rather that this patch does
not in any way affect InfiniBand's correctness.  In other words, the
result of applying this patch is bug-for-bug compatible with the original.

Signed-off-by: Paul E. McKenney <paul...@linux.vnet.ibm.com>
Cc: Doug Ledford <dledf...@redhat.com>
Cc: Jason Gunthorpe <j...@mellanox.com>
Cc: Richard Henderson <r...@twiddle.net>
Cc: Ivan Kokshaysky <i...@jurassic.park.msu.ru>
Cc: Matt Turner <matts...@gmail.com>
Cc: Michael Cree <mc...@orcon.net.nz>
Cc: Andrea Parri <parri.and...@gmail.com>
Cc: <linux-r...@vger.kernel.org>
Cc: <linux-alpha@vger.kernel.org>
[ paulmck: Removed drivers/dma/ioat/dma.c per Jason Gunthorpe's feedback. ]

diff --git a/drivers/infiniband/Kconfig b/drivers/infiniband/Kconfig
index 98ac46ed7214..3bb6e35b0bbf 100644
--- a/drivers/infiniband/Kconfig
+++ b/drivers/infiniband/Kconfig
@@ -4,6 +4,7 @@ menuconfig INFINIBAND
depends on NET
depends on INET
depends on m || IPV6 != m
+   depends on !ALPHA
select IRQ_POLL
---help---
  Core support for InfiniBand (IB).  Make sure to also select
diff --git a/drivers/infiniband/hw/hfi1/rc.c b/drivers/infiniband/hw/hfi1/rc.c
index fd01a760259f..f527bcda4650 100644
--- a/drivers/infiniband/hw/hfi1/rc.c
+++ b/drivers/infiniband/hw/hfi1/rc.c
@@ -302,7 +302,6 @@ int hfi1_make_rc_req(struct rvt_qp *qp, struct 
hfi1_pkt_state *ps)
if (!(ib_rvt_state_ops[qp->state] & RVT_FLUSH_SEND))
goto bail;
/* We are in the error state, flush the work request. */
-   smp_read_barrier_depends(); /* see post_one_send() */
if (qp->s_last == READ_ONCE(qp->s_head))
goto bail;
/* If DMAs are in progress, we can't flush immediately. */
@@ -346,7 +345,6 @@ int hfi1_make_rc_req(struct rvt_qp *qp, struct 
hfi1_pkt_state *ps)
newreq = 0;
if (qp->s_cur == qp->s_tail) {
/* Check if send work queue is empty. */
-   smp_read_barrier_depends(); /* see post_one_send() */
if (qp->s_tail == READ_ONCE(qp->s_head)) {
clear_ahg(qp);
goto bail;
@@ -900,7 +898,6 @@ void hfi1_send_rc_ack(struct hfi1_ctxtdata *rcd,
}
 
/* Ensure s_rdma_ack_cnt changes are committed */
-   smp_read_barrier_depends();
if (qp->s_rdma_ack_cnt) {
hfi1_queue_rc_ack(qp, is_fecn);
return;
@@ -1562,7 +1559,6 @@ static void rc_rcv_resp(struct hfi1_packet *packet)
t

Re: Do any Alpha systems include InfiniBand?

2017-10-23 Thread Paul E. McKenney 
On Mon, Oct 23, 2017 at 06:16:25PM +0100, Will Deacon wrote:
> On Mon, Oct 23, 2017 at 10:13:12AM -0700, Paul E. McKenney wrote:
> > On Mon, Oct 23, 2017 at 09:59:21PM +1300, Michael Cree wrote:
> > > On Fri, Oct 20, 2017 at 12:48:32PM -0700, Paul E. McKenney wrote:
> > > > Do any of the DEC Alpha systems that run recent kernels have InfiniBand?
> > > > Given my understanding of the history, I believe the answer to be "no".
> > > 
> > > I am not aware of any Alpha systems with InfiniBand.
> > 
> > Very good, then we need not worry about any Alpha-related consequences
> > of removing smp_read_barrier_depends() invocations from the InfiniBand
> > driver.  ;-)
> 
> But maybe make that driver depend on !CONFIG_ALPHA, so as to avoid planting
> any subtle ordering bugs?

Good point, that does sound like a very sensible precaution!

Thanx, Paul

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

Re: Do any Alpha systems include InfiniBand?

2017-10-23 Thread Paul E. McKenney 
On Mon, Oct 23, 2017 at 09:59:21PM +1300, Michael Cree wrote:
> On Fri, Oct 20, 2017 at 12:48:32PM -0700, Paul E. McKenney wrote:
> > Do any of the DEC Alpha systems that run recent kernels have InfiniBand?
> > Given my understanding of the history, I believe the answer to be "no".
> 
> I am not aware of any Alpha systems with InfiniBand.

Very good, then we need not worry about any Alpha-related consequences
of removing smp_read_barrier_depends() invocations from the InfiniBand
driver.  ;-)

Thanx, Paul

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

Re: Do any Alpha systems include InfiniBand?

2017-10-20 Thread Paul E. McKenney 
On Fri, Oct 20, 2017 at 01:00:14PM -0700, Matt Turner wrote:
> On Fri, Oct 20, 2017 at 12:48 PM, Paul E. McKenney
> <paul...@linux.vnet.ibm.com> wrote:
> > Hello, Michael,
> >
> > Do any of the DEC Alpha systems that run recent kernels have InfiniBand?
> > Given my understanding of the history, I believe the answer to be "no".
> > If I am wrong, please let me know, as I will otherwise interpret silence
> > as assent.  ;-)
> 
> I'm not aware of any shipping with Infiniband, and I haven't heard of
> anyone putting Infiniband in an Alpha.
> 
> Why do you ask? :)

Because the Linux-kernel InfiniBand driver contains a bunch of instances
of smp_read_barrier_depends(), which matter only on Alpha.  Plus the
InfiniBand maintainers have not responded to queries about what these
smp_read_barrier_depends() instances are supposed to do.  Now it might be
that they are just busy, or it might be that they don't know themselves.

If Alpha doesn't use InfiniBand, they can simply be removed.  On the
other hand, if Alpha -did- use InfiniBand, it would be necessary to
inspect the code to determine whether if READ_ONCE() needs to be added
somewhere to make up for the removal of smp_read_barrier_depends().

The reason this came up to begin with is that Will Deacon is adding an
smp_read_barrier_depends() to READ_ONCE() and Mark Rutland is converting
all ACCESS_ONCE() instances to either READ_ONCE() or WRITE_ONCE().
With these changes, core code need not know about Alpha, and almost all
instances of smp_read_barrier_depends() can be removed.

Hey, you asked!  ;-)

Thanx, Paul

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

Do any Alpha systems include InfiniBand?

2017-10-20 Thread Paul E. McKenney 
Hello, Michael,

Do any of the DEC Alpha systems that run recent kernels have InfiniBand?
Given my understanding of the history, I believe the answer to be "no".
If I am wrong, please let me know, as I will otherwise interpret silence
as assent.  ;-)

Thanx, Paul

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

Re: [RFC PATCH 1/2] arm64: mm: Use READ_ONCE/WRITE_ONCE when accessing page tables

2017-10-05 Thread Paul E. McKenney 
On Thu, Oct 05, 2017 at 09:31:48PM +0200, Andrea Parri wrote:
> Hi Will,
> 
> none of my comments below represent objections to this patch, but
> let me remark:
> 
> 
> On Thu, Oct 05, 2017 at 05:31:54PM +0100, Will Deacon wrote:
> > Hi Paul,
> > 
> > On Tue, Oct 03, 2017 at 12:11:10PM -0700, Paul E. McKenney wrote:
> > > On Fri, Sep 29, 2017 at 05:33:49PM +0100, Will Deacon wrote:
> > > > On Fri, Sep 29, 2017 at 09:29:39AM -0700, Paul E. McKenney wrote:
> > > > > On Fri, Sep 29, 2017 at 10:08:43AM +0100, Will Deacon wrote:
> > > > > > Ok, but where does that leave us wrt my initial proposal of moving
> > > > > > smp_read_barrier_depends() into READ_ONCE and getting rid of
> > > > > > lockless_dereference?
> > > > > > 
> > > > > > Michael (or anybody else running mainline on SMP Alpha) -- would 
> > > > > > you be
> > > > > > able to give the diff below a spin and see whether there's a 
> > > > > > measurable
> > > > > > performance impact?
> > > > > 
> > > > > This will be a sensitive test.  The smp_read_barrier_depends() can be
> > > > > removed from lockless_dereference().  Without this removal Alpha will
> > > > > get two memory barriers from rcu_dereference() and friends.
> > > > 
> > > > Oh yes, good point. I was trying to keep the diff simple, but you're
> > > > right that this is packing too many barriers. Fixed diff below.
> > > 
> > > Not seeing any objections thus far.  If there are none by (say) the
> > > end of this week, I would be happy to queue a patch for the 4.16
> > > merge window.  That should give ample opportunity for further review
> > > and testing.
> > 
> > Ok, full patch below.
> > 
> > Will
> > 
> > --->8
> > 
> > From 15956d0cc6b37208d8542b1858a8d8b64227acf4 Mon Sep 17 00:00:00 2001
> > From: Will Deacon <will.dea...@arm.com>
> > Date: Thu, 5 Oct 2017 16:57:36 +0100
> > Subject: [PATCH] locking/barriers: Kill lockless_dereference
> > 
> > lockless_dereference is a nice idea, but it's gained little traction in
> > kernel code since it's introduction three years ago. This is partly
> > because it's a pain to type, but also because using READ_ONCE instead
> > will work correctly on all architectures apart from Alpha, which is a
> > fully supported but somewhat niche architecture these days.
> 
> lockless_dereference might be a mouthful, but it does (explicitly)
> say/remark: "Yep, we are relying on the following address dep. to
> be "in strong-ppo" ".
> 
> Such information will be lost or, at least, not immediately clear
> by just reading a READ_ONCE(). (And Yes, this information is only
> relevant when we "include" Alpha in the picture/analysis.)

It is possible to argue that lockless_dereference() should remain for
this reason, even given READ_ONCE() containing smp_read_barrier_depends().
However, such arguments would be much more compelling if there were
tools that cared about the difference.

> > This patch moves smp_read_barrier_depends() (a NOP on all architectures
> > other than Alpha) from lockless_dereference into READ_ONCE, converts
> > the few actual users over to READ_ONCE and then finally removes
> > lockless_dereference altogether.
> 
> Notice that several "potential users" of lockless_dereference are
> currently hidden in other call sites for smp_read_barrier_depends
> (i.e., cases where this barrier is not called from within a lock-
> less or an RCU dereference).
> 
> Some of these usages (e.g.,
> 
>   include/linux/percpu-refcount.h:__ref_is_percpu,
>   mm/ksm.c:get_ksm_page,
>   security/keys/keyring.c:search_nested_keyrings )
> 
> precedes this barrier with a READ_ONCE; others (e.g.,
> 
>   arch/alpha/include/asm/pgtable.h:pmd_offset,
>   net/ipv4/netfilter/arp_tables.c:arpt_do_table
>   kernel/kernel/events/uprobes.c:get_trampiline_vaddr )
> 
> with a plain read.

I would welcome patches for the cases where smp_read_barrier_depends() is
preceded by READ_ONCE().  Perhaps the others should gain a READ_ONCE(),
and I suspect that they should, but ultimately that decision is in
the hands of the relevant maintainer, so any such patches need to be
separated and will need at least an ack from the relevant maintainers.

> There also appear to be cases where the barrier is preceded by an
> ACCESS_ONCE (c.f, fs/dcache.c:prepend_name) or by an xchg_release
> (c.f., kernel/locking/qspinlock.c:queued_spin_lock_slowpath), and
>

Re: [RFC PATCH 1/2] arm64: mm: Use READ_ONCE/WRITE_ONCE when accessing page tables

2017-09-29 Thread Paul E. McKenney 
On Fri, Sep 29, 2017 at 10:08:43AM +0100, Will Deacon wrote:
> On Thu, Sep 28, 2017 at 05:58:30PM -0700, Paul E. McKenney wrote:
> > On Fri, Sep 29, 2017 at 07:59:09AM +1300, Michael Cree wrote:
> > > On Thu, Sep 28, 2017 at 08:43:54AM -0700, Paul E. McKenney wrote:
> > > > On Thu, Sep 28, 2017 at 09:45:35AM +0100, Will Deacon wrote:
> > > > > On Thu, Sep 28, 2017 at 10:38:01AM +0200, Peter Zijlstra wrote:
> > > > > > On Wed, Sep 27, 2017 at 04:49:28PM +0100, Will Deacon wrote:
> > > > > > > In many cases, page tables can be accessed concurrently by either 
> > > > > > > another
> > > > > > > CPU (due to things like fast gup) or by the hardware page table 
> > > > > > > walker
> > > > > > > itself, which may set access/dirty bits. In such cases, it is 
> > > > > > > important
> > > > > > > to use READ_ONCE/WRITE_ONCE when accessing page table entries so 
> > > > > > > that
> > > > > > > entries cannot be torn, merged or subject to apparent loss of 
> > > > > > > coherence.
> > > > > > 
> > > > > > In fact, we should use lockless_dereference() for many of them. Yes
> > > > > > Alpha is the only one that cares about the difference between that 
> > > > > > and
> > > > > > READ_ONCE() and they do have the extra barrier, but if we're going 
> > > > > > to do
> > > > > > this, we might as well do it 'right' :-)
> > > > > 
> > > > > I know this sounds daft, but I think one of the big reasons why
> > > > > lockless_dereference() doesn't get an awful lot of use is because it's
> > > > > such a mouthful! Why don't we just move the smp_read_barrier_depends()
> > > > > into READ_ONCE? Would anybody actually care about the potential 
> > > > > impact on
> > > > > Alpha (which, frankly, is treading on thin ice given the low adoption 
> > > > > of
> > > > > lockless_dereference())?
> > > > 
> > > > This is my cue to ask my usual question...  ;-)
> > > > 
> > > > Are people still running mainline kernels on Alpha?  (Added Alpha 
> > > > folks.)
> > > 
> > > Yes.  I run two Alpha build daemons that build the unofficial
> > > debian-alpha port.  Debian popcon reports nine machines running
> > > Alpha, which are likely to be running the 4.12.y kernel which
> > > is currently in debian-alpha, (and presumably soon to be 4.13.y
> > > which is now built on Alpha in experimental).
> > 
> > I salute your dedication to Alpha!  ;-)
> 
> Ok, but where does that leave us wrt my initial proposal of moving
> smp_read_barrier_depends() into READ_ONCE and getting rid of
> lockless_dereference?
> 
> Michael (or anybody else running mainline on SMP Alpha) -- would you be
> able to give the diff below a spin and see whether there's a measurable
> performance impact?

This will be a sensitive test.  The smp_read_barrier_depends() can be
removed from lockless_dereference().  Without this removal Alpha will
get two memory barriers from rcu_dereference() and friends.

Thanx, Paul

> Cheers,
> 
> Will
> 
> --->8
> 
> diff --git a/include/linux/compiler.h b/include/linux/compiler.h
> index e95a2631e545..0ce21e25492a 100644
> --- a/include/linux/compiler.h
> +++ b/include/linux/compiler.h
> @@ -340,6 +340,7 @@ static __always_inline void __write_once_size(volatile 
> void *p, void *res, int s
>   __read_once_size(&(x), __u.__c, sizeof(x)); \
>   else\
>   __read_once_size_nocheck(&(x), __u.__c, sizeof(x)); \
> + smp_read_barrier_depends(); /* Enforce dependency ordering from x */ \
>   __u.__val;  \
>  })
>  #define READ_ONCE(x) __READ_ONCE(x, 1)
> 

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

Re: [RFC PATCH 1/2] arm64: mm: Use READ_ONCE/WRITE_ONCE when accessing page tables

2017-09-28 Thread Paul E. McKenney 
On Thu, Sep 28, 2017 at 04:49:54PM +0100, Will Deacon wrote:
> On Thu, Sep 28, 2017 at 08:43:54AM -0700, Paul E. McKenney wrote:
> > On Thu, Sep 28, 2017 at 09:45:35AM +0100, Will Deacon wrote:
> > > On Thu, Sep 28, 2017 at 10:38:01AM +0200, Peter Zijlstra wrote:
> > > > On Wed, Sep 27, 2017 at 04:49:28PM +0100, Will Deacon wrote:
> > > > > In many cases, page tables can be accessed concurrently by either 
> > > > > another
> > > > > CPU (due to things like fast gup) or by the hardware page table walker
> > > > > itself, which may set access/dirty bits. In such cases, it is 
> > > > > important
> > > > > to use READ_ONCE/WRITE_ONCE when accessing page table entries so that
> > > > > entries cannot be torn, merged or subject to apparent loss of 
> > > > > coherence.
> > > > 
> > > > In fact, we should use lockless_dereference() for many of them. Yes
> > > > Alpha is the only one that cares about the difference between that and
> > > > READ_ONCE() and they do have the extra barrier, but if we're going to do
> > > > this, we might as well do it 'right' :-)
> > > 
> > > I know this sounds daft, but I think one of the big reasons why
> > > lockless_dereference() doesn't get an awful lot of use is because it's
> > > such a mouthful! Why don't we just move the smp_read_barrier_depends()
> > > into READ_ONCE? Would anybody actually care about the potential impact on
> > > Alpha (which, frankly, is treading on thin ice given the low adoption of
> > > lockless_dereference())?
> > 
> > This is my cue to ask my usual question...  ;-)
> > 
> > Are people still running mainline kernels on Alpha?  (Added Alpha folks.)
> > 
> > As always, if anyone is, we must continue to support Alpha, but sounds
> > like time to check again.
> 
> I'll be honest and say that I haven't updated mine for a while, but I do
> have a soft spot for those machines :(

Let's see what the Alpha folks say.  I myself have had a close
relationship with Alpha for almost 20 years, but I suspect that in
my case it is more a hard spot on my head rather than a soft spot in
my heart.  ;-)

Thanx,
Paul

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

Re: [RFC PATCH 1/2] arm64: mm: Use READ_ONCE/WRITE_ONCE when accessing page tables

2017-09-28 Thread Paul E. McKenney 
On Thu, Sep 28, 2017 at 09:45:35AM +0100, Will Deacon wrote:
> On Thu, Sep 28, 2017 at 10:38:01AM +0200, Peter Zijlstra wrote:
> > On Wed, Sep 27, 2017 at 04:49:28PM +0100, Will Deacon wrote:
> > > In many cases, page tables can be accessed concurrently by either another
> > > CPU (due to things like fast gup) or by the hardware page table walker
> > > itself, which may set access/dirty bits. In such cases, it is important
> > > to use READ_ONCE/WRITE_ONCE when accessing page table entries so that
> > > entries cannot be torn, merged or subject to apparent loss of coherence.
> > 
> > In fact, we should use lockless_dereference() for many of them. Yes
> > Alpha is the only one that cares about the difference between that and
> > READ_ONCE() and they do have the extra barrier, but if we're going to do
> > this, we might as well do it 'right' :-)
> 
> I know this sounds daft, but I think one of the big reasons why
> lockless_dereference() doesn't get an awful lot of use is because it's
> such a mouthful! Why don't we just move the smp_read_barrier_depends()
> into READ_ONCE? Would anybody actually care about the potential impact on
> Alpha (which, frankly, is treading on thin ice given the low adoption of
> lockless_dereference())?

This is my cue to ask my usual question...  ;-)

Are people still running mainline kernels on Alpha?  (Added Alpha folks.)

As always, if anyone is, we must continue to support Alpha, but sounds
like time to check again.

Thanx, Paul

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

[PATCH RFC 09/26] alpha: Remove spin_unlock_wait() arch-specific definitions

2017-06-29 Thread Paul E. McKenney 
There is no agreed-upon definition of spin_unlock_wait()'s semantics,
and it appears that all callers could do just as well with a lock/unlock
pair.  This commit therefore removes the underlying arch-specific
arch_spin_unlock_wait().

Signed-off-by: Paul E. McKenney <paul...@linux.vnet.ibm.com>
Cc: Richard Henderson <r...@twiddle.net>
Cc: Ivan Kokshaysky <i...@jurassic.park.msu.ru>
Cc: Matt Turner <matts...@gmail.com>
Cc: <linux-alpha@vger.kernel.org>
Cc: Will Deacon <will.dea...@arm.com>
Cc: Peter Zijlstra <pet...@infradead.org>
Cc: Alan Stern <st...@rowland.harvard.edu>
Cc: Andrea Parri <parri.and...@gmail.com>
Cc: Linus Torvalds <torva...@linux-foundation.org>
---
 arch/alpha/include/asm/spinlock.h | 5 -
 1 file changed, 5 deletions(-)

diff --git a/arch/alpha/include/asm/spinlock.h 
b/arch/alpha/include/asm/spinlock.h
index a40b9fc0c6c3..718ac0b64adf 100644
--- a/arch/alpha/include/asm/spinlock.h
+++ b/arch/alpha/include/asm/spinlock.h
@@ -16,11 +16,6 @@
 #define arch_spin_lock_flags(lock, flags) arch_spin_lock(lock)
 #define arch_spin_is_locked(x) ((x)->lock != 0)
 
-static inline void arch_spin_unlock_wait(arch_spinlock_t *lock)
-{
-   smp_cond_load_acquire(>lock, !VAL);
-}
-
 static inline int arch_spin_value_unlocked(arch_spinlock_t lock)
 {
 return lock.lock == 0;
-- 
2.5.2

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

Re: [RFC 10/12] x86, rwsem: simplify __down_write

2016-06-10 Thread Paul E. McKenney 
On Thu, Jun 09, 2016 at 07:36:40PM +0200, Peter Zijlstra wrote:
> On Thu, Jun 09, 2016 at 03:40:58PM +0100, David Howells wrote:
> > Peter Zijlstra  wrote:
> > 
> > > Blergh; so looking at more asm there's still a few tricks we cannot do.
> > > So while overall size is down, some paths do end up more expensive. (It
> > > typically boils down to creative use of condition flags, which is very
> > > hard in C)
> > 
> > It can be done using ISO __atomic_fetch_add() and suchlike.
> 
> (ISO-C11, ISO as such is a bad abbreviation I think)
> 
> Maybe, but we're almost there with __GCC_ASM_FLAG_OUTPUTS__.
> 
> atomic_long_add_negative() can be made to do inc;j(n)s for __down_read.
> 
> the try_cmpxchg family you wanted to add independent from the ISO-C11
> bits can do the cmpxchg-j(n)z for __down_{read,write}_trylock.
> 
> That only leaves us wanting an atomic_long_fetch_add_negative() for
> __up_{read,write}().
> 
> Although I suppose, for this to be of use for our weakly ordered
> friends, we need _relaxed versions of all that (so that _acquire and
> _release variants are generated).

Historically, the compilers have won this sort of contest over the
long term.  That said, there is nothing quite like raising the bar for
them to help them generate decent code.  So, David and Peter, I am behind
both of you 100%.  ;-)

Thanx, Paul

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