Re: [PATCH 4/4] xfs: remove struct xfs_icdinode

[Christoph Hellwig <hch@lst.de>]

On Mon, Oct 21, 2019 at 10:29:58AM +1100, Dave Chinner wrote:
> This is, IMO, a step backards. We're going to end up failing to
> initialise new fields correctly with this...

How is that different from the plain xfs_inode fields?  In fact I suspect
most of the initializers cn just be removed entirely, so I'll look into
preloading that at the front of the series.

> This is a bug and should make all the 32-bit project ID tests fail.
> If it doesn't them we've got a problem with our test coverage. If it
> does fail, then I'm not sure this patchset has been adequately
> tested...

I don't think we have any coverage of that, at least I didn't see any
extra failures.

> > +	xfs_fsize_t		i_disk_size;	/* number of bytes in file */
> > +	xfs_rfsblock_t		i_nblocks;	/* direct & btree blocks used */
> > +	xfs_extlen_t		i_extsize;	/* extent size hint  */
> > +	xfs_extnum_t		i_nextents;	/* # of extents in data fork */
> > +	xfs_aextnum_t		i_anextents;	/* # of extents in attr fork */
> > +	uint8_t			i_forkoff;	/* attr fork offset */
> > +	int8_t			i_aformat;	/* attr fork format */
> > +	uint32_t		i_dmevmask;	/* DMIG event mask */
> > +	uint16_t		i_dmstate;	/* DMIG state info */
> 
> If we are cleaning up the icdinode, why do these still exist in
> memory?

Because we need them so that we put the right value in the log when
logging the inode core.  Otherwise a log recovery might clear these
values.  The only thing I could do is add a log incompat flag set
on a kernel that removes the field and then not apply changes to these
two fields when recoverying the log on a file system with that flag
set.