* [git patch review 1/6] IPoIB: Take dev->xmit_lock around mc_list accesses @ 2006-01-13 0:13 Roland Dreier 2006-01-13 0:13 ` [git patch review 2/6] IPoIB: Fix memory leak of multicast group structures Roland Dreier 0 siblings, 1 reply; 6+ messages in thread From: Roland Dreier @ 2006-01-13 0:13 UTC (permalink / raw) To: linux-kernel, openib-general dev->mc_list accesses must be protected by dev->xmit_lock. Found by Eli Cohen <eli@mellanox.co.il>. Signed-off-by: Michael S. Tsirkin <mst@mellanox.co.il> Signed-off-by: Roland Dreier <rolandd@cisco.com> --- drivers/infiniband/ulp/ipoib/ipoib_multicast.c | 7 +++++-- 1 files changed, 5 insertions(+), 2 deletions(-) 78bfe0b5b67fe126ed98608e42e42fb6ed9aabd4 diff --git a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c index 03b2ca6..bf1c08c 100644 --- a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c +++ b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c @@ -827,7 +827,8 @@ void ipoib_mcast_restart_task(void *dev_ ipoib_mcast_stop_thread(dev, 0); - spin_lock_irqsave(&priv->lock, flags); + spin_lock_irqsave(&dev->xmit_lock, flags); + spin_lock(&priv->lock); /* * Unfortunately, the networking core only gives us a list of all of @@ -899,7 +900,9 @@ void ipoib_mcast_restart_task(void *dev_ list_add_tail(&mcast->list, &remove_list); } } - spin_unlock_irqrestore(&priv->lock, flags); + + spin_unlock(&priv->lock); + spin_unlock_irqrestore(&dev->xmit_lock, flags); /* We have to cancel outside of the spinlock */ list_for_each_entry_safe(mcast, tmcast, &remove_list, list) { -- 1.0.7 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [git patch review 2/6] IPoIB: Fix memory leak of multicast group structures 2006-01-13 0:13 [git patch review 1/6] IPoIB: Take dev->xmit_lock around mc_list accesses Roland Dreier @ 2006-01-13 0:13 ` Roland Dreier 2006-01-13 0:13 ` [git patch review 3/6] IB/mthca: " Roland Dreier 0 siblings, 1 reply; 6+ messages in thread From: Roland Dreier @ 2006-01-13 0:13 UTC (permalink / raw) To: linux-kernel, openib-general The current handling of multicast groups in IPoIB ends up never freeing send-only multicast groups. It turns out the logic was much more complicated than it needed to be; we can fix this bug and completely kill ipoib_mcast_dev_down() at the same time. Signed-off-by: Eli Cohen <eli@mellanox.co.il> Signed-off-by: Michael S. Tsirkin <mst@mellanox.co.il> Signed-off-by: Roland Dreier <rolandd@cisco.com> --- drivers/infiniband/ulp/ipoib/ipoib_ib.c | 13 ----- drivers/infiniband/ulp/ipoib/ipoib_multicast.c | 61 +++--------------------- 2 files changed, 9 insertions(+), 65 deletions(-) 988bd50300ef2e2d5cb8563e2ac99453dd9acd86 diff --git a/drivers/infiniband/ulp/ipoib/ipoib_ib.c b/drivers/infiniband/ulp/ipoib/ipoib_ib.c index 2388580..f7e8489 100644 --- a/drivers/infiniband/ulp/ipoib/ipoib_ib.c +++ b/drivers/infiniband/ulp/ipoib/ipoib_ib.c @@ -453,17 +453,8 @@ int ipoib_ib_dev_down(struct net_device } ipoib_mcast_stop_thread(dev, 1); - - /* - * Flush the multicast groups first so we stop any multicast joins. The - * completion thread may have already died and we may deadlock waiting - * for the completion thread to finish some multicast joins. - */ ipoib_mcast_dev_flush(dev); - /* Delete broadcast and local addresses since they will be recreated */ - ipoib_mcast_dev_down(dev); - ipoib_flush_paths(dev); return 0; @@ -624,9 +615,7 @@ void ipoib_ib_dev_cleanup(struct net_dev ipoib_dbg(priv, "cleaning up ib_dev\n"); ipoib_mcast_stop_thread(dev, 1); - - /* Delete the broadcast address and the local address */ - ipoib_mcast_dev_down(dev); + ipoib_mcast_dev_flush(dev); ipoib_transport_dev_cleanup(dev); } diff --git a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c index bf1c08c..7403bac 100644 --- a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c +++ b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c @@ -742,50 +742,23 @@ void ipoib_mcast_dev_flush(struct net_de { struct ipoib_dev_priv *priv = netdev_priv(dev); LIST_HEAD(remove_list); - struct ipoib_mcast *mcast, *tmcast, *nmcast; + struct ipoib_mcast *mcast, *tmcast; unsigned long flags; ipoib_dbg_mcast(priv, "flushing multicast list\n"); spin_lock_irqsave(&priv->lock, flags); - list_for_each_entry_safe(mcast, tmcast, &priv->multicast_list, list) { - nmcast = ipoib_mcast_alloc(dev, 0); - if (nmcast) { - nmcast->flags = - mcast->flags & (1 << IPOIB_MCAST_FLAG_SENDONLY); - - nmcast->mcmember.mgid = mcast->mcmember.mgid; - - /* Add the new group in before the to-be-destroyed group */ - list_add_tail(&nmcast->list, &mcast->list); - list_del_init(&mcast->list); - - rb_replace_node(&mcast->rb_node, &nmcast->rb_node, - &priv->multicast_tree); - list_add_tail(&mcast->list, &remove_list); - } else { - ipoib_warn(priv, "could not reallocate multicast group " - IPOIB_GID_FMT "\n", - IPOIB_GID_ARG(mcast->mcmember.mgid)); - } + list_for_each_entry_safe(mcast, tmcast, &priv->multicast_list, list) { + list_del(&mcast->list); + rb_erase(&mcast->rb_node, &priv->multicast_tree); + list_add_tail(&mcast->list, &remove_list); } if (priv->broadcast) { - nmcast = ipoib_mcast_alloc(dev, 0); - if (nmcast) { - nmcast->mcmember.mgid = priv->broadcast->mcmember.mgid; - - rb_replace_node(&priv->broadcast->rb_node, - &nmcast->rb_node, - &priv->multicast_tree); - - list_add_tail(&priv->broadcast->list, &remove_list); - priv->broadcast = nmcast; - } else - ipoib_warn(priv, "could not reallocate broadcast group " - IPOIB_GID_FMT "\n", - IPOIB_GID_ARG(priv->broadcast->mcmember.mgid)); + rb_erase(&priv->broadcast->rb_node, &priv->multicast_tree); + list_add_tail(&priv->broadcast->list, &remove_list); + priv->broadcast = NULL; } spin_unlock_irqrestore(&priv->lock, flags); @@ -796,24 +769,6 @@ void ipoib_mcast_dev_flush(struct net_de } } -void ipoib_mcast_dev_down(struct net_device *dev) -{ - struct ipoib_dev_priv *priv = netdev_priv(dev); - unsigned long flags; - - /* Delete broadcast since it will be recreated */ - if (priv->broadcast) { - ipoib_dbg_mcast(priv, "deleting broadcast group\n"); - - spin_lock_irqsave(&priv->lock, flags); - rb_erase(&priv->broadcast->rb_node, &priv->multicast_tree); - spin_unlock_irqrestore(&priv->lock, flags); - ipoib_mcast_leave(dev, priv->broadcast); - ipoib_mcast_free(priv->broadcast); - priv->broadcast = NULL; - } -} - void ipoib_mcast_restart_task(void *dev_ptr) { struct net_device *dev = dev_ptr; -- 1.0.7 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [git patch review 3/6] IB/mthca: Fix memory leak of multicast group structures 2006-01-13 0:13 ` [git patch review 2/6] IPoIB: Fix memory leak of multicast group structures Roland Dreier @ 2006-01-13 0:13 ` Roland Dreier 2006-01-13 0:13 ` [git patch review 4/6] IB/mthca: Fix memory leaks in error handling Roland Dreier 0 siblings, 1 reply; 6+ messages in thread From: Roland Dreier @ 2006-01-13 0:13 UTC (permalink / raw) To: linux-kernel, openib-general Convert "/ (1 << lg)" to ">> lg" for a slight code size reduction. add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-24 (-24) function old new delta mthca_map_cmd 613 589 -24 Signed-off-by: Ishai Rabinovitz <ishai@mellanox.co.il> Signed-off-by: Michael S. Tsirkin <mst@mellanox.co.il> Signed-off-by: Roland Dreier <rolandd@cisco.com> --- drivers/infiniband/hw/mthca/mthca_cmd.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) 59f174faffd5dfee709fa0ead320cc6daf827e93 diff --git a/drivers/infiniband/hw/mthca/mthca_cmd.c b/drivers/infiniband/hw/mthca/mthca_cmd.c index 22ac72b..f69e489 100644 --- a/drivers/infiniband/hw/mthca/mthca_cmd.c +++ b/drivers/infiniband/hw/mthca/mthca_cmd.c @@ -606,7 +606,7 @@ static int mthca_map_cmd(struct mthca_de err = -EINVAL; goto out; } - for (i = 0; i < mthca_icm_size(&iter) / (1 << lg); ++i) { + for (i = 0; i < mthca_icm_size(&iter) >> lg; ++i) { if (virt != -1) { pages[nent * 2] = cpu_to_be64(virt); virt += 1 << lg; -- 1.0.7 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [git patch review 4/6] IB/mthca: Fix memory leaks in error handling 2006-01-13 0:13 ` [git patch review 3/6] IB/mthca: " Roland Dreier @ 2006-01-13 0:13 ` Roland Dreier 2006-01-13 0:13 ` [git patch review 5/6] IB/mthca: Cosmetic: use the ALIGN macro Roland Dreier 0 siblings, 1 reply; 6+ messages in thread From: Roland Dreier @ 2006-01-13 0:13 UTC (permalink / raw) To: linux-kernel, openib-general Fix memory leaks in mthca_create_qp() and mthca_create_srq() error handling. Signed-off-by: Jack Morgenstein <jackm@mellanox.co.il> Signed-off-by: Roland Dreier <rolandd@cisco.com> --- drivers/infiniband/hw/mthca/mthca_provider.c | 10 +++++++--- 1 files changed, 7 insertions(+), 3 deletions(-) 17e2e819517d75f2f3407e59c5f7f6f0ef305d14 diff --git a/drivers/infiniband/hw/mthca/mthca_provider.c b/drivers/infiniband/hw/mthca/mthca_provider.c index db35690..484a7e6 100644 --- a/drivers/infiniband/hw/mthca/mthca_provider.c +++ b/drivers/infiniband/hw/mthca/mthca_provider.c @@ -445,8 +445,10 @@ static struct ib_srq *mthca_create_srq(s if (pd->uobject) { context = to_mucontext(pd->uobject->context); - if (ib_copy_from_udata(&ucmd, udata, sizeof ucmd)) - return ERR_PTR(-EFAULT); + if (ib_copy_from_udata(&ucmd, udata, sizeof ucmd)) { + err = -EFAULT; + goto err_free; + } err = mthca_map_user_db(to_mdev(pd->device), &context->uar, context->db_tab, ucmd.db_index, @@ -522,8 +524,10 @@ static struct ib_qp *mthca_create_qp(str if (pd->uobject) { context = to_mucontext(pd->uobject->context); - if (ib_copy_from_udata(&ucmd, udata, sizeof ucmd)) + if (ib_copy_from_udata(&ucmd, udata, sizeof ucmd)) { + kfree(qp); return ERR_PTR(-EFAULT); + } err = mthca_map_user_db(to_mdev(pd->device), &context->uar, context->db_tab, -- 1.0.7 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [git patch review 5/6] IB/mthca: Cosmetic: use the ALIGN macro 2006-01-13 0:13 ` [git patch review 4/6] IB/mthca: Fix memory leaks in error handling Roland Dreier @ 2006-01-13 0:13 ` Roland Dreier 2006-01-13 0:13 ` [git patch review 6/6] IB/mthca: Initialize grh_present before using it Roland Dreier 0 siblings, 1 reply; 6+ messages in thread From: Roland Dreier @ 2006-01-13 0:13 UTC (permalink / raw) To: linux-kernel, openib-general Use the ALIGN macro to simplify some rounding code. Signed-off-by: Michael S. Tsirkin <mst@mellanox.co.il> Signed-off-by: Roland Dreier <rolandd@cisco.com> --- drivers/infiniband/hw/mthca/mthca_cmd.c | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-) c063a06835d3ccfa6c039d3a3869fcf22249c862 diff --git a/drivers/infiniband/hw/mthca/mthca_cmd.c b/drivers/infiniband/hw/mthca/mthca_cmd.c index f69e489..be1791b 100644 --- a/drivers/infiniband/hw/mthca/mthca_cmd.c +++ b/drivers/infiniband/hw/mthca/mthca_cmd.c @@ -727,8 +727,8 @@ int mthca_QUERY_FW(struct mthca_dev *dev * system pages needed. */ dev->fw.arbel.fw_pages = - (dev->fw.arbel.fw_pages + (1 << (PAGE_SHIFT - 12)) - 1) >> - (PAGE_SHIFT - 12); + ALIGN(dev->fw.arbel.fw_pages, PAGE_SIZE >> 12) >> + (PAGE_SHIFT - 12); mthca_dbg(dev, "Clear int @ %llx, EQ arm @ %llx, EQ set CI @ %llx\n", (unsigned long long) dev->fw.arbel.clr_int_base, @@ -1445,6 +1445,7 @@ int mthca_SET_ICM_SIZE(struct mthca_dev * pages needed. */ *aux_pages = (*aux_pages + (1 << (PAGE_SHIFT - 12)) - 1) >> (PAGE_SHIFT - 12); + *aux_pages = ALIGN(*aux_pages, PAGE_SIZE >> 12) >> (PAGE_SHIFT - 12); return 0; } -- 1.0.7 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [git patch review 6/6] IB/mthca: Initialize grh_present before using it 2006-01-13 0:13 ` [git patch review 5/6] IB/mthca: Cosmetic: use the ALIGN macro Roland Dreier @ 2006-01-13 0:13 ` Roland Dreier 0 siblings, 0 replies; 6+ messages in thread From: Roland Dreier @ 2006-01-13 0:13 UTC (permalink / raw) To: linux-kernel, openib-general build_mlx_header() was using sqp->ud_header.grh_present before it was initialized by mthca_read_ah(). Furthermore, header->grh_present is set by ib_ud_header_init, so there's no need to set it again in mthca_read_ah(). Signed-off-by: Michael S. Tsirkin <mst@mellanox.co.il> Signed-off-by: Roland Dreier <rolandd@cisco.com> --- drivers/infiniband/hw/mthca/mthca_av.c | 10 ++++++---- drivers/infiniband/hw/mthca/mthca_dev.h | 1 + drivers/infiniband/hw/mthca/mthca_qp.c | 2 +- 3 files changed, 8 insertions(+), 5 deletions(-) 9eacee2ac624bfa9740d49355dbe6ee88d0cba0a diff --git a/drivers/infiniband/hw/mthca/mthca_av.c b/drivers/infiniband/hw/mthca/mthca_av.c index 22fdc44..a14eed0 100644 --- a/drivers/infiniband/hw/mthca/mthca_av.c +++ b/drivers/infiniband/hw/mthca/mthca_av.c @@ -163,6 +163,11 @@ int mthca_destroy_ah(struct mthca_dev *d return 0; } +int mthca_ah_grh_present(struct mthca_ah *ah) +{ + return !!(ah->av->g_slid & 0x80); +} + int mthca_read_ah(struct mthca_dev *dev, struct mthca_ah *ah, struct ib_ud_header *header) { @@ -172,8 +177,7 @@ int mthca_read_ah(struct mthca_dev *dev, header->lrh.service_level = be32_to_cpu(ah->av->sl_tclass_flowlabel) >> 28; header->lrh.destination_lid = ah->av->dlid; header->lrh.source_lid = cpu_to_be16(ah->av->g_slid & 0x7f); - if (ah->av->g_slid & 0x80) { - header->grh_present = 1; + if (mthca_ah_grh_present(ah)) { header->grh.traffic_class = (be32_to_cpu(ah->av->sl_tclass_flowlabel) >> 20) & 0xff; header->grh.flow_label = @@ -184,8 +188,6 @@ int mthca_read_ah(struct mthca_dev *dev, &header->grh.source_gid); memcpy(header->grh.destination_gid.raw, ah->av->dgid, 16); - } else { - header->grh_present = 0; } return 0; diff --git a/drivers/infiniband/hw/mthca/mthca_dev.h b/drivers/infiniband/hw/mthca/mthca_dev.h index 795b379..a104ab0 100644 --- a/drivers/infiniband/hw/mthca/mthca_dev.h +++ b/drivers/infiniband/hw/mthca/mthca_dev.h @@ -520,6 +520,7 @@ int mthca_create_ah(struct mthca_dev *de int mthca_destroy_ah(struct mthca_dev *dev, struct mthca_ah *ah); int mthca_read_ah(struct mthca_dev *dev, struct mthca_ah *ah, struct ib_ud_header *header); +int mthca_ah_grh_present(struct mthca_ah *ah); int mthca_multicast_attach(struct ib_qp *ibqp, union ib_gid *gid, u16 lid); int mthca_multicast_detach(struct ib_qp *ibqp, union ib_gid *gid, u16 lid); diff --git a/drivers/infiniband/hw/mthca/mthca_qp.c b/drivers/infiniband/hw/mthca/mthca_qp.c index 564b6d5..fba608e 100644 --- a/drivers/infiniband/hw/mthca/mthca_qp.c +++ b/drivers/infiniband/hw/mthca/mthca_qp.c @@ -1434,7 +1434,7 @@ static int build_mlx_header(struct mthca u16 pkey; ib_ud_header_init(256, /* assume a MAD */ - sqp->ud_header.grh_present, + mthca_ah_grh_present(to_mah(wr->wr.ud.ah)), &sqp->ud_header); err = mthca_read_ah(dev, to_mah(wr->wr.ud.ah), &sqp->ud_header); -- 1.0.7 ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2006-01-13 0:15 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2006-01-13 0:13 [git patch review 1/6] IPoIB: Take dev->xmit_lock around mc_list accesses Roland Dreier 2006-01-13 0:13 ` [git patch review 2/6] IPoIB: Fix memory leak of multicast group structures Roland Dreier 2006-01-13 0:13 ` [git patch review 3/6] IB/mthca: " Roland Dreier 2006-01-13 0:13 ` [git patch review 4/6] IB/mthca: Fix memory leaks in error handling Roland Dreier 2006-01-13 0:13 ` [git patch review 5/6] IB/mthca: Cosmetic: use the ALIGN macro Roland Dreier 2006-01-13 0:13 ` [git patch review 6/6] IB/mthca: Initialize grh_present before using it Roland Dreier
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®