Re: [PATCH v3 01/15] VFS: note error returns in documentation for various lookup functions

[Jeff Layton]

On Wed, 2026-02-25 at 09:16 +1100, NeilBrown wrote:
> From: NeilBrown <neil@xxxxxxxxxx>
> 
> Darrick recently noted that try_lookup_noperm() is documented as
> "Look up a dentry by name in the dcache, returning NULL if it does not
> currently exist." but it can in fact return an error.
> 
> So update the documentation for that and related functions.
> 
> Link: https://lore.kernel.org/all/20260218234917.GA6490@frogsfrogsfrogs/
> Cc: "Darrick J. Wong" <djwong@xxxxxxxxxx>
> Signed-off-by: NeilBrown <neil@xxxxxxxxxx>
> ---
>  fs/namei.c | 29 ++++++++++++++++++++++++++++-
>  1 file changed, 28 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/namei.c b/fs/namei.c
> index 58f715f7657e..6f595f58acfe 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -3124,7 +3124,8 @@ static int lookup_one_common(struct mnt_idmap *idmap,
>   * @base:	base directory to lookup from
>   *
>   * Look up a dentry by name in the dcache, returning NULL if it does not
> - * currently exist.  The function does not try to create a dentry and if one
> + * currently exist or an error if there is a problem with the name.
> + * The function does not try to create a dentry and if one
>   * is found it doesn't try to revalidate it.
>   *
>   * Note that this routine is purely a helper for filesystem usage and should
> @@ -3132,6 +3133,11 @@ static int lookup_one_common(struct mnt_idmap *idmap,
>   *
>   * No locks need be held - only a counted reference to @base is needed.
>   *
> + * Returns:
> + *   - ref-counted dentry on success, or
> + *   - %NULL if name could not be found, or
> + *   - ERR_PTR(-EACCES) if name is dot or dotdot or contains a slash or nul, or
> + *   - ERR_PTR() if fs provide ->d_hash, and this returned an error.
>   */
>  struct dentry *try_lookup_noperm(struct qstr *name, struct dentry *base)
>  {
> @@ -3208,6 +3214,11 @@ EXPORT_SYMBOL(lookup_one);
>   *
>   * Unlike lookup_one, it should be called without the parent
>   * i_rwsem held, and will take the i_rwsem itself if necessary.
> + *
> + * Returns: - A dentry, possibly negative, or
> + *	    - same errors as try_lookup_noperm() or
> + *	    - ERR_PTR(-ENOENT) if parent has been removed, or
> + *	    - ERR_PTR(-EACCES) if parent directory is not searchable.
>   */
>  struct dentry *lookup_one_unlocked(struct mnt_idmap *idmap, struct qstr *name,
>  				   struct dentry *base)
> @@ -3244,6 +3255,10 @@ EXPORT_SYMBOL(lookup_one_unlocked);
>   * It should be called without the parent i_rwsem held, and will take
>   * the i_rwsem itself if necessary.  If a fatal signal is pending or
>   * delivered, it will return %-EINTR if the lock is needed.
> + *
> + * Returns: A dentry, possibly negative, or
> + *	   - same errors as lookup_one_unlocked() or
> + *	   - ERR_PTR(-EINTR) if a fatal signal is pending.
>   */
>  struct dentry *lookup_one_positive_killable(struct mnt_idmap *idmap,
>  					    struct qstr *name,

Claude says:

  lookup_one_positive_killable() documentation says "A dentry, possibly negative" but the function
  explicitly converts negative dentries to ERR_PTR(-ENOENT). It should say "A positive dentry" like
  the companion functions lookup_one_positive_unlocked() and lookup_noperm_positive_unlocked().

...but that seems to be the only "regression" it found. 


Aside from that nit, this looks fine to me.

Reviewed-by: Jeff Layton <jlayton@xxxxxxxxxx>