diff mbox series

fs: use helpers for opening kernel internal files

Message ID 20230614120917.2037482-1-amir73il@gmail.com (mailing list archive)
State New, archived
Headers show
Series fs: use helpers for opening kernel internal files | expand

Commit Message

Amir Goldstein June 14, 2023, 12:09 p.m. UTC
Overlayfs and cachefiles use vfs_open_tmpfile() to open a tmpfile
without accounting for nr_files.

Rename this helper to kernel_tmpfile_open() to better reflect this
helper is used for kernel internal users.

cachefiles uses open_with_fake_path() without the need for a fake path
only to use the noaccount feature of open_with_fake_path().

Fork open_with_fake_path() to kernel_file_open() which only does the
noaccount feature and use it in cachefiles.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---

Christian,

Per your request, here is an extra patch to decouple cachefiles
from open_with_fake_path().

This patch comes before the backing_file patches [1].
There is a minor merge conflict with backing_file patches
please see how I resolved it on my branch [2] -
Note that the mention of 'cachefiles' was removed from the
backing_file commit message and there is a slight rewording
that you also requested s/open_backing_file/backing_file_open,
to be consistent with kernel_file_open.

I truely hope that this patch is not going to steer a bike shedding
session over the names of the helpers.

Note that all the inernal kernel files opened by cachefiles using the
new kernel_*_open() helpers also have the S_KERNEL_FILE inode flag.

Thanks,
Amir.

[1] https://lore.kernel.org/linux-fsdevel/20230614074907.1943007-1-amir73il@gmail.com/
[2] https://github.com/amir73il/linux/commits/ovl_fake_path

 fs/cachefiles/namei.c    | 10 +++++-----
 fs/namei.c               | 24 +++++++++++++-----------
 fs/open.c                | 31 +++++++++++++++++++++++++++++++
 fs/overlayfs/overlayfs.h |  5 +++--
 include/linux/fs.h       |  9 ++++++---
 5 files changed, 58 insertions(+), 21 deletions(-)

Comments

Christoph Hellwig June 15, 2023, 4:26 a.m. UTC | #1
On Wed, Jun 14, 2023 at 03:09:17PM +0300, Amir Goldstein wrote:
> Overlayfs and cachefiles use vfs_open_tmpfile() to open a tmpfile
> without accounting for nr_files.
> 
> Rename this helper to kernel_tmpfile_open() to better reflect this
> helper is used for kernel internal users.
> 
> cachefiles uses open_with_fake_path() without the need for a fake path
> only to use the noaccount feature of open_with_fake_path().
> 
> Fork open_with_fake_path() to kernel_file_open() which only does the
> noaccount feature and use it in cachefiles.

Please split this into two patches, one for the
vfs_tmpfile_open rename, and one for the kernel_file_open helper.

> +EXPORT_SYMBOL(kernel_file_open);

EXPORT_SYMBOL_GPL, please.
Amir Goldstein June 15, 2023, 11:29 a.m. UTC | #2
On Thu, Jun 15, 2023 at 7:26 AM Christoph Hellwig <hch@lst.de> wrote:
>
> On Wed, Jun 14, 2023 at 03:09:17PM +0300, Amir Goldstein wrote:
> > Overlayfs and cachefiles use vfs_open_tmpfile() to open a tmpfile
> > without accounting for nr_files.
> >
> > Rename this helper to kernel_tmpfile_open() to better reflect this
> > helper is used for kernel internal users.
> >
> > cachefiles uses open_with_fake_path() without the need for a fake path
> > only to use the noaccount feature of open_with_fake_path().
> >
> > Fork open_with_fake_path() to kernel_file_open() which only does the
> > noaccount feature and use it in cachefiles.
>
> Please split this into two patches, one for the
> vfs_tmpfile_open rename, and one for the kernel_file_open helper.
>
> > +EXPORT_SYMBOL(kernel_file_open);
>
> EXPORT_SYMBOL_GPL, please.

Hmm. Technically, the renamed symbol {vfs,kernel}_tmpfile_open is a
new symbol so we can make it EXPORT_SYMBOL_GPL too.
The only users should be overlayfs and cachefiles.

I had already posted v5 without changing it, but if you think that is right,
I can ask Christian to change that on commit.

Thanks,
Amir.
diff mbox series

Patch

