Re: [RFC PATCH 0/3] debugfs: implement 'debugfs_create_dir_with_tmpfiles()'

[Roman Peniaev]

On Tue, Dec 8, 2015 at 12:49 PM, Greg Kroah-Hartman
<gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
> On Tue, Dec 08, 2015 at 10:51:03AM +0100, Roman Pen wrote:
>> Hello.
>>
>> Here is an attempt to solve annoying race, which exists between two operations
>> on debugfs entries: write (setting a request) and read (reading a response).
>>
>> E.g. let's assume that we have some storage device, which can have thousands
>> of snapshots (yeah, plenty of them, thus it is ridicoulous to create a debugfs
>> entry for each), and each snapshot is controlled by the handle, which is a UUID
>> or any non-numeric character sequence (for numeric sequence this problem can be
>> solved by 'seek' operation).  This device provides a debugfs entry 'snap_status',
>> which can be opened for reading and writing, where write - is an operation for
>> specifiying a request, and read - is an operation for getting a response back.
>>
[skipped]
>>
>> So basically creating a temporary file on demand with a specified name is a
>> way to provide one additional parameter for an 'read' operation.
>>
>> Probably, there is more elegant solution for that write-read race problem,
>> but I've not found any.
>>
>> PS. I did not want to use configfs, because I have nothing to configure (what
>>     I have described is not a configuration issue), and I do not like to keep
>>       dentries in a system if userspace forgets to remove them.
>
> Do you have a patch series that depends on these new apis?  I don't want
> to add things to debugfs without any in-tree users if at all possible.

I can show only similar write/read usage, which I tried to avoid.  I
did the grep and found
the following files which do exactly what I've described here:

     linux/drivers/bluetooth/btmrvl_debufgfs.c
        .read   = btmrvl_hscfgcmd_read,
        .write  = btmrvl_hscfgcmd_write,

        .read = btmrvl_pscmd_read,
        .write = btmrvl_pscmd_write,

        .read   = btmrvl_hscmd_read,
        .write  = btmrvl_hscmd_write,

     linux/drivers/mfd/ab8500-debugfs.c
        .open = ab8500_bank_open,
        .write = ab8500_bank_write,

        .open = ab8500_address_open,
        .write = ab8500_address_write,

        .open = ab8500_val_open,
        .write = ab8500_val_write,


     linux/drivers/mmc/card/mmc_test.c
        .open       = mtf_test_open,
        .read       = seq_read,
        .write      = mtf_test_write,

     linux/drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c
        .read =  ixgbe_dbg_reg_ops_read,
        .write = ixgbe_dbg_reg_ops_write,

        .read = ixgbe_dbg_netdev_ops_read,
        .write = ixgbe_dbg_netdev_ops_write,

     linux/drivers/platform/olpc/olpc-ec.c
        .write = ec_dbgfs_cmd_write,
        .read = ec_dbgfs_cmd_read,

     linux/kernel/time/test_udelay.c
        .open = udelay_test_open,
        .read = seq_read,
        .write = udelay_test_write,


Of course, I could miss something, because plenty of callers with
similar meaning,
but in the list above everything boils down to setting request on
write() and getting
response back on read().

For example this simplest and representative test_udelay.c:

     echo "100 10" >   debugfs/udelay_test
     cat  debugfs/udelay_test

can be replaced with atomic sequence:
    cat  debugfs/udelay_test/"100 10"

And frankly I do not know does it make sense to switch these functions
to new API
and to break userspace expectations, but for sure they are the
candidates to behave
atomically.

--
Roman
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/