On Tue, Jul 14, 2020 at 11:05:02AM -0700, Darrick J. Wong wrote:
> On Tue, Jul 14, 2020 at 08:57:56AM +0100, Christoph Hellwig wrote:
> > On Mon, Jul 13, 2020 at 06:32:00PM -0700, Darrick J. Wong wrote:
> > > From: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> > >
> > > Create a new type (xfs_dqtype_t) to represent the type of an incore
> > > dquot. Break the type field out from the dq_flags field of the incore
> > > dquot.
> >
> > I don't understand why we need separate in-core vs on-disk values for
> > the type. Why not something like this on top of the whole series:
>
> I want to keep the ondisk d_type values separate from the incore q_type
> values because they don't describe exactly the same concepts:
>
> First, the incore qtype has a NONE value that we can pass to the dquot
> core verifier when we don't actually know if this is a user, group, or
> project dquot. This should never end up on disk.
Which we can trivially verify. Or just get rid of NONE, which actually
cleans things up a fair bit (patch on top of my previous one below)
> Second, xfs_dqtype_t is a (barely concealed) enumeration type for quota
> callers to tell us that they want to perform an action on behalf of
> user, group, or project quotas. The incore q_flags and the ondisk
> d_type contain internal state that should not be exposed to quota
> callers.
I don't think that is an argument, as we do the same elsewhere.
>
> I feel a need to reiterate that I'm about to start adding more flags to
> d_type (for y2038+ time support), for which it will be very important to
> keep d_type and q_{type,flags} separate.
Why? We'll just OR the bigtime flag in before writing to disk.
diff --git a/fs/xfs/libxfs/xfs_dquot_buf.c b/fs/xfs/libxfs/xfs_dquot_buf.c
index ef9b8559ff6197..c48b77c223073e 100644
--- a/fs/xfs/libxfs/xfs_dquot_buf.c
+++ b/fs/xfs/libxfs/xfs_dquot_buf.c
@@ -37,11 +37,8 @@ xfs_failaddr_t
xfs_dquot_verify(
struct xfs_mount *mp,
struct xfs_disk_dquot *ddq,
- xfs_dqid_t id,
- xfs_dqtype_t type) /* used only during quotacheck */
+ xfs_dqid_t id)
{
- uint8_t ddq_type;
-
/*
* We can encounter an uninitialized dquot buffer for 2 reasons:
* 1. If we crash while deleting the quotainode(s), and those blks got
@@ -64,13 +61,14 @@ xfs_dquot_verify(
if (ddq->d_type & ~XFS_DQTYPE_ANY)
return __this_address;
- ddq_type = ddq->d_type & XFS_DQTYPE_REC_MASK;
- if (type != XFS_DQTYPE_NONE && ddq_type != type)
- return __this_address;
- if (ddq_type != XFS_DQTYPE_USER &&
- ddq_type != XFS_DQTYPE_PROJ &&
- ddq_type != XFS_DQTYPE_GROUP)
+ switch (ddq->d_type & XFS_DQTYPE_REC_MASK) {
+ case XFS_DQTYPE_USER:
+ case XFS_DQTYPE_PROJ:
+ case XFS_DQTYPE_GROUP:
+ break;
+ default:
return __this_address;
+ }
if (id != -1 && id != be32_to_cpu(ddq->d_id))
return __this_address;
@@ -100,14 +98,12 @@ xfs_failaddr_t
xfs_dqblk_verify(
struct xfs_mount *mp,
struct xfs_dqblk *dqb,
- xfs_dqid_t id,
- xfs_dqtype_t type) /* used only during quotacheck */
+ xfs_dqid_t id)
{
if (xfs_sb_version_hascrc(&mp->m_sb) &&
!uuid_equal(&dqb->dd_uuid, &mp->m_sb.sb_meta_uuid))
return __this_address;
-
- return xfs_dquot_verify(mp, &dqb->dd_diskdq, id, type);
+ return xfs_dquot_verify(mp, &dqb->dd_diskdq, id);
}
/*
@@ -210,7 +206,7 @@ xfs_dquot_buf_verify(
if (i == 0)
id = be32_to_cpu(ddq->d_id);
- fa = xfs_dqblk_verify(mp, &dqb[i], id + i, XFS_DQTYPE_NONE);
+ fa = xfs_dqblk_verify(mp, &dqb[i], id + i);
if (fa) {
if (!readahead)
xfs_buf_verifier_error(bp, -EFSCORRUPTED,
diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h
index e4178c804abf06..ec472131ea4b15 100644
--- a/fs/xfs/libxfs/xfs_format.h
+++ b/fs/xfs/libxfs/xfs_format.h
@@ -1150,13 +1150,11 @@ static inline void xfs_dinode_put_rdev(struct xfs_dinode *dip, xfs_dev_t rdev)
#define XFS_DQUOT_MAGIC 0x4451 /* 'DQ' */
#define XFS_DQUOT_VERSION (uint8_t)0x01 /* latest version number */
-#define XFS_DQTYPE_NONE 0
#define XFS_DQTYPE_USER 0x01 /* user dquot record */
#define XFS_DQTYPE_PROJ 0x02 /* project dquot record */
#define XFS_DQTYPE_GROUP 0x04 /* group dquot record */
#define XFS_DQTYPE_STRINGS \
- { XFS_DQTYPE_NONE, "NONE" }, \
{ XFS_DQTYPE_USER, "USER" }, \
{ XFS_DQTYPE_PROJ, "PROJ" }, \
{ XFS_DQTYPE_GROUP, "GROUP" }
diff --git a/fs/xfs/libxfs/xfs_quota_defs.h b/fs/xfs/libxfs/xfs_quota_defs.h
index 6edd249fdef4ea..5e3d6b49981707 100644
--- a/fs/xfs/libxfs/xfs_quota_defs.h
+++ b/fs/xfs/libxfs/xfs_quota_defs.h
@@ -129,9 +129,9 @@ typedef uint16_t xfs_qwarncnt_t;
#define XFS_QMOPT_RESBLK_MASK (XFS_QMOPT_RES_REGBLKS | XFS_QMOPT_RES_RTBLKS)
extern xfs_failaddr_t xfs_dquot_verify(struct xfs_mount *mp,
- struct xfs_disk_dquot *ddq, xfs_dqid_t id, xfs_dqtype_t type);
+ struct xfs_disk_dquot *ddq, xfs_dqid_t id);
extern xfs_failaddr_t xfs_dqblk_verify(struct xfs_mount *mp,
- struct xfs_dqblk *dqb, xfs_dqid_t id, xfs_dqtype_t type);
+ struct xfs_dqblk *dqb, xfs_dqid_t id);
extern int xfs_calc_dquots_per_chunk(unsigned int nbblks);
extern void xfs_dqblk_repair(struct xfs_mount *mp, struct xfs_dqblk *dqb,
xfs_dqid_t id, xfs_dqtype_t type);
diff --git a/fs/xfs/scrub/quota.c b/fs/xfs/scrub/quota.c
index f045895f28ffb1..a8eace16fdae74 100644
--- a/fs/xfs/scrub/quota.c
+++ b/fs/xfs/scrub/quota.c
@@ -31,7 +31,7 @@ xchk_quota_to_dqtype(
return XFS_DQTYPE_PROJ;
default:
ASSERT(0);
- return XFS_DQTYPE_NONE;
+ return 0;
}
}
diff --git a/fs/xfs/xfs_buf_item_recover.c b/fs/xfs/xfs_buf_item_recover.c
index c2f19d35e05dbd..e4f37c064497aa 100644
--- a/fs/xfs/xfs_buf_item_recover.c
+++ b/fs/xfs/xfs_buf_item_recover.c
@@ -494,8 +494,7 @@ xlog_recover_do_reg_buffer(
item->ri_buf[i].i_len, __func__);
goto next;
}
- fa = xfs_dquot_verify(mp, item->ri_buf[i].i_addr,
- -1, XFS_DQTYPE_NONE);
+ fa = xfs_dquot_verify(mp, item->ri_buf[i].i_addr, -1);
if (fa) {
xfs_alert(mp,
"dquot corrupt at %pS trying to replay into block 0x%llx",
diff --git a/fs/xfs/xfs_dquot_item_recover.c b/fs/xfs/xfs_dquot_item_recover.c
index b5afb9fb8cd4fd..2df6238ed19abc 100644
--- a/fs/xfs/xfs_dquot_item_recover.c
+++ b/fs/xfs/xfs_dquot_item_recover.c
@@ -108,7 +108,7 @@ xlog_recover_dquot_commit_pass2(
*/
dq_f = item->ri_buf[0].i_addr;
ASSERT(dq_f);
- fa = xfs_dquot_verify(mp, recddq, dq_f->qlf_id, XFS_DQTYPE_NONE);
+ fa = xfs_dquot_verify(mp, recddq, dq_f->qlf_id);
if (fa) {
xfs_alert(mp, "corrupt dquot ID 0x%x in log at %pS",
dq_f->qlf_id, fa);
diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
index 96171f4406e978..716b91b582ff56 100644
--- a/fs/xfs/xfs_qm.c
+++ b/fs/xfs/xfs_qm.c
@@ -828,7 +828,6 @@ xfs_qm_reset_dqcounts(
{
struct xfs_dqblk *dqb;
int j;
- xfs_failaddr_t fa;
trace_xfs_reset_dqcounts(bp, _RET_IP_);
@@ -853,8 +852,8 @@ xfs_qm_reset_dqcounts(
* find uninitialised dquot blks. See comment in
* xfs_dquot_verify.
*/
- fa = xfs_dqblk_verify(mp, &dqb[j], id + j, type);
- if (fa)
+ if (xfs_dqblk_verify(mp, &dqb[j], id + j, type) ||
+ dqb[j].d_type != type)
xfs_dqblk_repair(mp, &dqb[j], id + j, type);
/*
