From: "Darrick J. Wong" <djwong@kernel.org>
To: Christoph Hellwig <hch@lst.de>
Cc: linux-xfs@vger.kernel.org
Subject: Re: [PATCH 05/20] xfs: remove the di_dmevmask and di_dmstate fields from struct xfs_icdinode
Date: Tue, 30 Mar 2021 08:28:11 -0700 [thread overview]
Message-ID: <20210330152811.GQ4090233@magnolia> (raw)
In-Reply-To: <20210329053829.1851318-6-hch@lst.de>
On Mon, Mar 29, 2021 at 07:38:14AM +0200, Christoph Hellwig wrote:
> The legacy DMAPI fields were never set by upstream Linux XFS, and have no
> way to be read using the kernel APIs. So instead of bloating the in-core
> inode for them just copy them from the on-disk inode into the log when
> logging the inode. The only caveat is that we need to make sure to zero
> the fields for newly read or deleted inodes, which is solved using a new
> flag in the inode.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Do you want to put dmevmask/dmstate preservation on the deprecated list
too, seeing as the only (out of tree) user wrote in on the last
submission to say that they're not supporting it past 2022 and have
never supported mounting those XFSes with upstream kernels?
Granted, the difficulty there is how do you deprecate something that
isn't there? 8)
Anyway this series got through the first and second rounds of fstesting
so I guess it's good to go.
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
--D
> ---
> fs/xfs/libxfs/xfs_inode_buf.c | 7 +++----
> fs/xfs/libxfs/xfs_inode_buf.h | 2 --
> fs/xfs/xfs_inode.c | 5 ++---
> fs/xfs/xfs_inode.h | 1 +
> fs/xfs/xfs_inode_item.c | 31 +++++++++++++++++++++++++++++--
> fs/xfs/xfs_log_recover.c | 6 ------
> 6 files changed, 35 insertions(+), 17 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_inode_buf.c b/fs/xfs/libxfs/xfs_inode_buf.c
> index af5ee8bd7e6ac9..917a41828ffbe3 100644
> --- a/fs/xfs/libxfs/xfs_inode_buf.c
> +++ b/fs/xfs/libxfs/xfs_inode_buf.c
> @@ -230,10 +230,11 @@ xfs_inode_from_disk(
> to->di_nblocks = be64_to_cpu(from->di_nblocks);
> to->di_extsize = be32_to_cpu(from->di_extsize);
> to->di_forkoff = from->di_forkoff;
> - to->di_dmevmask = be32_to_cpu(from->di_dmevmask);
> - to->di_dmstate = be16_to_cpu(from->di_dmstate);
> to->di_flags = be16_to_cpu(from->di_flags);
>
> + if (from->di_dmevmask || from->di_dmstate)
> + xfs_iflags_set(ip, XFS_IPRESERVE_DM_FIELDS);
> +
> if (xfs_sb_version_has_v3inode(&ip->i_mount->m_sb)) {
> inode_set_iversion_queried(inode,
> be64_to_cpu(from->di_changecount));
> @@ -311,8 +312,6 @@ xfs_inode_to_disk(
> to->di_anextents = cpu_to_be16(xfs_ifork_nextents(ip->i_afp));
> to->di_forkoff = from->di_forkoff;
> to->di_aformat = xfs_ifork_format(ip->i_afp);
> - to->di_dmevmask = cpu_to_be32(from->di_dmevmask);
> - to->di_dmstate = cpu_to_be16(from->di_dmstate);
> to->di_flags = cpu_to_be16(from->di_flags);
>
> if (xfs_sb_version_has_v3inode(&ip->i_mount->m_sb)) {
> diff --git a/fs/xfs/libxfs/xfs_inode_buf.h b/fs/xfs/libxfs/xfs_inode_buf.h
> index b3097ea8b53366..d7a019df05d647 100644
> --- a/fs/xfs/libxfs/xfs_inode_buf.h
> +++ b/fs/xfs/libxfs/xfs_inode_buf.h
> @@ -22,8 +22,6 @@ struct xfs_icdinode {
> xfs_rfsblock_t di_nblocks; /* # of direct & btree blocks used */
> xfs_extlen_t di_extsize; /* basic/minimum extent size for file */
> uint8_t di_forkoff; /* attr fork offs, <<3 for 64b align */
> - uint32_t di_dmevmask; /* DMIG event mask */
> - uint16_t di_dmstate; /* DMIG state info */
> uint16_t di_flags; /* random flags, XFS_DIFLAG_... */
>
> uint64_t di_flags2; /* more random flags */
> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> index 8c164fe621548b..9c8fd11a53622a 100644
> --- a/fs/xfs/xfs_inode.c
> +++ b/fs/xfs/xfs_inode.c
> @@ -840,8 +840,6 @@ xfs_init_new_inode(
> inode->i_ctime = tv;
>
> ip->i_d.di_extsize = 0;
> - ip->i_d.di_dmevmask = 0;
> - ip->i_d.di_dmstate = 0;
> ip->i_d.di_flags = 0;
>
> if (xfs_sb_version_has_v3inode(&mp->m_sb)) {
> @@ -2615,9 +2613,10 @@ xfs_ifree(
> VFS_I(ip)->i_mode = 0; /* mark incore inode as free */
> ip->i_d.di_flags = 0;
> ip->i_d.di_flags2 = ip->i_mount->m_ino_geo.new_diflags2;
> - ip->i_d.di_dmevmask = 0;
> ip->i_d.di_forkoff = 0; /* mark the attr fork not in use */
> ip->i_df.if_format = XFS_DINODE_FMT_EXTENTS;
> + if (xfs_iflags_test(ip, XFS_IPRESERVE_DM_FIELDS))
> + xfs_iflags_clear(ip, XFS_IPRESERVE_DM_FIELDS);
>
> /* Don't attempt to replay owner changes for a deleted inode */
> spin_lock(&iip->ili_lock);
> diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
> index 13f4cd2e1f4fc5..d3369c0bcd9de7 100644
> --- a/fs/xfs/xfs_inode.h
> +++ b/fs/xfs/xfs_inode.h
> @@ -220,6 +220,7 @@ static inline bool xfs_inode_has_bigtime(struct xfs_inode *ip)
> #define XFS_IRECLAIM (1 << 0) /* started reclaiming this inode */
> #define XFS_ISTALE (1 << 1) /* inode has been staled */
> #define XFS_IRECLAIMABLE (1 << 2) /* inode can be reclaimed */
> +#define XFS_IPRESERVE_DM_FIELDS (1 << 4) /* has legacy DMAPI fields set */
> #define __XFS_INEW_BIT 3 /* inode has just been allocated */
> #define XFS_INEW (1 << __XFS_INEW_BIT)
> #define XFS_ITRUNCATED (1 << 5) /* truncated down so flush-on-close */
> diff --git a/fs/xfs/xfs_inode_item.c b/fs/xfs/xfs_inode_item.c
> index 17e20a6d8b4e27..16e758a689217e 100644
> --- a/fs/xfs/xfs_inode_item.c
> +++ b/fs/xfs/xfs_inode_item.c
> @@ -317,6 +317,33 @@ xfs_inode_to_log_dinode_ts(
> return its;
> }
>
> +/*
> + * The legacy DMAPI fields are only present in the on-disk and in-log inodes,
> + * but not in the in-memory one. But we are guaranteed to have an inode buffer
> + * in memory when logging an inode, so we can just copy it from the on-disk
> + * inode to the in-log inode here so that recovery of file system with these
> + * fields set to non-zero values doesn't lose them. For all other cases we zero
> + * the fields.
> + */
> +static void
> +xfs_copy_dm_fields_to_log_dinode(
> + struct xfs_inode *ip,
> + struct xfs_log_dinode *to)
> +{
> + struct xfs_dinode *dip;
> +
> + dip = xfs_buf_offset(ip->i_itemp->ili_item.li_buf,
> + ip->i_imap.im_boffset);
> +
> + if (xfs_iflags_test(ip, XFS_IPRESERVE_DM_FIELDS)) {
> + to->di_dmevmask = be32_to_cpu(dip->di_dmevmask);
> + to->di_dmstate = be16_to_cpu(dip->di_dmstate);
> + } else {
> + to->di_dmevmask = 0;
> + to->di_dmstate = 0;
> + }
> +}
> +
> static void
> xfs_inode_to_log_dinode(
> struct xfs_inode *ip,
> @@ -349,10 +376,10 @@ xfs_inode_to_log_dinode(
> to->di_anextents = xfs_ifork_nextents(ip->i_afp);
> to->di_forkoff = from->di_forkoff;
> to->di_aformat = xfs_ifork_format(ip->i_afp);
> - to->di_dmevmask = from->di_dmevmask;
> - to->di_dmstate = from->di_dmstate;
> to->di_flags = from->di_flags;
>
> + xfs_copy_dm_fields_to_log_dinode(ip, to);
> +
> /* log a dummy value to ensure log structure is fully initialised */
> to->di_next_unlinked = NULLAGINO;
>
> diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
> index 42a8c7da492aa8..e5dd1c0c2f03c4 100644
> --- a/fs/xfs/xfs_log_recover.c
> +++ b/fs/xfs/xfs_log_recover.c
> @@ -2696,12 +2696,6 @@ xlog_recover_process_one_iunlink(
> agino = be32_to_cpu(dip->di_next_unlinked);
> xfs_buf_relse(ibp);
>
> - /*
> - * Prevent any DMAPI event from being sent when the reference on
> - * the inode is dropped.
> - */
> - ip->i_d.di_dmevmask = 0;
> -
> xfs_irele(ip);
> return agino;
>
> --
> 2.30.1
>
next prev parent reply other threads:[~2021-03-30 15:29 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-03-29 5:38 xfs inode structure cleanups v4 Christoph Hellwig
2021-03-29 5:38 ` [PATCH 01/20] xfs: split xfs_imap_to_bp Christoph Hellwig
2021-03-29 5:38 ` [PATCH 02/20] xfs: consistently initialize di_flags2 Christoph Hellwig
2021-03-29 5:38 ` [PATCH 03/20] xfs: handle crtime more carefully in xfs_bulkstat_one_int Christoph Hellwig
2021-03-29 5:38 ` [PATCH 04/20] xfs: remove the unused xfs_icdinode_has_bigtime helper Christoph Hellwig
2021-03-29 5:38 ` [PATCH 05/20] xfs: remove the di_dmevmask and di_dmstate fields from struct xfs_icdinode Christoph Hellwig
2021-03-30 15:28 ` Darrick J. Wong [this message]
2021-03-30 17:33 ` Christoph Hellwig
2021-03-29 5:38 ` [PATCH 06/20] xfs: don't clear the "dinode core" in xfs_inode_alloc Christoph Hellwig
2021-03-29 5:38 ` [PATCH 07/20] xfs: move the di_projid field to struct xfs_inode Christoph Hellwig
2021-03-30 15:20 ` Darrick J. Wong
2021-03-29 5:38 ` [PATCH 08/20] xfs: move the di_size " Christoph Hellwig
2021-03-29 5:38 ` [PATCH 09/20] xfs: move the di_nblocks " Christoph Hellwig
2021-03-29 5:38 ` [PATCH 10/20] xfs: move the di_extsize " Christoph Hellwig
2021-03-29 5:38 ` [PATCH 11/20] xfs: move the di_cowextsize " Christoph Hellwig
2021-03-29 5:38 ` [PATCH 12/20] xfs: move the di_flushiter " Christoph Hellwig
2021-03-29 5:38 ` [PATCH 13/20] xfs: cleanup xfs_fill_fsxattr Christoph Hellwig
2021-03-29 5:38 ` [PATCH 14/20] xfs: use XFS_B_TO_FSB in xfs_ioctl_setattr Christoph Hellwig
2021-03-30 15:20 ` Darrick J. Wong
2021-03-29 5:38 ` [PATCH 15/20] xfs: use a union for i_cowextsize and i_flushiter Christoph Hellwig
2021-03-30 15:22 ` Darrick J. Wong
2021-03-29 5:38 ` [PATCH 16/20] xfs: move the di_forkoff field to struct xfs_inode Christoph Hellwig
2021-03-29 5:38 ` [PATCH 17/20] xfs: move the di_flags " Christoph Hellwig
2021-03-29 5:38 ` [PATCH 18/20] xfs: move the di_flags2 " Christoph Hellwig
2021-03-29 18:14 ` Darrick J. Wong
2021-03-29 5:38 ` [PATCH 19/20] xfs: move the di_crtime " Christoph Hellwig
2021-03-29 5:38 ` [PATCH 20/20] xfs: merge _xfs_dic2xflags into xfs_ip2xflags Christoph Hellwig
2021-03-30 15:25 ` Darrick J. Wong
2021-03-30 17:31 ` Christoph Hellwig
2021-03-30 18:06 ` Darrick J. Wong
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=20210330152811.GQ4090233@magnolia \
--to=djwong@kernel.org \
--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 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).