Skip to content

Commit

Permalink
virtiofsd: Convert lo_destroy to take the lo->mutex lock itself
Browse files Browse the repository at this point in the history
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 <vgoyal@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
  • Loading branch information
dagrh committed Jan 23, 2020
1 parent 951b312 commit fe4c157
Showing 1 changed file with 17 additions and 14 deletions.
31 changes: 17 additions & 14 deletions tools/virtiofsd/passthrough_ll.c
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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);
Expand Down Expand Up @@ -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;
Expand All @@ -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 = {
Expand Down

0 comments on commit fe4c157

Please sign in to comment.