On Fri, 19 Aug 2022 15:44:51 -0700 Andrew Morton akpm@linux-foundation.org wrote:
On Fri, 19 Aug 2022 21:16:31 +0000 SeongJae Park sj@kernel.org wrote:
It would be simpler (and less racy) to check the debugfs_create_dir() return value for IS_ERR()?
I was merely following Greg's previous advice for ignoring the return value[1] of the function, but I might misunderstanding his intention, so CC-ing Greg. Greg, may I ask your opinion?
[1] https://lore.kernel.org/linux-mm/YB1kZaD%2F7omxXztF@kroah.com/
Thing is, the correct functioning of the debugfs interfaces is utterly critical to damon. And that's apart from these memory leak and oops-we-killed-damon issues.
So damon simply cannot ignore the state of its debugfs interfaces and keep going along - because if something goes wrong at the debugfs layer, damon is dead and useless and the machine needs a reboot.
Perhaps this means that damon should not be using debugfs for its interfaces at all. Or it means that the debugfs interfaces are misdesigned. I go with the latter, which, alas, also affirms the former.
I'd save my word about the latter, but agreed on the former. Fortunately we already have an alternative (DAMON sysfs interface), and the debugfs interface deprecation plan was announced for a while ago. Not sure if the deprecation will be well as hoped, though.
From a quick scan it appears that a significant minority (20%?) of drivers are checking the debugfs_create_dir() return value.
Maybe partly owing to Greg's previous efforts for removing the checks[1,2]? Anyway, based on the previous discussions, I'd expect Greg might prefer not checking the return code.
Anyway, waiting for his opinion.
[1] https://lore.kernel.org/all/20190122152151.16139-14-gregkh@linuxfoundation.o... [2] https://lore.kernel.org/lkml/20190122152151.16139-7-gregkh@linuxfoundation.o...
Thanks, SJ