On Fri, 2014-11-21 at 10:06 -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@xxxxxxxxx> > --- > 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()]; > Please carefully read the code, before and after your change, then you'll see this change broke the code. Problem is that a bug like that can be really hard to diagnose and fix later, so really you have to be very careful doing these mechanical changes. IMO, current code+comment is better than with this lockless_dereference() which in this particular case obfuscates the code. more than anything. In this case we do have a lock (sort of), so lockless_dereference() is quite misleading. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
