From: suter Date: Tue, 11 Jun 2013 12:59:53 +0000 (+0200) Subject: simplify the simcall stack for file unlink X-Git-Tag: v3_9_90~284 X-Git-Url: http://info.iut-bm.univ-fcomte.fr/pub/gitweb/simgrid.git/commitdiff_plain/a7708e874685bdc7a883c4c9c09f37f0e39dca44 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 f72453abf1..f6c7bc16be 100644 --- 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 cbbf2ddaeb..b15cdb58d9 100644 --- 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 9822703413..1b00eedf10 100644 --- 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 3a85d32156..0fd7e61669 100644 --- 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 6a7e59018f..29144e0d1a 100644 --- 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 1e8c2a45fd..dbca4f2466 100644 --- 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 1fc86215bb..534b1d025a 100644 --- 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 31afbfab7e..b19db9d383 100644 --- 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; }