Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:4 as permitted sender) client-ip=2620:137:e000::3:4;
Message-ID: <1a7a8caa3fe9b4e3271239b86ebd24a41464b79f.camel@sipsolutions.net>
Subject: Re: [PATCH AUTOSEL 6.6 15/32] debugfs: annotate debugfs handlers
 vs. removal with lockdep
From:   Johannes Berg <johannes@sipsolutions.net>
To:     Sasha Levin <sashal@kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "stable@vger.kernel.org" <stable@vger.kernel.org>
Cc:     Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Date:   Mon, 04 Dec 2023 21:45:55 +0100
In-Reply-To: <20231204203317.2092321-15-sashal@kernel.org>
References: <20231204203317.2092321-1-sashal@kernel.org>
         <20231204203317.2092321-15-sashal@kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.50.1 (3.50.1-1.fc39) 
MIME-Version: 1.0
Precedence: bulk

On Mon, 2023-12-04 at 20:32 +0000, Sasha Levin wrote:
> From: Johannes Berg <johannes.berg@intel.com>
>=20
> [ Upstream commit f4acfcd4deb158b96595250cc332901b282d15b0 ]
>=20
> When you take a lock in a debugfs handler but also try
> to remove the debugfs file under that lock, things can
> deadlock since the removal has to wait for all users
> to finish.
>=20
> Add lockdep annotations in debugfs_file_get()/_put()
> to catch such issues.
>=20

This (and the previous patch) probably got picked up as dependencies for
the locking things, but ... we reverted this.

For 6.6, _maybe_ it's worth backporting this including the revert, but
then I'd do that only when the revert landed to have them together. But
then you should apply all the six patches listed below _and_ the revert,
the set as here doesn't do anything useful.

However ... given that debugfs is root-only, and you have to be
reading/writing a file _while_ disconnecting and the file is removed,
perhaps the whole thing isn't worth backporting at all.


For 6.1 and earlier, I believe it's not needed at all, so please drop
from there all of these:

 - debugfs: fix automount d_fsdata usage
 - debugfs: annotate debugfs handlers vs. removal with lockdep
 - debugfs: add API to allow debugfs operations cancellation
 - wifi: cfg80211: add locked debugfs wrappers
 - wifi: mac80211: use wiphy locked debugfs helpers for agg_status
 - wifi: mac80211: use wiphy locked debugfs for sdata/link


I'd kind of think just dropping all of these completely makes more
sense.

johannes