On Tue, 03 Mar 2026, Jeff Layton wrote: > On Wed, 2026-02-25 at 09:16 +1100, NeilBrown wrote: > > From: NeilBrown <[email protected]> > > > > 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" <[email protected]> > > Signed-off-by: NeilBrown <[email protected]> > > --- > > 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().
Thanks. The top patch in my next batch contains a bunch of similar documentation cleanups. I'll add this to that patch. > > ...but that seems to be the only "regression" it found. > Good to know! Thanks, NeilBrown > > Aside from that nit, this looks fine to me. > > Reviewed-by: Jeff Layton <[email protected]> >
