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
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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.