From patchwork Thu Feb 29 14:36:20 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Johannes Berg X-Patchwork-Id: 208393 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:7301:2097:b0:108:e6aa:91d0 with SMTP id gs23csp440195dyb; Thu, 29 Feb 2024 06:37:08 -0800 (PST) X-Forwarded-Encrypted: i=3; AJvYcCUzTaPTDB9njXCeqd4kgNCFk+V+ZAU790gAOQQCvoLRF3SLruSQOeznUptpDNMvo/EHWiHo7fw5su/fdQJAYeGJNmgBeQ== X-Google-Smtp-Source: AGHT+IF8qZ3awGSENKMet6Mvf+goD9lAi9MGnmForOXoExRwt+YmUahKAjjMq2GooXfbDPwAS0aH X-Received: by 2002:a05:6a00:2d1d:b0:6e4:6ca5:30b7 with SMTP id fa29-20020a056a002d1d00b006e46ca530b7mr2733941pfb.34.1709217428699; Thu, 29 Feb 2024 06:37:08 -0800 (PST) ARC-Seal: i=2; a=rsa-sha256; t=1709217428; cv=pass; d=google.com; s=arc-20160816; b=UWUzicbNp3XfZSBGx5Tfap3vSpqQUEVs9jkEuuWrBfp6WGSBaoCqxtVJXeu48zXtX6 npoZsSbSUlPcEDyxCOlBwQpZhVdi+mAufdxUs9zgaomaMS7nsEuhJNUYfJAvW4Srr46o I/7XmkNPf1zOIWTPRC3sYcwWgmD9S8Mec3yxZoJVrka4aw/9qyvzC1WsYEudokCQYYHj ZCSQq49HUaNL+f5KV+4G5VUBEeg58UhCmpJGcnvJxhCLUiK8BzYVgJs1lR0b9aUV7ftc wOXAxkPcSmuc07CVJ3r7SNd9sHurCfbkWgi3bxlyyzYoNPZhJpUZbW+VL3gZZ+XQUTHB VtLA== ARC-Message-Signature: i=2; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=content-transfer-encoding:mime-version:list-unsubscribe :list-subscribe:list-id:precedence:message-id:date:subject:cc:to :from:dkim-signature; bh=Yg7gA0jcv4W0xVz47D8GBOwfwX0WK4Erjbho1nblId0=; fh=OCa7KcQUgqZ9qCxroEOOjmTZdN85BqYgm9GMbzyFoEs=; b=H8HeGdnH+QGWphP2EV1FaIwQKXGtVGpLWxZVHmLNTIyt8KKv7hyMT/yoKYn1URTxRa jtVQMEOeZs4Sm5nt3HtNm1BMCqG0bXJXxGVkSfxDcTPRkq2S3ZGUo9HYZA8ZGiQN4W4g Awgj5vbSg1y+uCn+4dQMDUzDiek34j4QbS55ORG5jTwMCY47pkRzbiMlFny7zixKLM0C r6+nnTvlsr/9jSgAaWTMTpj9cxSPHjtPa94tIY83DCZreODR9lTiSqqn+Eb5I8Xwlep5 swiYSfB+MsHFObuITDfZ9uzNTYAO2SnLh4wJeH8Lz8exlp4hKuD/nJn6qzTUtpbXbe3l HRow==; dara=google.com ARC-Authentication-Results: i=2; mx.google.com; dkim=pass header.i=@sipsolutions.net header.s=mail header.b=GBo5Z4Dk; arc=pass (i=1 spf=pass spfdomain=sipsolutions.net dkim=pass dkdomain=sipsolutions.net dmarc=pass fromdomain=sipsolutions.net); spf=pass (google.com: domain of linux-kernel+bounces-86849-ouuuleilei=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) smtp.mailfrom="linux-kernel+bounces-86849-ouuuleilei=gmail.com@vger.kernel.org"; dmarc=pass (p=NONE sp=REJECT dis=NONE) header.from=sipsolutions.net Received: from sv.mirrors.kernel.org (sv.mirrors.kernel.org. [139.178.88.99]) by mx.google.com with ESMTPS id f12-20020a056a0022cc00b006e55993a954si1440511pfj.382.2024.02.29.06.37.08 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 29 Feb 2024 06:37:08 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel+bounces-86849-ouuuleilei=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99; Authentication-Results: mx.google.com; dkim=pass header.i=@sipsolutions.net header.s=mail header.b=GBo5Z4Dk; arc=pass (i=1 spf=pass spfdomain=sipsolutions.net dkim=pass dkdomain=sipsolutions.net dmarc=pass fromdomain=sipsolutions.net); spf=pass (google.com: domain of linux-kernel+bounces-86849-ouuuleilei=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) smtp.mailfrom="linux-kernel+bounces-86849-ouuuleilei=gmail.com@vger.kernel.org"; dmarc=pass (p=NONE sp=REJECT dis=NONE) header.from=sipsolutions.net Received: from smtp.subspace.kernel.org (wormhole.subspace.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by sv.mirrors.kernel.org (Postfix) with ESMTPS id 72C52283318 for ; Thu, 29 Feb 2024 14:37:08 +0000 (UTC) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 63785137763; Thu, 29 Feb 2024 14:36:54 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=sipsolutions.net header.i=@sipsolutions.net header.b="GBo5Z4Dk" Received: from sipsolutions.net (s3.sipsolutions.net [168.119.38.16]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id A69283E489; Thu, 29 Feb 2024 14:36:47 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=168.119.38.16 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1709217411; cv=none; b=n/+LQS/wE9aCQWy9GY3KeEiQKozqQVlzEc5ohT+xrsUNzq3ET0JSBGl3PkCaBw3seIgXN5CTcDQBX8l2gO+QYbStqLHSVFe/FgxdQziS+bgj3p+o9vyRuMz6rT7O8jBAbQae8cSipE5j9WRYWPgrGdCy+w7irxc7IjTQbnonJL8= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1709217411; c=relaxed/simple; bh=ShkOrOceasbzr+t0dTg3xM/G0Ce9lc06tQ+L5rDgUFE=; h=From:To:Cc:Subject:Date:Message-ID:MIME-Version; b=f7I9Gyjem/R9ZuBZOBCXJjdHaj2GovZZ+lWCPazTBzGj5CWlKKuo1BQ/GehP0U1WR5wvubhX9NuTLOpHyyB5kGI8Ey8qBKcHDDT0M8hmnJiOjJWX1LKYlveXhFx2Y+uKTkySDuwpXHxzj1ptTNjA+dZ4UCKGMTUoiXyYLJKDu/0= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=sipsolutions.net; spf=pass smtp.mailfrom=sipsolutions.net; dkim=pass (2048-bit key) header.d=sipsolutions.net header.i=@sipsolutions.net header.b=GBo5Z4Dk; arc=none smtp.client-ip=168.119.38.16 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=sipsolutions.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=sipsolutions.net DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sipsolutions.net; s=mail; h=Content-Transfer-Encoding:MIME-Version: Message-ID:Date:Subject:Cc:To:From:Content-Type:Sender:Reply-To:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-To:Resent-Cc: Resent-Message-ID:In-Reply-To:References; bh=Yg7gA0jcv4W0xVz47D8GBOwfwX0WK4Erjbho1nblId0=; t=1709217408; x=1710427008; b=GBo5Z4DkLB+CEbgGiAMfSSDsvM4tI0p2LTFfXQ9nMp1cM/oiQv4Z/QLF7LCuso1ZoZXV8YSXUHV tUKKl6H4FJg3Y+w7+YGFWSpTKO5W8Xz7TE6bMG7/sNegpRKF8zQ7mhhXGkvJFudlgQ2sMUX2w7eQj 2ZGTQUV/3p5mIIaI9kLG0Zi+vyBknWpiv8cYjN+ZUPRyoleL0g3OmotYgaYY30x0MVJ71MjCTBLkM ksKOjzQxhrzq4qZgKX2whaehhe7OB9QN82RpnhAQAPUMSL9c1d6kuIXWV3K2GqBgPfZDW/EzOYoVF FIVcYE2+KOiEBndlG7Mu29M694Od+4Ikja5w==; Received: by sipsolutions.net with esmtpsa (TLS1.3:ECDHE_X25519__RSA_PSS_RSAE_SHA256__AES_256_GCM:256) (Exim 4.97) (envelope-from ) id 1rfhWX-0000000Ddei-2Zad; Thu, 29 Feb 2024 15:36:37 +0100 From: Johannes Berg To: linux-wireless@vger.kernel.org, linux-kernel@vger.kernel.org Cc: Greg Kroah-Hartman , "Rafael J. Wysocki" , Johannes Berg , stable@vger.kernel.org, Ben Greear , Madhan Sai Subject: [PATCH] debugfs: fix wait/cancellation handling during remove Date: Thu, 29 Feb 2024 15:36:20 +0100 Message-ID: <20240229153635.6bfab7eb34d3.I6c7aeff8c9d6628a8bc1ddcf332205a49d801f17@changeid> X-Mailer: git-send-email 2.43.2 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1792244374716882940 X-GMAIL-MSGID: 1792244374716882940 From: Johannes Berg Ben Greear further reports deadlocks during concurrent debugfs remove while files are being accessed, even though the code in question now uses debugfs cancellations. Turns out that despite all the review on the locking, we missed completely that the logic is wrong: if the refcount hits zero we can finish (and need not wait for the completion), but if it doesn't we have to trigger all the cancellations. As written, we can _never_ get into the loop triggering the cancellations. Fix this, and explain it better while at it. Cc: stable@vger.kernel.org Fixes: 8c88a474357e ("debugfs: add API to allow debugfs operations cancellation") Reported-by: Ben Greear Closes: https://lore.kernel.org/r/1c9fa9e5-09f1-0522-fdbc-dbcef4d255ca@candelatech.com Tested-by: Madhan Sai Signed-off-by: Johannes Berg --- fs/debugfs/inode.c | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c index 034a617cb1a5..a40da0065433 100644 --- a/fs/debugfs/inode.c +++ b/fs/debugfs/inode.c @@ -751,13 +751,28 @@ static void __debugfs_file_removed(struct dentry *dentry) if ((unsigned long)fsd & DEBUGFS_FSDATA_IS_REAL_FOPS_BIT) return; - /* if we hit zero, just wait for all to finish */ - if (!refcount_dec_and_test(&fsd->active_users)) { - wait_for_completion(&fsd->active_users_drained); + /* if this was the last reference, we're done */ + if (refcount_dec_and_test(&fsd->active_users)) return; - } - /* if we didn't hit zero, try to cancel any we can */ + /* + * If there's still a reference, the code that obtained it can + * be in different states: + * - The common case of not using cancellations, or already + * after debugfs_leave_cancellation(), where we just need + * to wait for debugfs_file_put() which signals the completion; + * - inside a cancellation section, i.e. between + * debugfs_enter_cancellation() and debugfs_leave_cancellation(), + * in which case we need to trigger the ->cancel() function, + * and then wait for debugfs_file_put() just like in the + * previous case; + * - before debugfs_enter_cancellation() (but obviously after + * debugfs_file_get()), in which case we may not see the + * cancellation in the list on the first round of the loop, + * but debugfs_enter_cancellation() signals the completion + * after adding it, so this code gets woken up to call the + * ->cancel() function. + */ while (refcount_read(&fsd->active_users)) { struct debugfs_cancellation *c;