Open MPI logo

Open MPI Development Mailing List Archives

  |   Home   |   Support   |   FAQ   |   all Development mailing list

Subject: Re: [OMPI devel] RFC: Remove alignment code from rcache
From: Rolf vandeVaart (rvandevaart_at_[hidden])
Date: 2013-09-18 10:13:57


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

From: devel [mailto:devel-bounces_at_[hidden]] On Behalf Of Rolf vandeVaart
Sent: Tuesday, September 10, 2013 2:46 PM
To: devel_at_[hidden]
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_at_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_at_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.
-----------------------------------------------------------------------------------