[05/11] quota: Allow to pass quotactl a mountpoint

So far quotactl only allows a path to a block device to specify a
filesystem to work on. For filesystems that don't work on block devices
such as UBIFS there is no block device, so let quotactl take the
mountpoint as an alternative way.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 fs/quota/quota.c   | 69 ++++++++++++++++++++++++++++++++--------------
 fs/super.c         | 17 ++++++++++++
 include/linux/fs.h |  1 +
 3 files changed, 67 insertions(+), 20 deletions(-)

On Wed, Aug 14, 2019 at 02:18:28PM +0200, Sascha Hauer wrote:
> +/**
> + * reference_super - get a reference to a given superblock
> + * @sb: superblock to get the reference from
> + *
> + * Takes a reference to a superblock. Can be used as when the superblock
> + * is known and leaves it in a state as if get_super had been called.
> + */
> +void reference_super(struct super_block *sb)
> +{
> +	spin_lock(&sb_lock);
> +	sb->s_count++;
> +	spin_unlock(&sb_lock);
> +
> +	down_read(&sb->s_umount);
> +}
> +EXPORT_SYMBOL_GPL(reference_super);

NAK, for a plenty of reasons

1) introduction of EXPORT_SYMBOL_GPL garbage
2) aforementioned garbage on something that doesn't need to be exported
3) *way* too easily abused - get_super() is, at least, not tempting to
play with in random code.  This one is, and it's too low-level to
allow.

On Thu, Aug 15, 2019 at 12:36:32AM +0100, Al Viro wrote:
> On Wed, Aug 14, 2019 at 02:18:28PM +0200, Sascha Hauer wrote:
> > +/**
> > + * reference_super - get a reference to a given superblock
> > + * @sb: superblock to get the reference from
> > + *
> > + * Takes a reference to a superblock. Can be used as when the superblock
> > + * is known and leaves it in a state as if get_super had been called.
> > + */
> > +void reference_super(struct super_block *sb)
> > +{
> > +	spin_lock(&sb_lock);
> > +	sb->s_count++;
> > +	spin_unlock(&sb_lock);
> > +
> > +	down_read(&sb->s_umount);
> > +}
> > +EXPORT_SYMBOL_GPL(reference_super);
> 
> NAK, for a plenty of reasons
> 
> 1) introduction of EXPORT_SYMBOL_GPL garbage
> 2) aforementioned garbage on something that doesn't need to be exported
> 3) *way* too easily abused - get_super() is, at least, not tempting to
> play with in random code.  This one is, and it's too low-level to
> allow.

... and this is a crap userland API.

*IF* you want mountpoint-based variants of quotactl() commands, by all means,
add those.  Do not overload the old ones.  And for path-based you don't
need to mess with superblock references - just keep the struct path until
the end.  That will keep the superblock alive and active just fine.

On Thu, Aug 15, 2019 at 12:39:46AM +0100, Al Viro wrote:
> > 1) introduction of EXPORT_SYMBOL_GPL garbage
> > 2) aforementioned garbage on something that doesn't need to be exported
> > 3) *way* too easily abused - get_super() is, at least, not tempting to
> > play with in random code.  This one is, and it's too low-level to
> > allow.
> 
> ... and this is a crap userland API.
> 
> *IF* you want mountpoint-based variants of quotactl() commands, by all means,
> add those.  Do not overload the old ones.  And for path-based you don't
> need to mess with superblock references - just keep the struct path until
> the end.  That will keep the superblock alive and active just fine.

	To clarify: I suggest something like #define Q_PATH     0x400000
with users passing something like QCMD(Q_QUOTAON | Q_PATH, ...) instead
of QCMD(Q_QUOTAON, ...) to get a path-based behaviour.

On Thu, Aug 15, 2019 at 12:39:46AM +0100, Al Viro wrote:
> On Thu, Aug 15, 2019 at 12:36:32AM +0100, Al Viro wrote:
> > On Wed, Aug 14, 2019 at 02:18:28PM +0200, Sascha Hauer wrote:
> > > +/**
> > > + * reference_super - get a reference to a given superblock
> > > + * @sb: superblock to get the reference from
> > > + *
> > > + * Takes a reference to a superblock. Can be used as when the superblock
> > > + * is known and leaves it in a state as if get_super had been called.
> > > + */
> > > +void reference_super(struct super_block *sb)
> > > +{
> > > +	spin_lock(&sb_lock);
> > > +	sb->s_count++;
> > > +	spin_unlock(&sb_lock);
> > > +
> > > +	down_read(&sb->s_umount);
> > > +}
> > > +EXPORT_SYMBOL_GPL(reference_super);
> > 
> > NAK, for a plenty of reasons
> > 
> > 1) introduction of EXPORT_SYMBOL_GPL garbage
> > 2) aforementioned garbage on something that doesn't need to be exported
> > 3) *way* too easily abused - get_super() is, at least, not tempting to
> > play with in random code.  This one is, and it's too low-level to
> > allow.
> 
> ... and this is a crap userland API.
> 
> *IF* you want mountpoint-based variants of quotactl() commands, by all means,
> add those.  Do not overload the old ones.  And for path-based you don't
> need to mess with superblock references - just keep the struct path until
> the end.  That will keep the superblock alive and active just fine.

I'll happily drop these changes. To clarify, quota currently does:

	if (quotactl_cmd_onoff(cmd))
		sb = get_super_exclusive_thawed(bdev);
	else if (quotactl_cmd_write(cmd))
		sb = get_super_thawed(bdev);
	else
		sb = get_super(bdev);

You are saying that the struct super_block I get from a struct path pointer is
in a suitable state for all the cases above, right?

Sascha

On Thu 15-08-19 00:51:24, Al Viro wrote:
> On Thu, Aug 15, 2019 at 12:39:46AM +0100, Al Viro wrote:
> > > 1) introduction of EXPORT_SYMBOL_GPL garbage
> > > 2) aforementioned garbage on something that doesn't need to be exported
> > > 3) *way* too easily abused - get_super() is, at least, not tempting to
> > > play with in random code.  This one is, and it's too low-level to
> > > allow.
> > 
> > ... and this is a crap userland API.
> > 
> > *IF* you want mountpoint-based variants of quotactl() commands, by all means,
> > add those.  Do not overload the old ones.  And for path-based you don't
> > need to mess with superblock references - just keep the struct path until
> > the end.  That will keep the superblock alive and active just fine.
> 
> 	To clarify: I suggest something like #define Q_PATH     0x400000
> with users passing something like QCMD(Q_QUOTAON | Q_PATH, ...) instead
> of QCMD(Q_QUOTAON, ...) to get a path-based behaviour.

Yeah, this sounds like a good plan to me. If Sasha plans on using userspace
quota-tools for handling ubifs, some work will be needed there as well but
it's doable.

								Honza