From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lj1-f195.google.com (mail-lj1-f195.google.com [209.85.208.195]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 9F800469710 for ; Wed, 10 Jun 2020 12:53:21 +0300 (MSK) Received: by mail-lj1-f195.google.com with SMTP id y11so1663843ljm.9 for ; Wed, 10 Jun 2020 02:53:21 -0700 (PDT) From: Cyrill Gorcunov Date: Wed, 10 Jun 2020 12:53:14 +0300 Message-Id: <20200610095314.333247-1-gorcunov@gmail.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [Tarantool-patches] [PATCH v2] fio/coio: handle partial writes List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: tml Cc: Vladislav Shpilevoy , Alexander Turenko Writing less bytes than requested is perfectly fine. In turn out that fio.write/pwrite api simply returns 'true' even if only some part of a buffer has been written. Thus make coio_write and coio_pwrite to write the whole data in a cycle. Note in most situations there will be only one pass, partial writes are really the rare cases. Note that we're not handling nonblocking writes here (which could return EAGAIN) simply because we need an other api which would accept timeouts. Fixes #4651 Signed-off-by: Cyrill Gorcunov --- v2 by Vlad: - drop void* arithmetics - test comment style fixups - fix typo in changelog issue https://github.com/tarantool/tarantool/issues/4651 branch gorcunov/gh-4651-partial-write-2 src/lib/core/coio_file.c | 62 +++++++++++++++++++++++++++------ src/lib/core/errinj.h | 1 + test/app/fio.result | 75 ++++++++++++++++++++++++++++++++++++++++ test/app/fio.test.lua | 23 ++++++++++++ test/box/errinj.result | 1 + 5 files changed, 151 insertions(+), 11 deletions(-) diff --git a/src/lib/core/coio_file.c b/src/lib/core/coio_file.c index e2345567c..0f21380c5 100644 --- a/src/lib/core/coio_file.c +++ b/src/lib/core/coio_file.c @@ -164,10 +164,30 @@ coio_file_close(int fd) ssize_t coio_pwrite(int fd, const void *buf, size_t count, off_t offset) { - INIT_COEIO_FILE(eio); - eio_req *req = eio_write(fd, (void *) buf, count, offset, - 0, coio_complete, &eio); - return coio_wait_done(req, &eio); + ssize_t left = count, pos = 0, res, chunk; + eio_req *req; + + while (left > 0) { + INIT_COEIO_FILE(eio); + chunk = left; + + ERROR_INJECT(ERRINJ_COIO_WRITE_CHUNK, { + chunk = 1; + }); + + req = eio_write(fd, (char *)buf + pos, chunk, + offset + pos, EIO_PRI_DEFAULT, + coio_complete, &eio); + res = coio_wait_done(req, &eio); + if (res < 0) { + pos = -1; + break; + } else { + left -= res; + pos += res; + } + } + return pos; } ssize_t @@ -201,6 +221,11 @@ static void coio_do_write(eio_req *req) { struct coio_file_task *eio = (struct coio_file_task *)req->data; + + ERROR_INJECT(ERRINJ_COIO_WRITE_CHUNK, { + eio->write.count = 1; + }); + req->result = write(eio->write.fd, eio->write.buf, eio->write.count); eio->errorno = errno; } @@ -208,13 +233,28 @@ coio_do_write(eio_req *req) ssize_t coio_write(int fd, const void *buf, size_t count) { - INIT_COEIO_FILE(eio); - eio.write.buf = buf; - eio.write.count = count; - eio.write.fd = fd; - eio_req *req = eio_custom(coio_do_write, 0, - coio_complete, &eio); - return coio_wait_done(req, &eio); + ssize_t left = count, pos = 0, res; + eio_req *req; + + while (left > 0) { + INIT_COEIO_FILE(eio); + + eio.write.buf = (char *)buf + pos; + eio.write.count = left; + eio.write.fd = fd; + + req = eio_custom(coio_do_write, EIO_PRI_DEFAULT, + coio_complete, &eio); + res = coio_wait_done(req, &eio); + if (res < 0) { + pos = -1; + break; + } else { + left -= res; + pos += res; + } + } + return pos; } static void diff --git a/src/lib/core/errinj.h b/src/lib/core/errinj.h index 79073f529..261ab22a8 100644 --- a/src/lib/core/errinj.h +++ b/src/lib/core/errinj.h @@ -145,6 +145,7 @@ struct errinj { _(ERRINJ_VY_WRITE_ITERATOR_START_FAIL, ERRINJ_BOOL, {.bparam = false})\ _(ERRINJ_VY_RUN_OPEN, ERRINJ_INT, {.iparam = -1})\ _(ERRINJ_AUTO_UPGRADE, ERRINJ_BOOL, {.bparam = false})\ + _(ERRINJ_COIO_WRITE_CHUNK, ERRINJ_BOOL, {.bparam = false}) \ ENUM0(errinj_id, ERRINJ_LIST); extern struct errinj errinjs[]; diff --git a/test/app/fio.result b/test/app/fio.result index 783fa4fab..8a08755a2 100644 --- a/test/app/fio.result +++ b/test/app/fio.result @@ -776,6 +776,12 @@ file5 = fio.pathjoin(tree, 'file.5') file6 = fio.pathjoin(tree, 'file.6') --- ... +file7 = fio.pathjoin(tree, 'file.7') +--- +... +file8 = fio.pathjoin(tree, 'file.8') +--- +... fh1 = fio.open(file1, { 'O_RDWR', 'O_TRUNC', 'O_CREAT' }, tonumber('0777', 8)) --- ... @@ -865,6 +871,75 @@ fh6:close() --- - true ... +-- +-- gh-4651: Test partial write/pwrite via one byte transfer. +-- +fh7 = fio.open(file7, { 'O_RDWR', 'O_TRUNC', 'O_CREAT' }, tonumber('0644', 8)) +--- +... +errinj.set('ERRINJ_COIO_WRITE_CHUNK', true) +--- +- ok +... +fh7:write("one byte transfer, write") +--- +- true +... +errinj.set('ERRINJ_COIO_WRITE_CHUNK', false) +--- +- ok +... +fh7:seek(0) +--- +- 0 +... +fh7:read() +--- +- one byte transfer, write +... +fh7:close() +--- +- true +... +fh8 = fio.open(file8, { 'O_RDWR', 'O_TRUNC', 'O_CREAT' }, tonumber('0644', 8)) +--- +... +errinj.set('ERRINJ_COIO_WRITE_CHUNK', true) +--- +- ok +... +fh8:pwrite("one byte transfer, ", 0) +--- +- true +... +fh8:seek(0) +--- +- 0 +... +fh8:read() +--- +- 'one byte transfer, ' +... +fh8:pwrite("pwrite", 19) +--- +- true +... +errinj.set('ERRINJ_COIO_WRITE_CHUNK', false) +--- +- ok +... +fh8:seek(0) +--- +- 0 +... +fh8:read() +--- +- one byte transfer, pwrite +... +fh8:close() +--- +- true +... res, err = fio.copyfile(fio.pathjoin(tmp1, 'not_exists.txt'), tmp1) --- ... diff --git a/test/app/fio.test.lua b/test/app/fio.test.lua index 6825b882f..92ba3ff20 100644 --- a/test/app/fio.test.lua +++ b/test/app/fio.test.lua @@ -251,6 +251,8 @@ file3 = fio.pathjoin(tree, 'file.3') file4 = fio.pathjoin(tree, 'file.4') file5 = fio.pathjoin(tree, 'file.5') file6 = fio.pathjoin(tree, 'file.6') +file7 = fio.pathjoin(tree, 'file.7') +file8 = fio.pathjoin(tree, 'file.8') fh1 = fio.open(file1, { 'O_RDWR', 'O_TRUNC', 'O_CREAT' }, tonumber('0777', 8)) fh1:write("gogo") @@ -280,6 +282,27 @@ fh6:seek(0) fh6:read() fh6:close() +-- +-- gh-4651: Test partial write/pwrite via one byte transfer. +-- +fh7 = fio.open(file7, { 'O_RDWR', 'O_TRUNC', 'O_CREAT' }, tonumber('0644', 8)) +errinj.set('ERRINJ_COIO_WRITE_CHUNK', true) +fh7:write("one byte transfer, write") +errinj.set('ERRINJ_COIO_WRITE_CHUNK', false) +fh7:seek(0) +fh7:read() +fh7:close() +fh8 = fio.open(file8, { 'O_RDWR', 'O_TRUNC', 'O_CREAT' }, tonumber('0644', 8)) +errinj.set('ERRINJ_COIO_WRITE_CHUNK', true) +fh8:pwrite("one byte transfer, ", 0) +fh8:seek(0) +fh8:read() +fh8:pwrite("pwrite", 19) +errinj.set('ERRINJ_COIO_WRITE_CHUNK', false) +fh8:seek(0) +fh8:read() +fh8:close() + res, err = fio.copyfile(fio.pathjoin(tmp1, 'not_exists.txt'), tmp1) res err:match("failed to copy") ~= nil diff --git a/test/box/errinj.result b/test/box/errinj.result index a066bc4bc..1166f2b30 100644 --- a/test/box/errinj.result +++ b/test/box/errinj.result @@ -48,6 +48,7 @@ evals - ERRINJ_BUILD_INDEX_DELAY: false - ERRINJ_CHECK_FORMAT_DELAY: false - ERRINJ_COIO_SENDFILE_CHUNK: -1 + - ERRINJ_COIO_WRITE_CHUNK: false - ERRINJ_DYN_MODULE_COUNT: 0 - ERRINJ_FIBER_MADVISE: false - ERRINJ_FIBER_MPROTECT: -1 -- 2.26.2