Re: [PATCH AUTOSEL 6.6 15/32] debugfs: annotate debugfs handlers vs. removal with lockdep

Sasha Levin
2023-12-05T04:25:15Z

On Mon, Dec 04, 2023 at 09:45:55PM +0100, Johannes Berg wrote:
>On Mon, 2023-12-04 at 20:32 +0000, Sasha Levin wrote:
>> From: Johannes Berg <johannes.berg@intel.com>
>>
>> [ Upstream commit f4acfcd4deb158b96595250cc332901b282d15b0 ]
>>
>> 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.
>>
>> Add lockdep annotations in debugfs_file_get()/_put()
>> to catch such issues.
>>
>
>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.

Will do, thanks!

--
Thanks,
Sasha