Commit f7862dfe authored by Al Viro's avatar Al Viro Committed by Greg Kroah-Hartman
Browse files

saner replacement for debugfs_rename() 



Existing primitive has several problems:
	1) calling conventions are clumsy - it returns a dentry reference
that is either identical to its second argument or is an ERR_PTR(-E...);
in both cases no refcount changes happen.  Inconvenient for users and
bug-prone; it would be better to have it return 0 on success and -E... on
failure.
	2) it allows cross-directory moves; however, no such caller have
ever materialized and considering the way debugfs is used, it's unlikely
to happen in the future.  What's more, any such caller would have fun
issues to deal with wrt interplay with recursive removal.  It also makes
the calling conventions clumsier...
	3) tautological rename fails; the callers have no race-free way
to deal with that.
	4) new name must have been formed by the caller; quite a few
callers have it done by sprintf/kasprintf/etc., ending up with considerable
boilerplate.

Proposed replacement: int debugfs_change_name(dentry, fmt, ...).  All callers
convert to that easily, and it's simpler internally.

IMO debugfs_rename() should go; if we ever get a real-world use case for
cross-directory moves in debugfs, we can always look into the right way
to handle that.

Signed-off-by: default avatarAl Viro <viro@zeniv.linux.org.uk>
Link: https://lore.kernel.org/r/20250112080705.141166-21-viro@zeniv.linux.org.uk


Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent c2a3a216

Documentation/filesystems/debugfs.rst

+5 −7
Original line number Diff line number Diff line
@@ -211,18 +211,16 @@ seq_file content. 


There are a couple of other directory-oriented helper functions::



    struct dentry *debugfs_rename(struct dentry *old_dir,

    				  struct dentry *old_dentry,

		                  struct dentry *new_dir,

				  const char *new_name);

    struct dentry *debugfs_change_name(struct dentry *dentry,

					  const char *fmt, ...);



    struct dentry *debugfs_create_symlink(const char *name,

                                          struct dentry *parent,

				      	  const char *target);



A call to debugfs_rename() will give a new name to an existing debugfs

file, possibly in a different directory.  The new_name must not exist prior

to the call; the return value is old_dentry with updated information.

A call to debugfs_change_name() will give a new name to an existing debugfs

file, always in the same directory.  The new_name must not exist prior

to the call; the return value is 0 on success and -E... on failuer.

Symbolic links can be created with debugfs_create_symlink().



There is one important thing that all debugfs users must take into account:

drivers/net/bonding/bond_debugfs.c

+2 −7
Original line number Diff line number Diff line
@@ -63,13 +63,8 @@ void bond_debug_unregister(struct bonding *bond) 


void bond_debug_reregister(struct bonding *bond)

{

	struct dentry *d;



	d = debugfs_rename(bonding_debug_root, bond->debug_dir,

			   bonding_debug_root, bond->dev->name);

	if (!IS_ERR(d)) {

		bond->debug_dir = d;

	} else {

	int err = debugfs_change_name(bond->debug_dir, "%s", bond->dev->name);

	if (err) {

		netdev_warn(bond->dev, "failed to reregister, so just unregister old one\n");

		bond_debug_unregister(bond);

	}

drivers/net/ethernet/amd/xgbe/xgbe-debugfs.c

+2 −17
Original line number Diff line number Diff line
@@ -505,21 +505,6 @@ void xgbe_debugfs_exit(struct xgbe_prv_data *pdata) 


void xgbe_debugfs_rename(struct xgbe_prv_data *pdata)

{

	char *buf;



	if (!pdata->xgbe_debugfs)

		return;



	buf = kasprintf(GFP_KERNEL, "amd-xgbe-%s", pdata->netdev->name);

	if (!buf)

		return;



	if (!strcmp(pdata->xgbe_debugfs->d_name.name, buf))

		goto out;



	debugfs_rename(pdata->xgbe_debugfs->d_parent, pdata->xgbe_debugfs,

		       pdata->xgbe_debugfs->d_parent, buf);



out:

	kfree(buf);

	debugfs_change_name(pdata->xgbe_debugfs,

			    "amd-xgbe-%s", pdata->netdev->name);

}

drivers/net/ethernet/marvell/skge.c

+1 −4
Original line number Diff line number Diff line
@@ -3742,10 +3742,7 @@ static int skge_device_event(struct notifier_block *unused, 
	skge = netdev_priv(dev);

	switch (event) {

	case NETDEV_CHANGENAME:

		if (skge->debugfs)

			skge->debugfs = debugfs_rename(skge_debug,

						       skge->debugfs,

						       skge_debug, dev->name);

		debugfs_change_name(skge->debugfs, "%s", dev->name);

		break;



	case NETDEV_GOING_DOWN:

drivers/net/ethernet/marvell/sky2.c

+1 −4
Original line number Diff line number Diff line
@@ -4494,10 +4494,7 @@ static int sky2_device_event(struct notifier_block *unused, 


	switch (event) {

	case NETDEV_CHANGENAME:

		if (sky2->debugfs) {

			sky2->debugfs = debugfs_rename(sky2_debug, sky2->debugfs,

						       sky2_debug, dev->name);

		}

		debugfs_change_name(sky2->debugfs, "%s", dev->name);

		break;



	case NETDEV_GOING_DOWN: