* [PATCH VER 4] Extend project quotas to support 32bit project identificators.
@ 2010-09-22 17:42 Arkadiusz Miśkiewicz
2010-09-22 17:45 ` [PATCH VER 4] xfsprogs: projid32bit handling Arkadiusz Miśkiewicz
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Arkadiusz Miśkiewicz @ 2010-09-22 17:42 UTC (permalink / raw)
To: xfs
This patch adds support for 32bit project quota identificators.
On disk format is backward compatible with 16bit projid numbers. projid
on disk is now keept in two 16bit values - di_projid_lo (which holds the
same position as old 16bit projid value) and new di_projid_hi (takes
existing padding) and convertes from/to 32bit value on the fly.
PROJID32BIT feature2 flag is set automaticly when trying to use 32bit
quota project identificator.
Signed-off-by: Arkadiusz Miśkiewicz <arekm@maven.pl>
---
What has changed?
- sb_bad_features2 is also updated
- new helper - xfs_addprojid32bit
- drop 16bit projid protection in latest kernels
I think it's ready to merge but it lacks final Reviewed-by,
Tested-by (beside me) etc :-(
fs/xfs/linux-2.6/xfs_ioctl.c | 30 ++++++++++++++++--------------
fs/xfs/linux-2.6/xfs_ioctl32.c | 6 ++++--
fs/xfs/linux-2.6/xfs_ioctl32.h | 5 +++--
fs/xfs/quota/xfs_qm.c | 12 ++++++------
fs/xfs/quota/xfs_qm_bhv.c | 2 +-
fs/xfs/quota/xfs_qm_syscalls.c | 2 +-
fs/xfs/xfs_dinode.h | 5 +++--
fs/xfs/xfs_fs.h | 5 +++--
fs/xfs/xfs_inode.c | 14 ++++++++------
fs/xfs/xfs_inode.h | 24 +++++++++++++++++++++---
fs/xfs/xfs_itable.c | 3 ++-
fs/xfs/xfs_rename.c | 2 +-
fs/xfs/xfs_sb.h | 17 ++++++++++++++++-
fs/xfs/xfs_types.h | 2 --
fs/xfs/xfs_utils.c | 21 +++++++++++++++++++++
fs/xfs/xfs_utils.h | 1 +
fs/xfs/xfs_vnodeops.c | 14 +++++++-------
17 files changed, 114 insertions(+), 51 deletions(-)
diff --git a/fs/xfs/linux-2.6/xfs_ioctl.c b/fs/xfs/linux-2.6/xfs_ioctl.c
index 4fec427..aa72465 100644
--- a/fs/xfs/linux-2.6/xfs_ioctl.c
+++ b/fs/xfs/linux-2.6/xfs_ioctl.c
@@ -788,7 +788,7 @@ xfs_ioc_fsgetxattr(
xfs_ilock(ip, XFS_ILOCK_SHARED);
fa.fsx_xflags = xfs_ip2xflags(ip);
fa.fsx_extsize = ip->i_d.di_extsize << ip->i_mount->m_sb.sb_blocklog;
- fa.fsx_projid = ip->i_d.di_projid;
+ fa.fsx_projid = xfs_get_projid(ip);
if (attr) {
if (ip->i_afp) {
@@ -907,13 +907,6 @@ xfs_ioctl_setattr(
return XFS_ERROR(EIO);
/*
- * Disallow 32bit project ids because on-disk structure
- * is 16bit only.
- */
- if ((mask & FSX_PROJID) && (fa->fsx_projid > (__uint16_t)-1))
- return XFS_ERROR(EINVAL);
-
- /*
* If disk quotas is on, we make sure that the dquots do exist on disk,
* before we start any other transactions. Trying to do this later
* is messy. We don't care to take a readlock to look at the ids
@@ -953,13 +946,22 @@ xfs_ioctl_setattr(
goto error_return;
}
- /*
- * Do a quota reservation only if projid is actually going to change.
- */
if (mask & FSX_PROJID) {
+ /*
+ * Switch on the PROJID32BIT superblock bit when needed
+ * (implies also FEATURES2)
+ */
+ if (!xfs_sb_version_hasprojid32bit(&ip->i_mount->m_sb) &&
+ fa->fsx_projid > (__uint16_t)-1)
+ xfs_addprojid32bit(tp, ip);
+
+ /*
+ * Do a quota reservation only if projid is actually
+ * going to change.
+ */
if (XFS_IS_QUOTA_RUNNING(mp) &&
XFS_IS_PQUOTA_ON(mp) &&
- ip->i_d.di_projid != fa->fsx_projid) {
+ xfs_get_projid(ip) != fa->fsx_projid) {
ASSERT(tp);
code = xfs_qm_vop_chown_reserve(tp, ip, udqp, gdqp,
capable(CAP_FOWNER) ?
@@ -1061,12 +1063,12 @@ xfs_ioctl_setattr(
* Change the ownerships and register quota modifications
* in the transaction.
*/
- if (ip->i_d.di_projid != fa->fsx_projid) {
+ if (xfs_get_projid(ip) != fa->fsx_projid) {
if (XFS_IS_QUOTA_RUNNING(mp) && XFS_IS_PQUOTA_ON(mp)) {
olddquot = xfs_qm_vop_chown(tp, ip,
&ip->i_gdquot, gdqp);
}
- ip->i_d.di_projid = fa->fsx_projid;
+ xfs_set_projid(ip, fa->fsx_projid);
/*
* We may have to rev the inode as well as
diff --git a/fs/xfs/linux-2.6/xfs_ioctl32.c b/fs/xfs/linux-2.6/xfs_ioctl32.c
index 6c83f7f..2146196 100644
--- a/fs/xfs/linux-2.6/xfs_ioctl32.c
+++ b/fs/xfs/linux-2.6/xfs_ioctl32.c
@@ -164,7 +164,8 @@ xfs_ioctl32_bstat_copyin(
get_user(bstat->bs_extsize, &bstat32->bs_extsize) ||
get_user(bstat->bs_extents, &bstat32->bs_extents) ||
get_user(bstat->bs_gen, &bstat32->bs_gen) ||
- get_user(bstat->bs_projid, &bstat32->bs_projid) ||
+ get_user(bstat->bs_projid_lo, &bstat32->bs_projid_lo) ||
+ get_user(bstat->bs_projid_hi, &bstat32->bs_projid_hi) ||
get_user(bstat->bs_dmevmask, &bstat32->bs_dmevmask) ||
get_user(bstat->bs_dmstate, &bstat32->bs_dmstate) ||
get_user(bstat->bs_aextents, &bstat32->bs_aextents))
@@ -217,7 +218,8 @@ xfs_bulkstat_one_fmt_compat(
put_user(buffer->bs_extsize, &p32->bs_extsize) ||
put_user(buffer->bs_extents, &p32->bs_extents) ||
put_user(buffer->bs_gen, &p32->bs_gen) ||
- put_user(buffer->bs_projid, &p32->bs_projid) ||
+ put_user(buffer->bs_projid_lo, &p32->bs_projid_lo) ||
+ put_user(buffer->bs_projid_hi, &p32->bs_projid_hi) ||
put_user(buffer->bs_dmevmask, &p32->bs_dmevmask) ||
put_user(buffer->bs_dmstate, &p32->bs_dmstate) ||
put_user(buffer->bs_aextents, &p32->bs_aextents))
diff --git a/fs/xfs/linux-2.6/xfs_ioctl32.h b/fs/xfs/linux-2.6/xfs_ioctl32.h
index 1024c4f..7a22385 100644
--- a/fs/xfs/linux-2.6/xfs_ioctl32.h
+++ b/fs/xfs/linux-2.6/xfs_ioctl32.h
@@ -65,8 +65,9 @@ typedef struct compat_xfs_bstat {
__s32 bs_extsize; /* extent size */
__s32 bs_extents; /* number of extents */
__u32 bs_gen; /* generation count */
- __u16 bs_projid; /* project id */
- unsigned char bs_pad[14]; /* pad space, unused */
+ __u16 bs_projid_lo; /* lower part of project id */
+ __u16 bs_projid_hi; /* high part of project id */
+ unsigned char bs_pad[12]; /* pad space, unused */
__u32 bs_dmevmask; /* DMIG event mask */
__u16 bs_dmstate; /* DMIG state info */
__u16 bs_aextents; /* attribute number of extents */
diff --git a/fs/xfs/quota/xfs_qm.c b/fs/xfs/quota/xfs_qm.c
index 9a92407..9a8885e 100644
--- a/fs/xfs/quota/xfs_qm.c
+++ b/fs/xfs/quota/xfs_qm.c
@@ -837,7 +837,7 @@ xfs_qm_dqattach_locked(
xfs_qm_dqattach_one(ip, ip->i_d.di_gid, XFS_DQ_GROUP,
flags & XFS_QMOPT_DQALLOC,
ip->i_udquot, &ip->i_gdquot) :
- xfs_qm_dqattach_one(ip, ip->i_d.di_projid, XFS_DQ_PROJ,
+ xfs_qm_dqattach_one(ip, xfs_get_projid(ip), XFS_DQ_PROJ,
flags & XFS_QMOPT_DQALLOC,
ip->i_udquot, &ip->i_gdquot);
/*
@@ -1248,7 +1248,7 @@ xfs_qm_dqget_noattach(
XFS_QMOPT_DQALLOC|XFS_QMOPT_DOWARN,
&gdqp) :
xfs_qm_dqget(mp, ip,
- ip->i_d.di_projid, XFS_DQ_PROJ,
+ xfs_get_projid(ip), XFS_DQ_PROJ,
XFS_QMOPT_DQALLOC|XFS_QMOPT_DOWARN,
&gdqp);
if (error) {
@@ -2332,9 +2332,9 @@ xfs_qm_vop_dqalloc(
xfs_dqunlock(gq);
}
} else if ((flags & XFS_QMOPT_PQUOTA) && XFS_IS_PQUOTA_ON(mp)) {
- if (ip->i_d.di_projid != prid) {
+ if (xfs_get_projid(ip) != prid) {
xfs_iunlock(ip, lockflags);
- if ((error = xfs_qm_dqget(mp, NULL, (xfs_dqid_t)prid,
+ if ((error = xfs_qm_dqget(mp, NULL, prid,
XFS_DQ_PROJ,
XFS_QMOPT_DQALLOC |
XFS_QMOPT_DOWARN,
@@ -2454,7 +2454,7 @@ xfs_qm_vop_chown_reserve(
}
if (XFS_IS_OQUOTA_ON(ip->i_mount) && gdqp) {
if (XFS_IS_PQUOTA_ON(ip->i_mount) &&
- ip->i_d.di_projid != be32_to_cpu(gdqp->q_core.d_id))
+ xfs_get_projid(ip) != be32_to_cpu(gdqp->q_core.d_id))
prjflags = XFS_QMOPT_ENOSPC;
if (prjflags ||
@@ -2558,7 +2558,7 @@ xfs_qm_vop_create_dqattach(
ip->i_gdquot = gdqp;
ASSERT(XFS_IS_OQUOTA_ON(mp));
ASSERT((XFS_IS_GQUOTA_ON(mp) ?
- ip->i_d.di_gid : ip->i_d.di_projid) ==
+ ip->i_d.di_gid : xfs_get_projid(ip)) ==
be32_to_cpu(gdqp->q_core.d_id));
xfs_trans_mod_dquot(tp, gdqp, XFS_TRANS_DQ_ICOUNT, 1);
}
diff --git a/fs/xfs/quota/xfs_qm_bhv.c b/fs/xfs/quota/xfs_qm_bhv.c
index bea02d7..45b5cb1 100644
--- a/fs/xfs/quota/xfs_qm_bhv.c
+++ b/fs/xfs/quota/xfs_qm_bhv.c
@@ -81,7 +81,7 @@ xfs_qm_statvfs(
xfs_mount_t *mp = ip->i_mount;
xfs_dquot_t *dqp;
- if (!xfs_qm_dqget(mp, NULL, ip->i_d.di_projid, XFS_DQ_PROJ, 0, &dqp)) {
+ if (!xfs_qm_dqget(mp, NULL, xfs_get_projid(ip), XFS_DQ_PROJ, 0, &dqp)) {
xfs_fill_statvfs_from_dquot(statp, &dqp->q_core);
xfs_qm_dqput(dqp);
}
diff --git a/fs/xfs/quota/xfs_qm_syscalls.c b/fs/xfs/quota/xfs_qm_syscalls.c
index 45e5849..a89065b 100644
--- a/fs/xfs/quota/xfs_qm_syscalls.c
+++ b/fs/xfs/quota/xfs_qm_syscalls.c
@@ -1175,7 +1175,7 @@ xfs_qm_internalqcheck_adjust(
}
xfs_qm_internalqcheck_get_dquots(mp,
(xfs_dqid_t) ip->i_d.di_uid,
- (xfs_dqid_t) ip->i_d.di_projid,
+ (xfs_dqid_t) xfs_get_projid(ip),
(xfs_dqid_t) ip->i_d.di_gid,
&ud, &gd);
if (XFS_IS_UQUOTA_ON(mp)) {
diff --git a/fs/xfs/xfs_dinode.h b/fs/xfs/xfs_dinode.h
index e5b153b..dffba9b 100644
--- a/fs/xfs/xfs_dinode.h
+++ b/fs/xfs/xfs_dinode.h
@@ -49,8 +49,9 @@ typedef struct xfs_dinode {
__be32 di_uid; /* owner's user id */
__be32 di_gid; /* owner's group id */
__be32 di_nlink; /* number of links to file */
- __be16 di_projid; /* owner's project id */
- __u8 di_pad[8]; /* unused, zeroed space */
+ __be16 di_projid_lo; /* lower part of owner's project id */
+ __be16 di_projid_hi; /* higher part owner's project id */
+ __u8 di_pad[6]; /* unused, zeroed space */
__be16 di_flushiter; /* incremented on flush */
xfs_timestamp_t di_atime; /* time last accessed */
xfs_timestamp_t di_mtime; /* time last modified */
diff --git a/fs/xfs/xfs_fs.h b/fs/xfs/xfs_fs.h
index 87c2e9d..c03c752 100644
--- a/fs/xfs/xfs_fs.h
+++ b/fs/xfs/xfs_fs.h
@@ -293,9 +293,10 @@ typedef struct xfs_bstat {
__s32 bs_extsize; /* extent size */
__s32 bs_extents; /* number of extents */
__u32 bs_gen; /* generation count */
- __u16 bs_projid; /* project id */
+ __u16 bs_projid_lo; /* lower part of project id */
__u16 bs_forkoff; /* inode fork offset in bytes */
- unsigned char bs_pad[12]; /* pad space, unused */
+ __u16 bs_projid_hi; /* higher part of project id */
+ unsigned char bs_pad[10]; /* pad space, unused */
__u32 bs_dmevmask; /* DMIG event mask */
__u16 bs_dmstate; /* DMIG state info */
__u16 bs_aextents; /* attribute number of extents */
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index 34798f3..2ab5959 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -660,7 +660,8 @@ xfs_dinode_from_disk(
to->di_uid = be32_to_cpu(from->di_uid);
to->di_gid = be32_to_cpu(from->di_gid);
to->di_nlink = be32_to_cpu(from->di_nlink);
- to->di_projid = be16_to_cpu(from->di_projid);
+ to->di_projid_lo = be16_to_cpu(from->di_projid_lo);
+ to->di_projid_hi = be16_to_cpu(from->di_projid_hi);
memcpy(to->di_pad, from->di_pad, sizeof(to->di_pad));
to->di_flushiter = be16_to_cpu(from->di_flushiter);
to->di_atime.t_sec = be32_to_cpu(from->di_atime.t_sec);
@@ -695,7 +696,8 @@ xfs_dinode_to_disk(
to->di_uid = cpu_to_be32(from->di_uid);
to->di_gid = cpu_to_be32(from->di_gid);
to->di_nlink = cpu_to_be32(from->di_nlink);
- to->di_projid = cpu_to_be16(from->di_projid);
+ to->di_projid_lo = cpu_to_be16(from->di_projid_lo);
+ to->di_projid_hi = cpu_to_be16(from->di_projid_hi);
memcpy(to->di_pad, from->di_pad, sizeof(to->di_pad));
to->di_flushiter = cpu_to_be16(from->di_flushiter);
to->di_atime.t_sec = cpu_to_be32(from->di_atime.t_sec);
@@ -874,7 +876,7 @@ xfs_iread(
if (ip->i_d.di_version == 1) {
ip->i_d.di_nlink = ip->i_d.di_onlink;
ip->i_d.di_onlink = 0;
- ip->i_d.di_projid = 0;
+ xfs_set_projid(ip, 0);
}
ip->i_delayed_blks = 0;
@@ -983,7 +985,7 @@ xfs_ialloc(
xfs_nlink_t nlink,
xfs_dev_t rdev,
cred_t *cr,
- xfs_prid_t prid,
+ prid_t prid,
int okalloc,
xfs_buf_t **ialloc_context,
boolean_t *call_again,
@@ -1027,7 +1029,7 @@ xfs_ialloc(
ASSERT(ip->i_d.di_nlink == nlink);
ip->i_d.di_uid = current_fsuid();
ip->i_d.di_gid = current_fsgid();
- ip->i_d.di_projid = prid;
+ xfs_set_projid(ip, prid);
memset(&(ip->i_d.di_pad[0]), 0, sizeof(ip->i_d.di_pad));
/*
@@ -3008,7 +3010,7 @@ xfs_iflush_int(
memset(&(ip->i_d.di_pad[0]), 0, sizeof(ip->i_d.di_pad));
memset(&(dip->di_pad[0]), 0,
sizeof(dip->di_pad));
- ASSERT(ip->i_d.di_projid == 0);
+ ASSERT(xfs_get_projid(ip) == 0);
}
}
diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
index 0898c54..5bbb100 100644
--- a/fs/xfs/xfs_inode.h
+++ b/fs/xfs/xfs_inode.h
@@ -134,8 +134,9 @@ typedef struct xfs_icdinode {
__uint32_t di_uid; /* owner's user id */
__uint32_t di_gid; /* owner's group id */
__uint32_t di_nlink; /* number of links to file */
- __uint16_t di_projid; /* owner's project id */
- __uint8_t di_pad[8]; /* unused, zeroed space */
+ __uint16_t di_projid_lo; /* lower part of owner's project id */
+ __uint16_t di_projid_hi; /* higher part of owner's project id */
+ __uint8_t di_pad[6]; /* unused, zeroed space */
__uint16_t di_flushiter; /* incremented on flush */
xfs_ictimestamp_t di_atime; /* time last accessed */
xfs_ictimestamp_t di_mtime; /* time last modified */
@@ -335,6 +336,23 @@ xfs_iflags_test_and_clear(xfs_inode_t *ip, unsigned short flags)
}
/*
+ * Project quota id helpers
+ */
+static inline prid_t
+xfs_get_projid(xfs_inode_t *ip)
+{
+ return (prid_t)(ip->i_d.di_projid_hi) << 16 | ip->i_d.di_projid_lo;
+}
+
+static inline void
+xfs_set_projid(xfs_inode_t *ip,
+ prid_t projid)
+{
+ ip->i_d.di_projid_hi = (__uint16_t) (projid >> 16);
+ ip->i_d.di_projid_lo = (__uint16_t) (projid & 0xffff);
+}
+
+/*
* Manage the i_flush queue embedded in the inode. This completion
* queue synchronizes processes attempting to flush the in-core
* inode back to disk.
@@ -456,7 +474,7 @@ void xfs_inode_free(struct xfs_inode *ip);
* xfs_inode.c prototypes.
*/
int xfs_ialloc(struct xfs_trans *, xfs_inode_t *, mode_t,
- xfs_nlink_t, xfs_dev_t, cred_t *, xfs_prid_t,
+ xfs_nlink_t, xfs_dev_t, cred_t *, prid_t,
int, struct xfs_buf **, boolean_t *, xfs_inode_t **);
uint xfs_ip2xflags(struct xfs_inode *);
diff --git a/fs/xfs/xfs_itable.c b/fs/xfs/xfs_itable.c
index 7e3626e..dc1882a 100644
--- a/fs/xfs/xfs_itable.c
+++ b/fs/xfs/xfs_itable.c
@@ -92,7 +92,8 @@ xfs_bulkstat_one_int(
* further change.
*/
buf->bs_nlink = dic->di_nlink;
- buf->bs_projid = dic->di_projid;
+ buf->bs_projid_lo = dic->di_projid_lo;
+ buf->bs_projid_hi = dic->di_projid_hi;
buf->bs_ino = ino;
buf->bs_mode = dic->di_mode;
buf->bs_uid = dic->di_uid;
diff --git a/fs/xfs/xfs_rename.c b/fs/xfs/xfs_rename.c
index 8fca957..494b5cd 100644
--- a/fs/xfs/xfs_rename.c
+++ b/fs/xfs/xfs_rename.c
@@ -183,7 +183,7 @@ xfs_rename(
* tree quota mechanism would be circumvented.
*/
if (unlikely((target_dp->i_d.di_flags & XFS_DIFLAG_PROJINHERIT) &&
- (target_dp->i_d.di_projid != src_ip->i_d.di_projid))) {
+ (xfs_get_projid(target_dp) != xfs_get_projid(src_ip)))) {
error = XFS_ERROR(EXDEV);
goto error_return;
}
diff --git a/fs/xfs/xfs_sb.h b/fs/xfs/xfs_sb.h
index 1b017c6..f7674c4 100644
--- a/fs/xfs/xfs_sb.h
+++ b/fs/xfs/xfs_sb.h
@@ -80,10 +80,12 @@ struct xfs_mount;
#define XFS_SB_VERSION2_RESERVED4BIT 0x00000004
#define XFS_SB_VERSION2_ATTR2BIT 0x00000008 /* Inline attr rework */
#define XFS_SB_VERSION2_PARENTBIT 0x00000010 /* parent pointers */
+#define XFS_SB_VERSION2_PROJID32BIT 0x00000020 /* 32 bit project id */
#define XFS_SB_VERSION2_OKREALFBITS \
(XFS_SB_VERSION2_LAZYSBCOUNTBIT | \
- XFS_SB_VERSION2_ATTR2BIT)
+ XFS_SB_VERSION2_ATTR2BIT | \
+ XFS_SB_VERSION2_PROJID32BIT)
#define XFS_SB_VERSION2_OKSASHFBITS \
(0)
#define XFS_SB_VERSION2_OKREALBITS \
@@ -495,6 +497,19 @@ static inline void xfs_sb_version_removeattr2(xfs_sb_t *sbp)
sbp->sb_versionnum &= ~XFS_SB_VERSION_MOREBITSBIT;
}
+static inline int xfs_sb_version_hasprojid32bit(xfs_sb_t *sbp)
+{
+ return xfs_sb_version_hasmorebits(sbp) &&
+ (sbp->sb_features2 & XFS_SB_VERSION2_PROJID32BIT);
+}
+
+static inline void xfs_sb_version_addprojid32bit(xfs_sb_t *sbp)
+{
+ sbp->sb_versionnum |= XFS_SB_VERSION_MOREBITSBIT;
+ sbp->sb_features2 |= XFS_SB_VERSION2_PROJID32BIT;
+ sbp->sb_bad_features2 |= XFS_SB_VERSION2_PROJID32BIT;
+}
+
/*
* end of superblock version macros
*/
diff --git a/fs/xfs/xfs_types.h b/fs/xfs/xfs_types.h
index 3207752..26d1867 100644
--- a/fs/xfs/xfs_types.h
+++ b/fs/xfs/xfs_types.h
@@ -73,8 +73,6 @@ typedef __int32_t xfs_tid_t; /* transaction identifier */
typedef __uint32_t xfs_dablk_t; /* dir/attr block number (in file) */
typedef __uint32_t xfs_dahash_t; /* dir/attr hash value */
-typedef __uint16_t xfs_prid_t; /* prid_t truncated to 16bits in XFS */
-
typedef __uint32_t xlog_tid_t; /* transaction ID type */
/*
diff --git a/fs/xfs/xfs_utils.c b/fs/xfs/xfs_utils.c
index b7d5769..962efbd 100644
--- a/fs/xfs/xfs_utils.c
+++ b/fs/xfs/xfs_utils.c
@@ -320,3 +320,24 @@ xfs_bumplink(
xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
return 0;
}
+
+/*
+ * Switches on the PROJID32BIT superblock bit
+ * (implies also FEATURES2).
+ */
+
+int
+xfs_addprojid32bit(
+ xfs_trans_t *tp,
+ xfs_inode_t *ip)
+{
+ spin_lock(&ip->i_mount->m_sb_lock);
+ if (!xfs_sb_version_hasprojid32bit(&ip->i_mount->m_sb)) {
+ xfs_sb_version_addprojid32bit(&ip->i_mount->m_sb);
+ spin_unlock(&ip->i_mount->m_sb_lock);
+ xfs_mod_sb(tp,
+ XFS_SB_VERSIONNUM | XFS_SB_FEATURES2);
+ } else
+ spin_unlock(&ip->i_mount->m_sb_lock);
+ return 0;
+}
diff --git a/fs/xfs/xfs_utils.h b/fs/xfs/xfs_utils.h
index f55b967..5c40acd 100644
--- a/fs/xfs/xfs_utils.h
+++ b/fs/xfs/xfs_utils.h
@@ -24,5 +24,6 @@ extern int xfs_dir_ialloc(xfs_trans_t **, xfs_inode_t *, mode_t, xfs_nlink_t,
extern int xfs_droplink(xfs_trans_t *, xfs_inode_t *);
extern int xfs_bumplink(xfs_trans_t *, xfs_inode_t *);
extern void xfs_bump_ino_vers2(xfs_trans_t *, xfs_inode_t *);
+extern int xfs_addprojid32bit(xfs_trans_t *, xfs_inode_t *);
#endif /* __XFS_UTILS_H__ */
diff --git a/fs/xfs/xfs_vnodeops.c b/fs/xfs/xfs_vnodeops.c
index 4c7c7bf..72319a9 100644
--- a/fs/xfs/xfs_vnodeops.c
+++ b/fs/xfs/xfs_vnodeops.c
@@ -114,7 +114,7 @@ xfs_setattr(
*/
ASSERT(udqp == NULL);
ASSERT(gdqp == NULL);
- code = xfs_qm_vop_dqalloc(ip, uid, gid, ip->i_d.di_projid,
+ code = xfs_qm_vop_dqalloc(ip, uid, gid, xfs_get_projid(ip),
qflags, &udqp, &gdqp);
if (code)
return code;
@@ -1266,7 +1266,7 @@ xfs_create(
boolean_t unlock_dp_on_error = B_FALSE;
uint cancel_flags;
int committed;
- xfs_prid_t prid;
+ prid_t prid;
struct xfs_dquot *udqp = NULL;
struct xfs_dquot *gdqp = NULL;
uint resblks;
@@ -1279,7 +1279,7 @@ xfs_create(
return XFS_ERROR(EIO);
if (dp->i_d.di_flags & XFS_DIFLAG_PROJINHERIT)
- prid = dp->i_d.di_projid;
+ prid = xfs_get_projid(dp);
else
prid = dfltprid;
@@ -1880,7 +1880,7 @@ xfs_link(
* the tree quota mechanism could be circumvented.
*/
if (unlikely((tdp->i_d.di_flags & XFS_DIFLAG_PROJINHERIT) &&
- (tdp->i_d.di_projid != sip->i_d.di_projid))) {
+ (xfs_get_projid(tdp) != xfs_get_projid(sip)))) {
error = XFS_ERROR(EXDEV);
goto error_return;
}
@@ -1955,7 +1955,7 @@ xfs_symlink(
int byte_cnt;
int n;
xfs_buf_t *bp;
- xfs_prid_t prid;
+ prid_t prid;
struct xfs_dquot *udqp, *gdqp;
uint resblks;
@@ -1978,9 +1978,9 @@ xfs_symlink(
udqp = gdqp = NULL;
if (dp->i_d.di_flags & XFS_DIFLAG_PROJINHERIT)
- prid = dp->i_d.di_projid;
+ prid = xfs_get_projid(dp);
else
- prid = (xfs_prid_t)dfltprid;
+ prid = dfltprid;
/*
* Make sure that we have allocated dquot(s) on disk.
--
1.7.2.3
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH VER 4] xfsprogs: projid32bit handling 2010-09-22 17:42 [PATCH VER 4] Extend project quotas to support 32bit project identificators Arkadiusz Miśkiewicz @ 2010-09-22 17:45 ` Arkadiusz Miśkiewicz 2010-09-22 17:45 ` [PATCH VER 4] xfstests: Quota project id setting overflow Arkadiusz Miśkiewicz ` (2 more replies) 2010-09-22 18:42 ` [PATCH VER 4] Extend project quotas to support 32bit project identificators Christoph Hellwig 2010-09-23 22:51 ` Alex Elder 2 siblings, 3 replies; 10+ messages in thread From: Arkadiusz Miśkiewicz @ 2010-09-22 17:45 UTC (permalink / raw) To: xfs Add projid32bit handling to userspace. mkfs is able to enable this feature for new filesystems. xfs_db knows what projid_lo/hi are. Signed-off-by: Arkadiusz Miśkiewicz <arekm@maven.pl> --- What has changed? - sb_bad_features2 is also updated - bstat_get_projid helper added db/check.c | 2 +- db/inode.c | 6 ++++-- db/sb.c | 6 ++++++ include/xfs_dinode.h | 5 +++-- include/xfs_fs.h | 14 ++++++++++++-- include/xfs_inode.h | 23 ++++++++++++++++++++--- include/xfs_sb.h | 17 ++++++++++++++++- include/xfs_types.h | 2 -- libxfs/util.c | 13 ++++++++----- libxfs/xfs_ialloc.c | 3 ++- libxfs/xfs_inode.c | 6 ++++-- logprint/log_print_all.c | 6 ++++-- man/man3/xfsctl.3 | 6 ++++-- man/man8/mkfs.xfs.8 | 7 +++++++ man/man8/xfs_db.8 | 6 ++++-- mkfs/xfs_mkfs.c | 18 +++++++++++++++--- mkfs/xfs_mkfs.h | 3 ++- quota/quot.c | 2 +- repair/README | 2 +- 19 files changed, 114 insertions(+), 33 deletions(-) diff --git a/db/check.c b/db/check.c index 4f8a62a..a8939a4 100644 --- a/db/check.c +++ b/db/check.c @@ -2840,7 +2840,7 @@ process_inode( break; } if (ic) { - dqprid = idic.di_projid; /* dquot ID is u32 */ + dqprid = xfs_get_projid(idic); /* dquot ID is u32 */ quota_add(&dqprid, &idic.di_gid, &idic.di_uid, 0, bc, ic, rc); } diff --git a/db/inode.c b/db/inode.c index 4aa4e1a..6f8592a 100644 --- a/db/inode.c +++ b/db/inode.c @@ -81,8 +81,10 @@ const field_t inode_core_flds[] = { FLD_COUNT, TYP_NONE }, { "onlink", FLDT_UINT16D, OI(COFF(onlink)), inode_core_onlink_count, FLD_COUNT, TYP_NONE }, - { "projid", FLDT_UINT16D, OI(COFF(projid)), inode_core_projid_count, - FLD_COUNT, TYP_NONE }, + { "projid_lo", FLDT_UINT16D, OI(COFF(projid_lo)), + inode_core_projid_count, FLD_COUNT, TYP_NONE }, + { "projid_hi", FLDT_UINT16D, OI(COFF(projid_hi)), + inode_core_projid_count, FLD_COUNT, TYP_NONE }, { "uid", FLDT_UINT32D, OI(COFF(uid)), C1, 0, TYP_NONE }, { "gid", FLDT_UINT32D, OI(COFF(gid)), C1, 0, TYP_NONE }, { "flushiter", FLDT_UINT16D, OI(COFF(flushiter)), C1, 0, TYP_NONE }, diff --git a/db/sb.c b/db/sb.c index 961a939..21f38c5 100644 --- a/db/sb.c +++ b/db/sb.c @@ -620,6 +620,8 @@ version_string( strcat(s, ",ATTR2"); if (xfs_sb_version_haslazysbcount(sbp)) strcat(s, ",LAZYSBCOUNT"); + if (xfs_sb_version_hasprojid32bit(sbp)) + strcat(s, ",PROJID32BIT"); return s; } @@ -696,6 +698,10 @@ version_f( xfs_sb_version_addattr2(&mp->m_sb); version = mp->m_sb.sb_versionnum; features = mp->m_sb.sb_features2; + } else if (!strcasecmp(argv[1], "projid32bit")) { + xfs_sb_version_addprojid32bit(&mp->m_sb); + version = mp->m_sb.sb_versionnum; + features = mp->m_sb.sb_features2; } else { dbprintf(_("%s: invalid version change command \"%s\"\n"), progname, argv[1]); diff --git a/include/xfs_dinode.h b/include/xfs_dinode.h index d7cf392..f28c088 100644 --- a/include/xfs_dinode.h +++ b/include/xfs_dinode.h @@ -52,8 +52,9 @@ typedef struct xfs_dinode_core { __be32 di_uid; /* owner's user id */ __be32 di_gid; /* owner's group id */ __be32 di_nlink; /* number of links to file */ - __be16 di_projid; /* owner's project id */ - __u8 di_pad[8]; /* unused, zeroed space */ + __be16 di_projid_lo; /* lower part of owner's project id */ + __be16 di_projid_hi; /* higher part owner's project id */ + __u8 di_pad[6]; /* unused, zeroed space */ __be16 di_flushiter; /* incremented on flush */ xfs_timestamp_t di_atime; /* time last accessed */ xfs_timestamp_t di_mtime; /* time last modified */ diff --git a/include/xfs_fs.h b/include/xfs_fs.h index 74e7274..e73c705 100644 --- a/include/xfs_fs.h +++ b/include/xfs_fs.h @@ -299,9 +299,10 @@ typedef struct xfs_bstat { __s32 bs_extsize; /* extent size */ __s32 bs_extents; /* number of extents */ __u32 bs_gen; /* generation count */ - __u16 bs_projid; /* project id */ + __u16 bs_projid_lo; /* lower part of project id */ __u16 bs_forkoff; /* inode fork offset in bytes */ - unsigned char bs_pad[12]; /* pad space, unused */ + __u16 bs_projid_hi; /* higher part of project id */ + unsigned char bs_pad[10]; /* pad space, unused */ __u32 bs_dmevmask; /* DMIG event mask */ __u16 bs_dmstate; /* DMIG state info */ __u16 bs_aextents; /* attribute number of extents */ @@ -506,4 +507,13 @@ typedef struct xfs_handle { #define BBTOB(bbs) ((bbs) << BBSHIFT) #endif +/* + * Project quota id helpers (userspace). + */ +static inline __uint32_t +bstat_get_projid(xfs_bstat_t *bs) +{ + return ((__uint32_t)bs->bs_projid_hi << 16) | bs->bs_projid_lo; +} + #endif /* __XFS_FS_H__ */ diff --git a/include/xfs_inode.h b/include/xfs_inode.h index b19b467..49d0150 100644 --- a/include/xfs_inode.h +++ b/include/xfs_inode.h @@ -124,8 +124,9 @@ typedef struct xfs_icdinode { __uint32_t di_uid; /* owner's user id */ __uint32_t di_gid; /* owner's group id */ __uint32_t di_nlink; /* number of links to file */ - __uint16_t di_projid; /* owner's project id */ - __uint8_t di_pad[8]; /* unused, zeroed space */ + __uint16_t di_projid_lo; /* lower part of owner's project id */ + __uint16_t di_projid_hi; /* higher part of owner's project id */ + __uint8_t di_pad[6]; /* unused, zeroed space */ __uint16_t di_flushiter; /* incremented on flush */ xfs_ictimestamp_t di_atime; /* time last accessed */ xfs_ictimestamp_t di_mtime; /* time last modified */ @@ -204,6 +205,22 @@ typedef struct xfs_icdinode { ((ip)->i_d.di_anextents = (n))) +/* + * Project quota id helpers (userspace) + */ +static inline __uint32_t +xfs_get_projid(xfs_icdinode_t i_d) +{ + return ((__uint32_t)i_d.di_projid_hi << 16) | i_d.di_projid_lo; +} + +static inline void +xfs_set_projid(xfs_icdinode_t *i_d, + __uint32_t projid) +{ + i_d->di_projid_hi = (__uint16_t) (projid >> 16); + i_d->di_projid_lo = (__uint16_t) (projid & 0xffff); +} #ifdef __KERNEL__ @@ -510,7 +527,7 @@ int xfs_finish_reclaim_all(struct xfs_mount *, int); int xfs_iread(struct xfs_mount *, struct xfs_trans *, xfs_ino_t, xfs_inode_t **, xfs_daddr_t, uint); int xfs_ialloc(struct xfs_trans *, xfs_inode_t *, mode_t, - xfs_nlink_t, xfs_dev_t, struct cred *, xfs_prid_t, + xfs_nlink_t, xfs_dev_t, struct cred *, prid_t, int, struct xfs_buf **, boolean_t *, xfs_inode_t **); uint xfs_ip2xflags(struct xfs_inode *); diff --git a/include/xfs_sb.h b/include/xfs_sb.h index 1e86489..0a14773 100644 --- a/include/xfs_sb.h +++ b/include/xfs_sb.h @@ -80,10 +80,12 @@ struct xfs_mount; #define XFS_SB_VERSION2_RESERVED4BIT 0x00000004 #define XFS_SB_VERSION2_ATTR2BIT 0x00000008 /* Inline attr rework */ #define XFS_SB_VERSION2_PARENTBIT 0x00000010 /* parent pointers */ +#define XFS_SB_VERSION2_PROJID32BIT 0x00000020 /* 32 bit project id */ #define XFS_SB_VERSION2_OKREALFBITS \ (XFS_SB_VERSION2_LAZYSBCOUNTBIT | \ - XFS_SB_VERSION2_ATTR2BIT) + XFS_SB_VERSION2_ATTR2BIT | \ + XFS_SB_VERSION2_PROJID32BIT) #define XFS_SB_VERSION2_OKSASHFBITS \ (0) #define XFS_SB_VERSION2_OKREALBITS \ @@ -489,6 +491,19 @@ static inline void xfs_sb_version_removeattr2(xfs_sb_t *sbp) sbp->sb_versionnum &= ~XFS_SB_VERSION_MOREBITSBIT; } +static inline int xfs_sb_version_hasprojid32bit(xfs_sb_t *sbp) +{ + return xfs_sb_version_hasmorebits(sbp) && + (sbp->sb_features2 & XFS_SB_VERSION2_PROJID32BIT); +} + +static inline void xfs_sb_version_addprojid32bit(xfs_sb_t *sbp) +{ + sbp->sb_versionnum |= XFS_SB_VERSION_MOREBITSBIT; + sbp->sb_features2 |= XFS_SB_VERSION2_PROJID32BIT; + sbp->sb_bad_features2 |= XFS_SB_VERSION2_PROJID32BIT; +} + /* * end of superblock version macros */ diff --git a/include/xfs_types.h b/include/xfs_types.h index 0f51916..228b948 100644 --- a/include/xfs_types.h +++ b/include/xfs_types.h @@ -81,8 +81,6 @@ typedef __int32_t xfs_tid_t; /* transaction identifier */ typedef __uint32_t xfs_dablk_t; /* dir/attr block number (in file) */ typedef __uint32_t xfs_dahash_t; /* dir/attr hash value */ -typedef __uint16_t xfs_prid_t; /* prid_t truncated to 16bits in XFS */ - /* * These types are 64 bits on disk but are either 32 or 64 bits in memory. * Disk based types: diff --git a/libxfs/util.c b/libxfs/util.c index 409fb92..077d2a2 100644 --- a/libxfs/util.c +++ b/libxfs/util.c @@ -134,7 +134,7 @@ libxfs_iread( * made it 32 bits long. If this is an old format inode, * convert it in memory to look like a new one. If it gets * flushed to disk we will convert back before flushing or - * logging it. We zero out the new projid field and the old link + * logging it. We zero out the new projid_lo/hi field and the old link * count field. We'll handle clearing the pad field (the remains * of the old uuid field) when we actually convert the inode to * the new format. We don't change the version number so that we @@ -143,7 +143,7 @@ libxfs_iread( if (ip->i_d.di_version == XFS_DINODE_VERSION_1) { ip->i_d.di_nlink = ip->i_d.di_onlink; ip->i_d.di_onlink = 0; - ip->i_d.di_projid = 0; + xfs_set_projid(&ip->i_d, 0); } ip->i_delayed_blks = 0; @@ -219,7 +219,7 @@ libxfs_ialloc( ASSERT(ip->i_d.di_nlink == nlink); ip->i_d.di_uid = cr->cr_uid; ip->i_d.di_gid = cr->cr_gid; - ip->i_d.di_projid = pip ? 0 : fsx->fsx_projid; + xfs_set_projid(&ip->i_d, pip ? 0 : fsx->fsx_projid); memset(&(ip->i_d.di_pad[0]), 0, sizeof(ip->i_d.di_pad)); /* @@ -231,7 +231,10 @@ libxfs_ialloc( if (xfs_sb_version_hasnlink(&tp->t_mountp->m_sb) && ip->i_d.di_version == XFS_DINODE_VERSION_1) { ip->i_d.di_version = XFS_DINODE_VERSION_2; - /* old link count, projid field, pad field already zeroed */ + /* + * old link count, projid_lo/hi field, pad field + * already zeroed + */ } if (pip && (pip->i_d.di_mode & S_ISGID)) { @@ -446,7 +449,7 @@ libxfs_iflush_int(xfs_inode_t *ip, xfs_buf_t *bp) memset(&(ip->i_d.di_pad[0]), 0, sizeof(ip->i_d.di_pad)); memset(&(dip->di_core.di_pad[0]), 0, sizeof(dip->di_core.di_pad)); - ASSERT(ip->i_d.di_projid == 0); + ASSERT(xfs_get_projid(ip->i_d) == 0); } } diff --git a/libxfs/xfs_ialloc.c b/libxfs/xfs_ialloc.c index 39fdf96..32ae4b0 100644 --- a/libxfs/xfs_ialloc.c +++ b/libxfs/xfs_ialloc.c @@ -46,7 +46,8 @@ xfs_ialloc_log_di( offsetof(xfs_dinode_core_t, di_uid), offsetof(xfs_dinode_core_t, di_gid), offsetof(xfs_dinode_core_t, di_nlink), - offsetof(xfs_dinode_core_t, di_projid), + offsetof(xfs_dinode_core_t, di_projid_lo), + offsetof(xfs_dinode_core_t, di_projid_hi), offsetof(xfs_dinode_core_t, di_pad), offsetof(xfs_dinode_core_t, di_atime), offsetof(xfs_dinode_core_t, di_mtime), diff --git a/libxfs/xfs_inode.c b/libxfs/xfs_inode.c index b0adabc..1c9ea3b 100644 --- a/libxfs/xfs_inode.c +++ b/libxfs/xfs_inode.c @@ -589,7 +589,8 @@ xfs_dinode_from_disk( to->di_uid = be32_to_cpu(from->di_uid); to->di_gid = be32_to_cpu(from->di_gid); to->di_nlink = be32_to_cpu(from->di_nlink); - to->di_projid = be16_to_cpu(from->di_projid); + to->di_projid_lo = be16_to_cpu(from->di_projid_lo); + to->di_projid_hi = be16_to_cpu(from->di_projid_hi); memcpy(to->di_pad, from->di_pad, sizeof(to->di_pad)); to->di_flushiter = be16_to_cpu(from->di_flushiter); to->di_atime.t_sec = be32_to_cpu(from->di_atime.t_sec); @@ -624,7 +625,8 @@ xfs_dinode_to_disk( to->di_uid = cpu_to_be32(from->di_uid); to->di_gid = cpu_to_be32(from->di_gid); to->di_nlink = cpu_to_be32(from->di_nlink); - to->di_projid = cpu_to_be16(from->di_projid); + to->di_projid_lo = cpu_to_be16(from->di_projid_lo); + to->di_projid_hi = cpu_to_be16(from->di_projid_hi); memcpy(to->di_pad, from->di_pad, sizeof(to->di_pad)); to->di_flushiter = cpu_to_be16(from->di_flushiter); to->di_atime.t_sec = cpu_to_be32(from->di_atime.t_sec); diff --git a/logprint/log_print_all.c b/logprint/log_print_all.c index c21e05c..572dac8 100644 --- a/logprint/log_print_all.c +++ b/logprint/log_print_all.c @@ -238,8 +238,10 @@ xlog_recover_print_inode_core( "onlink:%d\n"), (di->di_magic>>8) & 0xff, di->di_magic & 0xff, di->di_mode, di->di_version, di->di_format, di->di_onlink); - printf(_(" uid:%d gid:%d nlink:%d projid:%d\n"), - di->di_uid, di->di_gid, di->di_nlink, (uint)di->di_projid); + printf(_(" uid:%d gid:%d nlink:%d\n"), + di->di_uid, di->di_gid, di->di_nlink); + printf(_(" projid_lo:%d projid_hi:%d\n"), + (uint)di->di_projid_lo, (uint)di->di_projid_hi); printf(_(" atime:%d mtime:%d ctime:%d\n"), di->di_atime.t_sec, di->di_mtime.t_sec, di->di_ctime.t_sec); printf(_(" flushiter:%d\n"), di->di_flushiter); diff --git a/man/man3/xfsctl.3 b/man/man3/xfsctl.3 index 784b3e0..7f3c2e8 100644 --- a/man/man3/xfsctl.3 +++ b/man/man3/xfsctl.3 @@ -564,8 +564,10 @@ The structure has the following elements: (number of extents), .B bs_gen (generation count), -.B bs_projid -(project id), +.B bs_projid_lo +(project id - low word), +.B bs_projid_hi +(project id - high word), .B bs_dmevmask (DMIG event mask), .B bs_dmstate diff --git a/man/man8/mkfs.xfs.8 b/man/man8/mkfs.xfs.8 index fdd38d4..e73fbae 100644 --- a/man/man8/mkfs.xfs.8 +++ b/man/man8/mkfs.xfs.8 @@ -350,6 +350,13 @@ between attribute and extent data. The previous version 1, which has fixed regions for attribute and extent data, is kept for backwards compatibility with kernels older than version 2.6.16. +.TP +.BI projid32bit[= value] +This is used to enable 32bit quota project identificators. The +.I value +is either 0 or 1, with 1 signifying that 32bit projid are to be enabled. +If the +is omitted, 0 is assumed. .RE .TP .BI \-l " log_section_options" diff --git a/man/man8/xfs_db.8 b/man/man8/xfs_db.8 index 629ae58..17ac601 100644 --- a/man/man8/xfs_db.8 +++ b/man/man8/xfs_db.8 @@ -1474,8 +1474,10 @@ number of links to the file in a version 1 inode. .B nlinkv2 number of links to the file in a version 2 inode. .TP -.B projid -owner's project id (version 2 inode only). +.B projid_lo +owner's project id (low word; version 2 inode only). +.B projid_hi +owner's project id (high word; version 2 inode only). .TP .B uid owner's user id. diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c index 2d09e36..cb38469 100644 --- a/mkfs/xfs_mkfs.c +++ b/mkfs/xfs_mkfs.c @@ -106,6 +106,8 @@ char *iopts[] = { "size", #define I_ATTR 5 "attr", +#define I_PROJID32BIT 6 + "projid32bit", NULL }; @@ -829,6 +831,7 @@ main( __uint64_t agsize; xfs_alloc_rec_t *arec; int attrversion; + int projid32bit; struct xfs_btree_block *block; int blflag; int blocklog; @@ -923,6 +926,7 @@ main( textdomain(PACKAGE); attrversion = 2; + projid32bit = 0; blflag = bsflag = slflag = ssflag = lslflag = lssflag = 0; blocklog = blocksize = 0; sectorlog = lsectorlog = XFS_MIN_SECTORSIZE_LOG; @@ -1259,6 +1263,14 @@ main( illegal(value, "i attr"); attrversion = c; break; + case I_PROJID32BIT: + if (!value) + value = "0"; + c = atoi(value); + if (c < 0 || c > 1) + illegal(value, "i projid32bit"); + projid32bit = c; + break; default: unknown('i', value); } @@ -2261,7 +2273,7 @@ an AG size that is one stripe unit smaller, for example %llu.\n"), if (!qflag || Nflag) { printf(_( "meta-data=%-22s isize=%-6d agcount=%lld, agsize=%lld blks\n" - " =%-22s sectsz=%-5u attr=%u\n" + " =%-22s sectsz=%-5u attr=%u projid32bit=%u\n" "data =%-22s bsize=%-6u blocks=%llu, imaxpct=%u\n" " =%-22s sunit=%-6u swidth=%u blks\n" "naming =version %-14u bsize=%-6u ascii-ci=%d\n" @@ -2269,7 +2281,7 @@ an AG size that is one stripe unit smaller, for example %llu.\n"), " =%-22s sectsz=%-5u sunit=%d blks, lazy-count=%d\n" "realtime =%-22s extsz=%-6d blocks=%lld, rtextents=%lld\n"), dfile, isize, (long long)agcount, (long long)agsize, - "", sectorsize, attrversion, + "", sectorsize, attrversion, projid32bit, "", blocksize, (long long)dblocks, imaxpct, "", dsunit, dswidth, dirversion, dirblocksize, nci, @@ -2336,7 +2348,7 @@ an AG size that is one stripe unit smaller, for example %llu.\n"), sbp->sb_logsectsize = 0; } sbp->sb_features2 = XFS_SB_VERSION2_MKFS(lazy_sb_counters, - attrversion == 2, 0); + attrversion == 2, projid32bit == 1, 0); sbp->sb_versionnum = XFS_SB_VERSION_MKFS(iaflag, dsunit != 0, logversion == 2, attrversion == 1, (sectorsize != BBSIZE || diff --git a/mkfs/xfs_mkfs.h b/mkfs/xfs_mkfs.h index 49401d6..f25a7f3 100644 --- a/mkfs/xfs_mkfs.h +++ b/mkfs/xfs_mkfs.h @@ -36,9 +36,10 @@ XFS_DFL_SB_VERSION_BITS | \ 0 ) : XFS_SB_VERSION_1 ) -#define XFS_SB_VERSION2_MKFS(lazycount, attr2, parent) (\ +#define XFS_SB_VERSION2_MKFS(lazycount, attr2, projid32bit, parent) (\ ((lazycount) ? XFS_SB_VERSION2_LAZYSBCOUNTBIT : 0) | \ ((attr2) ? XFS_SB_VERSION2_ATTR2BIT : 0) | \ + ((projid32bit) ? XFS_SB_VERSION2_PROJID32BIT : 0) | \ ((parent) ? XFS_SB_VERSION2_PARENTBIT : 0) | \ 0 ) diff --git a/quota/quot.c b/quota/quot.c index 09d349f..54387ea 100644 --- a/quota/quot.c +++ b/quota/quot.c @@ -102,7 +102,7 @@ quot_bulkstat_add( } for (i = 0; i < 3; i++) { id = (i == 0) ? p->bs_uid : ((i == 1) ? - p->bs_gid : p->bs_projid); + p->bs_gid : bstat_get_projid(p)); hp = &duhash[i][id % DUHASH]; for (dp = *hp; dp; dp = dp->next) if (dp->id == id) diff --git a/repair/README b/repair/README index 69cb0c5..7f168e6 100644 --- a/repair/README +++ b/repair/README @@ -130,7 +130,7 @@ D - 0) rewrite directory leaf block holemap comparison code. it does describe doesn't conflict with reality. D - 0) rewrite setting nlinks handling -- for version 1 - inodes, set both nlinks and onlinks (zero projid + inodes, set both nlinks and onlinks (zero projid_lo/hi and pad) if we have to change anything. For version 2, I think we're ok. -- 1.7.2.3 _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH VER 4] xfstests: Quota project id setting overflow 2010-09-22 17:45 ` [PATCH VER 4] xfsprogs: projid32bit handling Arkadiusz Miśkiewicz @ 2010-09-22 17:45 ` Arkadiusz Miśkiewicz 2010-09-22 18:44 ` [PATCH VER 4] xfsprogs: projid32bit handling Christoph Hellwig 2010-09-23 22:57 ` Alex Elder 2 siblings, 0 replies; 10+ messages in thread From: Arkadiusz Miśkiewicz @ 2010-09-22 17:45 UTC (permalink / raw) To: xfs From: Arkadiusz Miskiewicz <arekm@maven.pl> Test 3 quota project setting id conditions: - set 16bit project quota id -> should succeed - set 32bit project quota id -> should succeed (with projid32bit patch applied; fail otherwise) - over 32bit project quota id -> should always fail Signed-off-by: Arkadiusz Miśkiewicz <arekm@maven.pl> --- What has changed? - tested, fixed and tested again - gpl header added - now as test 244 244 | 123 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 244.out | 2 + group | 1 + 3 files changed, 126 insertions(+), 0 deletions(-) create mode 100644 244 create mode 100644 244.out diff --git a/244 b/244 new file mode 100644 index 0000000..611a43c --- /dev/null +++ b/244 @@ -0,0 +1,123 @@ +#! /bin/bash +# FS QA Test No. 244 +# +# test to verify that proper project quota id is correctly set +# +#----------------------------------------------------------------------- +# Copyright (c) 2010 Arkadiusz Miśkiewicz. All Rights Reserved. +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License as +# published by the Free Software Foundation. +# +# This program is distributed in the hope that it would be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write the Free Software Foundation, +# Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA +# +#----------------------------------------------------------------------- +# +# creator +owner=arekm@maven.pl + +seq=`basename $0` +echo "QA output created by $seq" + +here=`pwd` +tmp=/tmp/$$ +status=1 # failure is the default! +trap "_cleanup; exit \$status" 0 1 2 3 15 + +# get standard environment, filters and checks +. ./common.rc +. ./common.filter +. ./common.quota + +_cleanup() +{ + cd / + umount $SCRATCH_MNT 2>/dev/null + rm -f $tmp.* +} + +# real QA test starts here +_supported_fs xfs + +_require_xfs_quota + +dir=$SCRATCH_MNT/project + +_require_scratch +_scratch_mkfs_xfs >/dev/null 2>&1 + +#if pquota's already in mount options then we dont need to enable +# we can't run with group quotas +if ( echo $MOUNT_OPTIONS | egrep -q g??quota ) +then + _notrun "Can't run with group quotas enabled" +fi +EXTRA_MOUNT_OPTIONS="-o pquota" + +if ! _scratch_mount "$EXTRA_MOUNT_OPTIONS" >$tmp.out 2>&1 +then + cat $tmp.out + _fail "!!! failed to mount" +fi + +src/feature -p $SCRATCH_DEV +[ $? -ne 0 ] && _notrun "Installed kernel does not support project quotas" + +status=0 + +echo "Silence is golden" + +mkdir $dir +touch $dir/below16bit +# below 16bit value +xfs_quota -x -c "project -s -p $dir/below16bit 3422" $SCRATCH_DEV >> $seq.full +projid=$($XFS_IO_PROG -r -c "lsproj" $dir/below16bit) +if [ "projid = 3422" != "$projid" ]; then + echo "FAIL: returned projid value ($projid) doesn't match set one (projid = 3422)" + status=1 +fi + +# 0x20 - projid32bit features2 bit +features2_pre=$(xfs_db -x -r -c 'sb' -c 'print features2' $SCRATCH_DEV | awk ' { print $3 } ') +projid32bit_pre=$((features2_pre & 0x20)) +# over 16bit value +touch $dir/over16bit +if xfs_quota -x -c "project -s -p $dir/over16bit 108545" $SCRATCH_DEV >> $seq.full 2>&1; then + # success? verify if projid matches + projid=$($XFS_IO_PROG -r -c "lsproj" $dir/over16bit) + if [ "projid = 108545" != "$projid" ]; then + echo "FAIL: returned projid value ($projid) doesn't match set one (projid = 108545)" + status=1 + else + # remount to see updated by kernel features2 (otherwise we see old ones) + _scratch_remount + # if it really succeeded then projid32bit feature bit is supposed to be set automaticly + features2_post=$(xfs_db -x -r -c 'sb' -c 'print features2' $SCRATCH_DEV | awk ' { print $3 } ') + projid32bit_post=$((features2_post & 0x20)) + if [ "$projid32bit_post" -eq 0 ]; then + echo "FAIL: setting 32bit projid succeeded but projid32bit features2 bit wasn't automaticly set" + status=1 + fi + fi +else + # didn't succeed but it should if projid32bit feature bit was already set + if [ "$projid32bit_pre" -gt 0 ]; then + echo "FAIL: setting 32bit projid failed but it should succeeded" + status=1 + fi +fi + +# over 32bit value, should fail +touch $dir/over32bit +if xfs_quota -x -c "project -s -p $dir/over32bit 5344967296" $SCRATCH_DEV >> $seq.full 2>&1; then + echo "FAIL: setting over 32bit projid succeeded while it should fail" + status=1 +fi diff --git a/244.out b/244.out new file mode 100644 index 0000000..d075942 --- /dev/null +++ b/244.out @@ -0,0 +1,2 @@ +QA output created by 244 +Silence is golden diff --git a/group b/group index ff16bb3..e7ba59b 100644 --- a/group +++ b/group @@ -356,3 +356,4 @@ deprecated 241 auto 242 auto quick prealloc 243 auto quick prealloc +244 auto quota quick -- 1.7.2.3 _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH VER 4] xfsprogs: projid32bit handling 2010-09-22 17:45 ` [PATCH VER 4] xfsprogs: projid32bit handling Arkadiusz Miśkiewicz 2010-09-22 17:45 ` [PATCH VER 4] xfstests: Quota project id setting overflow Arkadiusz Miśkiewicz @ 2010-09-22 18:44 ` Christoph Hellwig 2010-09-23 22:57 ` Alex Elder 2 siblings, 0 replies; 10+ messages in thread From: Christoph Hellwig @ 2010-09-22 18:44 UTC (permalink / raw) To: Arkadiusz Mi??kiewicz; +Cc: xfs On Wed, Sep 22, 2010 at 07:45:20PM +0200, Arkadiusz Mi??kiewicz wrote: > Add projid32bit handling to userspace. mkfs is able to enable this > feature for new filesystems. xfs_db knows what projid_lo/hi are. Looks good except for the bstat field naming I already mentioned, and the possibly required addition of xfs_admin support to toggling the flag. _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH VER 4] xfsprogs: projid32bit handling 2010-09-22 17:45 ` [PATCH VER 4] xfsprogs: projid32bit handling Arkadiusz Miśkiewicz 2010-09-22 17:45 ` [PATCH VER 4] xfstests: Quota project id setting overflow Arkadiusz Miśkiewicz 2010-09-22 18:44 ` [PATCH VER 4] xfsprogs: projid32bit handling Christoph Hellwig @ 2010-09-23 22:57 ` Alex Elder 2 siblings, 0 replies; 10+ messages in thread From: Alex Elder @ 2010-09-23 22:57 UTC (permalink / raw) To: Arkadiusz Miśkiewicz; +Cc: xfs On Wed, 2010-09-22 at 19:45 +0200, Arkadiusz Miśkiewicz wrote: > Add projid32bit handling to userspace. mkfs is able to enable this > feature for new filesystems. xfs_db knows what projid_lo/hi are. I forgot to mention this before. Now that I see man pages I think I should do so. You should use the term "identifiers" rather than the (non-word) "identificators" throughout. A number of the comments I had on the kernel XFS code apply to the user-space counterpart code here, so I won't offer those comments here. I will just ask that you make the two be consistent with each other where there is matching code. I haven't given this code a very thorough review, just a scan for the most part. I'd like to hear your thoughts on my suggestions from your XFS patch. -Alex > Signed-off-by: Arkadiusz Miśkiewicz <arekm@maven.pl> > --- > > What has changed? > - sb_bad_features2 is also updated > - bstat_get_projid helper added _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH VER 4] Extend project quotas to support 32bit project identificators. 2010-09-22 17:42 [PATCH VER 4] Extend project quotas to support 32bit project identificators Arkadiusz Miśkiewicz 2010-09-22 17:45 ` [PATCH VER 4] xfsprogs: projid32bit handling Arkadiusz Miśkiewicz @ 2010-09-22 18:42 ` Christoph Hellwig 2010-09-22 19:01 ` Arkadiusz Miskiewicz 2010-09-23 22:51 ` Alex Elder 2 siblings, 1 reply; 10+ messages in thread From: Christoph Hellwig @ 2010-09-22 18:42 UTC (permalink / raw) To: Arkadiusz Mi??kiewicz; +Cc: xfs On Wed, Sep 22, 2010 at 07:42:23PM +0200, Arkadiusz Mi??kiewicz wrote: > if (mask & FSX_PROJID) { > + /* > + * Switch on the PROJID32BIT superblock bit when needed > + * (implies also FEATURES2) > + */ > + if (!xfs_sb_version_hasprojid32bit(&ip->i_mount->m_sb) && > + fa->fsx_projid > (__uint16_t)-1) > + xfs_addprojid32bit(tp, ip); Didn't we agree that we want to enable this feature explicitly via xfs_admin (or mkfs.xfs)? > diff --git a/fs/xfs/xfs_fs.h b/fs/xfs/xfs_fs.h > index 87c2e9d..c03c752 100644 > --- a/fs/xfs/xfs_fs.h > +++ b/fs/xfs/xfs_fs.h > @@ -293,9 +293,10 @@ typedef struct xfs_bstat { > __s32 bs_extsize; /* extent size */ > __s32 bs_extents; /* number of extents */ > __u32 bs_gen; /* generation count */ > - __u16 bs_projid; /* project id */ > + __u16 bs_projid_lo; /* lower part of project id */ > __u16 bs_forkoff; /* inode fork offset in bytes */ > - unsigned char bs_pad[12]; /* pad space, unused */ > + __u16 bs_projid_hi; /* higher part of project id */ > + unsigned char bs_pad[10]; /* pad space, unused */ Unlike in the inode we can't just rename the lo field here - that would break the compilation of existing applications. > /* > + * Project quota id helpers > + */ Maybe add a little comment here that the split is because the projid historically was just 16 bits on disk? > +static inline prid_t > +xfs_get_projid(xfs_inode_t *ip) Please always use struct xfs_foo instead of xfs_foo_t for new code. > return 0; > } > + > +/* > + * Switches on the PROJID32BIT superblock bit > + * (implies also FEATURES2). > + */ > + > +int No need for a space after the comment. > +{ > + spin_lock(&ip->i_mount->m_sb_lock); > + if (!xfs_sb_version_hasprojid32bit(&ip->i_mount->m_sb)) { Wedon't need the lock around the check, it's enough if it's inside the conditional. > + xfs_sb_version_addprojid32bit(&ip->i_mount->m_sb); > + spin_unlock(&ip->i_mount->m_sb_lock); > + xfs_mod_sb(tp, > + XFS_SB_VERSIONNUM | XFS_SB_FEATURES2); Weird formatting? Otherwise this looks good to me. _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH VER 4] Extend project quotas to support 32bit project identificators. 2010-09-22 18:42 ` [PATCH VER 4] Extend project quotas to support 32bit project identificators Christoph Hellwig @ 2010-09-22 19:01 ` Arkadiusz Miskiewicz 2010-09-24 0:37 ` Dave Chinner 0 siblings, 1 reply; 10+ messages in thread From: Arkadiusz Miskiewicz @ 2010-09-22 19:01 UTC (permalink / raw) To: Christoph Hellwig; +Cc: xfs On Wednesday 22 of September 2010, Christoph Hellwig wrote: > On Wed, Sep 22, 2010 at 07:42:23PM +0200, Arkadiusz Mi??kiewicz wrote: > > if (mask & FSX_PROJID) { > > > > + /* > > + * Switch on the PROJID32BIT superblock bit when needed > > + * (implies also FEATURES2) > > + */ > > + if (!xfs_sb_version_hasprojid32bit(&ip->i_mount->m_sb) && > > + fa->fsx_projid > (__uint16_t)-1) > > + xfs_addprojid32bit(tp, ip); > > Didn't we agree that we want to enable this feature explicitly via > xfs_admin (or mkfs.xfs)? Actually there was no agreement on this. Some think that it's good to do that automaticly (so user doesn't have to do anything and has everything working) and others think that it should be turned on explictly by xfs_admin/mkfs.xfs. For me both methods are fine, both have some advantages and disadvantages. (There was an agreement that separate projid (from group) quota should be turned on manually but was no such agreement on projid32bit) > > - __u16 bs_projid; /* project id */ > > + __u16 bs_projid_lo; /* lower part of project id */ > > > > __u16 bs_forkoff; /* inode fork offset in bytes */ > > > > - unsigned char bs_pad[12]; /* pad space, unused */ > > + __u16 bs_projid_hi; /* higher part of project id */ > > + unsigned char bs_pad[10]; /* pad space, unused */ > > Unlike in the inode we can't just rename the lo field here - that would > break the compilation of existing applications. Ok but maybe breaking these is good? So these can be extended to support 32bit projid. Otherwise these will get crap if 32bit projid is enabled (actually already built binaries will still get crap for > 16bit values with projid32bit patch). > > +{ > > + spin_lock(&ip->i_mount->m_sb_lock); > > + if (!xfs_sb_version_hasprojid32bit(&ip->i_mount->m_sb)) { > > Wedon't need the lock around the check, it's enough if it's inside the > conditional. Hm, xfs_bump_ino_vers2 does that exactly that way (for addnlink) thus I thought there is a reason for that. > > + xfs_sb_version_addprojid32bit(&ip->i_mount->m_sb); > > + spin_unlock(&ip->i_mount->m_sb_lock); > > + xfs_mod_sb(tp, > > + XFS_SB_VERSIONNUM | XFS_SB_FEATURES2); -- Arkadiusz Miśkiewicz PLD/Linux Team arekm / maven.pl http://ftp.pld-linux.org/ _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH VER 4] Extend project quotas to support 32bit project identificators. 2010-09-22 19:01 ` Arkadiusz Miskiewicz @ 2010-09-24 0:37 ` Dave Chinner 0 siblings, 0 replies; 10+ messages in thread From: Dave Chinner @ 2010-09-24 0:37 UTC (permalink / raw) To: Arkadiusz Miskiewicz; +Cc: Christoph Hellwig, xfs On Wed, Sep 22, 2010 at 09:01:13PM +0200, Arkadiusz Miskiewicz wrote: > On Wednesday 22 of September 2010, Christoph Hellwig wrote: > > On Wed, Sep 22, 2010 at 07:42:23PM +0200, Arkadiusz Mi??kiewicz wrote: > > > if (mask & FSX_PROJID) { > > > > > > + /* > > > + * Switch on the PROJID32BIT superblock bit when needed > > > + * (implies also FEATURES2) > > > + */ > > > + if (!xfs_sb_version_hasprojid32bit(&ip->i_mount->m_sb) && > > > + fa->fsx_projid > (__uint16_t)-1) > > > + xfs_addprojid32bit(tp, ip); > > > > Didn't we agree that we want to enable this feature explicitly via > > xfs_admin (or mkfs.xfs)? > > Actually there was no agreement on this. Some think that it's good to do that > automaticly (so user doesn't have to do anything and has everything working) > and others think that it should be turned on explictly by xfs_admin/mkfs.xfs. > > For me both methods are fine, both have some advantages and disadvantages. > > (There was an agreement that separate projid (from group) quota should be > turned on manually but was no such agreement on projid32bit) I think I eventually agreed with Christoph that xfs_admin/mkfs was the way to go, simply from the principle of least surprise. i.e. upgrading the kernel shouldn't make a projid set that previously failed now succeed and prevent a kernel downgrade that is only discovered during downgrade... > > > - __u16 bs_projid; /* project id */ > > > + __u16 bs_projid_lo; /* lower part of project id */ > > > > > > __u16 bs_forkoff; /* inode fork offset in bytes */ > > > > > > - unsigned char bs_pad[12]; /* pad space, unused */ > > > + __u16 bs_projid_hi; /* higher part of project id */ > > > + unsigned char bs_pad[10]; /* pad space, unused */ > > > > Unlike in the inode we can't just rename the lo field here - that would > > break the compilation of existing applications. > > Ok but maybe breaking these is good? No, there is never a good reason for breaking applications like this. > So these can be extended to support 32bit > projid. Otherwise these will get crap if 32bit projid is enabled (actually > already built binaries will still get crap for > 16bit values with projid32bit > patch). At which point a userspace upgrade is required to match the kernel upgrade. Another reason so only allowing the feature to be switched on via xfs-admin/mkfs - it means that userspace XFS utilities have already been upgraded to support 32 bit projids before it can be switched on. Cheers, Dave. -- Dave Chinner david@fromorbit.com _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH VER 4] Extend project quotas to support 32bit project identificators. 2010-09-22 17:42 [PATCH VER 4] Extend project quotas to support 32bit project identificators Arkadiusz Miśkiewicz 2010-09-22 17:45 ` [PATCH VER 4] xfsprogs: projid32bit handling Arkadiusz Miśkiewicz 2010-09-22 18:42 ` [PATCH VER 4] Extend project quotas to support 32bit project identificators Christoph Hellwig @ 2010-09-23 22:51 ` Alex Elder 2010-09-24 0:55 ` Dave Chinner 2 siblings, 1 reply; 10+ messages in thread From: Alex Elder @ 2010-09-23 22:51 UTC (permalink / raw) To: Arkadiusz Miśkiewicz; +Cc: xfs On Wed, 2010-09-22 at 19:42 +0200, Arkadiusz Miśkiewicz wrote: > This patch adds support for 32bit project quota identificators. > > On disk format is backward compatible with 16bit projid numbers. projid > on disk is now keept in two 16bit values - di_projid_lo (which holds the > same position as old 16bit projid value) and new di_projid_hi (takes > existing padding) and convertes from/to 32bit value on the fly. > > PROJID32BIT feature2 flag is set automaticly when trying to use 32bit > quota project identificator. > > Signed-off-by: Arkadiusz Miśkiewicz <arekm@maven.pl> > --- Sorry it took me so long to review this. I have some feedback. I think what you've done looks generally good. The only issues are related to the new feature bit. I also wonder whether there is *any* chance the (formerly pad) projid_hi field contains anything other than 0 on disk for any existing filesystems. First of all, I'll say that I think the superblock flag should be set only if it is known the file system has (at one time) held at least one project id requiring more than 16 bits to represent. Even if 32-bit project ids are supported, the flag should not be set on the superblock if no project id >= 2^16 is present. The way your code is now, in xfs_ioctl_setattr() no longer returns EINVAL when a passed-in project exceeds that representable in 16 bits. Instead, if the passed-in value is >= 2^16 you force the superblock to have the new PROJID32BIT feature flag turned on. (This is consistent with what I suggest, above.) But I don't agree with setting that flag in all cases, even when a 32-bit project id value is supplied. I could envision, for example, someone wanting to avoid exceeding the 16 bit limit to ensure their file system remains compatible with the older format. On the other hand, automatically setting it is useful as well. Instead, think it would be good to have a mount flag that indicates whether 32-bit project ids are to be supported (default: not supported). If the superblock read in at mount time had PROJID32BIT set, but the mount options did not indicate 32-bit project id support, the mount should fail with an explanation. (Without enabling the mount flag we would not handle large project ids properly). If the mount flag were supplied but the superblock did not (yet) have the flag set, that's OK. If any project id that required > 16 bits to represent got recorded, the superblock feature bit would be set to indicate that (as you propose). Matching mount and superblock flags would of course be a supported configuration. If the mount flag is set, then >= 2^16 project ids would be accepted in xfs_ioctl_setattr() (as you now do), but if it is not set it would not be allowed (as the existing code does). The mount flag could furthermore affect how the projid_hi superblock field is interpreted. If the 32-bit project id mount flag is not set, then the projid_hi would be forced to 0 when read off the disk and asserted 0 when writing to the disk. Similarly, if the superblock flag indicated no >16-bit project ids were present, their projid_hi values would be forced to 0 (even if the mount flag indicated 32-bit support). It may well be that XFS has always and consistently written 0's in the pad fields for the affected structures here. And if so, there's no reason to be concerned that any existing filesystems have non-zero garbage that would have any adverse effect. The "forced zero" handling I mentioned above would address most of these cases. If there is any risk of non-zero values in the projid_hi location we may need a utility of some kind to fix that before allowing a filesystem to be mounted with >16 bit projid support. OK, enough of that. I have a few other specific comments related to your actual code, below. -Alex > What has changed? > - sb_bad_features2 is also updated > - new helper - xfs_addprojid32bit > - drop 16bit projid protection in latest kernels > > I think it's ready to merge but it lacks final Reviewed-by, > Tested-by (beside me) etc :-( . . . > diff --git a/fs/xfs/linux-2.6/xfs_ioctl.c > b/fs/xfs/linux-2.6/xfs_ioctl.c > index 4fec427..aa72465 100644 > --- a/fs/xfs/linux-2.6/xfs_ioctl.c > +++ b/fs/xfs/linux-2.6/xfs_ioctl.c . . . > @@ -953,13 +946,22 @@ xfs_ioctl_setattr( > goto error_return; > } > > - /* > - * Do a quota reservation only if projid is actually going to > change. > - */ > if (mask & FSX_PROJID) { > + /* > + * Switch on the PROJID32BIT superblock bit when > needed > + * (implies also FEATURES2) > + */ > + if (!xfs_sb_version_hasprojid32bit(&ip->i_mount->m_sb) > && > + fa->fsx_projid > (__uint16_t)-1) Don't bother checking the version here, you do that again in the xfs_addprojid32bit() helper. Just check for exceeding the limit. > + xfs_addprojid32bit(tp, ip); > + . . . > diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h > index 0898c54..5bbb100 100644 > --- a/fs/xfs/xfs_inode.h > +++ b/fs/xfs/xfs_inode.h . . . > @@ -335,6 +336,23 @@ xfs_iflags_test_and_clear(xfs_inode_t *ip, unsigned short flags) > } > > /* > + * Project quota id helpers > + */ > +static inline prid_t > +xfs_get_projid(xfs_inode_t *ip) > +{ > + return (prid_t)(ip->i_d.di_projid_hi) << 16 | ip->i_d.di_projid_lo; No need for the parentheses around ip->i_d.di_projid_hi here. > +} > + > +static inline void > +xfs_set_projid(xfs_inode_t *ip, > + prid_t projid) > +{ > + ip->i_d.di_projid_hi = (__uint16_t) (projid >> 16); > + ip->i_d.di_projid_lo = (__uint16_t) (projid & 0xffff); > +} > + > +/* > * Manage the i_flush queue embedded in the inode. This completion > * queue synchronizes processes attempting to flush the in-core > * inode back to disk. . . . > diff --git a/fs/xfs/xfs_sb.h b/fs/xfs/xfs_sb.h > index 1b017c6..f7674c4 100644 > --- a/fs/xfs/xfs_sb.h > +++ b/fs/xfs/xfs_sb.h . . . > @@ -495,6 +497,19 @@ static inline void xfs_sb_version_removeattr2(xfs_sb_t *sbp) > sbp->sb_versionnum &= ~XFS_SB_VERSION_MOREBITSBIT; > } > > +static inline int xfs_sb_version_hasprojid32bit(xfs_sb_t *sbp) > +{ > + return xfs_sb_version_hasmorebits(sbp) && > + (sbp->sb_features2 & XFS_SB_VERSION2_PROJID32BIT); > +} > + > +static inline void xfs_sb_version_addprojid32bit(xfs_sb_t *sbp) > +{ > + sbp->sb_versionnum |= XFS_SB_VERSION_MOREBITSBIT; MOREBITSBIT is only meaningful for superblock version 4. So you should first convert this field to version 4. You might use using something like this (but setting the extra bits may be more than you want): sbp->sb_versionnum = xfs_sb_version_tonew(sbp->sb_versionnum); sbp->sb_versionnum |= XFS_SB_VERSION_MOREBITSBIT; (It looks like xfs_sb_version_addattr2() should have been written that way also.) > + sbp->sb_features2 |= XFS_SB_VERSION2_PROJID32BIT; > + sbp->sb_bad_features2 |= XFS_SB_VERSION2_PROJID32BIT; There is no need to set the sb_bad_features2 field. None of the other version functions do this. It is fixed when the file system is mounted if necessary. > +} > + > /* > * end of superblock version macros > */ . . . > diff --git a/fs/xfs/xfs_vnodeops.c b/fs/xfs/xfs_vnodeops.c > index 4c7c7bf..72319a9 100644 > --- a/fs/xfs/xfs_vnodeops.c > +++ b/fs/xfs/xfs_vnodeops.c . . . > @@ -1978,9 +1978,9 @@ xfs_symlink( > > udqp = gdqp = NULL; > if (dp->i_d.di_flags & XFS_DIFLAG_PROJINHERIT) > - prid = dp->i_d.di_projid; > + prid = xfs_get_projid(dp); > else > - prid = (xfs_prid_t)dfltprid; > + prid = dfltprid; I know this isn't your doing, but "dfltprid" is a complete CRAP symbol name, especially one that should be constrained to the XFS scope. How about renaming this XFS_PROJID_DEFAULT or something? > /* > * Make sure that we have allocated dquot(s) on disk. _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH VER 4] Extend project quotas to support 32bit project identificators. 2010-09-23 22:51 ` Alex Elder @ 2010-09-24 0:55 ` Dave Chinner 0 siblings, 0 replies; 10+ messages in thread From: Dave Chinner @ 2010-09-24 0:55 UTC (permalink / raw) To: Alex Elder; +Cc: xfs On Thu, Sep 23, 2010 at 05:51:01PM -0500, Alex Elder wrote: > On Wed, 2010-09-22 at 19:42 +0200, Arkadiusz Miśkiewicz wrote: > > This patch adds support for 32bit project quota identificators. > > > > On disk format is backward compatible with 16bit projid numbers. projid > > on disk is now keept in two 16bit values - di_projid_lo (which holds the > > same position as old 16bit projid value) and new di_projid_hi (takes > > existing padding) and convertes from/to 32bit value on the fly. > > > > PROJID32BIT feature2 flag is set automaticly when trying to use 32bit > > quota project identificator. > > > > Signed-off-by: Arkadiusz Miśkiewicz <arekm@maven.pl> > > --- > > Sorry it took me so long to review this. I have some feedback. > > I think what you've done looks generally good. The > only issues are related to the new feature bit. I also > wonder whether there is *any* chance the (formerly pad) > projid_hi field contains anything other than 0 on disk for > any existing filesystems. It will be. converion between v1 and v2 inodes has always zeroed the padding, and creation of inodes always zeros the entire inode before stamping it with the inode template. hence we can assume that the padding is always zero. .... > But I don't agree with setting that flag in all cases, > even when a 32-bit project id value is supplied. I could > envision, for example, someone wanting to avoid exceeding > the 16 bit limit to ensure their file system remains > compatible with the older format. On the other hand, > automatically setting it is useful as well. > > Instead, think it would be good to have a mount flag that > indicates whether 32-bit project ids are to be supported > (default: not supported). If the superblock read in at > mount time had PROJID32BIT set, but the mount options did > not indicate 32-bit project id support, the mount should > fail with an explanation. (Without enabling the mount > flag we would not handle large project ids properly). Too much complexity, and a mount option that needs to be supported forever. If the admin has to set a mount option, it's just as easy to ask them to run xfs_admin, and then we don't have a bunch of extra code and complex mount time logic to maintain forever. > > + return xfs_sb_version_hasmorebits(sbp) && > > + (sbp->sb_features2 & XFS_SB_VERSION2_PROJID32BIT); > > +} > > + > > +static inline void xfs_sb_version_addprojid32bit(xfs_sb_t *sbp) > > +{ > > + sbp->sb_versionnum |= XFS_SB_VERSION_MOREBITSBIT; > > MOREBITSBIT is only meaningful for superblock version 4. > So you should first convert this field to version 4. All XFS filesystems created since Irix 6.2 are version 4 filesystems, including every filesystem ever made on linux. The setting of the XFS_SB_VERSION_MOREBITSBIT like this is just fine. > You might use using something like this (but setting > the extra bits may be more than you want): > sbp->sb_versionnum = xfs_sb_version_tonew(sbp->sb_versionnum); > sbp->sb_versionnum |= XFS_SB_VERSION_MOREBITSBIT; > > (It looks like xfs_sb_version_addattr2() should have been > written that way also.) > > > + sbp->sb_features2 |= XFS_SB_VERSION2_PROJID32BIT; > > + sbp->sb_bad_features2 |= XFS_SB_VERSION2_PROJID32BIT; > > There is no need to set the sb_bad_features2 field. Actually, there is. The attr2 code is broken w.r.t. to this and needs complex logic during mount to ensure it isn't lost once set. Setting the sb_bad_features2 here means that we don't need to rely on other code to fix up the fact we didn't keep the sb_features2 and sb_bad_features2 fields consistent. Indeed, if we clear bits, we cannot rely on the mount code to fix that up, because it only detects missing bits and sets them again... > None > of the other version functions do this. Because none of them (except attr2) dynamically set feature2 bits. All the userspace code has been fixed to ensure that we when we set a feature2 bit, we also set the bad_features2 bit. > It is fixed when > the file system is mounted if necessary. That was introduced to try to fix up the mess of bad userspace tools writing to the wrong features2 field. attr2 handling is a mess because of this. We should not be writing new code that relies on the mount code to fix up mismatches, and we should be ensuring/fixing all the kernel code that sets/clears features2 bits to also sets/clears the bad_features2 bits as well. Cheers, Dave. -- Dave Chinner david@fromorbit.com _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2010-09-24 0:54 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-09-22 17:42 [PATCH VER 4] Extend project quotas to support 32bit project identificators Arkadiusz Miśkiewicz 2010-09-22 17:45 ` [PATCH VER 4] xfsprogs: projid32bit handling Arkadiusz Miśkiewicz 2010-09-22 17:45 ` [PATCH VER 4] xfstests: Quota project id setting overflow Arkadiusz Miśkiewicz 2010-09-22 18:44 ` [PATCH VER 4] xfsprogs: projid32bit handling Christoph Hellwig 2010-09-23 22:57 ` Alex Elder 2010-09-22 18:42 ` [PATCH VER 4] Extend project quotas to support 32bit project identificators Christoph Hellwig 2010-09-22 19:01 ` Arkadiusz Miskiewicz 2010-09-24 0:37 ` Dave Chinner 2010-09-23 22:51 ` Alex Elder 2010-09-24 0:55 ` Dave Chinner
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox