linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* UBIFS quota support
@ 2019-01-10 11:44 Sascha Hauer
  2019-01-10 11:44 ` Sascha Hauer
  2019-01-22 23:07 ` Richard Weinberger
  0 siblings, 2 replies; 12+ messages in thread
From: Sascha Hauer @ 2019-01-10 11:44 UTC (permalink / raw)
  To: linux-fsdevel
  Cc: Dongsheng Yang, Jan Kara, Richard Weinberger, Christoph Hellwig,
	linux-mtd, kernel

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?

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 |

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* UBIFS quota support
  2019-01-10 11:44 UBIFS quota support Sascha Hauer
@ 2019-01-10 11:44 ` Sascha Hauer
  2019-01-22 23:07 ` Richard Weinberger
  1 sibling, 0 replies; 12+ messages in thread
From: Sascha Hauer @ 2019-01-10 11:44 UTC (permalink / raw)
  To: linux-fsdevel
  Cc: Dongsheng Yang, Jan Kara, linux-mtd, Christoph Hellwig,
	Richard Weinberger, kernel

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?

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 |

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: UBIFS quota support
  2019-01-10 11:44 UBIFS quota support Sascha Hauer
  2019-01-10 11:44 ` Sascha Hauer
@ 2019-01-22 23:07 ` Richard Weinberger
  2019-01-23  9:43   ` Sascha Hauer
  2019-01-23 15:47   ` Jan Kara
  1 sibling, 2 replies; 12+ messages in thread
From: Richard Weinberger @ 2019-01-22 23:07 UTC (permalink / raw)
  To: Jan Kara, Christoph Hellwig
  Cc: linux-fsdevel, Dongsheng Yang, Richard Weinberger, linux-mtd,
	kernel, Sascha Hauer

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 fsck fixing 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?

-- 
Thanks,
//richard

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: UBIFS quota support
  2019-01-22 23:07 ` Richard Weinberger
@ 2019-01-23  9:43   ` Sascha Hauer
  2019-01-23  9:46     ` Richard Weinberger
  2019-01-23 15:47   ` Jan Kara
  1 sibling, 1 reply; 12+ messages in thread
From: Sascha Hauer @ 2019-01-23  9:43 UTC (permalink / raw)
  To: Richard Weinberger
  Cc: Jan Kara, Christoph Hellwig, linux-fsdevel, Dongsheng Yang,
	Richard Weinberger, linux-mtd, kernel

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 |

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: UBIFS quota support
  2019-01-23  9:43   ` Sascha Hauer
@ 2019-01-23  9:46     ` Richard Weinberger
  2019-01-23  9:55       ` Sascha Hauer
  0 siblings, 1 reply; 12+ messages in thread
From: Richard Weinberger @ 2019-01-23  9:46 UTC (permalink / raw)
  To: Sascha Hauer
  Cc: Richard Weinberger, Jan Kara, Christoph Hellwig, linux-fsdevel,
	Dongsheng Yang, linux-mtd, kernel

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.
Skipping S_SYNC is also possible, but then you need to fix up even more stuff
in the replay code.

Thanks,
//richard



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: UBIFS quota support
  2019-01-23  9:46     ` Richard Weinberger
@ 2019-01-23  9:55       ` Sascha Hauer
  2019-01-23 10:47         ` Richard Weinberger
  0 siblings, 1 reply; 12+ messages in thread
From: Sascha Hauer @ 2019-01-23  9:55 UTC (permalink / raw)
  To: Richard Weinberger
  Cc: Richard Weinberger, Jan Kara, Christoph Hellwig, linux-fsdevel,
	Dongsheng Yang, linux-mtd, kernel

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 |

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: UBIFS quota support
  2019-01-23  9:55       ` Sascha Hauer
@ 2019-01-23 10:47         ` Richard Weinberger
  0 siblings, 0 replies; 12+ messages in thread
From: Richard Weinberger @ 2019-01-23 10:47 UTC (permalink / raw)
  To: Sascha Hauer
  Cc: Richard Weinberger, Jan Kara, Christoph Hellwig, linux-fsdevel,
	Dongsheng Yang, linux-mtd, kernel

