Re: UBIFS quota support

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

On Wed, Jan 23, 2019 at 10:46:24AM +0100, Richard Weinberger wrote:
> Sascha,
> 
> Am Mittwoch, 23. Januar 2019, 10:43:05 CET schrieb Sascha Hauer:
> > 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.
> 
> This is why I said yesterday you need to touch the UBIFS journal replay code too.
> So, S_SYNC is not to keep the quota file consistent with UBIFS' state, it is
> to make sure we don't lose quota updates.

But how we handle a power cut between the update to the quota file and
the actual inode change then? During replay we'll find quota updates and
inode updates, but we do not have any information which quota update
belongs to which inode change.

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 |