Re: [PATCH 2/2] configfs: make a minimal path of symlink

[Joel Becker <jlbec@evilplan.org>]

On Mon, Apr 01, 2024 at 11:26:55AM +0300, Dmitry Bogdanov wrote:
> Symlinks in configfs are used to be created from near places. Currently the

Perhaps "... are usually created from nearby places. ..."

> path is artificially inflated by multiple ../ to the configfs root an then
> a full path of the target.
> 
> For scsi target subsystem the difference between such a path and a minimal
> possible path is ~100 characters.
> 
> This patch makes a minimal relative path of symlink - from the closest
> common parent.
> 
> Signed-off-by: Dmitry Bogdanov <d.bogdanov@yadro.com>
> ---
>  fs/configfs/symlink.c | 59 ++++++++++++++++++++++++++++---------------
>  1 file changed, 38 insertions(+), 21 deletions(-)
> 
> diff --git a/fs/configfs/symlink.c b/fs/configfs/symlink.c
> index 224c9e4899d4..a61f5a4763e1 100644

<snip>

>  static int configfs_get_target_path(struct config_item *item,
>  		struct config_item *target, char **path)
>  {
> -	int depth, size;
> +	struct config_item *pdest, *ptarget;
> +	int target_depth = 0, item_depth = 0;
> +	int size;
>  	char *s;
>  
> -	depth = item_depth(item);
> -	size = item_path_length(target) + depth * 3 - 1;
> +	/* find closest common parent to make a minimal path */
> +	for (ptarget = target;
> +	     ptarget && !configfs_is_root(ptarget);
> +	     ptarget = ptarget->ci_parent) {
> +		item_depth = 0;
> +		for (pdest = item;
> +		     pdest && !configfs_is_root(pdest);
> +		     pdest = pdest->ci_parent) {
> +			if (pdest == ptarget)
> +				goto out;
> +
> +			item_depth++;
> +		}
> +
> +		target_depth++;
> +	}

This O(n^2) loop tickles my spidey senses.  Reading it over, I think it
is correct, though I'm wary of how it might handle certain inputs.  I
also tried to think of ways it could short circuit the inner loop based
on the max target depth, but it can't know that without precomputing
the max target depth.

There are enough corner cases that I would think the depth computation
is a good candidate for KUnit tests.

Thanks,
Joel

-- 

"Hell is oneself, hell is alone, the other figures in it, merely projections."
        - T. S. Eliot

			http://www.jlbec.org/
			jlbec@evilplan.org