From: "Darrick J. Wong" <darrick.wong@oracle.com>
To: Christoph Hellwig <hch@lst.de>
Cc: linux-xfs@vger.kernel.org
Subject: Re: [PATCH 2/4] xfs: merge the projid fields in struct xfs_icdinode
Date: Tue, 12 Nov 2019 08:22:44 -0800 [thread overview]
Message-ID: <20191112162244.GY6219@magnolia> (raw)
In-Reply-To: <20191020082145.32515-3-hch@lst.de>
On Sun, Oct 20, 2019 at 10:21:43AM +0200, Christoph Hellwig wrote:
> There is no point in splitting the fields like this in an purely
> in-memory structure.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Looks ok, will give it a spin...
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
--D
> ---
> fs/xfs/libxfs/xfs_inode_buf.c | 11 +++++------
> fs/xfs/libxfs/xfs_inode_buf.h | 3 +--
> fs/xfs/xfs_dquot.c | 2 +-
> fs/xfs/xfs_icache.c | 4 ++--
> fs/xfs/xfs_inode.c | 6 +++---
> fs/xfs/xfs_inode.h | 21 +--------------------
> fs/xfs/xfs_inode_item.c | 4 ++--
> fs/xfs/xfs_ioctl.c | 8 ++++----
> fs/xfs/xfs_iops.c | 2 +-
> fs/xfs/xfs_itable.c | 2 +-
> fs/xfs/xfs_qm.c | 8 ++++----
> fs/xfs/xfs_qm_bhv.c | 2 +-
> 12 files changed, 26 insertions(+), 47 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_inode_buf.c b/fs/xfs/libxfs/xfs_inode_buf.c
> index d31156718b20..019c9be677cc 100644
> --- a/fs/xfs/libxfs/xfs_inode_buf.c
> +++ b/fs/xfs/libxfs/xfs_inode_buf.c
> @@ -213,13 +213,12 @@ xfs_inode_from_disk(
> to->di_version = from->di_version;
> if (to->di_version == 1) {
> set_nlink(inode, be16_to_cpu(from->di_onlink));
> - to->di_projid_lo = 0;
> - to->di_projid_hi = 0;
> + to->di_projid = 0;
> to->di_version = 2;
> } else {
> set_nlink(inode, be32_to_cpu(from->di_nlink));
> - to->di_projid_lo = be16_to_cpu(from->di_projid_lo);
> - to->di_projid_hi = be16_to_cpu(from->di_projid_hi);
> + to->di_projid = (prid_t)be16_to_cpu(from->di_projid_hi) << 16 |
> + be16_to_cpu(from->di_projid_lo);
> }
>
> to->di_format = from->di_format;
> @@ -279,8 +278,8 @@ xfs_inode_to_disk(
> to->di_format = from->di_format;
> to->di_uid = cpu_to_be32(from->di_uid);
> to->di_gid = cpu_to_be32(from->di_gid);
> - to->di_projid_lo = cpu_to_be16(from->di_projid_lo);
> - to->di_projid_hi = cpu_to_be16(from->di_projid_hi);
> + to->di_projid_lo = cpu_to_be16(from->di_projid & 0xffff);
> + to->di_projid_hi = cpu_to_be16(from->di_projid >> 16);
>
> memset(to->di_pad, 0, sizeof(to->di_pad));
> to->di_atime.t_sec = cpu_to_be32(inode->i_atime.tv_sec);
> diff --git a/fs/xfs/libxfs/xfs_inode_buf.h b/fs/xfs/libxfs/xfs_inode_buf.h
> index c9ac69c82d21..fd94b1078722 100644
> --- a/fs/xfs/libxfs/xfs_inode_buf.h
> +++ b/fs/xfs/libxfs/xfs_inode_buf.h
> @@ -21,8 +21,7 @@ struct xfs_icdinode {
> uint16_t di_flushiter; /* incremented on flush */
> uint32_t di_uid; /* owner's user id */
> uint32_t di_gid; /* owner's group id */
> - uint16_t di_projid_lo; /* lower part of owner's project id */
> - uint16_t di_projid_hi; /* higher part of owner's project id */
> + uint32_t di_projid; /* owner's project id */
> xfs_fsize_t di_size; /* number of bytes in file */
> xfs_rfsblock_t di_nblocks; /* # of direct & btree blocks used */
> xfs_extlen_t di_extsize; /* basic/minimum extent size for file */
> diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
> index aeb95e7391c1..12074c1d250c 100644
> --- a/fs/xfs/xfs_dquot.c
> +++ b/fs/xfs/xfs_dquot.c
> @@ -833,7 +833,7 @@ xfs_qm_id_for_quotatype(
> case XFS_DQ_GROUP:
> return ip->i_d.di_gid;
> case XFS_DQ_PROJ:
> - return xfs_get_projid(ip);
> + return ip->i_d.di_projid;
> }
> ASSERT(0);
> return 0;
> diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c
> index 944add5ff8e0..ec302b7e48f3 100644
> --- a/fs/xfs/xfs_icache.c
> +++ b/fs/xfs/xfs_icache.c
> @@ -1419,7 +1419,7 @@ xfs_inode_match_id(
> return 0;
>
> if ((eofb->eof_flags & XFS_EOF_FLAGS_PRID) &&
> - xfs_get_projid(ip) != eofb->eof_prid)
> + ip->i_d.di_projid != eofb->eof_prid)
> return 0;
>
> return 1;
> @@ -1443,7 +1443,7 @@ xfs_inode_match_id_union(
> return 1;
>
> if ((eofb->eof_flags & XFS_EOF_FLAGS_PRID) &&
> - xfs_get_projid(ip) == eofb->eof_prid)
> + ip->i_d.di_projid == eofb->eof_prid)
> return 1;
>
> return 0;
> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> index 24efdbf534c7..685c21d0a6ca 100644
> --- a/fs/xfs/xfs_inode.c
> +++ b/fs/xfs/xfs_inode.c
> @@ -809,7 +809,7 @@ xfs_ialloc(
> ip->i_d.di_uid = xfs_kuid_to_uid(current_fsuid());
> ip->i_d.di_gid = xfs_kgid_to_gid(current_fsgid());
> inode->i_rdev = rdev;
> - xfs_set_projid(ip, prid);
> + ip->i_d.di_projid = prid;
>
> if (pip && XFS_INHERIT_GID(pip)) {
> ip->i_d.di_gid = pip->i_d.di_gid;
> @@ -1417,7 +1417,7 @@ xfs_link(
> * the tree quota mechanism could be circumvented.
> */
> if (unlikely((tdp->i_d.di_flags & XFS_DIFLAG_PROJINHERIT) &&
> - (xfs_get_projid(tdp) != xfs_get_projid(sip)))) {
> + tdp->i_d.di_projid != sip->i_d.di_projid)) {
> error = -EXDEV;
> goto error_return;
> }
> @@ -3269,7 +3269,7 @@ xfs_rename(
> * tree quota mechanism would be circumvented.
> */
> if (unlikely((target_dp->i_d.di_flags & XFS_DIFLAG_PROJINHERIT) &&
> - (xfs_get_projid(target_dp) != xfs_get_projid(src_ip)))) {
> + target_dp->i_d.di_projid != src_ip->i_d.di_projid)) {
> error = -EXDEV;
> goto out_trans_cancel;
> }
> diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
> index 558173f95a03..a0ca7ded3ab8 100644
> --- a/fs/xfs/xfs_inode.h
> +++ b/fs/xfs/xfs_inode.h
> @@ -177,30 +177,11 @@ xfs_iflags_test_and_set(xfs_inode_t *ip, unsigned short flags)
> return ret;
> }
>
> -/*
> - * Project quota id helpers (previously projid was 16bit only
> - * and using two 16bit values to hold new 32bit projid was chosen
> - * to retain compatibility with "old" filesystems).
> - */
> -static inline prid_t
> -xfs_get_projid(struct xfs_inode *ip)
> -{
> - return (prid_t)ip->i_d.di_projid_hi << 16 | ip->i_d.di_projid_lo;
> -}
> -
> -static inline void
> -xfs_set_projid(struct xfs_inode *ip,
> - prid_t projid)
> -{
> - ip->i_d.di_projid_hi = (uint16_t) (projid >> 16);
> - ip->i_d.di_projid_lo = (uint16_t) (projid & 0xffff);
> -}
> -
> static inline prid_t
> xfs_get_initial_prid(struct xfs_inode *dp)
> {
> if (dp->i_d.di_flags & XFS_DIFLAG_PROJINHERIT)
> - return xfs_get_projid(dp);
> + return dp->i_d.di_projid;
>
> return XFS_PROJID_DEFAULT;
> }
> diff --git a/fs/xfs/xfs_inode_item.c b/fs/xfs/xfs_inode_item.c
> index a15db5d679ac..e6ffeb1b8a92 100644
> --- a/fs/xfs/xfs_inode_item.c
> +++ b/fs/xfs/xfs_inode_item.c
> @@ -309,8 +309,8 @@ xfs_inode_to_log_dinode(
> to->di_format = from->di_format;
> to->di_uid = from->di_uid;
> to->di_gid = from->di_gid;
> - to->di_projid_lo = from->di_projid_lo;
> - to->di_projid_hi = from->di_projid_hi;
> + to->di_projid_lo = from->di_projid & 0xffff;
> + to->di_projid_hi = from->di_projid >> 16;
>
> memset(to->di_pad, 0, sizeof(to->di_pad));
> memset(to->di_pad3, 0, sizeof(to->di_pad3));
> diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
> index d58f0d6a699e..6ff01e4a8b7b 100644
> --- a/fs/xfs/xfs_ioctl.c
> +++ b/fs/xfs/xfs_ioctl.c
> @@ -1116,7 +1116,7 @@ xfs_fill_fsxattr(
> fa->fsx_extsize = ip->i_d.di_extsize << ip->i_mount->m_sb.sb_blocklog;
> fa->fsx_cowextsize = ip->i_d.di_cowextsize <<
> ip->i_mount->m_sb.sb_blocklog;
> - fa->fsx_projid = xfs_get_projid(ip);
> + fa->fsx_projid = ip->i_d.di_projid;
>
> if (attr) {
> if (ip->i_afp) {
> @@ -1569,7 +1569,7 @@ xfs_ioctl_setattr(
> }
>
> if (XFS_IS_QUOTA_RUNNING(mp) && XFS_IS_PQUOTA_ON(mp) &&
> - xfs_get_projid(ip) != fa->fsx_projid) {
> + ip->i_d.di_projid != fa->fsx_projid) {
> code = xfs_qm_vop_chown_reserve(tp, ip, udqp, NULL, pdqp,
> capable(CAP_FOWNER) ? XFS_QMOPT_FORCE_RES : 0);
> if (code) /* out of quota */
> @@ -1606,13 +1606,13 @@ xfs_ioctl_setattr(
> VFS_I(ip)->i_mode &= ~(S_ISUID|S_ISGID);
>
> /* Change the ownerships and register project quota modifications */
> - if (xfs_get_projid(ip) != fa->fsx_projid) {
> + if (ip->i_d.di_projid != fa->fsx_projid) {
> if (XFS_IS_QUOTA_RUNNING(mp) && XFS_IS_PQUOTA_ON(mp)) {
> olddquot = xfs_qm_vop_chown(tp, ip,
> &ip->i_pdquot, pdqp);
> }
> ASSERT(ip->i_d.di_version > 1);
> - xfs_set_projid(ip, fa->fsx_projid);
> + ip->i_d.di_projid = fa->fsx_projid;
> }
>
> /*
> diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
> index 47d8cdb86e5c..c71c34798654 100644
> --- a/fs/xfs/xfs_iops.c
> +++ b/fs/xfs/xfs_iops.c
> @@ -663,7 +663,7 @@ xfs_setattr_nonsize(
> ASSERT(gdqp == NULL);
> error = xfs_qm_vop_dqalloc(ip, xfs_kuid_to_uid(uid),
> xfs_kgid_to_gid(gid),
> - xfs_get_projid(ip),
> + ip->i_d.di_projid,
> qflags, &udqp, &gdqp, NULL);
> if (error)
> return error;
> diff --git a/fs/xfs/xfs_itable.c b/fs/xfs/xfs_itable.c
> index 11771112a634..4b31c29b7e6b 100644
> --- a/fs/xfs/xfs_itable.c
> +++ b/fs/xfs/xfs_itable.c
> @@ -84,7 +84,7 @@ xfs_bulkstat_one_int(
> /* xfs_iget returns the following without needing
> * further change.
> */
> - buf->bs_projectid = xfs_get_projid(ip);
> + buf->bs_projectid = ip->i_d.di_projid;
> buf->bs_ino = ino;
> buf->bs_uid = dic->di_uid;
> buf->bs_gid = dic->di_gid;
> diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
> index ecd8ce152ab1..168f4ae4bdb8 100644
> --- a/fs/xfs/xfs_qm.c
> +++ b/fs/xfs/xfs_qm.c
> @@ -341,7 +341,7 @@ xfs_qm_dqattach_locked(
> }
>
> if (XFS_IS_PQUOTA_ON(mp) && !ip->i_pdquot) {
> - error = xfs_qm_dqattach_one(ip, xfs_get_projid(ip), XFS_DQ_PROJ,
> + error = xfs_qm_dqattach_one(ip, ip->i_d.di_projid, XFS_DQ_PROJ,
> doalloc, &ip->i_pdquot);
> if (error)
> goto done;
> @@ -1693,7 +1693,7 @@ xfs_qm_vop_dqalloc(
> }
> }
> if ((flags & XFS_QMOPT_PQUOTA) && XFS_IS_PQUOTA_ON(mp)) {
> - if (xfs_get_projid(ip) != prid) {
> + if (ip->i_d.di_projid != prid) {
> xfs_iunlock(ip, lockflags);
> error = xfs_qm_dqget(mp, (xfs_dqid_t)prid, XFS_DQ_PROJ,
> true, &pq);
> @@ -1827,7 +1827,7 @@ xfs_qm_vop_chown_reserve(
> }
>
> if (XFS_IS_PQUOTA_ON(ip->i_mount) && pdqp &&
> - xfs_get_projid(ip) != be32_to_cpu(pdqp->q_core.d_id)) {
> + ip->i_d.di_projid != be32_to_cpu(pdqp->q_core.d_id)) {
> prjflags = XFS_QMOPT_ENOSPC;
> pdq_delblks = pdqp;
> if (delblks) {
> @@ -1928,7 +1928,7 @@ xfs_qm_vop_create_dqattach(
> }
> if (pdqp && XFS_IS_PQUOTA_ON(mp)) {
> ASSERT(ip->i_pdquot == NULL);
> - ASSERT(xfs_get_projid(ip) == be32_to_cpu(pdqp->q_core.d_id));
> + ASSERT(ip->i_d.di_projid == be32_to_cpu(pdqp->q_core.d_id));
>
> ip->i_pdquot = xfs_qm_dqhold(pdqp);
> xfs_trans_mod_dquot(tp, pdqp, XFS_TRANS_DQ_ICOUNT, 1);
> diff --git a/fs/xfs/xfs_qm_bhv.c b/fs/xfs/xfs_qm_bhv.c
> index 5d72e88598b4..ac8885d0f752 100644
> --- a/fs/xfs/xfs_qm_bhv.c
> +++ b/fs/xfs/xfs_qm_bhv.c
> @@ -60,7 +60,7 @@ xfs_qm_statvfs(
> xfs_mount_t *mp = ip->i_mount;
> xfs_dquot_t *dqp;
>
> - if (!xfs_qm_dqget(mp, xfs_get_projid(ip), XFS_DQ_PROJ, false, &dqp)) {
> + if (!xfs_qm_dqget(mp, ip->i_d.di_projid, XFS_DQ_PROJ, false, &dqp)) {
> xfs_fill_statvfs_from_dquot(statp, dqp);
> xfs_qm_dqput(dqp);
> }
> --
> 2.20.1
>
next prev parent reply other threads:[~2019-11-12 16:23 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-10-20 8:21 xfs inode structure cleanups Christoph Hellwig
2019-10-20 8:21 ` [PATCH 1/4] xfs: use a struct timespec64 for the in-core crtime Christoph Hellwig
2019-11-12 16:20 ` Darrick J. Wong
2019-10-20 8:21 ` [PATCH 2/4] xfs: merge the projid fields in struct xfs_icdinode Christoph Hellwig
2019-11-12 16:22 ` Darrick J. Wong [this message]
2019-10-20 8:21 ` [PATCH 3/4] xfs: don't reset the "inode core" in xfs_iread Christoph Hellwig
2019-11-12 16:24 ` Darrick J. Wong
2019-11-12 16:25 ` Christoph Hellwig
2019-10-20 8:21 ` [PATCH 4/4] xfs: remove struct xfs_icdinode Christoph Hellwig
2019-10-20 23:29 ` Dave Chinner
2019-10-23 1:28 ` Christoph Hellwig
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=20191112162244.GY6219@magnolia \
--to=darrick.wong@oracle.com \
--cc=hch@lst.de \
--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.