From 24f91062f571ad2dd2ac22db3b7d456a2c8bd2cb Mon Sep 17 00:00:00 2001 From: "Dr. David Alan Gilbert" Date: Mon, 27 Jan 2020 19:02:23 +0100 Subject: [PATCH 112/116] virtiofsd: Convert lo_destroy to take the lo->mutex lock itself MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RH-Author: Dr. David Alan Gilbert Message-id: <20200127190227.40942-109-dgilbert@redhat.com> Patchwork-id: 93563 O-Subject: [RHEL-AV-8.2 qemu-kvm PATCH 108/112] virtiofsd: Convert lo_destroy to take the lo->mutex lock itself Bugzilla: 1694164 RH-Acked-by: Philippe Mathieu-Daudé RH-Acked-by: Stefan Hajnoczi RH-Acked-by: Sergio Lopez Pascual From: "Dr. David Alan Gilbert" 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 Signed-off-by: Dr. David Alan Gilbert Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Dr. David Alan Gilbert (cherry picked from commit fe4c15798a48143dd6b1f58d2d3cad12206ce211) Signed-off-by: Miroslav Rezanina --- 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 eb001b9..fc15d61 100644 --- a/tools/virtiofsd/passthrough_ll.c +++ b/tools/virtiofsd/passthrough_ll.c @@ -1344,14 +1344,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) { @@ -1362,15 +1361,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); @@ -2458,13 +2466,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; @@ -2475,8 +2477,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 = { -- 1.8.3.1