From 64ba4fde9fca5cfc059395a444b55f57940ab06b Mon Sep 17 00:00:00 2001 From: Susant Palai Date: Mon, 22 Jul 2019 15:20:43 +0530 Subject: locks/fencing: Address hang while lock preemption The fop_wind_count can go negative when fencing is enabled on unwind path of the IO leading to hang. Also changed code so that fop_wind_count needs to be maintained only till fencing is enabled on the file. > updates: bz#1717824 > Change-Id: Icd04b42bc16cd3d50eaa581ee57233910194f480 > signed-off-by: Susant Palai (backport of https://review.gluster.org/#/c/glusterfs/+/23088/) fixes: bz#1740494 Change-Id: Icd04b42bc16cd3d50eaa581ee57233910194f480 Signed-off-by: Susant Palai --- xlators/features/locks/src/common.c | 7 ------- xlators/features/locks/src/locks.h | 1 + xlators/features/locks/src/posix.c | 41 +++++++++++++++++++++++++------------ 3 files changed, 29 insertions(+), 20 deletions(-) (limited to 'xlators/features/locks') diff --git a/xlators/features/locks/src/common.c b/xlators/features/locks/src/common.c index 6e7fb4b2f63..ff00e01c9bb 100644 --- a/xlators/features/locks/src/common.c +++ b/xlators/features/locks/src/common.c @@ -1182,13 +1182,6 @@ pl_lock_preempt(pl_inode_t *pl_inode, posix_lock_t *reqlock) list_del_init(&rw->list); list_add(&rw->list, &unwind_rw_list); } - - while (pl_inode->fop_wind_count != 0) { - gf_msg(THIS->name, GF_LOG_TRACE, 0, 0, - "waiting for fops to be drained"); - pthread_cond_wait(&pl_inode->check_fop_wind_count, - &pl_inode->mutex); - } } pthread_mutex_unlock(&pl_inode->mutex); diff --git a/xlators/features/locks/src/locks.h b/xlators/features/locks/src/locks.h index b817960a4c4..0ab2aa6cbae 100644 --- a/xlators/features/locks/src/locks.h +++ b/xlators/features/locks/src/locks.h @@ -197,6 +197,7 @@ struct __pl_inode { */ int fop_wind_count; pthread_cond_t check_fop_wind_count; + gf_boolean_t track_fop_wind_count; }; typedef struct __pl_inode pl_inode_t; diff --git a/xlators/features/locks/src/posix.c b/xlators/features/locks/src/posix.c index 7217ec02232..8fb3a8fa5ad 100644 --- a/xlators/features/locks/src/posix.c +++ b/xlators/features/locks/src/posix.c @@ -245,20 +245,19 @@ pl_track_io_fop_count(pl_local_t *local, xlator_t *this, pl_count_op_t op) if (!pl_inode) return -1; - if (pl_inode->mlock_enforced) { + if (pl_inode->mlock_enforced && pl_inode->track_fop_wind_count) { pthread_mutex_lock(&pl_inode->mutex); { if (op == DECREMENT) { pl_inode->fop_wind_count--; - if (pl_inode->fop_wind_count == 0) { + /* fop_wind_count can go negative when lock enforcement is + * enabled on unwind path of an IO. Hence the "<" comparision. + */ + if (pl_inode->fop_wind_count <= 0) { pthread_cond_broadcast(&pl_inode->check_fop_wind_count); - } - /* - Possible race where lock was enforced in the unwind path - if (pl_inode->fop_wind_count == -1) { + pl_inode->track_fop_wind_count = _gf_false; pl_inode->fop_wind_count = 0; } - */ } else { pl_inode->fop_wind_count++; } @@ -596,7 +595,8 @@ pl_discard(call_frame_t *frame, xlator_t *this, fd_t *fd, off_t offset, allowed = pl_is_fop_allowed(pl_inode, ®ion, fd, GF_FOP_DISCARD, &can_block); if (allowed == 1) { - if (pl_inode->mlock_enforced) { + if (pl_inode->mlock_enforced && + pl_inode->track_fop_wind_count) { pl_inode->fop_wind_count++; } goto unlock; @@ -721,7 +721,8 @@ pl_zerofill(call_frame_t *frame, xlator_t *this, fd_t *fd, off_t offset, allowed = pl_is_fop_allowed(pl_inode, ®ion, fd, GF_FOP_ZEROFILL, &can_block); if (allowed == 1) { - if (pl_inode->mlock_enforced) { + if (pl_inode->mlock_enforced && + pl_inode->track_fop_wind_count) { pl_inode->fop_wind_count++; } goto unlock; @@ -869,7 +870,8 @@ truncate_stat_cbk(call_frame_t *frame, void *cookie, xlator_t *this, &can_block); if (allowed == 1) { - if (pl_inode->mlock_enforced) { + if (pl_inode->mlock_enforced && + pl_inode->track_fop_wind_count) { pl_inode->fop_wind_count++; } goto unlock; @@ -1945,8 +1947,10 @@ do_blocked_rw(pl_inode_t *pl_inode) if (__rw_allowable(pl_inode, &rw->region, rw->stub->fop)) { list_del_init(&rw->list); list_add_tail(&rw->list, &wind_list); - if (pl_inode->mlock_enforced) + if (pl_inode->mlock_enforced && + pl_inode->track_fop_wind_count) { pl_inode->fop_wind_count++; + } } } } @@ -2110,7 +2114,8 @@ pl_readv(call_frame_t *frame, xlator_t *this, fd_t *fd, size_t size, allowed = pl_is_fop_allowed(pl_inode, ®ion, fd, GF_FOP_READ, &can_block); if (allowed == 1) { - if (pl_inode->mlock_enforced) { + if (pl_inode->mlock_enforced && + pl_inode->track_fop_wind_count) { pl_inode->fop_wind_count++; } goto unlock; @@ -2227,7 +2232,8 @@ pl_writev(call_frame_t *frame, xlator_t *this, fd_t *fd, struct iovec *vector, allowed = pl_is_fop_allowed(pl_inode, ®ion, fd, GF_FOP_WRITE, &can_block); if (allowed == 1) { - if (pl_inode->mlock_enforced) { + if (pl_inode->mlock_enforced && + pl_inode->track_fop_wind_count) { pl_inode->fop_wind_count++; } goto unlock; @@ -3355,6 +3361,14 @@ pl_setxattr_cbk(call_frame_t *frame, void *cookie, xlator_t *this, pthread_mutex_lock(&pl_inode->mutex); { + while (pl_inode->fop_wind_count > 0) { + gf_msg(this->name, GF_LOG_INFO, 0, 0, + "waiting for existing fops (count %d) to drain for " + "gfid %s", + pl_inode->fop_wind_count, uuid_utoa(pl_inode->gfid)); + pthread_cond_wait(&pl_inode->check_fop_wind_count, + &pl_inode->mutex); + } pl_inode->mlock_enforced = _gf_true; pl_inode->check_mlock_info = _gf_false; } @@ -4409,6 +4423,7 @@ pl_removexattr_cbk(call_frame_t *frame, void *cookie, xlator_t *this, { pl_inode->mlock_enforced = _gf_false; pl_inode->check_mlock_info = _gf_false; + pl_inode->track_fop_wind_count = _gf_true; } pthread_mutex_unlock(&pl_inode->mutex); } -- cgit