simplify the simcall stack for file unlink

there are still a few leaks: as unlink does not totally destroy the
data structures. If the same file descriptor is reopened (new file is
created), here comes the leak ...

diff --git a/src/include/surf/surf.h b/src/include/surf/surf.h
index f72453a..f6c7bc1 100644 (file)
--- a/src/include/surf/surf.h
+++ b/src/include/surf/surf.h
@@ -232,9 +232,7 @@ typedef struct surf_storage_model_extension_public {
   surf_action_t(*write) (void *storage, const void* ptr, size_t size,
                          size_t nmemb, surf_file_t fd);
   surf_action_t(*stat) (void *storage, surf_file_t fd);
-  surf_action_t(*unlink) (void *storage, surf_file_t fd);
   surf_action_t(*ls) (void *storage, const char *path);
-  size_t (*get_size) (void *storage, surf_file_t fd);
 } s_surf_model_extension_storage_t;
 
      /** \ingroup SURF_models
@@ -270,7 +268,7 @@ typedef struct surf_workstation_model_extension_public {
   surf_action_t(*write) (void *workstation, const void* ptr, size_t size,
                          size_t nmemb, surf_file_t fd);
   surf_action_t(*stat) (void *workstation, surf_file_t fd);
-  surf_action_t(*unlink) (void *workstation, surf_file_t fd);
+  int(*unlink) (void *workstation, surf_file_t fd);
   surf_action_t(*ls) (void *workstation, const char* mount, const char *path);
   size_t (*get_size) (void *workstation, surf_file_t fd);
 

diff --git a/src/msg/msg_io.c b/src/msg/msg_io.c
index cbbf2dd..b15cdb5 100644 (file)
--- a/src/msg/msg_io.c
+++ b/src/msg/msg_io.c
@@ -58,7 +58,7 @@ size_t MSG_file_write(const void* ptr, size_t size, size_t nmemb, msg_file_t fd)
 msg_file_t MSG_file_open(const char* mount, const char* path)
 {
   msg_file_t file = xbt_new(s_msg_file_t,1);
-  file->name = strdup(path);
+  file->name = xbt_strdup(path);
   file->simdata = xbt_new0(s_simdata_file_t,1);
   file->simdata->smx_file = simcall_file_open(mount, path);
   return file;
@@ -87,11 +87,7 @@ int MSG_file_close(msg_file_t fd)
  */
 int MSG_file_unlink(msg_file_t fd)
 {
-  int res = simcall_file_unlink(fd->simdata->smx_file);
-  free(fd->name);
-  xbt_free(fd->simdata);
-  xbt_free(fd);
-  return res;
+  return simcall_file_unlink(fd->simdata->smx_file);
 }
 
 /** \ingroup msg_file_management

diff --git a/src/simix/smx_io.c b/src/simix/smx_io.c
index 9822703..1b00eed 100644 (file)
--- a/src/simix/smx_io.c
+++ b/src/simix/smx_io.c
@@ -172,16 +172,13 @@ smx_action_t SIMIX_file_close(smx_process_t process, smx_file_t fd)
 
 
 //SIMIX FILE UNLINK
-void SIMIX_pre_file_unlink(smx_simcall_t simcall, smx_file_t fd)
+int SIMIX_pre_file_unlink(smx_simcall_t simcall, smx_file_t fd)
 {
-  smx_action_t action = SIMIX_file_unlink(simcall->issuer, fd);
-  xbt_fifo_push(action->simcalls, simcall);
-  simcall->issuer->waiting_action = action;
+  return SIMIX_file_unlink(simcall->issuer, fd);
 }
 
-smx_action_t SIMIX_file_unlink(smx_process_t process, smx_file_t fd)
+int SIMIX_file_unlink(smx_process_t process, smx_file_t fd)
 {
-  smx_action_t action;
   smx_host_t host = process->smx_host;
   /* check if the host is active */
   if (surf_workstation_model->extension.
@@ -190,20 +187,11 @@ smx_action_t SIMIX_file_unlink(smx_process_t process, smx_file_t fd)
            sg_host_name(host));
   }
 
-  action = xbt_mallocator_get(simix_global->action_mallocator);
-  action->type = SIMIX_ACTION_IO;
-  action->name = NULL;
-#ifdef HAVE_TRACING
-  action->category = NULL;
-#endif
-
-  action->io.host = host;
-  action->io.surf_io = surf_workstation_model->extension.workstation.unlink(host, fd->surf_file);
-
-  surf_workstation_model->action_data_set(action->io.surf_io, action);
-  XBT_DEBUG("Create io action %p", action);
-
-  return action;
+  if (surf_workstation_model->extension.workstation.unlink(host, fd->surf_file)){
+    fd->surf_file = NULL;
+    return 1;
+  } else
+    return 0;
 }
 
 //SIMIX FILE LS
@@ -282,11 +270,6 @@ void SIMIX_post_io(smx_action_t action)
       simcall_file_read__set__result(simcall, (action->io.surf_io)->cost);
       break;
 
-    case SIMCALL_FILE_UNLINK:
-      xbt_free(simcall_file_unlink__get__fd(simcall));
-      simcall_file_unlink__set__result(simcall, 0);
-      break;
-
     case SIMCALL_FILE_LS:
 //      xbt_dict_foreach((action->io.surf_io)->ls_dict,cursor,key, src){
 //        // if there is a stat we have to duplicate it

diff --git a/src/simix/smx_io_private.h b/src/simix/smx_io_private.h
index 3a85d32..0fd7e61 100644 (file)
--- a/src/simix/smx_io_private.h
+++ b/src/simix/smx_io_private.h
@@ -17,7 +17,7 @@ void SIMIX_pre_file_write(smx_simcall_t simcall, const void *ptr, size_t size,
 void SIMIX_pre_file_open(smx_simcall_t simcall, const char* mount,
                         const char* path);
 void SIMIX_pre_file_close(smx_simcall_t simcall, smx_file_t fd);
-void SIMIX_pre_file_unlink(smx_simcall_t simcall, smx_file_t fd);
+int SIMIX_pre_file_unlink(smx_simcall_t simcall, smx_file_t fd);
 void SIMIX_pre_file_ls(smx_simcall_t simcall,
                        const char* mount, const char* path);
 size_t SIMIX_pre_file_get_size(smx_simcall_t simcall, smx_file_t fd);
@@ -29,7 +29,7 @@ smx_action_t SIMIX_file_write(smx_process_t process, const void* ptr,
 smx_action_t SIMIX_file_open(smx_process_t process, const char* storage,
                              const char* path);
 smx_action_t SIMIX_file_close(smx_process_t process, smx_file_t fd);
-smx_action_t SIMIX_file_unlink(smx_process_t process, smx_file_t fd);
+int SIMIX_file_unlink(smx_process_t process, smx_file_t fd);
 smx_action_t SIMIX_file_ls(smx_process_t process, const char *mount,
                            const char *path);
 size_t SIMIX_file_get_size(smx_process_t process, smx_file_t fd);

diff --git a/src/simix/smx_smurf_private.h b/src/simix/smx_smurf_private.h
index 6a7e590..29144e0 100644 (file)
--- a/src/simix/smx_smurf_private.h
+++ b/src/simix/smx_smurf_private.h
@@ -343,7 +343,7 @@ ACTION(SIMCALL_FILE_READ, file_read, WITHOUT_ANSWER, TDOUBLE(result), TPTR(ptr),
 ACTION(SIMCALL_FILE_WRITE, file_write, WITHOUT_ANSWER, TSIZE(result), TCPTR(ptr), TSIZE(size), TSIZE(nmemb), TSPEC(fd, smx_file_t)) sep \
 ACTION(SIMCALL_FILE_OPEN, file_open, WITHOUT_ANSWER, TSPEC(result, smx_file_t), TSTRING(mount), TSTRING(path)) sep \
 ACTION(SIMCALL_FILE_CLOSE, file_close, WITHOUT_ANSWER, TINT(result), TSPEC(fd, smx_file_t)) sep \
-ACTION(SIMCALL_FILE_UNLINK, file_unlink, WITHOUT_ANSWER, TINT(result), TSPEC(fd, smx_file_t)) sep \
+ACTION(SIMCALL_FILE_UNLINK, file_unlink, WITH_ANSWER, TINT(result), TSPEC(fd, smx_file_t)) sep \
 ACTION(SIMCALL_FILE_LS, file_ls, WITHOUT_ANSWER, TSPEC(result, xbt_dict_t), TSTRING(mount), TSTRING(path)) sep \
 ACTION(SIMCALL_FILE_GET_SIZE, file_get_size, WITH_ANSWER, TSIZE(result), TSPEC(fd, smx_file_t)) sep \
 ACTION(SIMCALL_ASR_GET_PROPERTIES, asr_get_properties, WITH_ANSWER, TSPEC(result, xbt_dict_t), TSTRING(name)) sep 

diff --git a/src/surf/storage.c b/src/surf/storage.c
index 1e8c2a4..dbca4f2 100644 (file)
--- a/src/surf/storage.c
+++ b/src/surf/storage.c
@@ -81,23 +81,6 @@ static surf_action_t storage_action_ls(void *storage, const char* path)
   return action;
 }
 
-static surf_action_t storage_action_unlink(void *storage, surf_file_t fd)
-{
-  surf_action_t action = storage_action_execute(storage,0, UNLINK);
-
-  // Add memory to storage
-  ((storage_t)storage)->used_size -= fd->size;
-
-  // Remove the file from storage
-  xbt_dict_t content_dict = ((storage_t)storage)->content;
-  xbt_dict_remove(content_dict,fd->name);
-
-  free(fd->name);
-  xbt_free(fd);
-
-  return action;
-}
-
 static surf_action_t storage_action_open(void *storage, const char* mount,
                                          const char* path)
 {
@@ -112,7 +95,7 @@ static surf_action_t storage_action_open(void *storage, const char* mount,
   surf_file_t file = xbt_new0(s_surf_file_t,1);
   file->name = xbt_strdup(path);
   file->size = size;
-  file->storage = mount;
+  file->storage = xbt_strdup(mount);
 
   surf_action_t action = storage_action_execute(storage,0, OPEN);
   action->file = (void *)file;
@@ -134,6 +117,7 @@ static surf_action_t storage_action_close(void *storage, surf_file_t fd)
   }
 
   free(fd->name);
+  free(fd->storage);
   xbt_free(fd);
   surf_action_t action = storage_action_execute(storage,0, CLOSE);
   return action;
@@ -191,7 +175,6 @@ static surf_action_t storage_action_execute (void *storage, double size, e_surf_
   case OPEN:
   case CLOSE:
   case STAT:
-  case UNLINK:
   case LS:
     break;
   case READ:
@@ -496,7 +479,6 @@ static void surf_storage_model_init_internal(void)
   surf_storage_model->extension.storage.close = storage_action_close;
   surf_storage_model->extension.storage.read = storage_action_read;
   surf_storage_model->extension.storage.write = storage_action_write;
-  surf_storage_model->extension.storage.unlink = storage_action_unlink;
   surf_storage_model->extension.storage.ls = storage_action_ls;
 
   if (!storage_maxmin_system) {

diff --git a/src/surf/storage_private.h b/src/surf/storage_private.h
index 1fc8621..534b1d0 100644 (file)
--- a/src/surf/storage_private.h
+++ b/src/surf/storage_private.h
@@ -22,7 +22,7 @@ typedef struct s_mount {
 
 typedef struct surf_file {
   char *name;
-  const char* storage;
+  char *storage;
   size_t size;
 } s_surf_file_t;
 
@@ -39,7 +39,7 @@ typedef struct storage {
 } s_storage_t, *storage_t;
 
 typedef enum {
-  READ=0, WRITE, STAT, OPEN, CLOSE, UNLINK, LS
+  READ=0, WRITE, STAT, OPEN, CLOSE, LS
 } e_surf_action_storage_type_t;
 
 typedef struct surf_action_storage {

diff --git a/src/surf/workstation.c b/src/surf/workstation.c
index 31afbfa..b19db9d 100644 (file)
--- a/src/surf/workstation.c
+++ b/src/surf/workstation.c
@@ -361,12 +361,33 @@ static surf_action_t ws_action_write(void *workstation, const void* ptr,
   return model->extension.storage.write(st,  ptr, size, nmemb, fd);
 }
 
-static surf_action_t ws_action_unlink(void *workstation, surf_file_t fd)
+static int ws_file_unlink(void *workstation, surf_file_t fd)
 {
-  storage_t st = find_storage_on_mount_list(workstation, fd->storage);
-  XBT_DEBUG("UNLINK on disk '%s'",st->generic_resource.name);
-  surf_model_t model = st->generic_resource.model;
-  return model->extension.storage.unlink(st, fd);
+  if (!fd){
+    XBT_WARN("No such file descriptor. Impossible to unlink");
+    return 0;
+  } else {
+//    XBT_INFO("%s %zu", fd->storage, fd->size);
+    storage_t st = find_storage_on_mount_list(workstation, fd->storage);
+    xbt_dict_t content_dict = (st)->content;
+    /* Check if the file is on this storage */
+    if (!xbt_dict_get_or_null(content_dict, fd->name)){
+      XBT_WARN("File %s is not on disk %s. Impossible to unlink", fd->name,
+          st->generic_resource.name);
+      return 0;
+    } else {
+      XBT_DEBUG("UNLINK on disk '%s'",st->generic_resource.name);
+      st->used_size -= fd->size;
+
+      // Remove the file from storage
+      xbt_dict_remove(content_dict,fd->name);
+
+      free(fd->name);
+      free(fd->storage);
+      xbt_free(fd);
+      return 1;
+    }
+  }
 }
 
 static surf_action_t ws_action_ls(void *workstation, const char* mount,
@@ -439,7 +460,7 @@ static void surf_workstation_model_init_internal(void)
   surf_workstation_model->extension.workstation.close = ws_action_close;
   surf_workstation_model->extension.workstation.read = ws_action_read;
   surf_workstation_model->extension.workstation.write = ws_action_write;
-  surf_workstation_model->extension.workstation.unlink = ws_action_unlink;
+  surf_workstation_model->extension.workstation.unlink = ws_file_unlink;
   surf_workstation_model->extension.workstation.ls = ws_action_ls;
   surf_workstation_model->extension.workstation.get_size = ws_file_get_size;
 }