linux-aio: fixes while setting O_DIRECT flag

Linux AIO needs O_DIRECT to be set for effective operation. O_DIRECT
in turn has constraints on when it can work (offset, size alignment)

So use O_DIRECT (unless instructed by application) only when offset
and size alignments match. Else, io_submit() will happen over
non-O_DIRECT fd, effectively blocking till the completion of the IO.

Also fix a multithreading bug where detection/setting of O_DIRECT
for a request was not atomic with io_submit() of that request.

Change-Id: I190017e8bc78217429aff0714dca224cbe6f251d
BUG: 837495
Signed-off-by: Anand Avati <avati@redhat.com>
Reviewed-on: http://review.gluster.org/4006
Tested-by: Gluster Build System <jenkins@build.gluster.com>
Reviewed-by: Amar Tumballi <amarts@redhat.com>
Tested-by: Amar Tumballi <amarts@redhat.com>

3 files changed, 60 insertions, 38 deletions

diff --git a/xlators/storage/posix/src/posix-aio.c b/xlators/storage/posix/src/posix-aio.c
index c520f44f..0a776b9b 100644
--- a/xlators/storage/posix/src/posix-aio.c
+++ b/xlators/storage/posix/src/posix-aio.c
@@ -21,6 +21,47 @@
 #include <libaio.h>
 
 
+void
+__posix_fd_set_odirect (fd_t *fd, struct posix_fd *pfd, int opflags,
+			off_t offset, size_t size)
+{
+	int odirect = 0;
+	int flags = 0;
+	int ret = 0;
+
+	odirect = pfd->odirect;
+
+	if ((fd->flags|opflags) & O_DIRECT) {
+		/* if instructed, use O_DIRECT always */
+		odirect = 1;
+	} else {
+		/* else use O_DIRECT when feasible */
+		if ((offset|size) & 0xfff)
+			odirect = 0;
+		else
+			odirect = 1;
+	}
+
+	if (!odirect && pfd->odirect) {
+		flags = fcntl (pfd->fd, F_GETFL);
+		ret = fcntl (pfd->fd, F_SETFL, (flags & (~O_DIRECT)));
+		pfd->odirect = 0;
+	}
+
+	if (odirect && !pfd->odirect) {
+		flags = fcntl (pfd->fd, F_GETFL);
+		ret = fcntl (pfd->fd, F_SETFL, (flags | O_DIRECT));
+		pfd->odirect = 1;
+	}
+
+	if (ret) {
+		gf_log (THIS->name, GF_LOG_WARNING,
+			"fcntl() failed (%s). fd=%d flags=%d pfd->odirect=%d",
+			strerror (errno), pfd->fd, flags, pfd->odirect);
+	}
+}
+
+
 struct posix_aio_cb {
         struct iocb     iocb;
         call_frame_t   *frame;
@@ -142,7 +183,7 @@ posix_aio_readv (call_frame_t *frame, xlator_t *this, fd_t *fd,
 
         priv = this->private;
 
-        ret = posix_fd_ctx_get_off (fd, this, &pfd, offset);
+        ret = posix_fd_ctx_get (fd, this, &pfd);
         if (ret < 0) {
                 op_errno = -ret;
                 gf_log (this->name, GF_LOG_WARNING,
@@ -186,7 +227,14 @@ posix_aio_readv (call_frame_t *frame, xlator_t *this, fd_t *fd,
 
         iocb = &paiocb->iocb;
 
-        ret = io_submit (priv->ctxp, 1, &iocb);
+	LOCK (&fd->lock);
+	{
+		__posix_fd_set_odirect (fd, pfd, flags, offset, size);
+
+		ret = io_submit (priv->ctxp, 1, &iocb);
+	}
+	UNLOCK (&fd->lock);
+
         if (ret != 1) {
                 gf_log (this->name, GF_LOG_ERROR,
                         "io_submit() returned %d", ret);
@@ -292,7 +340,7 @@ posix_aio_writev (call_frame_t *frame, xlator_t *this, fd_t *fd,
 
         priv = this->private;
 
-        ret = posix_fd_ctx_get_off (fd, this, &pfd, offset);
+        ret = posix_fd_ctx_get (fd, this, &pfd);
         if (ret < 0) {
                 op_errno = -ret;
                 gf_log (this->name, GF_LOG_WARNING,
@@ -334,7 +382,15 @@ posix_aio_writev (call_frame_t *frame, xlator_t *this, fd_t *fd,
 	}
 
 
-        ret = io_submit (priv->ctxp, 1, &iocb);
+	LOCK (&fd->lock);
+	{
+		__posix_fd_set_odirect (fd, pfd, flags, offset,
+					iov_length (iov, count));
+
+		ret = io_submit (priv->ctxp, 1, &iocb);
+	}
+	UNLOCK (&fd->lock);
+
         if (ret != 1) {
                 gf_log (this->name, GF_LOG_ERROR,
                         "io_submit() returned %d", ret);
diff --git a/xlators/storage/posix/src/posix-helpers.c b/xlators/storage/posix/src/posix-helpers.c
index 9ccf0827..dc1c1750 100644
--- a/xlators/storage/posix/src/posix-helpers.c
+++ b/xlators/storage/posix/src/posix-helpers.c
@@ -1044,35 +1044,3 @@ posix_fd_ctx_get (fd_t *fd, xlator_t *this, struct posix_fd **pfd)
 
         return ret;
 }
-
-
-int
-posix_fd_ctx_get_off (fd_t *fd, xlator_t *this, struct posix_fd **pfd,
-                      off_t offset)
-{
-        int   ret;
-        int   flags;
-
-        LOCK (&fd->inode->lock);
-        {
-                ret = __posix_fd_ctx_get (fd, this, pfd);
-                if (ret)
-                        goto unlock;
-
-                if ((offset & 0xfff) && (*pfd)->odirect) {
-                        flags = fcntl ((*pfd)->fd, F_GETFL);
-                        ret = fcntl ((*pfd)->fd, F_SETFL, (flags & (~O_DIRECT)));
-                        (*pfd)->odirect = 0;
-                }
-
-                if (((offset & 0xfff) == 0) && (!(*pfd)->odirect)) {
-                        flags = fcntl ((*pfd)->fd, F_GETFL);
-                        ret = fcntl ((*pfd)->fd, F_SETFL, (flags | O_DIRECT));
-                        (*pfd)->odirect = 1;
-                }
-        }
-unlock:
-        UNLOCK (&fd->inode->lock);
-
-        return ret;
-}
diff --git a/xlators/storage/posix/src/posix.h b/xlators/storage/posix/src/posix.h
index 193d5588..6f96e49d 100644
--- a/xlators/storage/posix/src/posix.h
+++ b/xlators/storage/posix/src/posix.h
@@ -156,8 +156,6 @@ int posix_entry_create_xattr_set (xlator_t *this, const char *path,
                                   dict_t *dict);
 
 int posix_fd_ctx_get (fd_t *fd, xlator_t *this, struct posix_fd **pfd);
-int posix_fd_ctx_get_off (fd_t *fd, xlator_t *this, struct posix_fd **pfd,
-                          off_t off);
 void posix_fill_ino_from_gfid (xlator_t *this, struct iatt *buf);
 
 gf_boolean_t posix_special_xattr (char **pattern, char *key);