Sascha,

Am Mittwoch, 23. Januar 2019, 10:55:35 CET schrieb Sascha Hauer:
> > > 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.

In worst case we need to store more info in the journal to get this infos.
That's why I asked Jan for more info. I don't know what exactly quota expects
from the filesystem in terms of consistency. Maybe there are already some helpers, etc...

Thanks,
//richard



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: UBIFS quota support
  2019-01-22 23:07 ` Richard Weinberger
  2019-01-23  9:43   ` Sascha Hauer
@ 2019-01-23 15:47   ` Jan Kara
  2019-01-25  9:21     ` Sascha Hauer
  1 sibling, 1 reply; 12+ messages in thread
From: Jan Kara @ 2019-01-23 15:47 UTC (permalink / raw)
  To: Richard Weinberger
  Cc: Jan Kara, Christoph Hellwig, linux-fsdevel, Dongsheng Yang,
	Richard Weinberger, linux-mtd, kernel, Sascha Hauer

Hi,

sorry for not getting to you earlier, this email got burried in my inbox...

On Wed 23-01-19 00:07:12, Richard Weinberger wrote:
> On Thu, Jan 10, 2019 at 12:45 PM Sascha Hauer <s.hauer@pengutronix.de> wrote:
> > 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?

So after some thought, yes, I think that passing mount point as a specifier
identifying a block device will be OK.

> *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 fsck
> fixing the quota file or having a data journal for the quota file.

Essentially yes but it depends on how exactly you decide to implement quota
files. First let me explain to you some details about how quota subsystem
works.

When quota structure (struct dquot) for some user gets
first attached to some inode we call ->acquire_dquot callback from
dquot_operations. This is responsible for allocating necessary disk space
for the structure (if not already allocated) and otherwise making sure that
the quota information can be easily stored later. Also it should fill in
current quota information if the structure for given ID already exists.

Similarly when the last reference to dquot is dropped ->release_dquot
callback is called.

When update to quota information happens, the core will call ->mark_dirty
callback. Then there are other callbacks but those are not that important
and we can figure them out later.

Essentially you have two sensible options:

1) Follow the path of ext4, ocfs2, and similar and use the format of quota
files handled by the generic code.

2) Define your own quota file format.

In the first case, we use ->mark_dirty callback to trigger update of the
quota file. This happens by ext4_mark_dquot_dirty() calling
ext4_write_dquot() calling dquot_commit() which updates corresponding block
in quota file by calling ->quota_write() callback - ext4_quota_write() then
takes care of attaching this update to the running transaction. As Sasha
has noted, we use the fact that we have a transaction running stored in
current->journal_info so this update to quota file just gets added to the
running transaction. So you will have to somehow replicate this
functionality in ubifs so that this update of quota file happens atomically
(wrt power failure) together with the block allocation.

In the second case you are somewhat more free in your options (but also
have to implement more code). E.g. OCFS2 uses this option. In this case you
get ->mark_dirty callback (essentially happening as a result of
dquot_alloc_space() or similar calls) and it is up to you associate new
quota information with appropriate metadata change.

> 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?

Well, quota code does not care about S_SYNC flag. So yes, you can set it
but the behavior of core quota code will not change in any way.

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: UBIFS quota support
  2019-01-23 15:47   ` Jan Kara
@ 2019-01-25  9:21     ` Sascha Hauer
  2019-01-28  8:09       ` Christoph Hellwig
  2019-01-30 12:45       ` Jan Kara
  0 siblings, 2 replies; 12+ messages in thread
From: Sascha Hauer @ 2019-01-25  9:21 UTC (permalink / raw)
  To: Jan Kara
  Cc: Richard Weinberger, Christoph Hellwig, linux-fsdevel,
	Dongsheng Yang, Richard Weinberger, linux-mtd, kernel

Hi Jan,

On Wed, Jan 23, 2019 at 04:47:43PM +0100, Jan Kara wrote:
> Hi,
> 
> sorry for not getting to you earlier, this email got burried in my inbox...
> 
> On Wed 23-01-19 00:07:12, Richard Weinberger wrote:
> > On Thu, Jan 10, 2019 at 12:45 PM Sascha Hauer <s.hauer@pengutronix.de> wrote:
> > > 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?
> 
> So after some thought, yes, I think that passing mount point as a specifier
> identifying a block device will be OK.
> 
> > *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 fsck
> > fixing the quota file or having a data journal for the quota file.
> 
> Essentially yes but it depends on how exactly you decide to implement quota
> files. First let me explain to you some details about how quota subsystem
> works.
> 
> When quota structure (struct dquot) for some user gets
> first attached to some inode we call ->acquire_dquot callback from
> dquot_operations. This is responsible for allocating necessary disk space
> for the structure (if not already allocated) and otherwise making sure that
> the quota information can be easily stored later. Also it should fill in
> current quota information if the structure for given ID already exists.

UBIFS has no idea of transactions and works a little different from
ext4. In UBIFS all filesystem data and metadata are stored in the leaf
nodes of a b+tree. These leaf nodes are constantly written to flash and
would be enough to reconstruct the FS. From time to time UBIFS does a
commit and writes the index nodes to flash. During recovery the tree can
be read from the index nodes from the last commit. The remaining leaf
nodes that were written after the last commit are then scanned and added
to the tree during replay.

Quota seems to work in the way that it has callbacks into the FS to read
and update dqblks. This is not very suitable for UBIFS. Instead it would
be nicer to read the full quota data from flash and hand it over to
quota. When UBIFS does a commit it would then request a consistent view
of the quota data and write it back to flash. During replay of an
uncleanly mounted FS UBIFS could read the quota data from the last
commit and update it with the remaining leaf nodes that need to be
replayed anyway.

In the latest UBIFS quota patchset storing the quota data is not safe
for unclean remounts, the data will become inconsistent with the actual
filesystem state.

