linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jan Kara <jack@suse.cz>
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,
	Sascha Hauer <s.hauer@pengutronix.de>
Subject: Re: UBIFS quota support
Date: Wed, 23 Jan 2019 16:47:43 +0100	[thread overview]
Message-ID: <20190123154743.GL13149@quack2.suse.cz> (raw)
In-Reply-To: <CAFLxGvyrNaq6ya9TNr-tdEEbV=qsi8qEko6R7oJ-DtkxPJMk0g@mail.gmail.com>

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

  parent reply	other threads:[~2019-01-23 15:47 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
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

Reply instructions:

You may reply publicly 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=20190123154743.GL13149@quack2.suse.cz \
    --to=jack@suse.cz \
    --cc=hch@infradead.org \
    --cc=kernel@pengutronix.de \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=richard.weinberger@gmail.com \
    --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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).