* [git patch review 1/2] IB/mad: Handle DR SMPs with a LID routed part @ 2006-02-04 16:33 Roland Dreier 2006-02-04 16:33 ` [git patch review 2/2] IB: Don't doublefree pages from scatterlist Roland Dreier 0 siblings, 1 reply; 4+ messages in thread From: Roland Dreier @ 2006-02-04 16:33 UTC (permalink / raw) To: linux-kernel, openib-general Fix handling of directed route SMPs with a beginning or ending LID routed part. Signed-off-by: Ralph Campbell <ralphc@pathscale.com> Signed-off-by: Hal Rosenstock <halr@voltaire.com> Signed-off-by: Roland Dreier <rolandd@cisco.com> --- drivers/infiniband/core/mad.c | 10 +++++++++- 1 files changed, 9 insertions(+), 1 deletions(-) 8cf3f04f45694db0699f608c0e3fb550c607cc88 diff --git a/drivers/infiniband/core/mad.c b/drivers/infiniband/core/mad.c index d393b50..c82f47a 100644 --- a/drivers/infiniband/core/mad.c +++ b/drivers/infiniband/core/mad.c @@ -665,7 +665,15 @@ static int handle_outgoing_dr_smp(struct struct ib_wc mad_wc; struct ib_send_wr *send_wr = &mad_send_wr->send_wr; - if (!smi_handle_dr_smp_send(smp, device->node_type, port_num)) { + /* + * Directed route handling starts if the initial LID routed part of + * a request or the ending LID routed part of a response is empty. + * If we are at the start of the LID routed part, don't update the + * hop_ptr or hop_cnt. See section 14.2.2, Vol 1 IB spec. + */ + if ((ib_get_smp_direction(smp) ? smp->dr_dlid : smp->dr_slid) == + IB_LID_PERMISSIVE && + !smi_handle_dr_smp_send(smp, device->node_type, port_num)) { ret = -EINVAL; printk(KERN_ERR PFX "Invalid directed route\n"); goto out; -- 1.1.3 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* [git patch review 2/2] IB: Don't doublefree pages from scatterlist 2006-02-04 16:33 [git patch review 1/2] IB/mad: Handle DR SMPs with a LID routed part Roland Dreier @ 2006-02-04 16:33 ` Roland Dreier 2006-02-06 22:29 ` Hugh Dickins 0 siblings, 1 reply; 4+ messages in thread From: Roland Dreier @ 2006-02-04 16:33 UTC (permalink / raw) To: linux-kernel, openib-general On some architectures, mapping the scatterlist may coalesce entries: if that coalesced list is then used for freeing the pages afterwards, there's a danger that pages may be doubly freed (and others leaked). Fix Infiniband's __ib_umem_release by freeing from a separate array beyond the scatterlist: IB_UMEM_MAX_PAGE_CHUNK lowered to fit one page. Signed-off-by: Hugh Dickins <hugh@veritas.com> Signed-off-by: Roland Dreier <rolandd@cisco.com> --- drivers/infiniband/core/uverbs_mem.c | 22 ++++++++++++++++------ include/rdma/ib_verbs.h | 3 +-- 2 files changed, 17 insertions(+), 8 deletions(-) 46fc99a4a1429f843e3b6df8ed1f571944bef4e2 diff --git a/drivers/infiniband/core/uverbs_mem.c b/drivers/infiniband/core/uverbs_mem.c index 36a32c3..87a363e 100644 --- a/drivers/infiniband/core/uverbs_mem.c +++ b/drivers/infiniband/core/uverbs_mem.c @@ -49,15 +49,18 @@ struct ib_umem_account_work { static void __ib_umem_release(struct ib_device *dev, struct ib_umem *umem, int dirty) { struct ib_umem_chunk *chunk, *tmp; + struct page **sg_pages; int i; list_for_each_entry_safe(chunk, tmp, &umem->chunk_list, list) { dma_unmap_sg(dev->dma_device, chunk->page_list, chunk->nents, DMA_BIDIRECTIONAL); + /* Scatterlist may have been coalesced: free saved pagelist */ + sg_pages = (struct page **) (chunk->page_list + chunk->nents); for (i = 0; i < chunk->nents; ++i) { if (umem->writable && dirty) - set_page_dirty_lock(chunk->page_list[i].page); - put_page(chunk->page_list[i].page); + set_page_dirty_lock(sg_pages[i]); + put_page(sg_pages[i]); } kfree(chunk); @@ -69,11 +72,13 @@ int ib_umem_get(struct ib_device *dev, s { struct page **page_list; struct ib_umem_chunk *chunk; + struct page **sg_pages; unsigned long locked; unsigned long lock_limit; unsigned long cur_base; unsigned long npages; int ret = 0; + int nents; int off; int i; @@ -121,16 +126,21 @@ int ib_umem_get(struct ib_device *dev, s off = 0; while (ret) { - chunk = kmalloc(sizeof *chunk + sizeof (struct scatterlist) * - min_t(int, ret, IB_UMEM_MAX_PAGE_CHUNK), + nents = min_t(int, ret, IB_UMEM_MAX_PAGE_CHUNK); + chunk = kmalloc(sizeof *chunk + + sizeof (struct scatterlist) * nents + + sizeof (struct page *) * nents, GFP_KERNEL); if (!chunk) { ret = -ENOMEM; goto out; } + /* Save pages to be freed in array beyond scatterlist */ + sg_pages = (struct page **) (chunk->page_list + nents); - chunk->nents = min_t(int, ret, IB_UMEM_MAX_PAGE_CHUNK); + chunk->nents = nents; for (i = 0; i < chunk->nents; ++i) { + sg_pages[i] = page_list[i + off]; chunk->page_list[i].page = page_list[i + off]; chunk->page_list[i].offset = 0; chunk->page_list[i].length = PAGE_SIZE; @@ -142,7 +152,7 @@ int ib_umem_get(struct ib_device *dev, s DMA_BIDIRECTIONAL); if (chunk->nmap <= 0) { for (i = 0; i < chunk->nents; ++i) - put_page(chunk->page_list[i].page); + put_page(sg_pages[i]); kfree(chunk); ret = -ENOMEM; diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h index 22fc886..239c11d 100644 --- a/include/rdma/ib_verbs.h +++ b/include/rdma/ib_verbs.h @@ -696,8 +696,7 @@ struct ib_udata { #define IB_UMEM_MAX_PAGE_CHUNK \ ((PAGE_SIZE - offsetof(struct ib_umem_chunk, page_list)) / \ - ((void *) &((struct ib_umem_chunk *) 0)->page_list[1] - \ - (void *) &((struct ib_umem_chunk *) 0)->page_list[0])) + (sizeof (struct scatterlist) + sizeof (struct page *))) struct ib_umem_object { struct ib_uobject uobject; -- 1.1.3 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [git patch review 2/2] IB: Don't doublefree pages from scatterlist 2006-02-04 16:33 ` [git patch review 2/2] IB: Don't doublefree pages from scatterlist Roland Dreier @ 2006-02-06 22:29 ` Hugh Dickins 2006-02-07 1:44 ` Roland Dreier 0 siblings, 1 reply; 4+ messages in thread From: Hugh Dickins @ 2006-02-06 22:29 UTC (permalink / raw) To: Roland Dreier; +Cc: Kai Makisara, Willem Riede, linux-kernel, openib-general On Sat, 4 Feb 2006, Roland Dreier wrote: > On some architectures, mapping the scatterlist may coalesce entries: > if that coalesced list is then used for freeing the pages afterwards, > there's a danger that pages may be doubly freed (and others leaked). > > Fix Infiniband's __ib_umem_release by freeing from a separate array > beyond the scatterlist: IB_UMEM_MAX_PAGE_CHUNK lowered to fit one page. It's now looking like this change won't be needed after all: Andi has just posted a patch in the "ipr" thread which should stop x86_64 from interfering with the scatterlist *page,offset,length fields, so what IB and others were doing should then work safely (current thinking is that x86_64 is the only architecture which coalesced in that way). Hugh ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [git patch review 2/2] IB: Don't doublefree pages from scatterlist 2006-02-06 22:29 ` Hugh Dickins @ 2006-02-07 1:44 ` Roland Dreier 0 siblings, 0 replies; 4+ messages in thread From: Roland Dreier @ 2006-02-07 1:44 UTC (permalink / raw) To: Hugh Dickins Cc: Roland Dreier, Kai Makisara, Willem Riede, linux-kernel, openib-general Hugh> It's now looking like this change won't be needed after all: Hugh> Andi has just posted a patch in the "ipr" thread which Hugh> should stop x86_64 from interfering with the scatterlist Hugh> *page,offset,length fields, so what IB and others were doing Hugh> should then work safely (current thinking is that x86_64 is Hugh> the only architecture which coalesced in that way). OK, I'll drop this from my tree. - R. ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2006-02-07 3:50 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2006-02-04 16:33 [git patch review 1/2] IB/mad: Handle DR SMPs with a LID routed part Roland Dreier 2006-02-04 16:33 ` [git patch review 2/2] IB: Don't doublefree pages from scatterlist Roland Dreier 2006-02-06 22:29 ` Hugh Dickins 2006-02-07 1:44 ` Roland Dreier
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox