From patchwork Wed Jun 22 05:17:25 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Matthew Ruffell X-Patchwork-Id: 1646329 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: bilbo.ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=canonical.com header.i=@canonical.com header.a=rsa-sha256 header.s=20210705 header.b=nV/Pjqp6; dkim-atps=neutral Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=lists.ubuntu.com (client-ip=91.189.94.19; helo=huckleberry.canonical.com; envelope-from=kernel-team-bounces@lists.ubuntu.com; receiver=) Received: from huckleberry.canonical.com (huckleberry.canonical.com [91.189.94.19]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by bilbo.ozlabs.org (Postfix) with ESMTPS id 4LSWqW6td3z9sGF for ; Wed, 22 Jun 2022 15:17:59 +1000 (AEST) Received: from localhost ([127.0.0.1] helo=huckleberry.canonical.com) by huckleberry.canonical.com with esmtp (Exim 4.86_2) (envelope-from ) id 1o3skK-0008Jx-3R; Wed, 22 Jun 2022 05:17:44 +0000 Received: from smtp-relay-internal-0.internal ([10.131.114.225] helo=smtp-relay-internal-0.canonical.com) by huckleberry.canonical.com with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.86_2) (envelope-from ) id 1o3skG-0008Jo-Vp for kernel-team@lists.ubuntu.com; Wed, 22 Jun 2022 05:17:40 +0000 Received: from mail-pf1-f200.google.com (mail-pf1-f200.google.com [209.85.210.200]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by smtp-relay-internal-0.canonical.com (Postfix) with ESMTPS id 52C0D3F496 for ; Wed, 22 Jun 2022 05:17:40 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=canonical.com; s=20210705; t=1655875060; bh=jw/0r2MFLHuFxr0JEGCJVgD/uLHhEC9EcwEhFl0PtpA=; h=From:To:Subject:Date:Message-Id:MIME-Version; b=nV/Pjqp6317GFG+cSoa7H3h/6HNVsNjFAW5Vy02n1AJ+ROncOgfQwigbTtkluvewl jN2UkiBq92tPMhr58/oZ38XbI3ag8XX3a8gIU85ZwqxP5Sm4n6pshfRZitqp2AKlCo ZRahyJdaYaiBzciB4wQsmoFQDgC+JlXBZBpanYEyvBIDQ7XDmP4gZKAipWekkm86pb PD/IrvDAEKkMtTc8hU8/6dB21TZlM+AWj7voQ3E4rFFbM24FtcKeOg2ctQs9SxTb92 fzLvPcIa1jVsSzv4uCj6kuBUgahSjiL48/q+CHIk36xIRWpmiZTmPMetQvlrK95Jmx Pdm7khGUEMmFg== Received: by mail-pf1-f200.google.com with SMTP id y132-20020a62648a000000b0052515367fc9so3477451pfb.1 for ; Tue, 21 Jun 2022 22:17:40 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:subject:date:message-id:mime-version :content-transfer-encoding; bh=jw/0r2MFLHuFxr0JEGCJVgD/uLHhEC9EcwEhFl0PtpA=; b=ZC6UCrYjv0MX84xpWjy8A5eCJoBV+iw4GYo9VQALv2kACWqD5c554QyzRB1hvllxgl WDjtN6/kXWMj4fx5Rn0shQV7ebeGS0uUb+ImZ0oF19nzK8tTR4/8I05T+IJUqw+0ZFKE O03RzNypYZ3DtLAsXBwrFAQSL+AMqCMQ+7Up55Fvu8Zun0nf7UFDHpaugA9j4QqtGTf4 ORDNCPfE6rNth9fasLAJxCCqjA7ytXnADxqMigp32Uoo6qj+OGPNlZlksIglCqpQkj5D RmvcnNdkfKDBCaHLSWkS4Hg5inqB/xYhV5PgnonCDQsgsMu6oe5B9+F6jcqBUf44e84F LsIw== X-Gm-Message-State: AJIora8FJ7jW9P3dQ7/91KIB3Oy397G/bcLTlfQ19gYufH0PUWmSV9AQ u1mdHzaLGm2efgUkchM/bLH0LH+FZ/OATzPoGO0hy6NQnhiIuJ1Fgnmy+KxcKo2gKazoU1QdHE5 4Z8g1OZhJ4+LN+yJ4RPhXQ90XbogxHOPA1zyi1eV4qA== X-Received: by 2002:a63:5a13:0:b0:3c6:3d28:87e5 with SMTP id o19-20020a635a13000000b003c63d2887e5mr1474445pgb.452.1655875058962; Tue, 21 Jun 2022 22:17:38 -0700 (PDT) X-Google-Smtp-Source: AGRyM1veRcZOwbUUs4X48I0/x+pMlZL0hc3pssc9YPqEpmriQ1YLuHUoWJnYpgLSd7lvz5IwBgVOWw== X-Received: by 2002:a63:5a13:0:b0:3c6:3d28:87e5 with SMTP id o19-20020a635a13000000b003c63d2887e5mr1474423pgb.452.1655875058525; Tue, 21 Jun 2022 22:17:38 -0700 (PDT) Received: from desktop.. (125-239-70-54-fibre.sparkbb.co.nz. [125.239.70.54]) by smtp.gmail.com with ESMTPSA id c200-20020a624ed1000000b005251b1fde90sm6450316pfb.219.2022.06.21.22.17.37 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 21 Jun 2022 22:17:38 -0700 (PDT) From: Matthew Ruffell To: kernel-team@lists.ubuntu.com Subject: [SRU][F][I][J][PATCH 0/6] nbd: requests can become stuck when disconnecting from server with qemu-nbd Date: Wed, 22 Jun 2022 17:17:25 +1200 Message-Id: <20220622051731.23563-1-matthew.ruffell@canonical.com> X-Mailer: git-send-email 2.34.1 MIME-Version: 1.0 X-BeenThere: kernel-team@lists.ubuntu.com X-Mailman-Version: 2.1.20 Precedence: list List-Id: Kernel team discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: kernel-team-bounces@lists.ubuntu.com Sender: "kernel-team" BugLink: https://bugs.launchpad.net/bugs/1896350 [Impact] After 2516ab1("nbd: only clear the queue on device teardown"), present in 4.12-rc1 onward, the ioctl NBD_CLEAR_SOCK can no longer clear requests currently being processed. This change was made to fix a race between using the NBD_CLEAR_SOCK ioctl to clear requests, and teardown of the device clearing requests. This worked for the most part, as several years ago systemd was not set up to watch nbd devices for changes in their state. But after: commit f82abfcda58168d9f667e2094d438763531d3fa6 From: Tony Asleson Date: Fri, 8 Feb 2019 15:47:10 -0600 Subject: rules: watch metadata changes on nbd devices Link: https://github.com/systemd/systemd/commit/f82abfcda58168d9f667e2094d438763531d3fa6 in systemd v242-rc1, nbd* devices were added to a udev rule to watch those devices for changes with the inotify subsystem. From man udev: > watch > Watch the device node with inotify; when the node is closed after being > opened for writing, a change uevent is synthesized. > > nowatch > Disable the watching of a device node with inotify. This changed the behaviour of device teardown, since systemd now keeps tabs on the device with inotify, outstanding requests cannot be cleared as nbd_xmit_timeout() will always return 'BLK_EH_RESET_TIMER', and requests get stuck, never to complete, because a disconnect has occurred, and never to timeout, as their timers keep being reset. Symptoms of this issue is that the nbd subsystem gets stuck with messages like: block nbd15: NBD_DISCONNECT block nbd15: Send disconnect failed -32 ... block nbd15: Possible stuck request 000000007fcf62ba: control (read@523915264,24576B). Runtime 30 seconds ... block nbd15: Possible stuck request 000000007fcf62ba: control (read@523915264,24576B). Runtime 150 seconds ... INFO: task qemu-nbd:1267 blocked for more than 120 seconds. Not tainted 5.15.0-23-generic #23-Ubuntu "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. task:qemu-nbd state:D stack: 0 pid: 1267 ppid: 1 flags:0x00000002 Call Trace: __schedule+0x23d/0x590 ? call_rcu+0xe/0x10 schedule+0x4e/0xb0 blk_mq_freeze_queue_wait+0x69/0xa0 ? wait_woken+0x70/0x70 blk_mq_freeze_queue+0x1b/0x30 nbd_add_socket+0x76/0x1f0 [nbd] __nbd_ioctl+0x18b/0x340 [nbd] ? security_capable+0x3d/0x60 nbd_ioctl+0x81/0xb0 [nbd] blkdev_ioctl+0x12e/0x270 ? __fget_files+0x86/0xc0 block_ioctl+0x46/0x50 __x64_sys_ioctl+0x91/0xc0 do_syscall_64+0x5c/0xc0 entry_SYSCALL_64_after_hwframe+0x44/0xae Additionally, in syslog you will also see systemd-udevd get stuck: systemd-udevd[419]: nbd15: Worker [2004] processing SEQNUM=5661 is taking a long time $ ps aux ... 419 1194 root D 0.1 systemd-udevd - We can workaround the issue by adding a higher priority udev rule to not watch nbd* devices. $ cat << EOF >> /etc/udev/rules.d/97-nbd-device.rules # Disable inotify watching of change events for NBD devices ACTION=="add|change", KERNEL=="nbd*", OPTIONS:="nowatch" EOF $ sudo udevadm control --reload-rules $ sudo udevadm trigger [Fix] The fix relies on infrastructure provided by the flag NBD_CMD_INFLIGHT, which was introduced in 5.16, and added to in 5.19. We need to backport all commits related to NBD_CMD_INFLIGHT to our kernels for the fix to be effective. For Focal, Impish and Jammy: commit 4e6eef5dc25b528e08ac5b5f64f6ca9d9987241d Author: Yu Kuai Date: Thu Sep 16 17:33:44 2021 +0800 Subject: nbd: don't handle response without a corresponding request message Link: https://github.com/torvalds/linux/commit/4e6eef5dc25b528e08ac5b5f64f6ca9d9987241d commit 07175cb1baf4c51051b1fbd391097e349f9a02a9 Author: Yu Kuai Date: Thu Sep 16 17:33:45 2021 +0800 Subject: nbd: make sure request completion won't concurrent Link: https://github.com/torvalds/linux/commit/07175cb1baf4c51051b1fbd391097e349f9a02a9 commit 2895f1831e911ca87d4efdf43e35eb72a0c7e66e Author: Yu Kuai Date: Sat May 21 15:37:46 2022 +0800 Subject: nbd: don't clear 'NBD_CMD_INFLIGHT' flag if request is not completed Link: https://github.com/torvalds/linux/commit/2895f1831e911ca87d4efdf43e35eb72a0c7e66e commit 09dadb5985023e27d4740ebd17e6fea4640110e5 Author: Yu Kuai Date: Sat May 21 15:37:47 2022 +0800 Subject: nbd: fix io hung while disconnecting device Link: https://github.com/torvalds/linux/commit/09dadb5985023e27d4740ebd17e6fea4640110e5 For Focal only (dependency commits): commit 7b11eab041dacfeaaa6d27d9183b247a995bc16d Author: Keith Busch Date: Fri May 29 07:51:59 2020 -0700 Subject: blk-mq: blk-mq: provide forced completion method Link: https://github.com/torvalds/linux/commit/7b11eab041dacfeaaa6d27d9183b247a995bc16d commit 15f73f5b3e5958f2d169fe13c420eeeeae07bbf2 Author: Christoph Hellwig Date: Thu Jun 11 08:44:47 2020 +0200 Subject: blk-mq: move failure injection out of blk_mq_complete_request Link: https://github.com/torvalds/linux/commit/15f73f5b3e5958f2d169fe13c420eeeeae07bbf2 I want to talk about the backport of "blk-mq: move failure injection out of blk_mq_complete_request" for Focal, since it changes a number of drivers using the blk_mq_complete_request() call, and can be considered a large regression risk. Now, blk_should_fake_timeout() relies on CONFIG_FAIL_IO_TIMEOUT to be enabled, as well as QUEUE_FLAG_FAIL_IO being present in the blk subsystem. CONFIG_FAIL_IO_TIMEOUT is not enabled on Ubuntu kernels, and thus blk_should_fake_timeout() just returns false, and is more or less a nop on our kernels. Because of this, if (likely(!blk_should_fake_timeout(req->q))), is really just if(true), and I did think about simply backporting that to the nbd patch "nbd: don't clear 'NBD_CMD_INFLIGHT' flag if request is not completed" but after talking with Jay, we decided backporting the entire patch was the best way to proceed if any of our users wish to use CONFIG_FAIL_IO_TIMEOUT on a custom kernel build with the nbd subsystem. Note: Bionic is also affected, but the backport list to the 4.15 kernel is unreasonable, and due to systemd in Bionic not having the udev rule set to watch nbd* devices by default, Bionic is not directly affected by the issue. Users would only see the problem if they add nbd* to the watch list in /usr/lib/udev/rules.d/60-block.rules, or if they had a privileged 20.04 or later LXC container running using the hosts nbd kernel module. Because of this Bionic will be Won't Fix. [Testcase] The issue can be easily reproduced with: $ sudo apt install qemu-utils $ cat << EOF >> reproducer.sh #!/bin/bash sudo modprobe nbd while : do qemu-img create -f qcow2 foo.img 500M sudo qemu-nbd --disconnect /dev/nbd15 || true sudo qemu-nbd --connect=/dev/nbd15 --cache=writeback --format=qcow2 foo.img sudo mkfs.ext4 -L root -O "^64bit" -E nodiscard /dev/nbd15 sudo qemu-nbd --disconnect /dev/nbd15 done EOF $ chmod +x reproducer.sh $ yes | ./reproducer.sh On the Ubuntu kernels, you will see the nbd subsystem hang within 30 seconds or so, and the kernel log will be filled with stuck request messages and hung task timeouts after the 120 second mark. Test kernels are available in the following ppa: https://launchpad.net/~mruffell/+archive/ubuntu/sf333142-test If you install these test kernels, and re-run the reproducer, your system will be perfectly stable for many hours with no issues. I have torture tested the backport to the Focal kernel for more than 4 hours with no issues observed. [Where problems can occur] Due to needing to backport the infrastructure for NBD_CMD_INFLIGHT, this change to the NBD subsystem is quite large, and changes how requests are processed and accounted for, depending if they are still outstanding or not. For Impish and Jammy, these risks are limited to the NBD subsystem itself. On Focal, the risk of regression is slightly larger due to the backport of "blk-mq: move failure injection out of blk_mq_complete_request", but is somewhat mitigated by blk_should_fake_timeout() being a NOP due to CONFIG_FAIL_IO_TIMEOUT being disabled on the Focal kernel. If a regression were to occur, users may see NBD requests fail, occur in the incorrect order, or cleared at incorrect times. There is no way to disable NBD_CMD_INFLIGHT during runtime, and users would need to revert to an older kernel while a fix is made. [Other info] My bug report to upstream can be found in the following mailing list thread: https://lkml.org/lkml/2022/4/22/61 You may be wondering why we cannot simply just backport "nbd: fix io hung while disconnecting device" and resolve the issue with a 1 line change. I actually built test kernels for this scenario to see if it was possible, and they are available here: https://launchpad.net/~mruffell/+archive/ubuntu/sf333142-test-single While it did sort of fix the issue, that is, connect and disconnect from nbd devices lasted longer than the 30 seconds the kernels lasted previously, after about 10 minutes of torture testing, I started experiencing race conditions of requests completing multiple times, leading to the following use after frees: Jammy 5.15 test kernel: https://paste.ubuntu.com/p/FSM6DrgjTy/ Impish 5.13 test kernel: https://paste.ubuntu.com/p/86tNfsM7Vs/ Focal 5.4 test kernel: https://paste.ubuntu.com/p/zzVzWx23sb/ This was similar to a mailing list thread I found previously: https://groups.google.com/g/syzkaller-bugs/c/jhvr3Yv_QH8/m/DgGG4xYFEQAJ https://lore.kernel.org/all/000000000000d0df7f058f625d13@google.com/T/ Keith Busch identified this as a double completion of the same request, which is resolved through the NBD_CMD_INFLIGHT infrastructure. Hence, we cannot just pull in the single line change, we need NBD_CMD_INFLIGHT infrastructure for a complete fix. Christoph Hellwig (1): blk-mq: move failure injection out of blk_mq_complete_request Keith Busch (1): blk-mq: blk-mq: provide forced completion method Yu Kuai (4): nbd: don't handle response without a corresponding request message nbd: make sure request completion won't concurrent nbd: don't clear 'NBD_CMD_INFLIGHT' flag if request is not completed nbd: fix io hung while disconnecting device block/blk-mq.c | 27 ++++++------------ block/blk-timeout.c | 6 ++-- block/blk.h | 8 ------ block/bsg-lib.c | 5 +++- drivers/block/loop.c | 6 ++-- drivers/block/mtip32xx/mtip32xx.c | 3 +- drivers/block/nbd.c | 46 +++++++++++++++++++++++++++++-- drivers/block/null_blk_main.c | 3 +- drivers/block/skd_main.c | 9 ++++-- drivers/block/virtio_blk.c | 3 +- drivers/block/xen-blkfront.c | 3 +- drivers/md/dm-rq.c | 3 +- drivers/mmc/core/block.c | 6 ++-- drivers/nvme/host/nvme.h | 3 +- drivers/s390/block/dasd.c | 2 +- drivers/s390/block/scm_blk.c | 3 +- drivers/scsi/scsi_lib.c | 12 ++------ include/linux/blk-mq.h | 11 +++++++- 18 files changed, 99 insertions(+), 60 deletions(-) Acked-by: Tim Gardner Acked-by: Bartlomiej Zolnierkiewicz