From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f54.google.com (mail-wm1-f54.google.com [209.85.128.54]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 02B6B1FD4 for ; Sun, 6 Sep 2026 13:22:46 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.54 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1788700968; cv=none; b=dOokO1mRbkWT56Jvg/NW6kyXbWOKgZvKJ0K1b7++/NNCeGLc9o+EDdbFV4bWAgiMrOzZPFTQMcmqvZSuZraWHzFn25ve5AeMlc2VIjoPSzW0c/Q/Vmm1FIToXQ2I2+HnPj4VyxLmy9vk1648WhcXQLjkuKB99O/5kFasv/Kzycg= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1788700968; c=relaxed/simple; bh=TN48gfAD+iP/BbuTXiw3TGQX/mZeORvIfLXRz9gxCGI=; h=From:To:Cc:Subject:In-Reply-To:References:Date:Message-ID: MIME-Version:Content-Type; b=O8w5yEVdl0LIJMVpt+OYyAsG4gThYAYN6fO2v0IBmqrBUvT9IwU2aKMU+6c6pHQmnG7lZlEq9PCU9LKr9piloERnCi9RlsqqK0/Y14Ns6iQDB+tpAkoOwCd0pKDPgnl52fz8iR+mAH6y82QNTuG+/Po8GD2GOTJKYgaRrH2Wbo0= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=fgIlrYA7; arc=none smtp.client-ip=209.85.128.54 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="fgIlrYA7" Received: by mail-wm1-f54.google.com with SMTP id 5b1f17b1804b1-49556f97a9dso20789295e9.1 for ; Sun, 06 Sep 2026 06:22:46 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1788700965; x=1789305765; darn=vger.kernel.org; h=content-type:mime-version:message-id:date:references:in-reply-to :subject:cc:to:from:from:to:cc:subject:date:message-id:reply-to :content-type; bh=xGMx1hxcqQv5YhxgiEUavmBXn/V3/g7fRKgYE76rark=; b=fgIlrYA7O5H2r01yffxpZDjG0c+aPHwsVqzpKAybJTZ007Ud0u+5MCjvK3IuecRvHQ E1dkfrGEQxnZZ5ajonXR3BsR9Rej8HI2ZE5haECaAUudYoysuK5KWPbeT/xpyP9cf8MU HgPLDHj6Y6fK6Yy6H8n3oFsfoDM1ngAq/WfSgaEzbLW/83UNn9wrBfv5lSFANNnz2zGQ NuDzyGqTNrhIYprmr8CX+mK4ng1OXpO00yBSBFDpbbBeS394eBE7osKEyHvbvcmDpMJQ xSYQNizCbz0a3gw85fHZNcS5TmTcyfDavngt8sw4yAMFkUdnsT/Sqh8Q8ZoSeelCaEQj wq+A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1788700965; x=1789305765; h=content-type:mime-version:message-id:date:references:in-reply-to :subject:cc:to:from:x-gm-gg:x-gm-message-state:from:to:cc:subject :date:message-id:reply-to:content-type; bh=xGMx1hxcqQv5YhxgiEUavmBXn/V3/g7fRKgYE76rark=; b=Hiqt6MCPobr2hsAutBqUfabvpXgZkXgC56OLSP+xsYfz8Nb2oQoOOTj15769J4gjLH V//b3ihbqD/tqeNYXzmrTEl5ZGdRxp9E4k0wR8QK8LLvggGZm91ywKWZxmY5KPvvRHWX 9CJVaNiXP4xOcqX8SzBbKr+ikawTCSXA8kmf+4j3/sDVrNbScqkP6D5vP4nx0YoUATJI jCR2ZwUlgqd+V4L8Gp09bIpoeBrfENJcyilBF22yzhVtPCxjgM0ybeGaGkXr9Q3U1kr7 0jKb155ojOLeyhqGVGLEepryJO23qsdY2Z8++EqYtx+GqHnbdJxCVG8DbV61o+BWUvJU m1VQ== X-Forwarded-Encrypted: i=1; AKwUvBzbHXzAr6CxTU170NreGJ+P/26jNzatnciNf9p15gEMbMZ8bM93YvQTAppqsUGro/hNg+FrMfVbzmZeaA==@vger.kernel.org X-Gm-Message-State: AFuF++nTRDX3LEemzNrE75R3A1wyu9xcenJQz3ejWJXfkWXtFL/y/tLe qhBXr5GUiHMWTfJJ8LvFcUFIiApCe7C8moZW0Nk0LcUIEyCbaDWWoznJ X-Gm-Gg: AYBFou1IEcRKoqOPQQ9qzBaYDlWGQ40bkuVuX2DReARuP4D5dtPExbUKcL6PKwJfMEp tVQeYCotOFmwTRwdTL0wURf7cD1BDYwOX/o0CmmGT+KKuv+KNbgpDqmx0wRd8+LD+aVHp/skiRL uu76rrPBhQMoXd8dncRAprOv4SjxGO5H5lFO0htM48QNDUQ6cfMIj5cXcG5ftIULtJAu1xvwoE9 V/LfY5lO9ZFdlli40Vg39PmnNVpxXv8919rYEMpdoiByjKjp+7RQZk9kcw7We+E/ryv0WjqVnCI E8G4vuFRHnNtDg9ob0HOAhhfsDE9i2XBrs4MNLb+QsJL5fH9lp3Sf1LkpAM1FuP6QLAVGRuNnrB twta/IBHjhacE/jPcO61ZPGxga6iDNMKQSbCRrR6hUOld3zTiHXxRwoNpzzTvreRyvD0RZcpq/S 5ujR5i0nW4rgaw2XZ+sp3KhHWkdldvc0H3Wetx1UC6zvq6jiVUm1lSXK9iZ2q884w5KmQXYLpoz jOFWqVAYolxRMkdvV7STwE9bLWVC2E= X-Received: by 2002:a05:600c:c48f:b0:497:fecd:5b00 with SMTP id 5b1f17b1804b1-49cf825d507mr177339265e9.9.1788700964867; Sun, 06 Sep 2026 06:22:44 -0700 (PDT) Received: from Abds-MacBook-Air.local ([2a02:3037:26b:8dd9:70ba:6dc3:6d92:e139]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-49cf7740d44sm527677145e9.15.2026.09.06.06.22.43 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 06 Sep 2026 06:22:44 -0700 (PDT) From: Abd-Alrhman Masalkhi To: Jinpu Wang , linux-raid , linux-block Cc: Song Liu , Yu Kuai , Jens Axboe , Christoph Hellwig , Ming Lei , Damien Le Moal , Nilay Shroff Subject: Re: [BUG] md/raid1: deadlock between a queue limits sysfs store and a spare re-add In-Reply-To: References: Date: Sun, 06 Sep 2026 15:22:43 +0200 Message-ID: Precedence: bulk X-Mailing-List: linux-block@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain On Sun, Sep 06, 2026 at 12:13 +0200, Abd-Alrhman Masalkhi wrote: > Hi Jack, > > On Sun, Sep 06, 2026 at 06:58 +0200, Jinpu Wang wrote: >> Hi, >> >> writing to a queue limit attribute of an md array while a spare is being >> re-added deadlocks the array, with three tasks left unkillable in D state. >> The host has to be rebooted to recover. >> >> We hit this in production on 6.12.100 with RAID1 arrays, triggered by a >> udev rule writing queue/max_sectors_kb. Reading the code, v7.2 looks >> affected too; see "Which versions" below for exactly what was tested and >> what was not. >> >> The cycle >> ========= >> >> Three tasks, one array: >> >> udev-worker queue_attr_store() holds q->limits_lock and waits in >> blk_mq_freeze_queue() for q_usage_counter to drain >> >> fio holds a q_usage_counter reference, waits in >> md_handle_request()'s is_suspended() loop >> >> md_start_sync holds mddev->suspended, waits for q->limits_lock >> >> Nobody can proceed: the freeze needs the in-flight I/O to finish, that >> I/O needs mddev->suspended cleared, and clearing it needs the spare add >> to finish, which is blocked on the lock the first task holds. >> >> The three legs in v7.2 (8d3ae59288f1): >> >> block/blk-sysfs.c, queue_attr_store(): >> >> struct queue_limits lim = queue_limits_start_update(q); >> >> res = entry->store_limit(disk, page, length, &lim); >> if (res < 0) { >> queue_limits_cancel_update(q); >> return res; >> } >> >> res = queue_limits_commit_update_frozen(q, &lim); >> >> queue_limits_start_update() takes q->limits_lock, and >> queue_limits_commit_update_frozen() calls blk_mq_freeze_queue() with it >> still held. max_sectors_kb is a QUEUE_LIM_RW_ENTRY, so a plain >> >> echo 1024 > /sys/block/mdN/queue/max_sectors_kb >> >> reaches this path. >> >> drivers/md/md.c, md_start_sync(): >> >> if (mddev->reshape_position == MaxSector && >> md_spares_need_change(mddev)) { >> suspend = true; >> mddev_suspend(mddev, false); >> } >> >> mddev_lock_nointr(mddev); >> >> and from there md_choose_sync_action() -> remove_and_add_spares() -> >> ->hot_add_disk() -> raid1_add_disk() -> mddev_stack_new_rdev(), which >> does a blocking >> >> lim = queue_limits_start_update(mddev->gendisk->queue); >> >> while mddev->suspended is set. >> >> drivers/md/md.c, md_handle_request(), where the in-flight I/O waits: >> >> if (is_suspended(mddev, bio)) { >> ... >> wait_event(mddev->sb_wait, !is_suspended(mddev, bio)); >> >> So md acquires q->limits_lock while holding a quiescing primitive that >> blocks exactly the I/O a concurrent freeze is waiting to drain. >> >> Backtraces >> ========== >> >> From a 6.12.100 based kernel: >> >> INFO: task kworker/2:2 blocked for more than 184 seconds. >> Workqueue: md_misc md_start_sync [md_mod] >> Call Trace: >> __mutex_lock.constprop.0+0x31c/0x6d0 >> mddev_stack_new_rdev+0x59/0x150 [md_mod] >> raid1_add_disk+0x97/0x180 [raid1] >> remove_and_add_spares+0xe8/0x230 [md_mod] >> md_start_sync+0x14c/0x3e0 [md_mod] >> process_one_work+0x162/0x370 >> >> INFO: task (udev-worker) blocked for more than 184 seconds. >> Call Trace: >> blk_mq_freeze_queue_wait+0x9e/0xd0 >> queue_limits_commit_update_frozen+0x12/0x40 >> queue_attr_store+0xc9/0x1c0 >> kernfs_fop_write_iter+0x133/0x220 >> vfs_write+0x29c/0x450 >> >> INFO: task fio blocked for more than 184 seconds. >> Call Trace: >> md_handle_request+0x10d/0x2b0 [md_mod] >> __submit_bio+0x23e/0x2f0 >> submit_bio_noacct_nocheck+0x1a3/0x3c0 >> blkdev_direct_IO+0x265/0x5d0 >> >> /proc/mdstat at that point, with the spare add never completing: >> >> md0 : active raid1 rnbd0[0] rnbd1[1](S) >> 5238784 blocks super 1.2 [2/1] [U_] >> >> Reproducer >> ========== >> >> On a scratch machine, with two ram devices: >> >> mdadm -C /dev/md111 --force -e 1.2 --assume-clean -l 1 \ >> --bitmap=internal -n 2 /dev/ram0 /dev/ram1 >> >> # keep I/O in flight >> fio --direct=1 --rw=randrw --ioengine=libaio --iodepth=32 --numjobs=4 \ >> --time_based=1 --runtime=180 --filename=/dev/md111 --name=repro & >> >> # stand in for the udev worker >> while :; do >> echo 128 > /sys/block/md111/queue/max_sectors_kb 2>/dev/null >> done & >> >> # drive spare re-adds >> for i in $(seq 20); do >> mdadm /dev/md111 --fail /dev/ram0 >> mdadm /dev/md111 --remove /dev/ram0 >> mdadm /dev/md111 --add /dev/ram0 >> mdadm --wait /dev/md111 >> done >> >> It reproduced on the first iteration for us, though it is a race, so it >> may need a few attempts on other machines. >> >> Which versions >> ============== >> >> Reproduced: 6.12.100 (distro kernel carrying the stable backport of >> c99f66e4084a), RAID1, repeatedly, on several hosts. >> >> Not reproduced, code inspection only: v7.2 (8d3ae59288f1). All three >> legs quoted above are from the v7.2 tree and are unchanged there, so it >> looks affected, but we have not run the reproducer on a mainline build. >> Happy to do that if it helps. >> >> raid10 calls mddev_stack_new_rdev() from raid10_add_disk() in the same >> way, so it looks exposed too; we have only tested raid1. >> >> When it started >> =============== >> >> Before commit c99f66e4084a ("block: fix queue freeze vs limits lock >> order in sysfs store methods"), queue_attr_store() froze the queue first >> and took limits_lock afterwards, so limits_lock was never held across the >> freeze wait and this cycle could not form. That commit moved the freeze >> inside queue_limits_commit_update_frozen(), i.e. under limits_lock: >> >> Fixes: c99f66e4084a ("block: fix queue freeze vs limits lock order >> in sysfs store methods") >> >> That is not an argument for reverting it: it exists so sd_revalidate_disk() >> can issue SCSI commands while holding the limits lock, which cannot work >> on a frozen queue. The md side is on the wrong side of the ordering the >> block layer expects, as spelled out in commit 06a2ff603f1f ("loop: Fix >> recently introduced lock inversion"): all block driver code takes >> queue_limits_start_update() before freezing. md instead takes a >> quiescing primitive of its own first. >> >> What we are running >> =================== >> >> We fixed it on the md side, by taking the limits update in >> md_start_sync() before mddev_suspend(), threading the queue_limits >> through ->hot_add_disk() so the personality stacks into it without >> taking the lock itself, and committing before resuming, so the update >> still lands while the array is quiesced. That removes the inversion >> without touching the shared block layer code. >> > > I am working on a bug in slot_store(). It calls remove_and_add_spares() > without suspending the array. If I just suspend the array without taking > care of q->limits_lock, it would trigger the same issue you > mentioned. To clarify, I haven't submitted my patch for slot_store() yet. I was going to, but your report shows that won't be enough. I'll hold off on my changes so I can use your queue_limits threading approach. > 2- state_store() also calls remove_and_add_spares(), while the array is > suspended by rdev_attr_store(), so it would probably trigger the same > issue as well. > >> Two approaches we tried first and discarded, in case they save someone >> the detour: >> >> - mutex_trylock() in mddev_stack_new_rdev() with a retry on contention. >> A writer that keeps retaking limits_lock wins nearly every time, so >> the retry does not converge: we measured 1132 backoffs against 1 >> success, the array staying degraded with an idle spare throughout, and >> md_check_recovery() suspending and resuming the array on every pass. >> >> - Dropping limits_lock around the freeze wait inside >> queue_limits_commit_update_frozen(). This works, but it inverts the >> ordering the block layer has standardised on, and a concurrent update >> committing in the window is then silently overwritten. >> >> We can post the md-side patch if that direction looks right, or defer to >> whatever you prefer. Reproducer script and the full logs are available. >> >> Thanks, >> Jack >> > > -- > Best Regards, > Abd-Alrhman -- Best Regards, Abd-Alrhman