Linux-mtd Archive on lore.kernel.org
 help / color / Atom feed
From: Jan Kara <jack@suse.cz>
To: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>,
	Richard Weinberger <richard@nod.at>,
	linux-mtd@lists.infradead.org, kernel@pengutronix.de,
	Jan Kara <jack@suse.com>,
	linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH 11/11] ubifs: Add quota support
Date: Thu, 15 Aug 2019 13:17:21 +0200
Message-ID: <20190815111721.GC14313@quack2.suse.cz> (raw)
In-Reply-To: <20190814121834.13983-12-s.hauer@pengutronix.de>

Hello,

On Wed 14-08-19 14:18:34, Sascha Hauer wrote:
> From: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
> 
> This introduces poor man's quota support for UBIFS. Unlike other
> implementations we never store anything on the flash. This has two
> big advantages:
> 
> - No possible regressions with a changed on-disk format
> - no quota files can get out of sync.
> 
> There are downsides aswell:
> 
> - During mount the whole index must be scanned which takes some time
> - The quota limits must be set manually each time a filesystem is mounted.
> 
> UBIFS is targetted for embedded systems and quota limits are likely not
> changed interactively, so having to restore the quota limits with a
> script shouldn't be a big deal. The mount time penalty is a price we
> must pay, but for that we get a simple and straight forward
> implementation for this rather rarely used feature.
> 
> The quota data itself is stored in a red-black tree in memory. It is
> implemented as a quota format. When enabled with the "quota" mount
> option all three quota types (user, group, project) are enabled.
> 
> The quota integration into UBIFS is taken from a series posted earlier
> by Dongsheng Yang. Like the earlier series we only account regular files
> for quota. All others are counted in the number of files, but do not
> require any quota space.
> 
> iSigned-off-by: Sascha Hauer <s.hauer@pengutronix.de>

Missing Signed-off-by from Dongsheng? Also yours has 'i' there.

> +/**
> + * ubifs_enable_quotas - enable quota
> + * @c: UBIFS file-system description object
> + *
> + * Enable usage tracking for all quota types.
> + */
> +int ubifs_enable_quotas(struct ubifs_info *c)
> +{
> +	struct super_block *sb = c->vfs_sb;
> +	struct quota_info *dqopt = sb_dqopt(sb);
> +	int type;
> +
> +	if (!c->quota_enable)
> +		return 0;
> +
> +	dqopt->flags |= DQUOT_QUOTA_SYS_FILE | DQUOT_NOLIST_DIRTY;
> +
> +	for (type = 0; type < UBIFS_MAXQUOTAS; type++) {
> +		struct mem_dqinfo *info = sb_dqinfo(sb, type);
> +		unsigned int flags = DQUOT_USAGE_ENABLED | DQUOT_LIMITS_ENABLED;
> +
> +		dqopt->flags |= dquot_state_flag(flags, type);
> +		dqopt->info[type].dqi_flags |= DQF_SYS_FILE;
> +		dqopt->ops[type] = &ubifs_format_ops;
> +
> +		info->dqi_max_spc_limit = 0x7fffffffffffffffLL;
> +		info->dqi_max_ino_limit = 0x7fffffffffffffffLL;

This is wrong. You shouldn't mess with quota internals like that. You
should use dquot_enable() (and you even implemented ->read_file_info()
format operation to properly fill in info structure).  So you just need to
change dquot_enable() to cope with situation when inode is NULL. Probably
create a variant dquot_enable_sb() that gets superblock pointer instead of
inode and then factor out bits from vfs_load_quota_inode() that are needed
also for the case without quota inode.

> @@ -956,6 +970,7 @@ static const match_table_t tokens = {
>  	{Opt_ignore, "ubi=%s"},
>  	{Opt_ignore, "vol=%s"},
>  	{Opt_assert, "assert=%s"},
> +	{Opt_quota, "quota"},
>  	{Opt_err, NULL},
>  };

Usually, we have usrquota, grpquota, prjquota mount options to enable
individual quota types. It would seem better not to differ from these
unless you have a good reason.

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

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

      reply index

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-14 12:18 [PATCH 00/11] Add quota support to UBIFS Sascha Hauer
2019-08-14 12:18 ` [PATCH 01/11] quota: Make inode optional Sascha Hauer
2019-08-14 12:18 ` [PATCH 02/11] quota: Only module_put the format when existing Sascha Hauer
2019-08-15 11:18   ` Jan Kara
2019-08-16 11:49     ` Sascha Hauer
2019-08-14 12:18 ` [PATCH 03/11] fs: move __get_super() out of loop Sascha Hauer
2019-08-14 23:32   ` Al Viro
2019-08-14 12:18 ` [PATCH 04/11] fs, quota: introduce wait_super_thawed() to wait until a superblock is thawed Sascha Hauer
2019-08-14 23:35   ` Al Viro
2019-08-14 12:18 ` [PATCH 05/11] quota: Allow to pass quotactl a mountpoint Sascha Hauer
2019-08-14 22:42   ` kbuild test robot
2019-08-14 23:33   ` kbuild test robot
2019-08-14 23:36   ` Al Viro
2019-08-14 23:39     ` Al Viro
2019-08-14 23:51       ` Al Viro
2019-08-15  9:53         ` Jan Kara
2019-08-15  7:46       ` Sascha Hauer
2019-08-14 12:18 ` [PATCH 06/11] ubifs: move checks and preparation into setflags() Sascha Hauer
2019-08-14 12:18 ` [PATCH 07/11] ubifs: Add support for FS_IOC_FS[SG]ETXATTR ioctls Sascha Hauer
2019-08-14 14:11   ` Mainz, Roland
2019-08-15  7:31     ` Sascha Hauer
2019-08-14 12:18 ` [PATCH 08/11] ubifs: do not ubifs_inode() on potentially NULL pointer Sascha Hauer
2019-08-14 12:18 ` [PATCH 09/11] ubifs: Add support for project id Sascha Hauer
2019-08-14 12:18 ` [PATCH 10/11] ubifs: export get_znode Sascha Hauer
2019-08-14 12:18 ` [PATCH 11/11] ubifs: Add quota support Sascha Hauer
2019-08-15 11:17   ` Jan Kara [this message]

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190815111721.GC14313@quack2.suse.cz \
    --to=jack@suse.cz \
    --cc=jack@suse.com \
    --cc=kernel@pengutronix.de \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=richard@nod.at \
    --cc=s.hauer@pengutronix.de \
    --cc=yangds.fnst@cn.fujitsu.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-mtd Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-mtd/0 linux-mtd/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-mtd linux-mtd/ https://lore.kernel.org/linux-mtd \
		linux-mtd@lists.infradead.org linux-mtd@archiver.kernel.org
	public-inbox-index linux-mtd


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.infradead.lists.linux-mtd


AGPL code for this site: git clone https://public-inbox.org/ public-inbox