On Mon 21-11-22 15:28:53, Lukas Czerner wrote:
> Implement user and group quota support for tmpfs using system quota file
> in vfsv0 quota format. Because everything in tmpfs is temporary and as a
> result is lost on umount, the quota files are initialized on every
> mount. This also goes for quota limits, that needs to be set up after
> every mount.
> 
> The quota support in tmpfs is well separated from the rest of the
> filesystem and is only enabled using mount option -o quota (and
> usrquota and grpquota for compatibility reasons). Only quota accounting
> is enabled this way, enforcement needs to be enable by regular quota
> tools (using Q_QUOTAON ioctl).
> 
> Signed-off-by: Lukas Czerner <lczerner@redhat.com>

...

> diff --git a/Documentation/filesystems/tmpfs.rst b/Documentation/filesystems/tmpfs.rst
> index 0408c245785e..9c4f228ef4f3 100644
> --- a/Documentation/filesystems/tmpfs.rst
> +++ b/Documentation/filesystems/tmpfs.rst
> @@ -86,6 +86,18 @@ use up all the memory on the machine; but enhances the scalability of
>  that instance in a system with many CPUs making intensive use of it.
>  
>  
> +tmpfs also supports quota with the following mount options
> +
> +========  =============================================================
> +quota     Quota accounting is enabled on the mount. Tmpfs is using
> +          hidden system quota files that are initialized on mount.
> +          Quota limits can quota enforcement can be enabled using
                          ^^^ and?

> +          standard quota tools.
> +usrquota  Same as quota option. Exists for compatibility reasons.
> +grpquota  Same as quota option. Exists for compatibility reasons.

As we discussed with V1, I'd prefer if user & group quotas could be enabled
/ disabled independently. Mostly to not differ from other filesystems
unnecessarily.

> +========  =============================================================
> +
> +
>  tmpfs has a mount option to set the NUMA memory allocation policy for
>  all files in that instance (if CONFIG_NUMA is enabled) - which can be
>  adjusted on the fly via 'mount -o remount ...'
> diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c
> index f1a7a03632a2..007604e7eb09 100644
> --- a/fs/quota/dquot.c
> +++ b/fs/quota/dquot.c
> @@ -716,11 +716,11 @@ int dquot_quota_sync(struct super_block *sb, int type)
>  	for (cnt = 0; cnt < MAXQUOTAS; cnt++) {
>  		if (type != -1 && cnt != type)
>  			continue;
> -		if (!sb_has_quota_active(sb, cnt))
> -			continue;
> -		inode_lock(dqopt->files[cnt]);
> -		truncate_inode_pages(&dqopt->files[cnt]->i_data, 0);
> -		inode_unlock(dqopt->files[cnt]);
> +		if (sb_has_quota_active(sb, cnt) && dqopt->files[cnt]) {
> +			inode_lock(dqopt->files[cnt]);
> +			truncate_inode_pages(&dqopt->files[cnt]->i_data, 0);
> +			inode_unlock(dqopt->files[cnt]);
> +		}
>  	}

No need to mess with this when you have DQUOT_QUOTA_SYS_FILE set.

> +/*
> + * We don't have any quota files to read, or write to/from, but quota code
> + * requires .quota_read and .quota_write to exist.
> + */
> +static ssize_t shmem_quota_write(struct super_block *sb, int type,
> +				const char *data, size_t len, loff_t off)
> +{
> +	return len;
> +}
> +
> +static ssize_t shmem_quota_read(struct super_block *sb, int type, char *data,
> +			       size_t len, loff_t off)
> +{
> +	return len;
> +}

Instead of these functions I'd go for attached patch.

> @@ -363,7 +438,7 @@ bool shmem_charge(struct inode *inode, long pages)
>  	struct shmem_inode_info *info = SHMEM_I(inode);
>  	unsigned long flags;
>  
> -	if (!shmem_inode_acct_block(inode, pages))
> +	if (shmem_inode_acct_block(inode, pages))
>  		return false;

As Brian asked, I'd prefer to have the calling convention change as a
separate patch.

> +static struct inode *shmem_get_inode(struct super_block *sb, struct inode *dir,
> +				     umode_t mode, dev_t dev, unsigned long flags)
> +{
> +	int err;
> +	struct inode *inode;
> +
> +	inode = shmem_get_inode_noquota(sb, dir, mode, dev, flags);
> +	if (inode) {
> +		err = dquot_initialize(inode);
> +		if (err)
> +			goto errout;
> +
> +		err = dquot_alloc_inode(inode);
> +		if (err) {
> +			dquot_drop(inode);
> +			goto errout;
> +		}
> +	}
> +	return inode;

I'd rather make shmem_get_inode() always return ERR_PTR or inode pointer.
It's more natural convention. Also this calling convention change should
go into a separate patch.

> +
> +errout:
> +	inode->i_flags |= S_NOQUOTA;
> +	iput(inode);
> +	shmem_free_inode(sb);
> +	if (err)
> +		return ERR_PTR(err);
> +	return NULL;
> +}
> +
...

> @@ -4196,8 +4348,10 @@ static struct file *__shmem_file_setup(struct vfsmount *mnt, const char *name, l
>  
>  	inode = shmem_get_inode(mnt->mnt_sb, NULL, S_IFREG | S_IRWXUGO, 0,
>  				flags);
> -	if (unlikely(!inode)) {
> +	if (IS_ERR_OR_NULL(inode)) {
>  		shmem_unacct_size(flags, size);
> +		if (IS_ERR(inode))
> +			return (struct file *)inode;
				^^ Uhuh. ERR_CAST()?

>  		return ERR_PTR(-ENOSPC);
>  	}
>  	inode->i_flags |= i_flags;


								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR