Merge branch 'dev-optional-size-checks' into 'master'

Dev optional size checks

See merge request !5

Makefile.am

@@ -31,7 +31,7 @@ lib_LTLIBRARIES = src/libbake-client.la src/libbake-server.la
 src_libbake_client_la_SOURCES =
 src_libbake_server_la_SOURCES =
 
-LDADD = src/libbake-client.la
+LDADD = src/libbake-client.la src/libbake-server.la
 
 pkgconfigdir = $(libdir)/pkgconfig
 pkgconfig_DATA = \
@@ -44,4 +44,3 @@ include Make.rules
 include $(top_srcdir)/src/Makefile.subdir
 include $(top_srcdir)/tests/Makefile.subdir
 include $(top_srcdir)/tests/proxy/Makefile.subdir
-

configure.ac

@@ -86,6 +86,8 @@ LIBS="$REMI_LIBS $LIBS"
 CPPFLAGS="$REMI_CFLAGS $CPPFLAGS"
 CFLAGS="$REMI_CFLAGS $CFLAGS"
 
+AC_ARG_ENABLE(sizecheck, [Adds a header in regions to keep track of region sizes], 
+		[AC_DEFINE([USE_SIZECHECK_HEADERS], 1, [Enable sizechecks])], [])
+
 AC_CONFIG_FILES([Makefile maint/bake-client.pc maint/bake-server.pc])
 AC_OUTPUT
- 

include/bake-client.h

@@ -189,11 +189,15 @@ int bake_proxy_write(
  *
  * @param [in] provider provider handle
  * @param [in] rid identifier for region
+ * @param [in] offset offset in the region
+ * @param [in] size of the region
  * @return BAKE_SUCCESS or corresponding error code.
  */
 int bake_persist(
         bake_provider_handle_t provider,
-        bake_region_id_t rid);
+        bake_region_id_t rid,
+        size_t offset,
+        size_t size);
 
 /**
  * Creates a bounded-size BAKE region, writes data into it, and persists
@@ -234,7 +238,10 @@ int bake_create_write_persist_proxy(
         bake_region_id_t *rid);
 
 /**
- * Checks the size of an existing BAKE region. 
+ * Checks the size of an existing BAKE region.
+ * This function only works if Bake has been compiled with --enable-sizecheck,
+ * otherwise Bake has no way of knowing the size of regions and it is up to
+ * the user to track the region sizes in some other ways.
  *
  * @param [in] provider provider handle
  * @param [in] rid identifier for region
@@ -326,6 +333,7 @@ int bake_proxy_read(
  *
  * @param source Source provider.
  * @param source_rid Region to migrate.
+ * @param region_size Size of the region to migrate.
  * @param remove_source Whether the source region should be removed.
  * @param dest_addr Address of the destination provider.
  * @param dest_provider_id Id of the destination provider.
@@ -337,43 +345,13 @@ int bake_proxy_read(
 int bake_migrate_region(
         bake_provider_handle_t source,
         bake_region_id_t source_rid,
+        size_t region_size,
         int remove_source,
         const char* dest_addr,
         uint16_t dest_provider_id,
         bake_target_id_t dest_target_id,
         bake_region_id_t* dest_rid);
 
-/** 
- * @brief Requests the source provider to migrate a particular
- * region (source_rid) to a destination provider. After the call,
- * the designated region will have been removed from the source
- * and the dest_rid parameter will be set to the new region id
- * in the destination provider.
- *
- * @param source Source provider.
- * @param source_rid Region to migrate.
- * @param remove_source Whether the source region should be removed.
- * @param dest_addr Address of the destination provider.
- * @param dest_provider_id Id of the destination provider.
- * @param dest_target_id Destination target.
- * @param dest_rid Resulting region id in the destination target.
- *
- * @return BAKE_SUCCESS or corresponding error code.
- */
-__attribute__((deprecated("use bake_migrate_region instead")))
-inline int bake_migrate(
-        bake_provider_handle_t source,
-        bake_region_id_t source_rid,
-        int remove_source,
-        const char* dest_addr,
-        uint16_t dest_provider_id,
-        bake_target_id_t dest_target_id,
-        bake_region_id_t* dest_rid)
-{
-    return bake_migrate_region(source, source_rid, remove_source,
-            dest_addr, dest_provider_id, dest_target_id, dest_rid);
-}
-
 /**
  * @brief Migrates a full target from a provider to another.
  *

include/bake.h

@@ -37,6 +37,7 @@ typedef struct {
 #define BAKE_ERR_UNKNOWN_REGION     (-8) /* Region id could not be found */
 #define BAKE_ERR_OUT_OF_BOUNDS      (-9) /* Attempting an out of bound access */
 #define BAKE_ERR_REMI              (-10) /* Error related to REMI */
