[PATCH 0/4] quota: add new quotactl Q_XGETQUOTA2

[PATCH 0/4] quota: add new quotactl Q_XGETQUOTA2

[Eric Sandeen]

This adds a new quotactl, Q_XGETQUOTA2.

Q_XGETQUOTA2 is exactly like Q_XGETQUOTA, except that it will
return quota information for the id equal to or greater than
the id requested.  In other words, if the specified id has
no quota, the command will return quota information for the
next higher id which does have a quota set.  If no higher id
has an active quota, -ESRCH is returned.

So if you ask for id X, you can get back quota for id X,
id X+N, or -ESRCH if no higher id has a quota.

This allows filesystems to do efficient iteration in kernelspace,
much like extN filesystems do in userspace when asked to report
all active quotas.

Today, filesystems such as XFS require getpwent()-style iterations,
and for systems which have i.e. LDAP backends, this can be very
slow, or even impossible, if iteration is not allowed in the
configuration.

I have patches to xfsprogs to allow xfs_quota to use this interface,
but I haven't looked at generic quota tools yet TBH.  I'll send the
xfsprogs patches to the xfs list shortly.

Using an id as input fits very nicely with existing interfaces;
however, I could imagine that making it a more generic "cookie"
interface might be more extensible; is there any filesystem which
wouldn't be collecting all active IDs in a linear fashion, and
would rather have an arbitrary cookie passed back & forth?
(I hope not) :)

Thanks,
-Eric

[PATCH 1/4] quota: add new quotactl Q_XGETQUOTA2

[Eric Sandeen]

Q_XGETQUOTA2 is exactly like Q_XGETQUOTA, except that it will
return quota information for the id equal to or greater than
the id requested.  In other words, if the specified id has
no quota, the command will return quota information for the
next higher id which does have a quota set.  If no higher id
has an active quota, -ESRCH is returned.

This allows filesystems to do efficient iteration in kernelspace,
much like extN filesystems do in userspace when asked to report
all active quotas.

Today, filesystems such as XFS require getpwent-style iterations,
and for systems which have i.e. LDAP backends, this can be very
slow, or even impossible, if iteration is not allowed in the
configuration.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 fs/quota/quota.c               |   30 ++++++++++++++++++++++++++++++
 include/linux/quota.h          |    2 ++
 include/uapi/linux/dqblk_xfs.h |    1 +
 3 files changed, 33 insertions(+), 0 deletions(-)

