On Wed, Apr 30, 2025 at 02:27:18PM +0000, Timur Tabi wrote: > On Wed, 2025-04-30 at 09:30 +0200, gregkh@xxxxxxxxxxxxxxxxxxx wrote: > > > added back in 2019, and why was that functionality *added* to > > > debugfs_remove? > > > > So we didn't have 2 functions that did the same thing and no one wanted > > to sweep the tree and rename everything at that time? I honestly don't > > remember, that was tens of thousands of patches ago :) > > I get that, but it seems pretty clear that at the time, the intent was to > replace debugfs_remove_recursive() with debugfs_remove(). The C function is > named debugfs_remove() and the macro is called debugfs_remove_recursive(). > > What you're saying now is that the C function should be renamed to > debugfs_remove_recursive() and the macro should be swapped. I don't think > that's a good idea. 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? Naming is hard. > > > I recently added a patch to Nouveau that used debugfs_remove() to clean up > > > all debugfs entries > > > specifically because it operates recursively. > > > > That's great, I'm not objecting to that, just that we need to stick with > > one or the other and I'd prefer the "recursive" name as that makes it > > blindingly obvious what is happening here while without it, people can > > get confused. > > 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. > 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. Especially now that we are reviewing the rust bindings for it, let's not end up duplicating that mess there. > 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. 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 :) thanks, greg k-h
