From patchwork Fri Jun 15 17:47:29 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Denis V. Lunev\" via" X-Patchwork-Id: 10467183 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 1F7D4601C2 for ; Fri, 15 Jun 2018 17:49:04 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id E4673288EF for ; Fri, 15 Jun 2018 17:48:59 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id D92E5288FE; Fri, 15 Jun 2018 17:48:59 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.8 required=2.0 tests=BAYES_00,DKIM_SIGNED, MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI, T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 223AB288EF for ; Fri, 15 Jun 2018 17:48:58 +0000 (UTC) Received: from localhost ([::1]:48601 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fTsqI-0003wA-1Q for patchwork-qemu-devel@patchwork.kernel.org; Fri, 15 Jun 2018 13:48:58 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60312) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fTsox-0003W4-GB for qemu-devel@nongnu.org; Fri, 15 Jun 2018 13:47:38 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fTsou-0007TQ-DC for qemu-devel@nongnu.org; Fri, 15 Jun 2018 13:47:35 -0400 Received: from mail-pg0-x243.google.com ([2607:f8b0:400e:c05::243]:33723) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1fTsou-0007Sd-3y for qemu-devel@nongnu.org; Fri, 15 Jun 2018 13:47:32 -0400 Received: by mail-pg0-x243.google.com with SMTP id e11-v6so4742546pgq.0 for ; Fri, 15 Jun 2018 10:47:31 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=digitalocean.com; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=diAviQ/PQ7dUbt4CZVx0CXj9/BnW/Xzh4hlMMG5OL8k=; b=L9ShcNm9kVKBnIvTmulFp+JpRW4a9h0hly+Co3pAGsDX3TDWBzOEgCIlyIrb7LxNpk P82zs+/wsocIrc0tgNS7iXEFD9y+3WZKQcywpqTitz1JvpH07KJ63FFz+BkZnssTLJK8 rL0bDHHAOOAJqygu861PnurOUJJY0kH0xTF4A= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=diAviQ/PQ7dUbt4CZVx0CXj9/BnW/Xzh4hlMMG5OL8k=; b=j2V4D8h8EfGcjfQuLC2Z9Yl3eWp3wa1bndl3N3SV8ZyeDoBFkg9+v7GRQaK/luOIUn TxtW03vYyGb8/joPVkFvKoT+x4X9f0eGN+f64yALHqGCBINgQD32skcqXNUwIylO3QfA txjZKrSsxpJBRnD3TTncgaxKm5cucGrlGaD9K4vvPBNljEhg51XR//cFgKAdCZ3epJ2C A0yQwCS16U+jAiM8d4qjkYR2qcRBElAVGDISA94no7MNtiaX2sExIAFJYPdwcEpKr7HM mjDGyMothmbxqfQuFj66tgicxhZXg18Y6+UjPB/C2yDajfieVVV3cRmd/+lIU/dzPO+P Vi4Q== X-Gm-Message-State: APt69E1tQv8D4/kIe7fPWe5bktNla89+HVAAFBFoItOtOtdc0EqQMB6e LaVAjjpF0hsnEIpxorSwLp9R0A== X-Google-Smtp-Source: ADUXVKJ2bzqeWTJ+y6LHyf87Qta8NQot4is5qxCgwZxO/LO8QI/4MdwWl01o3KLXChK5v8disSNFlQ== X-Received: by 2002:aa7:820e:: with SMTP id k14-v6mr2979500pfi.97.1529084850800; Fri, 15 Jun 2018 10:47:30 -0700 (PDT) Received: from breakout.internal.digitalocean.com (97-120-136-47.ptld.qwest.net. [97.120.136.47]) by smtp.gmail.com with ESMTPSA id r27-v6sm18044268pfg.94.2018.06.15.10.47.30 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Fri, 15 Jun 2018 10:47:30 -0700 (PDT) Received: by breakout.internal.digitalocean.com (Postfix, from userid 1000) id 80D9D8A2A52; Fri, 15 Jun 2018 10:47:29 -0700 (PDT) To: qemu-devel@nongnu.org Date: Fri, 15 Jun 2018 10:47:29 -0700 Message-Id: <20180615174729.20544-1-naravamudan@digitalocean.com> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20180614232119.31669-1-naravamudan@digitalocean.com> References: <20180614232119.31669-1-naravamudan@digitalocean.com> X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 2607:f8b0:400e:c05::243 Subject: [Qemu-devel] [PATCH] [RFC v2] aio: properly bubble up errors from initialization X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Nishanth Aravamudan via Qemu-devel From: "Denis V. Lunev\" via" Reply-To: Nishanth Aravamudan Cc: qemu-block@nongnu.org Errors-To: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: "Qemu-devel" X-Virus-Scanned: ClamAV using ClamSMTP laio_init() can fail for a couple of reasons, which will lead to a NULL pointer dereference in laio_attach_aio_context(). To solve this, add a aio_setup_linux_aio() function which is called before aio_get_linux_aio() where it is called currently, and which propogates setup errors up. The signature of aio_get_linux_aio() was not modified, because it seems preferable to return the actual errno from the possible failing initialization calls. With respect to the error-handling in the file-posix.c, we properly bubble any errors up in raw_co_prw and in the case s of raw_aio_{,un}plug, the result is the same as if s->use_linux_aio was not set (but there is no bubbling up). In all three cases, if the setup function fails, we fallback to the thread pool and an error message is emitted. It is trivial to make qemu segfault in my testing. Set /proc/sys/fs/aio-max-nr to 0 and start a guest with aio=native,cache=directsync. With this patch, the guest successfully starts (but obviously isn't using native AIO). Setting aio-max-nr back up to a reasonable value, AIO contexts are consumed normally. Signed-off-by: Nishanth Aravamudan --- Changes from v1 -> v2: Rather than affect virtio-scsi/blk at all, make all the changes internal to file-posix.c. Thanks to Kevin Wolf for the suggested change. --- block/file-posix.c | 24 ++++++++++++++++++++++++ block/linux-aio.c | 15 ++++++++++----- include/block/aio.h | 3 +++ include/block/raw-aio.h | 2 +- stubs/linux-aio.c | 2 +- util/async.c | 15 ++++++++++++--- 6 files changed, 51 insertions(+), 10 deletions(-) diff --git a/block/file-posix.c b/block/file-posix.c index 07bb061fe4..2415d09bf1 100644 --- a/block/file-posix.c +++ b/block/file-posix.c @@ -1665,6 +1665,14 @@ static int coroutine_fn raw_co_prw(BlockDriverState *bs, uint64_t offset, type |= QEMU_AIO_MISALIGNED; #ifdef CONFIG_LINUX_AIO } else if (s->use_linux_aio) { + int rc; + rc = aio_setup_linux_aio(bdrv_get_aio_context(bs)); + if (rc != 0) { + error_report("Unable to use native AIO, falling back to " + "thread pool."); + s->use_linux_aio = 0; + return rc; + } LinuxAioState *aio = aio_get_linux_aio(bdrv_get_aio_context(bs)); assert(qiov->size == bytes); return laio_co_submit(bs, aio, s->fd, offset, qiov, type); @@ -1695,6 +1703,14 @@ static void raw_aio_plug(BlockDriverState *bs) #ifdef CONFIG_LINUX_AIO BDRVRawState *s = bs->opaque; if (s->use_linux_aio) { + int rc; + rc = aio_setup_linux_aio(bdrv_get_aio_context(bs)); + if (rc != 0) { + error_report("Unable to use native AIO, falling back to " + "thread pool."); + s->use_linux_aio = 0; + return; + } LinuxAioState *aio = aio_get_linux_aio(bdrv_get_aio_context(bs)); laio_io_plug(bs, aio); } @@ -1706,6 +1722,14 @@ static void raw_aio_unplug(BlockDriverState *bs) #ifdef CONFIG_LINUX_AIO BDRVRawState *s = bs->opaque; if (s->use_linux_aio) { + int rc; + rc = aio_setup_linux_aio(bdrv_get_aio_context(bs)); + if (rc != 0) { + error_report("Unable to use native AIO, falling back to " + "thread pool."); + s->use_linux_aio = 0; + return; + } LinuxAioState *aio = aio_get_linux_aio(bdrv_get_aio_context(bs)); laio_io_unplug(bs, aio); } diff --git a/block/linux-aio.c b/block/linux-aio.c index 88b8d55ec7..4d799f85fe 100644 --- a/block/linux-aio.c +++ b/block/linux-aio.c @@ -470,28 +470,33 @@ void laio_attach_aio_context(LinuxAioState *s, AioContext *new_context) qemu_laio_poll_cb); } -LinuxAioState *laio_init(void) +int laio_init(LinuxAioState **linux_aio) { + int rc; LinuxAioState *s; s = g_malloc0(sizeof(*s)); - if (event_notifier_init(&s->e, false) < 0) { + rc = event_notifier_init(&s->e, false); + if (rc < 0) { goto out_free_state; } - if (io_setup(MAX_EVENTS, &s->ctx) != 0) { + rc = io_setup(MAX_EVENTS, &s->ctx); + if (rc != 0) { goto out_close_efd; } ioq_init(&s->io_q); - return s; + *linux_aio = s; + return 0; out_close_efd: event_notifier_cleanup(&s->e); out_free_state: g_free(s); - return NULL; + *linux_aio = NULL; + return rc; } void laio_cleanup(LinuxAioState *s) diff --git a/include/block/aio.h b/include/block/aio.h index ae6f354e6c..8900516ac5 100644 --- a/include/block/aio.h +++ b/include/block/aio.h @@ -381,6 +381,9 @@ GSource *aio_get_g_source(AioContext *ctx); /* Return the ThreadPool bound to this AioContext */ struct ThreadPool *aio_get_thread_pool(AioContext *ctx); +/* Setup the LinuxAioState bound to this AioContext */ +int aio_setup_linux_aio(AioContext *ctx); + /* Return the LinuxAioState bound to this AioContext */ struct LinuxAioState *aio_get_linux_aio(AioContext *ctx); diff --git a/include/block/raw-aio.h b/include/block/raw-aio.h index 0e717fd475..81b90e5fc6 100644 --- a/include/block/raw-aio.h +++ b/include/block/raw-aio.h @@ -43,7 +43,7 @@ /* linux-aio.c - Linux native implementation */ #ifdef CONFIG_LINUX_AIO typedef struct LinuxAioState LinuxAioState; -LinuxAioState *laio_init(void); +int laio_init(LinuxAioState **linux_aio); void laio_cleanup(LinuxAioState *s); int coroutine_fn laio_co_submit(BlockDriverState *bs, LinuxAioState *s, int fd, uint64_t offset, QEMUIOVector *qiov, int type); diff --git a/stubs/linux-aio.c b/stubs/linux-aio.c index ed47bd443c..88ab927e35 100644 --- a/stubs/linux-aio.c +++ b/stubs/linux-aio.c @@ -21,7 +21,7 @@ void laio_attach_aio_context(LinuxAioState *s, AioContext *new_context) abort(); } -LinuxAioState *laio_init(void) +int laio_init(LinuxAioState **linux_aio) { abort(); } diff --git a/util/async.c b/util/async.c index 03f62787f2..b3fb67af3c 100644 --- a/util/async.c +++ b/util/async.c @@ -323,12 +323,21 @@ ThreadPool *aio_get_thread_pool(AioContext *ctx) } #ifdef CONFIG_LINUX_AIO -LinuxAioState *aio_get_linux_aio(AioContext *ctx) +int aio_setup_linux_aio(AioContext *ctx) { + int rc; + rc = 0; if (!ctx->linux_aio) { - ctx->linux_aio = laio_init(); - laio_attach_aio_context(ctx->linux_aio, ctx); + rc = laio_init(&ctx->linux_aio); + if (rc == 0) { + laio_attach_aio_context(ctx->linux_aio, ctx); + } } + return rc; +} + +LinuxAioState *aio_get_linux_aio(AioContext *ctx) +{ return ctx->linux_aio; } #endif