|
|
1df6c8 |
From 30cbdf8c06145a0c290da42ecc0a7eae928200b7 Mon Sep 17 00:00:00 2001
|
|
|
1df6c8 |
From: Xavi Hernandez <xhernandez@redhat.com>
|
|
|
1df6c8 |
Date: Sun, 8 Mar 2020 18:36:45 +0100
|
|
|
1df6c8 |
Subject: [PATCH 374/375] open-behind: fix missing fd reference
|
|
|
1df6c8 |
|
|
|
1df6c8 |
Open behind was not keeping any reference on fd's pending to be
|
|
|
1df6c8 |
opened. This makes it possible that a concurrent close and en entry
|
|
|
1df6c8 |
fop (unlink, rename, ...) caused destruction of the fd while it
|
|
|
1df6c8 |
was still being used.
|
|
|
1df6c8 |
|
|
|
1df6c8 |
Upstream patch:
|
|
|
1df6c8 |
> Upstream patch link: https://review.gluster.org/c/glusterfs/+/24204
|
|
|
1df6c8 |
> Change-Id: Ie9e992902cf2cd7be4af1f8b4e57af9bd6afd8e9
|
|
|
1df6c8 |
> Fixes: bz#1810934
|
|
|
1df6c8 |
> Signed-off-by: Xavi Hernandez <xhernandez@redhat.com>
|
|
|
1df6c8 |
|
|
|
1df6c8 |
Change-Id: Ie9e992902cf2cd7be4af1f8b4e57af9bd6afd8e9
|
|
|
1df6c8 |
BUG: 1830713
|
|
|
1df6c8 |
Signed-off-by: Xavi Hernandez <xhernandez@redhat.com>
|
|
|
1df6c8 |
Reviewed-on: https://code.engineering.redhat.com/gerrit/199714
|
|
|
1df6c8 |
Tested-by: RHGS Build Bot <nigelb@redhat.com>
|
|
|
1df6c8 |
Reviewed-by: Sunil Kumar Heggodu Gopala Acharya <sheggodu@redhat.com>
|
|
|
1df6c8 |
---
|
|
|
1df6c8 |
xlators/performance/open-behind/src/open-behind.c | 27 ++++++++++++++---------
|
|
|
1df6c8 |
1 file changed, 16 insertions(+), 11 deletions(-)
|
|
|
1df6c8 |
|
|
|
1df6c8 |
diff --git a/xlators/performance/open-behind/src/open-behind.c b/xlators/performance/open-behind/src/open-behind.c
|
|
|
1df6c8 |
index 268c717..14ebc12 100644
|
|
|
1df6c8 |
--- a/xlators/performance/open-behind/src/open-behind.c
|
|
|
1df6c8 |
+++ b/xlators/performance/open-behind/src/open-behind.c
|
|
|
1df6c8 |
@@ -206,8 +206,13 @@ ob_fd_free(ob_fd_t *ob_fd)
|
|
|
1df6c8 |
if (ob_fd->xdata)
|
|
|
1df6c8 |
dict_unref(ob_fd->xdata);
|
|
|
1df6c8 |
|
|
|
1df6c8 |
- if (ob_fd->open_frame)
|
|
|
1df6c8 |
+ if (ob_fd->open_frame) {
|
|
|
1df6c8 |
+ /* If we sill have a frame it means that background open has never
|
|
|
1df6c8 |
+ * been triggered. We need to release the pending reference. */
|
|
|
1df6c8 |
+ fd_unref(ob_fd->fd);
|
|
|
1df6c8 |
+
|
|
|
1df6c8 |
STACK_DESTROY(ob_fd->open_frame->root);
|
|
|
1df6c8 |
+ }
|
|
|
1df6c8 |
|
|
|
1df6c8 |
GF_FREE(ob_fd);
|
|
|
1df6c8 |
}
|
|
|
1df6c8 |
@@ -297,6 +302,7 @@ ob_wake_cbk(call_frame_t *frame, void *cookie, xlator_t *this, int op_ret,
|
|
|
1df6c8 |
call_resume(stub);
|
|
|
1df6c8 |
}
|
|
|
1df6c8 |
|
|
|
1df6c8 |
+ /* The background open is completed. We can release the 'fd' reference. */
|
|
|
1df6c8 |
fd_unref(fd);
|
|
|
1df6c8 |
|
|
|
1df6c8 |
STACK_DESTROY(frame->root);
|
|
|
1df6c8 |
@@ -331,7 +337,9 @@ ob_fd_wake(xlator_t *this, fd_t *fd, ob_fd_t *ob_fd)
|
|
|
1df6c8 |
}
|
|
|
1df6c8 |
|
|
|
1df6c8 |
if (frame) {
|
|
|
1df6c8 |
- frame->local = fd_ref(fd);
|
|
|
1df6c8 |
+ /* We don't need to take a reference here. We already have a reference
|
|
|
1df6c8 |
+ * while the open is pending. */
|
|
|
1df6c8 |
+ frame->local = fd;
|
|
|
1df6c8 |
|
|
|
1df6c8 |
STACK_WIND(frame, ob_wake_cbk, FIRST_CHILD(this),
|
|
|
1df6c8 |
FIRST_CHILD(this)->fops->open, &ob_fd->loc, ob_fd->flags, fd,
|
|
|
1df6c8 |
@@ -345,15 +353,12 @@ void
|
|
|
1df6c8 |
ob_inode_wake(xlator_t *this, struct list_head *ob_fds)
|
|
|
1df6c8 |
{
|
|
|
1df6c8 |
ob_fd_t *ob_fd = NULL, *tmp = NULL;
|
|
|
1df6c8 |
- fd_t *fd = NULL;
|
|
|
1df6c8 |
|
|
|
1df6c8 |
if (!list_empty(ob_fds)) {
|
|
|
1df6c8 |
list_for_each_entry_safe(ob_fd, tmp, ob_fds, ob_fds_on_inode)
|
|
|
1df6c8 |
{
|
|
|
1df6c8 |
ob_fd_wake(this, ob_fd->fd, ob_fd);
|
|
|
1df6c8 |
- fd = ob_fd->fd;
|
|
|
1df6c8 |
ob_fd_free(ob_fd);
|
|
|
1df6c8 |
- fd_unref(fd);
|
|
|
1df6c8 |
}
|
|
|
1df6c8 |
}
|
|
|
1df6c8 |
}
|
|
|
1df6c8 |
@@ -365,7 +370,7 @@ ob_fd_copy(ob_fd_t *src, ob_fd_t *dst)
|
|
|
1df6c8 |
if (!src || !dst)
|
|
|
1df6c8 |
goto out;
|
|
|
1df6c8 |
|
|
|
1df6c8 |
- dst->fd = __fd_ref(src->fd);
|
|
|
1df6c8 |
+ dst->fd = src->fd;
|
|
|
1df6c8 |
dst->loc.inode = inode_ref(src->loc.inode);
|
|
|
1df6c8 |
gf_uuid_copy(dst->loc.gfid, src->loc.gfid);
|
|
|
1df6c8 |
dst->flags = src->flags;
|
|
|
1df6c8 |
@@ -509,7 +514,6 @@ ob_open_behind(call_frame_t *frame, xlator_t *this, loc_t *loc, int flags,
|
|
|
1df6c8 |
|
|
|
1df6c8 |
ob_fd->ob_inode = ob_inode;
|
|
|
1df6c8 |
|
|
|
1df6c8 |
- /* don't do fd_ref, it'll cause leaks */
|
|
|
1df6c8 |
ob_fd->fd = fd;
|
|
|
1df6c8 |
|
|
|
1df6c8 |
ob_fd->open_frame = copy_frame(frame);
|
|
|
1df6c8 |
@@ -539,15 +543,16 @@ ob_open_behind(call_frame_t *frame, xlator_t *this, loc_t *loc, int flags,
|
|
|
1df6c8 |
}
|
|
|
1df6c8 |
UNLOCK(&fd->inode->lock);
|
|
|
1df6c8 |
|
|
|
1df6c8 |
- if (!open_in_progress && !unlinked) {
|
|
|
1df6c8 |
- fd_ref(fd);
|
|
|
1df6c8 |
+ /* We take a reference while the background open is pending or being
|
|
|
1df6c8 |
+ * processed. If we finally wind the request in the foreground, then
|
|
|
1df6c8 |
+ * ob_fd_free() will take care of this additional reference. */
|
|
|
1df6c8 |
+ fd_ref(fd);
|
|
|
1df6c8 |
|
|
|
1df6c8 |
+ if (!open_in_progress && !unlinked) {
|
|
|
1df6c8 |
STACK_UNWIND_STRICT(open, frame, 0, 0, fd, xdata);
|
|
|
1df6c8 |
|
|
|
1df6c8 |
if (!conf->lazy_open)
|
|
|
1df6c8 |
ob_fd_wake(this, fd, NULL);
|
|
|
1df6c8 |
-
|
|
|
1df6c8 |
- fd_unref(fd);
|
|
|
1df6c8 |
} else {
|
|
|
1df6c8 |
ob_fd_free(ob_fd);
|
|
|
1df6c8 |
STACK_WIND(frame, default_open_cbk, FIRST_CHILD(this),
|
|
|
1df6c8 |
--
|
|
|
1df6c8 |
1.8.3.1
|
|
|
1df6c8 |
|