On Wed, Dec 9, 2020 at 2:51 AM <jma...@redhat.com> wrote:
>
> From: Jon Maloy <jma...@redhat.com>
>
> The function tipc_report_overlap() is called from the binding table
> with numerous parameters taken from an instance of struct publication.
> A closer look reveals that it always is safe to send along a pointer
> to the instance itself, and hence reduce the call signature. We do
> that in this commit.
>
> Signed-off-by: Jon Maloy <jma...@redhat.com>
> ---
>  net/tipc/name_table.c | 20 ++++++---------
>  net/tipc/subscr.c     | 57 +++++++++++++++++++++++--------------------
>  net/tipc/subscr.h     |  9 +++----
>  3 files changed, 41 insertions(+), 45 deletions(-)
>
> diff --git a/net/tipc/name_table.c b/net/tipc/name_table.c
> index 554a006d7c0d..4dfac3b9d26c 100644
> --- a/net/tipc/name_table.c
> +++ b/net/tipc/name_table.c
> @@ -357,9 +357,7 @@ static bool tipc_service_insert_publ(struct net *net,
>
>         /* Any subscriptions waiting for notification?  */
>         list_for_each_entry_safe(sub, tmp, &sc->subscriptions, service_list) {
> -               tipc_sub_report_overlap(sub, p->sr.lower, p->sr.upper,
> -                                       TIPC_PUBLISHED, p->sk.ref, p->sk.node,
> -                                       p->scope, first);
> +               tipc_sub_report_overlap(sub, p, TIPC_PUBLISHED, first);
>         }
>         return true;
>  err:
> @@ -451,9 +449,7 @@ static void tipc_service_subscribe(struct tipc_service 
> *service,
>         /* Sort the publications before reporting */
>         list_sort(NULL, &publ_list, tipc_publ_sort);
>         list_for_each_entry_safe(p, tmp, &publ_list, list) {
> -               tipc_sub_report_overlap(sub, p->sr.lower, p->sr.upper,
> -                                       TIPC_PUBLISHED, p->sk.ref, p->sk.node,
> -                                       p->scope, true);
> +               tipc_sub_report_overlap(sub, p, TIPC_PUBLISHED, true);
>                 list_del_init(&p->list);
>         }
>  }
> @@ -532,9 +528,7 @@ struct publication *tipc_nametbl_remove_publ(struct net 
> *net,
>         /* Notify any waiting subscriptions */
>         last = list_empty(&sr->all_publ);
>         list_for_each_entry_safe(sub, tmp, &sc->subscriptions, service_list) {
> -               tipc_sub_report_overlap(sub, ua->sr.lower, ua->sr.upper,
> -                                       TIPC_WITHDRAWN, sk->ref, sk->node,
> -                                       ua->scope, last);
> +               tipc_sub_report_overlap(sub, p, TIPC_WITHDRAWN, last);
>         }
>
>         /* Remove service range item if this was its last publication */
> @@ -543,7 +537,7 @@ struct publication *tipc_nametbl_remove_publ(struct net 
> *net,
>                 kfree(sr);
>         }
>
> -       /* Delete service item if this no more publications and subscriptions 
> */
> +       /* Delete service item if no more publications and subscriptions */
>         if (RB_EMPTY_ROOT(&sc->ranges) && list_empty(&sc->subscriptions)) {
>                 hlist_del_init_rcu(&sc->service_list);
>                 kfree_rcu(sc, rcu);
> @@ -842,7 +836,8 @@ bool tipc_nametbl_subscribe(struct tipc_subscription *sub)
>         struct tipc_uaddr ua;
>         bool res = true;
>
> -       tipc_uaddr(&ua, TIPC_SERVICE_RANGE, TIPC_NODE_SCOPE, type, 0, 0);
> +       tipc_uaddr(&ua, TIPC_SERVICE_RANGE, TIPC_NODE_SCOPE, type,
> +                  tipc_sub_read(s, seq.lower), tipc_sub_read(s, seq.upper));
>         spin_lock_bh(&tn->nametbl_lock);
>         sc = tipc_service_find(sub->net, &ua);
>         if (!sc)
> @@ -873,7 +868,8 @@ void tipc_nametbl_unsubscribe(struct tipc_subscription 
> *sub)
>         struct tipc_service *sc;
>         struct tipc_uaddr ua;
>
> -       tipc_uaddr(&ua, TIPC_SERVICE_RANGE, TIPC_NODE_SCOPE, type, 0, 0);
> +       tipc_uaddr(&ua, TIPC_SERVICE_RANGE, TIPC_NODE_SCOPE, type,
> +                  tipc_sub_read(s, seq.lower), tipc_sub_read(s, seq.upper));
>         spin_lock_bh(&tn->nametbl_lock);
>         sc = tipc_service_find(sub->net, &ua);
>         if (!sc)
> diff --git a/net/tipc/subscr.c b/net/tipc/subscr.c
> index f6ad0005218c..feda0b6bbf1b 100644
> --- a/net/tipc/subscr.c
> +++ b/net/tipc/subscr.c
> @@ -40,18 +40,26 @@
>  #include "subscr.h"
>
>  static void tipc_sub_send_event(struct tipc_subscription *sub,
> -                               u32 found_lower, u32 found_upper,
> -                               u32 event, u32 port, u32 node)
> +                               struct publication *p,
> +                               u32 event)
>  {
> +       struct tipc_subscr *s = &sub->evt.s;
>         struct tipc_event *evt = &sub->evt;
>
>         if (sub->inactive)
>                 return;
>         tipc_evt_write(evt, event, event);
> -       tipc_evt_write(evt, found_lower, found_lower);
> -       tipc_evt_write(evt, found_upper, found_upper);
> -       tipc_evt_write(evt, port.ref, port);
> -       tipc_evt_write(evt, port.node, node);
> +       if (p) {
> +               tipc_evt_write(evt, found_lower, p->sr.lower);
> +               tipc_evt_write(evt, found_upper, p->sr.upper);
> +               tipc_evt_write(evt, port.ref, p->sk.ref);
> +               tipc_evt_write(evt, port.node, p->sk.node);
> +       } else {
> +               tipc_evt_write(evt, found_lower, s->seq.lower);
> +               tipc_evt_write(evt, found_upper, s->seq.upper);
> +               tipc_evt_write(evt, port.ref, 0);
> +               tipc_evt_write(evt, port.node, 0);
> +       }
>         tipc_topsrv_queue_evt(sub->net, sub->conid, event, evt);
>  }
>
> @@ -61,24 +69,23 @@ static void tipc_sub_send_event(struct tipc_subscription 
> *sub,
>   * @found_lower: lower value to test
>   * @found_upper: upper value to test
>   *
> - * Return: 1 if there is overlap, otherwise 0.
> + * Returns true if there is overlap, otherwise false.
>   */
> -int tipc_sub_check_overlap(struct tipc_service_range *seq, u32 found_lower,
> -                          u32 found_upper)
> +bool tipc_sub_check_overlap(struct tipc_service_range *sr,
> +                           u32 found_lower, u32 found_upper)
This function should be changed to 'static' after its declaration is
deleted below.

>  {
> -       if (found_lower < seq->lower)
> -               found_lower = seq->lower;
> -       if (found_upper > seq->upper)
> -               found_upper = seq->upper;
> +       if (found_lower < sr->lower)
> +               found_lower = sr->lower;
> +       if (found_upper > sr->upper)
> +               found_upper = sr->upper;
>         if (found_lower > found_upper)
> -               return 0;
> -       return 1;
> +               return false;
> +       return true;
>  }
>
>  void tipc_sub_report_overlap(struct tipc_subscription *sub,
> -                            u32 found_lower, u32 found_upper,
> -                            u32 event, u32 port, u32 node,
> -                            u32 scope, int must)
> +                            struct publication *p,
> +                            u32 event, bool must)
>  {
>         struct tipc_subscr *s = &sub->evt.s;
>         u32 filter = tipc_sub_read(s, filter);
> @@ -88,29 +95,25 @@ void tipc_sub_report_overlap(struct tipc_subscription 
> *sub,
>         seq.lower = tipc_sub_read(s, seq.lower);
>         seq.upper = tipc_sub_read(s, seq.upper);
>
> -       if (!tipc_sub_check_overlap(&seq, found_lower, found_upper))
> +       if (!tipc_sub_check_overlap(&seq, p->sr.lower, p->sr.upper))
>                 return;
> -
>         if (!must && !(filter & TIPC_SUB_PORTS))
>                 return;
> -       if (filter & TIPC_SUB_CLUSTER_SCOPE && scope == TIPC_NODE_SCOPE)
> +       if (filter & TIPC_SUB_CLUSTER_SCOPE && p->scope == TIPC_NODE_SCOPE)
>                 return;
> -       if (filter & TIPC_SUB_NODE_SCOPE && scope != TIPC_NODE_SCOPE)
> +       if (filter & TIPC_SUB_NODE_SCOPE && p->scope != TIPC_NODE_SCOPE)
>                 return;
>         spin_lock(&sub->lock);
> -       tipc_sub_send_event(sub, found_lower, found_upper,
> -                           event, port, node);
> +       tipc_sub_send_event(sub, p, event);
>         spin_unlock(&sub->lock);
>  }
>
>  static void tipc_sub_timeout(struct timer_list *t)
>  {
>         struct tipc_subscription *sub = from_timer(sub, t, timer);
> -       struct tipc_subscr *s = &sub->evt.s;
>
>         spin_lock(&sub->lock);
> -       tipc_sub_send_event(sub, s->seq.lower, s->seq.upper,
> -                           TIPC_SUBSCR_TIMEOUT, 0, 0);
> +       tipc_sub_send_event(sub, NULL, TIPC_SUBSCR_TIMEOUT);
>         sub->inactive = true;
>         spin_unlock(&sub->lock);
>  }
> diff --git a/net/tipc/subscr.h b/net/tipc/subscr.h
> index 3ded27391d54..5fce2e13fc1a 100644
> --- a/net/tipc/subscr.h
> +++ b/net/tipc/subscr.h
> @@ -43,6 +43,7 @@
>  #define TIPC_MAX_SUBSCR         65535
>  #define TIPC_MAX_PUBL           65535
>
> +struct publication;
>  struct tipc_subscription;
>  struct tipc_conn;
>
> @@ -74,13 +75,9 @@ struct tipc_subscription *tipc_sub_subscribe(struct net 
> *net,
>                                              struct tipc_subscr *s,
>                                              int conid);
>  void tipc_sub_unsubscribe(struct tipc_subscription *sub);
> -
> -int tipc_sub_check_overlap(struct tipc_service_range *seq,
> -                          u32 found_lower, u32 found_upper);
>  void tipc_sub_report_overlap(struct tipc_subscription *sub,
> -                            u32 found_lower, u32 found_upper,
> -                            u32 event, u32 port, u32 node,
> -                            u32 scope, int must);
> +                            struct publication *p,
> +                            u32 event, bool must);
>
>  int __net_init tipc_topsrv_init_net(struct net *net);
>  void __net_exit tipc_topsrv_exit_net(struct net *net);
> --
> 2.28.0
>
>
>
> _______________________________________________
> tipc-discussion mailing list
> tipc-discussion@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/tipc-discussion


_______________________________________________
tipc-discussion mailing list
tipc-discussion@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tipc-discussion

Reply via email to