On Wed, 2025-04-30 at 16:57 +0200, gregkh@xxxxxxxxxxxxxxxxxxx wrote: > I don't really remember what we were talking about in 2019 for this, but > look at how many of each there are in the tree: > 402 debugfs_remove > 627 debugfs_remove_recursive > > so we need to pick one and just stick to it. Majority wins? Shortest > function name wins? Most descriptive winse? How about "intent of the original patch wins"? I'm pretty sure that if my patch had been merged in 2019, these numbers would be swapped. > Naming is hard. Sure, but it just seems pretty clear that this has already been decided, and the only mistake was in not updating the documentation when that decision was made in 2019. > > > > > Well, wouldn't you think it's confusing to call a function named > > debugfs_remove_recursive() in order to remove a single file? > > As debugfs doesn't really care about files vs. directories it just > doesn't matter. Well, the word "recursive" should mean something. > > If you want, I can change the documentation to say, "please use > > debugfs_remove_recursive() to remove directories, and debugfs_remove() to > > remove files". > > Let's just pick one and be done with it please. That's what my patch does! > Especially now that we > are reviewing the rust bindings for it, let's not end up duplicating > that mess there. Coincidentally, I noticed this because I'm doing some Rust debugfs work. > > We could also modify debugfs_remove_recursive() to issue a WARN if it is > > called on a file. > > Never call WARN(). If you do, you just rebooted the box because a few > billion Linux machined have panic-on-warn enabled. Fine, pr_warn() then. But like you said, you want one of these functions removed, not enhanced. > So I'll defer to you, which one do you want? You originally said > debugfs_remove(), which is fine, but you get to send a patch touching > all of those files :) If you really want me to send out a patch modifying 600+ calls, I will, but I think that will cause more harm than good, and I'll just make more enemies than friends. All I was trying to do with my patch was have the documentation align with the code. This would be a low-impact first step to replacing debugfs_remove_recursive with debugfs_remove everywhere.
