Re: [PATCH 0/2] use DEFINE_DEBUGFS_ATTRIBUTE instead of DEFINE_SIMPLE_ATTRIBUTE

From: Lars-Peter Clausen
Date: Tue Mar 31 2020 - 07:08:12 EST


On 3/31/20 12:58 PM, Sa, Nuno wrote:
Hi Rohit,

From: linux-iio-owner@xxxxxxxxxxxxxxx <linux-iio-owner@xxxxxxxxxxxxxxx> On
Behalf Of Ardelean, Alexandru
Sent: Montag, 30. MÃrz 2020 11:20
To: jic23@xxxxxxxxxx; rohitsarkar5398@xxxxxxxxx
Cc: stefan.popa@xxxxxxxxxx; zhongjiang@xxxxxxxxxx; linux-
iio@xxxxxxxxxxxxxxx; Bogdan, Dragos <Dragos.Bogdan@xxxxxxxxxx>;
pmeerw@xxxxxxxxxx; knaack.h@xxxxxx; Hennerich, Michael
<Michael.Hennerich@xxxxxxxxxx>; linux-kernel@xxxxxxxxxxxxxxx;
lars@xxxxxxxxxx
Subject: Re: [PATCH 0/2] use DEFINE_DEBUGFS_ATTRIBUTE instead of
DEFINE_SIMPLE_ATTRIBUTE

On Sun, 2020-03-29 at 10:38 +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,

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.

yep
my bad;
will try to make room these days for that old one


I don't have the exact parts that this patch is touching but I have other parts where this patch
applies and should be same. So, the idea to test this is to read the files in debugfs? Maybe also
some unbind + binding?

I will try to test this still today...

The stress test is to open the debugfs file, then unbind the device and then read from the still open debugfs file.

- Lars