diff options
author | Xavi Hernandez <xhernandez@redhat.com> | 2020-05-12 23:54:54 +0200 |
---|---|---|
committer | Amar Tumballi <amar@kadalu.io> | 2020-06-04 07:34:39 +0000 |
commit | d405498e377754e5465c5c8e849b9df4deb65c49 (patch) | |
tree | 4c6223b31e20b4c7b1047abbd96cf06c772d548e /libglusterfs | |
parent | cab995fd9a66083a7b8f591105f3a6d550571496 (diff) |
open-behind: rewrite of internal logic
There was a critical flaw in the previous implementation of open-behind.
When an open is done in the background, it's necessary to take a
reference on the fd_t object because once we "fake" the open answer,
the fd could be destroyed. However as long as there's a reference,
the release function won't be called. So, if the application closes
the file descriptor without having actually opened it, there will
always remain at least 1 reference, causing a leak.
To avoid this problem, the previous implementation didn't take a
reference on the fd_t, so there were races where the fd could be
destroyed while it was still in use.
To fix this, I've implemented a new xlator cbk that gets called from
fuse when the application closes a file descriptor.
The whole logic of handling background opens have been simplified and
it's more efficient now. Only if the fop needs to be delayed until an
open completes, a stub is created. Otherwise no memory allocations are
needed.
Correctly handling the close request while the open is still pending
has added a bit of complexity, but overall normal operation is simpler.
Change-Id: I6376a5491368e0e1c283cc452849032636261592
Fixes: #1225
Signed-off-by: Xavi Hernandez <xhernandez@redhat.com>
Diffstat (limited to 'libglusterfs')
-rw-r--r-- | libglusterfs/src/fd.c | 26 | ||||
-rw-r--r-- | libglusterfs/src/glusterfs/fd.h | 3 | ||||
-rw-r--r-- | libglusterfs/src/glusterfs/xlator.h | 4 | ||||
-rw-r--r-- | libglusterfs/src/libglusterfs.sym | 1 |
4 files changed, 34 insertions, 0 deletions
diff --git a/libglusterfs/src/fd.c b/libglusterfs/src/fd.c index e0767a7e61f..62606e91164 100644 --- a/libglusterfs/src/fd.c +++ b/libglusterfs/src/fd.c @@ -502,6 +502,32 @@ out: } void +fd_close(fd_t *fd) +{ + xlator_t *xl, *old_THIS; + + old_THIS = THIS; + + for (xl = fd->inode->table->xl->graph->first; xl != NULL; xl = xl->next) { + if (!xl->call_cleanup) { + THIS = xl; + + if (IA_ISDIR(fd->inode->ia_type)) { + if (xl->cbks->fdclosedir != NULL) { + xl->cbks->fdclosedir(xl, fd); + } + } else { + if (xl->cbks->fdclose != NULL) { + xl->cbks->fdclose(xl, fd); + } + } + } + } + + THIS = old_THIS; +} + +void fd_unref(fd_t *fd) { int32_t refcount = 0; diff --git a/libglusterfs/src/glusterfs/fd.h b/libglusterfs/src/glusterfs/fd.h index 28906d34e4d..3ffaaa60504 100644 --- a/libglusterfs/src/glusterfs/fd.h +++ b/libglusterfs/src/glusterfs/fd.h @@ -106,6 +106,9 @@ fd_ref(fd_t *fd); void fd_unref(fd_t *fd); +void +fd_close(fd_t *fd); + fd_t * fd_create(struct _inode *inode, pid_t pid); diff --git a/libglusterfs/src/glusterfs/xlator.h b/libglusterfs/src/glusterfs/xlator.h index c94bd75cf1c..23004ab2245 100644 --- a/libglusterfs/src/glusterfs/xlator.h +++ b/libglusterfs/src/glusterfs/xlator.h @@ -702,6 +702,8 @@ typedef size_t (*cbk_inodectx_size_t)(xlator_t *this, inode_t *inode); typedef size_t (*cbk_fdctx_size_t)(xlator_t *this, fd_t *fd); +typedef void (*cbk_fdclose_t)(xlator_t *this, fd_t *fd); + struct xlator_cbks { cbk_forget_t forget; cbk_release_t release; @@ -712,6 +714,8 @@ struct xlator_cbks { cbk_ictxmerge_t ictxmerge; cbk_inodectx_size_t ictxsize; cbk_fdctx_size_t fdctxsize; + cbk_fdclose_t fdclose; + cbk_fdclose_t fdclosedir; }; typedef int32_t (*dumpop_priv_t)(xlator_t *this); diff --git a/libglusterfs/src/libglusterfs.sym b/libglusterfs/src/libglusterfs.sym index f5e273e4ca0..c80b892df25 100644 --- a/libglusterfs/src/libglusterfs.sym +++ b/libglusterfs/src/libglusterfs.sym @@ -451,6 +451,7 @@ gf_event_unregister_close fd_anonymous fd_anonymous_with_flags fd_bind +fd_close fd_create fd_create_uint64 __fd_ctx_del |