Re: [PATCH] fs: rename S_KERNEL_FILE

[Amir Goldstein <amir73il@gmail.com>]

On Sat, Jan 22, 2022 at 2:40 AM Christoph Hellwig <hch@lst.de> wrote:
>
> S_KERNEL_FILE is grossly misnamed.  We have plenty of files hold open by
> the kernel kernel using filp_open.  This flag OTOH signals the inode as
> being a special snowflage that cachefiles holds onto that can't be

^^^^^^^^^^^^^^^^snowflake

> unlinked becaue of ..., erm, pixie dust.  So clearly mark it as such.
>
^^^^^^^^^^because

> Signed-off-by: Christoph Hellwig <hch@lst.de>

That's a much better name IMO.

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

> ---
>  fs/cachefiles/namei.c | 12 ++++++------
>  fs/namei.c            |  2 +-
>  include/linux/fs.h    |  2 +-
>  3 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/fs/cachefiles/namei.c b/fs/cachefiles/namei.c
> index 9bd692870617c..599dc13a7d9ab 100644
> --- a/fs/cachefiles/namei.c
> +++ b/fs/cachefiles/namei.c
> @@ -20,8 +20,8 @@ static bool __cachefiles_mark_inode_in_use(struct cachefiles_object *object,
>         struct inode *inode = d_backing_inode(dentry);
>         bool can_use = false;
>
> -       if (!(inode->i_flags & S_KERNEL_FILE)) {
> -               inode->i_flags |= S_KERNEL_FILE;
> +       if (!(inode->i_flags & S_CACHEFILE)) {
> +               inode->i_flags |= S_CACHEFILE;
>                 trace_cachefiles_mark_active(object, inode);
>                 can_use = true;
>         } else {
> @@ -51,7 +51,7 @@ static void __cachefiles_unmark_inode_in_use(struct cachefiles_object *object,
>  {
>         struct inode *inode = d_backing_inode(dentry);
>
> -       inode->i_flags &= ~S_KERNEL_FILE;
> +       inode->i_flags &= ~S_CACHEFILE;
>         trace_cachefiles_mark_inactive(object, inode);
>  }
>
> @@ -742,7 +742,7 @@ static struct dentry *cachefiles_lookup_for_cull(struct cachefiles_cache *cache,
>                 goto lookup_error;
>         if (d_is_negative(victim))
>                 goto lookup_put;
> -       if (d_inode(victim)->i_flags & S_KERNEL_FILE)
> +       if (d_inode(victim)->i_flags & S_CACHEFILE)
>                 goto lookup_busy;
>         return victim;
>
> @@ -789,11 +789,11 @@ int cachefiles_cull(struct cachefiles_cache *cache, struct dentry *dir,
>         /* check to see if someone is using this object */
>         inode = d_inode(victim);
>         inode_lock(inode);
> -       if (inode->i_flags & S_KERNEL_FILE) {
> +       if (inode->i_flags & S_CACHEFILE) {
>                 ret = -EBUSY;
>         } else {
>                 /* Stop the cache from picking it back up */
> -               inode->i_flags |= S_KERNEL_FILE;
> +               inode->i_flags |= S_CACHEFILE;
>                 ret = 0;
>         }
>         inode_unlock(inode);
> diff --git a/fs/namei.c b/fs/namei.c
> index d81f04f8d8188..7402277ecc1f5 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -3959,7 +3959,7 @@ int vfs_rmdir(struct user_namespace *mnt_userns, struct inode *dir,
>
>         error = -EBUSY;
>         if (is_local_mountpoint(dentry) ||
> -           (dentry->d_inode->i_flags & S_KERNEL_FILE))
> +           (dentry->d_inode->i_flags & S_CACHEFILE))
>                 goto out;
>
>         error = security_inode_rmdir(dir, dentry);
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index c8510da6cc6dc..099d7e03d46e6 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -2173,7 +2173,7 @@ struct super_operations {
>  #define S_ENCRYPTED    (1 << 14) /* Encrypted file (using fs/crypto/) */
>  #define S_CASEFOLD     (1 << 15) /* Casefolded file */
>  #define S_VERITY       (1 << 16) /* Verity file (using fs/verity/) */
> -#define S_KERNEL_FILE  (1 << 17) /* File is in use by the kernel (eg. fs/cachefiles) */
> +#define S_CACHEFILE    (1 << 17) /* In use as cachefile, can't be unlinked */
>
>  /*
>   * Note that nosuid etc flags are inode-specific: setting some file-system
> --
> 2.30.2
>