I will wait another week on this since I know a lot of folks were traveling.  
Any input welcome.

From: devel [mailto:devel-boun...@open-mpi.org] On Behalf Of Rolf vandeVaart
Sent: Tuesday, September 10, 2013 2:46 PM
To: de...@open-mpi.org
Subject: [OMPI devel] RFC: Remove alignment code from rcache

WHAT: Remove alignment code from ompi/mca/rcache/vma module
WHY: Because it is redundant and causing problems for memory pools that want 
different alignment
WHERE: ompi/mca/rcache/vma/rcache_vma.c, 
ompi/mca/mpool/grdma/mpool_grdma_module.c (Detailed changes attached)
WHEN: Tuesday,  September 17, 2013 COB
More detail:
This RFC looks to remove the alignment code from the rcache as it seems 
unnecessary.  In all use cases in the library, alignment requirements are 
handled in the memory pool layer (or in the case of the vader btl, in the btl 
layer).  It seems more logical that the alignment is in the upper layer as that 
code is also where any registration restrictions would be known.  The rcache 
alignment code causes problems for me where I want to have different alignment 
requirements than the rcache is forcing on me.  (The rcache defaults to an 
alignment of mca_mpool_base_page_size_log=4K on my machine)  Therefore, I would 
like to make the change as attached to this email.

I have run through some tests and all seems OK.  Is there anything I am missing 
such that we need this  code in the rcache?

Thanks,
Rolf

[rvandevaart@sm064 ompi-trunk-tuesday]$ svn diff
Index: ompi/mca/rcache/vma/rcache_vma.c
===================================================================
--- ompi/mca/rcache/vma/rcache_vma.c             (revision 29155)
+++ ompi/mca/rcache/vma/rcache_vma.c          (working copy)
@@ -48,15 +48,13 @@
         void* addr, size_t size, mca_mpool_base_registration_t **reg)  {
     int rc;
-    void* base_addr;
-    void* bound_addr;
+    unsigned char* bound_addr;

     if(size == 0) {
         return OMPI_ERROR;
     }

-    base_addr = down_align_addr(addr, mca_mpool_base_page_size_log);
-    bound_addr = up_align_addr((void*) ((unsigned long) addr + size - 1), 
mca_mpool_base_page_size_log);
+    bound_addr = addr + size - 1;

     /* Check to ensure that the cache is valid */
     if (OPAL_UNLIKELY(opal_memory_changed() && @@ -65,8 +63,8 @@
         return rc;
     }

-    *reg = mca_rcache_vma_tree_find((mca_rcache_vma_module_t*)rcache, 
(unsigned char*)base_addr,
-            (unsigned char*)bound_addr);
+    *reg = mca_rcache_vma_tree_find((mca_rcache_vma_module_t*)rcache, 
(unsigned char*)addr,
+            bound_addr);

     return OMPI_SUCCESS;
}
@@ -76,14 +74,13 @@
         int reg_cnt)
{
     int rc;
-    void *base_addr, *bound_addr;
+    unsigned char *bound_addr;

     if(size == 0) {
         return OMPI_ERROR;
     }

-    base_addr = down_align_addr(addr, mca_mpool_base_page_size_log);
-    bound_addr = up_align_addr((void*) ((unsigned long) addr + size - 1), 
mca_mpool_base_page_size_log);
+    bound_addr = addr + size - 1;

     /* Check to ensure that the cache is valid */
     if (OPAL_UNLIKELY(opal_memory_changed() && @@ -93,7 +90,7 @@
     }

     return mca_rcache_vma_tree_find_all((mca_rcache_vma_module_t*)rcache,
-            (unsigned char*)base_addr, (unsigned char*)bound_addr, regs,
+            (unsigned char*)addr, bound_addr, regs,
             reg_cnt);
}

Index: ompi/mca/mpool/grdma/mpool_grdma_module.c
===================================================================
--- ompi/mca/mpool/grdma/mpool_grdma_module.c   (revision 29155)
+++ ompi/mca/mpool/grdma/mpool_grdma_module.c                (working copy)
@@ -233,7 +233,7 @@
      * Persistent registration are always registered and placed in the cache */
     if(!(bypass_cache || persist)) {
         /* check to see if memory is registered */
-        mpool->rcache->rcache_find(mpool->rcache, addr, size, reg);
+        mpool->rcache->rcache_find(mpool->rcache, base, bound - base +
+ 1, reg);
         if (*reg && !(flags & MCA_MPOOL_FLAGS_INVALID)) {
             if (0 == (*reg)->ref_count) {
                 /* Leave pinned must be set for this to still be in the 
rcache. */ @@ -346,7 +346,7 @@

     OPAL_THREAD_LOCK(&mpool->rcache->lock);

-    rc = mpool->rcache->rcache_find(mpool->rcache, addr, size, reg);
+    rc = mpool->rcache->rcache_find(mpool->rcache, base, bound - base +
+ 1, reg);
     if(NULL != *reg &&
             (mca_mpool_grdma_component.leave_pinned ||
              ((*reg)->flags & MCA_MPOOL_FLAGS_PERSIST) ||
[rvandevaart@sm064 ompi-trunk-tuesday]$

-----------------------------------------------------------------------------------
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.
-----------------------------------------------------------------------------------

Reply via email to