+#define BAKE_ERR_OP_UNSUPPORTED    (-11) /* Operation not supported */
 
 #ifdef __cplusplus
 }

src/bake-client.c

@@ -475,7 +475,9 @@ int bake_create(
 
 int bake_persist(
     bake_provider_handle_t provider,
-    bake_region_id_t rid)
+    bake_region_id_t rid,
+    size_t offset,
+    size_t size)
 {
     hg_return_t hret;
     hg_handle_t handle;
@@ -484,6 +486,8 @@ int bake_persist(
     int ret;
 
     in.rid = rid;
+    in.offset = offset;
+    in.size = size;
 
     hret = margo_create(provider->client->mid, provider->addr,
             provider->client->bake_persist_id, &handle);
@@ -729,6 +733,7 @@ int bake_get_data(
 int bake_migrate_region(
         bake_provider_handle_t source,
         bake_region_id_t source_rid,
+        size_t region_size,
         int remove_source,
         const char* dest_addr,
         uint16_t dest_provider_id,
@@ -742,6 +747,7 @@ int bake_migrate_region(
     int ret;
 
     in.source_rid       = source_rid;
+    in.region_size      = region_size;
     in.remove_src       = remove_source;
     in.dest_addr        = dest_addr;
     in.dest_provider_id = dest_provider_id;

src/bake-copy-to.c

@@ -182,7 +182,7 @@ int main(int argc, char **argv)
     munmap(local_region, statbuf.st_size);
     close(fd);
 
-    ret = bake_persist(bph, rid);
+    ret = bake_persist(bph, rid, 0, statbuf.st_size);
     if(ret != 0)
     {
         bake_provider_handle_release(bph);

src/bake-latency-bench.c

@@ -167,7 +167,7 @@ static void bench_routine_write(bake_provider_handle_t bph, bake_target_id_t bti
     }
 
     /* persist */
-    ret = bake_persist(bph, rid);
+    ret = bake_persist(bph, rid, 0, size*iterations);
     assert(ret == 0);
 
     free(buffer);

src/bake-rpc.h

@@ -50,7 +50,9 @@ MERCURY_GEN_PROC(bake_eager_write_out_t,
 
 /* BAKE persist */
 MERCURY_GEN_PROC(bake_persist_in_t,
-    ((bake_region_id_t)(rid)))
+    ((bake_region_id_t)(rid))\
+    ((uint64_t)(offset))\
+    ((uint64_t)(size)))
 MERCURY_GEN_PROC(bake_persist_out_t,
     ((int32_t)(ret)))
 
@@ -125,6 +127,7 @@ MERCURY_GEN_PROC(bake_remove_out_t,
 /* BAKE migrate region */
 MERCURY_GEN_PROC(bake_migrate_region_in_t,
     ((bake_region_id_t)(source_rid))\
+    ((uint64_t)(region_size))\
     ((int32_t)(remove_src))\
     ((hg_const_string_t)(dest_addr))\
     ((uint16_t)(dest_provider_id))\

src/bake-server.c

@@ -43,7 +43,9 @@ typedef struct
 } pmemobj_region_id_t;
 
 typedef struct {
+#ifdef USE_SIZECHECK_HEADERS
     uint64_t size;
+#endif
     char data[1];
 } region_content_t;
 
@@ -423,7 +425,12 @@ static void bake_create_ult(hg_handle_t handle)
 
     prid = (pmemobj_region_id_t*)out.rid.data;
 
+#ifdef USE_SIZECHECK_HEADERS
     size_t content_size = in.region_size + sizeof(uint64_t);
+#else
+    size_t content_size = in.region_size;
+#endif
+
     int ret = pmemobj_alloc(entry->pmem_pool, &prid->oid,
             content_size, 0, NULL, NULL);
     if(ret != 0) {
@@ -436,9 +443,13 @@ static void bake_create_ult(hg_handle_t handle)
         out.ret = BAKE_ERR_PMEM;
         goto finish;
     }
+#ifdef USE_SIZECHECK_HEADERS
     region->size = in.region_size;
+#endif
     PMEMobjpool* pmem_pool = pmemobj_pool_by_oid(prid->oid);
-    pmemobj_persist(pmem_pool, &(region->size), sizeof(region->size));
+#ifdef USE_SIZECHECK_HEADERS
+    pmemobj_persist(pmem_pool, region, sizeof(region->size));
+#endif
 
     out.ret = BAKE_SUCCESS;
 
@@ -499,10 +510,12 @@ static void bake_write_ult(hg_handle_t handle)
         goto finish;
     }
 
+#ifdef USE_SIZECHECK_HEADERS
     if(in.region_offset + in.bulk_size > region->size) {
         out.ret = BAKE_ERR_OUT_OF_BOUNDS;
         goto finish;
     }
+#endif
 
     buffer = region->data + in.region_offset;
 
@@ -599,10 +612,12 @@ static void bake_eager_write_ult(hg_handle_t handle)
         goto finish;
     }
 
+#ifdef USE_SIZECHECK_HEADERS
     if(in.size + in.region_offset > region->size) {
         out.ret = BAKE_ERR_OUT_OF_BOUNDS;
         goto finish;
     }
+#endif
 
     buffer = region->data + in.region_offset;
 
@@ -664,7 +679,7 @@ static void bake_persist_ult(hg_handle_t handle)
 
     /* TODO: should this have an abt shim in case it blocks? */
     PMEMobjpool* pmem_pool = pmemobj_pool_by_oid(prid->oid);
-    pmemobj_persist(pmem_pool, buffer, region->size);
+    pmemobj_persist(pmem_pool, buffer + in.offset, in.size);
 
     out.ret = BAKE_SUCCESS;
 
@@ -743,7 +758,9 @@ static void bake_create_write_persist_ult(hg_handle_t handle)
         out.ret = BAKE_ERR_PMEM;
         goto finish;
     }
+#ifdef USE_SIZECHECK_HEADERS
     region->size = in.bulk_size;
+#endif
     buffer = region->data;
 
     /* create bulk handle for local side of transfer */
@@ -824,6 +841,7 @@ static void bake_get_size_ult(hg_handle_t handle)
         goto finish;
     }
 
+#ifdef USE_SIZECHECK_HEADERS
     prid = (pmemobj_region_id_t*)in.rid.data;
     /* lock provider */
     lock = svr_ctx->lock;
@@ -836,6 +854,9 @@ static void bake_get_size_ult(hg_handle_t handle)
     }
     out.size = region->size;
     out.ret = BAKE_SUCCESS;
+#else
+    out.ret = BAKE_ERR_OP_UNSUPPORTED;
+#endif
 
 finish:
     if(lock != ABT_RWLOCK_NULL)
@@ -961,17 +982,20 @@ static void bake_read_ult(hg_handle_t handle)
         goto finish;
     }
 
+    size_to_read = in.bulk_size;
+
+#ifdef USE_SIZECHECK_HEADERS
     if(in.region_offset > region->size)
     {
         out.ret = BAKE_ERR_OUT_OF_BOUNDS;
         goto finish;
     }
-
     if(in.region_offset + in.bulk_size > region->size) {
         size_to_read = region->size - in.region_offset;
     } else {
         size_to_read = in.bulk_size;
     }
+#endif
 
     buffer = region->data + in.region_offset;
 
@@ -1069,17 +1093,18 @@ static void bake_eager_read_ult(hg_handle_t handle)
         goto finish;
     }
 
+    size_to_read = in.size;
+
+#ifdef USE_SIZECHECK_HEADERS
     if(in.region_offset > region->size)
     {
         out.ret = BAKE_ERR_OUT_OF_BOUNDS;
         goto finish;
     }
-
     if(in.region_offset + in.size > region->size) {
         size_to_read = region->size - in.region_offset;
-    } else {
-        size_to_read = in.size;
     }
+#endif
 
     buffer = region->data + in.region_offset;
 
@@ -1213,10 +1238,19 @@ static void bake_migrate_region_ult(hg_handle_t handle)
         out.ret = BAKE_ERR_UNKNOWN_REGION;
         goto finish;
     }
+
     /* get the size of the region */
-    size_t region_size = region->size;
+    size_t region_size = in.region_size;
     char* region_data  = region->data;
 
+#ifdef USE_SIZECHECK_HEADERS
+    /* check region size */
+    if(in.region_size != region->size) {
+        out.ret = BAKE_ERR_INVALID_ARG;
+        goto finish;
+    }
+#endif
+
     /* lookup the address of the destination provider */
     hret = margo_addr_lookup(mid, in.dest_addr, &dest_addr);
     if(hret != HG_SUCCESS) {