From d79660ccc65f163e0d9cf91cc13a199bec04d5f1 Mon Sep 17 00:00:00 2001 From: Xavi Hernandez Juan Date: Wed, 20 May 2020 12:55:43 +0000 Subject: [PATCH 378/379] Revert "open-behind: fix missing fd reference" This reverts commit 30cbdf8c06145a0c290da42ecc0a7eae928200b7. The patch is not complete because there have been some crash reports upstream recently after the patch was released. A new patch that should cover all corner cases is under review (), but it's a big change and it could be risky to backport it without enough testing. Since there exists a workaround to avoid the problem (disable open-behind), for now we revert the patch. Change-Id: I9cfc55623c33758cf5530b18f03c0d795b0f650b BUG: 1830713 Signed-off-by: Xavi Hernandez Reviewed-on: https://code.engineering.redhat.com/gerrit/200952 Tested-by: RHGS Build Bot Reviewed-by: Sunil Kumar Heggodu Gopala Acharya --- xlators/performance/open-behind/src/open-behind.c | 27 +++++++++-------------- 1 file changed, 11 insertions(+), 16 deletions(-) diff --git a/xlators/performance/open-behind/src/open-behind.c b/xlators/performance/open-behind/src/open-behind.c index 14ebc12..268c717 100644 --- a/xlators/performance/open-behind/src/open-behind.c +++ b/xlators/performance/open-behind/src/open-behind.c @@ -206,13 +206,8 @@ ob_fd_free(ob_fd_t *ob_fd) if (ob_fd->xdata) dict_unref(ob_fd->xdata); - if (ob_fd->open_frame) { - /* If we sill have a frame it means that background open has never - * been triggered. We need to release the pending reference. */ - fd_unref(ob_fd->fd); - + if (ob_fd->open_frame) STACK_DESTROY(ob_fd->open_frame->root); - } GF_FREE(ob_fd); } @@ -302,7 +297,6 @@ ob_wake_cbk(call_frame_t *frame, void *cookie, xlator_t *this, int op_ret, call_resume(stub); } - /* The background open is completed. We can release the 'fd' reference. */ fd_unref(fd); STACK_DESTROY(frame->root); @@ -337,9 +331,7 @@ ob_fd_wake(xlator_t *this, fd_t *fd, ob_fd_t *ob_fd) } if (frame) { - /* We don't need to take a reference here. We already have a reference - * while the open is pending. */ - frame->local = fd; + frame->local = fd_ref(fd); STACK_WIND(frame, ob_wake_cbk, FIRST_CHILD(this), FIRST_CHILD(this)->fops->open, &ob_fd->loc, ob_fd->flags, fd, @@ -353,12 +345,15 @@ void ob_inode_wake(xlator_t *this, struct list_head *ob_fds) { ob_fd_t *ob_fd = NULL, *tmp = NULL; + fd_t *fd = NULL; if (!list_empty(ob_fds)) { list_for_each_entry_safe(ob_fd, tmp, ob_fds, ob_fds_on_inode) { ob_fd_wake(this, ob_fd->fd, ob_fd); + fd = ob_fd->fd; ob_fd_free(ob_fd); + fd_unref(fd); } } } @@ -370,7 +365,7 @@ ob_fd_copy(ob_fd_t *src, ob_fd_t *dst) if (!src || !dst) goto out; - dst->fd = src->fd; + dst->fd = __fd_ref(src->fd); dst->loc.inode = inode_ref(src->loc.inode); gf_uuid_copy(dst->loc.gfid, src->loc.gfid); dst->flags = src->flags; @@ -514,6 +509,7 @@ ob_open_behind(call_frame_t *frame, xlator_t *this, loc_t *loc, int flags, ob_fd->ob_inode = ob_inode; + /* don't do fd_ref, it'll cause leaks */ ob_fd->fd = fd; ob_fd->open_frame = copy_frame(frame); @@ -543,16 +539,15 @@ ob_open_behind(call_frame_t *frame, xlator_t *this, loc_t *loc, int flags, } UNLOCK(&fd->inode->lock); - /* We take a reference while the background open is pending or being - * processed. If we finally wind the request in the foreground, then - * ob_fd_free() will take care of this additional reference. */ - fd_ref(fd); - if (!open_in_progress && !unlinked) { + fd_ref(fd); + STACK_UNWIND_STRICT(open, frame, 0, 0, fd, xdata); if (!conf->lazy_open) ob_fd_wake(this, fd, NULL); + + fd_unref(fd); } else { ob_fd_free(ob_fd); STACK_WIND(frame, default_open_cbk, FIRST_CHILD(this), -- 1.8.3.1