Re: [PATCH v4 1/3] fs: configfs: add helpers for opening non-configfs paths

[Runyu Xiao]

Thanks for the reviews.

> > +bool configfs_path_is_configfs(const struct path *path)
> > +{
> > +	return path->dentry->d_sb->s_type == &configfs_fs_type;
> > +}
> > +EXPORT_SYMBOL_GPL(configfs_path_is_configfs);
> > +
> > +struct file *configfs_open_root(const struct path *root, const char *name,
> > +			int flags, umode_t mode)
> > +{
> > +	if (configfs_path_is_configfs(root))
> > +		return ERR_PTR(-EINVAL);
> > +
> > +	return file_open_root(root, name, flags, mode);
> > +}
> > +EXPORT_SYMBOL_GPL(configfs_open_root);
>
> These two aren't used outside this file, so they could be static,
> or in case of configfs_path_is_configfs even be folded into the
> only caller.  Or do you plan to have other users?

I folded the filesystem-type check into configfs_open_root() and dropped
the predicate entirely.  configfs_file_open() is for callers that have a
pathname, while configfs_open_root() is for callers that keep a resolved
root and open files below it.

> > +struct file *configfs_file_open(const char *filename, int flags, umode_t mode)
>
> Can you add a kerneldoc comment explaining how/why this should be used?

I will add kerneldoc comments for both helpers in v5.

> Didn't you also have patches for drivers/target/ that should use
> the new helper?

Yes.  The target-core patch titled "scsi: target: pin db_root for metadata
writes" is another consumer of configfs_open_root().  I will rebase that
patch on this series and drop its duplicate configfs changes.

The separate patch titled "scsi: target: file: avoid recursive configfs open
in fd_init_prot()" will use configfs_file_open(), not configfs_open_root().

I will send v5 as a new thread.  Patches 2/3 and 3/3 are unchanged and
retain your Reviewed-by.

Thanks,
Runyu