[PATCH 15/16] netfilter: Replace smp_read_barrier_depends() with lockless_dereference()

[PATCH 15/16] netfilter: Replace smp_read_barrier_depends() with lockless_dereference()

[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>
---
 net/ipv4/netfilter/arp_tables.c | 3 +--
 net/ipv4/netfilter/ip_tables.c  | 3 +--
 net/ipv6/netfilter/ip6_tables.c | 3 +--
 3 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/net/ipv4/netfilter/arp_tables.c b/net/ipv4/netfilter/arp_tables.c
index f95b6f9..fc7533d 100644
--- a/net/ipv4/netfilter/arp_tables.c
+++ b/net/ipv4/netfilter/arp_tables.c
@@ -270,12 +270,11 @@ unsigned int arpt_do_table(struct sk_buff *skb,
 
 	local_bh_disable();
 	addend = xt_write_recseq_begin();
-	private = table->private;
 	/*
 	 * Ensure we load private-> members after we've fetched the base
 	 * pointer.
 	 */
-	smp_read_barrier_depends();
+	private = lockless_dereference(table->private);
 	table_base = private->entries[smp_processor_id()];
 
 	e = get_entry(table_base, private->hook_entry[hook]);
diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c
index 99e810f..e0fd044 100644
--- a/net/ipv4/netfilter/ip_tables.c
+++ b/net/ipv4/netfilter/ip_tables.c
@@ -325,13 +325,12 @@ ipt_do_table(struct sk_buff *skb,
 	IP_NF_ASSERT(table->valid_hooks & (1 << hook));
 	local_bh_disable();
 	addend = xt_write_recseq_begin();
-	private = table->private;
 	cpu        = smp_processor_id();
 	/*
 	 * Ensure we load private-> members after we've fetched the base
 	 * pointer.
 	 */
-	smp_read_barrier_depends();
+	private = lockless_dereference(table->private);
 	table_base = private->entries[cpu];
 	jumpstack  = (struct ipt_entry **)private->jumpstack[cpu];
 	stackptr   = per_cpu_ptr(private->stackptr, cpu);
diff --git a/net/ipv6/netfilter/ip6_tables.c b/net/ipv6/netfilter/ip6_tables.c
index e080fbb..0459d6a 100644
--- a/net/ipv6/netfilter/ip6_tables.c
+++ b/net/ipv6/netfilter/ip6_tables.c
@@ -348,12 +348,11 @@ ip6t_do_table(struct sk_buff *skb,
 
 	local_bh_disable();
 	addend = xt_write_recseq_begin();
-	private = table->private;
 	/*
 	 * Ensure we load private-> members after we've fetched the base
 	 * pointer.
 	 */
-	smp_read_barrier_depends();
+	private = lockless_dereference(table->private);
 	cpu        = smp_processor_id();
 	table_base = private->entries[cpu];
 	jumpstack  = (struct ip6t_entry **)private->jumpstack[cpu];
-- 
1.9.1

[PATCH 16/16] rxrpc: Replace smp_read_barrier_depends() with lockless_dereference()

[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>
---
 net/rxrpc/ar-ack.c | 22 +++++++++-------------
 1 file changed, 9 insertions(+), 13 deletions(-)

diff --git a/net/rxrpc/ar-ack.c b/net/rxrpc/ar-ack.c
index c6be17a..9237448 100644
--- a/net/rxrpc/ar-ack.c
+++ b/net/rxrpc/ar-ack.c
@@ -234,8 +234,7 @@ static void rxrpc_resend(struct rxrpc_call *call)
 	     loop != call->acks_head || stop;
 	     loop = (loop + 1) &  (call->acks_winsz - 1)
 	     ) {
-		p_txb = call->acks_window + loop;
-		smp_read_barrier_depends();
+		p_txb = lockless_dereference(call)->acks_window + loop;
 		if (*p_txb & 1)
 			continue;
 
@@ -303,8 +302,7 @@ static void rxrpc_resend_timer(struct rxrpc_call *call)
 	     loop != call->acks_head;
 	     loop = (loop + 1) &  (call->acks_winsz - 1)
 	     ) {
-		p_txb = call->acks_window + loop;
-		smp_read_barrier_depends();
+		p_txb = lockless_dereference(call)->acks_window + loop;
 		txb = (struct sk_buff *) (*p_txb & ~1);
 		sp = rxrpc_skb(txb);
 
@@ -354,9 +352,10 @@ static int rxrpc_process_soft_ACKs(struct rxrpc_call *call,
 	resend = 0;
 	resend_at = 0;
 	for (loop = 0; loop < ack->nAcks; loop++) {
-		p_txb = call->acks_window;
-		p_txb += (call->acks_tail + loop) & (call->acks_winsz - 1);
-		smp_read_barrier_depends();
+		struct rxrpc_call *callp = lockless_dereference(call);
+
+		p_txb = callp->acks_window;
+		p_txb += (callp->acks_tail + loop) & (callp->acks_winsz - 1);
 		txb = (struct sk_buff *) (*p_txb & ~1);
 		sp = rxrpc_skb(txb);
 
@@ -385,8 +384,7 @@ static int rxrpc_process_soft_ACKs(struct rxrpc_call *call,
 	     loop != call->acks_head;
 	     loop = (loop + 1) &  (call->acks_winsz - 1)
 	     ) {
-		p_txb = call->acks_window + loop;
-		smp_read_barrier_depends();
+		p_txb = lockless_dereference(call)->acks_window + loop;
 		txb = (struct sk_buff *) (*p_txb & ~1);
 		sp = rxrpc_skb(txb);
 
@@ -432,8 +430,7 @@ static void rxrpc_rotate_tx_window(struct rxrpc_call *call, u32 hard)
 	ASSERTCMP(hard - call->acks_hard, <=, win);
 
 	while (call->acks_hard < hard) {
-		smp_read_barrier_depends();
-		_skb = call->acks_window[tail] & ~1;
+		_skb = lockless_dereference(call)->acks_window[tail] & ~1;
 		rxrpc_free_skb((struct sk_buff *) _skb);
 		old_tail = tail;
 		tail = (tail + 1) & (call->acks_winsz - 1);
@@ -577,8 +574,7 @@ static void rxrpc_zap_tx_window(struct rxrpc_call *call)
 	call->acks_window = NULL;
 
 	while (CIRC_CNT(call->acks_head, call->acks_tail, winsz) > 0) {
-		tail = call->acks_tail;
-		smp_read_barrier_depends();
+		tail = lockless_dereference(call)->acks_tail;
 		_skb = acks_window[tail] & ~1;
 		smp_mb();
 		call->acks_tail = (call->acks_tail + 1) & (winsz - 1);
-- 
1.9.1

Re: [PATCH 16/16] rxrpc: Replace smp_read_barrier_depends() with lockless_dereference()

[David Howells]

Pranith Kumar <bobby.prani@gmail.com> wrote:

>  	     loop != call->acks_head || stop;
>  	     loop = (loop + 1) &  (call->acks_winsz - 1)
>  	     ) {
> -		p_txb = call->acks_window + loop;
> -		smp_read_barrier_depends();
> +		p_txb = lockless_dereference(call)->acks_window + loop;

Nack.  You've stuck an implicit barrier on a dereference that doesn't matter.
And similar for other hunks of this patch.

David

Re: [PATCH 16/16] rxrpc: Replace smp_read_barrier_depends() with lockless_dereference()

[David Howells]

Pranith Kumar <bobby.prani@gmail.com> 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.

Actually, the use of smp_read_barrier_depends() is wrong in circular
buffering.  See Documentation/circular-buffers.txt

David

Re: [PATCH 16/16] rxrpc: Replace smp_read_barrier_depends() with lockless_dereference()

[Pranith Kumar]

On 11/13/2014 03:47 PM, David Howells wrote:
> Pranith Kumar <bobby.prani@gmail.com> 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.
> 
> Actually, the use of smp_read_barrier_depends() is wrong in circular
> buffering.  See Documentation/circular-buffers.txt
> 

OK. Should I send in a patch removing these barriers then?

--
Pranith

Re: [PATCH 16/16] rxrpc: Replace smp_read_barrier_depends() with lockless_dereference()

[David Howells]

Pranith Kumar <bobby.prani@gmail.com> wrote:

> OK. Should I send in a patch removing these barriers then?

No.  There need to be stronger barriers, at least in some of the cases.
circular-buffers.txt details what is required, but not all of the cases match
the pattern there, so it needs a bit more consideration.

David

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>