On 12/12/19 5:39 PM, Dr. David Alan Gilbert (git) wrote:
From: "Dr. David Alan Gilbert" <dgilb...@redhat.com>lo_destroy was relying on some implicit knowledge of the locking; we can avoid this if we create an unref_inode that doesn't take the lock and then grab it for the whole of the lo_destroy. Suggested-by: Vivek Goyal <vgo...@redhat.com> Signed-off-by: Dr. David Alan Gilbert <dgilb...@redhat.com> --- tools/virtiofsd/passthrough_ll.c | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c index 38f4948e61..c37f57157e 100644 --- a/tools/virtiofsd/passthrough_ll.c +++ b/tools/virtiofsd/passthrough_ll.c @@ -1328,14 +1328,13 @@ static void lo_unlink(fuse_req_t req, fuse_ino_t parent, const char *name) lo_inode_put(lo, &inode); }-static void unref_inode_lolocked(struct lo_data *lo, struct lo_inode *inode,- uint64_t n) +/* To be called with lo->mutex held */ +static void unref_inode(struct lo_data *lo, struct lo_inode *inode, uint64_t n) { if (!inode) { return; }- pthread_mutex_lock(&lo->mutex);assert(inode->nlookup >= n); inode->nlookup -= n; if (!inode->nlookup) { @@ -1346,15 +1345,24 @@ static void unref_inode_lolocked(struct lo_data *lo, struct lo_inode *inode, } g_hash_table_destroy(inode->posix_locks); pthread_mutex_destroy(&inode->plock_mutex); - pthread_mutex_unlock(&lo->mutex);/* Drop our refcount from lo_do_lookup() */lo_inode_put(lo, &inode); - } else { - pthread_mutex_unlock(&lo->mutex); } }+static void unref_inode_lolocked(struct lo_data *lo, struct lo_inode *inode,+ uint64_t n) +{ + if (!inode) { + return; + } + + pthread_mutex_lock(&lo->mutex); + unref_inode(lo, inode, n); + pthread_mutex_unlock(&lo->mutex); +} + static void lo_forget_one(fuse_req_t req, fuse_ino_t ino, uint64_t nlookup) { struct lo_data *lo = lo_data(req); @@ -2441,13 +2449,7 @@ static void lo_destroy(void *userdata) { struct lo_data *lo = (struct lo_data *)userdata;- /*- * Normally lo->mutex must be taken when traversing lo->inodes but - * lo_destroy() is a serialized request so no races are possible here. - * - * In addition, we cannot acquire lo->mutex since unref_inode() takes it - * too and this would result in a recursive lock. - */ + pthread_mutex_lock(&lo->mutex); while (true) { GHashTableIter iter; gpointer key, value; @@ -2458,8 +2460,9 @@ static void lo_destroy(void *userdata) }struct lo_inode *inode = value;- unref_inode_lolocked(lo, inode, inode->nlookup); + unref_inode(lo, inode, inode->nlookup); } + pthread_mutex_unlock(&lo->mutex); }static struct fuse_lowlevel_ops lo_oper = {
Reviewed-by: Philippe Mathieu-Daudé <phi...@redhat.com>
