[RFC PATCH 00/16] Replace smp_read_barrier_depends() with lockless_derefrence()

[RFC PATCH 00/16] Replace smp_read_barrier_depends() with lockless_derefrence()

[Pranith Kumar]

Recently lockless_dereference() was added which can be used in place of
hard-coding smp_read_barrier_depends(). 

http://lkml.iu.edu/hypermail/linux/kernel/1410.3/04561.html

The following series tries to do this.

There are still some hard-coded locations which I was not sure how to replace
with. I will send in separate patches/questions regarding them.

Pranith Kumar (16):
  crypto: caam - Remove unnecessary smp_read_barrier_depends()
  doc: memory-barriers.txt: Document use of lockless_dereference()
  drivers: dma: Replace smp_read_barrier_depends() with
    lockless_dereference()
  dcache: Replace smp_read_barrier_depends() with lockless_dereference()
  overlayfs: Replace smp_read_barrier_depends() with
    lockless_dereference()
  assoc_array: Replace smp_read_barrier_depends() with
    lockless_dereference()
  hyperv: Replace smp_read_barrier_depends() with lockless_dereference()
  rcupdate: Replace smp_read_barrier_depends() with
    lockless_dereference()
  percpu: Replace smp_read_barrier_depends() with lockless_dereference()
  perf: Replace smp_read_barrier_depends() with lockless_dereference()
  seccomp: Replace smp_read_barrier_depends() with
    lockless_dereference()
  task_work: Replace smp_read_barrier_depends() with
    lockless_dereference()
  ksm: Replace smp_read_barrier_depends() with lockless_dereference()
  slab: Replace smp_read_barrier_depends() with lockless_dereference()
  netfilter: Replace smp_read_barrier_depends() with
    lockless_dereference()
  rxrpc: Replace smp_read_barrier_depends() with lockless_dereference()

 Documentation/memory-barriers.txt |  2 +-
 drivers/crypto/caam/jr.c          |  3 ---
 drivers/dma/ioat/dma_v2.c         |  3 +--
 drivers/dma/ioat/dma_v3.c         |  3 +--
 fs/dcache.c                       |  7 ++-----
 fs/overlayfs/super.c              |  4 +---
 include/linux/assoc_array_priv.h  | 11 +++++++----
 include/linux/hyperv.h            |  9 ++++-----
 include/linux/percpu-refcount.h   |  4 +---
 include/linux/rcupdate.h          | 10 +++++-----
 kernel/events/core.c              |  3 +--
 kernel/events/uprobes.c           |  8 ++++----
 kernel/seccomp.c                  |  7 +++----
 kernel/task_work.c                |  3 +--
 lib/assoc_array.c                 |  7 -------
 mm/ksm.c                          |  7 +++----
 mm/slab.h                         |  6 +++---
 net/ipv4/netfilter/arp_tables.c   |  3 +--
 net/ipv4/netfilter/ip_tables.c    |  3 +--
 net/ipv6/netfilter/ip6_tables.c   |  3 +--
 net/rxrpc/ar-ack.c                | 22 +++++++++-------------
 security/keys/keyring.c           |  6 ------
 22 files changed, 50 insertions(+), 84 deletions(-)

-- 
1.9.1

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 01/16] crypto: caam - Remove unnecessary smp_read_barrier_depends()

[Pranith Kumar]

Recently lockless_dereference() was added which can be used in place of
hard-coding smp_read_barrier_depends(). The following PATCH makes the change.

Signed-off-by: Pranith Kumar <bobby.prani@gmail.com>
---
 drivers/crypto/caam/jr.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/crypto/caam/jr.c b/drivers/crypto/caam/jr.c