Thinking about it storing the quota data is completely optional. In UBIFS
it's easy to retrieve the quota data by scanning the index tree during mount
time. The quota limits could be set during runtime by some init script.
For our usecase I think there's no need to store the limits in the FS
itself. With this I could turn the UBIFS quota patches into something
useful and consistently working with relatively little effort. Storing
the quota data on flash needs some discussion how to integrate properly
into UBIFS, but becomes an optimization left for a future exercise.

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 |

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: UBIFS quota support
  2019-01-25  9:21     ` Sascha Hauer
@ 2019-01-28  8:09       ` Christoph Hellwig
  2019-01-30 12:45       ` Jan Kara
  1 sibling, 0 replies; 12+ messages in thread
From: Christoph Hellwig @ 2019-01-28  8:09 UTC (permalink / raw)
  To: Sascha Hauer
  Cc: Jan Kara, Richard Weinberger, Christoph Hellwig, linux-fsdevel,
	Dongsheng Yang, Richard Weinberger, linux-mtd, kernel

FYI, the main reason (well, besides history) that XFS uses its own
quota code is so that it can have full integration with the transaction
subsystem and the on-disk data structures.

While it is going to be a lot more code you might want to look into
implementing quota accounting in ubifs itself and only use the
standard interfaces to report and adminstrate the quota values.

That way you might not even have to maintain a separate quota file and
could instead maintain usage statistics in the btree nodes, and only
summarize them in memory, possibly with a periodical full state dump.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: UBIFS quota support
  2019-01-25  9:21     ` Sascha Hauer
  2019-01-28  8:09       ` Christoph Hellwig
@ 2019-01-30 12:45       ` Jan Kara
  2019-01-31  7:37         ` Sascha Hauer
  1 sibling, 1 reply; 12+ messages in thread
From: Jan Kara @ 2019-01-30 12:45 UTC (permalink / raw)
  To: Sascha Hauer
  Cc: Jan Kara, Richard Weinberger, Christoph Hellwig, linux-fsdevel,
	Dongsheng Yang, Richard Weinberger, linux-mtd, kernel

Hello,

On Fri 25-01-19 10:21:56, Sascha Hauer wrote:
> On Wed, Jan 23, 2019 at 04:47:43PM +0100, Jan Kara wrote:
> > sorry for not getting to you earlier, this email got burried in my inbox...
> > 
> > On Wed 23-01-19 00:07:12, Richard Weinberger wrote:
> > > On Thu, Jan 10, 2019 at 12:45 PM Sascha Hauer <s.hauer@pengutronix.de> wrote:
> > > > 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?
> > 
> > So after some thought, yes, I think that passing mount point as a specifier
> > identifying a block device will be OK.
> > 
> > > *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 fsck
> > > fixing the quota file or having a data journal for the quota file.
> > 
> > Essentially yes but it depends on how exactly you decide to implement quota
> > files. First let me explain to you some details about how quota subsystem
> > works.
> > 
> > When quota structure (struct dquot) for some user gets
> > first attached to some inode we call ->acquire_dquot callback from
> > dquot_operations. This is responsible for allocating necessary disk space
> > for the structure (if not already allocated) and otherwise making sure that
> > the quota information can be easily stored later. Also it should fill in
> > current quota information if the structure for given ID already exists.
> 
> UBIFS has no idea of transactions and works a little different from
> ext4. In UBIFS all filesystem data and metadata are stored in the leaf
> nodes of a b+tree. These leaf nodes are constantly written to flash and
> would be enough to reconstruct the FS. From time to time UBIFS does a
> commit and writes the index nodes to flash. During recovery the tree can
> be read from the index nodes from the last commit. The remaining leaf
> nodes that were written after the last commit are then scanned and added
> to the tree during replay.
> 
> Quota seems to work in the way that it has callbacks into the FS to read
> and update dqblks. This is not very suitable for UBIFS. Instead it would

Yes, generally it works by requesting loading / storing of quota
information for a particular user from the filesystem.

> be nicer to read the full quota data from flash and hand it over to
> quota. When UBIFS does a commit it would then request a consistent view
> of the quota data and write it back to flash. During replay of an
> uncleanly mounted FS UBIFS could read the quota data from the last
> commit and update it with the remaining leaf nodes that need to be
> replayed anyway.

Well, I don't think writing all quota data for each commit is a good
design. That will write out lot of unnecessary stuff that didn't change
since last time. It is like if you rewrite the whole file to update
one block in it... Similarly loading all quota data doesn't look great for
performance. There can be thousands or tens of thousands different users of
the filesystem (yes, I've seen such deployments) and you don't want to read
and keep in memory information you don't currently need. But I guess UBIFS
is targetted at smaller deployments?

Anyway, what is easily doable is that you would just ignore requests for
update from quota subsystem (just let quota subsystem mark dquot as dirty)
and then on commit call dquot_writeback_dquots() that will call
->write_dquot callback for each dquot that is dirty. That way you'd get
full dump of dquots that changed since last commit. You'd need to somehow
make sure this gets merged with information from previous commit. Then on
crash recovery you'd load quota information from commit and update it with
the changes that happened since that last commit.

> In the latest UBIFS quota patchset storing the quota data is not safe
> for unclean remounts, the data will become inconsistent with the actual
> filesystem state.
> 
> Thinking about it storing the quota data is completely optional. In UBIFS
> it's easy to retrieve the quota data by scanning the index tree during mount
> time. The quota limits could be set during runtime by some init script.
> For our usecase I think there's no need to store the limits in the FS
> itself. With this I could turn the UBIFS quota patches into something
> useful and consistently working with relatively little effort. Storing
> the quota data on flash needs some discussion how to integrate properly
> into UBIFS, but becomes an optimization left for a future exercise.

Honestly loading quota limits by init scripts looks like a hack to me. Note
that quota limits rarely change so you can easily store them separately
from usage information and load them on mount. Since setting of limits does
not have to be crash-safe (well, it needs to keep the quota information in
a state that is recoverable by mount but it doesn't need to coordinate with
any other filesystem operations), I don't think implementing that would be
hard...

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: UBIFS quota support
  2019-01-30 12:45       ` Jan Kara
