Re: [net-next PATCH 01/02] net/ipv4: VTI support rx-path hook in xfrm4_mode_tunnel.

[Joe Perches <joe@perches.com>]

On Tue, 2012-07-17 at 12:44 -0700, Saurabh wrote:
> Incorporated David and Steffen's comments.
> Add hook for rx-path xfmr4_mode_tunnel for VTI tunnel module.
[]
> diff --git a/net/ipv4/xfrm4_mode_tunnel.c b/net/ipv4/xfrm4_mode_tunnel.c
[]
> +int xfrm4_mode_tunnel_input_register(struct xfrm_tunnel *handler)
> +{
> +	struct xfrm_tunnel __rcu **pprev;
> +	struct xfrm_tunnel *t;
> +	int ret = -EEXIST;
> +	int priority = handler->priority;
> +
> +	mutex_lock(&xfrm4_mode_tunnel_input_mutex);
> +
> +	for (pprev = &rcv_notify_handlers;
> +	     (t = rcu_dereference_protected(*pprev,
> +	     lockdep_is_held(&xfrm4_mode_tunnel_input_mutex))) != NULL;
> +	     pprev = &t->next) {
> +		if (t->priority > priority)
> +			break;
> +		if (t->priority == priority)
> +			goto err;
> +
> +	}
> +
> +	handler->next = *pprev;
> +	rcu_assign_pointer(*pprev, handler);
> +
> +	ret = 0;
> +
> +err:
> +	mutex_unlock(&xfrm4_mode_tunnel_input_mutex);
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(xfrm4_mode_tunnel_input_register);

Isn't the multiple indirection of **pprev unnecessary?
Perhaps something like this is simpler and easier to read?

int xfrm4_mode_tunnel_input_register(struct xfrm_tunnel *handler)
{
	struct xfrm_tunnel __rcu *prev;
	struct xfrm_tunnel *t;
	int ret = -EEXIST;
	int priority = handler->priority;

	mutex_lock(&xfrm4_mode_tunnel_input_mutex);

	prev = rcv_notify_handlers;
	while ((t = rcu_dereference_protected(prev,
					      lockdep_is_held(&xfrm4_mode_tunnel_input_mutex))) {
		if (t->priority > priority)
			break;
		if (t->priority == priority)
			goto err;
		prev = t->next;
	}

	handler->next = prev;
	rcu_assign_pointer(prev, handler);

	ret = 0;

err:
	mutex_unlock(&xfrm4_mode_tunnel_input_mutex);
	return ret;
}