From dae281f7ed9fe1a8e57eaef5eeaebe44990fae8a Mon Sep 17 00:00:00 2001 From: Amar Tumballi Date: Mon, 8 Oct 2012 14:11:15 +0530 Subject: 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: 859406 Signed-off-by: Amar Tumballi Reviewed-on: http://review.gluster.org/4006 Tested-by: Amar Tumballi Original-Author: Anand Avati Reviewed-on: https://code.engineering.redhat.com/gerrit/61 Reviewed-by: Vijay Bellur Tested-by: Vijay Bellur Reviewed-on: https://code.engineering.redhat.com/gerrit/1874 --- xlators/storage/posix/src/posix-aio.c | 66 ++++++++++++++++++++++++++++--- xlators/storage/posix/src/posix-helpers.c | 32 --------------- xlators/storage/posix/src/posix.h | 2 - 3 files changed, 60 insertions(+), 40 deletions(-) diff --git a/xlators/storage/posix/src/posix-aio.c b/xlators/storage/posix/src/posix-aio.c index ac0ce870506..075fd320e7e 100644 --- a/xlators/storage/posix/src/posix-aio.c +++ b/xlators/storage/posix/src/posix-aio.c @@ -31,6 +31,47 @@ #include +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; @@ -49,7 +90,6 @@ posix_aio_readv_complete (struct posix_aio_cb *paiocb, int res, int res2) call_frame_t *frame = NULL; xlator_t *this = NULL; struct iobuf *iobuf = NULL; - struct iatt prebuf = {0,}; struct iatt postbuf = {0,}; int _fd = -1; int op_ret = -1; @@ -65,7 +105,6 @@ posix_aio_readv_complete (struct posix_aio_cb *paiocb, int res, int res2) this = frame->this; priv = this->private; iobuf = paiocb->iobuf; - prebuf = paiocb->prebuf; _fd = paiocb->fd; offset = paiocb->offset; @@ -154,7 +193,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, @@ -198,7 +237,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); @@ -304,7 +350,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, @@ -346,7 +392,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 7d419cd9222..2e7bce3ed7b 100644 --- a/xlators/storage/posix/src/posix-helpers.c +++ b/xlators/storage/posix/src/posix-helpers.c @@ -1060,35 +1060,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 ce5c94c1686..eccd4d85fa7 100644 --- a/xlators/storage/posix/src/posix.h +++ b/xlators/storage/posix/src/posix.h @@ -165,8 +165,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); -- cgit