Re: UBIFS quota support

From: Sascha Hauer <s.hauer@pengutronix.de>
To: Richard Weinberger <richard.weinberger@gmail.com>
Cc: Jan Kara <jack@suse.cz>, Christoph Hellwig <hch@infradead.org>,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>,
	Dongsheng Yang <yangds.fnst@cn.fujitsu.com>,
	Richard Weinberger <richard@nod.at>,
	linux-mtd@lists.infradead.org, kernel@pengutronix.de
Subject: Re: UBIFS quota support
Date: Wed, 23 Jan 2019 10:43:05 +0100	[thread overview]
Message-ID: <20190123094305.7owpfwzgtzz2snat@pengutronix.de> (raw)
In-Reply-To: <CAFLxGvyrNaq6ya9TNr-tdEEbV=qsi8qEko6R7oJ-DtkxPJMk0g@mail.gmail.com>

On Wed, Jan 23, 2019 at 12:07:12AM +0100, Richard Weinberger wrote:
> On Thu, Jan 10, 2019 at 12:45 PM Sascha Hauer <s.hauer@pengutronix.de> wrote:
> >
> > Hi all,
> >
> > I'm currently working on resurrecting the UBIFS quota patches posted back in
> > 2015 by Dongsheng Yang, last posted here:
> >
> > http://lists.infradead.org/pipermail/linux-mtd/2015-September/061812.html
> >
> > First of all I think work stopped there, there is no newer UBIFS quota
> > support I am missing, right?
> >
> > One problem with this series was that the quotactl systemcall expects a
> > path to a block device. UBIFS doesn't work on a block device but on a
> > character device instead.
> > The solution in this series was to pass the path to the cdev in
> > quotactl.  A struct cdev * member was added to struct super_block which
> > was used to identify the superblock for a given cdev. This approach was
> > rejected by Christoph ("I don't think the cdev has any business in core
> > VFS code.").  Apart from that UBIFS can not only be mounted with a path
> > to the character device (mount -t ubifs /dev/ubix_y /mnt) but also in
> > the form ubix:volname (mount -t ubifs ubix:volname /mnt) in which case
> > userspace doesn't have any valid path it could pass in quotactl.
> >
> > An idea out of this would be to allow to pass the mountpoint instead of
> > the path to the block device in quotactl which would work with nfs or
> > even tmpfs aswell. Would that be acceptable? Any other ideas?
> 
> *kind ping*
> 
> Jan, another thing Sascha and I are not sure about, what are the consistency
> constraints of the quota file?
> If I read the code correctly, quota just writes to the quota file and
> assumes that
> the file system makes sure about consistency. Either by fsckfixing the quota
> file or having a data journal for the quota file.
> In case of UBIFS where we have a data journal this should be doable.
> Is it okay when the quota file has S_SYNC set?

S_SYNC won't help us. We need to make sure that a change of an inode and
the corresponding update to the quota file is done atomically. Otherwise
it may happen that we only change the size of an inode, but miss the
corresponding quota updates, or depending on the implementation, maybe
the other way round.

ext4 does this with transactions. As an example with ext4_setattr():

ext4_setattr()
	-> handle = ext4_journal_start(inode, EXT4_HT_QUOTA, ...)
	-> dquot_transfer(inode, attr);
		-> After going through the quota code with several calls
		   back into ext4 ends up in ext4_quota_write() which
		   updates the quota file
	-> change inode
	-> ext4_mark_inode_dirty(handle, inode)
	-> ext4_journal_stop(handle);

Everything between ext4_journal_start() and ext4_journal_stop() is
either done or not.

The analogy in UBIFS is the grouped nodes, but these are not very well
suited to be initialized in one function and arbitrarily extended
somewhere further down the call stack.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |