diff options
author | Raghavendra Talur <rtalur@redhat.com> | 2016-11-02 19:51:26 +0530 |
---|---|---|
committer | Shyamsundar Ranganathan <srangana@redhat.com> | 2016-11-05 13:32:58 -0700 |
commit | e65738818dd22462ec00dda021566654d1c702b1 (patch) | |
tree | 1b11d86b657dd622c08981ea989e27da18901cc7 /api/src/glfs-fops.c | |
parent | de7fe24663713fff364cfc2b52b675e3e979ee68 (diff) |
gfapi: async fops should unref in callbacks
If fd is unref'd at the end of async call then the unref in cbks would
lead to double unref and possible crash. Removing duplicate unrefs.
Added unref only in failure cases.
A simple test case has been added to test async write case. Need to
extend the same for other async APIs too.
Details:
All glfd based calls in libgfapi, except for glfs_open and glfs_close,
behave in the same way. At the start of the operation, they take a ref
on glfd and fd. At the end of the operation, they unref it. Async calls
are a little different as they unref in the cbk function. A successfull
open call does not unref either the glfd or fd, thereby functioning as a
reference for a OPEN file object. glfs_close makes a syncop_flush call
sandwiched between a fd ref and unref(this can be removed, more on this
below), followed by a call to glfs_mark_glfd_for_deletion which unrefs
glfd and also calls glfs_fd_destroy as a release function thereby doing
a unref on fd too.
Functionally, there is no problem with how everything works when as
described above. However, it is a little non-intuitive that we need to
perform a fd_unref as a consequence of a implicit fd_ref that happens
within glfs_resolve_fd. As we perform a GF_REF_GET(glfd) at the start of
every operation, it would be worthwhile to remove the fd_ref that
glfs_resovle_fd takes and do away with explicit fd_unref()s at the end
of every operation. This is the same reason why we don't need the fd_ref
in glfs_close. This is however not in the scope of this patch.
Change-Id: I86b1d3b2ad846b16ea527d541dc82b5e90b0ba85
BUG: 1391086
Signed-off-by: Raghavendra Talur <rtalur@redhat.com>
Reviewed-on: http://review.gluster.org/15768
Smoke: Gluster Build System <jenkins@build.gluster.org>
NetBSD-regression: NetBSD Build System <jenkins@build.gluster.org>
Reviewed-by: Rajesh Joseph <rjoseph@redhat.com>
Reviewed-by: Xavier Hernandez <xhernandez@datalab.es>
Reviewed-by: soumya k <skoduri@redhat.com>
Reviewed-by: Prasanna Kumar Kalever <pkalever@redhat.com>
CentOS-regression: Gluster Build System <jenkins@build.gluster.org>
Reviewed-by: Shyamsundar Ranganathan <srangana@redhat.com>
Diffstat (limited to 'api/src/glfs-fops.c')
-rw-r--r-- | api/src/glfs-fops.c | 30 |
1 files changed, 12 insertions, 18 deletions
diff --git a/api/src/glfs-fops.c b/api/src/glfs-fops.c index 0a59a8dcf1c..ea61c8b7a43 100644 --- a/api/src/glfs-fops.c +++ b/api/src/glfs-fops.c @@ -934,10 +934,9 @@ pub_glfs_preadv_async (struct glfs_fd *glfd, const struct iovec *iovec, offset, flags, NULL); out: - if (fd) - fd_unref (fd); - if (ret) { + if (fd) + fd_unref (fd); if (glfd) GF_REF_PUT (glfd); if (gio) { @@ -1252,10 +1251,9 @@ pub_glfs_pwritev_async (struct glfs_fd *glfd, const struct iovec *iovec, ret = 0; out: - if (fd) - fd_unref (fd); - if (ret) { + if (fd) + fd_unref (fd); if (glfd) GF_REF_PUT (glfd); GF_FREE (gio); @@ -1439,10 +1437,9 @@ glfs_fsync_async_common (struct glfs_fd *glfd, glfs_io_cbk fn, void *data, subvol->fops->fsync, fd, dataonly, NULL); out: - if (fd) - fd_unref (fd); - if (ret) { + if (fd) + fd_unref (fd); if (glfd) GF_REF_PUT (glfd); GF_FREE (gio); @@ -1693,10 +1690,9 @@ pub_glfs_ftruncate_async (struct glfs_fd *glfd, off_t offset, glfs_io_cbk fn, ret = 0; out: - if (fd) - fd_unref (fd); - if (ret) { + if (fd) + fd_unref (fd); if (glfd) GF_REF_PUT (glfd); GF_FREE (gio); @@ -2552,10 +2548,9 @@ pub_glfs_discard_async (struct glfs_fd *glfd, off_t offset, size_t len, ret = 0; out: - if (fd) - fd_unref (fd); - if (ret) { + if (fd) + fd_unref (fd); if (glfd) GF_REF_PUT (glfd); GF_FREE (gio); @@ -2640,10 +2635,9 @@ pub_glfs_zerofill_async (struct glfs_fd *glfd, off_t offset, off_t len, subvol->fops->zerofill, fd, offset, len, NULL); ret = 0; out: - if (fd) - fd_unref (fd); - if (ret) { + if (fd) + fd_unref (fd); if (glfd) GF_REF_PUT (glfd); GF_FREE (gio); |