diff --git a/fs/cachefiles/namei.c b/fs/cachefiles/namei.c
index 82219a8f6084..499cf73f097b 100644
--- a/fs/cachefiles/namei.c
+++ b/fs/cachefiles/namei.c
@@ -451,9 +451,9 @@  struct file *cachefiles_create_tmpfile(struct cachefiles_object *object)
 
 	ret = cachefiles_inject_write_error();
 	if (ret == 0) {
-		file = vfs_tmpfile_open(&nop_mnt_idmap, &parentpath, S_IFREG,
-					O_RDWR | O_LARGEFILE | O_DIRECT,
-					cache->cache_cred);
+		file = kernel_tmpfile_open(&nop_mnt_idmap, &parentpath, S_IFREG,
+					   O_RDWR | O_LARGEFILE | O_DIRECT,
+					   cache->cache_cred);
 		ret = PTR_ERR_OR_ZERO(file);
 	}
 	if (ret) {
@@ -560,8 +560,8 @@  static bool cachefiles_open_file(struct cachefiles_object *object,
 	 */
 	path.mnt = cache->mnt;
 	path.dentry = dentry;
-	file = open_with_fake_path(&path, O_RDWR | O_LARGEFILE | O_DIRECT,
-				   d_backing_inode(dentry), cache->cache_cred);
+	file = kernel_file_open(&path, O_RDWR | O_LARGEFILE | O_DIRECT,
+				d_backing_inode(dentry), cache->cache_cred);
 	if (IS_ERR(file)) {
 		trace_cachefiles_vfs_error(object, d_backing_inode(dentry),
 					   PTR_ERR(file),
diff --git a/fs/namei.c b/fs/namei.c
index e4fe0879ae55..36e335c39c44 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -3703,7 +3703,7 @@  static int vfs_tmpfile(struct mnt_idmap *idmap,
 }
 
 /**
- * vfs_tmpfile_open - open a tmpfile for kernel internal use
+ * kernel_tmpfile_open - open a tmpfile for kernel internal use
  * @idmap:	idmap of the mount the inode was found from
  * @parentpath:	path of the base directory
  * @mode:	mode of the new tmpfile
@@ -3714,24 +3714,26 @@  static int vfs_tmpfile(struct mnt_idmap *idmap,
  * hence this is only for kernel internal use, and must not be installed into
  * file tables or such.
  */
-struct file *vfs_tmpfile_open(struct mnt_idmap *idmap,
-			  const struct path *parentpath,
-			  umode_t mode, int open_flag, const struct cred *cred)
+struct file *kernel_tmpfile_open(struct mnt_idmap *idmap,
+				 const struct path *parentpath,
+				 umode_t mode, int open_flag,
+				 const struct cred *cred)
 {
 	struct file *file;
 	int error;
 
 	file = alloc_empty_file_noaccount(open_flag, cred);
-	if (!IS_ERR(file)) {
-		error = vfs_tmpfile(idmap, parentpath, file, mode);
-		if (error) {
-			fput(file);
-			file = ERR_PTR(error);
-		}
+	if (IS_ERR(file))
+		return file;
+
+	error = vfs_tmpfile(idmap, parentpath, file, mode);
+	if (error) {
+		fput(file);
+		file = ERR_PTR(error);
 	}
 	return file;
 }
-EXPORT_SYMBOL(vfs_tmpfile_open);
+EXPORT_SYMBOL(kernel_tmpfile_open);
 
 static int do_tmpfile(struct nameidata *nd, unsigned flags,
 		const struct open_flags *op,
diff --git a/fs/open.c b/fs/open.c
index 005ca91a173b..c3491ecd9ae8 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -1121,6 +1121,37 @@  struct file *dentry_create(const struct path *path, int flags, umode_t mode,
 }
 EXPORT_SYMBOL(dentry_create);
 
+/**
+ * kernel_file_open - open a file for kernel internal use
+ * @path:	path of the file to open
+ * @flags:	open flags
+ * @inode:	the inode
+ * @cred:	credentials for open
+ *
+ * Open a file that is not accounted in nr_files.
+ * This is only for kernel internal use, and must not be installed into
+ * file tables or such.
+ */
+struct file *kernel_file_open(const struct path *path, int flags,
+				struct inode *inode, const struct cred *cred)
+{
+	struct file *f;
+	int error;
+
+	f = alloc_empty_file_noaccount(flags, cred);
+	if (IS_ERR(f))
+		return f;
+
+	f->f_path = *path;
+	error = do_dentry_open(f, inode, NULL);
+	if (error) {
+		fput(f);
+		f = ERR_PTR(error);
+	}
+	return f;
+}
+EXPORT_SYMBOL(kernel_file_open);
+
 struct file *open_with_fake_path(const struct path *path, int flags,
 				struct inode *inode, const struct cred *cred)
 {
diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index fcac4e2c56ab..6129f0984cf7 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -329,8 +329,9 @@  static inline struct file *ovl_do_tmpfile(struct ovl_fs *ofs,
 					  struct dentry *dentry, umode_t mode)
 {
 	struct path path = { .mnt = ovl_upper_mnt(ofs), .dentry = dentry };
-	struct file *file = vfs_tmpfile_open(ovl_upper_mnt_idmap(ofs), &path, mode,
-					O_LARGEFILE | O_WRONLY, current_cred());
+	struct file *file = kernel_tmpfile_open(ovl_upper_mnt_idmap(ofs), &path,
+						mode, O_LARGEFILE | O_WRONLY,
+						current_cred());
 	int err = PTR_ERR_OR_ZERO(file);
 
 	pr_debug("tmpfile(%pd2, 0%o) = %i\n", dentry, mode, err);
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 21a981680856..1f8486e773af 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1672,9 +1672,12 @@  static inline int vfs_whiteout(struct mnt_idmap *idmap,
 			 WHITEOUT_DEV);
 }
 
-struct file *vfs_tmpfile_open(struct mnt_idmap *idmap,
-			const struct path *parentpath,
-			umode_t mode, int open_flag, const struct cred *cred);
+struct file *kernel_tmpfile_open(struct mnt_idmap *idmap,
+				 const struct path *parentpath,
+				 umode_t mode, int open_flag,
+				 const struct cred *cred);
+struct file *kernel_file_open(const struct path *path, int flags,
+			      struct inode *inode, const struct cred *cred);
 
 int vfs_mkobj(struct dentry *, umode_t,
 		int (*f)(struct dentry *, umode_t, void *),