diff --git a/fs/quota/quota.c b/fs/quota/quota.c
index 3746367..441a4e6 100644
--- a/fs/quota/quota.c
+++ b/fs/quota/quota.c
@@ -33,6 +33,7 @@ static int check_quotactl_permission(struct super_block *sb, int type, int cmd,
 	/* allow to query information for dquots we "own" */
 	case Q_GETQUOTA:
 	case Q_XGETQUOTA:
+	case Q_XGETQUOTA2:
 		if ((type == USRQUOTA && uid_eq(current_euid(), make_kuid(current_user_ns(), id))) ||
 		    (type == GRPQUOTA && in_egroup_p(make_kgid(current_user_ns(), id))))
 			break;
@@ -625,6 +626,32 @@ static int quota_getxquota(struct super_block *sb, int type, qid_t id,
 	return ret;
 }
 
+/*
+ * Return quota for next active quota >= this id, if any exists,
+ * otherwise return -ESRCH.
+ */
+static int quota_getxquota2(struct super_block *sb, int type, qid_t id,
+			    void __user *addr)
+{
+	struct fs_disk_quota fdq;
+	struct qc_dqblk qdq;
+	struct kqid qid;
+	int ret;
+
+	if (!sb->s_qcop->get_dqblk2)
+		return -ENOSYS;
+	qid = make_kqid(current_user_ns(), type, id);
+	if (!qid_valid(qid))
+		return -EINVAL;
+	ret = sb->s_qcop->get_dqblk2(sb, qid, &id, &qdq);
+	if (ret)
+		return ret;
+	copy_to_xfs_dqblk(&fdq, &qdq, type, id);
+	if (copy_to_user(addr, &fdq, sizeof(fdq)))
+		return -EFAULT;
+	return ret;
+}
+
 static int quota_rmxquota(struct super_block *sb, void __user *addr)
 {
 	__u32 flags;
@@ -690,6 +717,8 @@ static int do_quotactl(struct super_block *sb, int type, int cmd, qid_t id,
 		return quota_setxquota(sb, type, id, addr);
 	case Q_XGETQUOTA:
 		return quota_getxquota(sb, type, id, addr);
+	case Q_XGETQUOTA2:
+		return quota_getxquota2(sb, type, id, addr);
 	case Q_XQUOTASYNC:
 		if (sb->s_flags & MS_RDONLY)
 			return -EROFS;
@@ -712,6 +741,7 @@ static int quotactl_cmd_write(int cmd)
 	case Q_XGETQSTAT:
 	case Q_XGETQSTATV:
 	case Q_XGETQUOTA:
+	case Q_XGETQUOTA2:
 	case Q_XQUOTASYNC:
 		return 0;
 	}
diff --git a/include/linux/quota.h b/include/linux/quota.h
index b2505ac..49e4f7c 100644
--- a/include/linux/quota.h
+++ b/include/linux/quota.h
@@ -425,6 +425,8 @@ struct quotactl_ops {
 	int (*quota_sync)(struct super_block *, int);
 	int (*set_info)(struct super_block *, int, struct qc_info *);
 	int (*get_dqblk)(struct super_block *, struct kqid, struct qc_dqblk *);
+	int (*get_dqblk2)(struct super_block *, struct kqid, qid_t *,
+			  struct qc_dqblk *);
 	int (*set_dqblk)(struct super_block *, struct kqid, struct qc_dqblk *);
 	int (*get_state)(struct super_block *, struct qc_state *);
 	int (*rm_xquota)(struct super_block *, unsigned int);
diff --git a/include/uapi/linux/dqblk_xfs.h b/include/uapi/linux/dqblk_xfs.h
index dcd75cc..62441da 100644
--- a/include/uapi/linux/dqblk_xfs.h
+++ b/include/uapi/linux/dqblk_xfs.h
@@ -39,6 +39,7 @@
 #define Q_XQUOTARM	XQM_CMD(6)	/* free disk space used by dquots */
 #define Q_XQUOTASYNC	XQM_CMD(7)	/* delalloc flush, updates dquots */
 #define Q_XGETQSTATV	XQM_CMD(8)	/* newer version of get quota */
+#define Q_XGETQUOTA2	XQM_CMD(9)	/* get disk limits and usage >= ID */
 
 /*
  * fs_disk_quota structure:
-- 
1.7.1

[PATCH 2/4] xfs: get quota inode from mp & flags rather than dqp

[Eric Sandeen]

Allow us to get the appropriate quota inode from any
mp & quota flags, not necessarily associated with a
particular dqp.  Needed for when we are searching for
the next active ID with quotas and we want to examine
the quota inode.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 fs/xfs/xfs_dquot.c |    3 ++-
 fs/xfs/xfs_qm.h    |   10 +++++-----
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
index 9c44d38..1983afc 100644
--- a/fs/xfs/xfs_dquot.c
+++ b/fs/xfs/xfs_dquot.c
@@ -464,12 +464,13 @@ xfs_qm_dqtobp(
 	struct xfs_bmbt_irec	map;
 	int			nmaps = 1, error;
 	struct xfs_buf		*bp;
-	struct xfs_inode	*quotip = xfs_dq_to_quota_inode(dqp);
+	struct xfs_inode	*quotip;
 	struct xfs_mount	*mp = dqp->q_mount;
 	xfs_dqid_t		id = be32_to_cpu(dqp->q_core.d_id);
 	struct xfs_trans	*tp = (tpp ? *tpp : NULL);
 	uint			lock_mode;
 
+	quotip = xfs_quota_inode(dqp->q_mount, dqp->dq_flags);
 	dqp->q_fileoffset = (xfs_fileoff_t)id / mp->m_quotainfo->qi_dqperchunk;
 
 	lock_mode = xfs_ilock_data_map_shared(quotip);
diff --git a/fs/xfs/xfs_qm.h b/fs/xfs/xfs_qm.h
index 996a040..8901a01 100644
--- a/fs/xfs/xfs_qm.h
+++ b/fs/xfs/xfs_qm.h
@@ -104,15 +104,15 @@ xfs_dquot_tree(
 }
 
 static inline struct xfs_inode *
-xfs_dq_to_quota_inode(struct xfs_dquot *dqp)
+xfs_quota_inode(xfs_mount_t *mp, uint dq_flags)
 {
-	switch (dqp->dq_flags & XFS_DQ_ALLTYPES) {
+	switch (dq_flags & XFS_DQ_ALLTYPES) {
 	case XFS_DQ_USER:
-		return dqp->q_mount->m_quotainfo->qi_uquotaip;
+		return mp->m_quotainfo->qi_uquotaip;
 	case XFS_DQ_GROUP:
-		return dqp->q_mount->m_quotainfo->qi_gquotaip;
+		return mp->m_quotainfo->qi_gquotaip;
 	case XFS_DQ_PROJ:
-		return dqp->q_mount->m_quotainfo->qi_pquotaip;
+		return mp->m_quotainfo->qi_pquotaip;
 	default:
 		ASSERT(0);
 	}
-- 
1.7.1

[PATCH 3/4] xfs: Factor xfs_seek_hole_data into helper

[Eric Sandeen]

Factor xfs_seek_hole_data into an unlocked helper which takes
an xfs inode rather than a file for internal use.

Also allow specification of "end" - the vfs lseek interface is
defined such that any offset past eof/i_size shall return -ENXIO,
but we will use this for quota code which does not maintain i_size,
and we want to be able to SEEK_DATA past i_size as well. So the
lseek path can send in i_size, and the quota code can determine
its own ending offset.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
fs/xfs/xfs_file.c | 82 ++++++++++++++++++++++++++++++++++++----------------
fs/xfs/xfs_inode.h | 2 +
2 files changed, 59 insertions(+), 25 deletions(-)

diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index ebe9b82..5dc7113 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -1337,31 +1337,31 @@ out:
return found;
}

-STATIC loff_t
-xfs_seek_hole_data(
- struct file *file,
+/*
+ * caller must lock inode with xfs_ilock_data_map_shared,
+ * can we craft an appropriate ASSERT?
+ *
+ * end is because the VFS-level lseek interface is defined such that any
+ * offset past i_size shall return -ENXIO, but we use this for quota code
+ * which does not maintain i_size, and we want to SEEK_DATA past i_size.
+ */
+loff_t
+__xfs_seek_hole_data(
+ struct inode *inode,
loff_t start,
+ loff_t end,
int whence)
{
- struct inode *inode = file->f_mapping->host;
struct xfs_inode *ip = XFS_I(inode);
struct xfs_mount *mp = ip->i_mount;
loff_t uninitialized_var(offset);
- xfs_fsize_t isize;
xfs_fileoff_t fsbno;
- xfs_filblks_t end;
- uint lock;
+ xfs_filblks_t lastbno;
int error;

- if (XFS_FORCED_SHUTDOWN(mp))
- return -EIO;
-
- lock = xfs_ilock_data_map_shared(ip);
-
- isize = i_size_read(inode);
- if (start >= isize) {
+ if (start >= end) {
error = -ENXIO;
- goto out_unlock;
+ goto out_error;
}

/*
@@ -1369,22 +1369,22 @@ xfs_seek_hole_data(
* by fsbno to the end block of the file.
*/
fsbno = XFS_B_TO_FSBT(mp, start);
- end = XFS_B_TO_FSB(mp, isize);
+ lastbno = XFS_B_TO_FSB(mp, end);

for (;;) {
struct xfs_bmbt_irec map[2];
int nmap = 2;
unsigned int i;

- error = xfs_bmapi_read(ip, fsbno, end - fsbno, map, &nmap,
+ error = xfs_bmapi_read(ip, fsbno, lastbno - fsbno, map, &nmap,
XFS_BMAPI_ENTIRE);
if (error)
- goto out_unlock;
+ goto out_error;

/* No extents at given offset, must be beyond EOF */
if (nmap == 0) {
error = -ENXIO;
- goto out_unlock;
+ goto out_error;
}

for (i = 0; i < nmap; i++) {
@@ -1426,7 +1426,7 @@ xfs_seek_hole_data(
* hole at the end of any file).
*/
if (whence == SEEK_HOLE) {
- offset = isize;
+ offset = end;
break;
}
/*
@@ -1434,7 +1434,7 @@ xfs_seek_hole_data(
*/
ASSERT(whence == SEEK_DATA);
error = -ENXIO;
- goto out_unlock;
+ goto out_error;
}

ASSERT(i > 1);
@@ -1445,14 +1445,14 @@ xfs_seek_hole_data(
*/
fsbno = map[i - 1].br_startoff + map[i - 1].br_blockcount;
start = XFS_FSB_TO_B(mp, fsbno);
- if (start >= isize) {
+ if (start >= end) {
if (whence == SEEK_HOLE) {
- offset = isize;
+ offset = end;
break;
}
ASSERT(whence == SEEK_DATA);
error = -ENXIO;
- goto out_unlock;
+ goto out_error;
}
}

@@ -1464,7 +1464,39 @@ out:
* situation in particular.
*/
if (whence == SEEK_HOLE)
- offset = min_t(loff_t, offset, isize);
+ offset = min_t(loff_t, offset, end);
+
+ return offset;
+
+out_error:
+ return error;
+}
+
+STATIC loff_t
+xfs_seek_hole_data(
+ struct file *file,
+ loff_t start,
+ int whence)
+{
+ struct inode *inode = file->f_mapping->host;
+ struct xfs_inode *ip = XFS_I(inode);
+ struct xfs_mount *mp = ip->i_mount;
+ uint lock;
+ loff_t offset, end;
+ int error = 0;
+
+ if (XFS_FORCED_SHUTDOWN(mp))
+ return -EIO;
+
+ lock = xfs_ilock_data_map_shared(ip);
+
+ end = i_size_read(inode);
+ offset = __xfs_seek_hole_data(inode, start, end, whence);
+ if (offset < 0) {
+ error = offset;
+ goto out_unlock;
+ }
+
offset = vfs_setpos(file, offset, inode->i_sb->s_maxbytes);

out_unlock:
diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
index ca9e119..ed7e933 100644
--- a/fs/xfs/xfs_inode.h
+++ b/fs/xfs/xfs_inode.h
@@ -437,6 +437,8 @@ int xfs_update_prealloc_flags(struct xfs_inode *ip,
int xfs_zero_eof(struct xfs_inode *ip, xfs_off_t offset,
xfs_fsize_t isize, bool *did_zeroing);
int xfs_iozero(struct xfs_inode *ip, loff_t pos, size_t count);
+loff_t __xfs_seek_hole_data(struct inode *inode, loff_t start,
+ loff_t eof, int whence);


/* from xfs_iops.c */
-- 
1.7.1

Re: [PATCH 3/4] xfs: Factor xfs_seek_hole_data into helper

[Jan Kara]

On Fri 08-01-16 10:58:57, Eric Sandeen wrote:
> Factor xfs_seek_hole_data into an unlocked helper which takes
> an xfs inode rather than a file for internal use.
> 
> Also allow specification of "end" - the vfs lseek interface is
> defined such that any offset past eof/i_size shall return -ENXIO,
> but we will use this for quota code which does not maintain i_size,
> and we want to be able to SEEK_DATA past i_size as well. So the
> lseek path can send in i_size, and the quota code can determine
> its own ending offset.

This patch looks whitespace damaged making it difficult to read...

								Honza
 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
> fs/xfs/xfs_file.c | 82 ++++++++++++++++++++++++++++++++++++----------------
> fs/xfs/xfs_inode.h | 2 +
> 2 files changed, 59 insertions(+), 25 deletions(-)
> 
> diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
> index ebe9b82..5dc7113 100644
> --- a/fs/xfs/xfs_file.c
> +++ b/fs/xfs/xfs_file.c
> @@ -1337,31 +1337,31 @@ out:
> return found;
> }
> 
> -STATIC loff_t
> -xfs_seek_hole_data(
> - struct file *file,
> +/*
> + * caller must lock inode with xfs_ilock_data_map_shared,
> + * can we craft an appropriate ASSERT?
> + *
> + * end is because the VFS-level lseek interface is defined such that any
> + * offset past i_size shall return -ENXIO, but we use this for quota code
> + * which does not maintain i_size, and we want to SEEK_DATA past i_size.
> + */
> +loff_t
> +__xfs_seek_hole_data(
> + struct inode *inode,
> loff_t start,
> + loff_t end,
> int whence)
> {
> - struct inode *inode = file->f_mapping->host;
> struct xfs_inode *ip = XFS_I(inode);
> struct xfs_mount *mp = ip->i_mount;
> loff_t uninitialized_var(offset);
> - xfs_fsize_t isize;
> xfs_fileoff_t fsbno;
> - xfs_filblks_t end;
> - uint lock;
> + xfs_filblks_t lastbno;
> int error;
> 
> - if (XFS_FORCED_SHUTDOWN(mp))
> - return -EIO;
> -
> - lock = xfs_ilock_data_map_shared(ip);
> -
> - isize = i_size_read(inode);
> - if (start >= isize) {
> + if (start >= end) {
> error = -ENXIO;
> - goto out_unlock;
> + goto out_error;
> }
> 
> /*
> @@ -1369,22 +1369,22 @@ xfs_seek_hole_data(
> * by fsbno to the end block of the file.
> */
> fsbno = XFS_B_TO_FSBT(mp, start);
> - end = XFS_B_TO_FSB(mp, isize);
> + lastbno = XFS_B_TO_FSB(mp, end);
> 
> for (;;) {
> struct xfs_bmbt_irec map[2];
> int nmap = 2;
> unsigned int i;
> 
> - error = xfs_bmapi_read(ip, fsbno, end - fsbno, map, &nmap,
> + error = xfs_bmapi_read(ip, fsbno, lastbno - fsbno, map, &nmap,
> XFS_BMAPI_ENTIRE);
> if (error)
> - goto out_unlock;
> + goto out_error;
> 
> /* No extents at given offset, must be beyond EOF */
> if (nmap == 0) {
> error = -ENXIO;
> - goto out_unlock;
> + goto out_error;
> }
> 
> for (i = 0; i < nmap; i++) {
> @@ -1426,7 +1426,7 @@ xfs_seek_hole_data(
> * hole at the end of any file).
> */
> if (whence == SEEK_HOLE) {
> - offset = isize;
> + offset = end;
> break;
> }
> /*
> @@ -1434,7 +1434,7 @@ xfs_seek_hole_data(
> */
> ASSERT(whence == SEEK_DATA);
> error = -ENXIO;
> - goto out_unlock;
> + goto out_error;
> }
> 
> ASSERT(i > 1);
> @@ -1445,14 +1445,14 @@ xfs_seek_hole_data(
> */
> fsbno = map[i - 1].br_startoff + map[i - 1].br_blockcount;
> start = XFS_FSB_TO_B(mp, fsbno);
> - if (start >= isize) {
> + if (start >= end) {
> if (whence == SEEK_HOLE) {
> - offset = isize;
> + offset = end;
> break;
> }
> ASSERT(whence == SEEK_DATA);
> error = -ENXIO;
> - goto out_unlock;
> + goto out_error;
> }
> }
> 
> @@ -1464,7 +1464,39 @@ out:
> * situation in particular.
> */
> if (whence == SEEK_HOLE)
> - offset = min_t(loff_t, offset, isize);
> + offset = min_t(loff_t, offset, end);
> +
> + return offset;
> +
> +out_error:
> + return error;
> +}
> +
> +STATIC loff_t
> +xfs_seek_hole_data(
> + struct file *file,
> + loff_t start,
> + int whence)
> +{
> + struct inode *inode = file->f_mapping->host;
> + struct xfs_inode *ip = XFS_I(inode);
> + struct xfs_mount *mp = ip->i_mount;
> + uint lock;
> + loff_t offset, end;
> + int error = 0;
> +
> + if (XFS_FORCED_SHUTDOWN(mp))
> + return -EIO;
> +
> + lock = xfs_ilock_data_map_shared(ip);
> +
> + end = i_size_read(inode);
> + offset = __xfs_seek_hole_data(inode, start, end, whence);
> + if (offset < 0) {
> + error = offset;
> + goto out_unlock;
> + }
> +
> offset = vfs_setpos(file, offset, inode->i_sb->s_maxbytes);
> 
> out_unlock:
> diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
> index ca9e119..ed7e933 100644
> --- a/fs/xfs/xfs_inode.h
> +++ b/fs/xfs/xfs_inode.h
> @@ -437,6 +437,8 @@ int xfs_update_prealloc_flags(struct xfs_inode *ip,
> int xfs_zero_eof(struct xfs_inode *ip, xfs_off_t offset,
> xfs_fsize_t isize, bool *did_zeroing);
> int xfs_iozero(struct xfs_inode *ip, loff_t pos, size_t count);
> +loff_t __xfs_seek_hole_data(struct inode *inode, loff_t start,
> + loff_t eof, int whence);
> 
> 
> /* from xfs_iops.c */
> -- 
> 1.7.1
> 
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

[PATCH 3/4 V3] xfs: Factor xfs_seek_hole_data into helper

[Eric Sandeen]

Factor xfs_seek_hole_data into an unlocked helper which takes
an xfs inode rather than a file for internal use.

Also allow specification of "end" - the vfs lseek interface is
defined such that any offset past eof/i_size shall return -ENXIO,
but we will use this for quota code which does not maintain i_size,
and we want to be able to SEEK_DATA past i_size as well.  So the
lseek path can send in i_size, and the quota code can determine
its own ending offset.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

(Ugh, no idea what happened to that formatting!)
(V3 same as V2 but actually hitting both lists - email is hard!)

 fs/xfs/xfs_file.c  |   82 ++++++++++++++++++++++++++++++++++++----------------
 fs/xfs/xfs_inode.h |    2 +
 2 files changed, 59 insertions(+), 25 deletions(-)

diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index ebe9b82..5dc7113 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -1337,31 +1337,31 @@ out:
 	return found;
 }
 
-STATIC loff_t
-xfs_seek_hole_data(
-	struct file		*file,
+/*
+ * caller must lock inode with xfs_ilock_data_map_shared,
+ * can we craft an appropriate ASSERT?
+ *
+ * end is because the VFS-level lseek interface is defined such that any
+ * offset past i_size shall return -ENXIO, but we use this for quota code
+ * which does not maintain i_size, and we want to SEEK_DATA past i_size.
+ */
+loff_t
+__xfs_seek_hole_data(
+	struct inode		*inode,
 	loff_t			start,
+	loff_t			end,
 	int			whence)
 {
-	struct inode		*inode = file->f_mapping->host;
 	struct xfs_inode	*ip = XFS_I(inode);
 	struct xfs_mount	*mp = ip->i_mount;
 	loff_t			uninitialized_var(offset);
-	xfs_fsize_t		isize;
 	xfs_fileoff_t		fsbno;
-	xfs_filblks_t		end;
-	uint			lock;
+	xfs_filblks_t		lastbno;
 	int			error;
 
-	if (XFS_FORCED_SHUTDOWN(mp))
-		return -EIO;
-
-	lock = xfs_ilock_data_map_shared(ip);
-
-	isize = i_size_read(inode);
-	if (start >= isize) {
+	if (start >= end) {
 		error = -ENXIO;
-		goto out_unlock;
+		goto out_error;
 	}
 
 	/*
@@ -1369,22 +1369,22 @@ xfs_seek_hole_data(
 	 * by fsbno to the end block of the file.
 	 */
 	fsbno = XFS_B_TO_FSBT(mp, start);
-	end = XFS_B_TO_FSB(mp, isize);
+	lastbno = XFS_B_TO_FSB(mp, end);
 
 	for (;;) {
 		struct xfs_bmbt_irec	map[2];
 		int			nmap = 2;
 		unsigned int		i;
 
-		error = xfs_bmapi_read(ip, fsbno, end - fsbno, map, &nmap,
+		error = xfs_bmapi_read(ip, fsbno, lastbno - fsbno, map, &nmap,
 				       XFS_BMAPI_ENTIRE);
 		if (error)
-			goto out_unlock;
+			goto out_error;
 
 		/* No extents at given offset, must be beyond EOF */
 		if (nmap == 0) {
 			error = -ENXIO;
-			goto out_unlock;
+			goto out_error;
 		}
 
 		for (i = 0; i < nmap; i++) {
@@ -1426,7 +1426,7 @@ xfs_seek_hole_data(
 			 * hole at the end of any file).
 		 	 */
 			if (whence == SEEK_HOLE) {
-				offset = isize;
+				offset = end;
 				break;
 			}
 			/*
@@ -1434,7 +1434,7 @@ xfs_seek_hole_data(
 			 */
 			ASSERT(whence == SEEK_DATA);
 			error = -ENXIO;
-			goto out_unlock;
+			goto out_error;
 		}
 
 		ASSERT(i > 1);
@@ -1445,14 +1445,14 @@ xfs_seek_hole_data(
 		 */
 		fsbno = map[i - 1].br_startoff + map[i - 1].br_blockcount;
 		start = XFS_FSB_TO_B(mp, fsbno);
-		if (start >= isize) {
+		if (start >= end) {
 			if (whence == SEEK_HOLE) {
-				offset = isize;
+				offset = end;
 				break;
 			}
 			ASSERT(whence == SEEK_DATA);
 			error = -ENXIO;
-			goto out_unlock;
+			goto out_error;
 		}
 	}
 
@@ -1464,7 +1464,39 @@ out:
 	 * situation in particular.
 	 */
 	if (whence == SEEK_HOLE)
-		offset = min_t(loff_t, offset, isize);
+		offset = min_t(loff_t, offset, end);
+
+	return offset;
+
+out_error:
+	return error;
+}
+
+STATIC loff_t
+xfs_seek_hole_data(
+	struct file		*file,
+	loff_t			start,
+	int			whence)
+{
+	struct inode		*inode = file->f_mapping->host;
+	struct xfs_inode	*ip = XFS_I(inode);
+	struct xfs_mount	*mp = ip->i_mount;
+	uint			lock;
+	loff_t			offset, end;
+	int			error = 0;
+
+	if (XFS_FORCED_SHUTDOWN(mp))
+		return -EIO;
+
+	lock = xfs_ilock_data_map_shared(ip);
+
+	end = i_size_read(inode);
+	offset = __xfs_seek_hole_data(inode, start, end, whence);
+	if (offset < 0) {
+		error = offset;
+		goto out_unlock;
+	}
+
 	offset = vfs_setpos(file, offset, inode->i_sb->s_maxbytes);
 
 out_unlock:
diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
index ca9e119..ed7e933 100644
--- a/fs/xfs/xfs_inode.h
+++ b/fs/xfs/xfs_inode.h
@@ -437,6 +437,8 @@ int	xfs_update_prealloc_flags(struct xfs_inode *ip,
 int	xfs_zero_eof(struct xfs_inode *ip, xfs_off_t offset,
 		     xfs_fsize_t isize, bool *did_zeroing);
 int	xfs_iozero(struct xfs_inode *ip, loff_t pos, size_t count);
+loff_t	__xfs_seek_hole_data(struct inode *inode, loff_t start,
+			     loff_t eof, int whence);
 
 
 /* from xfs_iops.c */
-- 
1.7.1


_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 4/4] xfs: wire up Q_XGETQUOTA2 / get_dqblk2

[Eric Sandeen]

Add code to allow the Q_XGETQUOTA2 quotactl to quickly find
all active quotas by examining the quota inode, and skipping
over unallocated or uninitialized regions.

Userspace can then use this interface rather than i.e. a
getpwent() loop when asked to report all active quotas.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 fs/xfs/libxfs/xfs_quota_defs.h |    3 +-
 fs/xfs/xfs_dquot.c             |   93 ++++++++++++++++++++++++++++++++++++++++
 fs/xfs/xfs_qm.h                |    2 +-
 fs/xfs/xfs_qm_syscalls.c       |   13 +++++-
 fs/xfs/xfs_quotaops.c          |   22 +++++++++-
 5 files changed, 128 insertions(+), 5 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_quota_defs.h b/fs/xfs/libxfs/xfs_quota_defs.h
index 1b0a083..428d882 100644
--- a/fs/xfs/libxfs/xfs_quota_defs.h
+++ b/fs/xfs/libxfs/xfs_quota_defs.h
@@ -37,7 +37,7 @@ typedef __uint16_t	xfs_qwarncnt_t;
 #define XFS_DQ_PROJ		0x0002		/* project quota */
 #define XFS_DQ_GROUP		0x0004		/* a group quota */
 #define XFS_DQ_DIRTY		0x0008		/* dquot is dirty */
-#define XFS_DQ_FREEING		0x0010		/* dquot is beeing torn down */
+#define XFS_DQ_FREEING		0x0010		/* dquot is being torn down */
 
 #define XFS_DQ_ALLTYPES		(XFS_DQ_USER|XFS_DQ_PROJ|XFS_DQ_GROUP)
 
@@ -116,6 +116,7 @@ typedef __uint16_t	xfs_qwarncnt_t;
 #define XFS_QMOPT_DQREPAIR	0x0001000 /* repair dquot if damaged */
 #define XFS_QMOPT_GQUOTA	0x0002000 /* group dquot requested */
 #define XFS_QMOPT_ENOSPC	0x0004000 /* enospc instead of edquot (prj) */
+#define XFS_QMOPT_DQNEXT	0x0008000 /* return next dquot >= this ID */
 
 /*
  * flags to xfs_trans_mod_dquot to indicate which field needs to be
diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
index 1983afc..83ce4aa 100644
--- a/fs/xfs/xfs_dquot.c
+++ b/fs/xfs/xfs_dquot.c
@@ -686,6 +686,56 @@ error0:
 }
 
 /*
+ * Advance to the next id in the current chunk, or if at the
+ * end of the chunk, skip ahead to first id in next allocated chunk
+ * using the SEEK_DATA interface.
+ */
+int
+xfs_dq_get_next_id(
+	xfs_mount_t		*mp,
+	uint			type,
+	xfs_dqid_t		*id,
+	loff_t			eof)
+{
+	struct xfs_inode	*quotip;
+	xfs_fsblock_t		start;
+	loff_t			offset;
+	uint			lock;
+	xfs_dqid_t		next_id;
+	int			error = 0;
+
+	/* Simple advance */
+	next_id = *id + 1;
+	
+	/* If new ID is within the current chunk, advancing it sufficed */
+	if (next_id % mp->m_quotainfo->qi_dqperchunk) {
+		*id = next_id;
+		return 0;
+	}
+
+	/* Nope, next_id is now past the current chunk, so find the next one */
+	start = (xfs_fsblock_t)next_id / mp->m_quotainfo->qi_dqperchunk;
+
+	quotip = xfs_quota_inode(mp, type);
+	lock = xfs_ilock_data_map_shared(quotip);
+
+	offset = __xfs_seek_hole_data(VFS_I(quotip), XFS_FSB_TO_B(mp, start),
+				      eof, SEEK_DATA);
+	if (offset < 0)
+		error = offset;
+
+	xfs_iunlock(quotip, lock);
+
+	/* -ENXIO is essentially "no more data" */
+	if (error)
+		return (error == -ENXIO ? -ESRCH : error);
+
+	/* Convert next data offset back to a quota id */
+	*id = XFS_B_TO_FSB(mp, offset) * mp->m_quotainfo->qi_dqperchunk;
+	return 0;
+}
+
+/*
  * Given the file system, inode OR id, and type (UDQUOT/GDQUOT), return a
  * a locked dquot, doing an allocation (if requested) as needed.
  * When both an inode and an id are given, the inode's id takes precedence.
@@ -705,6 +755,7 @@ xfs_qm_dqget(
 	struct xfs_quotainfo	*qi = mp->m_quotainfo;
 	struct radix_tree_root *tree = xfs_dquot_tree(qi, type);
 	struct xfs_dquot	*dqp;
+	loff_t			eof = 0;
 	int			error;
 
 	ASSERT(XFS_IS_QUOTA_RUNNING(mp));
@@ -732,6 +783,18 @@ xfs_qm_dqget(
 	}
 #endif
 
+	/* Get the end of the quota file if we need it */
+	if (flags & XFS_QMOPT_DQNEXT) {
+		struct xfs_inode	*quotip;
+		xfs_fileoff_t		last;
+
+		quotip = xfs_quota_inode(mp, type);
+		error = xfs_bmap_last_offset(quotip, &last, XFS_DATA_FORK);
+		if (error)
+			return error;
+		eof = XFS_FSB_TO_B(mp, last);
+	}
+
 restart:
 	mutex_lock(&qi->qi_tree_lock);
 	dqp = radix_tree_lookup(tree, id);
@@ -745,6 +808,18 @@ restart:
 			goto restart;
 		}
 
+		/* uninit / unused quota found in radix tree, keep looking  */
+		if (flags & XFS_QMOPT_DQNEXT) {
+			if (XFS_IS_DQUOT_UNINITIALIZED(dqp)) {
+				xfs_dqunlock(dqp);
+				mutex_unlock(&qi->qi_tree_lock);
+				error = xfs_dq_get_next_id(mp, type, &id, eof);
+				if (error)
+					return error;
+				goto restart;
+			}
+		}
+
 		dqp->q_nrefs++;
 		mutex_unlock(&qi->qi_tree_lock);
 
@@ -771,6 +846,13 @@ restart:
 	if (ip)
 		xfs_ilock(ip, XFS_ILOCK_EXCL);
 
+	/* If we are asked to find next active id, keep looking */
+	if (error == -ENOENT && (flags & XFS_QMOPT_DQNEXT)) {
+		error = xfs_dq_get_next_id(mp, type, &id, eof);
+		if (!error)
+			goto restart;
+	}
+
 	if (error)
 		return error;
 
@@ -821,6 +903,17 @@ restart:
 	qi->qi_dquots++;
 	mutex_unlock(&qi->qi_tree_lock);
 
+	/* If we are asked to find next active id, keep looking */
+	if (flags & XFS_QMOPT_DQNEXT) {
+		if (XFS_IS_DQUOT_UNINITIALIZED(dqp)) {
+			xfs_qm_dqput(dqp);
+			error = xfs_dq_get_next_id(mp, type, &id, eof);
+			if (error)
+				return error;
+			goto restart;
+		}
+	}
+
  dqret:
 	ASSERT((ip == NULL) || xfs_isilocked(ip, XFS_ILOCK_EXCL));
 	trace_xfs_dqget_miss(dqp);
diff --git a/fs/xfs/xfs_qm.h b/fs/xfs/xfs_qm.h
index 8901a01..ab462f2 100644
--- a/fs/xfs/xfs_qm.h
+++ b/fs/xfs/xfs_qm.h
@@ -165,7 +165,7 @@ extern void		xfs_qm_dqrele_all_inodes(struct xfs_mount *, uint);
 /* quota ops */
 extern int		xfs_qm_scall_trunc_qfiles(struct xfs_mount *, uint);
 extern int		xfs_qm_scall_getquota(struct xfs_mount *, xfs_dqid_t,
-					uint, struct qc_dqblk *);
+					uint, struct qc_dqblk *, qid_t *);
 extern int		xfs_qm_scall_setqlim(struct xfs_mount *, xfs_dqid_t, uint,
 					struct qc_dqblk *);
 extern int		xfs_qm_scall_quotaon(struct xfs_mount *, uint);
diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c
index 3640c6e..57c64ff 100644
--- a/fs/xfs/xfs_qm_syscalls.c
+++ b/fs/xfs/xfs_qm_syscalls.c
@@ -637,17 +637,23 @@ xfs_qm_scall_getquota(
 	struct xfs_mount	*mp,
 	xfs_dqid_t		id,
 	uint			type,
-	struct qc_dqblk		*dst)
+	struct qc_dqblk		*dst,
+	qid_t			*id_out)
 {
 	struct xfs_dquot	*dqp;
 	int			error;
+	uint			flags = 0;
+
+	/* Asking for *id_out means we want the next active one */
+	if (id_out)
+		flags = XFS_QMOPT_DQNEXT;
 
 	/*
 	 * Try to get the dquot. We don't want it allocated on disk, so
 	 * we aren't passing the XFS_QMOPT_DOALLOC flag. If it doesn't
 	 * exist, we'll get ENOENT back.
 	 */
-	error = xfs_qm_dqget(mp, NULL, id, type, 0, &dqp);
+	error = xfs_qm_dqget(mp, NULL, id, type, flags, &dqp);
 	if (error)
 		return error;
 
@@ -681,6 +687,9 @@ xfs_qm_scall_getquota(
 	dst->d_rt_spc_timer = be32_to_cpu(dqp->q_core.d_rtbtimer);
 	dst->d_rt_spc_warns = be16_to_cpu(dqp->q_core.d_rtbwarns);
 
+	if (id_out)
+		*id_out = be32_to_cpu(dqp->q_core.d_id);
+
 	/*
 	 * Internally, we don't reset all the timers when quota enforcement
 	 * gets turned off. No need to confuse the user level code,
diff --git a/fs/xfs/xfs_quotaops.c b/fs/xfs/xfs_quotaops.c
index 7795e0d..3dd5369 100644
--- a/fs/xfs/xfs_quotaops.c
+++ b/fs/xfs/xfs_quotaops.c
@@ -238,7 +238,26 @@ xfs_fs_get_dqblk(
 		return -ESRCH;
 
 	return xfs_qm_scall_getquota(mp, from_kqid(&init_user_ns, qid),
-				      xfs_quota_type(qid.type), qdq);
+				      xfs_quota_type(qid.type), qdq, NULL);
+}
+
+/* Return quota info for active quota >= this qid */
+STATIC int
+xfs_fs_get_dqblk2(
+	struct super_block	*sb,
+	struct kqid		qid,
+	qid_t			*id,
+	struct qc_dqblk		*qdq)
+{
+	struct xfs_mount	*mp = XFS_M(sb);
+
+	if (!XFS_IS_QUOTA_RUNNING(mp))
+		return -ENOSYS;
+	if (!XFS_IS_QUOTA_ON(mp))
+		return -ESRCH;
+
+	return xfs_qm_scall_getquota(mp, from_kqid(&init_user_ns, qid),
+				      xfs_quota_type(qid.type), qdq, id);
 }
 
 STATIC int
@@ -267,5 +286,6 @@ const struct quotactl_ops xfs_quotactl_operations = {
 	.quota_disable		= xfs_quota_disable,
 	.rm_xquota		= xfs_fs_rm_xquota,
 	.get_dqblk		= xfs_fs_get_dqblk,
+	.get_dqblk2		= xfs_fs_get_dqblk2,
 	.set_dqblk		= xfs_fs_set_dqblk,
 };
-- 
1.7.1

[PATCH] linux-quota: wire Q_XGETQUOTA2 into generic repquota

[Eric Sandeen]

Here's a patch to hook Q_XGETQUOTA2 into the generic quota
tools repquota command.

Rather than looping over getpwent(), it increments the id sent
into the quotactl until it gets back ESRCH.

If Q_XGETQUOTA2 doesn't exist it falls back to the old method.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

diff --git a/dqblk_xfs.h b/dqblk_xfs.h
index 415e646..6c7693d 100644
--- a/dqblk_xfs.h
+++ b/dqblk_xfs.h
@@ -10,6 +10,7 @@
 #define Q_XFS_QUOTAON	Q_XQUOTAON
 #define Q_XFS_QUOTAOFF	Q_XQUOTAOFF
 #define Q_XFS_GETQUOTA	Q_XGETQUOTA
+#define Q_XFS_GETQUOTA2	Q_XGETQUOTA2
 #define Q_XFS_SETQLIM	Q_XSETQLIM
 #define Q_XFS_GETQSTAT	Q_XGETQSTAT
 #define Q_XFS_QUOTARM	Q_XQUOTARM
diff --git a/quotaio_generic.c b/quotaio_generic.c
index 5001a56..ad84cc0 100644
--- a/quotaio_generic.c
+++ b/quotaio_generic.c
@@ -161,3 +161,52 @@ int generic_scan_dquots(struct quota_handle *h,
 	free(dquot);
 	return ret;
 }
+
+/* Generic quota scanning using Q_XGETQUOTA2... */
+int generic_scan_dquots2(struct quota_handle *h,
+			int (*process_dquot)(struct dquot *dquot, char *dqname),
+			int (*get_dquot)(struct dquot *dquot))
+{
+	struct dquot *dquot = get_empty_dquot();
+	char namebuf[MAXNAMELEN];
+	int ret = 0;
+
+	dquot->dq_id = 0;
+	dquot->dq_h = h;
+	if (h->qh_type == USRQUOTA) {
+		while (1) {
+			ret = scan_one_dquot(dquot, get_dquot);
+			if (ret < 0) {
+				if (errno == ESRCH)
+					ret =0;
+				break;
+			}
+			if (ret > 0)
+				continue;
+			id2name(dquot->dq_id, dquot->dq_h->qh_type, namebuf);
+			ret = process_dquot(dquot, namebuf);
+			if (ret < 0)
+				break;
+			dquot->dq_id++;
+		}
+	} else if (h->qh_type == GRPQUOTA) {
+		while (1) {
+			ret = scan_one_dquot(dquot, get_dquot);
+			if (ret < 0) {
+				if (errno == ESRCH)
+					ret =0;
+				break;
+			}
+			if (ret > 0)
+				continue;
+			id2name(dquot->dq_id, dquot->dq_h->qh_type, namebuf);
+			ret = process_dquot(dquot, namebuf);
+			if (ret < 0)
+				break;
+			dquot->dq_id++;
+		}
+	}
+	free(dquot);
+	return ret;
+}
+
diff --git a/quotaio_generic.h b/quotaio_generic.h
index 5edc11c..099a6b1 100644
--- a/quotaio_generic.h
+++ b/quotaio_generic.h
@@ -26,5 +26,9 @@ int vfs_set_dquot(struct dquot *dquot, int flags);
 int generic_scan_dquots(struct quota_handle *h,
 			int (*process_dquot)(struct dquot *dquot, char *dqname),
 			int (*get_dquot)(struct dquot *dquot));
+/* Generic routine for scanning dquots when kernel can do the scanning */
+int generic_scan_dquots2(struct quota_handle *h,
+			int (*process_dquot)(struct dquot *dquot, char *dqname),
+			int (*get_dquot)(struct dquot *dquot));
 
 #endif
diff --git a/quotaio_xfs.c b/quotaio_xfs.c
index 903c03e..a3f516b 100644
--- a/quotaio_xfs.c
+++ b/quotaio_xfs.c
@@ -192,14 +192,42 @@ static int xfs_get_dquot(struct dquot *dq)
 }
 
 /*
+ *	xfs_scan_dquots helper - processes a single dquot with Q_XGETQUOTA2
+ */
+static int xfs_get_dquot2(struct dquot *dq)
+{
+	struct xfs_kern_dqblk d;
+	int qcmd = QCMD(Q_XFS_GETQUOTA2, dq->dq_h->qh_type);
+	int ret;
+
+	memset(&d, 0, sizeof(d));
+	ret = quotactl(qcmd, dq->dq_h->qh_quotadev, dq->dq_id, (void *)&d);
+	if (ret < 0) {
+		if (errno == ENOENT)
+			return 0;
+		return -1;
+	}
+	dq->dq_id = d.d_id;
+	xfs_kern2utildqblk(&dq->dq_dqb, &d);
+	return 0;
+}
+
+/*
  *	Scan all known dquots and call callback on each
  */
 static int xfs_scan_dquots(struct quota_handle *h, int (*process_dquot) (struct dquot *dquot, char *dqname))
 {
+	int ret;
+
 	if (!XFS_USRQUOTA(h) && !XFS_GRPQUOTA(h))
 		return 0;
 
-	return generic_scan_dquots(h, process_dquot, xfs_get_dquot);
+	ret = generic_scan_dquots2(h, process_dquot, xfs_get_dquot2);
+
+	if (ret)
+		ret = generic_scan_dquots(h, process_dquot, xfs_get_dquot);
+
+	return ret;
 }
 
 /*
diff --git a/quotaio_xfs.h b/quotaio_xfs.h
index 54725b0..eabee3e 100644
--- a/quotaio_xfs.h
+++ b/quotaio_xfs.h
@@ -46,6 +46,7 @@
 #define Q_XSETQLIM   XQM_CMD(0x4)	/* set disk limits only */
 #define Q_XGETQSTAT  XQM_CMD(0x5)	/* returns fs_quota_stat_t struct */
 #define Q_XQUOTARM   XQM_CMD(0x6)	/* free quota files' space */
+#define Q_XGETQUOTA2 XQM_CMD(0x9)	/* get disk limits and usage >= ID */
 
 /*
  * fs_disk_quota structure:

Re: [PATCH 0/4] quota: add new quotactl Q_XGETQUOTA2

[Christoph Hellwig]

On Fri, Jan 08, 2016 at 10:56:12AM -0600, Eric Sandeen wrote:
> This adds a new quotactl, Q_XGETQUOTA2.
> 
> Q_XGETQUOTA2 is exactly like Q_XGETQUOTA, except that it will
> return quota information for the id equal to or greater than
> the id requested.  In other words, if the specified id has
> no quota, the command will return quota information for the
> next higher id which does have a quota set.  If no higher id
> has an active quota, -ESRCH is returned.

Please add a flags argument to Q_XGETQUOTA2, and then make the
new behavior the first flag.  Keep Q_XGETQUOTA behavior for the
flag-less case.  That way we get future etensibility for free.

Re: [PATCH 0/4] quota: add new quotactl Q_XGETQUOTA2

[Jan Kara]

On Fri 08-01-16 23:26:00, Christoph Hellwig wrote:
> On Fri, Jan 08, 2016 at 10:56:12AM -0600, Eric Sandeen wrote:
> > This adds a new quotactl, Q_XGETQUOTA2.
> > 
> > Q_XGETQUOTA2 is exactly like Q_XGETQUOTA, except that it will
> > return quota information for the id equal to or greater than
> > the id requested.  In other words, if the specified id has
> > no quota, the command will return quota information for the
> > next higher id which does have a quota set.  If no higher id
> > has an active quota, -ESRCH is returned.
> 
> Please add a flags argument to Q_XGETQUOTA2, and then make the
> new behavior the first flag.  Keep Q_XGETQUOTA behavior for the
> flag-less case.  That way we get future etensibility for free.

So this is what I wanted to suggest at first as well. What I somewhat
dislike is that 'addr' must now point to something like:

struct getquota_args {
	__u64 flags;
	struct fs_disk_quota ret;
};

which is not as nice as passing pointer to fs_disk_quota directly. But
probably still OK. So I agree with the flags idea.

Another issue is that OCFS2 and ext4 with quota in hidden inodes would need
call with these capabilities as well (they have the same problem as xfs).
For reporting VFS quota we use bytes for space (since some filesystems need
it) and we don't need RT fields (easy so zero-fill) and warning fields
(there zeros may be confusing). So we would need something like struct
qc_dqblk (currently only internal) extended with ID and maybe flags field
XFS is using to return quota type which can contain all the information and
tell which info is actually valid as return structure.

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

Re: [PATCH 0/4] quota: add new quotactl Q_XGETQUOTA2

[Eric Sandeen]

On 1/11/16 7:26 AM, Jan Kara wrote:
> On Fri 08-01-16 23:26:00, Christoph Hellwig wrote:
>> On Fri, Jan 08, 2016 at 10:56:12AM -0600, Eric Sandeen wrote:
>>> This adds a new quotactl, Q_XGETQUOTA2.
>>>
>>> Q_XGETQUOTA2 is exactly like Q_XGETQUOTA, except that it will
>>> return quota information for the id equal to or greater than
>>> the id requested.  In other words, if the specified id has
>>> no quota, the command will return quota information for the
>>> next higher id which does have a quota set.  If no higher id
>>> has an active quota, -ESRCH is returned.
>>
>> Please add a flags argument to Q_XGETQUOTA2, and then make the
>> new behavior the first flag.  Keep Q_XGETQUOTA behavior for the
>> flag-less case.  That way we get future etensibility for free.
> 
> So this is what I wanted to suggest at first as well. What I somewhat
> dislike is that 'addr' must now point to something like:
> 
> struct getquota_args {
> 	__u64 flags;
> 	struct fs_disk_quota ret;
> };

Ok...

> which is not as nice as passing pointer to fs_disk_quota directly. But
> probably still OK. So I agree with the flags idea.

I understand that flags are for future-proofing, for possibly-not-yet-imagined
scenarios, but on the other hand, I wonder how many different flavors of
"get me a disk quota" we really expect to have?

> Another issue is that OCFS2 and ext4 with quota in hidden inodes would need
> call with these capabilities as well (they have the same problem as xfs).
> For reporting VFS quota we use bytes for space (since some filesystems need
> it) and we don't need RT fields (easy so zero-fill) and warning fields
> (there zeros may be confusing). So we would need something like struct
> qc_dqblk (currently only internal) extended with ID and maybe flags field
> XFS is using to return quota type which can contain all the information and
> tell which info is actually valid as return structure.

Hohum, yeah - I forgot that ext4 has (can have?) a hidden quota inode.
And I didn't know about OCFS2.

And TBH I did the xfs-specific one because it was trivial to extend, with the
id already in the returned quota struct.

So ok, it sounds like this needs a more significant overhaul at the vfs
quota level...

-Eric

> 
> 								Honza
> 

Re: [PATCH 0/4] quota: add new quotactl Q_XGETQUOTA2

[Jan Kara]

On Mon 11-01-16 10:07:22, Eric Sandeen wrote:
> On 1/11/16 7:26 AM, Jan Kara wrote:
> > On Fri 08-01-16 23:26:00, Christoph Hellwig wrote:
> >> On Fri, Jan 08, 2016 at 10:56:12AM -0600, Eric Sandeen wrote:
> >>> This adds a new quotactl, Q_XGETQUOTA2.
> >>>
> >>> Q_XGETQUOTA2 is exactly like Q_XGETQUOTA, except that it will
> >>> return quota information for the id equal to or greater than
> >>> the id requested.  In other words, if the specified id has
> >>> no quota, the command will return quota information for the
> >>> next higher id which does have a quota set.  If no higher id
> >>> has an active quota, -ESRCH is returned.
> >>
> >> Please add a flags argument to Q_XGETQUOTA2, and then make the
> >> new behavior the first flag.  Keep Q_XGETQUOTA behavior for the
> >> flag-less case.  That way we get future etensibility for free.
> > 
> > So this is what I wanted to suggest at first as well. What I somewhat
> > dislike is that 'addr' must now point to something like:
> > 
> > struct getquota_args {
> > 	__u64 flags;
> > 	struct fs_disk_quota ret;
> > };
> 
> Ok...
> 
> > which is not as nice as passing pointer to fs_disk_quota directly. But
> > probably still OK. So I agree with the flags idea.
> 
> I understand that flags are for future-proofing, for possibly-not-yet-imagined
> scenarios, but on the other hand, I wonder how many different flavors of
> "get me a disk quota" we really expect to have?
> 
> > Another issue is that OCFS2 and ext4 with quota in hidden inodes would need
> > call with these capabilities as well (they have the same problem as xfs).
> > For reporting VFS quota we use bytes for space (since some filesystems need
> > it) and we don't need RT fields (easy so zero-fill) and warning fields
> > (there zeros may be confusing). So we would need something like struct
> > qc_dqblk (currently only internal) extended with ID and maybe flags field
> > XFS is using to return quota type which can contain all the information and
> > tell which info is actually valid as return structure.
> 
> Hohum, yeah - I forgot that ext4 has (can have?) a hidden quota inode.
> And I didn't know about OCFS2.
> 
> And TBH I did the xfs-specific one because it was trivial to extend, with the
> id already in the returned quota struct.
> 
> So ok, it sounds like this needs a more significant overhaul at the vfs
> quota level...

Actually, what I want from you is just an interface which is usable for VFS
quotas as well since I'd like to avoid adding GETQUOTA2 quotactl shortly
after XGETQUOTA2 :). Using id as a cooking is fine for VFS quotas as well so
that part can stay.

I can have a look at implementing the new handler for VFS quota format(s)
which should be pretty easy once the interface part is merged or you can
do it if you're interested in learning more about quota internals.

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

Re: [PATCH 0/4] quota: add new quotactl Q_XGETQUOTA2

[Eric Sandeen]

On 1/11/16 10:28 AM, Jan Kara wrote:
> On Mon 11-01-16 10:07:22, Eric Sandeen wrote:
>> On 1/11/16 7:26 AM, Jan Kara wrote:
>>> On Fri 08-01-16 23:26:00, Christoph Hellwig wrote:
>>>> On Fri, Jan 08, 2016 at 10:56:12AM -0600, Eric Sandeen wrote:
>>>>> This adds a new quotactl, Q_XGETQUOTA2.
>>>>>
>>>>> Q_XGETQUOTA2 is exactly like Q_XGETQUOTA, except that it will
>>>>> return quota information for the id equal to or greater than
>>>>> the id requested.  In other words, if the specified id has
>>>>> no quota, the command will return quota information for the
>>>>> next higher id which does have a quota set.  If no higher id
>>>>> has an active quota, -ESRCH is returned.
>>>>
>>>> Please add a flags argument to Q_XGETQUOTA2, and then make the
>>>> new behavior the first flag.  Keep Q_XGETQUOTA behavior for the
>>>> flag-less case.  That way we get future etensibility for free.
>>>
>>> So this is what I wanted to suggest at first as well. What I somewhat
>>> dislike is that 'addr' must now point to something like:
>>>
>>> struct getquota_args {
>>> 	__u64 flags;
>>> 	struct fs_disk_quota ret;
>>> };
>>
>> Ok...
>>
>>> which is not as nice as passing pointer to fs_disk_quota directly. But
>>> probably still OK. So I agree with the flags idea.
>>
>> I understand that flags are for future-proofing, for possibly-not-yet-imagined
>> scenarios, but on the other hand, I wonder how many different flavors of
>> "get me a disk quota" we really expect to have?
>>
>>> Another issue is that OCFS2 and ext4 with quota in hidden inodes would need
>>> call with these capabilities as well (they have the same problem as xfs).
>>> For reporting VFS quota we use bytes for space (since some filesystems need
>>> it) and we don't need RT fields (easy so zero-fill) and warning fields
>>> (there zeros may be confusing). So we would need something like struct
>>> qc_dqblk (currently only internal) extended with ID and maybe flags field
>>> XFS is using to return quota type which can contain all the information and
>>> tell which info is actually valid as return structure.
>>
>> Hohum, yeah - I forgot that ext4 has (can have?) a hidden quota inode.
>> And I didn't know about OCFS2.
>>
>> And TBH I did the xfs-specific one because it was trivial to extend, with the
>> id already in the returned quota struct.
>>
>> So ok, it sounds like this needs a more significant overhaul at the vfs
>> quota level...
> 
> Actually, what I want from you is just an interface which is usable for VFS
> quotas as well since I'd like to avoid adding GETQUOTA2 quotactl shortly
> after XGETQUOTA2 :).

Actually, that's exactly what I thought would *need* to happen ... we already
have this weird 15-year-old split-brain quota interface, so if xfs and ext4
both need the same functionality, then we'd probably add both GETQUOTA2 and
XGETQUOTA2.  If we were doing this all from scratch, sure, but adding a new
handles-both-quota-types interface when every other operation is already split
between the two almost seems to make matters worse.

-Eric

Re: [PATCH 0/4] quota: add new quotactl Q_XGETQUOTA2

[Jan Kara]

On Wed 13-01-16 16:40:58, Eric Sandeen wrote:
> On 1/11/16 10:28 AM, Jan Kara wrote:
> > On Mon 11-01-16 10:07:22, Eric Sandeen wrote:
> >> On 1/11/16 7:26 AM, Jan Kara wrote:
> >>> On Fri 08-01-16 23:26:00, Christoph Hellwig wrote:
> >>>> On Fri, Jan 08, 2016 at 10:56:12AM -0600, Eric Sandeen wrote:
> >>>>> This adds a new quotactl, Q_XGETQUOTA2.
> >>>>>
> >>>>> Q_XGETQUOTA2 is exactly like Q_XGETQUOTA, except that it will
> >>>>> return quota information for the id equal to or greater than
> >>>>> the id requested.  In other words, if the specified id has
> >>>>> no quota, the command will return quota information for the
> >>>>> next higher id which does have a quota set.  If no higher id
> >>>>> has an active quota, -ESRCH is returned.
> >>>>
> >>>> Please add a flags argument to Q_XGETQUOTA2, and then make the
> >>>> new behavior the first flag.  Keep Q_XGETQUOTA behavior for the
> >>>> flag-less case.  That way we get future etensibility for free.
> >>>
> >>> So this is what I wanted to suggest at first as well. What I somewhat
> >>> dislike is that 'addr' must now point to something like:
> >>>
> >>> struct getquota_args {
> >>> 	__u64 flags;
> >>> 	struct fs_disk_quota ret;
> >>> };
> >>
> >> Ok...
> >>
> >>> which is not as nice as passing pointer to fs_disk_quota directly. But
> >>> probably still OK. So I agree with the flags idea.
> >>
> >> I understand that flags are for future-proofing, for possibly-not-yet-imagined
> >> scenarios, but on the other hand, I wonder how many different flavors of
> >> "get me a disk quota" we really expect to have?
> >>
> >>> Another issue is that OCFS2 and ext4 with quota in hidden inodes would need
> >>> call with these capabilities as well (they have the same problem as xfs).
> >>> For reporting VFS quota we use bytes for space (since some filesystems need
> >>> it) and we don't need RT fields (easy so zero-fill) and warning fields
> >>> (there zeros may be confusing). So we would need something like struct
> >>> qc_dqblk (currently only internal) extended with ID and maybe flags field
> >>> XFS is using to return quota type which can contain all the information and
> >>> tell which info is actually valid as return structure.
> >>
> >> Hohum, yeah - I forgot that ext4 has (can have?) a hidden quota inode.
> >> And I didn't know about OCFS2.
> >>
> >> And TBH I did the xfs-specific one because it was trivial to extend, with the
> >> id already in the returned quota struct.
> >>
> >> So ok, it sounds like this needs a more significant overhaul at the vfs
> >> quota level...
> > 
> > Actually, what I want from you is just an interface which is usable for VFS
> > quotas as well since I'd like to avoid adding GETQUOTA2 quotactl shortly
> > after XGETQUOTA2 :).
> 
> Actually, that's exactly what I thought would *need* to happen ... we already
> have this weird 15-year-old split-brain quota interface, so if xfs and ext4
> both need the same functionality, then we'd probably add both GETQUOTA2 and
> XGETQUOTA2.  If we were doing this all from scratch, sure, but adding a new
> handles-both-quota-types interface when every other operation is already split
> between the two almost seems to make matters worse.

Well, currently GETQUOTA and XGETQUOTA (and all the other quotactls) are
actually translated so they work regardless of the underlying filesystem.
So the only difference between XFS and VFS quotactls is in the formatting
of input/output structures. So from kernel POV it seems somewhat pointless
to add two calls doing the same thing and differing just in the formatting
of output - especially when we want the call to be extensible.

I agree that having a unified call means having a new structure for passing
dquot info between kernel and userspace. So just for adding that one small
feature you want it seems like an overkill. But when thinking about new
extensible getquota quotactl it IMHO makes sense to unify the VFS/XFS split
brain. Thoughts?

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

Re: [PATCH 0/4] quota: add new quotactl Q_XGETQUOTA2

[Eric Sandeen]

Hi Jan -

On 1/15/16 3:35 AM, Jan Kara wrote:
> On Wed 13-01-16 16:40:58, Eric Sandeen wrote:
>> On 1/11/16 10:28 AM, Jan Kara wrote:

...

>>> Actually, what I want from you is just an interface which is usable for VFS
>>> quotas as well since I'd like to avoid adding GETQUOTA2 quotactl shortly
>>> after XGETQUOTA2 :).
>>
>> Actually, that's exactly what I thought would *need* to happen ... we already
>> have this weird 15-year-old split-brain quota interface, so if xfs and ext4
>> both need the same functionality, then we'd probably add both GETQUOTA2 and
>> XGETQUOTA2.  If we were doing this all from scratch, sure, but adding a new
>> handles-both-quota-types interface when every other operation is already split
>> between the two almost seems to make matters worse.
> 
> Well, currently GETQUOTA and XGETQUOTA (and all the other quotactls) are
> actually translated so they work regardless of the underlying filesystem.
> So the only difference between XFS and VFS quotactls is in the formatting
> of input/output structures. So from kernel POV it seems somewhat pointless
> to add two calls doing the same thing and differing just in the formatting
> of output - especially when we want the call to be extensible.
> 
> I agree that having a unified call means having a new structure for passing
> dquot info between kernel and userspace. So just for adding that one small
> feature you want it seems like an overkill. But when thinking about new
> extensible getquota quotactl it IMHO makes sense to unify the VFS/XFS split
> brain. Thoughts?

My first lazy/hacky thought is "how terrible would it be to overload the
quotactl syscall return value with quota ID for Q_GETQUOTA2 calls?"

For a purpose-built interface of "find the next ID" that wouldn't require any
structure or interface changes...

We could name it Q_GETNEXTQUOTA / Q_XGETNEXTQUOTA to make it explicit about
the purpose, and document that return behavior.  Done & done.  ;)

A new grand unified extensible quota call sounds like a great idea, I just
hate to gate this work on designing a brand-new interface.

-Eric

Re: [PATCH 0/4] quota: add new quotactl Q_XGETQUOTA2

[Eric Sandeen]

On 1/15/16 11:31 AM, Eric Sandeen wrote:

> My first lazy/hacky thought is "how terrible would it be to overload the
> quotactl syscall return value with quota ID for Q_GETQUOTA2 calls?"

Never mind, we can use all 32 bits of the unsigned ID, I think, so that
doesn't work.  (thanks Zach) :)

-Eric

Re: [PATCH 0/4] quota: add new quotactl Q_XGETQUOTA2

[Jan Kara]

On Fri 15-01-16 11:31:00, Eric Sandeen wrote:
> Hi Jan -
> 
> On 1/15/16 3:35 AM, Jan Kara wrote:
> > On Wed 13-01-16 16:40:58, Eric Sandeen wrote:
> >> On 1/11/16 10:28 AM, Jan Kara wrote:
> 
> ...
> 
> >>> Actually, what I want from you is just an interface which is usable for VFS
> >>> quotas as well since I'd like to avoid adding GETQUOTA2 quotactl shortly
> >>> after XGETQUOTA2 :).
> >>
> >> Actually, that's exactly what I thought would *need* to happen ... we already
> >> have this weird 15-year-old split-brain quota interface, so if xfs and ext4
> >> both need the same functionality, then we'd probably add both GETQUOTA2 and
> >> XGETQUOTA2.  If we were doing this all from scratch, sure, but adding a new
> >> handles-both-quota-types interface when every other operation is already split
> >> between the two almost seems to make matters worse.
> > 
> > Well, currently GETQUOTA and XGETQUOTA (and all the other quotactls) are
> > actually translated so they work regardless of the underlying filesystem.
> > So the only difference between XFS and VFS quotactls is in the formatting
> > of input/output structures. So from kernel POV it seems somewhat pointless
> > to add two calls doing the same thing and differing just in the formatting
> > of output - especially when we want the call to be extensible.
> > 
> > I agree that having a unified call means having a new structure for passing
> > dquot info between kernel and userspace. So just for adding that one small
> > feature you want it seems like an overkill. But when thinking about new
> > extensible getquota quotactl it IMHO makes sense to unify the VFS/XFS split
> > brain. Thoughts?
> 
> My first lazy/hacky thought is "how terrible would it be to overload the
> quotactl syscall return value with quota ID for Q_GETQUOTA2 calls?"

As you said, that won't quite work...

> For a purpose-built interface of "find the next ID" that wouldn't require any
> structure or interface changes...
> 
> We could name it Q_GETNEXTQUOTA / Q_XGETNEXTQUOTA to make it explicit about
> the purpose, and document that return behavior.  Done & done.  ;)
>
> A new grand unified extensible quota call sounds like a great idea, I just
> hate to gate this work on designing a brand-new interface.

OK, ok. I like Dave's proposal for quotactl2(). So let's leave the unification
for later and implent Q_GETNEXTQUOTA and Q_XGETNEXTQUOTA with the
functionality of your original Q_XGETQUOTA2. Having separate call to get
next ID would save us one new quotactl but OTOH we would need two syscalls
(and quota structure lookups) to report one structure and there are
potentially *lots* of them.

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

Re: [PATCH 0/4] quota: add new quotactl Q_XGETQUOTA2

[Christoph Hellwig]

On Mon, Jan 18, 2016 at 11:33:02AM +0100, Jan Kara wrote:
> OK, ok. I like Dave's proposal for quotactl2(). So let's leave the unification
> for later and implent Q_GETNEXTQUOTA and Q_XGETNEXTQUOTA with the
> functionality of your original Q_XGETQUOTA2. Having separate call to get
> next ID would save us one new quotactl but OTOH we would need two syscalls
> (and quota structure lookups) to report one structure and there are
> potentially *lots* of them.

Oh well.  Guess I should give in and just accept the original patch..

Re: [PATCH 0/4] quota: add new quotactl Q_XGETQUOTA2

[Eric Sandeen]

On 1/18/16 4:33 AM, Jan Kara wrote:
> On Fri 15-01-16 11:31:00, Eric Sandeen wrote:

...

>> For a purpose-built interface of "find the next ID" that wouldn't require any
>> structure or interface changes...
>>
>> We could name it Q_GETNEXTQUOTA / Q_XGETNEXTQUOTA to make it explicit about
>> the purpose, and document that return behavior.  Done & done.  ;)
>>
>> A new grand unified extensible quota call sounds like a great idea, I just
>> hate to gate this work on designing a brand-new interface.
> 
> OK, ok. I like Dave's proposal for quotactl2(). So let's leave the unification
> for later and implent Q_GETNEXTQUOTA and Q_XGETNEXTQUOTA with the
> functionality of your original Q_XGETQUOTA2. Having separate call to get
> next ID would save us one new quotactl but OTOH we would need two syscalls
> (and quota structure lookups) to report one structure and there are
> potentially *lots* of them.

Ok, I can re-do it with the new Q_[X]GETNEXTQUOTA names, I've already done
the non-xfs one as well, just starting testing on that.

With or without the flags argument?

Thanks,
-Eric

Re: [PATCH 0/4] quota: add new quotactl Q_XGETQUOTA2

[Jan Kara]

On Mon 18-01-16 09:18:16, Eric Sandeen wrote:
> On 1/18/16 4:33 AM, Jan Kara wrote:
> > On Fri 15-01-16 11:31:00, Eric Sandeen wrote:
> 
> ...
> 
> >> For a purpose-built interface of "find the next ID" that wouldn't require any
> >> structure or interface changes...
> >>
> >> We could name it Q_GETNEXTQUOTA / Q_XGETNEXTQUOTA to make it explicit about
> >> the purpose, and document that return behavior.  Done & done.  ;)
> >>
> >> A new grand unified extensible quota call sounds like a great idea, I just
> >> hate to gate this work on designing a brand-new interface.
> > 
> > OK, ok. I like Dave's proposal for quotactl2(). So let's leave the unification
> > for later and implent Q_GETNEXTQUOTA and Q_XGETNEXTQUOTA with the
> > functionality of your original Q_XGETQUOTA2. Having separate call to get
> > next ID would save us one new quotactl but OTOH we would need two syscalls
> > (and quota structure lookups) to report one structure and there are
> > potentially *lots* of them.
> 
> Ok, I can re-do it with the new Q_[X]GETNEXTQUOTA names, I've already done
> the non-xfs one as well, just starting testing on that.
> 
> With or without the flags argument?

Without. For further extensibility I'd really go for the unified API in the
end.

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

Re: [PATCH 0/4] quota: add new quotactl Q_XGETQUOTA2

[Dave Chinner]

On Fri, Jan 15, 2016 at 10:35:07AM +0100, Jan Kara wrote:
> On Wed 13-01-16 16:40:58, Eric Sandeen wrote:
> > Actually, that's exactly what I thought would *need* to happen ... we already
> > have this weird 15-year-old split-brain quota interface, so if xfs and ext4
> > both need the same functionality, then we'd probably add both GETQUOTA2 and
> > XGETQUOTA2.  If we were doing this all from scratch, sure, but adding a new
> > handles-both-quota-types interface when every other operation is already split
> > between the two almost seems to make matters worse.
> 
> Well, currently GETQUOTA and XGETQUOTA (and all the other quotactls) are
> actually translated so they work regardless of the underlying filesystem.
> So the only difference between XFS and VFS quotactls is in the formatting
> of input/output structures. So from kernel POV it seems somewhat pointless
> to add two calls doing the same thing and differing just in the formatting
> of output - especially when we want the call to be extensible.
> 
> I agree that having a unified call means having a new structure for passing
> dquot info between kernel and userspace. So just for adding that one small
> feature you want it seems like an overkill. But when thinking about new
> extensible getquota quotactl it IMHO makes sense to unify the VFS/XFS split
> brain. Thoughts?

A new unified userspace quota API would be nice, but it's a
completely separate piece of work. We'd also need to do userspace
work to support it, so I think this is a valid medium term goal,
but not somethign that should gate the work Eric is doing.

i.e. it might be best to introduce a quotactl2 syscall for a
unified interface, such that the syscall itself has a flags
argument rather than having to jump through hoops to embed flags
in argument structures.....

FWIW, converting xfs_quota to use a new kernel API is trivial (half
an hour's work) as it already has a kernel API abstraction layer for
cross platform support. Hence we can quickly get userspace support
in place to test such a new kernel API...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com