@ 2019-01-31  7:37         ` Sascha Hauer
  0 siblings, 0 replies; 12+ messages in thread
From: Sascha Hauer @ 2019-01-31  7:37 UTC (permalink / raw)
  To: Jan Kara
  Cc: Richard Weinberger, Christoph Hellwig, linux-fsdevel,
	Dongsheng Yang, Richard Weinberger, linux-mtd, kernel

On Wed, Jan 30, 2019 at 01:45:35PM +0100, Jan Kara wrote:
> Hello,
> 
> > UBIFS has no idea of transactions and works a little different from
> > ext4. In UBIFS all filesystem data and metadata are stored in the leaf
> > nodes of a b+tree. These leaf nodes are constantly written to flash and
> > would be enough to reconstruct the FS. From time to time UBIFS does a
> > commit and writes the index nodes to flash. During recovery the tree can
> > be read from the index nodes from the last commit. The remaining leaf
> > nodes that were written after the last commit are then scanned and added
> > to the tree during replay.
> > 
> > Quota seems to work in the way that it has callbacks into the FS to read
> > and update dqblks. This is not very suitable for UBIFS. Instead it would
> 
> Yes, generally it works by requesting loading / storing of quota
> information for a particular user from the filesystem.
> 
> > be nicer to read the full quota data from flash and hand it over to
> > quota. When UBIFS does a commit it would then request a consistent view
> > of the quota data and write it back to flash. During replay of an
> > uncleanly mounted FS UBIFS could read the quota data from the last
> > commit and update it with the remaining leaf nodes that need to be
> > replayed anyway.
> 
> Well, I don't think writing all quota data for each commit is a good
> design. That will write out lot of unnecessary stuff that didn't change
> since last time. It is like if you rewrite the whole file to update
> one block in it... Similarly loading all quota data doesn't look great for
> performance. There can be thousands or tens of thousands different users of
> the filesystem (yes, I've seen such deployments) and you don't want to read
> and keep in memory information you don't currently need. But I guess UBIFS
> is targetted at smaller deployments?

It seems our perspective to quota is quite different ;)
UBIFS targets embedded systems, in a typical usecase I wouldn't expect
more than a handful of users and certainly not more users than we fit
dqblks in a single page. From my perspective it adds overhead that we
have a tree at all and that the current quota formats constantly changes
the single page that we have all our quota data in, instead of writing
the whole data at once every once in a while.

> 
> Anyway, what is easily doable is that you would just ignore requests for
> update from quota subsystem (just let quota subsystem mark dquot as dirty)
> and then on commit call dquot_writeback_dquots() that will call
> ->write_dquot callback for each dquot that is dirty. That way you'd get
> full dump of dquots that changed since last commit. You'd need to somehow
> make sure this gets merged with information from previous commit. Then on
> crash recovery you'd load quota information from commit and update it with
> the changes that happened since that last commit.

Yes, something like that should work.

> 
> Honestly loading quota limits by init scripts looks like a hack to me. Note
> that quota limits rarely change so you can easily store them separately
> from usage information and load them on mount. Since setting of limits does
> not have to be crash-safe (well, it needs to keep the quota information in
> a state that is recoverable by mount but it doesn't need to coordinate with
> any other filesystem operations), I don't think implementing that would be
> hard...

It's not my goal to put quota limits into init scripts, still I think it
could be a nice stopover to divide the whole thing into more managable
parts. I think I'll just implement it up to that point and post the
result. Then we can still see if it's enough to merge or if I have to
implement the rest before merging.

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 |

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2019-01-31  7:37 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-10 11:44 UBIFS quota support Sascha Hauer
2019-01-10 11:44 ` Sascha Hauer
2019-01-22 23:07 ` Richard Weinberger
2019-01-23  9:43   ` Sascha Hauer
2019-01-23  9:46     ` Richard Weinberger
2019-01-23  9:55       ` Sascha Hauer
2019-01-23 10:47         ` Richard Weinberger
2019-01-23 15:47   ` Jan Kara
2019-01-25  9:21     ` Sascha Hauer
2019-01-28  8:09       ` Christoph Hellwig
2019-01-30 12:45       ` Jan Kara
2019-01-31  7:37         ` Sascha Hauer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).