Re: [PATCH v2 1/4] fs: introduce helper d_path_unsafe()

[Andy Shevchenko]

On Wed, Jun 23, 2021 at 01:50:08PM +0800, Jia He wrote:
> This helper is similar to d_path() except that it doesn't take any
> seqlock/spinlock. It is typical for debugging purposes. Besides,
> an additional return value *prenpend_len* is used to get the full
> path length of the dentry, ingoring the tail '\0'.
> the full path length = end - buf - prepend_length - 1.
> 
> Previously it will skip the prepend_name() loop at once in
> __prepen_path() when the buffer length is not enough or even negative.
> prepend_name_with_len() will get the full length of dentry name
> together with the parent recursively regardless of the buffer length.

...

>  /**
>   * prepend_name - prepend a pathname in front of current buffer pointer
> - * @buffer: buffer pointer
> - * @buflen: allocated length of the buffer
> - * @name:   name string and length qstr structure
> + * @p: prepend buffer which contains buffer pointer and allocated length
> + * @name: name string and length qstr structure
>   *
>   * With RCU path tracing, it may race with d_move(). Use READ_ONCE() to
>   * make sure that either the old or the new name pointer and length are

This should be separate patch. You are sending new version too fast...
Instead of speeding up it will slow down the review process.

...

> +	const char *dname = smp_load_acquire(&name->name); /* ^^^ */

I have commented on the comment here. What does it mean for mere reader?

> +	int dlen = READ_ONCE(name->len);
> +	char *s;
> +	int last_len = p->len;

Reversed xmas tree order, please.

The rule of thumb is when you have gotten a comment against a piece of code,
try to fix all similar places at once.

...

> @@ -108,8 +181,7 @@ static int __prepend_path(const struct dentry *dentry, const struct mount *mnt,
>   * prepend_path - Prepend path string to a buffer
>   * @path: the dentry/vfsmount to report
>   * @root: root vfsmnt/dentry
> - * @buffer: pointer to the end of the buffer
> - * @buflen: pointer to buffer length
> + * @p: prepend buffer which contains buffer pointer and allocated length
>   *
>   * The function will first try to write out the pathname without taking any
>   * lock other than the RCU read lock to make sure that dentries won't go away.

Kernel doc fix should be in a separate patch.


-- 
With Best Regards,
Andy Shevchenko