index bae20d8..9b3ef1bc 100644
--- a/drivers/crypto/caam/jr.c
+++ b/drivers/crypto/caam/jr.c
@@ -181,8 +181,6 @@ static void caam_jr_dequeue(unsigned long devarg)
 		for (i = 0; CIRC_CNT(head, tail + i, JOBR_DEPTH) >= 1; i++) {
 			sw_idx = (tail + i) & (JOBR_DEPTH - 1);
 
-			smp_read_barrier_depends();
-
 			if (jrp->outring[hw_idx].desc ==
 			    jrp->entinfo[sw_idx].desc_addr_dma)
 				break; /* found */
@@ -218,7 +216,6 @@ static void caam_jr_dequeue(unsigned long devarg)
 		if (sw_idx == tail) {
 			do {
 				tail = (tail + 1) & (JOBR_DEPTH - 1);
-				smp_read_barrier_depends();
 			} while (CIRC_CNT(head, tail, JOBR_DEPTH) >= 1 &&
 				 jrp->entinfo[tail].desc_addr_dma == 0);
 
-- 
1.9.1

Re: [RFC PATCH 00/16] Replace smp_read_barrier_depends() with lockless_derefrence()

[Paul E. McKenney]

On Thu, Nov 13, 2014 at 02:24:06PM -0500, Pranith Kumar wrote:
> Recently lockless_dereference() was added which can be used in place of
> hard-coding smp_read_barrier_depends(). 
> 
> http://lkml.iu.edu/hypermail/linux/kernel/1410.3/04561.html
> 
> The following series tries to do this.
> 
> There are still some hard-coded locations which I was not sure how to replace
> with. I will send in separate patches/questions regarding them.

Thank you for taking this on!  Some questions and comments in response
to the individual patches.

							Thanx, Paul

> Pranith Kumar (16):
>   crypto: caam - Remove unnecessary smp_read_barrier_depends()
>   doc: memory-barriers.txt: Document use of lockless_dereference()
>   drivers: dma: Replace smp_read_barrier_depends() with
>     lockless_dereference()
>   dcache: Replace smp_read_barrier_depends() with lockless_dereference()
>   overlayfs: Replace smp_read_barrier_depends() with
>     lockless_dereference()
>   assoc_array: Replace smp_read_barrier_depends() with
>     lockless_dereference()
>   hyperv: Replace smp_read_barrier_depends() with lockless_dereference()
>   rcupdate: Replace smp_read_barrier_depends() with
>     lockless_dereference()
>   percpu: Replace smp_read_barrier_depends() with lockless_dereference()
>   perf: Replace smp_read_barrier_depends() with lockless_dereference()
>   seccomp: Replace smp_read_barrier_depends() with
>     lockless_dereference()
>   task_work: Replace smp_read_barrier_depends() with
>     lockless_dereference()
>   ksm: Replace smp_read_barrier_depends() with lockless_dereference()
>   slab: Replace smp_read_barrier_depends() with lockless_dereference()
>   netfilter: Replace smp_read_barrier_depends() with
>     lockless_dereference()
>   rxrpc: Replace smp_read_barrier_depends() with lockless_dereference()
> 
>  Documentation/memory-barriers.txt |  2 +-
>  drivers/crypto/caam/jr.c          |  3 ---
>  drivers/dma/ioat/dma_v2.c         |  3 +--
>  drivers/dma/ioat/dma_v3.c         |  3 +--
>  fs/dcache.c                       |  7 ++-----
>  fs/overlayfs/super.c              |  4 +---
>  include/linux/assoc_array_priv.h  | 11 +++++++----
>  include/linux/hyperv.h            |  9 ++++-----
>  include/linux/percpu-refcount.h   |  4 +---
>  include/linux/rcupdate.h          | 10 +++++-----
>  kernel/events/core.c              |  3 +--
>  kernel/events/uprobes.c           |  8 ++++----
>  kernel/seccomp.c                  |  7 +++----
>  kernel/task_work.c                |  3 +--
>  lib/assoc_array.c                 |  7 -------
>  mm/ksm.c                          |  7 +++----
>  mm/slab.h                         |  6 +++---
>  net/ipv4/netfilter/arp_tables.c   |  3 +--
>  net/ipv4/netfilter/ip_tables.c    |  3 +--
>  net/ipv6/netfilter/ip6_tables.c   |  3 +--
>  net/rxrpc/ar-ack.c                | 22 +++++++++-------------
>  security/keys/keyring.c           |  6 ------
>  22 files changed, 50 insertions(+), 84 deletions(-)
> 
> -- 
> 1.9.1
> 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 01/16] crypto: caam - Remove unnecessary smp_read_barrier_depends()

[Paul E. McKenney]

On Thu, Nov 13, 2014 at 02:24:07PM -0500, Pranith Kumar wrote:
> Recently lockless_dereference() was added which can be used in place of
> hard-coding smp_read_barrier_depends(). The following PATCH makes the change.
> 
> Signed-off-by: Pranith Kumar <bobby.prani@gmail.com>
> ---
>  drivers/crypto/caam/jr.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/drivers/crypto/caam/jr.c b/drivers/crypto/caam/jr.c
> index bae20d8..9b3ef1bc 100644
> --- a/drivers/crypto/caam/jr.c
> +++ b/drivers/crypto/caam/jr.c
> @@ -181,8 +181,6 @@ static void caam_jr_dequeue(unsigned long devarg)
>  		for (i = 0; CIRC_CNT(head, tail + i, JOBR_DEPTH) >= 1; i++) {
>  			sw_idx = (tail + i) & (JOBR_DEPTH - 1);
> 
> -			smp_read_barrier_depends();
> -

Did you mean to add a lockless_dereference() somewhere?  I would guess
that one is required on the load into sw_idx before the "for" loop,
but I cannot claim to be familiar with this code.

							Thanx, Paul

>  			if (jrp->outring[hw_idx].desc ==
>  			    jrp->entinfo[sw_idx].desc_addr_dma)
>  				break; /* found */
> @@ -218,7 +216,6 @@ static void caam_jr_dequeue(unsigned long devarg)
>  		if (sw_idx == tail) {
>  			do {
>  				tail = (tail + 1) & (JOBR_DEPTH - 1);
> -				smp_read_barrier_depends();
>  			} while (CIRC_CNT(head, tail, JOBR_DEPTH) >= 1 &&
>  				 jrp->entinfo[tail].desc_addr_dma == 0);
> 
> -- 
> 1.9.1
> 

Re: [PATCH 01/16] crypto: caam - Remove unnecessary smp_read_barrier_depends()

[Kim Phillips]

On Thu, 13 Nov 2014 16:51:12 -0500
Pranith Kumar <bobby.prani@gmail.com> wrote:

> On 11/13/2014 03:10 PM, Paul E. McKenney wrote:
> > On Thu, Nov 13, 2014 at 02:24:07PM -0500, Pranith Kumar wrote:
> >> Recently lockless_dereference() was added which can be used in place of
> >> hard-coding smp_read_barrier_depends(). The following PATCH makes the change.
> >>
> >> Signed-off-by: Pranith Kumar <bobby.prani@gmail.com>
> >> ---
> >>  drivers/crypto/caam/jr.c | 3 ---
> >>  1 file changed, 3 deletions(-)
> >>
> >> diff --git a/drivers/crypto/caam/jr.c b/drivers/crypto/caam/jr.c
> >> index bae20d8..9b3ef1bc 100644
> >> --- a/drivers/crypto/caam/jr.c
> >> +++ b/drivers/crypto/caam/jr.c
> >> @@ -181,8 +181,6 @@ static void caam_jr_dequeue(unsigned long devarg)
> >>  		for (i = 0; CIRC_CNT(head, tail + i, JOBR_DEPTH) >= 1; i++) {
> >>  			sw_idx = (tail + i) & (JOBR_DEPTH - 1);
> >>
> >> -			smp_read_barrier_depends();
> >> -
> > 
> > Did you mean to add a lockless_dereference() somewhere?  I would guess
> > that one is required on the load into sw_idx before the "for" loop,
> > but I cannot claim to be familiar with this code.
> 
> No, I could not understand why the barrier was here in the first place. The change log does not reflect the subject line.
> 
> sw_idx is a local variable and is used to index the entinfo array. It does not seem like the barrier is needed. If not a comment saying why could be added I guess.

I likely misinterpreted the "read index before reading contents at
that index" instructions in early circular buffer documentation, and
left it in for the benefit of the doubt.  Now it looks to me it's not
necessary, given both sw_idx and tail are just being computed within
a lock, and removing both barriers doesn't affect the compiler
output, so:

Reviewed-by: Kim Phillips <kim.phillips@freescale.com>

Thanks,

Kim