Re: [OMPI devel] RFC: Allocate free list payload if free list isn't specified
On Tue, 21 Feb 2012, Rolf vandeVaart wrote: I think I am OK with this. Alternatively, you could have done something like is done in the TCP BTL where the payload and header are added together for the frag size? To state more clearly, I was trying to say you could do something similar to what is done at line 1015 in btl_tcp_component.c and ended up with the same results? That will more or less work for my current use case (I found those examples this morning). I would have to pad my fragments to ensure cache line alignment (if that ends up being faster for SMSG). This is just making the payload buffer a different chunk of memory than the headers? Yes. I am just trying to understand the motivation for the change. The motivation is to allow the payload to be aligned separately from the header. Currently, if I want payload alignment I have to pad the header to get the correct alignment on the data. I think the way you have it is more correct so we can support the case where someone specifies the header size and the payload size differently and expects the free list code to do the right thing. This is one of my motivations. When writing the uGNI BTL I expected the free list to do the "right thing" and allocate a payload buffer even if I didn't specify an mpool. -Nathan
Re: [OMPI devel] RFC: Allocate free list payload if free list isn't specified
I think I am OK with this. Alternatively, you could have done something like is done in the TCP BTL where the payload and header are added together for the frag size? To state more clearly, I was trying to say you could do something similar to what is done at line 1015 in btl_tcp_component.c and ended up with the same results? This is just making the payload buffer a different chunk of memory than the headers? I am just trying to understand the motivation for the change. I think the way you have it is more correct so we can support the case where someone specifies the header size and the payload size differently and expects the free list code to do the right thing. Rolf >-Original Message- >From: devel-boun...@open-mpi.org [mailto:devel-boun...@open-mpi.org] >On Behalf Of Nathan Hjelm >Sent: Tuesday, February 21, 2012 3:59 PM >To: Open MPI Developers >Subject: Re: [OMPI devel] RFC: Allocate free list payload if free list isn't >specified > >Opps, screwed up the title. Should be: RFC: Allocate requested free list >payload even if an mpool isn't specified. > >-Nathan > >On Tue, 21 Feb 2012, Nathan Hjelm wro > >> What: Allocate free list payload even if a payload size is specified >> even if no mpool is specified. >> >> When: Thursday, Feb 23, 2012 >> >> Why: The current behavior is to ignore the payload size if no mpool is >> specified. I see no reason why a payload buffer should't be allocated >> in the no mpool case. Thoughts? >> >> Patch is attached. >> >> -Nathan Hjelm >> HPC-3, LANL >> >___ >devel mailing list >de...@open-mpi.org >http://www.open-mpi.org/mailman/listinfo.cgi/devel --- This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply email and destroy all copies of the original message. ---
Re: [OMPI devel] RFC: Allocate free list payload if free list isn't specified
Opps, screwed up the title. Should be: RFC: Allocate requested free list payload even if an mpool isn't specified. -Nathan On Tue, 21 Feb 2012, Nathan Hjelm wrote: What: Allocate free list payload even if a payload size is specified even if no mpool is specified. When: Thursday, Feb 23, 2012 Why: The current behavior is to ignore the payload size if no mpool is specified. I see no reason why a payload buffer should't be allocated in the no mpool case. Thoughts? Patch is attached. -Nathan Hjelm HPC-3, LANL
[OMPI devel] RFC: Allocate free list payload if free list isn't specified
What: Allocate free list payload even if a payload size is specified even if no
mpool is specified.
When: Thursday, Feb 23, 2012
Why: The current behavior is to ignore the payload size if no mpool is
specified. I see no reason why a payload buffer should't be allocated in the no
mpool case. Thoughts?
Patch is attached.
-Nathan Hjelm
HPC-3, LANLdiff --git a/ompi/class/ompi_free_list.c b/ompi/class/ompi_free_list.c
index d468a70..e3c0988 100644
--- a/ompi/class/ompi_free_list.c
+++ b/ompi/class/ompi_free_list.c
@@ -1,4 +1,4 @@
-/* -*- Mode: C; c-basic-offset:4 ; -*- */
+/* -*- Mode: C; c-basic-offset:4 ; indent-tabs-mode:nil -*- */
/*
* Copyright (c) 2004-2005 The Trustees of Indiana University and Indiana
* University Research and Technology
@@ -13,6 +13,8 @@
* Copyright (c) 2006-2007 Mellanox Technologies. All rights reserved.
* Copyright (c) 2010 Cisco Systems, Inc. All rights reserved.
* Copyright (c) 2011 NVIDIA Corporation. All rights reserved.
+ * Copyright (c) 2012 Los Alamos National Security, LLC. All rights
+ * reserved.
* $COPYRIGHT$
*
* Additional copyrights may follow
@@ -70,23 +72,19 @@ static void ompi_free_list_destruct(ompi_free_list_t* fl)
}
#endif
-if( NULL != fl->fl_mpool ) {
-while(NULL != (item = opal_list_remove_first(&(fl->fl_allocations {
-fl_mem = (ompi_free_list_memory_t*)item;
+while(NULL != (item = opal_list_remove_first(&(fl->fl_allocations {
+fl_mem = (ompi_free_list_memory_t*)item;
+if( NULL != fl->fl_mpool ) {
fl->fl_mpool->mpool_free(fl->fl_mpool, fl_mem->ptr,
fl_mem->registration);
-
-/* destruct the item (we constructed it), then free the memory
chunk */
-OBJ_DESTRUCT(item);
-free(item);
-}
-} else {
-while(NULL != (item = opal_list_remove_first(&(fl->fl_allocations {
-/* destruct the item (we constructed it), then free the memory
chunk */
-OBJ_DESTRUCT(item);
-free(item);
+} else if (fl_mem->ptr) {
+free (fl_mem->ptr);
}
+
+/* destruct the item (we constructed it), then free the memory chunk */
+OBJ_DESTRUCT(item);
+free(item);
}
OBJ_DESTRUCT(>fl_allocations);
@@ -171,7 +169,7 @@ int ompi_free_list_init_ex_new(
}
int ompi_free_list_grow(ompi_free_list_t* flist, size_t num_elements)
{
-unsigned char *ptr, *mpool_alloc_ptr = NULL;
+unsigned char *ptr, *mpool_alloc_ptr = NULL, *payload_ptr;
ompi_free_list_memory_t *alloc_ptr;
size_t i, alloc_size, head_size, elem_size = 0;
mca_mpool_base_registration_t *reg = NULL;
@@ -201,7 +199,7 @@ int ompi_free_list_grow(ompi_free_list_t* flist, size_t
num_elements)
elem_size = OPAL_ALIGN(flist->fl_payload_buffer_size,
flist->fl_payload_buffer_alignment, size_t);
if(elem_size != 0) {
-mpool_alloc_ptr = (unsigned char *)
flist->fl_mpool->mpool_alloc(flist->fl_mpool,
+payload_ptr = mpool_alloc_ptr = (unsigned char *)
flist->fl_mpool->mpool_alloc(flist->fl_mpool,
num_elements * elem_size,
flist->fl_payload_buffer_alignment,
MCA_MPOOL_FLAGS_CACHE_BYPASS |
MCA_MPOOL_FLAGS_CUDA_REGISTER_MEM, );
if(NULL == mpool_alloc_ptr) {
@@ -209,6 +207,26 @@ int ompi_free_list_grow(ompi_free_list_t* flist, size_t
num_elements)
return OMPI_ERR_TEMP_OUT_OF_RESOURCE;
}
}
+} else if (0 != flist->fl_payload_buffer_size) {
+elem_size = OPAL_ALIGN(flist->fl_payload_buffer_size,
+ flist->fl_payload_buffer_alignment, size_t);
+if(elem_size != 0) {
+#ifdef HAVE_POSIX_MEMALIGN
+posix_memalign ((void **) _alloc_ptr,
flist->fl_payload_buffer_alignment,
+num_elements * elem_size);
+payload_ptr = mpool_alloc_ptr;
+#else
+mpool_alloc_ptr = malloc (num_elements * elem_size +
+ flist->fl_payload_buffer_alignment);
+payload_ptr = (void*)OPAL_ALIGN((uintptr_t)mpool_alloc_ptr,
+flist->fl_payload_buffer_alignment,
+uintptr_t);
+#endif
+if(NULL == mpool_alloc_ptr) {
+free(alloc_ptr);
+return OMPI_ERR_TEMP_OUT_OF_RESOURCE;
+}
+}
}
/* make the alloc_ptr a list item, save the chunk in the allocations list,
@@ -225,7 +243,7 @@ int ompi_free_list_grow(ompi_free_list_t* flist, size_t
num_elements)
for(i=0; iregistration = reg;
-item->ptr = mpool_alloc_ptr;
+item->ptr = payload_ptr;
