nfs-ganesha
nfs-ganesha copied to clipboard
nfs-ganesha
Question about lock release?
Several locks or cond are initialized in ganesha code, but I don't see where to destroy? So I want to ask where it was destroyed or not needed to be destroyed. Thanks!
The lock is as follows： fsal_status_t dirmap_lru_init(struct mdcache_fsal_export *exp) rc = pthread_mutex_init(&exp->dirent_map.mtx, NULL);
fsal_status_t mdcache_fsal_create_export() PTHREAD_RWLOCK_init(&myself->mdc_exp_lock, &attrs);
void up_ready_init(struct fsal_up_vector *up_ops) PTHREAD_MUTEX_init(&up_ops->up_mutex, NULL); PTHREAD_COND_init(&up_ops->up_cond, NULL);
Those don't appear to be destroyed, it's true. They probably should be, but I don't think it ultimately makes much of a difference, since the memory is freed right after it would have been destroyed.
It might be good to destroy these. Linux consumes no hidden resource for pthread objects but other OS might.
@ffilz @dang I have encountered a new confusion, I don't understand how the memory requested in the two functions nfs4_add_clid_entry and nfs4_add_rfh_entry is released? The memory requested in the nfs4_add_clid_entry function, I saw that the nfs4_cleanup_clid_entries function will clean up. But the function nfs4_cleanup_clid_entries will only be called when grace starts. Do you not need to call it to clean up in other cases, such as grace end or stop ganesha. The memory requested by the function nfs4_add_rfh_entry, I didn't find the relevant code for release. So am I understanding it wrong? Thanks.
clid_entry_t *nfs4_add_clid_entry(char *cl_name) { clid_entry_t *new_ent = gsh_malloc(sizeof(clid_entry_t));
glist_init(&new_ent->cl_rfh_list);
(void) strlcpy(new_ent->cl_name, cl_name, sizeof(new_ent->cl_name));
glist_add(&clid_list, &new_ent->cl_list);
++clid_count;
return new_ent;
} rdel_fh_t *nfs4_add_rfh_entry(clid_entry_t *clid_ent, char *rfh_name) { rdel_fh_t *new_ent = gsh_malloc(sizeof(rdel_fh_t));
new_ent->rdfh_handle_str = gsh_strdup(rfh_name);
glist_add(&clid_ent->cl_rfh_list, &new_ent->rdfh_list);
return new_ent;
}
void nfs4_cleanup_clid_entries(void) { struct clid_entry clid_entry; / when not doing a takeover, start with an empty list */ while ((clid_entry = glist_first_entry(&clid_list, struct clid_entry, cl_list)) != NULL) { glist_del(&clid_entry->cl_list); gsh_free(clid_entry); --clid_count; } assert(clid_count == 0); atomic_store_int32_t(&reclaim_completes, 0); }
Revisit this. I think maybe some cleanup has been done, but this could be revisited for V5.x
I'm working on this....
Also covering condition variables and will also check thread/lock/condition variable attributes
Basically anything that has a pthread._init should have a corresponding pthread._destroy call.
OK, after extensive work, I have addressed almost all of the cases of pthread resource init and destroy:
https://review.gerrithub.io/c/ffilz/nfs-ganesha/+/552214
Missing:
log.h defines and statically initializes a mutex for each rate limited message. I don't see an easy way to fix that. But at least it's a limited number that will live for the lifetime of Ganesha. Probably OK. One disadvantage of them being statically initialized is they won't be initialized with the forthcoming standard Ganesha initialization...
PROXY_V3 and PROXY_V4 have some resources that aren't cleaned up that require additional work. Github issues for those:
https://github.com/nfs-ganesha/nfs-ganesha/issues/923 https://github.com/nfs-ganesha/nfs-ganesha/issues/920
Note that at least one mutex was not initialized (one of the uses of so_mutex). Linux seems OK with an all zeros initialization (the static INITIALIZERS may actually be all zeros).
Also, some research shows that at least BSD allocates resources that must be released with destroy (actually the whole object, pthread_mutex_t for example is just a pointer to a dynamically allocated mutex structure). This makes an all zeros (NULL) initialization OK (though I'm not sure how the library assures that the object is allocated once and only once - hopefully it uses an atomic test and set...).