| Message ID | 20230611132732.1502040-2-amir73il@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Handle notifications on overlayfs fake path files | expand |
On Sun, Jun 11, 2023 at 04:27:30PM +0300, Amir Goldstein wrote: > Rename the flag FMODE_NOACCOUNT that is used to mark internal files of > overlayfs and cachefiles to the more generic name FMODE_INTERNAL, which > also indicates that the file's f_path is possibly "fake". FMODE_INTERNAL is completely meaningless. Plase come up with a name that actually explain what is special about these files.
On Mon, Jun 12, 2023 at 7:27 AM Christoph Hellwig <hch@infradead.org> wrote: > > On Sun, Jun 11, 2023 at 04:27:30PM +0300, Amir Goldstein wrote: > > Rename the flag FMODE_NOACCOUNT that is used to mark internal files of > > overlayfs and cachefiles to the more generic name FMODE_INTERNAL, which > > also indicates that the file's f_path is possibly "fake". > > FMODE_INTERNAL is completely meaningless. Plase come up with a name > that actually explain what is special about these files. > Well, I am not sure if FMODE_FAKE_PATH in v3 is a better name, because you did rightfully say that "fake path" is not that descriptive, but I will think of a better way to describe "fake path" and match the flag to the file container name. Thanks, Amir.
On Mon, Jun 12, 2023 at 09:08:37AM +0300, Amir Goldstein wrote: > Well, I am not sure if FMODE_FAKE_PATH in v3 is a better name, > because you did rightfully say that "fake path" is not that descriptive, > but I will think of a better way to describe "fake path" and match the > flag to the file container name. I suspect the just claling it out what it is and naming it FMODE_OVERLAYFS might be a good idea. We'd just need to make sure not to set it for the cachefiles use case, which is probably a good idea anyway.
On Sun, Jun 11, 2023 at 11:11:25PM -0700, Christoph Hellwig wrote: > On Mon, Jun 12, 2023 at 09:08:37AM +0300, Amir Goldstein wrote: > > Well, I am not sure if FMODE_FAKE_PATH in v3 is a better name, > > because you did rightfully say that "fake path" is not that descriptive, > > but I will think of a better way to describe "fake path" and match the > > flag to the file container name. > > I suspect the just claling it out what it is and naming it > FMODE_OVERLAYFS might be a good idea. We'd just need to make sure not > to set it for the cachefiles use case, which is probably a good idea > anyway. Adding Dave: not sure if I'm missing something, but is there any good reason cachefs doesn't juse use dentry_open() ?
On Mon, Jun 12, 2023 at 9:11 AM Christoph Hellwig <hch@infradead.org> wrote: > > On Mon, Jun 12, 2023 at 09:08:37AM +0300, Amir Goldstein wrote: > > Well, I am not sure if FMODE_FAKE_PATH in v3 is a better name, > > because you did rightfully say that "fake path" is not that descriptive, > > but I will think of a better way to describe "fake path" and match the > > flag to the file container name. > > I suspect the just claling it out what it is and naming it > FMODE_OVERLAYFS might be a good idea. We'd just need to make sure not > to set it for the cachefiles use case, which is probably a good idea > anyway. Agree to both. As I told Christian, I was reluctant to use the last available flag bit (although you did free up a couple of flags:)), but making FMODE_OVERLAYFS overlayfs only and keeping cachefiles with FMODE_NOACCOUNT would be the cleaner thing to do. Thanks, Amir.
On Mon, Jun 12, 2023 at 09:32:05AM +0300, Amir Goldstein wrote: > Agree to both. > As I told Christian, I was reluctant to use the last available flag bit > (although you did free up a couple of flags:)), but making > FMODE_OVERLAYFS overlayfs only and keeping cachefiles with > FMODE_NOACCOUNT would be the cleaner thing to do. Please go ahead with the bit. In addition to the block series I plan to move all static flags from f_mode to a new f_op.flags field in the next merge window, which should help with avaiability.
diff --git a/fs/file_table.c b/fs/file_table.c index 372653b92617..d64d3933f3e4 100644 --- a/fs/file_table.c +++ b/fs/file_table.c @@ -55,7 +55,7 @@ static void file_free_rcu(struct rcu_head *head) static inline void file_free(struct file *f) { security_file_free(f); - if (!(f->f_mode & FMODE_NOACCOUNT)) + if (!(f->f_mode & FMODE_INTERNAL)) percpu_counter_dec(&nr_files); call_rcu(&f->f_rcuhead, file_free_rcu); } @@ -205,12 +205,12 @@ struct file *alloc_empty_file(int flags, const struct cred *cred) * * Should not be used unless there's a very good reason to do so. */ -struct file *alloc_empty_file_noaccount(int flags, const struct cred *cred) +struct file *alloc_empty_file_internal(int flags, const struct cred *cred) { struct file *f = __alloc_file(flags, cred); if (!IS_ERR(f)) - f->f_mode |= FMODE_NOACCOUNT; + f->f_mode |= FMODE_INTERNAL; return f; } diff --git a/fs/internal.h b/fs/internal.h index bd3b2810a36b..018605caf597 100644 --- a/fs/internal.h +++ b/fs/internal.h @@ -97,8 +97,9 @@ extern void chroot_fs_refs(const struct path *, const struct path *); /* * file_table.c */ -extern struct file *alloc_empty_file(int, const struct cred *); -extern struct file *alloc_empty_file_noaccount(int, const struct cred *); +extern struct file *alloc_empty_file(int flags, const struct cred *cred); +extern struct file *alloc_empty_file_internal(int flags, + const struct cred *cred); static inline void put_file_access(struct file *file) { diff --git a/fs/namei.c b/fs/namei.c index e4fe0879ae55..167f5acb0acf 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -3721,7 +3721,7 @@ struct file *vfs_tmpfile_open(struct mnt_idmap *idmap, struct file *file; int error; - file = alloc_empty_file_noaccount(open_flag, cred); + file = alloc_empty_file_internal(open_flag, cred); if (!IS_ERR(file)) { error = vfs_tmpfile(idmap, parentpath, file, mode); if (error) { diff --git a/fs/open.c b/fs/open.c index 81444ebf6091..23f862708a4f 100644 --- a/fs/open.c +++ b/fs/open.c @@ -1124,7 +1124,7 @@ EXPORT_SYMBOL(dentry_create); struct file *open_with_fake_path(const struct path *path, int flags, struct inode *inode, const struct cred *cred) { - struct file *f = alloc_empty_file_noaccount(flags, cred); + struct file *f = alloc_empty_file_internal(flags, cred); if (!IS_ERR(f)) { int error; diff --git a/include/linux/fs.h b/include/linux/fs.h index 21a981680856..13eec1e8ca86 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -180,8 +180,8 @@ typedef int (dio_iodone_t)(struct kiocb *iocb, loff_t offset, /* File represents mount that needs unmounting */ #define FMODE_NEED_UNMOUNT ((__force fmode_t)0x10000000) -/* File does not contribute to nr_files count */ -#define FMODE_NOACCOUNT ((__force fmode_t)0x20000000) +/* File is kernel internal does not contribute to nr_files count */ +#define FMODE_INTERNAL ((__force fmode_t)0x20000000) /* File supports async buffered reads */ #define FMODE_BUF_RASYNC ((__force fmode_t)0x40000000)
Rename the flag FMODE_NOACCOUNT that is used to mark internal files of overlayfs and cachefiles to the more generic name FMODE_INTERNAL, which also indicates that the file's f_path is possibly "fake". Signed-off-by: Amir Goldstein <amir73il@gmail.com> --- fs/file_table.c | 6 +++--- fs/internal.h | 5 +++-- fs/namei.c | 2 +- fs/open.c | 2 +- include/linux/fs.h | 4 ++-- 5 files changed, 10 insertions(+), 9 deletions(-)