Re: [PATCH vfs.tmpfs 2/5] tmpfs: track free_ispace instead of free_inodes

[Carlos Maiolino <cem@kernel.org>]

On Tue, Aug 08, 2023 at 09:32:21PM -0700, Hugh Dickins wrote:
> In preparation for assigning some inode space to extended attributes,
> keep track of free_ispace instead of number of free_inodes: as if one
> tmpfs inode (and accompanying dentry) occupies very approximately 1KiB.
> 
> Unsigned long is large enough for free_ispace, on 64-bit and on 32-bit:
> but take care to enforce the maximum.  And fix the nr_blocks maximum on
> 32-bit: S64_MAX would be too big for it there, so say LONG_MAX instead.
> 
> Delete the incorrect limited<->unlimited blocks/inodes comment above
> shmem_reconfigure(): leave it to the error messages below to describe.
> 
> Signed-off-by: Hugh Dickins <hughd@google.com>

Looks fine, feel free to add:
Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>

> ---
>  include/linux/shmem_fs.h |  2 +-
>  mm/shmem.c               | 33 +++++++++++++++++----------------
>  2 files changed, 18 insertions(+), 17 deletions(-)
> 
> diff --git a/include/linux/shmem_fs.h b/include/linux/shmem_fs.h
> index 9b2d2faff1d0..6b0c626620f5 100644
> --- a/include/linux/shmem_fs.h
> +++ b/include/linux/shmem_fs.h
> @@ -54,7 +54,7 @@ struct shmem_sb_info {
>  	unsigned long max_blocks;   /* How many blocks are allowed */
>  	struct percpu_counter used_blocks;  /* How many are allocated */
>  	unsigned long max_inodes;   /* How many inodes are allowed */
> -	unsigned long free_inodes;  /* How many are left for allocation */
> +	unsigned long free_ispace;  /* How much ispace left for allocation */
>  	raw_spinlock_t stat_lock;   /* Serialize shmem_sb_info changes */
>  	umode_t mode;		    /* Mount mode for root directory */
>  	unsigned char huge;	    /* Whether to try for hugepages */
> diff --git a/mm/shmem.c b/mm/shmem.c
> index df3cabf54206..c39471384168 100644
> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -90,6 +90,9 @@ static struct vfsmount *shm_mnt;
>  /* Pretend that each entry is of this size in directory's i_size */
>  #define BOGO_DIRENT_SIZE 20
> 
> +/* Pretend that one inode + its dentry occupy this much memory */
> +#define BOGO_INODE_SIZE 1024
> +
>  /* Symlink up to this size is kmalloc'ed instead of using a swappable page */
>  #define SHORT_SYMLINK_LEN 128
> 
> @@ -137,7 +140,8 @@ static unsigned long shmem_default_max_inodes(void)
>  {
>  	unsigned long nr_pages = totalram_pages();
> 
> -	return min(nr_pages - totalhigh_pages(), nr_pages / 2);
> +	return min3(nr_pages - totalhigh_pages(), nr_pages / 2,
> +			ULONG_MAX / BOGO_INODE_SIZE);
>  }
>  #endif
> 
> @@ -331,11 +335,11 @@ static int shmem_reserve_inode(struct super_block *sb, ino_t *inop)
>  	if (!(sb->s_flags & SB_KERNMOUNT)) {
>  		raw_spin_lock(&sbinfo->stat_lock);
>  		if (sbinfo->max_inodes) {
> -			if (!sbinfo->free_inodes) {
> +			if (sbinfo->free_ispace < BOGO_INODE_SIZE) {
>  				raw_spin_unlock(&sbinfo->stat_lock);
>  				return -ENOSPC;
>  			}
> -			sbinfo->free_inodes--;
> +			sbinfo->free_ispace -= BOGO_INODE_SIZE;
>  		}
>  		if (inop) {
>  			ino = sbinfo->next_ino++;
> @@ -394,7 +398,7 @@ static void shmem_free_inode(struct super_block *sb)
>  	struct shmem_sb_info *sbinfo = SHMEM_SB(sb);
>  	if (sbinfo->max_inodes) {
>  		raw_spin_lock(&sbinfo->stat_lock);
> -		sbinfo->free_inodes++;
> +		sbinfo->free_ispace += BOGO_INODE_SIZE;
>  		raw_spin_unlock(&sbinfo->stat_lock);
>  	}
>  }
> @@ -3155,7 +3159,7 @@ static int shmem_statfs(struct dentry *dentry, struct kstatfs *buf)
>  	}
>  	if (sbinfo->max_inodes) {
>  		buf->f_files = sbinfo->max_inodes;
> -		buf->f_ffree = sbinfo->free_inodes;
> +		buf->f_ffree = sbinfo->free_ispace / BOGO_INODE_SIZE;
>  	}
>  	/* else leave those fields 0 like simple_statfs */
> 
> @@ -3815,13 +3819,13 @@ static int shmem_parse_one(struct fs_context *fc, struct fs_parameter *param)
>  		break;
>  	case Opt_nr_blocks:
>  		ctx->blocks = memparse(param->string, &rest);
> -		if (*rest || ctx->blocks > S64_MAX)
> +		if (*rest || ctx->blocks > LONG_MAX)
>  			goto bad_value;
>  		ctx->seen |= SHMEM_SEEN_BLOCKS;
>  		break;
>  	case Opt_nr_inodes:
>  		ctx->inodes = memparse(param->string, &rest);
> -		if (*rest)
> +		if (*rest || ctx->inodes > ULONG_MAX / BOGO_INODE_SIZE)
>  			goto bad_value;
>  		ctx->seen |= SHMEM_SEEN_INODES;
>  		break;
> @@ -4002,21 +4006,17 @@ static int shmem_parse_options(struct fs_context *fc, void *data)
> 
>  /*
>   * Reconfigure a shmem filesystem.
> - *
> - * Note that we disallow change from limited->unlimited blocks/inodes while any
> - * are in use; but we must separately disallow unlimited->limited, because in
> - * that case we have no record of how much is already in use.
>   */
>  static int shmem_reconfigure(struct fs_context *fc)
>  {
>  	struct shmem_options *ctx = fc->fs_private;
>  	struct shmem_sb_info *sbinfo = SHMEM_SB(fc->root->d_sb);
> -	unsigned long inodes;
> +	unsigned long used_isp;
>  	struct mempolicy *mpol = NULL;
>  	const char *err;
> 
>  	raw_spin_lock(&sbinfo->stat_lock);
> -	inodes = sbinfo->max_inodes - sbinfo->free_inodes;
> +	used_isp = sbinfo->max_inodes * BOGO_INODE_SIZE - sbinfo->free_ispace;
> 
>  	if ((ctx->seen & SHMEM_SEEN_BLOCKS) && ctx->blocks) {
>  		if (!sbinfo->max_blocks) {
> @@ -4034,7 +4034,7 @@ static int shmem_reconfigure(struct fs_context *fc)
>  			err = "Cannot retroactively limit inodes";
>  			goto out;
>  		}
> -		if (ctx->inodes < inodes) {
> +		if (ctx->inodes * BOGO_INODE_SIZE < used_isp) {
>  			err = "Too few inodes for current use";
>  			goto out;
>  		}
> @@ -4080,7 +4080,7 @@ static int shmem_reconfigure(struct fs_context *fc)
>  		sbinfo->max_blocks  = ctx->blocks;
>  	if (ctx->seen & SHMEM_SEEN_INODES) {
>  		sbinfo->max_inodes  = ctx->inodes;
> -		sbinfo->free_inodes = ctx->inodes - inodes;
> +		sbinfo->free_ispace = ctx->inodes * BOGO_INODE_SIZE - used_isp;
>  	}
> 
>  	/*
> @@ -4211,7 +4211,8 @@ static int shmem_fill_super(struct super_block *sb, struct fs_context *fc)
>  	sb->s_flags |= SB_NOUSER;
>  #endif
>  	sbinfo->max_blocks = ctx->blocks;
> -	sbinfo->free_inodes = sbinfo->max_inodes = ctx->inodes;
> +	sbinfo->max_inodes = ctx->inodes;
> +	sbinfo->free_ispace = sbinfo->max_inodes * BOGO_INODE_SIZE;
>  	if (sb->s_flags & SB_KERNMOUNT) {
>  		sbinfo->ino_batch = alloc_percpu(ino_t);
>  		if (!sbinfo->ino_batch)
> --
> 2.35.3
>