| Message ID | 153628137203.8267.14277020278461943610.stgit@noble (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Beginning of multi-rail support for drivers/staging/lustre | expand |
I'm assuming that a future patch will be chaining the NI structure on to the NET structure it belongs to. This patch is just not chaining the NIs on a global NIS list anymore. As such, ni_cptlist is being "repurposed". Reviewed-by: Doug Oucharek <dougso@me.com> Doug On 9/6/18, 5:54 PM, "NeilBrown" <neilb@suse.com> wrote: This isn't used any more. The new comment is odd - this is no net_ni_cpt !! The ni_cptlist linkage is no longer used - should it go too? This is part of 8cbb8cd3e771e7f7e0f99cafc19fad32770dc015 LU-7734 lnet: Multi-Rail local NI split Signed-off-by: NeilBrown <neilb@suse.com> --- .../staging/lustre/include/linux/lnet/lib-types.h | 4 +--- drivers/staging/lustre/lnet/lnet/api-ni.c | 7 ------- 2 files changed, 1 insertion(+), 10 deletions(-) diff --git a/drivers/staging/lustre/include/linux/lnet/lib-types.h b/drivers/staging/lustre/include/linux/lnet/lib-types.h index 6c34ecf22021..dc15fa75a9d2 100644 --- a/drivers/staging/lustre/include/linux/lnet/lib-types.h +++ b/drivers/staging/lustre/include/linux/lnet/lib-types.h @@ -305,7 +305,7 @@ struct lnet_net { struct lnet_ni { /* chain on the lnet_net structure */ struct list_head ni_netlist; - /* chain on ln_nis_cpt */ + /* chain on net_ni_cpt */ struct list_head ni_cptlist; spinlock_t ni_lock; @@ -671,8 +671,6 @@ struct lnet { /* LND instances */ struct list_head ln_nets; - /* NIs bond on specific CPT(s) */ - struct list_head ln_nis_cpt; /* the loopback NI */ struct lnet_ni *ln_loni; /* network zombie list */ diff --git a/drivers/staging/lustre/lnet/lnet/api-ni.c b/drivers/staging/lustre/lnet/lnet/api-ni.c index 546d5101360f..960f235df5e7 100644 --- a/drivers/staging/lustre/lnet/lnet/api-ni.c +++ b/drivers/staging/lustre/lnet/lnet/api-ni.c @@ -538,7 +538,6 @@ lnet_prepare(lnet_pid_t requested_pid) INIT_LIST_HEAD(&the_lnet.ln_test_peers); INIT_LIST_HEAD(&the_lnet.ln_nets); - INIT_LIST_HEAD(&the_lnet.ln_nis_cpt); INIT_LIST_HEAD(&the_lnet.ln_routers); INIT_LIST_HEAD(&the_lnet.ln_drop_rules); INIT_LIST_HEAD(&the_lnet.ln_delay_rules); @@ -616,7 +615,6 @@ lnet_unprepare(void) LASSERT(!the_lnet.ln_refcount); LASSERT(list_empty(&the_lnet.ln_test_peers)); LASSERT(list_empty(&the_lnet.ln_nets)); - LASSERT(list_empty(&the_lnet.ln_nis_cpt)); lnet_portals_destroy(); @@ -1294,11 +1292,6 @@ lnet_startup_lndni(struct lnet_ni *ni, struct lnet_lnd_tunables *tun) /* refcount for ln_nis */ lnet_ni_addref_locked(ni, 0); list_add_tail(&ni->ni_net->net_list, &the_lnet.ln_nets); - if (ni->ni_cpts) { - lnet_ni_addref_locked(ni, 0); - list_add_tail(&ni->ni_cptlist, &the_lnet.ln_nis_cpt); - } - lnet_net_unlock(LNET_LOCK_EX); ni->ni_state = LNET_NI_STATE_ACTIVE;
On Wed, Sep 12 2018, Doug Oucharek wrote: > I'm assuming that a future patch will be chaining the NI structure on to the NET structure it belongs to. This patch is just not chaining the NIs on a global NIS list anymore. As such, ni_cptlist is being "repurposed". The NI is already chained onto the NET through lnet_net.net_ni_list and lnet_ni.ni_netlist ni_cptlist is not used even in current master. It is never added to any list, but lnet_ni_unlink_locked() does remove it from a list. Is that code wrong (should be checking ni_netlist), or is it cruft that should be removed? Thanks, NeilBrown > > Reviewed-by: Doug Oucharek <dougso@me.com> > > Doug > > On 9/6/18, 5:54 PM, "NeilBrown" <neilb@suse.com> wrote: > > This isn't used any more. > The new comment is odd - this is no net_ni_cpt !! > The ni_cptlist linkage is no longer used - should it go too? > > This is part of > 8cbb8cd3e771e7f7e0f99cafc19fad32770dc015 > LU-7734 lnet: Multi-Rail local NI split > > Signed-off-by: NeilBrown <neilb@suse.com> > --- > .../staging/lustre/include/linux/lnet/lib-types.h | 4 +--- > drivers/staging/lustre/lnet/lnet/api-ni.c | 7 ------- > 2 files changed, 1 insertion(+), 10 deletions(-) > > diff --git a/drivers/staging/lustre/include/linux/lnet/lib-types.h b/drivers/staging/lustre/include/linux/lnet/lib-types.h > index 6c34ecf22021..dc15fa75a9d2 100644 > --- a/drivers/staging/lustre/include/linux/lnet/lib-types.h > +++ b/drivers/staging/lustre/include/linux/lnet/lib-types.h > @@ -305,7 +305,7 @@ struct lnet_net { > struct lnet_ni { > /* chain on the lnet_net structure */ > struct list_head ni_netlist; > - /* chain on ln_nis_cpt */ > + /* chain on net_ni_cpt */ > struct list_head ni_cptlist; > > spinlock_t ni_lock; > @@ -671,8 +671,6 @@ struct lnet { > > /* LND instances */ > struct list_head ln_nets; > - /* NIs bond on specific CPT(s) */ > - struct list_head ln_nis_cpt; > /* the loopback NI */ > struct lnet_ni *ln_loni; > /* network zombie list */ > diff --git a/drivers/staging/lustre/lnet/lnet/api-ni.c b/drivers/staging/lustre/lnet/lnet/api-ni.c > index 546d5101360f..960f235df5e7 100644 > --- a/drivers/staging/lustre/lnet/lnet/api-ni.c > +++ b/drivers/staging/lustre/lnet/lnet/api-ni.c > @@ -538,7 +538,6 @@ lnet_prepare(lnet_pid_t requested_pid) > > INIT_LIST_HEAD(&the_lnet.ln_test_peers); > INIT_LIST_HEAD(&the_lnet.ln_nets); > - INIT_LIST_HEAD(&the_lnet.ln_nis_cpt); > INIT_LIST_HEAD(&the_lnet.ln_routers); > INIT_LIST_HEAD(&the_lnet.ln_drop_rules); > INIT_LIST_HEAD(&the_lnet.ln_delay_rules); > @@ -616,7 +615,6 @@ lnet_unprepare(void) > LASSERT(!the_lnet.ln_refcount); > LASSERT(list_empty(&the_lnet.ln_test_peers)); > LASSERT(list_empty(&the_lnet.ln_nets)); > - LASSERT(list_empty(&the_lnet.ln_nis_cpt)); > > lnet_portals_destroy(); > > @@ -1294,11 +1292,6 @@ lnet_startup_lndni(struct lnet_ni *ni, struct lnet_lnd_tunables *tun) > /* refcount for ln_nis */ > lnet_ni_addref_locked(ni, 0); > list_add_tail(&ni->ni_net->net_list, &the_lnet.ln_nets); > - if (ni->ni_cpts) { > - lnet_ni_addref_locked(ni, 0); > - list_add_tail(&ni->ni_cptlist, &the_lnet.ln_nis_cpt); > - } > - > lnet_net_unlock(LNET_LOCK_EX); > > ni->ni_state = LNET_NI_STATE_ACTIVE; > > >
This area was re-implemented. There is no need for ni_cptlist any longer. I looked at the current code and it's not being used. Originally it was being used to place the the ni on a global list: ln_nis_cpt. Which was traversed when attempting to calculate the cpt for a NID using lnet_cpt_of_nid_locked(). However that latter function has been re-implemented due to how MR works now. So there is not need for ni_cptlist. On Thu, 6 Sep 2018 at 18:05, NeilBrown <neilb@suse.com> wrote: > This isn't used any more. > The new comment is odd - this is no net_ni_cpt !! > The ni_cptlist linkage is no longer used - should it go too? > > This is part of > 8cbb8cd3e771e7f7e0f99cafc19fad32770dc015 > LU-7734 lnet: Multi-Rail local NI split > > Signed-off-by: NeilBrown <neilb@suse.com> > --- > .../staging/lustre/include/linux/lnet/lib-types.h | 4 +--- > drivers/staging/lustre/lnet/lnet/api-ni.c | 7 ------- > 2 files changed, 1 insertion(+), 10 deletions(-) > > diff --git a/drivers/staging/lustre/include/linux/lnet/lib-types.h > b/drivers/staging/lustre/include/linux/lnet/lib-types.h > index 6c34ecf22021..dc15fa75a9d2 100644 > --- a/drivers/staging/lustre/include/linux/lnet/lib-types.h > +++ b/drivers/staging/lustre/include/linux/lnet/lib-types.h > @@ -305,7 +305,7 @@ struct lnet_net { > struct lnet_ni { > /* chain on the lnet_net structure */ > struct list_head ni_netlist; > - /* chain on ln_nis_cpt */ > + /* chain on net_ni_cpt */ > struct list_head ni_cptlist; > > spinlock_t ni_lock; > @@ -671,8 +671,6 @@ struct lnet { > > /* LND instances */ > struct list_head ln_nets; > - /* NIs bond on specific CPT(s) */ > - struct list_head ln_nis_cpt; > /* the loopback NI */ > struct lnet_ni *ln_loni; > /* network zombie list */ > diff --git a/drivers/staging/lustre/lnet/lnet/api-ni.c > b/drivers/staging/lustre/lnet/lnet/api-ni.c > index 546d5101360f..960f235df5e7 100644 > --- a/drivers/staging/lustre/lnet/lnet/api-ni.c > +++ b/drivers/staging/lustre/lnet/lnet/api-ni.c > @@ -538,7 +538,6 @@ lnet_prepare(lnet_pid_t requested_pid) > > INIT_LIST_HEAD(&the_lnet.ln_test_peers); > INIT_LIST_HEAD(&the_lnet.ln_nets); > - INIT_LIST_HEAD(&the_lnet.ln_nis_cpt); > INIT_LIST_HEAD(&the_lnet.ln_routers); > INIT_LIST_HEAD(&the_lnet.ln_drop_rules); > INIT_LIST_HEAD(&the_lnet.ln_delay_rules); > @@ -616,7 +615,6 @@ lnet_unprepare(void) > LASSERT(!the_lnet.ln_refcount); > LASSERT(list_empty(&the_lnet.ln_test_peers)); > LASSERT(list_empty(&the_lnet.ln_nets)); > - LASSERT(list_empty(&the_lnet.ln_nis_cpt)); > > lnet_portals_destroy(); > > @@ -1294,11 +1292,6 @@ lnet_startup_lndni(struct lnet_ni *ni, struct > lnet_lnd_tunables *tun) > /* refcount for ln_nis */ > lnet_ni_addref_locked(ni, 0); > list_add_tail(&ni->ni_net->net_list, &the_lnet.ln_nets); > - if (ni->ni_cpts) { > - lnet_ni_addref_locked(ni, 0); > - list_add_tail(&ni->ni_cptlist, &the_lnet.ln_nis_cpt); > - } > - > lnet_net_unlock(LNET_LOCK_EX); > > ni->ni_state = LNET_NI_STATE_ACTIVE; > > > _______________________________________________ > lustre-devel mailing list > lustre-devel@lists.lustre.org > http://lists.lustre.org/listinfo.cgi/lustre-devel-lustre.org > <div dir="ltr"><div>This area was re-implemented. There is no need for ni_cptlist any longer. I looked at the current code and it's not being used.</div><div>Originally it was being used to place the the ni on a global list: ln_nis_cpt. Which was traversed when attempting to calculate the cpt for a NID using lnet_cpt_of_nid_locked(). However that latter function has been re-implemented due to how MR works now. So there is not need for ni_cptlist.<br></div></div><br><div class="gmail_quote"><div dir="ltr">On Thu, 6 Sep 2018 at 18:05, NeilBrown <<a href="mailto:neilb@suse.com">neilb@suse.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">This isn't used any more.<br> The new comment is odd - this is no net_ni_cpt !!<br> The ni_cptlist linkage is no longer used - should it go too?<br> <br> This is part of<br> 8cbb8cd3e771e7f7e0f99cafc19fad32770dc015<br> LU-7734 lnet: Multi-Rail local NI split<br> <br> Signed-off-by: NeilBrown <<a href="mailto:neilb@suse.com" target="_blank">neilb@suse.com</a>><br> ---<br> .../staging/lustre/include/linux/lnet/lib-types.h | 4 +---<br> drivers/staging/lustre/lnet/lnet/api-ni.c | 7 -------<br> 2 files changed, 1 insertion(+), 10 deletions(-)<br> <br> diff --git a/drivers/staging/lustre/include/linux/lnet/lib-types.h b/drivers/staging/lustre/include/linux/lnet/lib-types.h<br> index 6c34ecf22021..dc15fa75a9d2 100644<br> --- a/drivers/staging/lustre/include/linux/lnet/lib-types.h<br> +++ b/drivers/staging/lustre/include/linux/lnet/lib-types.h<br> @@ -305,7 +305,7 @@ struct lnet_net {<br> struct lnet_ni {<br> /* chain on the lnet_net structure */<br> struct list_head ni_netlist;<br> - /* chain on ln_nis_cpt */<br> + /* chain on net_ni_cpt */<br> struct list_head ni_cptlist;<br> <br> spinlock_t ni_lock;<br> @@ -671,8 +671,6 @@ struct lnet {<br> <br> /* LND instances */<br> struct list_head ln_nets;<br> - /* NIs bond on specific CPT(s) */<br> - struct list_head ln_nis_cpt;<br> /* the loopback NI */<br> struct lnet_ni *ln_loni;<br> /* network zombie list */<br> diff --git a/drivers/staging/lustre/lnet/lnet/api-ni.c b/drivers/staging/lustre/lnet/lnet/api-ni.c<br> index 546d5101360f..960f235df5e7 100644<br> --- a/drivers/staging/lustre/lnet/lnet/api-ni.c<br> +++ b/drivers/staging/lustre/lnet/lnet/api-ni.c<br> @@ -538,7 +538,6 @@ lnet_prepare(lnet_pid_t requested_pid)<br> <br> INIT_LIST_HEAD(&the_lnet.ln_test_peers);<br> INIT_LIST_HEAD(&the_lnet.ln_nets);<br> - INIT_LIST_HEAD(&the_lnet.ln_nis_cpt);<br> INIT_LIST_HEAD(&the_lnet.ln_routers);<br> INIT_LIST_HEAD(&the_lnet.ln_drop_rules);<br> INIT_LIST_HEAD(&the_lnet.ln_delay_rules);<br> @@ -616,7 +615,6 @@ lnet_unprepare(void)<br> LASSERT(!the_lnet.ln_refcount);<br> LASSERT(list_empty(&the_lnet.ln_test_peers));<br> LASSERT(list_empty(&the_lnet.ln_nets));<br> - LASSERT(list_empty(&the_lnet.ln_nis_cpt));<br> <br> lnet_portals_destroy();<br> <br> @@ -1294,11 +1292,6 @@ lnet_startup_lndni(struct lnet_ni *ni, struct lnet_lnd_tunables *tun)<br> /* refcount for ln_nis */<br> lnet_ni_addref_locked(ni, 0);<br> list_add_tail(&ni->ni_net->net_list, &the_lnet.ln_nets);<br> - if (ni->ni_cpts) {<br> - lnet_ni_addref_locked(ni, 0);<br> - list_add_tail(&ni->ni_cptlist, &the_lnet.ln_nis_cpt);<br> - }<br> -<br> lnet_net_unlock(LNET_LOCK_EX);<br> <br> ni->ni_state = LNET_NI_STATE_ACTIVE;<br> <br> <br> _______________________________________________<br> lustre-devel mailing list<br> <a href="mailto:lustre-devel@lists.lustre.org" target="_blank">lustre-devel@lists.lustre.org</a><br> <a href="http://lists.lustre.org/listinfo.cgi/lustre-devel-lustre.org" rel="noreferrer" target="_blank">http://lists.lustre.org/listinfo.cgi/lustre-devel-lustre.org</a><br> </blockquote></div>
did you read my response to that question? Pasted below: --- This area was re-implemented. There is no need for ni_cptlist any longer. I looked at the current code and it's not being used. Originally it was being used to place the the ni on a global list: ln_nis_cpt. Which was traversed when attempting to calculate the cpt for a NID using lnet_cpt_of_nid_locked(). However that latter function has been re-implemented due to how MR works now. So there is not need for ni_cptlist.
On Thu, Sep 13 2018, Amir Shehata wrote: > did you read my response to that question? Pasted below: > > --- > This area was re-implemented. There is no need for ni_cptlist any longer. I > looked at the current code and it's not being used. > Originally it was being used to place the the ni on a global list: > ln_nis_cpt. Which was traversed when attempting to calculate the cpt for a > NID using lnet_cpt_of_nid_locked(). However that latter function has been > re-implemented due to how MR works now. So there is not need for ni_cptlist. > ___ Thanks. I've queued this patch. NeilBrown From: NeilBrown <neilb@suse.com> Date: Mon, 24 Sep 2018 15:57:13 +1000 Subject: [PATCH] lustre: remove ni_cptlist field. This is never used in a meaningful way. Signed-off-by: NeilBrown <neilb@suse.com> --- drivers/staging/lustre/include/linux/lnet/lib-types.h | 2 -- drivers/staging/lustre/lnet/lnet/api-ni.c | 5 ----- drivers/staging/lustre/lnet/lnet/config.c | 1 - 3 files changed, 8 deletions(-) diff --git a/drivers/staging/lustre/include/linux/lnet/lib-types.h b/drivers/staging/lustre/include/linux/lnet/lib-types.h index 89158b52dc1b..1423aea83747 100644 --- a/drivers/staging/lustre/include/linux/lnet/lib-types.h +++ b/drivers/staging/lustre/include/linux/lnet/lib-types.h @@ -309,8 +309,6 @@ struct lnet_ni { spinlock_t ni_lock; /* chain on the lnet_net structure */ struct list_head ni_netlist; - /* chain on net_ni_cpt */ - struct list_head ni_cptlist; /* number of CPTs */ int ni_ncpts; diff --git a/drivers/staging/lustre/lnet/lnet/api-ni.c b/drivers/staging/lustre/lnet/lnet/api-ni.c index 69ea18ce2dcc..20fa3fea04b9 100644 --- a/drivers/staging/lustre/lnet/lnet/api-ni.c +++ b/drivers/staging/lustre/lnet/lnet/api-ni.c @@ -1115,11 +1115,6 @@ lnet_ni_tq_credits(struct lnet_ni *ni) static void lnet_ni_unlink_locked(struct lnet_ni *ni) { - if (!list_empty(&ni->ni_cptlist)) { - list_del_init(&ni->ni_cptlist); - lnet_ni_decref_locked(ni, 0); - } - /* move it to zombie list and nobody can find it anymore */ LASSERT(!list_empty(&ni->ni_netlist)); list_move(&ni->ni_netlist, &ni->ni_net->net_ni_zombie); diff --git a/drivers/staging/lustre/lnet/lnet/config.c b/drivers/staging/lustre/lnet/lnet/config.c index 92694b51f223..9539ce07ae05 100644 --- a/drivers/staging/lustre/lnet/lnet/config.c +++ b/drivers/staging/lustre/lnet/lnet/config.c @@ -438,7 +438,6 @@ lnet_ni_alloc(struct lnet_net *net, struct cfs_expr_list *el, char *iface) } spin_lock_init(&ni->ni_lock); - INIT_LIST_HEAD(&ni->ni_cptlist); INIT_LIST_HEAD(&ni->ni_netlist); ni->ni_refs = cfs_percpt_alloc(lnet_cpt_table(), sizeof(*ni->ni_refs[0]));
diff --git a/drivers/staging/lustre/include/linux/lnet/lib-types.h b/drivers/staging/lustre/include/linux/lnet/lib-types.h index 6c34ecf22021..dc15fa75a9d2 100644 --- a/drivers/staging/lustre/include/linux/lnet/lib-types.h +++ b/drivers/staging/lustre/include/linux/lnet/lib-types.h @@ -305,7 +305,7 @@ struct lnet_net { struct lnet_ni { /* chain on the lnet_net structure */ struct list_head ni_netlist; - /* chain on ln_nis_cpt */ + /* chain on net_ni_cpt */ struct list_head ni_cptlist; spinlock_t ni_lock; @@ -671,8 +671,6 @@ struct lnet { /* LND instances */ struct list_head ln_nets; - /* NIs bond on specific CPT(s) */ - struct list_head ln_nis_cpt; /* the loopback NI */ struct lnet_ni *ln_loni; /* network zombie list */ diff --git a/drivers/staging/lustre/lnet/lnet/api-ni.c b/drivers/staging/lustre/lnet/lnet/api-ni.c index 546d5101360f..960f235df5e7 100644 --- a/drivers/staging/lustre/lnet/lnet/api-ni.c +++ b/drivers/staging/lustre/lnet/lnet/api-ni.c @@ -538,7 +538,6 @@ lnet_prepare(lnet_pid_t requested_pid) INIT_LIST_HEAD(&the_lnet.ln_test_peers); INIT_LIST_HEAD(&the_lnet.ln_nets); - INIT_LIST_HEAD(&the_lnet.ln_nis_cpt); INIT_LIST_HEAD(&the_lnet.ln_routers); INIT_LIST_HEAD(&the_lnet.ln_drop_rules); INIT_LIST_HEAD(&the_lnet.ln_delay_rules); @@ -616,7 +615,6 @@ lnet_unprepare(void) LASSERT(!the_lnet.ln_refcount); LASSERT(list_empty(&the_lnet.ln_test_peers)); LASSERT(list_empty(&the_lnet.ln_nets)); - LASSERT(list_empty(&the_lnet.ln_nis_cpt)); lnet_portals_destroy(); @@ -1294,11 +1292,6 @@ lnet_startup_lndni(struct lnet_ni *ni, struct lnet_lnd_tunables *tun) /* refcount for ln_nis */ lnet_ni_addref_locked(ni, 0); list_add_tail(&ni->ni_net->net_list, &the_lnet.ln_nets); - if (ni->ni_cpts) { - lnet_ni_addref_locked(ni, 0); - list_add_tail(&ni->ni_cptlist, &the_lnet.ln_nis_cpt); - } - lnet_net_unlock(LNET_LOCK_EX); ni->ni_state = LNET_NI_STATE_ACTIVE;
This isn't used any more. The new comment is odd - this is no net_ni_cpt !! The ni_cptlist linkage is no longer used - should it go too? This is part of 8cbb8cd3e771e7f7e0f99cafc19fad32770dc015 LU-7734 lnet: Multi-Rail local NI split Signed-off-by: NeilBrown <neilb@suse.com> --- .../staging/lustre/include/linux/lnet/lib-types.h | 4 +--- drivers/staging/lustre/lnet/lnet/api-ni.c | 7 ------- 2 files changed, 1 insertion(+), 10 deletions(-)