From: "Darrick J. Wong" <darrick.wong@oracle.com>
To: Christoph Hellwig <hch@infradead.org>
Cc: linux-xfs@vger.kernel.org
Subject: Re: [PATCH 05/26] xfs: split the incore dquot type into a separate field
Date: Tue, 14 Jul 2020 11:05:02 -0700 [thread overview]
Message-ID: <20200714180502.GB7606@magnolia> (raw)
In-Reply-To: <20200714075756.GB19883@infradead.org>
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@oracle.com>
> >
> > 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.
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 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.
I will change the series to define the DQTYPE flags to match the DDQTYPE
flags when they do overlap so that I can drop xfs_dquot_type_to_disk,
but I'm not going to separate the flag/type/whatever namespaces just to
combine them again.
--D
> diff --git a/fs/xfs/libxfs/xfs_dquot_buf.c b/fs/xfs/libxfs/xfs_dquot_buf.c
> index 889e34b1a03335..ef9b8559ff6197 100644
> --- a/fs/xfs/libxfs/xfs_dquot_buf.c
> +++ b/fs/xfs/libxfs/xfs_dquot_buf.c
> @@ -62,15 +62,14 @@ xfs_dquot_verify(
> if (ddq->d_version != XFS_DQUOT_VERSION)
> return __this_address;
>
> - if (ddq->d_type & ~XFS_DDQTYPE_ANY)
> + if (ddq->d_type & ~XFS_DQTYPE_ANY)
> return __this_address;
> - ddq_type = ddq->d_type & XFS_DDQTYPE_REC_MASK;
> - if (type != XFS_DQTYPE_NONE &&
> - ddq_type != xfs_dquot_type_to_disk(type))
> + ddq_type = ddq->d_type & XFS_DQTYPE_REC_MASK;
> + if (type != XFS_DQTYPE_NONE && ddq_type != type)
> return __this_address;
> - if (ddq_type != XFS_DDQTYPE_USER &&
> - ddq_type != XFS_DDQTYPE_PROJ &&
> - ddq_type != XFS_DDQTYPE_GROUP)
> + if (ddq_type != XFS_DQTYPE_USER &&
> + ddq_type != XFS_DQTYPE_PROJ &&
> + ddq_type != XFS_DQTYPE_GROUP)
> return __this_address;
>
> if (id != -1 && id != be32_to_cpu(ddq->d_id))
> @@ -129,7 +128,7 @@ xfs_dqblk_repair(
>
> dqb->dd_diskdq.d_magic = cpu_to_be16(XFS_DQUOT_MAGIC);
> dqb->dd_diskdq.d_version = XFS_DQUOT_VERSION;
> - dqb->dd_diskdq.d_type = xfs_dquot_type_to_disk(type);
> + dqb->dd_diskdq.d_type = type;
> dqb->dd_diskdq.d_id = cpu_to_be32(id);
>
> if (xfs_sb_version_hascrc(&mp->m_sb)) {
> diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h
> index eed0c4d5baddbe..e4178c804abf06 100644
> --- a/fs/xfs/libxfs/xfs_format.h
> +++ b/fs/xfs/libxfs/xfs_format.h
> @@ -1150,16 +1150,25 @@ 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_DDQTYPE_USER 0x01 /* user dquot record */
> -#define XFS_DDQTYPE_PROJ 0x02 /* project dquot record */
> -#define XFS_DDQTYPE_GROUP 0x04 /* group dquot record */
> +#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" }
>
> /* bitmask to determine if this is a user/group/project dquot */
> -#define XFS_DDQTYPE_REC_MASK (XFS_DDQTYPE_USER | \
> - XFS_DDQTYPE_PROJ | \
> - XFS_DDQTYPE_GROUP)
> +#define XFS_DQTYPE_REC_MASK (XFS_DQTYPE_USER | \
> + XFS_DQTYPE_PROJ | \
> + XFS_DQTYPE_GROUP)
> +
> +#define XFS_DQTYPE_ANY (XFS_DQTYPE_REC_MASK)
>
> -#define XFS_DDQTYPE_ANY (XFS_DDQTYPE_REC_MASK)
> +typedef uint8_t xfs_dqtype_t;
>
> /*
> * This is the main portion of the on-disk representation of quota
> @@ -1170,7 +1179,7 @@ static inline void xfs_dinode_put_rdev(struct xfs_dinode *dip, xfs_dev_t rdev)
> struct xfs_disk_dquot {
> __be16 d_magic; /* dquot magic = XFS_DQUOT_MAGIC */
> __u8 d_version; /* dquot version */
> - __u8 d_type; /* XFS_DDQTYPE_* */
> + xfs_dqtype_t d_type; /* XFS_DQTYPE_* */
> __be32 d_id; /* user,project,group id */
> __be64 d_blk_hardlimit;/* absolute limit on disk blks */
> __be64 d_blk_softlimit;/* preferred limit on disk blks */
> diff --git a/fs/xfs/libxfs/xfs_quota_defs.h b/fs/xfs/libxfs/xfs_quota_defs.h
> index 0650fa71fa2bcf..6edd249fdef4ea 100644
> --- a/fs/xfs/libxfs/xfs_quota_defs.h
> +++ b/fs/xfs/libxfs/xfs_quota_defs.h
> @@ -18,36 +18,6 @@
> typedef uint64_t xfs_qcnt_t;
> typedef uint16_t xfs_qwarncnt_t;
>
> -typedef uint8_t xfs_dqtype_t;
> -
> -#define XFS_DQTYPE_NONE (0)
> -#define XFS_DQTYPE_USER (1)
> -#define XFS_DQTYPE_PROJ (2)
> -#define XFS_DQTYPE_GROUP (3)
> -
> -#define XFS_DQTYPE_STRINGS \
> - { XFS_DQTYPE_NONE, "NONE?" }, \
> - { XFS_DQTYPE_USER, "USER" }, \
> - { XFS_DQTYPE_PROJ, "PROJ" }, \
> - { XFS_DQTYPE_GROUP, "GROUP" }
> -
> -static inline __u8
> -xfs_dquot_type_to_disk(
> - xfs_dqtype_t type)
> -{
> - switch (type) {
> - case XFS_DQTYPE_USER:
> - return XFS_DDQTYPE_USER;
> - case XFS_DQTYPE_GROUP:
> - return XFS_DDQTYPE_GROUP;
> - case XFS_DQTYPE_PROJ:
> - return XFS_DDQTYPE_PROJ;
> - default:
> - ASSERT(0);
> - return 0;
> - }
> -}
> -
> /*
> * flags for q_flags field in the dquot.
> */
> diff --git a/fs/xfs/xfs_buf_item_recover.c b/fs/xfs/xfs_buf_item_recover.c
> index dd150f8bbf5a2a..c2f19d35e05dbd 100644
> --- a/fs/xfs/xfs_buf_item_recover.c
> +++ b/fs/xfs/xfs_buf_item_recover.c
> @@ -548,11 +548,11 @@ xlog_recover_do_dquot_buffer(
>
> type = 0;
> if (buf_f->blf_flags & XFS_BLF_UDQUOT_BUF)
> - type |= XFS_DDQTYPE_USER;
> + type |= XFS_DQTYPE_USER;
> if (buf_f->blf_flags & XFS_BLF_PDQUOT_BUF)
> - type |= XFS_DDQTYPE_PROJ;
> + type |= XFS_DQTYPE_PROJ;
> if (buf_f->blf_flags & XFS_BLF_GDQUOT_BUF)
> - type |= XFS_DDQTYPE_GROUP;
> + type |= XFS_DQTYPE_GROUP;
> /*
> * This type of quotas was turned off, so ignore this buffer
> */
> diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
> index 14b0c62943d54e..0581cb930cd75e 100644
> --- a/fs/xfs/xfs_dquot.c
> +++ b/fs/xfs/xfs_dquot.c
> @@ -182,7 +182,7 @@ xfs_qm_init_dquot_blk(
> d->dd_diskdq.d_magic = cpu_to_be16(XFS_DQUOT_MAGIC);
> d->dd_diskdq.d_version = XFS_DQUOT_VERSION;
> d->dd_diskdq.d_id = cpu_to_be32(curid);
> - d->dd_diskdq.d_type = xfs_dquot_type_to_disk(type);
> + d->dd_diskdq.d_type = type;
> if (xfs_sb_version_hascrc(&mp->m_sb)) {
> uuid_copy(&d->dd_uuid, &mp->m_sb.sb_meta_uuid);
> xfs_update_cksum((char *)d, sizeof(struct xfs_dqblk),
> @@ -481,13 +481,13 @@ xfs_dquot_from_disk(
> struct xfs_buf *bp)
> {
> struct xfs_disk_dquot *ddqp = bp->b_addr + dqp->q_bufoffset;
> - __u8 ddq_type = xfs_dquot_type_to_disk(dqp->q_type);
> + __u8 ddq_type = dqp->q_type;
>
> /*
> * Ensure that we got the type and ID we were looking for.
> * Everything else was checked by the dquot buffer verifier.
> */
> - if ((ddqp->d_type & XFS_DDQTYPE_REC_MASK) != ddq_type ||
> + if ((ddqp->d_type & XFS_DQTYPE_REC_MASK) != ddq_type ||
> be32_to_cpu(ddqp->d_id) != dqp->q_id) {
> xfs_alert_tag(bp->b_mount, XFS_PTAG_VERIFIER_ERROR,
> "Metadata corruption detected at %pS, quota %u",
> @@ -537,7 +537,7 @@ xfs_dquot_to_disk(
> {
> ddqp->d_magic = cpu_to_be16(XFS_DQUOT_MAGIC);
> ddqp->d_version = XFS_DQUOT_VERSION;
> - ddqp->d_type = xfs_dquot_type_to_disk(dqp->q_type);
> + ddqp->d_type = dqp->q_type;
> ddqp->d_id = cpu_to_be32(dqp->q_id);
> ddqp->d_pad0 = 0;
> ddqp->d_pad = 0;
> diff --git a/fs/xfs/xfs_dquot_item_recover.c b/fs/xfs/xfs_dquot_item_recover.c
> index 0955f183a02758..b5afb9fb8cd4fd 100644
> --- a/fs/xfs/xfs_dquot_item_recover.c
> +++ b/fs/xfs/xfs_dquot_item_recover.c
> @@ -39,7 +39,7 @@ xlog_recover_dquot_ra_pass2(
> if (item->ri_buf[1].i_len < sizeof(struct xfs_disk_dquot))
> return;
>
> - type = recddq->d_type & XFS_DDQTYPE_REC_MASK;
> + type = recddq->d_type & XFS_DQTYPE_REC_MASK;
> ASSERT(type);
> if (log->l_quotaoffs_flag & type)
> return;
> @@ -91,7 +91,7 @@ xlog_recover_dquot_commit_pass2(
> /*
> * This type of quotas was turned off, so ignore this record.
> */
> - type = recddq->d_type & XFS_DDQTYPE_REC_MASK;
> + type = recddq->d_type & XFS_DQTYPE_REC_MASK;
> ASSERT(type);
> if (log->l_quotaoffs_flag & type)
> return 0;
> @@ -185,11 +185,11 @@ xlog_recover_quotaoff_commit_pass1(
> * group/project quotaoff or both.
> */
> if (qoff_f->qf_flags & XFS_UQUOTA_ACCT)
> - log->l_quotaoffs_flag |= XFS_DDQTYPE_USER;
> + log->l_quotaoffs_flag |= XFS_DQTYPE_USER;
> if (qoff_f->qf_flags & XFS_PQUOTA_ACCT)
> - log->l_quotaoffs_flag |= XFS_DDQTYPE_PROJ;
> + log->l_quotaoffs_flag |= XFS_DQTYPE_PROJ;
> if (qoff_f->qf_flags & XFS_GQUOTA_ACCT)
> - log->l_quotaoffs_flag |= XFS_DDQTYPE_GROUP;
> + log->l_quotaoffs_flag |= XFS_DQTYPE_GROUP;
>
> return 0;
> }
next prev parent reply other threads:[~2020-07-14 18:05 UTC|newest]
Thread overview: 49+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-14 1:31 [PATCH v3 00/26] xfs: remove xfs_disk_quot from incore dquot Darrick J. Wong
2020-07-14 1:31 ` [PATCH 01/26] xfs: clear XFS_DQ_FREEING if we can't lock the dquot buffer to flush Darrick J. Wong
2020-07-14 1:31 ` [PATCH 02/26] xfs: fix inode quota reservation checks Darrick J. Wong
2020-07-14 1:31 ` [PATCH 03/26] xfs: validate ondisk/incore dquot flags Darrick J. Wong
2020-07-14 1:31 ` [PATCH 04/26] xfs: move the flags argument of xfs_qm_scall_trunc_qfiles to XFS_QMOPT_* Darrick J. Wong
2020-07-14 7:24 ` Christoph Hellwig
2020-07-14 1:32 ` [PATCH 05/26] xfs: split the incore dquot type into a separate field Darrick J. Wong
2020-07-14 7:57 ` Christoph Hellwig
2020-07-14 18:05 ` Darrick J. Wong [this message]
2020-07-15 17:43 ` Christoph Hellwig
2020-07-15 18:38 ` Darrick J. Wong
2020-07-15 18:39 ` Christoph Hellwig
2020-07-15 21:20 ` Darrick J. Wong
2020-07-16 2:17 ` Darrick J. Wong
2020-07-14 1:32 ` [PATCH 06/26] xfs: refactor quotacheck flags usage Darrick J. Wong
2020-07-14 7:58 ` Christoph Hellwig
2020-07-14 1:32 ` [PATCH 07/26] xfs: rename dquot incore state flags Darrick J. Wong
2020-07-14 8:00 ` Christoph Hellwig
2020-07-14 1:32 ` [PATCH 08/26] xfs: move the ondisk dquot flags to their own namespace Darrick J. Wong
2020-07-14 8:05 ` Christoph Hellwig
2020-07-14 18:07 ` Darrick J. Wong
2020-07-14 1:32 ` [PATCH 09/26] xfs: make XFS_DQUOT_CLUSTER_SIZE_FSB part of the ondisk format Darrick J. Wong
2020-07-14 1:32 ` [PATCH 10/26] xfs: stop using q_core.d_flags in the quota code Darrick J. Wong
2020-07-14 8:00 ` Christoph Hellwig
2020-07-14 1:32 ` [PATCH 11/26] xfs: stop using q_core.d_id " Darrick J. Wong
2020-07-14 1:32 ` [PATCH 12/26] xfs: use a per-resource struct for incore dquot data Darrick J. Wong
2020-07-14 1:32 ` [PATCH 13/26] xfs: stop using q_core limits in the quota code Darrick J. Wong
2020-07-14 1:32 ` [PATCH 14/26] xfs: stop using q_core counters " Darrick J. Wong
2020-07-14 1:33 ` [PATCH 15/26] xfs: stop using q_core warning " Darrick J. Wong
2020-07-14 1:33 ` [PATCH 16/26] xfs: stop using q_core timers " Darrick J. Wong
2020-07-14 1:33 ` [PATCH 17/26] xfs: remove qcore from incore dquots Darrick J. Wong
2020-07-14 1:33 ` [PATCH 18/26] xfs: refactor default quota limits by resource Darrick J. Wong
2020-07-14 1:33 ` [PATCH 19/26] xfs: remove unnecessary arguments from quota adjust functions Darrick J. Wong
2020-07-14 1:33 ` [PATCH 20/26] xfs: refactor quota exceeded test Darrick J. Wong
2020-07-14 8:01 ` Christoph Hellwig
2020-07-14 1:33 ` [PATCH 21/26] xfs: refactor xfs_qm_scall_setqlim Darrick J. Wong
2020-07-14 1:33 ` [PATCH 22/26] xfs: refactor xfs_trans_dqresv Darrick J. Wong
2020-07-14 8:01 ` Christoph Hellwig
2020-07-14 1:33 ` [PATCH 23/26] xfs: refactor xfs_trans_apply_dquot_deltas Darrick J. Wong
2020-07-14 8:02 ` Christoph Hellwig
2020-07-14 1:34 ` [PATCH 24/26] xfs: assume the default quota limits are always set in xfs_qm_adjust_dqlimits Darrick J. Wong
2020-07-14 8:02 ` Christoph Hellwig
2020-07-14 1:34 ` [PATCH 25/26] xfs: actually bump warning counts when we send warnings Darrick J. Wong
2020-07-14 8:03 ` Christoph Hellwig
2020-07-14 1:34 ` [PATCH 26/26] xfs: add more dquot tracepoints Darrick J. Wong
2020-07-14 8:03 ` Christoph Hellwig
2020-07-14 1:43 ` [PATCH v3 00/26] xfs: remove xfs_disk_quot from incore dquot Darrick J. Wong
-- strict thread matches above, loose matches on Subject: below --
2020-07-15 1:50 [PATCH v4 " Darrick J. Wong
2020-07-15 1:51 ` [PATCH 05/26] xfs: split the incore dquot type into a separate field Darrick J. Wong
2020-07-16 6:59 ` Chandan Babu R
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200714180502.GB7606@magnolia \
--to=darrick.wong@oracle.com \
--cc=hch@infradead.org \
--cc=linux-xfs@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).