> For another the rcu protected macros rcu_dereference() and
> rcu_assign_pointer() for the bat_priv->curr_gw and curr_gw->orig_node
> need to be used.

What? Since when is curr_gw::orig_node rcu protected?

> -     return curr_gateway_tmp->orig_node;
> +     orig_node = rcu_dereference(curr_gateway_tmp->orig_node);
> +     if (orig_node) {
> +             rcu_read_unlock();
> +             return NULL;
> +     }
> +
> +     rcu_read_unlock();
> +     return orig_node;
>  }


And you cannot return the (it is not, but when it would be) rcu protected 
element without increasing the refcounter.

> +             orig_node = rcu_dereference(gw_node->orig_node);
> +             if (!orig_node->router)
>                       continue;

Same here

> -     if (bat_priv->curr_gw != curr_gw_tmp) {
> -             if ((bat_priv->curr_gw) && (!curr_gw_tmp))
> +     if (curr_gw != curr_gw_tmp) {
> +             orig_node = rcu_dereference(curr_gw_tmp->orig_node);
> +             if ((curr_gw) && (!curr_gw_tmp))

And here

> -     if (bat_priv->curr_gw != curr_gw_tmp) {
> -             if ((bat_priv->curr_gw) && (!curr_gw_tmp))
> +     if (curr_gw != curr_gw_tmp) {
> +             orig_node = rcu_dereference(curr_gw_tmp->orig_node);
> +             if ((curr_gw) && (!curr_gw_tmp))

Here again

> -     if (bat_priv->curr_gw != curr_gw_tmp) {
> -             if ((bat_priv->curr_gw) && (!curr_gw_tmp))
> +     if (curr_gw != curr_gw_tmp) {
> +             orig_node = rcu_dereference(curr_gw_tmp->orig_node);
> +             if ((curr_gw) && (!curr_gw_tmp))

Here....

> -     gw_bandwidth_to_kbit(gw_node->orig_node->gw_flags, &down, &up);
> +     rcu_read_lock();
> +     curr_gw = rcu_dereference(bat_priv->curr_gw);
> +     orig_node = rcu_dereference(gw_node->orig_node);
> +     gw_bandwidth_to_kbit(orig_node->gw_flags, &down, &up);

....

> diff --git a/batman-adv/unicast.c b/batman-adv/unicast.c
> index 6a9ab61..8816102 100644
> --- a/batman-adv/unicast.c
> +++ b/batman-adv/unicast.c
> @@ -298,7 +298,6 @@ int unicast_send_skb(struct sk_buff *skb, struct
> bat_priv *bat_priv) if (!orig_node)
>                       goto trans_search;
> 
> -             kref_get(&orig_node->refcount);
>               goto find_router;
>       } else {
>               rcu_read_lock();

Why? Maybe this should be part of the patch 3/3?

And please to merge the refcounting stuff with the rcu pointer in one patch... 
and maybe it is better to merge patch 2/3 with the rcu pointer patch.

Best regards,
        Sven

Attachment: signature.asc
Description: This is a digitally signed message part.

Reply via email to