summaryrefslogtreecommitdiffstats
path: root/libglusterfs/src/stack.h
diff options
context:
space:
mode:
authorPoornima G <pgurusid@redhat.com>2016-11-21 19:57:08 +0530
committerRaghavendra G <rgowdapp@redhat.com>2016-11-28 19:59:08 -0800
commit8943c19a2ef51b6e4fa66cb57211d469fe558579 (patch)
tree5322a4092887cdbd9ba34173d1a1f9c8e981c96e /libglusterfs/src/stack.h
parenta3e5c0566a7d867d16d80ca28657238ff1008a22 (diff)
libglusterfs: Fix a read hang
Issue: ===== In certain cases, there was no unwind of read from read-ahead xlator, thus resulting in hang. RCA: ==== In certain cases, ioc_readv() issues STACK_WIND_TAIL() instead of STACK_WIND(). One such case is when inode_ctx for that file is not present (can happen if readdirp was called, and populates md-cache and serves all the lookups from cache). Consider the following graph: ... io-cache (parent) | readdir-ahead | read-ahead ... Below is the code snippet of ioc_readv calling STACK_WIND_TAIL: ioc_readv() { ... if (!inode_ctx) STACK_WIND_TAIL (frame, FIRST_CHILD (frame->this), FIRST_CHILD (frame->this)->fops->readv, fd, size, offset, flags, xdata); /* Ideally, this stack_wind should wind to readdir-ahead:readv() but it winds to read-ahead:readv(). See below for explaination. */ ... } STACK_WIND_TAIL (frame, obj, fn, ...) { frame->this = obj; /* for the above mentioned graph, frame->this will be readdir-ahead * frame->this = FIRST_CHILD (frame->this) i.e. readdir-ahead, which * is as expected */ ... THIS = obj; /* THIS will be read-ahead instead of readdir-ahead!, as obj expands * to "FIRST_CHILD (frame->this)" and frame->this was pointing * to readdir-ahead in the previous statement. */ ... fn (frame, obj, params); /* fn will call read-ahead:readv() instead of readdir-ahead:readv()! * as fn expands to "FIRST_CHILD (frame->this)->fops->readv" and * frame->this was pointing ro readdir-ahead in the first statement */ ... } Thus, the readdir-ahead's readv() implementation will be skipped, and ra_readv() will be called with frame->this = "readdir-ahead" and this = "read-ahead". This can lead to corruption / hang / other problems. But in this perticular case, when 'frame->this' and 'this' passed to ra_readv() doesn't match, it causes ra_readv() to call ra_readv() again!. Thus the logic of read-ahead readv() falls apart and leads to hang. Solution: ========= Modify STACK_WIND_TAIL() as: STACK_WIND_TAIL (frame, obj, fn, ...) { next_xl = obj /* resolve obj as the variables passed in obj macro can be overwritten in the further instrucions */ next_xl_fn = fn /* resolve fn and store in a tmp variable, before modifying any variables */ frame->this = next_xl; ... THIS = next_xl; ... next_xl_fn (frame, next_xl, params); ... } As a part of http://review.gluster.org/15901/ the caller io-cache was fixed. BUG: 1388292 Change-Id: Ie662ac8f18fa16909376f1e59387bc5b886bd0f9 Signed-off-by: Poornima G <pgurusid@redhat.com> Reviewed-on: http://review.gluster.org/15923 Smoke: Gluster Build System <jenkins@build.gluster.org> NetBSD-regression: NetBSD Build System <jenkins@build.gluster.org> Reviewed-by: Rajesh Joseph <rjoseph@redhat.com> CentOS-regression: Gluster Build System <jenkins@build.gluster.org> Reviewed-by: Raghavendra G <rgowdapp@redhat.com>
Diffstat (limited to 'libglusterfs/src/stack.h')
-rw-r--r--libglusterfs/src/stack.h8
1 files changed, 5 insertions, 3 deletions
diff --git a/libglusterfs/src/stack.h b/libglusterfs/src/stack.h
index d6d44d22396..8a7a42a171d 100644
--- a/libglusterfs/src/stack.h
+++ b/libglusterfs/src/stack.h
@@ -281,17 +281,19 @@ STACK_RESET (call_stack_t *stack)
#define STACK_WIND_TAIL(frame, obj, fn, params ...) \
do { \
xlator_t *old_THIS = NULL; \
+ xlator_t *next_xl = obj; \
+ typeof(fn) next_xl_fn = fn; \
\
- frame->this = obj; \
+ frame->this = next_xl; \
frame->wind_to = #fn; \
old_THIS = THIS; \
- THIS = obj; \
+ THIS = next_xl; \
gf_msg_trace ("stack-trace", 0, \
"stack-address: %p, " \
"winding from %s to %s", \
frame->root, old_THIS->name, \
THIS->name); \
- fn (frame, obj, params); \
+ next_xl_fn (frame, next_xl, params); \
THIS = old_THIS; \
} while (0)