* Vivek Goyal ([email protected]) wrote:
> On Fri, Jun 07, 2019 at 04:15:18PM +0100, Dr. David Alan Gilbert wrote:
> [..]
> > > Index: qemu/contrib/virtiofsd/passthrough_ll.c
> > > ===================================================================
> > > --- qemu.orig/contrib/virtiofsd/passthrough_ll.c 2019-04-25
> > > 10:49:14.103386416 -0400
> > > +++ qemu/contrib/virtiofsd/passthrough_ll.c 2019-05-30
> > > 14:02:55.598483536 -0400
> > > @@ -58,6 +58,12 @@
> > > #include <gmodule.h>
> > > #include "seccomp.h"
> > >
> > > +/* Keep track of inode posix locks for each owner. */
> > > +struct lo_inode_plock {
> > > + uint64_t lock_owner;
> > > + int fd; /* fd for OFD locks */
> > > +};
> > > +
> > > struct lo_map_elem {
> > > union {
> > > struct lo_inode *inode;
> > > @@ -86,6 +92,8 @@ struct lo_inode {
> > > struct lo_key key;
> > > uint64_t refcount; /* protected by lo->mutex */
> > > fuse_ino_t fuse_ino;
> > > + pthread_mutex_t mutex;
> >
> > If this mutex is purely for posix_locks then there's probably a better
> > name.
>
> Hi Dave,
>
> As of now it is only for posix locks. But it could be used for locking
> other inode specific data structures as well. That's why I left it with
> a generic name. But if it bothers you, I can open to change the name
> to something else.
Well the important thing is that it's not the whole lo_inode; someone
who didn't know the code might think it was.
> [..]
> > > @@ -1038,6 +1064,10 @@ static void unref_inode(struct lo_data *
> > > if (!inode->refcount) {
> > > lo_map_remove(&lo->ino_map, inode->fuse_ino);
> > > g_hash_table_remove(lo->inodes, &inode->key);
> > > + if (g_hash_table_size(inode->posix_locks)) {
> > > + warn("Hash table is not empty\n");
> > > + }
> > > + g_hash_table_destroy(inode->posix_locks);
> > > pthread_mutex_unlock(&lo->mutex);
> > > close(inode->fd);
> >
> > pthread_mutex_destroy(&inode->mutex) ?
>
> So is it necessary to call this? Is it about that if same inode is
> allocated next time and if we call pthread_mutex_init(), it might
> have issues?
>
> IOW, trying to understand why one should call pthread_mutex_destroy()
> before freeing inode.
My understanding is that it's always best to destroy a mutex before
freeing the memory it's allocated in.
Dave
> Vivek
--
Dr. David Alan Gilbert / [email protected] / Manchester, UK
