On Wed, Dec 9, 2020 at 2:51 AM <jma...@redhat.com> wrote:
>
> From: Jon Maloy <jma...@redhat.com>
>
> We instantiante struct publication in tipc_nametbl_insert_publ()
> instead of as currently in tipc_service_insert_publ(). This has the
> advantage that we can pass a pointer to the publication struct to
> the next call levels, instead of the numerous individual parameters
> we pass on now. It also gives us a location to keep the contents of
> the additional fields we will introduce in a later commit.
>
> Signed-off-by: Jon Maloy <jma...@redhat.com>
> ---
> net/tipc/name_table.c | 63 ++++++++++++++++++++++---------------------
> 1 file changed, 32 insertions(+), 31 deletions(-)
>
> diff --git a/net/tipc/name_table.c b/net/tipc/name_table.c
> index c37a4a9c87ca..68e269b49780 100644
> --- a/net/tipc/name_table.c
> +++ b/net/tipc/name_table.c
> @@ -327,49 +327,44 @@ static struct service_range
> *tipc_service_create_range(struct tipc_service *sc,
> return sr;
> }
>
> -static struct publication *tipc_service_insert_publ(struct net *net,
> - struct tipc_service *sc,
> - u32 type, u32 lower,
> - u32 upper, u32 scope,
> - u32 node, u32 port,
> - u32 key)
> +static bool tipc_service_insert_publ(struct net *net,
> + struct tipc_service *sc,
> + struct publication *p)
> {
> struct tipc_subscription *sub, *tmp;
> struct service_range *sr;
> - struct publication *p;
> + struct publication *_p;
> + u32 node = p->sk.node;
> bool first = false;
>
> - sr = tipc_service_create_range(sc, lower, upper);
> + sr = tipc_service_create_range(sc, p->sr.lower, p->sr.upper);
> if (!sr)
> goto err;
>
> first = list_empty(&sr->all_publ);
>
> /* Return if the publication already exists */
> - list_for_each_entry(p, &sr->all_publ, all_publ) {
> - if (p->key == key && (!p->sk.node || p->sk.node == node))
> - return NULL;
> + list_for_each_entry(_p, &sr->all_publ, all_publ) {
> + if (_p->key == p->key && (!_p->sk.node || _p->sk.node ==
> node))
> + return false;
> }
>
> - /* Create and insert publication */
> - p = tipc_publ_create(type, lower, upper, scope, node, port, key);
> - if (!p)
> - goto err;
> - /* Suppose there shouldn't be a huge gap btw publs i.e. >INT_MAX */
> - p->id = sc->publ_cnt++;
> - if (in_own_node(net, node))
> + if (in_own_node(net, p->sk.node))
> list_add(&p->local_publ, &sr->local_publ);
> list_add(&p->all_publ, &sr->all_publ);
> + p->id = sc->publ_cnt++;
>
> /* 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->sr.lower, p->sr.upper,
> + TIPC_PUBLISHED, p->sk.ref, p->sk.node,
> + p->scope, first);
> }
> - return p;
> + return true;
> err:
> - pr_warn("Failed to bind to %u,%u,%u, no memory\n", type, lower,
> upper);
> - return NULL;
> + pr_warn("Failed to bind to %u,%u,%u, no memory\n",
> + p->sr.type, p->sr.lower, p->sr.upper);
> + return false;
> }
>
> /**
> @@ -481,6 +476,11 @@ struct publication *tipc_nametbl_insert_publ(struct net
> *net, u32 type,
> struct name_table *nt = tipc_name_table(net);
> struct tipc_service *sc;
> struct publication *p;
> + bool res = false;
> +
> + p = tipc_publ_create(type, lower, upper, scope, node, port, key);
> + if (!p)
> + return NULL;
>
> if (scope > TIPC_NODE_SCOPE || lower > upper) {
> pr_debug("Failed to bind illegal {%u,%u,%u} with scope %u\n",
> @@ -490,14 +490,15 @@ struct publication *tipc_nametbl_insert_publ(struct net
> *net, u32 type,
> sc = tipc_service_find(net, type);
> if (!sc)
> sc = tipc_service_create(type, &nt->services[hash(type)]);
> - if (!sc)
> - return NULL;
> -
> - spin_lock_bh(&sc->lock);
> - p = tipc_service_insert_publ(net, sc, type, lower, upper,
> - scope, node, port, key);
> - spin_unlock_bh(&sc->lock);
> - return p;
> + if (sc) {
> + spin_lock_bh(&sc->lock);
> + res = tipc_service_insert_publ(net, sc, p);
> + spin_unlock_bh(&sc->lock);
If move spin_(un)lock_bh into tipc_service_insert_publ(), we can do:
if (sc && tipc_service_insert_publ(net, sc, p))
return p;
> + }
> + if (res)
> + return p;
and remove 'bool res = false;' and this 'if'.
> + kfree(p);
> + return NULL;
> }
>
> struct publication *tipc_nametbl_remove_publ(struct net *net, u32 type,
> --
> 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
