On 3/29/20 1:34 PM, Rohit Sarkar wrote:
On Sun, Mar 29, 2020 at 10:38:18AM +0100, Jonathan Cameron wrote:
On Sat, 28 Mar 2020 12:04:53 +0530
Rohit Sarkar <rohitsarkar5398@xxxxxxxxx> wrote:
The debugfs_create_file_unsafe method does not protect the fops given to
it from file removal. It must be used with DEFINE_DEBUGFS_ATTRIBUTE
which makes the fops aware of the file lifetime.
Further using DEFINE_DEBUGFS_ATTRIBUTE along with
debugfs_create_file_unsafe significantly reduces the overhead introduced by
debugfs_create_file which creates a lifetime managing proxy around each
fops handed in. Refer [1] for more on this.
Fixes the following warnings reported by coccinelle:
drivers/iio/imu//adis16460.c:126:0-23: WARNING: adis16460_flash_count_fops should be defined with DEFINE_DEBUGFS_ATTRIBUTE
drivers/iio/imu//adis16460.c:108:0-23: WARNING: adis16460_product_id_fops should be defined with DEFINE_DEBUGFS_ATTRIBUTE
drivers/iio/imu//adis16460.c:90:0-23: WARNING: adis16460_serial_number_fops should be defined with DEFINE_DEBUGFS_ATTRIBUTE
drivers/iio/imu//adis16400.c:278:0-23: WARNING: adis16400_flash_count_fops should be defined with DEFINE_DEBUGFS_ATTRIBUTE
drivers/iio/imu//adis16400.c:261:0-23: WARNING: adis16400_product_id_fops should be defined with DEFINE_DEBUGFS_ATTRIBUTE
[1]: https://lists.gt.net/linux/kernel/2369498
Rohit Sarkar (2):
iio: imu: adis16400: use DEFINE_DEBUGFS_ATTRIBUTE instead of
DEFINE_SIMPLE_ATTRIBUTE
iio: imu: adis16460: use DEFINE_DEBUGFS_ATTRIBUTE instead of
DEFINE_SIMPLE_ATTRIBUTE
drivers/iio/imu/adis16400.c | 4 ++--
drivers/iio/imu/adis16460.c | 6 +++---
2 files changed, 5 insertions(+), 5 deletions(-)
Hi Rohit,
Hey,
You've opened a can of worms with this one. There as a previous series
posted doing exactly this change back in 2019 by Zhong Jiang (cc'd)
At the time I did a bit of looking into why this had been universally taken
up cross tree and turned out there are some potential issues.
Alexandru added it to the list of things to test, but I guess it got
buried under other work and is still outstanding.
https://lkml.org/lkml/2019/10/30/144
Acc. to the patch by Zhong this change kind of comes off as a cosmetic
change as in the commit message he mentions "it is more clear".
But there is certainly more to it than that:
In the current scenario since we are using debugfs_create_file_unsafe
the file has no protection whatsoever against removal.
The drivers you are patching all use debugfs_create_file() as far as I
can see.
The way I understand it using DEFINE_DEBUGFS_ATTRIBUTE without switching
to debugfs_create_file_unsafe() will not make a difference. There will
only be more overhead since the files are protected twice.
- Lars
