Fix race between read-ahead and write.

Change-Id: I0ed1aca585733302b5e3840f392849e12f0b0f0d
BUG: 783313
Signed-off-by: Jeff Darcy <jdarcy@redhat.com>
Reviewed-on: http://review.gluster.com/2666
Tested-by: Gluster Build System <jenkins@build.gluster.com>
Reviewed-by: Anand Avati <avati@gluster.com>

1 files changed, 24 insertions, 7 deletions

diff --git a/xlators/performance/read-ahead/src/page.c b/xlators/performance/read-ahead/src/page.c
index 9778ef542..0c9a61853 100644
--- a/xlators/performance/read-ahead/src/page.c
+++ b/xlators/performance/read-ahead/src/page.c
@@ -175,14 +175,8 @@ ra_fault_cbk (call_frame_t *frame, void *cookie, xlator_t *this,
                 if (op_ret >= 0)
                         file->stbuf = *stbuf;
 
-                if (op_ret < 0) {
-                        page = ra_page_get (file, pending_offset);
-                        if (page)
-                                waitq = ra_page_error (page, op_ret, op_errno);
-                        goto unlock;
-                }
-
                 page = ra_page_get (file, pending_offset);
+
                 if (!page) {
                         gf_log (this->name, GF_LOG_TRACE,
                                 "wasted copy: %"PRId64"[+%"PRId64"] file=%p",
@@ -190,6 +184,29 @@ ra_fault_cbk (call_frame_t *frame, void *cookie, xlator_t *this,
                         goto unlock;
                 }
 
+                /*
+                 * "Dirty" means that the request was a pure read-ahead; it's
+                 * set for requests we issue ourselves, and cleared when user
+                 * requests are issued or put on the waitq.  "Poisoned" means
+                 * that we got a write while a read was still in flight, and we
+                 * couldn't stop it so we marked it instead.  If it's both
+                 * dirty and poisoned by the time we get here, we cancel its
+                 * effect so that a subsequent user read doesn't get data that
+                 * we know is stale (because we made it stale ourselves).  We
+                 * can't use ESTALE because that has special significance.
+                 * ECANCELED has no such special meaning, and is close to what
+                 * we're trying to indicate.
+                 */
+                if (page->dirty && page->poisoned) {
+                        op_ret = -1;
+                        op_errno = ECANCELED;
+                }
+
+                if (op_ret < 0) {
+                        waitq = ra_page_error (page, op_ret, op_errno);
+                        goto unlock;
+                }
+
                 if (page->vector) {
                         iobref_unref (page->iobref);
                         GF_FREE (page->vector);