io-cache: 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:
=========
Ideally, STACK_WIND_TAIL() should be modified 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);
  ...
}
But for this solution, knowing the type of variable 'next_xl_fn' is
a challenge and is not easy. Hence just modifying all the existing
callers to pass "FIRST_CHILD (this)" as obj, instead of
"FIRST_CHILD (frame->this)".

Change-Id: I179ffe3d1f154bc5a1935fd2ee44e912eb0fbb61
BUG: 1388292
Signed-off-by: Poornima G <pgurusid@redhat.com>
Reviewed-on: http://review.gluster.org/15901
Smoke: Gluster Build System <jenkins@build.gluster.org>
Reviewed-by: Raghavendra G <rgowdapp@redhat.com>
NetBSD-regression: NetBSD Build System <jenkins@build.gluster.org>
CentOS-regression: Gluster Build System <jenkins@build.gluster.org>

1 files changed, 6 insertions, 6 deletions

diff --git a/xlators/performance/io-cache/src/io-cache.c b/xlators/performance/io-cache/src/io-cache.c
index 98c37746921..df247c2318a 100644
--- a/xlators/performance/io-cache/src/io-cache.c
+++ b/xlators/performance/io-cache/src/io-cache.c
@@ -1105,16 +1105,16 @@ ioc_readv (call_frame_t *frame, xlator_t *this, fd_t *fd,
         ioc_inode = (ioc_inode_t *)(long)tmp_ioc_inode;
         if (!ioc_inode) {
                 /* caching disabled, go ahead with normal readv */
-                STACK_WIND_TAIL (frame, FIRST_CHILD (frame->this),
-                                 FIRST_CHILD (frame->this)->fops->readv, fd,
+                STACK_WIND_TAIL (frame, FIRST_CHILD (this),
+                                 FIRST_CHILD (this)->fops->readv, fd,
                                  size, offset, flags, xdata);
                 return 0;
         }
 
         if (flags & O_DIRECT) {
                 /* disable caching for this fd, if O_DIRECT is used */
-                STACK_WIND_TAIL (frame, FIRST_CHILD (frame->this),
-                                 FIRST_CHILD (frame->this)->fops->readv, fd,
+                STACK_WIND_TAIL (frame, FIRST_CHILD (this),
+                                 FIRST_CHILD (this)->fops->readv, fd,
                                  size, offset, flags, xdata);
                 return 0;
         }
@@ -1149,8 +1149,8 @@ ioc_readv (call_frame_t *frame, xlator_t *this, fd_t *fd,
 
         if (!fd_ctx_get (fd, this, NULL)) {
                 /* disable caching for this fd, go ahead with normal readv */
-                STACK_WIND_TAIL (frame, FIRST_CHILD (frame->this),
-                                 FIRST_CHILD (frame->this)->fops->readv, fd,
+                STACK_WIND_TAIL (frame, FIRST_CHILD (this),
+                                 FIRST_CHILD (this)->fops->readv, fd,
                                  size, offset, flags, xdata);
                 return 0;
         }