[PATCH v2 0/8] xfs: shrink the xfs_icdinode

[PATCH v2 0/8] xfs: shrink the xfs_icdinode

[Dave Chinner]

Hi folks,

This is a followup to the original RFC patchset posted here:

http://oss.sgi.com/archives/xfs/2016-01/msg00366.html

This patchset actually works, and passes through xfstests without
any regressions at all. So feel free to test and comment on this
version.

-Dave.

Version 2
- timestamps are signed, so need casting to int before being stored
  in a 64 bit variable when read from disk. Fixes generic/258
  failure (patch 2).
- nlink wasn't being accounted correctly for O_TMPFILE files and
  rename whiteouts. Fixes generic/078 and others. (patch 5)
- VFS does not initialise i_mode, ever, so we need to initialise it
  directly after slab allocation to ensure that we correctly detect
  new file creates and hence don't add the inode the sb list twice,
  corrupting it. (busy inodes after unmount, self destruct in 5
  seconds, have a nice day!) (patch 8)
- when reallocating a reclaimable inode, we need to save the
  variables that hold on-disk values across the inode_init_always()
  call, otherwise we lose them. This results in corruption of these
  fields. (patch 8)

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 1/8] xfs: introduce inode log format object

[Dave Chinner]

From: Dave Chinner <dchinner@redhat.com>

We currently carry around and log an entire inode core in the
struct xfs_inode. A lot of the information in the inode core is
duplicated in the VFS inode, but we cannot remove this duplication
of infomration because the inode core is logged directly in
xfs_inode_item_format().

Add a new function xfs_inode_item_format_core() that copies the
inode core data into a struct xfs_icdinode that is pulled directly
from the log vector buffer. This means we no longer directly
copy the inode core, but copy the structures one member at a time.
This will be slightly less efficient than copying, but will allow us
to remove duplicate and unnecessary items from the struct xfs_inode.

To enable us to do this, call the new structure a xfs_log_dinode,
so that we know it's different to the physical xfs_dinode and the
in-core xfs_icdinode.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
 fs/xfs/libxfs/xfs_inode_buf.c  |   8 +--
 fs/xfs/libxfs/xfs_inode_buf.h  |  53 ++++++++++++++++-
 fs/xfs/libxfs/xfs_log_format.h |  15 ++---
 fs/xfs/xfs_icache.c            |   2 +-
 fs/xfs/xfs_inode.c             |   6 +-
 fs/xfs/xfs_inode.h             |   2 +-
 fs/xfs/xfs_inode_item.c        | 128 +++++++++++++++++++++++++++++++++++++++--
 fs/xfs/xfs_inode_item.h        |   2 +
 fs/xfs/xfs_log_recover.c       |  52 +++++++++--------
 9 files changed, 221 insertions(+), 47 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_inode_buf.c b/fs/xfs/libxfs/xfs_inode_buf.c
index 1aabfda..63d46bf 100644
--- a/fs/xfs/libxfs/xfs_inode_buf.c
+++ b/fs/xfs/libxfs/xfs_inode_buf.c
@@ -196,8 +196,8 @@ xfs_imap_to_bp(
 
 void
 xfs_dinode_from_disk(
-	xfs_icdinode_t		*to,
-	xfs_dinode_t		*from)
+	struct xfs_icdinode	*to,
+	struct xfs_dinode	*from)
 {
 	to->di_magic = be16_to_cpu(from->di_magic);
 	to->di_mode = be16_to_cpu(from->di_mode);
@@ -243,8 +243,8 @@ xfs_dinode_from_disk(
 
 void
 xfs_dinode_to_disk(
-	xfs_dinode_t		*to,
-	xfs_icdinode_t		*from)
+	struct xfs_dinode	*to,
+	struct xfs_icdinode	*from)
 {
 	to->di_magic = cpu_to_be16(from->di_magic);
 	to->di_mode = cpu_to_be16(from->di_mode);
diff --git a/fs/xfs/libxfs/xfs_inode_buf.h b/fs/xfs/libxfs/xfs_inode_buf.h
index 9308c47..642f2a2 100644
--- a/fs/xfs/libxfs/xfs_inode_buf.h
+++ b/fs/xfs/libxfs/xfs_inode_buf.h
@@ -20,7 +20,58 @@
 
 struct xfs_inode;
 struct xfs_dinode;
-struct xfs_icdinode;
+
+/*
+ * In memory representation of the XFS inode. This is held in the in-core
+ * struct xfs_inode to represent the on disk values, but no longer needs to be
+ * identical to the on-disk structure as it is always translated to on-disk
+ * format specific structures at the appropriate time.
+ */
+struct xfs_icdinode {
+	__uint16_t	di_magic;	/* inode magic # = XFS_DINODE_MAGIC */
+	__uint16_t	di_mode;	/* mode and type of file */
+	__int8_t	di_version;	/* inode version */
+	__int8_t	di_format;	/* format of di_c data */
+	__uint16_t	di_onlink;	/* old number of links to file */
+	__uint32_t	di_uid;		/* owner's user id */
+	__uint32_t	di_gid;		/* owner's group id */
+	__uint32_t	di_nlink;	/* number of links to file */
+	__uint16_t	di_projid_lo;	/* lower part of owner's project id */
+	__uint16_t	di_projid_hi;	/* higher part of owner's project id */
+	__uint8_t	di_pad[6];	/* unused, zeroed space */
+	__uint16_t	di_flushiter;	/* incremented on flush */
+	xfs_ictimestamp_t di_atime;	/* time last accessed */
+	xfs_ictimestamp_t di_mtime;	/* time last modified */
+	xfs_ictimestamp_t di_ctime;	/* time created/inode modified */
+	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 */
+	xfs_extnum_t	di_nextents;	/* number of extents in data fork */
+	xfs_aextnum_t	di_anextents;	/* number of extents in attribute fork*/
+	__uint8_t	di_forkoff;	/* attr fork offs, <<3 for 64b align */
+	__int8_t	di_aformat;	/* format of attr fork's data */
+	__uint32_t	di_dmevmask;	/* DMIG event mask */
+	__uint16_t	di_dmstate;	/* DMIG state info */
+	__uint16_t	di_flags;	/* random flags, XFS_DIFLAG_... */
+	__uint32_t	di_gen;		/* generation number */
+
+	/* di_next_unlinked is the only non-core field in the old dinode */
+	xfs_agino_t	di_next_unlinked;/* agi unlinked list ptr */
+
+	/* start of the extended dinode, writable fields */
+	__uint32_t	di_crc;		/* CRC of the inode */
+	__uint64_t	di_changecount;	/* number of attribute changes */
+	xfs_lsn_t	di_lsn;		/* flush sequence */
+	__uint64_t	di_flags2;	/* more random flags */
+	__uint8_t	di_pad2[16];	/* more padding for future expansion */
+
+	/* fields only written to during inode creation */
+	xfs_ictimestamp_t di_crtime;	/* time created */
+	xfs_ino_t	di_ino;		/* inode number */
+	uuid_t		di_uuid;	/* UUID of the filesystem */
+
+	/* structure must be padded to 64 bit alignment */
+};
 
 /*
  * Inode location information.  Stored in the inode and passed to
diff --git a/fs/xfs/libxfs/xfs_log_format.h b/fs/xfs/libxfs/xfs_log_format.h
index 2653146..d00ed63 100644
--- a/fs/xfs/libxfs/xfs_log_format.h
+++ b/fs/xfs/libxfs/xfs_log_format.h
@@ -290,6 +290,7 @@ typedef struct xfs_inode_log_format_64 {
 	__int32_t		ilf_boffset;	/* off of inode in buffer */
 } xfs_inode_log_format_64_t;
 
+
 /*
  * Flags for xfs_trans_log_inode flags field.
  */
@@ -360,10 +361,10 @@ typedef struct xfs_ictimestamp {
 } xfs_ictimestamp_t;
 
 /*
- * NOTE:  This structure must be kept identical to struct xfs_dinode
- *	  except for the endianness annotations.
+ * Define the format of the inode core that is logged. This structure must be
+ * kept identical to struct xfs_dinode except for the endianness annotations.
  */
-typedef struct xfs_icdinode {
+struct xfs_log_dinode {
 	__uint16_t	di_magic;	/* inode magic # = XFS_DINODE_MAGIC */
 	__uint16_t	di_mode;	/* mode and type of file */
 	__int8_t	di_version;	/* inode version */
@@ -407,13 +408,13 @@ typedef struct xfs_icdinode {
 	uuid_t		di_uuid;	/* UUID of the filesystem */
 
 	/* structure must be padded to 64 bit alignment */
-} xfs_icdinode_t;
+};
 
-static inline uint xfs_icdinode_size(int version)
+static inline uint xfs_log_dinode_size(int version)
 {
 	if (version == 3)
-		return sizeof(struct xfs_icdinode);
-	return offsetof(struct xfs_icdinode, di_next_unlinked);
+		return sizeof(struct xfs_log_dinode);
+	return offsetof(struct xfs_log_dinode, di_next_unlinked);
 }
 
 /*
diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c
index d7a490f..7c26f86 100644
--- a/fs/xfs/xfs_icache.c
+++ b/fs/xfs/xfs_icache.c
@@ -79,7 +79,7 @@ xfs_inode_alloc(
 	memset(&ip->i_df, 0, sizeof(xfs_ifork_t));
 	ip->i_flags = 0;
 	ip->i_delayed_blks = 0;
-	memset(&ip->i_d, 0, sizeof(xfs_icdinode_t));
+	memset(&ip->i_d, 0, sizeof(ip->i_d));
 
 	return ip;
 }
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index ae3758a..7e24232 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -650,9 +650,9 @@ _xfs_dic2xflags(
 
 uint
 xfs_ip2xflags(
-	xfs_inode_t		*ip)
+	struct xfs_inode	*ip)
 {
-	xfs_icdinode_t		*dic = &ip->i_d;
+	struct xfs_icdinode	*dic = &ip->i_d;
 
 	return _xfs_dic2xflags(dic->di_flags) |
 				(XFS_IFORK_Q(ip) ? XFS_XFLAG_HASATTR : 0);
@@ -660,7 +660,7 @@ xfs_ip2xflags(
 
 uint
 xfs_dic2xflags(
-	xfs_dinode_t		*dip)
+	struct xfs_dinode	*dip)
 {
 	return _xfs_dic2xflags(be16_to_cpu(dip->di_flags)) |
 				(XFS_DFORK_Q(dip) ? XFS_XFLAG_HASATTR : 0);
diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
index ca9e119..aef5452 100644
--- a/fs/xfs/xfs_inode.h
+++ b/fs/xfs/xfs_inode.h
@@ -63,7 +63,7 @@ typedef struct xfs_inode {
 	unsigned long		i_flags;	/* see defined flags below */
 	unsigned int		i_delayed_blks;	/* count of delay alloc blks */
 
-	xfs_icdinode_t		i_d;		/* most of ondisk inode */
+	struct xfs_icdinode	i_d;		/* most of ondisk inode */
 
 	/* VFS inode */
 	struct inode		i_vnode;	/* embedded VFS inode */
diff --git a/fs/xfs/xfs_inode_item.c b/fs/xfs/xfs_inode_item.c
index d14b12b..3ad9972 100644
--- a/fs/xfs/xfs_inode_item.c
+++ b/fs/xfs/xfs_inode_item.c
@@ -135,7 +135,7 @@ xfs_inode_item_size(
 
 	*nvecs += 2;
 	*nbytes += sizeof(struct xfs_inode_log_format) +
-		   xfs_icdinode_size(ip->i_d.di_version);
+		   xfs_log_dinode_size(ip->i_d.di_version);
 
 	xfs_inode_item_data_fork_size(iip, nvecs, nbytes);
 	if (XFS_IFORK_Q(ip))
@@ -322,6 +322,127 @@ xfs_inode_item_format_attr_fork(
 	}
 }
 
+static void
+xfs_icdinode_to_log_dinode(
+	struct xfs_icdinode	*from,
+	struct xfs_log_dinode	*to)
+{
+	to->di_magic = from->di_magic;
+	to->di_mode = from->di_mode;
+	to->di_version = from->di_version;
+	to->di_format = from->di_format;
+	to->di_onlink = from->di_onlink;
+	to->di_uid = from->di_uid;
+	to->di_gid = from->di_gid;
+	to->di_nlink = from->di_nlink;
+	to->di_projid_lo = from->di_projid_lo;
+	to->di_projid_hi = from->di_projid_hi;
+	memcpy(to->di_pad, from->di_pad, sizeof(to->di_pad));
+	to->di_atime.t_sec = from->di_atime.t_sec;
+	to->di_atime.t_nsec = from->di_atime.t_nsec;
+	to->di_mtime.t_sec = from->di_mtime.t_sec;
+	to->di_mtime.t_nsec = from->di_mtime.t_nsec;
+	to->di_ctime.t_sec = from->di_ctime.t_sec;
+	to->di_ctime.t_nsec = from->di_ctime.t_nsec;
+	to->di_size = from->di_size;
+	to->di_nblocks = from->di_nblocks;
+	to->di_extsize = from->di_extsize;
+	to->di_nextents = from->di_nextents;
+	to->di_anextents = from->di_anextents;
+	to->di_forkoff = from->di_forkoff;
+	to->di_aformat = from->di_aformat;
+	to->di_dmevmask = from->di_dmevmask;
+	to->di_dmstate = from->di_dmstate;
+	to->di_flags = from->di_flags;
+	to->di_gen = from->di_gen;
+
+	if (from->di_version == 3) {
+		to->di_changecount = from->di_changecount;
+		to->di_crtime.t_sec = from->di_crtime.t_sec;
+		to->di_crtime.t_nsec = from->di_crtime.t_nsec;
+		to->di_flags2 = from->di_flags2;
+		to->di_ino = from->di_ino;
+		to->di_lsn = from->di_lsn;
+		memcpy(to->di_pad2, from->di_pad2, sizeof(to->di_pad2));
+		uuid_copy(&to->di_uuid, &from->di_uuid);
+		to->di_flushiter = 0;
+	} else {
+		to->di_flushiter = from->di_flushiter;
+	}
+}
+
+/*
+ * Recovery needs to be able to convert a log dinode back to a real dinode
+ * for writeback we do that by converting a log dinode to a icdinode, and
+ * then passing that to the formatting function.
+ */
+void
+xfs_log_dinode_to_icdinode(
+	struct xfs_log_dinode	*from,
+	struct xfs_icdinode	*to)
+{
+	to->di_magic = from->di_magic;
+	to->di_mode = from->di_mode;
+	to->di_version = from->di_version;
+	to->di_format = from->di_format;
+	to->di_onlink = from->di_onlink;
+	to->di_uid = from->di_uid;
+	to->di_gid = from->di_gid;
+	to->di_nlink = from->di_nlink;
+	to->di_projid_lo = from->di_projid_lo;
+	to->di_projid_hi = from->di_projid_hi;
+	memset(to->di_pad, 0, sizeof(to->di_pad));
+	to->di_atime.t_sec = from->di_atime.t_sec;
+	to->di_atime.t_nsec = from->di_atime.t_nsec;
+	to->di_mtime.t_sec = from->di_mtime.t_sec;
+	to->di_mtime.t_nsec = from->di_mtime.t_nsec;
+	to->di_ctime.t_sec = from->di_ctime.t_sec;
+	to->di_ctime.t_nsec = from->di_ctime.t_nsec;
+	to->di_size = from->di_size;
+	to->di_nblocks = from->di_nblocks;
+	to->di_extsize = from->di_extsize;
+	to->di_nextents = from->di_nextents;
+	to->di_anextents = from->di_anextents;
+	to->di_forkoff = from->di_forkoff;
+	to->di_aformat = from->di_aformat;
+	to->di_dmevmask = from->di_dmevmask;
+	to->di_dmstate = from->di_dmstate;
+	to->di_flags = from->di_flags;
+	to->di_gen = from->di_gen;
+
+	if (from->di_version == 3) {
+		to->di_changecount = from->di_changecount;
+		to->di_crtime.t_sec = from->di_crtime.t_sec;
+		to->di_crtime.t_nsec = from->di_crtime.t_nsec;
+		to->di_flags2 = from->di_flags2;
+		to->di_ino = from->di_ino;
+		to->di_lsn = from->di_lsn;
+		memcpy(to->di_pad2, from->di_pad2, sizeof(to->di_pad2));
+		uuid_copy(&to->di_uuid, &from->di_uuid);
+		to->di_flushiter = 0;
+	} else {
+		to->di_flushiter = from->di_flushiter;
+	}
+}
+
+/*
+ * Format the inode core. Current timestamp data is only in the VFS inode
+ * fields, so we need to grab them from there. Hence rather than just copying
+ * the XFS inode core structure, format the fields directly into the iovec.
+ */
+static void
+xfs_inode_item_format_core(
+	struct xfs_inode	*ip,
+	struct xfs_log_vec	*lv,
+	struct xfs_log_iovec	**vecp)
+{
+	struct xfs_log_dinode	*dic;
+
+	dic = xlog_prepare_iovec(lv, vecp, XLOG_REG_TYPE_ICORE);
+	xfs_icdinode_to_log_dinode(&ip->i_d, dic);
+	xlog_finish_iovec(lv, *vecp, xfs_log_dinode_size(ip->i_d.di_version));
+}
+
 /*
  * This is called to fill in the vector of log iovecs for the given inode
  * log item.  It fills the first item with an inode log format structure,
@@ -351,10 +472,7 @@ xfs_inode_item_format(
 	ilf->ilf_size = 2; /* format + core */
 	xlog_finish_iovec(lv, vecp, sizeof(struct xfs_inode_log_format));
 
-	xlog_copy_iovec(lv, &vecp, XLOG_REG_TYPE_ICORE,
-			&ip->i_d,
-			xfs_icdinode_size(ip->i_d.di_version));
-
+	xfs_inode_item_format_core(ip, lv, &vecp);
 	xfs_inode_item_format_data_fork(iip, ilf, lv, &vecp);
 	if (XFS_IFORK_Q(ip)) {
 		xfs_inode_item_format_attr_fork(iip, ilf, lv, &vecp);
diff --git a/fs/xfs/xfs_inode_item.h b/fs/xfs/xfs_inode_item.h
index 4c7722e..2426118 100644
--- a/fs/xfs/xfs_inode_item.h
+++ b/fs/xfs/xfs_inode_item.h
@@ -49,6 +49,8 @@ extern void xfs_istale_done(struct xfs_buf *, struct xfs_log_item *);
 extern void xfs_iflush_abort(struct xfs_inode *, bool);
 extern int xfs_inode_item_format_convert(xfs_log_iovec_t *,
 					 xfs_inode_log_format_t *);
+extern void xfs_log_dinode_to_icdinode(struct xfs_log_dinode *from,
+				       struct xfs_icdinode *to);
 
 extern struct kmem_zone	*xfs_ili_zone;
 
diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
index da37beb..3120f7b 100644
--- a/fs/xfs/xfs_log_recover.c
+++ b/fs/xfs/xfs_log_recover.c
@@ -2839,7 +2839,8 @@ xlog_recover_inode_pass2(
 	int			error;
 	int			attr_index;
 	uint			fields;
-	xfs_icdinode_t		*dicp;
+	struct xfs_log_dinode	*ldip;
+	struct xfs_icdinode	icic;
 	uint			isize;
 	int			need_free = 0;
 
@@ -2892,8 +2893,8 @@ xlog_recover_inode_pass2(
 		error = -EFSCORRUPTED;
 		goto out_release;
 	}
-	dicp = item->ri_buf[1].i_addr;
-	if (unlikely(dicp->di_magic != XFS_DINODE_MAGIC)) {
+	ldip = item->ri_buf[1].i_addr;
+	if (unlikely(ldip->di_magic != XFS_DINODE_MAGIC)) {
 		xfs_alert(mp,
 			"%s: Bad inode log record, rec ptr 0x%p, ino %Ld",
 			__func__, item, in_f->ilf_ino);
@@ -2929,13 +2930,13 @@ xlog_recover_inode_pass2(
 	 * to skip replay when the on disk inode is newer than the log one
 	 */
 	if (!xfs_sb_version_hascrc(&mp->m_sb) &&
-	    dicp->di_flushiter < be16_to_cpu(dip->di_flushiter)) {
+	    ldip->di_flushiter < be16_to_cpu(dip->di_flushiter)) {
 		/*
 		 * Deal with the wrap case, DI_MAX_FLUSH is less
 		 * than smaller numbers
 		 */
 		if (be16_to_cpu(dip->di_flushiter) == DI_MAX_FLUSH &&
-		    dicp->di_flushiter < (DI_MAX_FLUSH >> 1)) {
+		    ldip->di_flushiter < (DI_MAX_FLUSH >> 1)) {
 			/* do nothing */
 		} else {
 			trace_xfs_log_recover_inode_skip(log, in_f);
@@ -2945,13 +2946,13 @@ xlog_recover_inode_pass2(
 	}
 
 	/* Take the opportunity to reset the flush iteration count */
-	dicp->di_flushiter = 0;
+	ldip->di_flushiter = 0;
 
-	if (unlikely(S_ISREG(dicp->di_mode))) {
-		if ((dicp->di_format != XFS_DINODE_FMT_EXTENTS) &&
-		    (dicp->di_format != XFS_DINODE_FMT_BTREE)) {
+	if (unlikely(S_ISREG(ldip->di_mode))) {
+		if ((ldip->di_format != XFS_DINODE_FMT_EXTENTS) &&
+		    (ldip->di_format != XFS_DINODE_FMT_BTREE)) {
 			XFS_CORRUPTION_ERROR("xlog_recover_inode_pass2(3)",
-					 XFS_ERRLEVEL_LOW, mp, dicp);
+					 XFS_ERRLEVEL_LOW, mp, ldip);
 			xfs_alert(mp,
 		"%s: Bad regular inode log record, rec ptr 0x%p, "
 		"ino ptr = 0x%p, ino bp = 0x%p, ino %Ld",
@@ -2959,12 +2960,12 @@ xlog_recover_inode_pass2(
 			error = -EFSCORRUPTED;
 			goto out_release;
 		}
-	} else if (unlikely(S_ISDIR(dicp->di_mode))) {
-		if ((dicp->di_format != XFS_DINODE_FMT_EXTENTS) &&
-		    (dicp->di_format != XFS_DINODE_FMT_BTREE) &&
-		    (dicp->di_format != XFS_DINODE_FMT_LOCAL)) {
+	} else if (unlikely(S_ISDIR(ldip->di_mode))) {
+		if ((ldip->di_format != XFS_DINODE_FMT_EXTENTS) &&
+		    (ldip->di_format != XFS_DINODE_FMT_BTREE) &&
+		    (ldip->di_format != XFS_DINODE_FMT_LOCAL)) {
 			XFS_CORRUPTION_ERROR("xlog_recover_inode_pass2(4)",
-					     XFS_ERRLEVEL_LOW, mp, dicp);
+					     XFS_ERRLEVEL_LOW, mp, ldip);
 			xfs_alert(mp,
 		"%s: Bad dir inode log record, rec ptr 0x%p, "
 		"ino ptr = 0x%p, ino bp = 0x%p, ino %Ld",
@@ -2973,32 +2974,32 @@ xlog_recover_inode_pass2(
 			goto out_release;
 		}
 	}
-	if (unlikely(dicp->di_nextents + dicp->di_anextents > dicp->di_nblocks)){
+	if (unlikely(ldip->di_nextents + ldip->di_anextents > ldip->di_nblocks)){
 		XFS_CORRUPTION_ERROR("xlog_recover_inode_pass2(5)",
-				     XFS_ERRLEVEL_LOW, mp, dicp);
+				     XFS_ERRLEVEL_LOW, mp, ldip);
 		xfs_alert(mp,
 	"%s: Bad inode log record, rec ptr 0x%p, dino ptr 0x%p, "
 	"dino bp 0x%p, ino %Ld, total extents = %d, nblocks = %Ld",
 			__func__, item, dip, bp, in_f->ilf_ino,
-			dicp->di_nextents + dicp->di_anextents,
-			dicp->di_nblocks);
+			ldip->di_nextents + ldip->di_anextents,
+			ldip->di_nblocks);
 		error = -EFSCORRUPTED;
 		goto out_release;
 	}
-	if (unlikely(dicp->di_forkoff > mp->m_sb.sb_inodesize)) {
+	if (unlikely(ldip->di_forkoff > mp->m_sb.sb_inodesize)) {
 		XFS_CORRUPTION_ERROR("xlog_recover_inode_pass2(6)",
-				     XFS_ERRLEVEL_LOW, mp, dicp);
+				     XFS_ERRLEVEL_LOW, mp, ldip);
 		xfs_alert(mp,
 	"%s: Bad inode log record, rec ptr 0x%p, dino ptr 0x%p, "
 	"dino bp 0x%p, ino %Ld, forkoff 0x%x", __func__,
-			item, dip, bp, in_f->ilf_ino, dicp->di_forkoff);
+			item, dip, bp, in_f->ilf_ino, ldip->di_forkoff);
 		error = -EFSCORRUPTED;
 		goto out_release;
 	}
-	isize = xfs_icdinode_size(dicp->di_version);
+	isize = xfs_log_dinode_size(ldip->di_version);
 	if (unlikely(item->ri_buf[1].i_len > isize)) {
 		XFS_CORRUPTION_ERROR("xlog_recover_inode_pass2(7)",
-				     XFS_ERRLEVEL_LOW, mp, dicp);
+				     XFS_ERRLEVEL_LOW, mp, ldip);
 		xfs_alert(mp,
 			"%s: Bad inode log record length %d, rec ptr 0x%p",
 			__func__, item->ri_buf[1].i_len, item);
@@ -3007,7 +3008,8 @@ xlog_recover_inode_pass2(
 	}
 
 	/* The core is in in-core format */
-	xfs_dinode_to_disk(dip, dicp);
+	xfs_log_dinode_to_icdinode(ldip, &icic);
+	xfs_dinode_to_disk(dip, &icic);
 
 	/* the rest is in on-disk format */
 	if (item->ri_buf[1].i_len > isize) {
-- 
2.5.0

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 1/8] xfs: introduce inode log format object

[Brian Foster]

On Thu, Jan 14, 2016 at 05:09:18PM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> We currently carry around and log an entire inode core in the
> struct xfs_inode. A lot of the information in the inode core is
> duplicated in the VFS inode, but we cannot remove this duplication
> of infomration because the inode core is logged directly in
> xfs_inode_item_format().
> 
> Add a new function xfs_inode_item_format_core() that copies the
> inode core data into a struct xfs_icdinode that is pulled directly
> from the log vector buffer. This means we no longer directly
> copy the inode core, but copy the structures one member at a time.
> This will be slightly less efficient than copying, but will allow us
> to remove duplicate and unnecessary items from the struct xfs_inode.
> 
> To enable us to do this, call the new structure a xfs_log_dinode,
> so that we know it's different to the physical xfs_dinode and the
> in-core xfs_icdinode.
> 
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> ---

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  fs/xfs/libxfs/xfs_inode_buf.c  |   8 +--
>  fs/xfs/libxfs/xfs_inode_buf.h  |  53 ++++++++++++++++-
>  fs/xfs/libxfs/xfs_log_format.h |  15 ++---
>  fs/xfs/xfs_icache.c            |   2 +-
>  fs/xfs/xfs_inode.c             |   6 +-
>  fs/xfs/xfs_inode.h             |   2 +-
>  fs/xfs/xfs_inode_item.c        | 128 +++++++++++++++++++++++++++++++++++++++--
>  fs/xfs/xfs_inode_item.h        |   2 +
>  fs/xfs/xfs_log_recover.c       |  52 +++++++++--------
>  9 files changed, 221 insertions(+), 47 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_inode_buf.c b/fs/xfs/libxfs/xfs_inode_buf.c
> index 1aabfda..63d46bf 100644
> --- a/fs/xfs/libxfs/xfs_inode_buf.c
> +++ b/fs/xfs/libxfs/xfs_inode_buf.c
> @@ -196,8 +196,8 @@ xfs_imap_to_bp(
>  
>  void
>  xfs_dinode_from_disk(
> -	xfs_icdinode_t		*to,
> -	xfs_dinode_t		*from)
> +	struct xfs_icdinode	*to,
> +	struct xfs_dinode	*from)
>  {
>  	to->di_magic = be16_to_cpu(from->di_magic);
>  	to->di_mode = be16_to_cpu(from->di_mode);
> @@ -243,8 +243,8 @@ xfs_dinode_from_disk(
>  
>  void
>  xfs_dinode_to_disk(
> -	xfs_dinode_t		*to,
> -	xfs_icdinode_t		*from)
> +	struct xfs_dinode	*to,
> +	struct xfs_icdinode	*from)
>  {
>  	to->di_magic = cpu_to_be16(from->di_magic);
>  	to->di_mode = cpu_to_be16(from->di_mode);
> diff --git a/fs/xfs/libxfs/xfs_inode_buf.h b/fs/xfs/libxfs/xfs_inode_buf.h
> index 9308c47..642f2a2 100644
> --- a/fs/xfs/libxfs/xfs_inode_buf.h
> +++ b/fs/xfs/libxfs/xfs_inode_buf.h
> @@ -20,7 +20,58 @@
>  
>  struct xfs_inode;
>  struct xfs_dinode;
> -struct xfs_icdinode;
> +
> +/*
> + * In memory representation of the XFS inode. This is held in the in-core
> + * struct xfs_inode to represent the on disk values, but no longer needs to be
> + * identical to the on-disk structure as it is always translated to on-disk
> + * format specific structures at the appropriate time.
> + */
> +struct xfs_icdinode {
> +	__uint16_t	di_magic;	/* inode magic # = XFS_DINODE_MAGIC */
> +	__uint16_t	di_mode;	/* mode and type of file */
> +	__int8_t	di_version;	/* inode version */
> +	__int8_t	di_format;	/* format of di_c data */
> +	__uint16_t	di_onlink;	/* old number of links to file */
> +	__uint32_t	di_uid;		/* owner's user id */
> +	__uint32_t	di_gid;		/* owner's group id */
> +	__uint32_t	di_nlink;	/* number of links to file */
> +	__uint16_t	di_projid_lo;	/* lower part of owner's project id */
> +	__uint16_t	di_projid_hi;	/* higher part of owner's project id */
> +	__uint8_t	di_pad[6];	/* unused, zeroed space */
> +	__uint16_t	di_flushiter;	/* incremented on flush */
> +	xfs_ictimestamp_t di_atime;	/* time last accessed */
> +	xfs_ictimestamp_t di_mtime;	/* time last modified */
> +	xfs_ictimestamp_t di_ctime;	/* time created/inode modified */
> +	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 */
> +	xfs_extnum_t	di_nextents;	/* number of extents in data fork */
> +	xfs_aextnum_t	di_anextents;	/* number of extents in attribute fork*/
> +	__uint8_t	di_forkoff;	/* attr fork offs, <<3 for 64b align */
> +	__int8_t	di_aformat;	/* format of attr fork's data */
> +	__uint32_t	di_dmevmask;	/* DMIG event mask */
> +	__uint16_t	di_dmstate;	/* DMIG state info */
> +	__uint16_t	di_flags;	/* random flags, XFS_DIFLAG_... */
> +	__uint32_t	di_gen;		/* generation number */
> +
> +	/* di_next_unlinked is the only non-core field in the old dinode */
> +	xfs_agino_t	di_next_unlinked;/* agi unlinked list ptr */
> +
> +	/* start of the extended dinode, writable fields */
> +	__uint32_t	di_crc;		/* CRC of the inode */
> +	__uint64_t	di_changecount;	/* number of attribute changes */
> +	xfs_lsn_t	di_lsn;		/* flush sequence */
> +	__uint64_t	di_flags2;	/* more random flags */
> +	__uint8_t	di_pad2[16];	/* more padding for future expansion */
> +
> +	/* fields only written to during inode creation */
> +	xfs_ictimestamp_t di_crtime;	/* time created */
> +	xfs_ino_t	di_ino;		/* inode number */
> +	uuid_t		di_uuid;	/* UUID of the filesystem */
> +
> +	/* structure must be padded to 64 bit alignment */
> +};
>  
>  /*
>   * Inode location information.  Stored in the inode and passed to
> diff --git a/fs/xfs/libxfs/xfs_log_format.h b/fs/xfs/libxfs/xfs_log_format.h
> index 2653146..d00ed63 100644
> --- a/fs/xfs/libxfs/xfs_log_format.h
> +++ b/fs/xfs/libxfs/xfs_log_format.h
> @@ -290,6 +290,7 @@ typedef struct xfs_inode_log_format_64 {
>  	__int32_t		ilf_boffset;	/* off of inode in buffer */
>  } xfs_inode_log_format_64_t;
>  
> +
>  /*
>   * Flags for xfs_trans_log_inode flags field.
>   */
> @@ -360,10 +361,10 @@ typedef struct xfs_ictimestamp {
>  } xfs_ictimestamp_t;
>  
>  /*
> - * NOTE:  This structure must be kept identical to struct xfs_dinode
> - *	  except for the endianness annotations.
> + * Define the format of the inode core that is logged. This structure must be
> + * kept identical to struct xfs_dinode except for the endianness annotations.
>   */
> -typedef struct xfs_icdinode {
> +struct xfs_log_dinode {
>  	__uint16_t	di_magic;	/* inode magic # = XFS_DINODE_MAGIC */
>  	__uint16_t	di_mode;	/* mode and type of file */
>  	__int8_t	di_version;	/* inode version */
> @@ -407,13 +408,13 @@ typedef struct xfs_icdinode {
>  	uuid_t		di_uuid;	/* UUID of the filesystem */
>  
>  	/* structure must be padded to 64 bit alignment */
> -} xfs_icdinode_t;
> +};
>  
> -static inline uint xfs_icdinode_size(int version)
> +static inline uint xfs_log_dinode_size(int version)
>  {
>  	if (version == 3)
> -		return sizeof(struct xfs_icdinode);
> -	return offsetof(struct xfs_icdinode, di_next_unlinked);
> +		return sizeof(struct xfs_log_dinode);
> +	return offsetof(struct xfs_log_dinode, di_next_unlinked);
>  }
>  
>  /*
> diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c
> index d7a490f..7c26f86 100644
> --- a/fs/xfs/xfs_icache.c
> +++ b/fs/xfs/xfs_icache.c
> @@ -79,7 +79,7 @@ xfs_inode_alloc(
>  	memset(&ip->i_df, 0, sizeof(xfs_ifork_t));
>  	ip->i_flags = 0;
>  	ip->i_delayed_blks = 0;
> -	memset(&ip->i_d, 0, sizeof(xfs_icdinode_t));
> +	memset(&ip->i_d, 0, sizeof(ip->i_d));
>  
>  	return ip;
>  }
> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> index ae3758a..7e24232 100644
> --- a/fs/xfs/xfs_inode.c
> +++ b/fs/xfs/xfs_inode.c
> @@ -650,9 +650,9 @@ _xfs_dic2xflags(
>  
>  uint
>  xfs_ip2xflags(
> -	xfs_inode_t		*ip)
> +	struct xfs_inode	*ip)
>  {
> -	xfs_icdinode_t		*dic = &ip->i_d;
> +	struct xfs_icdinode	*dic = &ip->i_d;
>  
>  	return _xfs_dic2xflags(dic->di_flags) |
>  				(XFS_IFORK_Q(ip) ? XFS_XFLAG_HASATTR : 0);
> @@ -660,7 +660,7 @@ xfs_ip2xflags(
>  
>  uint
>  xfs_dic2xflags(
> -	xfs_dinode_t		*dip)
> +	struct xfs_dinode	*dip)
>  {
>  	return _xfs_dic2xflags(be16_to_cpu(dip->di_flags)) |
>  				(XFS_DFORK_Q(dip) ? XFS_XFLAG_HASATTR : 0);
> diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
> index ca9e119..aef5452 100644
> --- a/fs/xfs/xfs_inode.h
> +++ b/fs/xfs/xfs_inode.h
> @@ -63,7 +63,7 @@ typedef struct xfs_inode {
>  	unsigned long		i_flags;	/* see defined flags below */
>  	unsigned int		i_delayed_blks;	/* count of delay alloc blks */
>  
> -	xfs_icdinode_t		i_d;		/* most of ondisk inode */
> +	struct xfs_icdinode	i_d;		/* most of ondisk inode */
>  
>  	/* VFS inode */
>  	struct inode		i_vnode;	/* embedded VFS inode */
> diff --git a/fs/xfs/xfs_inode_item.c b/fs/xfs/xfs_inode_item.c
> index d14b12b..3ad9972 100644
> --- a/fs/xfs/xfs_inode_item.c
> +++ b/fs/xfs/xfs_inode_item.c
> @@ -135,7 +135,7 @@ xfs_inode_item_size(
>  
>  	*nvecs += 2;
>  	*nbytes += sizeof(struct xfs_inode_log_format) +
> -		   xfs_icdinode_size(ip->i_d.di_version);
> +		   xfs_log_dinode_size(ip->i_d.di_version);
>  
>  	xfs_inode_item_data_fork_size(iip, nvecs, nbytes);
>  	if (XFS_IFORK_Q(ip))
> @@ -322,6 +322,127 @@ xfs_inode_item_format_attr_fork(
>  	}
>  }
>  
> +static void
> +xfs_icdinode_to_log_dinode(
> +	struct xfs_icdinode	*from,
> +	struct xfs_log_dinode	*to)
> +{
> +	to->di_magic = from->di_magic;
> +	to->di_mode = from->di_mode;
> +	to->di_version = from->di_version;
> +	to->di_format = from->di_format;
> +	to->di_onlink = from->di_onlink;
> +	to->di_uid = from->di_uid;
> +	to->di_gid = from->di_gid;
> +	to->di_nlink = from->di_nlink;
> +	to->di_projid_lo = from->di_projid_lo;
> +	to->di_projid_hi = from->di_projid_hi;
> +	memcpy(to->di_pad, from->di_pad, sizeof(to->di_pad));
> +	to->di_atime.t_sec = from->di_atime.t_sec;
> +	to->di_atime.t_nsec = from->di_atime.t_nsec;
> +	to->di_mtime.t_sec = from->di_mtime.t_sec;
> +	to->di_mtime.t_nsec = from->di_mtime.t_nsec;
> +	to->di_ctime.t_sec = from->di_ctime.t_sec;
> +	to->di_ctime.t_nsec = from->di_ctime.t_nsec;
> +	to->di_size = from->di_size;
> +	to->di_nblocks = from->di_nblocks;
> +	to->di_extsize = from->di_extsize;
> +	to->di_nextents = from->di_nextents;
> +	to->di_anextents = from->di_anextents;
> +	to->di_forkoff = from->di_forkoff;
> +	to->di_aformat = from->di_aformat;
> +	to->di_dmevmask = from->di_dmevmask;
> +	to->di_dmstate = from->di_dmstate;
> +	to->di_flags = from->di_flags;
> +	to->di_gen = from->di_gen;
> +
> +	if (from->di_version == 3) {
> +		to->di_changecount = from->di_changecount;
> +		to->di_crtime.t_sec = from->di_crtime.t_sec;
> +		to->di_crtime.t_nsec = from->di_crtime.t_nsec;
> +		to->di_flags2 = from->di_flags2;
> +		to->di_ino = from->di_ino;
> +		to->di_lsn = from->di_lsn;
> +		memcpy(to->di_pad2, from->di_pad2, sizeof(to->di_pad2));
> +		uuid_copy(&to->di_uuid, &from->di_uuid);
> +		to->di_flushiter = 0;
> +	} else {
> +		to->di_flushiter = from->di_flushiter;
> +	}
> +}
> +
> +/*
> + * Recovery needs to be able to convert a log dinode back to a real dinode
> + * for writeback we do that by converting a log dinode to a icdinode, and
> + * then passing that to the formatting function.
> + */
> +void
> +xfs_log_dinode_to_icdinode(
> +	struct xfs_log_dinode	*from,
> +	struct xfs_icdinode	*to)
> +{
> +	to->di_magic = from->di_magic;
> +	to->di_mode = from->di_mode;
> +	to->di_version = from->di_version;
> +	to->di_format = from->di_format;
> +	to->di_onlink = from->di_onlink;
> +	to->di_uid = from->di_uid;
> +	to->di_gid = from->di_gid;
> +	to->di_nlink = from->di_nlink;
> +	to->di_projid_lo = from->di_projid_lo;
> +	to->di_projid_hi = from->di_projid_hi;
> +	memset(to->di_pad, 0, sizeof(to->di_pad));
> +	to->di_atime.t_sec = from->di_atime.t_sec;
> +	to->di_atime.t_nsec = from->di_atime.t_nsec;
> +	to->di_mtime.t_sec = from->di_mtime.t_sec;
> +	to->di_mtime.t_nsec = from->di_mtime.t_nsec;
> +	to->di_ctime.t_sec = from->di_ctime.t_sec;
> +	to->di_ctime.t_nsec = from->di_ctime.t_nsec;
> +	to->di_size = from->di_size;
> +	to->di_nblocks = from->di_nblocks;
> +	to->di_extsize = from->di_extsize;
> +	to->di_nextents = from->di_nextents;
> +	to->di_anextents = from->di_anextents;
> +	to->di_forkoff = from->di_forkoff;
> +	to->di_aformat = from->di_aformat;
> +	to->di_dmevmask = from->di_dmevmask;
> +	to->di_dmstate = from->di_dmstate;
> +	to->di_flags = from->di_flags;
> +	to->di_gen = from->di_gen;
> +
> +	if (from->di_version == 3) {
> +		to->di_changecount = from->di_changecount;
> +		to->di_crtime.t_sec = from->di_crtime.t_sec;
> +		to->di_crtime.t_nsec = from->di_crtime.t_nsec;
> +		to->di_flags2 = from->di_flags2;
> +		to->di_ino = from->di_ino;
> +		to->di_lsn = from->di_lsn;
> +		memcpy(to->di_pad2, from->di_pad2, sizeof(to->di_pad2));
> +		uuid_copy(&to->di_uuid, &from->di_uuid);
> +		to->di_flushiter = 0;
> +	} else {
> +		to->di_flushiter = from->di_flushiter;
> +	}
> +}
> +
> +/*
> + * Format the inode core. Current timestamp data is only in the VFS inode
> + * fields, so we need to grab them from there. Hence rather than just copying
> + * the XFS inode core structure, format the fields directly into the iovec.
> + */
> +static void
> +xfs_inode_item_format_core(
> +	struct xfs_inode	*ip,
> +	struct xfs_log_vec	*lv,
> +	struct xfs_log_iovec	**vecp)
> +{
> +	struct xfs_log_dinode	*dic;
> +
> +	dic = xlog_prepare_iovec(lv, vecp, XLOG_REG_TYPE_ICORE);
> +	xfs_icdinode_to_log_dinode(&ip->i_d, dic);
> +	xlog_finish_iovec(lv, *vecp, xfs_log_dinode_size(ip->i_d.di_version));
> +}
> +
>  /*
>   * This is called to fill in the vector of log iovecs for the given inode
>   * log item.  It fills the first item with an inode log format structure,
> @@ -351,10 +472,7 @@ xfs_inode_item_format(
>  	ilf->ilf_size = 2; /* format + core */
>  	xlog_finish_iovec(lv, vecp, sizeof(struct xfs_inode_log_format));
>  
> -	xlog_copy_iovec(lv, &vecp, XLOG_REG_TYPE_ICORE,
> -			&ip->i_d,
> -			xfs_icdinode_size(ip->i_d.di_version));
> -
> +	xfs_inode_item_format_core(ip, lv, &vecp);
>  	xfs_inode_item_format_data_fork(iip, ilf, lv, &vecp);
>  	if (XFS_IFORK_Q(ip)) {
>  		xfs_inode_item_format_attr_fork(iip, ilf, lv, &vecp);
> diff --git a/fs/xfs/xfs_inode_item.h b/fs/xfs/xfs_inode_item.h
> index 4c7722e..2426118 100644
> --- a/fs/xfs/xfs_inode_item.h
> +++ b/fs/xfs/xfs_inode_item.h
> @@ -49,6 +49,8 @@ extern void xfs_istale_done(struct xfs_buf *, struct xfs_log_item *);
>  extern void xfs_iflush_abort(struct xfs_inode *, bool);
>  extern int xfs_inode_item_format_convert(xfs_log_iovec_t *,
>  					 xfs_inode_log_format_t *);
> +extern void xfs_log_dinode_to_icdinode(struct xfs_log_dinode *from,
> +				       struct xfs_icdinode *to);
>  
>  extern struct kmem_zone	*xfs_ili_zone;
>  
> diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
> index da37beb..3120f7b 100644
> --- a/fs/xfs/xfs_log_recover.c
> +++ b/fs/xfs/xfs_log_recover.c
> @@ -2839,7 +2839,8 @@ xlog_recover_inode_pass2(
>  	int			error;
>  	int			attr_index;
>  	uint			fields;
> -	xfs_icdinode_t		*dicp;
> +	struct xfs_log_dinode	*ldip;
> +	struct xfs_icdinode	icic;
>  	uint			isize;
>  	int			need_free = 0;
>  
> @@ -2892,8 +2893,8 @@ xlog_recover_inode_pass2(
>  		error = -EFSCORRUPTED;
>  		goto out_release;
>  	}
> -	dicp = item->ri_buf[1].i_addr;
> -	if (unlikely(dicp->di_magic != XFS_DINODE_MAGIC)) {
> +	ldip = item->ri_buf[1].i_addr;
> +	if (unlikely(ldip->di_magic != XFS_DINODE_MAGIC)) {
>  		xfs_alert(mp,
>  			"%s: Bad inode log record, rec ptr 0x%p, ino %Ld",
>  			__func__, item, in_f->ilf_ino);
> @@ -2929,13 +2930,13 @@ xlog_recover_inode_pass2(
>  	 * to skip replay when the on disk inode is newer than the log one
>  	 */
>  	if (!xfs_sb_version_hascrc(&mp->m_sb) &&
> -	    dicp->di_flushiter < be16_to_cpu(dip->di_flushiter)) {
> +	    ldip->di_flushiter < be16_to_cpu(dip->di_flushiter)) {
>  		/*
>  		 * Deal with the wrap case, DI_MAX_FLUSH is less
>  		 * than smaller numbers
>  		 */
>  		if (be16_to_cpu(dip->di_flushiter) == DI_MAX_FLUSH &&
> -		    dicp->di_flushiter < (DI_MAX_FLUSH >> 1)) {
> +		    ldip->di_flushiter < (DI_MAX_FLUSH >> 1)) {
>  			/* do nothing */
>  		} else {
>  			trace_xfs_log_recover_inode_skip(log, in_f);
> @@ -2945,13 +2946,13 @@ xlog_recover_inode_pass2(
>  	}
>  
>  	/* Take the opportunity to reset the flush iteration count */
> -	dicp->di_flushiter = 0;
> +	ldip->di_flushiter = 0;
>  
> -	if (unlikely(S_ISREG(dicp->di_mode))) {
> -		if ((dicp->di_format != XFS_DINODE_FMT_EXTENTS) &&
> -		    (dicp->di_format != XFS_DINODE_FMT_BTREE)) {
> +	if (unlikely(S_ISREG(ldip->di_mode))) {
> +		if ((ldip->di_format != XFS_DINODE_FMT_EXTENTS) &&
> +		    (ldip->di_format != XFS_DINODE_FMT_BTREE)) {
>  			XFS_CORRUPTION_ERROR("xlog_recover_inode_pass2(3)",
> -					 XFS_ERRLEVEL_LOW, mp, dicp);
> +					 XFS_ERRLEVEL_LOW, mp, ldip);
>  			xfs_alert(mp,
>  		"%s: Bad regular inode log record, rec ptr 0x%p, "
>  		"ino ptr = 0x%p, ino bp = 0x%p, ino %Ld",
> @@ -2959,12 +2960,12 @@ xlog_recover_inode_pass2(
>  			error = -EFSCORRUPTED;
>  			goto out_release;
>  		}
> -	} else if (unlikely(S_ISDIR(dicp->di_mode))) {
> -		if ((dicp->di_format != XFS_DINODE_FMT_EXTENTS) &&
> -		    (dicp->di_format != XFS_DINODE_FMT_BTREE) &&
> -		    (dicp->di_format != XFS_DINODE_FMT_LOCAL)) {
> +	} else if (unlikely(S_ISDIR(ldip->di_mode))) {
> +		if ((ldip->di_format != XFS_DINODE_FMT_EXTENTS) &&
> +		    (ldip->di_format != XFS_DINODE_FMT_BTREE) &&
> +		    (ldip->di_format != XFS_DINODE_FMT_LOCAL)) {
>  			XFS_CORRUPTION_ERROR("xlog_recover_inode_pass2(4)",
> -					     XFS_ERRLEVEL_LOW, mp, dicp);
> +					     XFS_ERRLEVEL_LOW, mp, ldip);
>  			xfs_alert(mp,
>  		"%s: Bad dir inode log record, rec ptr 0x%p, "
>  		"ino ptr = 0x%p, ino bp = 0x%p, ino %Ld",
> @@ -2973,32 +2974,32 @@ xlog_recover_inode_pass2(
>  			goto out_release;
>  		}
>  	}
> -	if (unlikely(dicp->di_nextents + dicp->di_anextents > dicp->di_nblocks)){
> +	if (unlikely(ldip->di_nextents + ldip->di_anextents > ldip->di_nblocks)){
>  		XFS_CORRUPTION_ERROR("xlog_recover_inode_pass2(5)",
> -				     XFS_ERRLEVEL_LOW, mp, dicp);
> +				     XFS_ERRLEVEL_LOW, mp, ldip);
>  		xfs_alert(mp,
>  	"%s: Bad inode log record, rec ptr 0x%p, dino ptr 0x%p, "
>  	"dino bp 0x%p, ino %Ld, total extents = %d, nblocks = %Ld",
>  			__func__, item, dip, bp, in_f->ilf_ino,
> -			dicp->di_nextents + dicp->di_anextents,
> -			dicp->di_nblocks);
> +			ldip->di_nextents + ldip->di_anextents,
> +			ldip->di_nblocks);
>  		error = -EFSCORRUPTED;
>  		goto out_release;
>  	}
> -	if (unlikely(dicp->di_forkoff > mp->m_sb.sb_inodesize)) {
> +	if (unlikely(ldip->di_forkoff > mp->m_sb.sb_inodesize)) {
>  		XFS_CORRUPTION_ERROR("xlog_recover_inode_pass2(6)",
> -				     XFS_ERRLEVEL_LOW, mp, dicp);
> +				     XFS_ERRLEVEL_LOW, mp, ldip);
>  		xfs_alert(mp,
>  	"%s: Bad inode log record, rec ptr 0x%p, dino ptr 0x%p, "
>  	"dino bp 0x%p, ino %Ld, forkoff 0x%x", __func__,
> -			item, dip, bp, in_f->ilf_ino, dicp->di_forkoff);
> +			item, dip, bp, in_f->ilf_ino, ldip->di_forkoff);
>  		error = -EFSCORRUPTED;
>  		goto out_release;
>  	}
> -	isize = xfs_icdinode_size(dicp->di_version);
> +	isize = xfs_log_dinode_size(ldip->di_version);
>  	if (unlikely(item->ri_buf[1].i_len > isize)) {
>  		XFS_CORRUPTION_ERROR("xlog_recover_inode_pass2(7)",
> -				     XFS_ERRLEVEL_LOW, mp, dicp);
> +				     XFS_ERRLEVEL_LOW, mp, ldip);
>  		xfs_alert(mp,
>  			"%s: Bad inode log record length %d, rec ptr 0x%p",
>  			__func__, item->ri_buf[1].i_len, item);
> @@ -3007,7 +3008,8 @@ xlog_recover_inode_pass2(
>  	}
>  
>  	/* The core is in in-core format */
> -	xfs_dinode_to_disk(dip, dicp);
> +	xfs_log_dinode_to_icdinode(ldip, &icic);
> +	xfs_dinode_to_disk(dip, &icic);
>  
>  	/* the rest is in on-disk format */
>  	if (item->ri_buf[1].i_len > isize) {
> -- 
> 2.5.0
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 2/8] xfs: remove timestamps from incore inode

[Dave Chinner]

From: Dave Chinner <dchinner@redhat.com>

The struct xfs_inode has two copies of the current timestamps in it,
one in the vfs inode and one in the struct xfs_icdinode. Now that we
no longer log the struct xfs_icdinode directly, we don't need to
keep the timestamps in this structure. instead we can copy them
straight out of the VFS inode when formatting the inode log item or
the on-disk inode.

This reduces the struct xfs_inode in size by 24 bytes.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
 fs/xfs/libxfs/xfs_inode_buf.c | 91 +++++++++++++++++++++++++++++++++++++------
 fs/xfs/libxfs/xfs_inode_buf.h |  9 ++---
 fs/xfs/libxfs/xfs_rtbitmap.c  |  2 +-
 fs/xfs/xfs_inode.c            | 21 +++++-----
 fs/xfs/xfs_inode_item.c       | 73 ++++++----------------------------
 fs/xfs/xfs_inode_item.h       |  2 -
 fs/xfs/xfs_iops.c             | 37 ++++--------------
 fs/xfs/xfs_itable.c           | 16 +++++---
 fs/xfs/xfs_log_recover.c      |  8 ++--
 fs/xfs/xfs_rtalloc.c          |  2 +-
 fs/xfs/xfs_trans_inode.c      | 12 +-----
 11 files changed, 130 insertions(+), 143 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_inode_buf.c b/fs/xfs/libxfs/xfs_inode_buf.c
index 63d46bf..26e3c46 100644
--- a/fs/xfs/libxfs/xfs_inode_buf.c
+++ b/fs/xfs/libxfs/xfs_inode_buf.c
@@ -195,10 +195,13 @@ xfs_imap_to_bp(
 }
 
 void
-xfs_dinode_from_disk(
-	struct xfs_icdinode	*to,
+xfs_inode_from_disk(
+	struct xfs_inode	*ip,
 	struct xfs_dinode	*from)
 {
+	struct xfs_icdinode	*to = &ip->i_d;
+	struct inode		*inode = VFS_I(ip);
+
 	to->di_magic = be16_to_cpu(from->di_magic);
 	to->di_mode = be16_to_cpu(from->di_mode);
 	to->di_version = from ->di_version;
@@ -211,12 +214,20 @@ xfs_dinode_from_disk(
 	to->di_projid_hi = be16_to_cpu(from->di_projid_hi);
 	memcpy(to->di_pad, from->di_pad, sizeof(to->di_pad));
 	to->di_flushiter = be16_to_cpu(from->di_flushiter);
-	to->di_atime.t_sec = be32_to_cpu(from->di_atime.t_sec);
-	to->di_atime.t_nsec = be32_to_cpu(from->di_atime.t_nsec);
-	to->di_mtime.t_sec = be32_to_cpu(from->di_mtime.t_sec);
-	to->di_mtime.t_nsec = be32_to_cpu(from->di_mtime.t_nsec);
-	to->di_ctime.t_sec = be32_to_cpu(from->di_ctime.t_sec);
-	to->di_ctime.t_nsec = be32_to_cpu(from->di_ctime.t_nsec);
+
+	/*
+	 * time is signed, so need to convert to signed 32 bit before
+	 * storing in inode timestamp which may be 64 bit. Otherwise
+	 * a time before epoch is converted to a time long after epoch
+	 * on 64 bit systems.
+	 */
+	inode->i_atime.tv_sec = (int)be32_to_cpu(from->di_atime.t_sec);
+	inode->i_atime.tv_nsec = (int)be32_to_cpu(from->di_atime.t_nsec);
+	inode->i_mtime.tv_sec = (int)be32_to_cpu(from->di_mtime.t_sec);
+	inode->i_mtime.tv_nsec = (int)be32_to_cpu(from->di_mtime.t_nsec);
+	inode->i_ctime.tv_sec = (int)be32_to_cpu(from->di_ctime.t_sec);
+	inode->i_ctime.tv_nsec = (int)be32_to_cpu(from->di_ctime.t_nsec);
+
 	to->di_size = be64_to_cpu(from->di_size);
 	to->di_nblocks = be64_to_cpu(from->di_nblocks);
 	to->di_extsize = be32_to_cpu(from->di_extsize);
@@ -242,9 +253,63 @@ xfs_dinode_from_disk(
 }
 
 void
-xfs_dinode_to_disk(
-	struct xfs_dinode	*to,
-	struct xfs_icdinode	*from)
+xfs_inode_to_disk(
+	struct xfs_inode	*ip,
+	struct xfs_dinode	*to)
+{
+	struct xfs_icdinode	*from = &ip->i_d;
+	struct inode		*inode = VFS_I(ip);
+
+	to->di_magic = cpu_to_be16(from->di_magic);
+	to->di_mode = cpu_to_be16(from->di_mode);
+	to->di_version = from ->di_version;
+	to->di_format = from->di_format;
+	to->di_onlink = cpu_to_be16(from->di_onlink);
+	to->di_uid = cpu_to_be32(from->di_uid);
+	to->di_gid = cpu_to_be32(from->di_gid);
+	to->di_nlink = cpu_to_be32(from->di_nlink);
+	to->di_projid_lo = cpu_to_be16(from->di_projid_lo);
+	to->di_projid_hi = cpu_to_be16(from->di_projid_hi);
+	memcpy(to->di_pad, from->di_pad, sizeof(to->di_pad));
+
+	to->di_atime.t_sec = cpu_to_be32(inode->i_atime.tv_sec);
+	to->di_atime.t_nsec = cpu_to_be32(inode->i_atime.tv_nsec);
+	to->di_mtime.t_sec = cpu_to_be32(inode->i_mtime.tv_sec);
+	to->di_mtime.t_nsec = cpu_to_be32(inode->i_mtime.tv_nsec);
+	to->di_ctime.t_sec = cpu_to_be32(inode->i_ctime.tv_sec);
+	to->di_ctime.t_nsec = cpu_to_be32(inode->i_ctime.tv_nsec);
+
+	to->di_size = cpu_to_be64(from->di_size);
+	to->di_nblocks = cpu_to_be64(from->di_nblocks);
+	to->di_extsize = cpu_to_be32(from->di_extsize);
+	to->di_nextents = cpu_to_be32(from->di_nextents);
+	to->di_anextents = cpu_to_be16(from->di_anextents);
+	to->di_forkoff = from->di_forkoff;
+	to->di_aformat = from->di_aformat;
+	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);
+	to->di_gen = cpu_to_be32(from->di_gen);
+
+	if (from->di_version == 3) {
+		to->di_changecount = cpu_to_be64(from->di_changecount);
+		to->di_crtime.t_sec = cpu_to_be32(from->di_crtime.t_sec);
+		to->di_crtime.t_nsec = cpu_to_be32(from->di_crtime.t_nsec);
+		to->di_flags2 = cpu_to_be64(from->di_flags2);
+		to->di_ino = cpu_to_be64(from->di_ino);
+		to->di_lsn = cpu_to_be64(from->di_lsn);
+		memcpy(to->di_pad2, from->di_pad2, sizeof(to->di_pad2));
+		uuid_copy(&to->di_uuid, &from->di_uuid);
+		to->di_flushiter = 0;
+	} else {
+		to->di_flushiter = cpu_to_be16(from->di_flushiter);
+	}
+}
+
+void
+xfs_log_dinode_to_disk(
+	struct xfs_log_dinode	*from,
+	struct xfs_dinode	*to)
 {
 	to->di_magic = cpu_to_be16(from->di_magic);
 	to->di_mode = cpu_to_be16(from->di_mode);
@@ -257,12 +322,14 @@ xfs_dinode_to_disk(
 	to->di_projid_lo = cpu_to_be16(from->di_projid_lo);
 	to->di_projid_hi = cpu_to_be16(from->di_projid_hi);
 	memcpy(to->di_pad, from->di_pad, sizeof(to->di_pad));
+
 	to->di_atime.t_sec = cpu_to_be32(from->di_atime.t_sec);
 	to->di_atime.t_nsec = cpu_to_be32(from->di_atime.t_nsec);
 	to->di_mtime.t_sec = cpu_to_be32(from->di_mtime.t_sec);
 	to->di_mtime.t_nsec = cpu_to_be32(from->di_mtime.t_nsec);
 	to->di_ctime.t_sec = cpu_to_be32(from->di_ctime.t_sec);
 	to->di_ctime.t_nsec = cpu_to_be32(from->di_ctime.t_nsec);
+
 	to->di_size = cpu_to_be64(from->di_size);
 	to->di_nblocks = cpu_to_be64(from->di_nblocks);
 	to->di_extsize = cpu_to_be32(from->di_extsize);
@@ -403,7 +470,7 @@ xfs_iread(
 	 * Otherwise, just get the truly permanent information.
 	 */
 	if (dip->di_mode) {
-		xfs_dinode_from_disk(&ip->i_d, dip);
+		xfs_inode_from_disk(ip, dip);
 		error = xfs_iformat_fork(ip, dip);
 		if (error)  {
 #ifdef DEBUG
diff --git a/fs/xfs/libxfs/xfs_inode_buf.h b/fs/xfs/libxfs/xfs_inode_buf.h
index 642f2a2..adcc9bf 100644
--- a/fs/xfs/libxfs/xfs_inode_buf.h
+++ b/fs/xfs/libxfs/xfs_inode_buf.h
@@ -40,9 +40,6 @@ struct xfs_icdinode {
 	__uint16_t	di_projid_hi;	/* higher part of owner's project id */
 	__uint8_t	di_pad[6];	/* unused, zeroed space */
 	__uint16_t	di_flushiter;	/* incremented on flush */
-	xfs_ictimestamp_t di_atime;	/* time last accessed */
-	xfs_ictimestamp_t di_mtime;	/* time last modified */
-	xfs_ictimestamp_t di_ctime;	/* time created/inode modified */
 	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 */
@@ -89,8 +86,10 @@ int	xfs_imap_to_bp(struct xfs_mount *, struct xfs_trans *,
 int	xfs_iread(struct xfs_mount *, struct xfs_trans *,
 		  struct xfs_inode *, uint);
 void	xfs_dinode_calc_crc(struct xfs_mount *, struct xfs_dinode *);
-void	xfs_dinode_to_disk(struct xfs_dinode *to, struct xfs_icdinode *from);
-void	xfs_dinode_from_disk(struct xfs_icdinode *to, struct xfs_dinode *from);
+void	xfs_inode_to_disk(struct xfs_inode *ip, struct xfs_dinode *to);
+void	xfs_inode_from_disk(struct xfs_inode *ip, struct xfs_dinode *from);
+void	xfs_log_dinode_to_disk(struct xfs_log_dinode *from,
+			       struct xfs_dinode *to);
 
 #if defined(DEBUG)
 void	xfs_inobp_check(struct xfs_mount *, struct xfs_buf *);
diff --git a/fs/xfs/libxfs/xfs_rtbitmap.c b/fs/xfs/libxfs/xfs_rtbitmap.c
index 9b59ffa..acc71dd 100644
--- a/fs/xfs/libxfs/xfs_rtbitmap.c
+++ b/fs/xfs/libxfs/xfs_rtbitmap.c
@@ -983,7 +983,7 @@ xfs_rtfree_extent(
 	    mp->m_sb.sb_rextents) {
 		if (!(mp->m_rbmip->i_d.di_flags & XFS_DIFLAG_NEWRTBM))
 			mp->m_rbmip->i_d.di_flags |= XFS_DIFLAG_NEWRTBM;
-		*(__uint64_t *)&mp->m_rbmip->i_d.di_atime = 0;
+		*(__uint64_t *)&VFS_I(mp->m_rbmip)->i_atime = 0;
 		xfs_trans_log_inode(tp, mp->m_rbmip, XFS_ILOG_CORE);
 	}
 	return 0;
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index 7e24232..3e9b8a0 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -757,6 +757,7 @@ xfs_ialloc(
 	uint		flags;
 	int		error;
 	struct timespec	tv;
+	struct inode	*inode;
 
 	/*
 	 * Call the space management code to pick
@@ -782,6 +783,7 @@ xfs_ialloc(
 	if (error)
 		return error;
 	ASSERT(ip != NULL);
+	inode = VFS_I(ip);
 
 	/*
 	 * We always convert v1 inodes to v2 now - we only support filesystems
@@ -823,10 +825,9 @@ xfs_ialloc(
 	ASSERT(ip->i_d.di_nblocks == 0);
 
 	tv = current_fs_time(mp->m_super);
-	ip->i_d.di_mtime.t_sec = (__int32_t)tv.tv_sec;
-	ip->i_d.di_mtime.t_nsec = (__int32_t)tv.tv_nsec;
-	ip->i_d.di_atime = ip->i_d.di_mtime;
-	ip->i_d.di_ctime = ip->i_d.di_mtime;
+	inode->i_mtime = tv;
+	inode->i_atime = tv;
+	inode->i_ctime = tv;
 
 	/*
 	 * di_gen will have been taken care of in xfs_iread.
@@ -844,7 +845,8 @@ xfs_ialloc(
 		ip->i_d.di_lsn = 0;
 		ip->i_d.di_flags2 = 0;
 		memset(&(ip->i_d.di_pad2[0]), 0, sizeof(ip->i_d.di_pad2));
-		ip->i_d.di_crtime = ip->i_d.di_mtime;
+		ip->i_d.di_crtime.t_sec = (__int32_t)tv.tv_sec;
+		ip->i_d.di_crtime.t_nsec = (__int32_t)tv.tv_nsec;
 	}
 
 
@@ -3509,12 +3511,11 @@ xfs_iflush_int(
 		ip->i_d.di_flushiter++;
 
 	/*
-	 * Copy the dirty parts of the inode into the on-disk
-	 * inode.  We always copy out the core of the inode,
-	 * because if the inode is dirty at all the core must
-	 * be.
+	 * Copy the dirty parts of the inode into the on-disk inode.  We always
+	 * copy out the core of the inode, because if the inode is dirty at all
+	 * the core must be.
 	 */
-	xfs_dinode_to_disk(dip, &ip->i_d);
+	xfs_inode_to_disk(ip, dip);
 
 	/* Wrap, we never let the log put out DI_MAX_FLUSH */
 	if (ip->i_d.di_flushiter == DI_MAX_FLUSH)
diff --git a/fs/xfs/xfs_inode_item.c b/fs/xfs/xfs_inode_item.c
index 3ad9972..9dcbf58 100644
--- a/fs/xfs/xfs_inode_item.c
+++ b/fs/xfs/xfs_inode_item.c
@@ -323,10 +323,13 @@ xfs_inode_item_format_attr_fork(
 }
 
 static void
-xfs_icdinode_to_log_dinode(
-	struct xfs_icdinode	*from,
+xfs_inode_to_log_dinode(
+	struct xfs_inode	*ip,
 	struct xfs_log_dinode	*to)
 {
+	struct xfs_icdinode	*from = &ip->i_d;
+	struct inode		*inode = VFS_I(ip);
+
 	to->di_magic = from->di_magic;
 	to->di_mode = from->di_mode;
 	to->di_version = from->di_version;
@@ -338,66 +341,14 @@ xfs_icdinode_to_log_dinode(
 	to->di_projid_lo = from->di_projid_lo;
 	to->di_projid_hi = from->di_projid_hi;
 	memcpy(to->di_pad, from->di_pad, sizeof(to->di_pad));
-	to->di_atime.t_sec = from->di_atime.t_sec;
-	to->di_atime.t_nsec = from->di_atime.t_nsec;
-	to->di_mtime.t_sec = from->di_mtime.t_sec;
-	to->di_mtime.t_nsec = from->di_mtime.t_nsec;
-	to->di_ctime.t_sec = from->di_ctime.t_sec;
-	to->di_ctime.t_nsec = from->di_ctime.t_nsec;
-	to->di_size = from->di_size;
-	to->di_nblocks = from->di_nblocks;
-	to->di_extsize = from->di_extsize;
-	to->di_nextents = from->di_nextents;
-	to->di_anextents = from->di_anextents;
-	to->di_forkoff = from->di_forkoff;
-	to->di_aformat = from->di_aformat;
-	to->di_dmevmask = from->di_dmevmask;
-	to->di_dmstate = from->di_dmstate;
-	to->di_flags = from->di_flags;
-	to->di_gen = from->di_gen;
 
-	if (from->di_version == 3) {
-		to->di_changecount = from->di_changecount;
-		to->di_crtime.t_sec = from->di_crtime.t_sec;
-		to->di_crtime.t_nsec = from->di_crtime.t_nsec;
-		to->di_flags2 = from->di_flags2;
-		to->di_ino = from->di_ino;
-		to->di_lsn = from->di_lsn;
-		memcpy(to->di_pad2, from->di_pad2, sizeof(to->di_pad2));
-		uuid_copy(&to->di_uuid, &from->di_uuid);
-		to->di_flushiter = 0;
-	} else {
-		to->di_flushiter = from->di_flushiter;
-	}
-}
+	to->di_atime.t_sec = inode->i_atime.tv_sec;
+	to->di_atime.t_nsec = inode->i_atime.tv_nsec;
+	to->di_mtime.t_sec = inode->i_mtime.tv_sec;
+	to->di_mtime.t_nsec = inode->i_mtime.tv_nsec;
+	to->di_ctime.t_sec = inode->i_ctime.tv_sec;
+	to->di_ctime.t_nsec = inode->i_ctime.tv_nsec;
 
-/*
- * Recovery needs to be able to convert a log dinode back to a real dinode
- * for writeback we do that by converting a log dinode to a icdinode, and
- * then passing that to the formatting function.
- */
-void
-xfs_log_dinode_to_icdinode(
-	struct xfs_log_dinode	*from,
-	struct xfs_icdinode	*to)
-{
-	to->di_magic = from->di_magic;
-	to->di_mode = from->di_mode;
-	to->di_version = from->di_version;
-	to->di_format = from->di_format;
-	to->di_onlink = from->di_onlink;
-	to->di_uid = from->di_uid;
-	to->di_gid = from->di_gid;
-	to->di_nlink = from->di_nlink;
-	to->di_projid_lo = from->di_projid_lo;
-	to->di_projid_hi = from->di_projid_hi;
-	memset(to->di_pad, 0, sizeof(to->di_pad));
-	to->di_atime.t_sec = from->di_atime.t_sec;
-	to->di_atime.t_nsec = from->di_atime.t_nsec;
-	to->di_mtime.t_sec = from->di_mtime.t_sec;
-	to->di_mtime.t_nsec = from->di_mtime.t_nsec;
-	to->di_ctime.t_sec = from->di_ctime.t_sec;
-	to->di_ctime.t_nsec = from->di_ctime.t_nsec;
 	to->di_size = from->di_size;
 	to->di_nblocks = from->di_nblocks;
 	to->di_extsize = from->di_extsize;
@@ -439,7 +390,7 @@ xfs_inode_item_format_core(
 	struct xfs_log_dinode	*dic;
 
 	dic = xlog_prepare_iovec(lv, vecp, XLOG_REG_TYPE_ICORE);
-	xfs_icdinode_to_log_dinode(&ip->i_d, dic);
+	xfs_inode_to_log_dinode(ip, dic);
 	xlog_finish_iovec(lv, *vecp, xfs_log_dinode_size(ip->i_d.di_version));
 }
 
diff --git a/fs/xfs/xfs_inode_item.h b/fs/xfs/xfs_inode_item.h
index 2426118..4c7722e 100644
--- a/fs/xfs/xfs_inode_item.h
+++ b/fs/xfs/xfs_inode_item.h
@@ -49,8 +49,6 @@ extern void xfs_istale_done(struct xfs_buf *, struct xfs_log_item *);
 extern void xfs_iflush_abort(struct xfs_inode *, bool);
 extern int xfs_inode_item_format_convert(xfs_log_iovec_t *,
 					 xfs_inode_log_format_t *);
-extern void xfs_log_dinode_to_icdinode(struct xfs_log_dinode *from,
-				       struct xfs_icdinode *to);
 
 extern struct kmem_zone	*xfs_ili_zone;
 
diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
index 245268a..b008677 100644
--- a/fs/xfs/xfs_iops.c
+++ b/fs/xfs/xfs_iops.c
@@ -517,21 +517,12 @@ xfs_setattr_time(
 
 	ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
 
-	if (iattr->ia_valid & ATTR_ATIME) {
+	if (iattr->ia_valid & ATTR_ATIME)
 		inode->i_atime = iattr->ia_atime;
-		ip->i_d.di_atime.t_sec = iattr->ia_atime.tv_sec;
-		ip->i_d.di_atime.t_nsec = iattr->ia_atime.tv_nsec;
-	}
-	if (iattr->ia_valid & ATTR_CTIME) {
+	if (iattr->ia_valid & ATTR_CTIME)
 		inode->i_ctime = iattr->ia_ctime;
-		ip->i_d.di_ctime.t_sec = iattr->ia_ctime.tv_sec;
-		ip->i_d.di_ctime.t_nsec = iattr->ia_ctime.tv_nsec;
-	}
-	if (iattr->ia_valid & ATTR_MTIME) {
+	if (iattr->ia_valid & ATTR_MTIME)
 		inode->i_mtime = iattr->ia_mtime;
-		ip->i_d.di_mtime.t_sec = iattr->ia_mtime.tv_sec;
-		ip->i_d.di_mtime.t_nsec = iattr->ia_mtime.tv_nsec;
-	}
 }
 
 int
@@ -986,21 +977,13 @@ xfs_vn_update_time(
 	}
 
 	xfs_ilock(ip, XFS_ILOCK_EXCL);
-	if (flags & S_CTIME) {
+	if (flags & S_CTIME)
 		inode->i_ctime = *now;
-		ip->i_d.di_ctime.t_sec = (__int32_t)now->tv_sec;
-		ip->i_d.di_ctime.t_nsec = (__int32_t)now->tv_nsec;
-	}
-	if (flags & S_MTIME) {
+	if (flags & S_MTIME)
 		inode->i_mtime = *now;
-		ip->i_d.di_mtime.t_sec = (__int32_t)now->tv_sec;
-		ip->i_d.di_mtime.t_nsec = (__int32_t)now->tv_nsec;
-	}
-	if (flags & S_ATIME) {
+	if (flags & S_ATIME)
 		inode->i_atime = *now;
-		ip->i_d.di_atime.t_sec = (__int32_t)now->tv_sec;
-		ip->i_d.di_atime.t_nsec = (__int32_t)now->tv_nsec;
-	}
+
 	xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL);
 	xfs_trans_log_inode(tp, ip, XFS_ILOG_TIMESTAMP);
 	return xfs_trans_commit(tp);
@@ -1247,12 +1230,6 @@ xfs_setup_inode(
 
 	inode->i_generation = ip->i_d.di_gen;
 	i_size_write(inode, ip->i_d.di_size);
-	inode->i_atime.tv_sec	= ip->i_d.di_atime.t_sec;
-	inode->i_atime.tv_nsec	= ip->i_d.di_atime.t_nsec;
-	inode->i_mtime.tv_sec	= ip->i_d.di_mtime.t_sec;
-	inode->i_mtime.tv_nsec	= ip->i_d.di_mtime.t_nsec;
-	inode->i_ctime.tv_sec	= ip->i_d.di_ctime.t_sec;
-	inode->i_ctime.tv_nsec	= ip->i_d.di_ctime.t_nsec;
 	xfs_diflags_to_iflags(inode, ip);
 
 	ip->d_ops = ip->i_mount->m_nondir_inode_ops;
diff --git a/fs/xfs/xfs_itable.c b/fs/xfs/xfs_itable.c
index 930ebd8..2acda42 100644
--- a/fs/xfs/xfs_itable.c
+++ b/fs/xfs/xfs_itable.c
@@ -57,6 +57,7 @@ xfs_bulkstat_one_int(
 {
 	struct xfs_icdinode	*dic;		/* dinode core info pointer */
 	struct xfs_inode	*ip;		/* incore inode pointer */
+	struct inode		*inode;
 	struct xfs_bstat	*buf;		/* return buffer */
 	int			error = 0;	/* error value */
 
@@ -77,6 +78,7 @@ xfs_bulkstat_one_int(
 
 	ASSERT(ip != NULL);
 	ASSERT(ip->i_imap.im_blkno != 0);
+	inode = VFS_I(ip);
 
 	dic = &ip->i_d;
 
@@ -91,12 +93,14 @@ xfs_bulkstat_one_int(
 	buf->bs_uid = dic->di_uid;
 	buf->bs_gid = dic->di_gid;
 	buf->bs_size = dic->di_size;
-	buf->bs_atime.tv_sec = dic->di_atime.t_sec;
-	buf->bs_atime.tv_nsec = dic->di_atime.t_nsec;
-	buf->bs_mtime.tv_sec = dic->di_mtime.t_sec;
-	buf->bs_mtime.tv_nsec = dic->di_mtime.t_nsec;
-	buf->bs_ctime.tv_sec = dic->di_ctime.t_sec;
-	buf->bs_ctime.tv_nsec = dic->di_ctime.t_nsec;
+
+	buf->bs_atime.tv_sec = inode->i_atime.tv_sec;
+	buf->bs_atime.tv_nsec = inode->i_atime.tv_nsec;
+	buf->bs_mtime.tv_sec = inode->i_mtime.tv_sec;
+	buf->bs_mtime.tv_nsec = inode->i_mtime.tv_nsec;
+	buf->bs_ctime.tv_sec = inode->i_ctime.tv_sec;
+	buf->bs_ctime.tv_nsec = inode->i_ctime.tv_nsec;
+
 	buf->bs_xflags = xfs_ip2xflags(ip);
 	buf->bs_extsize = dic->di_extsize << mp->m_sb.sb_blocklog;
 	buf->bs_extents = dic->di_nextents;
diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
index 3120f7b..4b79cf0 100644
--- a/fs/xfs/xfs_log_recover.c
+++ b/fs/xfs/xfs_log_recover.c
@@ -2793,7 +2793,7 @@ xfs_recover_inode_owner_change(
 		return -ENOMEM;
 
 	/* instantiate the inode */
-	xfs_dinode_from_disk(&ip->i_d, dip);
+	xfs_inode_from_disk(ip, dip);
 	ASSERT(ip->i_d.di_version >= 3);
 
 	error = xfs_iformat_fork(ip, dip);
@@ -2840,7 +2840,6 @@ xlog_recover_inode_pass2(
 	int			attr_index;
 	uint			fields;
 	struct xfs_log_dinode	*ldip;
-	struct xfs_icdinode	icic;
 	uint			isize;
 	int			need_free = 0;
 
@@ -3007,9 +3006,8 @@ xlog_recover_inode_pass2(
 		goto out_release;
 	}
 
-	/* The core is in in-core format */
-	xfs_log_dinode_to_icdinode(ldip, &icic);
-	xfs_dinode_to_disk(dip, &icic);
+	/* recover the log dinode inode into the on disk inode */
+	xfs_log_dinode_to_disk(ldip, dip);
 
 	/* the rest is in on-disk format */
 	if (item->ri_buf[1].i_len > isize) {
diff --git a/fs/xfs/xfs_rtalloc.c b/fs/xfs/xfs_rtalloc.c
index be02a68..abf4443 100644
--- a/fs/xfs/xfs_rtalloc.c
+++ b/fs/xfs/xfs_rtalloc.c
@@ -1272,7 +1272,7 @@ xfs_rtpick_extent(
 
 	ASSERT(xfs_isilocked(mp->m_rbmip, XFS_ILOCK_EXCL));
 
-	seqp = (__uint64_t *)&mp->m_rbmip->i_d.di_atime;
+	seqp = (__uint64_t *)&VFS_I(mp->m_rbmip)->i_atime;
 	if (!(mp->m_rbmip->i_d.di_flags & XFS_DIFLAG_NEWRTBM)) {
 		mp->m_rbmip->i_d.di_flags |= XFS_DIFLAG_NEWRTBM;
 		*seqp = 0;
diff --git a/fs/xfs/xfs_trans_inode.c b/fs/xfs/xfs_trans_inode.c
index b97f1df..3f0d466 100644
--- a/fs/xfs/xfs_trans_inode.c
+++ b/fs/xfs/xfs_trans_inode.c
@@ -75,18 +75,10 @@ xfs_trans_ichgtime(
 
 	tv = current_fs_time(inode->i_sb);
 
-	if ((flags & XFS_ICHGTIME_MOD) &&
-	    !timespec_equal(&inode->i_mtime, &tv)) {
+	if (flags & XFS_ICHGTIME_MOD)
 		inode->i_mtime = tv;
-		ip->i_d.di_mtime.t_sec = tv.tv_sec;
-		ip->i_d.di_mtime.t_nsec = tv.tv_nsec;
-	}
-	if ((flags & XFS_ICHGTIME_CHG) &&
-	    !timespec_equal(&inode->i_ctime, &tv)) {
+	if (flags & XFS_ICHGTIME_CHG)
 		inode->i_ctime = tv;
-		ip->i_d.di_ctime.t_sec = tv.tv_sec;
-		ip->i_d.di_ctime.t_nsec = tv.tv_nsec;
-	}
 }
 
 /*
-- 
2.5.0

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 2/8] xfs: remove timestamps from incore inode

[Brian Foster]

On Thu, Jan 14, 2016 at 05:09:19PM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> The struct xfs_inode has two copies of the current timestamps in it,
> one in the vfs inode and one in the struct xfs_icdinode. Now that we
> no longer log the struct xfs_icdinode directly, we don't need to
> keep the timestamps in this structure. instead we can copy them
> straight out of the VFS inode when formatting the inode log item or
> the on-disk inode.
> 
> This reduces the struct xfs_inode in size by 24 bytes.
> 
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> ---

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  fs/xfs/libxfs/xfs_inode_buf.c | 91 +++++++++++++++++++++++++++++++++++++------
>  fs/xfs/libxfs/xfs_inode_buf.h |  9 ++---
>  fs/xfs/libxfs/xfs_rtbitmap.c  |  2 +-
>  fs/xfs/xfs_inode.c            | 21 +++++-----
>  fs/xfs/xfs_inode_item.c       | 73 ++++++----------------------------
>  fs/xfs/xfs_inode_item.h       |  2 -
>  fs/xfs/xfs_iops.c             | 37 ++++--------------
>  fs/xfs/xfs_itable.c           | 16 +++++---
>  fs/xfs/xfs_log_recover.c      |  8 ++--
>  fs/xfs/xfs_rtalloc.c          |  2 +-
>  fs/xfs/xfs_trans_inode.c      | 12 +-----
>  11 files changed, 130 insertions(+), 143 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_inode_buf.c b/fs/xfs/libxfs/xfs_inode_buf.c
> index 63d46bf..26e3c46 100644
> --- a/fs/xfs/libxfs/xfs_inode_buf.c
> +++ b/fs/xfs/libxfs/xfs_inode_buf.c
> @@ -195,10 +195,13 @@ xfs_imap_to_bp(
>  }
>  
>  void
> -xfs_dinode_from_disk(
> -	struct xfs_icdinode	*to,
> +xfs_inode_from_disk(
> +	struct xfs_inode	*ip,
>  	struct xfs_dinode	*from)
>  {
> +	struct xfs_icdinode	*to = &ip->i_d;
> +	struct inode		*inode = VFS_I(ip);
> +
>  	to->di_magic = be16_to_cpu(from->di_magic);
>  	to->di_mode = be16_to_cpu(from->di_mode);
>  	to->di_version = from ->di_version;
> @@ -211,12 +214,20 @@ xfs_dinode_from_disk(
>  	to->di_projid_hi = be16_to_cpu(from->di_projid_hi);
>  	memcpy(to->di_pad, from->di_pad, sizeof(to->di_pad));
>  	to->di_flushiter = be16_to_cpu(from->di_flushiter);
> -	to->di_atime.t_sec = be32_to_cpu(from->di_atime.t_sec);
> -	to->di_atime.t_nsec = be32_to_cpu(from->di_atime.t_nsec);
> -	to->di_mtime.t_sec = be32_to_cpu(from->di_mtime.t_sec);
> -	to->di_mtime.t_nsec = be32_to_cpu(from->di_mtime.t_nsec);
> -	to->di_ctime.t_sec = be32_to_cpu(from->di_ctime.t_sec);
> -	to->di_ctime.t_nsec = be32_to_cpu(from->di_ctime.t_nsec);
> +
> +	/*
> +	 * time is signed, so need to convert to signed 32 bit before
> +	 * storing in inode timestamp which may be 64 bit. Otherwise
> +	 * a time before epoch is converted to a time long after epoch
> +	 * on 64 bit systems.
> +	 */
> +	inode->i_atime.tv_sec = (int)be32_to_cpu(from->di_atime.t_sec);
> +	inode->i_atime.tv_nsec = (int)be32_to_cpu(from->di_atime.t_nsec);
> +	inode->i_mtime.tv_sec = (int)be32_to_cpu(from->di_mtime.t_sec);
> +	inode->i_mtime.tv_nsec = (int)be32_to_cpu(from->di_mtime.t_nsec);
> +	inode->i_ctime.tv_sec = (int)be32_to_cpu(from->di_ctime.t_sec);
> +	inode->i_ctime.tv_nsec = (int)be32_to_cpu(from->di_ctime.t_nsec);
> +
>  	to->di_size = be64_to_cpu(from->di_size);
>  	to->di_nblocks = be64_to_cpu(from->di_nblocks);
>  	to->di_extsize = be32_to_cpu(from->di_extsize);
> @@ -242,9 +253,63 @@ xfs_dinode_from_disk(
>  }
>  
>  void
> -xfs_dinode_to_disk(
> -	struct xfs_dinode	*to,
> -	struct xfs_icdinode	*from)
> +xfs_inode_to_disk(
> +	struct xfs_inode	*ip,
> +	struct xfs_dinode	*to)
> +{
> +	struct xfs_icdinode	*from = &ip->i_d;
> +	struct inode		*inode = VFS_I(ip);
> +
> +	to->di_magic = cpu_to_be16(from->di_magic);
> +	to->di_mode = cpu_to_be16(from->di_mode);
> +	to->di_version = from ->di_version;
> +	to->di_format = from->di_format;
> +	to->di_onlink = cpu_to_be16(from->di_onlink);
> +	to->di_uid = cpu_to_be32(from->di_uid);
> +	to->di_gid = cpu_to_be32(from->di_gid);
> +	to->di_nlink = cpu_to_be32(from->di_nlink);
> +	to->di_projid_lo = cpu_to_be16(from->di_projid_lo);
> +	to->di_projid_hi = cpu_to_be16(from->di_projid_hi);
> +	memcpy(to->di_pad, from->di_pad, sizeof(to->di_pad));
> +
> +	to->di_atime.t_sec = cpu_to_be32(inode->i_atime.tv_sec);
> +	to->di_atime.t_nsec = cpu_to_be32(inode->i_atime.tv_nsec);
> +	to->di_mtime.t_sec = cpu_to_be32(inode->i_mtime.tv_sec);
> +	to->di_mtime.t_nsec = cpu_to_be32(inode->i_mtime.tv_nsec);
> +	to->di_ctime.t_sec = cpu_to_be32(inode->i_ctime.tv_sec);
> +	to->di_ctime.t_nsec = cpu_to_be32(inode->i_ctime.tv_nsec);
> +
> +	to->di_size = cpu_to_be64(from->di_size);
> +	to->di_nblocks = cpu_to_be64(from->di_nblocks);
> +	to->di_extsize = cpu_to_be32(from->di_extsize);
> +	to->di_nextents = cpu_to_be32(from->di_nextents);
> +	to->di_anextents = cpu_to_be16(from->di_anextents);
> +	to->di_forkoff = from->di_forkoff;
> +	to->di_aformat = from->di_aformat;
> +	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);
> +	to->di_gen = cpu_to_be32(from->di_gen);
> +
> +	if (from->di_version == 3) {
> +		to->di_changecount = cpu_to_be64(from->di_changecount);
> +		to->di_crtime.t_sec = cpu_to_be32(from->di_crtime.t_sec);
> +		to->di_crtime.t_nsec = cpu_to_be32(from->di_crtime.t_nsec);
> +		to->di_flags2 = cpu_to_be64(from->di_flags2);
> +		to->di_ino = cpu_to_be64(from->di_ino);
> +		to->di_lsn = cpu_to_be64(from->di_lsn);
> +		memcpy(to->di_pad2, from->di_pad2, sizeof(to->di_pad2));
> +		uuid_copy(&to->di_uuid, &from->di_uuid);
> +		to->di_flushiter = 0;
> +	} else {
> +		to->di_flushiter = cpu_to_be16(from->di_flushiter);
> +	}
> +}
> +
> +void
> +xfs_log_dinode_to_disk(
> +	struct xfs_log_dinode	*from,
> +	struct xfs_dinode	*to)
>  {
>  	to->di_magic = cpu_to_be16(from->di_magic);
>  	to->di_mode = cpu_to_be16(from->di_mode);
> @@ -257,12 +322,14 @@ xfs_dinode_to_disk(
>  	to->di_projid_lo = cpu_to_be16(from->di_projid_lo);
>  	to->di_projid_hi = cpu_to_be16(from->di_projid_hi);
>  	memcpy(to->di_pad, from->di_pad, sizeof(to->di_pad));
> +
>  	to->di_atime.t_sec = cpu_to_be32(from->di_atime.t_sec);
>  	to->di_atime.t_nsec = cpu_to_be32(from->di_atime.t_nsec);
>  	to->di_mtime.t_sec = cpu_to_be32(from->di_mtime.t_sec);
>  	to->di_mtime.t_nsec = cpu_to_be32(from->di_mtime.t_nsec);
>  	to->di_ctime.t_sec = cpu_to_be32(from->di_ctime.t_sec);
>  	to->di_ctime.t_nsec = cpu_to_be32(from->di_ctime.t_nsec);
> +
>  	to->di_size = cpu_to_be64(from->di_size);
>  	to->di_nblocks = cpu_to_be64(from->di_nblocks);
>  	to->di_extsize = cpu_to_be32(from->di_extsize);
> @@ -403,7 +470,7 @@ xfs_iread(
>  	 * Otherwise, just get the truly permanent information.
>  	 */
>  	if (dip->di_mode) {
> -		xfs_dinode_from_disk(&ip->i_d, dip);
> +		xfs_inode_from_disk(ip, dip);
>  		error = xfs_iformat_fork(ip, dip);
>  		if (error)  {
>  #ifdef DEBUG
> diff --git a/fs/xfs/libxfs/xfs_inode_buf.h b/fs/xfs/libxfs/xfs_inode_buf.h
> index 642f2a2..adcc9bf 100644
> --- a/fs/xfs/libxfs/xfs_inode_buf.h
> +++ b/fs/xfs/libxfs/xfs_inode_buf.h
> @@ -40,9 +40,6 @@ struct xfs_icdinode {
>  	__uint16_t	di_projid_hi;	/* higher part of owner's project id */
>  	__uint8_t	di_pad[6];	/* unused, zeroed space */
>  	__uint16_t	di_flushiter;	/* incremented on flush */
> -	xfs_ictimestamp_t di_atime;	/* time last accessed */
> -	xfs_ictimestamp_t di_mtime;	/* time last modified */
> -	xfs_ictimestamp_t di_ctime;	/* time created/inode modified */
>  	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 */
> @@ -89,8 +86,10 @@ int	xfs_imap_to_bp(struct xfs_mount *, struct xfs_trans *,
>  int	xfs_iread(struct xfs_mount *, struct xfs_trans *,
>  		  struct xfs_inode *, uint);
>  void	xfs_dinode_calc_crc(struct xfs_mount *, struct xfs_dinode *);
> -void	xfs_dinode_to_disk(struct xfs_dinode *to, struct xfs_icdinode *from);
> -void	xfs_dinode_from_disk(struct xfs_icdinode *to, struct xfs_dinode *from);
> +void	xfs_inode_to_disk(struct xfs_inode *ip, struct xfs_dinode *to);
> +void	xfs_inode_from_disk(struct xfs_inode *ip, struct xfs_dinode *from);
> +void	xfs_log_dinode_to_disk(struct xfs_log_dinode *from,
> +			       struct xfs_dinode *to);
>  
>  #if defined(DEBUG)
>  void	xfs_inobp_check(struct xfs_mount *, struct xfs_buf *);
> diff --git a/fs/xfs/libxfs/xfs_rtbitmap.c b/fs/xfs/libxfs/xfs_rtbitmap.c
> index 9b59ffa..acc71dd 100644
> --- a/fs/xfs/libxfs/xfs_rtbitmap.c
> +++ b/fs/xfs/libxfs/xfs_rtbitmap.c
> @@ -983,7 +983,7 @@ xfs_rtfree_extent(
>  	    mp->m_sb.sb_rextents) {
>  		if (!(mp->m_rbmip->i_d.di_flags & XFS_DIFLAG_NEWRTBM))
>  			mp->m_rbmip->i_d.di_flags |= XFS_DIFLAG_NEWRTBM;
> -		*(__uint64_t *)&mp->m_rbmip->i_d.di_atime = 0;
> +		*(__uint64_t *)&VFS_I(mp->m_rbmip)->i_atime = 0;
>  		xfs_trans_log_inode(tp, mp->m_rbmip, XFS_ILOG_CORE);
>  	}
>  	return 0;
> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> index 7e24232..3e9b8a0 100644
> --- a/fs/xfs/xfs_inode.c
> +++ b/fs/xfs/xfs_inode.c
> @@ -757,6 +757,7 @@ xfs_ialloc(
>  	uint		flags;
>  	int		error;
>  	struct timespec	tv;
> +	struct inode	*inode;
>  
>  	/*
>  	 * Call the space management code to pick
> @@ -782,6 +783,7 @@ xfs_ialloc(
>  	if (error)
>  		return error;
>  	ASSERT(ip != NULL);
> +	inode = VFS_I(ip);
>  
>  	/*
>  	 * We always convert v1 inodes to v2 now - we only support filesystems
> @@ -823,10 +825,9 @@ xfs_ialloc(
>  	ASSERT(ip->i_d.di_nblocks == 0);
>  
>  	tv = current_fs_time(mp->m_super);
> -	ip->i_d.di_mtime.t_sec = (__int32_t)tv.tv_sec;
> -	ip->i_d.di_mtime.t_nsec = (__int32_t)tv.tv_nsec;
> -	ip->i_d.di_atime = ip->i_d.di_mtime;
> -	ip->i_d.di_ctime = ip->i_d.di_mtime;
> +	inode->i_mtime = tv;
> +	inode->i_atime = tv;
> +	inode->i_ctime = tv;
>  
>  	/*
>  	 * di_gen will have been taken care of in xfs_iread.
> @@ -844,7 +845,8 @@ xfs_ialloc(
>  		ip->i_d.di_lsn = 0;
>  		ip->i_d.di_flags2 = 0;
>  		memset(&(ip->i_d.di_pad2[0]), 0, sizeof(ip->i_d.di_pad2));
> -		ip->i_d.di_crtime = ip->i_d.di_mtime;
> +		ip->i_d.di_crtime.t_sec = (__int32_t)tv.tv_sec;
> +		ip->i_d.di_crtime.t_nsec = (__int32_t)tv.tv_nsec;
>  	}
>  
>  
> @@ -3509,12 +3511,11 @@ xfs_iflush_int(
>  		ip->i_d.di_flushiter++;
>  
>  	/*
> -	 * Copy the dirty parts of the inode into the on-disk
> -	 * inode.  We always copy out the core of the inode,
> -	 * because if the inode is dirty at all the core must
> -	 * be.
> +	 * Copy the dirty parts of the inode into the on-disk inode.  We always
> +	 * copy out the core of the inode, because if the inode is dirty at all
> +	 * the core must be.
>  	 */
> -	xfs_dinode_to_disk(dip, &ip->i_d);
> +	xfs_inode_to_disk(ip, dip);
>  
>  	/* Wrap, we never let the log put out DI_MAX_FLUSH */
>  	if (ip->i_d.di_flushiter == DI_MAX_FLUSH)
> diff --git a/fs/xfs/xfs_inode_item.c b/fs/xfs/xfs_inode_item.c
> index 3ad9972..9dcbf58 100644
> --- a/fs/xfs/xfs_inode_item.c
> +++ b/fs/xfs/xfs_inode_item.c
> @@ -323,10 +323,13 @@ xfs_inode_item_format_attr_fork(
>  }
>  
>  static void
> -xfs_icdinode_to_log_dinode(
> -	struct xfs_icdinode	*from,
> +xfs_inode_to_log_dinode(
> +	struct xfs_inode	*ip,
>  	struct xfs_log_dinode	*to)
>  {
> +	struct xfs_icdinode	*from = &ip->i_d;
> +	struct inode		*inode = VFS_I(ip);
> +
>  	to->di_magic = from->di_magic;
>  	to->di_mode = from->di_mode;
>  	to->di_version = from->di_version;
> @@ -338,66 +341,14 @@ xfs_icdinode_to_log_dinode(
>  	to->di_projid_lo = from->di_projid_lo;
>  	to->di_projid_hi = from->di_projid_hi;
>  	memcpy(to->di_pad, from->di_pad, sizeof(to->di_pad));
> -	to->di_atime.t_sec = from->di_atime.t_sec;
> -	to->di_atime.t_nsec = from->di_atime.t_nsec;
> -	to->di_mtime.t_sec = from->di_mtime.t_sec;
> -	to->di_mtime.t_nsec = from->di_mtime.t_nsec;
> -	to->di_ctime.t_sec = from->di_ctime.t_sec;
> -	to->di_ctime.t_nsec = from->di_ctime.t_nsec;
> -	to->di_size = from->di_size;
> -	to->di_nblocks = from->di_nblocks;
> -	to->di_extsize = from->di_extsize;
> -	to->di_nextents = from->di_nextents;
> -	to->di_anextents = from->di_anextents;
> -	to->di_forkoff = from->di_forkoff;
> -	to->di_aformat = from->di_aformat;
> -	to->di_dmevmask = from->di_dmevmask;
> -	to->di_dmstate = from->di_dmstate;
> -	to->di_flags = from->di_flags;
> -	to->di_gen = from->di_gen;
>  
> -	if (from->di_version == 3) {
> -		to->di_changecount = from->di_changecount;
> -		to->di_crtime.t_sec = from->di_crtime.t_sec;
> -		to->di_crtime.t_nsec = from->di_crtime.t_nsec;
> -		to->di_flags2 = from->di_flags2;
> -		to->di_ino = from->di_ino;
> -		to->di_lsn = from->di_lsn;
> -		memcpy(to->di_pad2, from->di_pad2, sizeof(to->di_pad2));
> -		uuid_copy(&to->di_uuid, &from->di_uuid);
> -		to->di_flushiter = 0;
> -	} else {
> -		to->di_flushiter = from->di_flushiter;
> -	}
> -}
> +	to->di_atime.t_sec = inode->i_atime.tv_sec;
> +	to->di_atime.t_nsec = inode->i_atime.tv_nsec;
> +	to->di_mtime.t_sec = inode->i_mtime.tv_sec;
> +	to->di_mtime.t_nsec = inode->i_mtime.tv_nsec;
> +	to->di_ctime.t_sec = inode->i_ctime.tv_sec;
> +	to->di_ctime.t_nsec = inode->i_ctime.tv_nsec;
>  
> -/*
> - * Recovery needs to be able to convert a log dinode back to a real dinode
> - * for writeback we do that by converting a log dinode to a icdinode, and
> - * then passing that to the formatting function.
> - */
> -void
> -xfs_log_dinode_to_icdinode(
> -	struct xfs_log_dinode	*from,
> -	struct xfs_icdinode	*to)
> -{
> -	to->di_magic = from->di_magic;
> -	to->di_mode = from->di_mode;
> -	to->di_version = from->di_version;
> -	to->di_format = from->di_format;
> -	to->di_onlink = from->di_onlink;
> -	to->di_uid = from->di_uid;
> -	to->di_gid = from->di_gid;
> -	to->di_nlink = from->di_nlink;
> -	to->di_projid_lo = from->di_projid_lo;
> -	to->di_projid_hi = from->di_projid_hi;
> -	memset(to->di_pad, 0, sizeof(to->di_pad));
> -	to->di_atime.t_sec = from->di_atime.t_sec;
> -	to->di_atime.t_nsec = from->di_atime.t_nsec;
> -	to->di_mtime.t_sec = from->di_mtime.t_sec;
> -	to->di_mtime.t_nsec = from->di_mtime.t_nsec;
> -	to->di_ctime.t_sec = from->di_ctime.t_sec;
> -	to->di_ctime.t_nsec = from->di_ctime.t_nsec;
>  	to->di_size = from->di_size;
>  	to->di_nblocks = from->di_nblocks;
>  	to->di_extsize = from->di_extsize;
> @@ -439,7 +390,7 @@ xfs_inode_item_format_core(
>  	struct xfs_log_dinode	*dic;
>  
>  	dic = xlog_prepare_iovec(lv, vecp, XLOG_REG_TYPE_ICORE);
> -	xfs_icdinode_to_log_dinode(&ip->i_d, dic);
> +	xfs_inode_to_log_dinode(ip, dic);
>  	xlog_finish_iovec(lv, *vecp, xfs_log_dinode_size(ip->i_d.di_version));
>  }
>  
> diff --git a/fs/xfs/xfs_inode_item.h b/fs/xfs/xfs_inode_item.h
> index 2426118..4c7722e 100644
> --- a/fs/xfs/xfs_inode_item.h
> +++ b/fs/xfs/xfs_inode_item.h
> @@ -49,8 +49,6 @@ extern void xfs_istale_done(struct xfs_buf *, struct xfs_log_item *);
>  extern void xfs_iflush_abort(struct xfs_inode *, bool);
>  extern int xfs_inode_item_format_convert(xfs_log_iovec_t *,
>  					 xfs_inode_log_format_t *);
> -extern void xfs_log_dinode_to_icdinode(struct xfs_log_dinode *from,
> -				       struct xfs_icdinode *to);
>  
>  extern struct kmem_zone	*xfs_ili_zone;
>  
> diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
> index 245268a..b008677 100644
> --- a/fs/xfs/xfs_iops.c
> +++ b/fs/xfs/xfs_iops.c
> @@ -517,21 +517,12 @@ xfs_setattr_time(
>  
>  	ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
>  
> -	if (iattr->ia_valid & ATTR_ATIME) {
> +	if (iattr->ia_valid & ATTR_ATIME)
>  		inode->i_atime = iattr->ia_atime;
> -		ip->i_d.di_atime.t_sec = iattr->ia_atime.tv_sec;
> -		ip->i_d.di_atime.t_nsec = iattr->ia_atime.tv_nsec;
> -	}
> -	if (iattr->ia_valid & ATTR_CTIME) {
> +	if (iattr->ia_valid & ATTR_CTIME)
>  		inode->i_ctime = iattr->ia_ctime;
> -		ip->i_d.di_ctime.t_sec = iattr->ia_ctime.tv_sec;
> -		ip->i_d.di_ctime.t_nsec = iattr->ia_ctime.tv_nsec;
> -	}
> -	if (iattr->ia_valid & ATTR_MTIME) {
> +	if (iattr->ia_valid & ATTR_MTIME)
>  		inode->i_mtime = iattr->ia_mtime;
> -		ip->i_d.di_mtime.t_sec = iattr->ia_mtime.tv_sec;
> -		ip->i_d.di_mtime.t_nsec = iattr->ia_mtime.tv_nsec;
> -	}
>  }
>  
>  int
> @@ -986,21 +977,13 @@ xfs_vn_update_time(
>  	}
>  
>  	xfs_ilock(ip, XFS_ILOCK_EXCL);
> -	if (flags & S_CTIME) {
> +	if (flags & S_CTIME)
>  		inode->i_ctime = *now;
> -		ip->i_d.di_ctime.t_sec = (__int32_t)now->tv_sec;
> -		ip->i_d.di_ctime.t_nsec = (__int32_t)now->tv_nsec;
> -	}
> -	if (flags & S_MTIME) {
> +	if (flags & S_MTIME)
>  		inode->i_mtime = *now;
> -		ip->i_d.di_mtime.t_sec = (__int32_t)now->tv_sec;
> -		ip->i_d.di_mtime.t_nsec = (__int32_t)now->tv_nsec;
> -	}
> -	if (flags & S_ATIME) {
> +	if (flags & S_ATIME)
>  		inode->i_atime = *now;
> -		ip->i_d.di_atime.t_sec = (__int32_t)now->tv_sec;
> -		ip->i_d.di_atime.t_nsec = (__int32_t)now->tv_nsec;
> -	}
> +
>  	xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL);
>  	xfs_trans_log_inode(tp, ip, XFS_ILOG_TIMESTAMP);
>  	return xfs_trans_commit(tp);
> @@ -1247,12 +1230,6 @@ xfs_setup_inode(
>  
>  	inode->i_generation = ip->i_d.di_gen;
>  	i_size_write(inode, ip->i_d.di_size);
> -	inode->i_atime.tv_sec	= ip->i_d.di_atime.t_sec;
> -	inode->i_atime.tv_nsec	= ip->i_d.di_atime.t_nsec;
> -	inode->i_mtime.tv_sec	= ip->i_d.di_mtime.t_sec;
> -	inode->i_mtime.tv_nsec	= ip->i_d.di_mtime.t_nsec;
> -	inode->i_ctime.tv_sec	= ip->i_d.di_ctime.t_sec;
> -	inode->i_ctime.tv_nsec	= ip->i_d.di_ctime.t_nsec;
>  	xfs_diflags_to_iflags(inode, ip);
>  
>  	ip->d_ops = ip->i_mount->m_nondir_inode_ops;
> diff --git a/fs/xfs/xfs_itable.c b/fs/xfs/xfs_itable.c
> index 930ebd8..2acda42 100644
> --- a/fs/xfs/xfs_itable.c
> +++ b/fs/xfs/xfs_itable.c
> @@ -57,6 +57,7 @@ xfs_bulkstat_one_int(
>  {
>  	struct xfs_icdinode	*dic;		/* dinode core info pointer */
>  	struct xfs_inode	*ip;		/* incore inode pointer */
> +	struct inode		*inode;
>  	struct xfs_bstat	*buf;		/* return buffer */
>  	int			error = 0;	/* error value */
>  
> @@ -77,6 +78,7 @@ xfs_bulkstat_one_int(
>  
>  	ASSERT(ip != NULL);
>  	ASSERT(ip->i_imap.im_blkno != 0);
> +	inode = VFS_I(ip);
>  
>  	dic = &ip->i_d;
>  
> @@ -91,12 +93,14 @@ xfs_bulkstat_one_int(
>  	buf->bs_uid = dic->di_uid;
>  	buf->bs_gid = dic->di_gid;
>  	buf->bs_size = dic->di_size;
> -	buf->bs_atime.tv_sec = dic->di_atime.t_sec;
> -	buf->bs_atime.tv_nsec = dic->di_atime.t_nsec;
> -	buf->bs_mtime.tv_sec = dic->di_mtime.t_sec;
> -	buf->bs_mtime.tv_nsec = dic->di_mtime.t_nsec;
> -	buf->bs_ctime.tv_sec = dic->di_ctime.t_sec;
> -	buf->bs_ctime.tv_nsec = dic->di_ctime.t_nsec;
> +
> +	buf->bs_atime.tv_sec = inode->i_atime.tv_sec;
> +	buf->bs_atime.tv_nsec = inode->i_atime.tv_nsec;
> +	buf->bs_mtime.tv_sec = inode->i_mtime.tv_sec;
> +	buf->bs_mtime.tv_nsec = inode->i_mtime.tv_nsec;
> +	buf->bs_ctime.tv_sec = inode->i_ctime.tv_sec;
> +	buf->bs_ctime.tv_nsec = inode->i_ctime.tv_nsec;
> +
>  	buf->bs_xflags = xfs_ip2xflags(ip);
>  	buf->bs_extsize = dic->di_extsize << mp->m_sb.sb_blocklog;
>  	buf->bs_extents = dic->di_nextents;
> diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
> index 3120f7b..4b79cf0 100644
> --- a/fs/xfs/xfs_log_recover.c
> +++ b/fs/xfs/xfs_log_recover.c
> @@ -2793,7 +2793,7 @@ xfs_recover_inode_owner_change(
>  		return -ENOMEM;
>  
>  	/* instantiate the inode */
> -	xfs_dinode_from_disk(&ip->i_d, dip);
> +	xfs_inode_from_disk(ip, dip);
>  	ASSERT(ip->i_d.di_version >= 3);
>  
>  	error = xfs_iformat_fork(ip, dip);
> @@ -2840,7 +2840,6 @@ xlog_recover_inode_pass2(
>  	int			attr_index;
>  	uint			fields;
>  	struct xfs_log_dinode	*ldip;
> -	struct xfs_icdinode	icic;
>  	uint			isize;
>  	int			need_free = 0;
>  
> @@ -3007,9 +3006,8 @@ xlog_recover_inode_pass2(
>  		goto out_release;
>  	}
>  
> -	/* The core is in in-core format */
> -	xfs_log_dinode_to_icdinode(ldip, &icic);
> -	xfs_dinode_to_disk(dip, &icic);
> +	/* recover the log dinode inode into the on disk inode */
> +	xfs_log_dinode_to_disk(ldip, dip);
>  
>  	/* the rest is in on-disk format */
>  	if (item->ri_buf[1].i_len > isize) {
> diff --git a/fs/xfs/xfs_rtalloc.c b/fs/xfs/xfs_rtalloc.c
> index be02a68..abf4443 100644
> --- a/fs/xfs/xfs_rtalloc.c
> +++ b/fs/xfs/xfs_rtalloc.c
> @@ -1272,7 +1272,7 @@ xfs_rtpick_extent(
>  
>  	ASSERT(xfs_isilocked(mp->m_rbmip, XFS_ILOCK_EXCL));
>  
> -	seqp = (__uint64_t *)&mp->m_rbmip->i_d.di_atime;
> +	seqp = (__uint64_t *)&VFS_I(mp->m_rbmip)->i_atime;
>  	if (!(mp->m_rbmip->i_d.di_flags & XFS_DIFLAG_NEWRTBM)) {
>  		mp->m_rbmip->i_d.di_flags |= XFS_DIFLAG_NEWRTBM;
>  		*seqp = 0;
> diff --git a/fs/xfs/xfs_trans_inode.c b/fs/xfs/xfs_trans_inode.c
> index b97f1df..3f0d466 100644
> --- a/fs/xfs/xfs_trans_inode.c
> +++ b/fs/xfs/xfs_trans_inode.c
> @@ -75,18 +75,10 @@ xfs_trans_ichgtime(
>  
>  	tv = current_fs_time(inode->i_sb);
>  
> -	if ((flags & XFS_ICHGTIME_MOD) &&
> -	    !timespec_equal(&inode->i_mtime, &tv)) {
> +	if (flags & XFS_ICHGTIME_MOD)
>  		inode->i_mtime = tv;
> -		ip->i_d.di_mtime.t_sec = tv.tv_sec;
> -		ip->i_d.di_mtime.t_nsec = tv.tv_nsec;
> -	}
> -	if ((flags & XFS_ICHGTIME_CHG) &&
> -	    !timespec_equal(&inode->i_ctime, &tv)) {
> +	if (flags & XFS_ICHGTIME_CHG)
>  		inode->i_ctime = tv;
> -		ip->i_d.di_ctime.t_sec = tv.tv_sec;
> -		ip->i_d.di_ctime.t_nsec = tv.tv_nsec;
> -	}
>  }
>  
>  /*
> -- 
> 2.5.0
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 3/8] xfs; cull unnecessary icdinode fields

[Dave Chinner]

From: Dave Chinner <dchinner@redhat.com>

Now that the struct xfs_icdinode is not directly related to the
on-disk format, we can cull things in it we really don't need to
store:

	- magic number never changes
	- padding is not necessary
	- next_unlinked is never used
	- inode number is redundant
	- uuid is redundant
	- lsn is accessed directly from dinode
	- inode CRC is only accessed directly from dinode

Hence we can remove these from the struct xfs_icdinode and redirect
the code that uses them to the xfs_dinode appripriately.  This
reduces the size of the struct icdinode from 152 bytes to 88 bytes,
and removes a fair chunk of unnecessary code, too.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
 fs/xfs/libxfs/xfs_inode_buf.c | 39 +++++++++++++--------------------------
 fs/xfs/libxfs/xfs_inode_buf.h | 27 +++++++--------------------
 fs/xfs/xfs_inode.c            | 19 +------------------
 fs/xfs/xfs_inode_item.c       | 19 +++++++++++--------
 4 files changed, 32 insertions(+), 72 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_inode_buf.c b/fs/xfs/libxfs/xfs_inode_buf.c
index 26e3c46..17ff4e8 100644
--- a/fs/xfs/libxfs/xfs_inode_buf.c
+++ b/fs/xfs/libxfs/xfs_inode_buf.c
@@ -202,7 +202,6 @@ xfs_inode_from_disk(
 	struct xfs_icdinode	*to = &ip->i_d;
 	struct inode		*inode = VFS_I(ip);
 
-	to->di_magic = be16_to_cpu(from->di_magic);
 	to->di_mode = be16_to_cpu(from->di_mode);
 	to->di_version = from ->di_version;
 	to->di_format = from->di_format;
@@ -212,7 +211,6 @@ xfs_inode_from_disk(
 	to->di_nlink = 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);
-	memcpy(to->di_pad, from->di_pad, sizeof(to->di_pad));
 	to->di_flushiter = be16_to_cpu(from->di_flushiter);
 
 	/*
@@ -245,24 +243,22 @@ xfs_inode_from_disk(
 		to->di_crtime.t_sec = be32_to_cpu(from->di_crtime.t_sec);
 		to->di_crtime.t_nsec = be32_to_cpu(from->di_crtime.t_nsec);
 		to->di_flags2 = be64_to_cpu(from->di_flags2);
-		to->di_ino = be64_to_cpu(from->di_ino);
-		to->di_lsn = be64_to_cpu(from->di_lsn);
-		memcpy(to->di_pad2, from->di_pad2, sizeof(to->di_pad2));
-		uuid_copy(&to->di_uuid, &from->di_uuid);
 	}
 }
 
 void
 xfs_inode_to_disk(
 	struct xfs_inode	*ip,
-	struct xfs_dinode	*to)
+	struct xfs_dinode	*to,
+	xfs_lsn_t		lsn)
 {
 	struct xfs_icdinode	*from = &ip->i_d;
 	struct inode		*inode = VFS_I(ip);
 
-	to->di_magic = cpu_to_be16(from->di_magic);
+	to->di_magic = cpu_to_be16(XFS_DINODE_MAGIC);
+
 	to->di_mode = cpu_to_be16(from->di_mode);
-	to->di_version = from ->di_version;
+	to->di_version = from->di_version;
 	to->di_format = from->di_format;
 	to->di_onlink = cpu_to_be16(from->di_onlink);
 	to->di_uid = cpu_to_be32(from->di_uid);
@@ -270,8 +266,8 @@ xfs_inode_to_disk(
 	to->di_nlink = cpu_to_be32(from->di_nlink);
 	to->di_projid_lo = cpu_to_be16(from->di_projid_lo);
 	to->di_projid_hi = cpu_to_be16(from->di_projid_hi);
-	memcpy(to->di_pad, from->di_pad, sizeof(to->di_pad));
 
+	memset(to->di_pad, 0, sizeof(to->di_pad));
 	to->di_atime.t_sec = cpu_to_be32(inode->i_atime.tv_sec);
 	to->di_atime.t_nsec = cpu_to_be32(inode->i_atime.tv_nsec);
 	to->di_mtime.t_sec = cpu_to_be32(inode->i_mtime.tv_sec);
@@ -296,10 +292,11 @@ xfs_inode_to_disk(
 		to->di_crtime.t_sec = cpu_to_be32(from->di_crtime.t_sec);
 		to->di_crtime.t_nsec = cpu_to_be32(from->di_crtime.t_nsec);
 		to->di_flags2 = cpu_to_be64(from->di_flags2);
-		to->di_ino = cpu_to_be64(from->di_ino);
-		to->di_lsn = cpu_to_be64(from->di_lsn);
-		memcpy(to->di_pad2, from->di_pad2, sizeof(to->di_pad2));
-		uuid_copy(&to->di_uuid, &from->di_uuid);
+
+		to->di_ino = cpu_to_be64(ip->i_ino);
+		to->di_lsn = cpu_to_be64(lsn);
+		memset(to->di_pad2, 0, sizeof(to->di_pad2));
+		uuid_copy(&to->di_uuid, &ip->i_mount->m_sb.sb_meta_uuid);
 		to->di_flushiter = 0;
 	} else {
 		to->di_flushiter = cpu_to_be16(from->di_flushiter);
@@ -434,13 +431,10 @@ xfs_iread(
 	    !(mp->m_flags & XFS_MOUNT_IKEEP)) {
 		/* initialise the on-disk inode core */
 		memset(&ip->i_d, 0, sizeof(ip->i_d));
-		ip->i_d.di_magic = XFS_DINODE_MAGIC;
 		ip->i_d.di_gen = prandom_u32();
-		if (xfs_sb_version_hascrc(&mp->m_sb)) {
+		if (xfs_sb_version_hascrc(&mp->m_sb))
 			ip->i_d.di_version = 3;
-			ip->i_d.di_ino = ip->i_ino;
-			uuid_copy(&ip->i_d.di_uuid, &mp->m_sb.sb_meta_uuid);
-		} else
+		else
 			ip->i_d.di_version = 2;
 		return 0;
 	}
@@ -484,16 +478,10 @@ xfs_iread(
 		 * Partial initialisation of the in-core inode. Just the bits
 		 * that xfs_ialloc won't overwrite or relies on being correct.
 		 */
-		ip->i_d.di_magic = be16_to_cpu(dip->di_magic);
 		ip->i_d.di_version = dip->di_version;
 		ip->i_d.di_gen = be32_to_cpu(dip->di_gen);
 		ip->i_d.di_flushiter = be16_to_cpu(dip->di_flushiter);
 
-		if (dip->di_version == 3) {
-			ip->i_d.di_ino = be64_to_cpu(dip->di_ino);
-			uuid_copy(&ip->i_d.di_uuid, &dip->di_uuid);
-		}
-
 		/*
 		 * Make sure to pull in the mode here as well in
 		 * case the inode is released without being used.
@@ -514,7 +502,6 @@ xfs_iread(
 	 */
 	if (ip->i_d.di_version == 1) {
 		ip->i_d.di_version = 2;
-		memset(&(ip->i_d.di_pad[0]), 0, sizeof(ip->i_d.di_pad));
 		ip->i_d.di_nlink = ip->i_d.di_onlink;
 		ip->i_d.di_onlink = 0;
 		xfs_set_projid(ip, 0);
diff --git a/fs/xfs/libxfs/xfs_inode_buf.h b/fs/xfs/libxfs/xfs_inode_buf.h
index adcc9bf..69d626e 100644
--- a/fs/xfs/libxfs/xfs_inode_buf.h
+++ b/fs/xfs/libxfs/xfs_inode_buf.h
@@ -22,24 +22,22 @@ struct xfs_inode;
 struct xfs_dinode;
 
 /*
- * In memory representation of the XFS inode. This is held in the in-core
- * struct xfs_inode to represent the on disk values, but no longer needs to be
- * identical to the on-disk structure as it is always translated to on-disk
- * format specific structures at the appropriate time.
+ * In memory representation of the XFS inode. This is held in the in-core struct
+ * xfs_inode to represent the on disk values, but it's struct is in no way
+ * related to what is stored on disk. That is, this structure is always
+ * translated to on-disk format specific structures at the appropriate time.
  */
 struct xfs_icdinode {
-	__uint16_t	di_magic;	/* inode magic # = XFS_DINODE_MAGIC */
 	__uint16_t	di_mode;	/* mode and type of file */
 	__int8_t	di_version;	/* inode version */
 	__int8_t	di_format;	/* format of di_c data */
 	__uint16_t	di_onlink;	/* old number of links to file */
+	__uint16_t	di_flushiter;	/* incremented on flush */
 	__uint32_t	di_uid;		/* owner's user id */
 	__uint32_t	di_gid;		/* owner's group id */
 	__uint32_t	di_nlink;	/* number of links to file */
 	__uint16_t	di_projid_lo;	/* lower part of owner's project id */
 	__uint16_t	di_projid_hi;	/* higher part of owner's project id */
-	__uint8_t	di_pad[6];	/* unused, zeroed space */
-	__uint16_t	di_flushiter;	/* incremented on flush */
 	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 */
@@ -52,22 +50,10 @@ struct xfs_icdinode {
 	__uint16_t	di_flags;	/* random flags, XFS_DIFLAG_... */
 	__uint32_t	di_gen;		/* generation number */
 
-	/* di_next_unlinked is the only non-core field in the old dinode */
-	xfs_agino_t	di_next_unlinked;/* agi unlinked list ptr */
-
-	/* start of the extended dinode, writable fields */
-	__uint32_t	di_crc;		/* CRC of the inode */
 	__uint64_t	di_changecount;	/* number of attribute changes */
-	xfs_lsn_t	di_lsn;		/* flush sequence */
 	__uint64_t	di_flags2;	/* more random flags */
-	__uint8_t	di_pad2[16];	/* more padding for future expansion */
 
-	/* fields only written to during inode creation */
 	xfs_ictimestamp_t di_crtime;	/* time created */
-	xfs_ino_t	di_ino;		/* inode number */
-	uuid_t		di_uuid;	/* UUID of the filesystem */
-
-	/* structure must be padded to 64 bit alignment */
 };
 
 /*
@@ -86,7 +72,8 @@ int	xfs_imap_to_bp(struct xfs_mount *, struct xfs_trans *,
 int	xfs_iread(struct xfs_mount *, struct xfs_trans *,
 		  struct xfs_inode *, uint);
 void	xfs_dinode_calc_crc(struct xfs_mount *, struct xfs_dinode *);
-void	xfs_inode_to_disk(struct xfs_inode *ip, struct xfs_dinode *to);
+void	xfs_inode_to_disk(struct xfs_inode *ip, struct xfs_dinode *to,
+			  xfs_lsn_t lsn);
 void	xfs_inode_from_disk(struct xfs_inode *ip, struct xfs_dinode *from);
 void	xfs_log_dinode_to_disk(struct xfs_log_dinode *from,
 			       struct xfs_dinode *to);
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index 3e9b8a0..3f020b5 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -800,7 +800,6 @@ xfs_ialloc(
 	ip->i_d.di_uid = xfs_kuid_to_uid(current_fsuid());
 	ip->i_d.di_gid = xfs_kgid_to_gid(current_fsgid());
 	xfs_set_projid(ip, prid);
-	memset(&(ip->i_d.di_pad[0]), 0, sizeof(ip->i_d.di_pad));
 
 	if (pip && XFS_INHERIT_GID(pip)) {
 		ip->i_d.di_gid = pip->i_d.di_gid;
@@ -838,13 +837,8 @@ xfs_ialloc(
 	ip->i_d.di_flags = 0;
 
 	if (ip->i_d.di_version == 3) {
-		ASSERT(ip->i_d.di_ino == ino);
-		ASSERT(uuid_equal(&ip->i_d.di_uuid, &mp->m_sb.sb_meta_uuid));
-		ip->i_d.di_crc = 0;
 		ip->i_d.di_changecount = 1;
-		ip->i_d.di_lsn = 0;
 		ip->i_d.di_flags2 = 0;
-		memset(&(ip->i_d.di_pad2[0]), 0, sizeof(ip->i_d.di_pad2));
 		ip->i_d.di_crtime.t_sec = (__int32_t)tv.tv_sec;
 		ip->i_d.di_crtime.t_nsec = (__int32_t)tv.tv_nsec;
 	}
@@ -3450,13 +3444,6 @@ xfs_iflush_int(
 			__func__, ip->i_ino, be16_to_cpu(dip->di_magic), dip);
 		goto corrupt_out;
 	}
-	if (XFS_TEST_ERROR(ip->i_d.di_magic != XFS_DINODE_MAGIC,
-				mp, XFS_ERRTAG_IFLUSH_2, XFS_RANDOM_IFLUSH_2)) {
-		xfs_alert_tag(mp, XFS_PTAG_IFLUSH,
-			"%s: Bad inode %Lu, ptr 0x%p, magic number 0x%x",
-			__func__, ip->i_ino, ip, ip->i_d.di_magic);
-		goto corrupt_out;
-	}
 	if (S_ISREG(ip->i_d.di_mode)) {
 		if (XFS_TEST_ERROR(
 		    (ip->i_d.di_format != XFS_DINODE_FMT_EXTENTS) &&
@@ -3515,7 +3502,7 @@ xfs_iflush_int(
 	 * copy out the core of the inode, because if the inode is dirty at all
 	 * the core must be.
 	 */
-	xfs_inode_to_disk(ip, dip);
+	xfs_inode_to_disk(ip, dip, iip->ili_item.li_lsn);
 
 	/* Wrap, we never let the log put out DI_MAX_FLUSH */
 	if (ip->i_d.di_flushiter == DI_MAX_FLUSH)
@@ -3567,10 +3554,6 @@ xfs_iflush_int(
 	 */
 	xfs_buf_attach_iodone(bp, xfs_iflush_done, &iip->ili_item);
 
-	/* update the lsn in the on disk inode if required */
-	if (ip->i_d.di_version == 3)
-		dip->di_lsn = cpu_to_be64(iip->ili_item.li_lsn);
-
 	/* generate the checksum. */
 	xfs_dinode_calc_crc(mp, dip);
 
diff --git a/fs/xfs/xfs_inode_item.c b/fs/xfs/xfs_inode_item.c
index 9dcbf58..ae60087 100644
--- a/fs/xfs/xfs_inode_item.c
+++ b/fs/xfs/xfs_inode_item.c
@@ -325,12 +325,14 @@ xfs_inode_item_format_attr_fork(
 static void
 xfs_inode_to_log_dinode(
 	struct xfs_inode	*ip,
-	struct xfs_log_dinode	*to)
+	struct xfs_log_dinode	*to,
+	xfs_lsn_t		lsn)
 {
 	struct xfs_icdinode	*from = &ip->i_d;
 	struct inode		*inode = VFS_I(ip);
 
-	to->di_magic = from->di_magic;
+	to->di_magic = XFS_DINODE_MAGIC;
+
 	to->di_mode = from->di_mode;
 	to->di_version = from->di_version;
 	to->di_format = from->di_format;
@@ -340,8 +342,8 @@ xfs_inode_to_log_dinode(
 	to->di_nlink = from->di_nlink;
 	to->di_projid_lo = from->di_projid_lo;
 	to->di_projid_hi = from->di_projid_hi;
-	memcpy(to->di_pad, from->di_pad, sizeof(to->di_pad));
 
+	memset(to->di_pad, 0, sizeof(to->di_pad));
 	to->di_atime.t_sec = inode->i_atime.tv_sec;
 	to->di_atime.t_nsec = inode->i_atime.tv_nsec;
 	to->di_mtime.t_sec = inode->i_mtime.tv_sec;
@@ -366,10 +368,11 @@ xfs_inode_to_log_dinode(
 		to->di_crtime.t_sec = from->di_crtime.t_sec;
 		to->di_crtime.t_nsec = from->di_crtime.t_nsec;
 		to->di_flags2 = from->di_flags2;
-		to->di_ino = from->di_ino;
-		to->di_lsn = from->di_lsn;
-		memcpy(to->di_pad2, from->di_pad2, sizeof(to->di_pad2));
-		uuid_copy(&to->di_uuid, &from->di_uuid);
+
+		to->di_ino = ip->i_ino;
+		to->di_lsn = lsn;
+		memset(to->di_pad2, 0, sizeof(to->di_pad2));
+		uuid_copy(&to->di_uuid, &ip->i_mount->m_sb.sb_uuid);
 		to->di_flushiter = 0;
 	} else {
 		to->di_flushiter = from->di_flushiter;
@@ -390,7 +393,7 @@ xfs_inode_item_format_core(
 	struct xfs_log_dinode	*dic;
 
 	dic = xlog_prepare_iovec(lv, vecp, XLOG_REG_TYPE_ICORE);
-	xfs_inode_to_log_dinode(ip, dic);
+	xfs_inode_to_log_dinode(ip, dic, ip->i_itemp->ili_item.li_lsn);
 	xlog_finish_iovec(lv, *vecp, xfs_log_dinode_size(ip->i_d.di_version));
 }
 
-- 
2.5.0

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 3/8] xfs; cull unnecessary icdinode fields

[Brian Foster]

On Thu, Jan 14, 2016 at 05:09:20PM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> Now that the struct xfs_icdinode is not directly related to the
> on-disk format, we can cull things in it we really don't need to
> store:
> 
> 	- magic number never changes
> 	- padding is not necessary
> 	- next_unlinked is never used
> 	- inode number is redundant
> 	- uuid is redundant
> 	- lsn is accessed directly from dinode
> 	- inode CRC is only accessed directly from dinode
> 
> Hence we can remove these from the struct xfs_icdinode and redirect
> the code that uses them to the xfs_dinode appripriately.  This
> reduces the size of the struct icdinode from 152 bytes to 88 bytes,
> and removes a fair chunk of unnecessary code, too.
> 
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> ---

A couple nits and a minor potential bug: you have a semicolon in the
subject header rather than a colon...

>  fs/xfs/libxfs/xfs_inode_buf.c | 39 +++++++++++++--------------------------
>  fs/xfs/libxfs/xfs_inode_buf.h | 27 +++++++--------------------
>  fs/xfs/xfs_inode.c            | 19 +------------------
>  fs/xfs/xfs_inode_item.c       | 19 +++++++++++--------
>  4 files changed, 32 insertions(+), 72 deletions(-)
> 
...
> diff --git a/fs/xfs/libxfs/xfs_inode_buf.h b/fs/xfs/libxfs/xfs_inode_buf.h
> index adcc9bf..69d626e 100644
> --- a/fs/xfs/libxfs/xfs_inode_buf.h
> +++ b/fs/xfs/libxfs/xfs_inode_buf.h
> @@ -22,24 +22,22 @@ struct xfs_inode;
>  struct xfs_dinode;
>  
>  /*
> - * In memory representation of the XFS inode. This is held in the in-core
> - * struct xfs_inode to represent the on disk values, but no longer needs to be
> - * identical to the on-disk structure as it is always translated to on-disk
> - * format specific structures at the appropriate time.
> + * In memory representation of the XFS inode. This is held in the in-core struct
> + * xfs_inode to represent the on disk values, but it's struct is in no way

Nit:						     its

> + * related to what is stored on disk. That is, this structure is always
> + * translated to on-disk format specific structures at the appropriate time.

FWIW, it might be more clear to say something like: "This is held in the
in-core struct xfs_inode and represents on-disk values, but the
structure is not in on-disk format. In other words, the structure is
always translated to on-disk format specific structures ... "

>   */
>  struct xfs_icdinode {
> -	__uint16_t	di_magic;	/* inode magic # = XFS_DINODE_MAGIC */
>  	__uint16_t	di_mode;	/* mode and type of file */
>  	__int8_t	di_version;	/* inode version */
>  	__int8_t	di_format;	/* format of di_c data */
>  	__uint16_t	di_onlink;	/* old number of links to file */
> +	__uint16_t	di_flushiter;	/* incremented on flush */
>  	__uint32_t	di_uid;		/* owner's user id */
>  	__uint32_t	di_gid;		/* owner's group id */
>  	__uint32_t	di_nlink;	/* number of links to file */
>  	__uint16_t	di_projid_lo;	/* lower part of owner's project id */
>  	__uint16_t	di_projid_hi;	/* higher part of owner's project id */
> -	__uint8_t	di_pad[6];	/* unused, zeroed space */
> -	__uint16_t	di_flushiter;	/* incremented on flush */
>  	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_inode_item.c b/fs/xfs/xfs_inode_item.c
> index 9dcbf58..ae60087 100644
> --- a/fs/xfs/xfs_inode_item.c
> +++ b/fs/xfs/xfs_inode_item.c
> @@ -325,12 +325,14 @@ xfs_inode_item_format_attr_fork(
>  static void
>  xfs_inode_to_log_dinode(
>  	struct xfs_inode	*ip,
> -	struct xfs_log_dinode	*to)
> +	struct xfs_log_dinode	*to,
> +	xfs_lsn_t		lsn)
>  {
>  	struct xfs_icdinode	*from = &ip->i_d;
>  	struct inode		*inode = VFS_I(ip);
>  
> -	to->di_magic = from->di_magic;
> +	to->di_magic = XFS_DINODE_MAGIC;
> +
>  	to->di_mode = from->di_mode;
>  	to->di_version = from->di_version;
>  	to->di_format = from->di_format;
> @@ -340,8 +342,8 @@ xfs_inode_to_log_dinode(
>  	to->di_nlink = from->di_nlink;
>  	to->di_projid_lo = from->di_projid_lo;
>  	to->di_projid_hi = from->di_projid_hi;
> -	memcpy(to->di_pad, from->di_pad, sizeof(to->di_pad));
>  
> +	memset(to->di_pad, 0, sizeof(to->di_pad));
>  	to->di_atime.t_sec = inode->i_atime.tv_sec;
>  	to->di_atime.t_nsec = inode->i_atime.tv_nsec;
>  	to->di_mtime.t_sec = inode->i_mtime.tv_sec;
> @@ -366,10 +368,11 @@ xfs_inode_to_log_dinode(
>  		to->di_crtime.t_sec = from->di_crtime.t_sec;
>  		to->di_crtime.t_nsec = from->di_crtime.t_nsec;
>  		to->di_flags2 = from->di_flags2;
> -		to->di_ino = from->di_ino;
> -		to->di_lsn = from->di_lsn;
> -		memcpy(to->di_pad2, from->di_pad2, sizeof(to->di_pad2));
> -		uuid_copy(&to->di_uuid, &from->di_uuid);
> +
> +		to->di_ino = ip->i_ino;
> +		to->di_lsn = lsn;
> +		memset(to->di_pad2, 0, sizeof(to->di_pad2));
> +		uuid_copy(&to->di_uuid, &ip->i_mount->m_sb.sb_uuid);

Shouldn't this be sb_meta_uuid?

Brian

>  		to->di_flushiter = 0;
>  	} else {
>  		to->di_flushiter = from->di_flushiter;
> @@ -390,7 +393,7 @@ xfs_inode_item_format_core(
>  	struct xfs_log_dinode	*dic;
>  
>  	dic = xlog_prepare_iovec(lv, vecp, XLOG_REG_TYPE_ICORE);
> -	xfs_inode_to_log_dinode(ip, dic);
> +	xfs_inode_to_log_dinode(ip, dic, ip->i_itemp->ili_item.li_lsn);
>  	xlog_finish_iovec(lv, *vecp, xfs_log_dinode_size(ip->i_d.di_version));
>  }
>  
> -- 
> 2.5.0
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 4/8] xfs: move v1 inode conversion to xfs_inode_from_disk

[Dave Chinner]

From: Dave Chinner <dchinner@redhat.com>

So we don't have to carry an di_onlink variable around anymore, move
the inode conversion from v1 inode format to v2 inode format into
xfs_inode_from_disk(). This means we can remove the di_onlink fields
from the struct xfs_icdinode.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
 fs/xfs/libxfs/xfs_inode_buf.c  | 42 ++++++++++++++++++++----------------------
 fs/xfs/libxfs/xfs_inode_buf.h  |  1 -
 fs/xfs/libxfs/xfs_log_format.h |  2 +-
 fs/xfs/xfs_inode.c             |  2 --
 fs/xfs/xfs_inode_item.c        |  2 +-
 5 files changed, 22 insertions(+), 27 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_inode_buf.c b/fs/xfs/libxfs/xfs_inode_buf.c
index 17ff4e8..8a1b460 100644
--- a/fs/xfs/libxfs/xfs_inode_buf.c
+++ b/fs/xfs/libxfs/xfs_inode_buf.c
@@ -204,13 +204,25 @@ xfs_inode_from_disk(
 
 	to->di_mode = be16_to_cpu(from->di_mode);
 	to->di_version = from ->di_version;
+
+	/*
+	 * Convert v1 inodes immediately to v2 inode format as this is the
+	 * minimum inode version format we support in the rest of the code.
+	 */
+	if (to->di_version == 1) {
+		to->di_nlink = be16_to_cpu(from->di_onlink);
+		to->di_projid_lo = 0;
+		to->di_projid_hi = 0;
+		to->di_version = 2;
+	} else {
+		to->di_nlink = 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_format = from->di_format;
-	to->di_onlink = be16_to_cpu(from->di_onlink);
 	to->di_uid = be32_to_cpu(from->di_uid);
 	to->di_gid = be32_to_cpu(from->di_gid);
-	to->di_nlink = 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_flushiter = be16_to_cpu(from->di_flushiter);
 
 	/*
@@ -256,11 +268,11 @@ xfs_inode_to_disk(
 	struct inode		*inode = VFS_I(ip);
 
 	to->di_magic = cpu_to_be16(XFS_DINODE_MAGIC);
+	to->di_onlink = 0;
 
 	to->di_mode = cpu_to_be16(from->di_mode);
 	to->di_version = from->di_version;
 	to->di_format = from->di_format;
-	to->di_onlink = cpu_to_be16(from->di_onlink);
 	to->di_uid = cpu_to_be32(from->di_uid);
 	to->di_gid = cpu_to_be32(from->di_gid);
 	to->di_nlink = cpu_to_be32(from->di_nlink);
@@ -310,9 +322,9 @@ xfs_log_dinode_to_disk(
 {
 	to->di_magic = cpu_to_be16(from->di_magic);
 	to->di_mode = cpu_to_be16(from->di_mode);
-	to->di_version = from ->di_version;
+	to->di_version = from->di_version;
 	to->di_format = from->di_format;
-	to->di_onlink = cpu_to_be16(from->di_onlink);
+	to->di_onlink = 0;
 	to->di_uid = cpu_to_be32(from->di_uid);
 	to->di_gid = cpu_to_be32(from->di_gid);
 	to->di_nlink = cpu_to_be32(from->di_nlink);
@@ -492,21 +504,7 @@ xfs_iread(
 		ip->i_d.di_mode = 0;
 	}
 
-	/*
-	 * Automatically convert version 1 inode formats in memory to version 2
-	 * inode format. If the inode is modified, it will get logged and
-	 * rewritten as a version 2 inode. We can do this because we set the
-	 * superblock feature bit for v2 inodes unconditionally during mount
-	 * and it means the reast of the code can assume the inode version is 2
-	 * or higher.
-	 */
-	if (ip->i_d.di_version == 1) {
-		ip->i_d.di_version = 2;
-		ip->i_d.di_nlink = ip->i_d.di_onlink;
-		ip->i_d.di_onlink = 0;
-		xfs_set_projid(ip, 0);
-	}
-
+	ASSERT(ip->i_d.di_version >= 2);
 	ip->i_delayed_blks = 0;
 
 	/*
diff --git a/fs/xfs/libxfs/xfs_inode_buf.h b/fs/xfs/libxfs/xfs_inode_buf.h
index 69d626e..feb04e6 100644
--- a/fs/xfs/libxfs/xfs_inode_buf.h
+++ b/fs/xfs/libxfs/xfs_inode_buf.h
@@ -31,7 +31,6 @@ struct xfs_icdinode {
 	__uint16_t	di_mode;	/* mode and type of file */
 	__int8_t	di_version;	/* inode version */
 	__int8_t	di_format;	/* format of di_c data */
-	__uint16_t	di_onlink;	/* old number of links to file */
 	__uint16_t	di_flushiter;	/* incremented on flush */
 	__uint32_t	di_uid;		/* owner's user id */
 	__uint32_t	di_gid;		/* owner's group id */
diff --git a/fs/xfs/libxfs/xfs_log_format.h b/fs/xfs/libxfs/xfs_log_format.h
index d00ed63..03f90b9 100644
--- a/fs/xfs/libxfs/xfs_log_format.h
+++ b/fs/xfs/libxfs/xfs_log_format.h
@@ -369,7 +369,7 @@ struct xfs_log_dinode {
 	__uint16_t	di_mode;	/* mode and type of file */
 	__int8_t	di_version;	/* inode version */
 	__int8_t	di_format;	/* format of di_c data */
-	__uint16_t	di_onlink;	/* old number of links to file */
+	__uint8_t	di_pad3[2];	/* unused in v2/3 inodes */
 	__uint32_t	di_uid;		/* owner's user id */
 	__uint32_t	di_gid;		/* owner's group id */
 	__uint32_t	di_nlink;	/* number of links to file */
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index 3f020b5..914ec41 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -794,9 +794,7 @@ xfs_ialloc(
 		ip->i_d.di_version = 2;
 
 	ip->i_d.di_mode = mode;
-	ip->i_d.di_onlink = 0;
 	ip->i_d.di_nlink = nlink;
-	ASSERT(ip->i_d.di_nlink == nlink);
 	ip->i_d.di_uid = xfs_kuid_to_uid(current_fsuid());
 	ip->i_d.di_gid = xfs_kgid_to_gid(current_fsgid());
 	xfs_set_projid(ip, prid);
diff --git a/fs/xfs/xfs_inode_item.c b/fs/xfs/xfs_inode_item.c
index ae60087..d777b7a 100644
--- a/fs/xfs/xfs_inode_item.c
+++ b/fs/xfs/xfs_inode_item.c
@@ -336,7 +336,6 @@ xfs_inode_to_log_dinode(
 	to->di_mode = from->di_mode;
 	to->di_version = from->di_version;
 	to->di_format = from->di_format;
-	to->di_onlink = from->di_onlink;
 	to->di_uid = from->di_uid;
 	to->di_gid = from->di_gid;
 	to->di_nlink = from->di_nlink;
@@ -344,6 +343,7 @@ xfs_inode_to_log_dinode(
 	to->di_projid_hi = from->di_projid_hi;
 
 	memset(to->di_pad, 0, sizeof(to->di_pad));
+	memset(to->di_pad3, 0, sizeof(to->di_pad3));
 	to->di_atime.t_sec = inode->i_atime.tv_sec;
 	to->di_atime.t_nsec = inode->i_atime.tv_nsec;
 	to->di_mtime.t_sec = inode->i_mtime.tv_sec;
-- 
2.5.0

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 4/8] xfs: move v1 inode conversion to xfs_inode_from_disk

[Brian Foster]

On Thu, Jan 14, 2016 at 05:09:21PM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> So we don't have to carry an di_onlink variable around anymore, move
> the inode conversion from v1 inode format to v2 inode format into
> xfs_inode_from_disk(). This means we can remove the di_onlink fields
> from the struct xfs_icdinode.
> 
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> ---

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  fs/xfs/libxfs/xfs_inode_buf.c  | 42 ++++++++++++++++++++----------------------
>  fs/xfs/libxfs/xfs_inode_buf.h  |  1 -
>  fs/xfs/libxfs/xfs_log_format.h |  2 +-
>  fs/xfs/xfs_inode.c             |  2 --
>  fs/xfs/xfs_inode_item.c        |  2 +-
>  5 files changed, 22 insertions(+), 27 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_inode_buf.c b/fs/xfs/libxfs/xfs_inode_buf.c
> index 17ff4e8..8a1b460 100644
> --- a/fs/xfs/libxfs/xfs_inode_buf.c
> +++ b/fs/xfs/libxfs/xfs_inode_buf.c
> @@ -204,13 +204,25 @@ xfs_inode_from_disk(
>  
>  	to->di_mode = be16_to_cpu(from->di_mode);
>  	to->di_version = from ->di_version;
> +
> +	/*
> +	 * Convert v1 inodes immediately to v2 inode format as this is the
> +	 * minimum inode version format we support in the rest of the code.
> +	 */
> +	if (to->di_version == 1) {
> +		to->di_nlink = be16_to_cpu(from->di_onlink);
> +		to->di_projid_lo = 0;
> +		to->di_projid_hi = 0;
> +		to->di_version = 2;
> +	} else {
> +		to->di_nlink = 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_format = from->di_format;
> -	to->di_onlink = be16_to_cpu(from->di_onlink);
>  	to->di_uid = be32_to_cpu(from->di_uid);
>  	to->di_gid = be32_to_cpu(from->di_gid);
> -	to->di_nlink = 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_flushiter = be16_to_cpu(from->di_flushiter);
>  
>  	/*
> @@ -256,11 +268,11 @@ xfs_inode_to_disk(
>  	struct inode		*inode = VFS_I(ip);
>  
>  	to->di_magic = cpu_to_be16(XFS_DINODE_MAGIC);
> +	to->di_onlink = 0;
>  
>  	to->di_mode = cpu_to_be16(from->di_mode);
>  	to->di_version = from->di_version;
>  	to->di_format = from->di_format;
> -	to->di_onlink = cpu_to_be16(from->di_onlink);
>  	to->di_uid = cpu_to_be32(from->di_uid);
>  	to->di_gid = cpu_to_be32(from->di_gid);
>  	to->di_nlink = cpu_to_be32(from->di_nlink);
> @@ -310,9 +322,9 @@ xfs_log_dinode_to_disk(
>  {
>  	to->di_magic = cpu_to_be16(from->di_magic);
>  	to->di_mode = cpu_to_be16(from->di_mode);
> -	to->di_version = from ->di_version;
> +	to->di_version = from->di_version;
>  	to->di_format = from->di_format;
> -	to->di_onlink = cpu_to_be16(from->di_onlink);
> +	to->di_onlink = 0;
>  	to->di_uid = cpu_to_be32(from->di_uid);
>  	to->di_gid = cpu_to_be32(from->di_gid);
>  	to->di_nlink = cpu_to_be32(from->di_nlink);
> @@ -492,21 +504,7 @@ xfs_iread(
>  		ip->i_d.di_mode = 0;
>  	}
>  
> -	/*
> -	 * Automatically convert version 1 inode formats in memory to version 2
> -	 * inode format. If the inode is modified, it will get logged and
> -	 * rewritten as a version 2 inode. We can do this because we set the
> -	 * superblock feature bit for v2 inodes unconditionally during mount
> -	 * and it means the reast of the code can assume the inode version is 2
> -	 * or higher.
> -	 */
> -	if (ip->i_d.di_version == 1) {
> -		ip->i_d.di_version = 2;
> -		ip->i_d.di_nlink = ip->i_d.di_onlink;
> -		ip->i_d.di_onlink = 0;
> -		xfs_set_projid(ip, 0);
> -	}
> -
> +	ASSERT(ip->i_d.di_version >= 2);
>  	ip->i_delayed_blks = 0;
>  
>  	/*
> diff --git a/fs/xfs/libxfs/xfs_inode_buf.h b/fs/xfs/libxfs/xfs_inode_buf.h
> index 69d626e..feb04e6 100644
> --- a/fs/xfs/libxfs/xfs_inode_buf.h
> +++ b/fs/xfs/libxfs/xfs_inode_buf.h
> @@ -31,7 +31,6 @@ struct xfs_icdinode {
>  	__uint16_t	di_mode;	/* mode and type of file */
>  	__int8_t	di_version;	/* inode version */
>  	__int8_t	di_format;	/* format of di_c data */
> -	__uint16_t	di_onlink;	/* old number of links to file */
>  	__uint16_t	di_flushiter;	/* incremented on flush */
>  	__uint32_t	di_uid;		/* owner's user id */
>  	__uint32_t	di_gid;		/* owner's group id */
> diff --git a/fs/xfs/libxfs/xfs_log_format.h b/fs/xfs/libxfs/xfs_log_format.h
> index d00ed63..03f90b9 100644
> --- a/fs/xfs/libxfs/xfs_log_format.h
> +++ b/fs/xfs/libxfs/xfs_log_format.h
> @@ -369,7 +369,7 @@ struct xfs_log_dinode {
>  	__uint16_t	di_mode;	/* mode and type of file */
>  	__int8_t	di_version;	/* inode version */
>  	__int8_t	di_format;	/* format of di_c data */
> -	__uint16_t	di_onlink;	/* old number of links to file */
> +	__uint8_t	di_pad3[2];	/* unused in v2/3 inodes */
>  	__uint32_t	di_uid;		/* owner's user id */
>  	__uint32_t	di_gid;		/* owner's group id */
>  	__uint32_t	di_nlink;	/* number of links to file */
> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> index 3f020b5..914ec41 100644
> --- a/fs/xfs/xfs_inode.c
> +++ b/fs/xfs/xfs_inode.c
> @@ -794,9 +794,7 @@ xfs_ialloc(
>  		ip->i_d.di_version = 2;
>  
>  	ip->i_d.di_mode = mode;
> -	ip->i_d.di_onlink = 0;
>  	ip->i_d.di_nlink = nlink;
> -	ASSERT(ip->i_d.di_nlink == nlink);
>  	ip->i_d.di_uid = xfs_kuid_to_uid(current_fsuid());
>  	ip->i_d.di_gid = xfs_kgid_to_gid(current_fsgid());
>  	xfs_set_projid(ip, prid);
> diff --git a/fs/xfs/xfs_inode_item.c b/fs/xfs/xfs_inode_item.c
> index ae60087..d777b7a 100644
> --- a/fs/xfs/xfs_inode_item.c
> +++ b/fs/xfs/xfs_inode_item.c
> @@ -336,7 +336,6 @@ xfs_inode_to_log_dinode(
>  	to->di_mode = from->di_mode;
>  	to->di_version = from->di_version;
>  	to->di_format = from->di_format;
> -	to->di_onlink = from->di_onlink;
>  	to->di_uid = from->di_uid;
>  	to->di_gid = from->di_gid;
>  	to->di_nlink = from->di_nlink;
> @@ -344,6 +343,7 @@ xfs_inode_to_log_dinode(
>  	to->di_projid_hi = from->di_projid_hi;
>  
>  	memset(to->di_pad, 0, sizeof(to->di_pad));
> +	memset(to->di_pad3, 0, sizeof(to->di_pad3));
>  	to->di_atime.t_sec = inode->i_atime.tv_sec;
>  	to->di_atime.t_nsec = inode->i_atime.tv_nsec;
>  	to->di_mtime.t_sec = inode->i_mtime.tv_sec;
> -- 
> 2.5.0
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 5/8] xfs: use vfs inode nlink field everywhere

[Dave Chinner]

From: Dave Chinner <dchinner@redhat.com>

The Vfs tracks the inodenlink just like the xfs_icdinode. We can
remove the variable from the icdinode and use the vfs inode variable
everywhere, reducing the size of the xfs_icdinode by a further 4
bytes.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
 fs/xfs/libxfs/xfs_inode_buf.c |  6 ++--
 fs/xfs/libxfs/xfs_inode_buf.h |  1 -
 fs/xfs/xfs_inode.c            | 73 ++++++++++++++++++++-----------------------
 fs/xfs/xfs_inode.h            |  2 --
 fs/xfs/xfs_inode_item.c       |  2 +-
 fs/xfs/xfs_iops.c             |  3 +-
 fs/xfs/xfs_itable.c           |  2 +-
 fs/xfs/xfs_log_recover.c      |  2 +-
 8 files changed, 41 insertions(+), 50 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_inode_buf.c b/fs/xfs/libxfs/xfs_inode_buf.c
index 8a1b460..0dcaa9a 100644
--- a/fs/xfs/libxfs/xfs_inode_buf.c
+++ b/fs/xfs/libxfs/xfs_inode_buf.c
@@ -210,12 +210,12 @@ xfs_inode_from_disk(
 	 * minimum inode version format we support in the rest of the code.
 	 */
 	if (to->di_version == 1) {
-		to->di_nlink = be16_to_cpu(from->di_onlink);
+		set_nlink(inode, be16_to_cpu(from->di_onlink));
 		to->di_projid_lo = 0;
 		to->di_projid_hi = 0;
 		to->di_version = 2;
 	} else {
-		to->di_nlink = be32_to_cpu(from->di_nlink);
+		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);
 	}
@@ -275,7 +275,6 @@ 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_nlink = cpu_to_be32(from->di_nlink);
 	to->di_projid_lo = cpu_to_be16(from->di_projid_lo);
 	to->di_projid_hi = cpu_to_be16(from->di_projid_hi);
 
@@ -286,6 +285,7 @@ xfs_inode_to_disk(
 	to->di_mtime.t_nsec = cpu_to_be32(inode->i_mtime.tv_nsec);
 	to->di_ctime.t_sec = cpu_to_be32(inode->i_ctime.tv_sec);
 	to->di_ctime.t_nsec = cpu_to_be32(inode->i_ctime.tv_nsec);
+	to->di_nlink = cpu_to_be32(inode->i_nlink);
 
 	to->di_size = cpu_to_be64(from->di_size);
 	to->di_nblocks = cpu_to_be64(from->di_nblocks);
diff --git a/fs/xfs/libxfs/xfs_inode_buf.h b/fs/xfs/libxfs/xfs_inode_buf.h
index feb04e6..774d71f 100644
--- a/fs/xfs/libxfs/xfs_inode_buf.h
+++ b/fs/xfs/libxfs/xfs_inode_buf.h
@@ -34,7 +34,6 @@ 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 */
-	__uint32_t	di_nlink;	/* number of links to file */
 	__uint16_t	di_projid_lo;	/* lower part of owner's project id */
 	__uint16_t	di_projid_hi;	/* higher part of owner's project id */
 	xfs_fsize_t	di_size;	/* number of bytes in file */
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index 914ec41..9ee766b 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -58,7 +58,7 @@ kmem_zone_t *xfs_inode_zone;
 #define	XFS_ITRUNC_MAX_EXTENTS	2
 
 STATIC int xfs_iflush_int(xfs_inode_t *, xfs_buf_t *);
-
+STATIC int xfs_iunlink(xfs_trans_t *, xfs_inode_t *, bool);
 STATIC int xfs_iunlink_remove(xfs_trans_t *, xfs_inode_t *);
 
 /*
@@ -794,7 +794,7 @@ xfs_ialloc(
 		ip->i_d.di_version = 2;
 
 	ip->i_d.di_mode = mode;
-	ip->i_d.di_nlink = nlink;
+	set_nlink(inode, nlink);
 	ip->i_d.di_uid = xfs_kuid_to_uid(current_fsuid());
 	ip->i_d.di_gid = xfs_kgid_to_gid(current_fsgid());
 	xfs_set_projid(ip, prid);
@@ -1072,35 +1072,24 @@ xfs_dir_ialloc(
 }
 
 /*
- * Decrement the link count on an inode & log the change.
- * If this causes the link count to go to zero, initiate the
- * logging activity required to truncate a file.
+ * Decrement the link count on an inode & log the change.  If this causes the
+ * link count to go to zero, move the inode to AGI unlinked list so that it can
+ * be freed when the last active reference goes away via xfs_inactive().
  */
 int				/* error */
 xfs_droplink(
 	xfs_trans_t *tp,
 	xfs_inode_t *ip)
 {
-	int	error;
-
 	xfs_trans_ichgtime(tp, ip, XFS_ICHGTIME_CHG);
 
-	ASSERT (ip->i_d.di_nlink > 0);
-	ip->i_d.di_nlink--;
 	drop_nlink(VFS_I(ip));
 	xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
 
-	error = 0;
-	if (ip->i_d.di_nlink == 0) {
-		/*
-		 * We're dropping the last link to this file.
-		 * Move the on-disk inode to the AGI unlinked list.
-		 * From xfs_inactive() we will pull the inode from
-		 * the list and free it.
-		 */
-		error = xfs_iunlink(tp, ip);
-	}
-	return error;
+	if (VFS_I(ip)->i_nlink)
+		return 0;
+
+	return xfs_iunlink(tp, ip, false);
 }
 
 /*
@@ -1114,8 +1103,6 @@ xfs_bumplink(
 	xfs_trans_ichgtime(tp, ip, XFS_ICHGTIME_CHG);
 
 	ASSERT(ip->i_d.di_version > 1);
-	ASSERT(ip->i_d.di_nlink > 0 || (VFS_I(ip)->i_state & I_LINKABLE));
-	ip->i_d.di_nlink++;
 	inc_nlink(VFS_I(ip));
 	xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
 	return 0;
@@ -1373,8 +1360,7 @@ xfs_create_tmpfile(
 	 */
 	xfs_qm_vop_create_dqattach(tp, ip, udqp, gdqp, pdqp);
 
-	ip->i_d.di_nlink--;
-	error = xfs_iunlink(tp, ip);
+	error = xfs_iunlink(tp, ip, true);
 	if (error)
 		goto out_trans_cancel;
 
@@ -1472,7 +1458,10 @@ xfs_link(
 
 	xfs_bmap_init(&free_list, &first_block);
 
-	if (sip->i_d.di_nlink == 0) {
+	/*
+	 * Handle initial link state of O_TMPFILE inode
+	 */
+	if (VFS_I(sip)->i_nlink == 0) {
 		error = xfs_iunlink_remove(tp, sip);
 		if (error)
 			goto error_return;
@@ -1659,7 +1648,7 @@ xfs_release(
 		}
 	}
 
-	if (ip->i_d.di_nlink == 0)
+	if (VFS_I(ip)->i_nlink == 0)
 		return 0;
 
 	if (xfs_can_free_eofblocks(ip, false)) {
@@ -1875,7 +1864,7 @@ xfs_inactive(
 	if (mp->m_flags & XFS_MOUNT_RDONLY)
 		return;
 
-	if (ip->i_d.di_nlink != 0) {
+	if (VFS_I(ip)->i_nlink != 0) {
 		/*
 		 * force is true because we are evicting an inode from the
 		 * cache. Post-eof blocks must be freed, lest we end up with
@@ -1932,14 +1921,20 @@ xfs_inactive(
 }
 
 /*
- * This is called when the inode's link count goes to 0.
- * We place the on-disk inode on a list in the AGI.  It
- * will be pulled from this list when the inode is freed.
+ * This is called when the inode's link count goes to 0 or we are creating a
+ * tmpfile via O_TMPFILE. In the case of a tmpfile, @ignore_linkcount will be
+ * set to true as the link count is dropped to zero by the VFS after we've
+ * created the file successfully, so we have to add it to the unlinked list
+ * while the link count is non-zero.
+ *
+ * We place the on-disk inode on a list in the AGI.  It will be pulled from this
+ * list when the inode is freed.
  */
-int
+STATIC int
 xfs_iunlink(
-	xfs_trans_t	*tp,
-	xfs_inode_t	*ip)
+	struct xfs_trans *tp,
+	struct xfs_inode *ip,
+	bool		ignore_linkcount)
 {
 	xfs_mount_t	*mp;
 	xfs_agi_t	*agi;
@@ -1951,7 +1946,7 @@ xfs_iunlink(
 	int		offset;
 	int		error;
 
-	ASSERT(ip->i_d.di_nlink == 0);
+	ASSERT(VFS_I(ip)->i_nlink == 0 || ignore_linkcount);
 	ASSERT(ip->i_d.di_mode != 0);
 
 	mp = tp->t_mountp;
@@ -2392,7 +2387,7 @@ xfs_ifree(
 	struct xfs_icluster	xic = { 0 };
 
 	ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
-	ASSERT(ip->i_d.di_nlink == 0);
+	ASSERT(VFS_I(ip)->i_nlink == 0);
 	ASSERT(ip->i_d.di_nextents == 0);
 	ASSERT(ip->i_d.di_anextents == 0);
 	ASSERT(ip->i_d.di_size == 0 || !S_ISREG(ip->i_d.di_mode));
@@ -2560,8 +2555,8 @@ xfs_remove(
 	 * If we're removing a directory perform some additional validation.
 	 */
 	if (is_dir) {
-		ASSERT(ip->i_d.di_nlink >= 2);
-		if (ip->i_d.di_nlink != 2) {
+		ASSERT(VFS_I(ip)->i_nlink >= 2);
+		if (VFS_I(ip)->i_nlink != 2) {
 			error = -ENOTEMPTY;
 			goto out_trans_cancel;
 		}
@@ -3017,7 +3012,7 @@ xfs_rename(
 			 * Make sure target dir is empty.
 			 */
 			if (!(xfs_dir_isempty(target_ip)) ||
-			    (target_ip->i_d.di_nlink > 2)) {
+			    (VFS_I(target_ip)->i_nlink > 2)) {
 				error = -EEXIST;
 				goto out_trans_cancel;
 			}
@@ -3124,7 +3119,7 @@ xfs_rename(
 	 * intermediate state on disk.
 	 */
 	if (wip) {
-		ASSERT(VFS_I(wip)->i_nlink == 0 && wip->i_d.di_nlink == 0);
+		ASSERT(VFS_I(wip)->i_nlink == 0);
 		error = xfs_bumplink(tp, wip);
 		if (error)
 			goto out_bmap_cancel;
diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
index aef5452..e74d13d 100644
--- a/fs/xfs/xfs_inode.h
+++ b/fs/xfs/xfs_inode.h
@@ -405,8 +405,6 @@ int		xfs_ifree(struct xfs_trans *, xfs_inode_t *,
 			   struct xfs_bmap_free *);
 int		xfs_itruncate_extents(struct xfs_trans **, struct xfs_inode *,
 				      int, xfs_fsize_t);
-int		xfs_iunlink(struct xfs_trans *, xfs_inode_t *);
-
 void		xfs_iext_realloc(xfs_inode_t *, int, int);
 
 void		xfs_iunpin_wait(xfs_inode_t *);
diff --git a/fs/xfs/xfs_inode_item.c b/fs/xfs/xfs_inode_item.c
index d777b7a..4e3611d 100644
--- a/fs/xfs/xfs_inode_item.c
+++ b/fs/xfs/xfs_inode_item.c
@@ -338,7 +338,6 @@ 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_nlink = from->di_nlink;
 	to->di_projid_lo = from->di_projid_lo;
 	to->di_projid_hi = from->di_projid_hi;
 
@@ -350,6 +349,7 @@ xfs_inode_to_log_dinode(
 	to->di_mtime.t_nsec = inode->i_mtime.tv_nsec;
 	to->di_ctime.t_sec = inode->i_ctime.tv_sec;
 	to->di_ctime.t_nsec = inode->i_ctime.tv_nsec;
+	to->di_nlink = inode->i_nlink;
 
 	to->di_size = from->di_size;
 	to->di_nblocks = from->di_nblocks;
diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
index b008677..c424d4b 100644
--- a/fs/xfs/xfs_iops.c
+++ b/fs/xfs/xfs_iops.c
@@ -455,7 +455,7 @@ xfs_vn_getattr(
 	stat->size = XFS_ISIZE(ip);
 	stat->dev = inode->i_sb->s_dev;
 	stat->mode = ip->i_d.di_mode;
-	stat->nlink = ip->i_d.di_nlink;
+	stat->nlink = inode->i_nlink;
 	stat->uid = inode->i_uid;
 	stat->gid = inode->i_gid;
 	stat->ino = ip->i_ino;
@@ -1212,7 +1212,6 @@ xfs_setup_inode(
 	hlist_add_fake(&inode->i_hash);
 
 	inode->i_mode	= ip->i_d.di_mode;
-	set_nlink(inode, ip->i_d.di_nlink);
 	inode->i_uid    = xfs_uid_to_kuid(ip->i_d.di_uid);
 	inode->i_gid    = xfs_gid_to_kgid(ip->i_d.di_gid);
 
diff --git a/fs/xfs/xfs_itable.c b/fs/xfs/xfs_itable.c
index 2acda42..cfb6527 100644
--- a/fs/xfs/xfs_itable.c
+++ b/fs/xfs/xfs_itable.c
@@ -85,7 +85,6 @@ xfs_bulkstat_one_int(
 	/* xfs_iget returns the following without needing
 	 * further change.
 	 */
-	buf->bs_nlink = dic->di_nlink;
 	buf->bs_projid_lo = dic->di_projid_lo;
 	buf->bs_projid_hi = dic->di_projid_hi;
 	buf->bs_ino = ino;
@@ -94,6 +93,7 @@ xfs_bulkstat_one_int(
 	buf->bs_gid = dic->di_gid;
 	buf->bs_size = dic->di_size;
 
+	buf->bs_nlink = inode->i_nlink;
 	buf->bs_atime.tv_sec = inode->i_atime.tv_sec;
 	buf->bs_atime.tv_nsec = inode->i_atime.tv_nsec;
 	buf->bs_mtime.tv_sec = inode->i_mtime.tv_sec;
diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
index 4b79cf0..611c25c 100644
--- a/fs/xfs/xfs_log_recover.c
+++ b/fs/xfs/xfs_log_recover.c
@@ -4337,7 +4337,7 @@ xlog_recover_process_one_iunlink(
 	if (error)
 		goto fail_iput;
 
-	ASSERT(ip->i_d.di_nlink == 0);
+	ASSERT(VFS_I(ip)->i_nlink == 0);
 	ASSERT(ip->i_d.di_mode != 0);
 
 	/* setup for the next pass */
-- 
2.5.0

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 5/8] xfs: use vfs inode nlink field everywhere

[Brian Foster]

On Thu, Jan 14, 2016 at 05:09:22PM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> The Vfs tracks the inodenlink just like the xfs_icdinode. We can
> remove the variable from the icdinode and use the vfs inode variable
> everywhere, reducing the size of the xfs_icdinode by a further 4
> bytes.
> 
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> ---
>  fs/xfs/libxfs/xfs_inode_buf.c |  6 ++--
>  fs/xfs/libxfs/xfs_inode_buf.h |  1 -
>  fs/xfs/xfs_inode.c            | 73 ++++++++++++++++++++-----------------------
>  fs/xfs/xfs_inode.h            |  2 --
>  fs/xfs/xfs_inode_item.c       |  2 +-
>  fs/xfs/xfs_iops.c             |  3 +-
>  fs/xfs/xfs_itable.c           |  2 +-
>  fs/xfs/xfs_log_recover.c      |  2 +-
>  8 files changed, 41 insertions(+), 50 deletions(-)
> 
...
> diff --git a/fs/xfs/libxfs/xfs_inode_buf.h b/fs/xfs/libxfs/xfs_inode_buf.h
> index feb04e6..774d71f 100644
> --- a/fs/xfs/libxfs/xfs_inode_buf.h
> +++ b/fs/xfs/libxfs/xfs_inode_buf.h
> @@ -34,7 +34,6 @@ 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 */
> -	__uint32_t	di_nlink;	/* number of links to file */
>  	__uint16_t	di_projid_lo;	/* lower part of owner's project id */
>  	__uint16_t	di_projid_hi;	/* higher part of owner's project id */
>  	xfs_fsize_t	di_size;	/* number of bytes in file */
> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> index 914ec41..9ee766b 100644
> --- a/fs/xfs/xfs_inode.c
> +++ b/fs/xfs/xfs_inode.c
> @@ -58,7 +58,7 @@ kmem_zone_t *xfs_inode_zone;
>  #define	XFS_ITRUNC_MAX_EXTENTS	2
>  
>  STATIC int xfs_iflush_int(xfs_inode_t *, xfs_buf_t *);
> -
> +STATIC int xfs_iunlink(xfs_trans_t *, xfs_inode_t *, bool);

Nit:			 (struct xfs_trans *, struct xfs_inode *, ...)

Might as well fix the neighbors as well I suppose.

>  STATIC int xfs_iunlink_remove(xfs_trans_t *, xfs_inode_t *);
>  
>  /*
...
> diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
> index b008677..c424d4b 100644
> --- a/fs/xfs/xfs_iops.c
> +++ b/fs/xfs/xfs_iops.c
> @@ -455,7 +455,7 @@ xfs_vn_getattr(
>  	stat->size = XFS_ISIZE(ip);
>  	stat->dev = inode->i_sb->s_dev;
>  	stat->mode = ip->i_d.di_mode;
> -	stat->nlink = ip->i_d.di_nlink;
> +	stat->nlink = inode->i_nlink;
>  	stat->uid = inode->i_uid;
>  	stat->gid = inode->i_gid;
>  	stat->ino = ip->i_ino;
> @@ -1212,7 +1212,6 @@ xfs_setup_inode(
>  	hlist_add_fake(&inode->i_hash);
>  
>  	inode->i_mode	= ip->i_d.di_mode;
> -	set_nlink(inode, ip->i_d.di_nlink);
>  	inode->i_uid    = xfs_uid_to_kuid(ip->i_d.di_uid);
>  	inode->i_gid    = xfs_gid_to_kgid(ip->i_d.di_gid);
>  

I'm wondering if we have an issue here if we happen to get an inode
that's reclaimable. E.g., we get an xfs_fs_destroy_inode() call on an
inode and set the reclaimable tag. IIUC, a lookup that occurs after that
point but before we actually reclaim the inode can go through the
associated XFS_IRECLAIMABLE section of xfs_iget_cache_hit(). In there,
we call inode_init_always() which fixes inode->i_nlink = 1.

It looks like we'd call into here (xfs_setup_inode()) since we set
XFS_INEW, and thus previously this would update inode->i_nlink based on
the ip->i_d.di_nlink value, but I don't see where that would happen
now..  hmm?

Brian

> diff --git a/fs/xfs/xfs_itable.c b/fs/xfs/xfs_itable.c
> index 2acda42..cfb6527 100644
> --- a/fs/xfs/xfs_itable.c
> +++ b/fs/xfs/xfs_itable.c
> @@ -85,7 +85,6 @@ xfs_bulkstat_one_int(
>  	/* xfs_iget returns the following without needing
>  	 * further change.
>  	 */
> -	buf->bs_nlink = dic->di_nlink;
>  	buf->bs_projid_lo = dic->di_projid_lo;
>  	buf->bs_projid_hi = dic->di_projid_hi;
>  	buf->bs_ino = ino;
> @@ -94,6 +93,7 @@ xfs_bulkstat_one_int(
>  	buf->bs_gid = dic->di_gid;
>  	buf->bs_size = dic->di_size;
>  
> +	buf->bs_nlink = inode->i_nlink;
>  	buf->bs_atime.tv_sec = inode->i_atime.tv_sec;
>  	buf->bs_atime.tv_nsec = inode->i_atime.tv_nsec;
>  	buf->bs_mtime.tv_sec = inode->i_mtime.tv_sec;
> diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
> index 4b79cf0..611c25c 100644
> --- a/fs/xfs/xfs_log_recover.c
> +++ b/fs/xfs/xfs_log_recover.c
> @@ -4337,7 +4337,7 @@ xlog_recover_process_one_iunlink(
>  	if (error)
>  		goto fail_iput;
>  
> -	ASSERT(ip->i_d.di_nlink == 0);
> +	ASSERT(VFS_I(ip)->i_nlink == 0);
>  	ASSERT(ip->i_d.di_mode != 0);
>  
>  	/* setup for the next pass */
> -- 
> 2.5.0
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 6/8] xfs: move inode generation count to VFS inode

[Dave Chinner]

From: Dave Chinner <dchinner@redhat.com>

Pull another 4 bytes out of the xfs_icdinode.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
 fs/xfs/libxfs/xfs_inode_buf.c | 8 ++++----
 fs/xfs/libxfs/xfs_inode_buf.h | 1 -
 fs/xfs/xfs_export.c           | 2 +-
 fs/xfs/xfs_inode.c            | 5 +----
 fs/xfs/xfs_inode_item.c       | 2 +-
 fs/xfs/xfs_ioctl.c            | 2 +-
 fs/xfs/xfs_iops.c             | 1 -
 fs/xfs/xfs_itable.c           | 2 +-
 8 files changed, 9 insertions(+), 14 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_inode_buf.c b/fs/xfs/libxfs/xfs_inode_buf.c
index 0dcaa9a..0a3f10b 100644
--- a/fs/xfs/libxfs/xfs_inode_buf.c
+++ b/fs/xfs/libxfs/xfs_inode_buf.c
@@ -237,6 +237,7 @@ xfs_inode_from_disk(
 	inode->i_mtime.tv_nsec = (int)be32_to_cpu(from->di_mtime.t_nsec);
 	inode->i_ctime.tv_sec = (int)be32_to_cpu(from->di_ctime.t_sec);
 	inode->i_ctime.tv_nsec = (int)be32_to_cpu(from->di_ctime.t_nsec);
+	inode->i_generation = be32_to_cpu(from->di_gen);
 
 	to->di_size = be64_to_cpu(from->di_size);
 	to->di_nblocks = be64_to_cpu(from->di_nblocks);
@@ -248,7 +249,6 @@ xfs_inode_from_disk(
 	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);
-	to->di_gen	= be32_to_cpu(from->di_gen);
 
 	if (to->di_version == 3) {
 		to->di_changecount = be64_to_cpu(from->di_changecount);
@@ -286,6 +286,7 @@ xfs_inode_to_disk(
 	to->di_ctime.t_sec = cpu_to_be32(inode->i_ctime.tv_sec);
 	to->di_ctime.t_nsec = cpu_to_be32(inode->i_ctime.tv_nsec);
 	to->di_nlink = cpu_to_be32(inode->i_nlink);
+	to->di_gen = cpu_to_be32(inode->i_generation);
 
 	to->di_size = cpu_to_be64(from->di_size);
 	to->di_nblocks = cpu_to_be64(from->di_nblocks);
@@ -297,7 +298,6 @@ xfs_inode_to_disk(
 	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);
-	to->di_gen = cpu_to_be32(from->di_gen);
 
 	if (from->di_version == 3) {
 		to->di_changecount = cpu_to_be64(from->di_changecount);
@@ -443,7 +443,7 @@ xfs_iread(
 	    !(mp->m_flags & XFS_MOUNT_IKEEP)) {
 		/* initialise the on-disk inode core */
 		memset(&ip->i_d, 0, sizeof(ip->i_d));
-		ip->i_d.di_gen = prandom_u32();
+		VFS_I(ip)->i_generation = prandom_u32();
 		if (xfs_sb_version_hascrc(&mp->m_sb))
 			ip->i_d.di_version = 3;
 		else
@@ -491,7 +491,7 @@ xfs_iread(
 		 * that xfs_ialloc won't overwrite or relies on being correct.
 		 */
 		ip->i_d.di_version = dip->di_version;
-		ip->i_d.di_gen = be32_to_cpu(dip->di_gen);
+		VFS_I(ip)->i_generation = be32_to_cpu(dip->di_gen);
 		ip->i_d.di_flushiter = be16_to_cpu(dip->di_flushiter);
 
 		/*
diff --git a/fs/xfs/libxfs/xfs_inode_buf.h b/fs/xfs/libxfs/xfs_inode_buf.h
index 774d71f..68da576 100644
--- a/fs/xfs/libxfs/xfs_inode_buf.h
+++ b/fs/xfs/libxfs/xfs_inode_buf.h
@@ -46,7 +46,6 @@ struct xfs_icdinode {
 	__uint32_t	di_dmevmask;	/* DMIG event mask */
 	__uint16_t	di_dmstate;	/* DMIG state info */
 	__uint16_t	di_flags;	/* random flags, XFS_DIFLAG_... */
-	__uint32_t	di_gen;		/* generation number */
 
 	__uint64_t	di_changecount;	/* number of attribute changes */
 	__uint64_t	di_flags2;	/* more random flags */
diff --git a/fs/xfs/xfs_export.c b/fs/xfs/xfs_export.c
index 652cd3c..2816d42 100644
--- a/fs/xfs/xfs_export.c
+++ b/fs/xfs/xfs_export.c
@@ -152,7 +152,7 @@ xfs_nfs_get_inode(
 		return ERR_PTR(error);
 	}
 
-	if (ip->i_d.di_gen != generation) {
+	if (VFS_I(ip)->i_generation != generation) {
 		IRELE(ip);
 		return ERR_PTR(-ESTALE);
 	}
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index 9ee766b..391d676 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -826,9 +826,6 @@ xfs_ialloc(
 	inode->i_atime = tv;
 	inode->i_ctime = tv;
 
-	/*
-	 * di_gen will have been taken care of in xfs_iread.
-	 */
 	ip->i_d.di_extsize = 0;
 	ip->i_d.di_dmevmask = 0;
 	ip->i_d.di_dmstate = 0;
@@ -2414,7 +2411,7 @@ xfs_ifree(
 	 * Bump the generation count so no one will be confused
 	 * by reincarnations of this inode.
 	 */
-	ip->i_d.di_gen++;
+	VFS_I(ip)->i_generation++;
 	xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
 
 	if (xic.deleted)
diff --git a/fs/xfs/xfs_inode_item.c b/fs/xfs/xfs_inode_item.c
index 4e3611d..5cdf5ef 100644
--- a/fs/xfs/xfs_inode_item.c
+++ b/fs/xfs/xfs_inode_item.c
@@ -350,6 +350,7 @@ xfs_inode_to_log_dinode(
 	to->di_ctime.t_sec = inode->i_ctime.tv_sec;
 	to->di_ctime.t_nsec = inode->i_ctime.tv_nsec;
 	to->di_nlink = inode->i_nlink;
+	to->di_gen = inode->i_generation;
 
 	to->di_size = from->di_size;
 	to->di_nblocks = from->di_nblocks;
@@ -361,7 +362,6 @@ xfs_inode_to_log_dinode(
 	to->di_dmevmask = from->di_dmevmask;
 	to->di_dmstate = from->di_dmstate;
 	to->di_flags = from->di_flags;
-	to->di_gen = from->di_gen;
 
 	if (from->di_version == 3) {
 		to->di_changecount = from->di_changecount;
diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
index d42738d..2c40c5f 100644
--- a/fs/xfs/xfs_ioctl.c
+++ b/fs/xfs/xfs_ioctl.c
@@ -114,7 +114,7 @@ xfs_find_handle(
 		handle.ha_fid.fid_len = sizeof(xfs_fid_t) -
 					sizeof(handle.ha_fid.fid_len);
 		handle.ha_fid.fid_pad = 0;
-		handle.ha_fid.fid_gen = ip->i_d.di_gen;
+		handle.ha_fid.fid_gen = inode->i_generation;
 		handle.ha_fid.fid_ino = ip->i_ino;
 
 		hsize = XFS_HSIZE(handle);
diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
index c424d4b..397ce85 100644
--- a/fs/xfs/xfs_iops.c
+++ b/fs/xfs/xfs_iops.c
@@ -1227,7 +1227,6 @@ xfs_setup_inode(
 		break;
 	}
 
-	inode->i_generation = ip->i_d.di_gen;
 	i_size_write(inode, ip->i_d.di_size);
 	xfs_diflags_to_iflags(inode, ip);
 
diff --git a/fs/xfs/xfs_itable.c b/fs/xfs/xfs_itable.c
index cfb6527..6162e65 100644
--- a/fs/xfs/xfs_itable.c
+++ b/fs/xfs/xfs_itable.c
@@ -100,11 +100,11 @@ xfs_bulkstat_one_int(
 	buf->bs_mtime.tv_nsec = inode->i_mtime.tv_nsec;
 	buf->bs_ctime.tv_sec = inode->i_ctime.tv_sec;
 	buf->bs_ctime.tv_nsec = inode->i_ctime.tv_nsec;
+	buf->bs_gen = inode->i_generation;
 
 	buf->bs_xflags = xfs_ip2xflags(ip);
 	buf->bs_extsize = dic->di_extsize << mp->m_sb.sb_blocklog;
 	buf->bs_extents = dic->di_nextents;
-	buf->bs_gen = dic->di_gen;
 	memset(buf->bs_pad, 0, sizeof(buf->bs_pad));
 	buf->bs_dmevmask = dic->di_dmevmask;
 	buf->bs_dmstate = dic->di_dmstate;
-- 
2.5.0

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 6/8] xfs: move inode generation count to VFS inode

[Brian Foster]

On Thu, Jan 14, 2016 at 05:09:23PM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> Pull another 4 bytes out of the xfs_icdinode.
> 
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> ---
>  fs/xfs/libxfs/xfs_inode_buf.c | 8 ++++----
>  fs/xfs/libxfs/xfs_inode_buf.h | 1 -
>  fs/xfs/xfs_export.c           | 2 +-
>  fs/xfs/xfs_inode.c            | 5 +----
>  fs/xfs/xfs_inode_item.c       | 2 +-
>  fs/xfs/xfs_ioctl.c            | 2 +-
>  fs/xfs/xfs_iops.c             | 1 -
>  fs/xfs/xfs_itable.c           | 2 +-
>  8 files changed, 9 insertions(+), 14 deletions(-)
> 
...
> diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
> index c424d4b..397ce85 100644
> --- a/fs/xfs/xfs_iops.c
> +++ b/fs/xfs/xfs_iops.c
> @@ -1227,7 +1227,6 @@ xfs_setup_inode(
>  		break;
>  	}
>  
> -	inode->i_generation = ip->i_d.di_gen;
>  	i_size_write(inode, ip->i_d.di_size);
>  	xfs_diflags_to_iflags(inode, ip);
>  

Same question here wrt to reclaimable inodes as the previous patch and
nlink..? Seems Ok otherwise.

Brian

> diff --git a/fs/xfs/xfs_itable.c b/fs/xfs/xfs_itable.c
> index cfb6527..6162e65 100644
> --- a/fs/xfs/xfs_itable.c
> +++ b/fs/xfs/xfs_itable.c
> @@ -100,11 +100,11 @@ xfs_bulkstat_one_int(
>  	buf->bs_mtime.tv_nsec = inode->i_mtime.tv_nsec;
>  	buf->bs_ctime.tv_sec = inode->i_ctime.tv_sec;
>  	buf->bs_ctime.tv_nsec = inode->i_ctime.tv_nsec;
> +	buf->bs_gen = inode->i_generation;
>  
>  	buf->bs_xflags = xfs_ip2xflags(ip);
>  	buf->bs_extsize = dic->di_extsize << mp->m_sb.sb_blocklog;
>  	buf->bs_extents = dic->di_nextents;
> -	buf->bs_gen = dic->di_gen;
>  	memset(buf->bs_pad, 0, sizeof(buf->bs_pad));
>  	buf->bs_dmevmask = dic->di_dmevmask;
>  	buf->bs_dmstate = dic->di_dmstate;
> -- 
> 2.5.0
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 7/8] xfs: move di_changecount to VFS inode

[Dave Chinner]

From: Dave Chinner <dchinner@redhat.com>

We can store the di_changecount in the i_version field of the VFS
inode and remove another 8 bytes from the xfs_icdinode.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
 fs/xfs/libxfs/xfs_inode_buf.c | 4 ++--
 fs/xfs/libxfs/xfs_inode_buf.h | 1 -
 fs/xfs/xfs_inode.c            | 2 +-
 fs/xfs/xfs_inode_item.c       | 2 +-
 fs/xfs/xfs_trans_inode.c      | 2 +-
 5 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_inode_buf.c b/fs/xfs/libxfs/xfs_inode_buf.c
index 0a3f10b..9807c21 100644
--- a/fs/xfs/libxfs/xfs_inode_buf.c
+++ b/fs/xfs/libxfs/xfs_inode_buf.c
@@ -251,7 +251,7 @@ xfs_inode_from_disk(
 	to->di_flags	= be16_to_cpu(from->di_flags);
 
 	if (to->di_version == 3) {
-		to->di_changecount = be64_to_cpu(from->di_changecount);
+		inode->i_version = be64_to_cpu(from->di_changecount);
 		to->di_crtime.t_sec = be32_to_cpu(from->di_crtime.t_sec);
 		to->di_crtime.t_nsec = be32_to_cpu(from->di_crtime.t_nsec);
 		to->di_flags2 = be64_to_cpu(from->di_flags2);
@@ -300,7 +300,7 @@ xfs_inode_to_disk(
 	to->di_flags = cpu_to_be16(from->di_flags);
 
 	if (from->di_version == 3) {
-		to->di_changecount = cpu_to_be64(from->di_changecount);
+		to->di_changecount = cpu_to_be64(inode->i_version);
 		to->di_crtime.t_sec = cpu_to_be32(from->di_crtime.t_sec);
 		to->di_crtime.t_nsec = cpu_to_be32(from->di_crtime.t_nsec);
 		to->di_flags2 = cpu_to_be64(from->di_flags2);
diff --git a/fs/xfs/libxfs/xfs_inode_buf.h b/fs/xfs/libxfs/xfs_inode_buf.h
index 68da576..e49c229 100644
--- a/fs/xfs/libxfs/xfs_inode_buf.h
+++ b/fs/xfs/libxfs/xfs_inode_buf.h
@@ -47,7 +47,6 @@ struct xfs_icdinode {
 	__uint16_t	di_dmstate;	/* DMIG state info */
 	__uint16_t	di_flags;	/* random flags, XFS_DIFLAG_... */
 
-	__uint64_t	di_changecount;	/* number of attribute changes */
 	__uint64_t	di_flags2;	/* more random flags */
 
 	xfs_ictimestamp_t di_crtime;	/* time created */
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index 391d676..96aee23 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -832,7 +832,7 @@ xfs_ialloc(
 	ip->i_d.di_flags = 0;
 
 	if (ip->i_d.di_version == 3) {
-		ip->i_d.di_changecount = 1;
+		inode->i_version = 1;
 		ip->i_d.di_flags2 = 0;
 		ip->i_d.di_crtime.t_sec = (__int32_t)tv.tv_sec;
 		ip->i_d.di_crtime.t_nsec = (__int32_t)tv.tv_nsec;
diff --git a/fs/xfs/xfs_inode_item.c b/fs/xfs/xfs_inode_item.c
index 5cdf5ef..334d657 100644
--- a/fs/xfs/xfs_inode_item.c
+++ b/fs/xfs/xfs_inode_item.c
@@ -364,7 +364,7 @@ xfs_inode_to_log_dinode(
 	to->di_flags = from->di_flags;
 
 	if (from->di_version == 3) {
-		to->di_changecount = from->di_changecount;
+		to->di_changecount = inode->i_version;
 		to->di_crtime.t_sec = from->di_crtime.t_sec;
 		to->di_crtime.t_nsec = from->di_crtime.t_nsec;
 		to->di_flags2 = from->di_flags2;
diff --git a/fs/xfs/xfs_trans_inode.c b/fs/xfs/xfs_trans_inode.c
index 3f0d466..11a3af0 100644
--- a/fs/xfs/xfs_trans_inode.c
+++ b/fs/xfs/xfs_trans_inode.c
@@ -117,7 +117,7 @@ xfs_trans_log_inode(
 	 */
 	if (!(ip->i_itemp->ili_item.li_desc->lid_flags & XFS_LID_DIRTY) &&
 	    IS_I_VERSION(VFS_I(ip))) {
-		ip->i_d.di_changecount = ++VFS_I(ip)->i_version;
+		VFS_I(ip)->i_version++;
 		flags |= XFS_ILOG_CORE;
 	}
 
-- 
2.5.0

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 7/8] xfs: move di_changecount to VFS inode

[Brian Foster]

On Thu, Jan 14, 2016 at 05:09:24PM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> We can store the di_changecount in the i_version field of the VFS
> inode and remove another 8 bytes from the xfs_icdinode.
> 
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> ---

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  fs/xfs/libxfs/xfs_inode_buf.c | 4 ++--
>  fs/xfs/libxfs/xfs_inode_buf.h | 1 -
>  fs/xfs/xfs_inode.c            | 2 +-
>  fs/xfs/xfs_inode_item.c       | 2 +-
>  fs/xfs/xfs_trans_inode.c      | 2 +-
>  5 files changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_inode_buf.c b/fs/xfs/libxfs/xfs_inode_buf.c
> index 0a3f10b..9807c21 100644
> --- a/fs/xfs/libxfs/xfs_inode_buf.c
> +++ b/fs/xfs/libxfs/xfs_inode_buf.c
> @@ -251,7 +251,7 @@ xfs_inode_from_disk(
>  	to->di_flags	= be16_to_cpu(from->di_flags);
>  
>  	if (to->di_version == 3) {
> -		to->di_changecount = be64_to_cpu(from->di_changecount);
> +		inode->i_version = be64_to_cpu(from->di_changecount);
>  		to->di_crtime.t_sec = be32_to_cpu(from->di_crtime.t_sec);
>  		to->di_crtime.t_nsec = be32_to_cpu(from->di_crtime.t_nsec);
>  		to->di_flags2 = be64_to_cpu(from->di_flags2);
> @@ -300,7 +300,7 @@ xfs_inode_to_disk(
>  	to->di_flags = cpu_to_be16(from->di_flags);
>  
>  	if (from->di_version == 3) {
> -		to->di_changecount = cpu_to_be64(from->di_changecount);
> +		to->di_changecount = cpu_to_be64(inode->i_version);
>  		to->di_crtime.t_sec = cpu_to_be32(from->di_crtime.t_sec);
>  		to->di_crtime.t_nsec = cpu_to_be32(from->di_crtime.t_nsec);
>  		to->di_flags2 = cpu_to_be64(from->di_flags2);
> diff --git a/fs/xfs/libxfs/xfs_inode_buf.h b/fs/xfs/libxfs/xfs_inode_buf.h
> index 68da576..e49c229 100644
> --- a/fs/xfs/libxfs/xfs_inode_buf.h
> +++ b/fs/xfs/libxfs/xfs_inode_buf.h
> @@ -47,7 +47,6 @@ struct xfs_icdinode {
>  	__uint16_t	di_dmstate;	/* DMIG state info */
>  	__uint16_t	di_flags;	/* random flags, XFS_DIFLAG_... */
>  
> -	__uint64_t	di_changecount;	/* number of attribute changes */
>  	__uint64_t	di_flags2;	/* more random flags */
>  
>  	xfs_ictimestamp_t di_crtime;	/* time created */
> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> index 391d676..96aee23 100644
> --- a/fs/xfs/xfs_inode.c
> +++ b/fs/xfs/xfs_inode.c
> @@ -832,7 +832,7 @@ xfs_ialloc(
>  	ip->i_d.di_flags = 0;
>  
>  	if (ip->i_d.di_version == 3) {
> -		ip->i_d.di_changecount = 1;
> +		inode->i_version = 1;
>  		ip->i_d.di_flags2 = 0;
>  		ip->i_d.di_crtime.t_sec = (__int32_t)tv.tv_sec;
>  		ip->i_d.di_crtime.t_nsec = (__int32_t)tv.tv_nsec;
> diff --git a/fs/xfs/xfs_inode_item.c b/fs/xfs/xfs_inode_item.c
> index 5cdf5ef..334d657 100644
> --- a/fs/xfs/xfs_inode_item.c
> +++ b/fs/xfs/xfs_inode_item.c
> @@ -364,7 +364,7 @@ xfs_inode_to_log_dinode(
>  	to->di_flags = from->di_flags;
>  
>  	if (from->di_version == 3) {
> -		to->di_changecount = from->di_changecount;
> +		to->di_changecount = inode->i_version;
>  		to->di_crtime.t_sec = from->di_crtime.t_sec;
>  		to->di_crtime.t_nsec = from->di_crtime.t_nsec;
>  		to->di_flags2 = from->di_flags2;
> diff --git a/fs/xfs/xfs_trans_inode.c b/fs/xfs/xfs_trans_inode.c
> index 3f0d466..11a3af0 100644
> --- a/fs/xfs/xfs_trans_inode.c
> +++ b/fs/xfs/xfs_trans_inode.c
> @@ -117,7 +117,7 @@ xfs_trans_log_inode(
>  	 */
>  	if (!(ip->i_itemp->ili_item.li_desc->lid_flags & XFS_LID_DIRTY) &&
>  	    IS_I_VERSION(VFS_I(ip))) {
> -		ip->i_d.di_changecount = ++VFS_I(ip)->i_version;
> +		VFS_I(ip)->i_version++;
>  		flags |= XFS_ILOG_CORE;
>  	}
>  
> -- 
> 2.5.0
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 8/8] xfs: mode di_mode to vfs inode

[Dave Chinner]

From: Dave Chinner <dchinner@redhat.com>

Move the di_mode value from the xfs_icdinode to the VFS inode, reducing
the xfs_icdinode byte another 2 bytes and collapsing another 2 byte hole
in the structure.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
 fs/fs-writeback.c              |  1 +
 fs/xfs/libxfs/xfs_bmap.c       |  6 +++---
 fs/xfs/libxfs/xfs_dir2.c       | 12 +++++------
 fs/xfs/libxfs/xfs_inode_buf.c  |  8 +++----
 fs/xfs/libxfs/xfs_inode_buf.h  |  1 -
 fs/xfs/libxfs/xfs_inode_fork.c |  2 +-
 fs/xfs/xfs_bmap_util.c         |  4 ++--
 fs/xfs/xfs_dir2_readdir.c      |  2 +-
 fs/xfs/xfs_file.c              |  6 +++---
 fs/xfs/xfs_filestream.c        |  4 ++--
 fs/xfs/xfs_icache.c            | 42 +++++++++++++++++++++++++++++++-----
 fs/xfs/xfs_inode.c             | 48 ++++++++++++++++++++----------------------
 fs/xfs/xfs_inode.h             |  4 ++--
 fs/xfs/xfs_inode_item.c        |  2 +-
 fs/xfs/xfs_ioctl.c             | 14 ++++++------
 fs/xfs/xfs_iops.c              | 12 ++++-------
 fs/xfs/xfs_itable.c            |  2 +-
 fs/xfs/xfs_log_recover.c       |  2 +-
 fs/xfs/xfs_mount.c             |  2 +-
 19 files changed, 100 insertions(+), 74 deletions(-)

diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
index 023f6a1..0858457 100644
--- a/fs/fs-writeback.c
+++ b/fs/fs-writeback.c
@@ -2106,6 +2106,7 @@ EXPORT_SYMBOL(__mark_inode_dirty);
  * completed by the time we have gained the lock and waited for all IO that is
  * in progress regardless of the order callers are granted the lock.
  */
+#include "linux/delay.h"
 static void wait_sb_inodes(struct super_block *sb)
 {
 	struct inode *inode, *old_inode = NULL;
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index ef00156..6a05166 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -912,7 +912,7 @@ xfs_bmap_local_to_extents(
 	 * We don't want to deal with the case of keeping inode data inline yet.
 	 * So sending the data fork of a regular inode is invalid.
 	 */
-	ASSERT(!(S_ISREG(ip->i_d.di_mode) && whichfork == XFS_DATA_FORK));
+	ASSERT(!(S_ISREG(VFS_I(ip)->i_mode) && whichfork == XFS_DATA_FORK));
 	ifp = XFS_IFORK_PTR(ip, whichfork);
 	ASSERT(XFS_IFORK_FORMAT(ip, whichfork) == XFS_DINODE_FMT_LOCAL);
 
@@ -1079,7 +1079,7 @@ xfs_bmap_add_attrfork_local(
 	if (ip->i_df.if_bytes <= XFS_IFORK_DSIZE(ip))
 		return 0;
 
-	if (S_ISDIR(ip->i_d.di_mode)) {
+	if (S_ISDIR(VFS_I(ip)->i_mode)) {
 		memset(&dargs, 0, sizeof(dargs));
 		dargs.geo = ip->i_mount->m_dir_geo;
 		dargs.dp = ip;
@@ -1091,7 +1091,7 @@ xfs_bmap_add_attrfork_local(
 		return xfs_dir2_sf_to_block(&dargs);
 	}
 
-	if (S_ISLNK(ip->i_d.di_mode))
+	if (S_ISLNK(VFS_I(ip)->i_mode))
 		return xfs_bmap_local_to_extents(tp, ip, firstblock, 1,
 						 flags, XFS_DATA_FORK,
 						 xfs_symlink_local_to_remote);
diff --git a/fs/xfs/libxfs/xfs_dir2.c b/fs/xfs/libxfs/xfs_dir2.c
index 2fb53a5..af0f9d1 100644
--- a/fs/xfs/libxfs/xfs_dir2.c
+++ b/fs/xfs/libxfs/xfs_dir2.c
@@ -176,7 +176,7 @@ xfs_dir_isempty(
 {
 	xfs_dir2_sf_hdr_t	*sfp;
 
-	ASSERT(S_ISDIR(dp->i_d.di_mode));
+	ASSERT(S_ISDIR(VFS_I(dp)->i_mode));
 	if (dp->i_d.di_size == 0)	/* might happen during shutdown. */
 		return 1;
 	if (dp->i_d.di_size > XFS_IFORK_DSIZE(dp))
@@ -231,7 +231,7 @@ xfs_dir_init(
 	struct xfs_da_args *args;
 	int		error;
 
-	ASSERT(S_ISDIR(dp->i_d.di_mode));
+	ASSERT(S_ISDIR(VFS_I(dp)->i_mode));
 	error = xfs_dir_ino_validate(tp->t_mountp, pdp->i_ino);
 	if (error)
 		return error;
@@ -266,7 +266,7 @@ xfs_dir_createname(
 	int			rval;
 	int			v;		/* type-checking value */
 
-	ASSERT(S_ISDIR(dp->i_d.di_mode));
+	ASSERT(S_ISDIR(VFS_I(dp)->i_mode));
 	if (inum) {
 		rval = xfs_dir_ino_validate(tp->t_mountp, inum);
 		if (rval)
@@ -364,7 +364,7 @@ xfs_dir_lookup(
 	int		v;		/* type-checking value */
 	int		lock_mode;
 
-	ASSERT(S_ISDIR(dp->i_d.di_mode));
+	ASSERT(S_ISDIR(VFS_I(dp)->i_mode));
 	XFS_STATS_INC(dp->i_mount, xs_dir_lookup);
 
 	/*
@@ -443,7 +443,7 @@ xfs_dir_removename(
 	int		rval;
 	int		v;		/* type-checking value */
 
-	ASSERT(S_ISDIR(dp->i_d.di_mode));
+	ASSERT(S_ISDIR(VFS_I(dp)->i_mode));
 	XFS_STATS_INC(dp->i_mount, xs_dir_remove);
 
 	args = kmem_zalloc(sizeof(*args), KM_SLEEP | KM_NOFS);
@@ -505,7 +505,7 @@ xfs_dir_replace(
 	int		rval;
 	int		v;		/* type-checking value */
 
-	ASSERT(S_ISDIR(dp->i_d.di_mode));
+	ASSERT(S_ISDIR(VFS_I(dp)->i_mode));
 
 	rval = xfs_dir_ino_validate(tp->t_mountp, inum);
 	if (rval)
diff --git a/fs/xfs/libxfs/xfs_inode_buf.c b/fs/xfs/libxfs/xfs_inode_buf.c
index 9807c21..4a388b0 100644
--- a/fs/xfs/libxfs/xfs_inode_buf.c
+++ b/fs/xfs/libxfs/xfs_inode_buf.c
@@ -202,13 +202,12 @@ xfs_inode_from_disk(
 	struct xfs_icdinode	*to = &ip->i_d;
 	struct inode		*inode = VFS_I(ip);
 
-	to->di_mode = be16_to_cpu(from->di_mode);
-	to->di_version = from ->di_version;
 
 	/*
 	 * Convert v1 inodes immediately to v2 inode format as this is the
 	 * minimum inode version format we support in the rest of the code.
 	 */
+	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;
@@ -238,6 +237,7 @@ xfs_inode_from_disk(
 	inode->i_ctime.tv_sec = (int)be32_to_cpu(from->di_ctime.t_sec);
 	inode->i_ctime.tv_nsec = (int)be32_to_cpu(from->di_ctime.t_nsec);
 	inode->i_generation = be32_to_cpu(from->di_gen);
+	inode->i_mode = be16_to_cpu(from->di_mode);
 
 	to->di_size = be64_to_cpu(from->di_size);
 	to->di_nblocks = be64_to_cpu(from->di_nblocks);
@@ -270,7 +270,6 @@ xfs_inode_to_disk(
 	to->di_magic = cpu_to_be16(XFS_DINODE_MAGIC);
 	to->di_onlink = 0;
 
-	to->di_mode = cpu_to_be16(from->di_mode);
 	to->di_version = from->di_version;
 	to->di_format = from->di_format;
 	to->di_uid = cpu_to_be32(from->di_uid);
@@ -287,6 +286,7 @@ xfs_inode_to_disk(
 	to->di_ctime.t_nsec = cpu_to_be32(inode->i_ctime.tv_nsec);
 	to->di_nlink = cpu_to_be32(inode->i_nlink);
 	to->di_gen = cpu_to_be32(inode->i_generation);
+	to->di_mode = cpu_to_be16(inode->i_mode);
 
 	to->di_size = cpu_to_be64(from->di_size);
 	to->di_nblocks = cpu_to_be64(from->di_nblocks);
@@ -501,7 +501,7 @@ xfs_iread(
 		 * the inode is already free and not try to mess
 		 * with the uninitialized part of it.
 		 */
-		ip->i_d.di_mode = 0;
+		VFS_I(ip)->i_mode = 0;
 	}
 
 	ASSERT(ip->i_d.di_version >= 2);
diff --git a/fs/xfs/libxfs/xfs_inode_buf.h b/fs/xfs/libxfs/xfs_inode_buf.h
index e49c229..5f349a2 100644
--- a/fs/xfs/libxfs/xfs_inode_buf.h
+++ b/fs/xfs/libxfs/xfs_inode_buf.h
@@ -28,7 +28,6 @@ struct xfs_dinode;
  * translated to on-disk format specific structures at the appropriate time.
  */
 struct xfs_icdinode {
-	__uint16_t	di_mode;	/* mode and type of file */
 	__int8_t	di_version;	/* inode version */
 	__int8_t	di_format;	/* format of di_c data */
 	__uint16_t	di_flushiter;	/* incremented on flush */
diff --git a/fs/xfs/libxfs/xfs_inode_fork.c b/fs/xfs/libxfs/xfs_inode_fork.c
index 0defbd0..0bf1c74 100644
--- a/fs/xfs/libxfs/xfs_inode_fork.c
+++ b/fs/xfs/libxfs/xfs_inode_fork.c
@@ -120,7 +120,7 @@ xfs_iformat_fork(
 		return -EFSCORRUPTED;
 	}
 
-	switch (ip->i_d.di_mode & S_IFMT) {
+	switch (VFS_I(ip)->i_mode & S_IFMT) {
 	case S_IFIFO:
 	case S_IFCHR:
 	case S_IFBLK:
diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
index 45ec9e4..7087756 100644
--- a/fs/xfs/xfs_bmap_util.c
+++ b/fs/xfs/xfs_bmap_util.c
@@ -821,7 +821,7 @@ bool
 xfs_can_free_eofblocks(struct xfs_inode *ip, bool force)
 {
 	/* prealloc/delalloc exists only on regular files */
-	if (!S_ISREG(ip->i_d.di_mode))
+	if (!S_ISREG(VFS_I(ip)->i_mode))
 		return false;
 
 	/*
@@ -1726,7 +1726,7 @@ xfs_swap_extents(
 	xfs_lock_two_inodes(ip, tip, XFS_MMAPLOCK_EXCL);
 
 	/* Verify that both files have the same format */
-	if ((ip->i_d.di_mode & S_IFMT) != (tip->i_d.di_mode & S_IFMT)) {
+	if ((VFS_I(ip)->i_mode & S_IFMT) != (VFS_I(tip)->i_mode & S_IFMT)) {
 		error = -EINVAL;
 		goto out_unlock;
 	}
diff --git a/fs/xfs/xfs_dir2_readdir.c b/fs/xfs/xfs_dir2_readdir.c
index 642d55d..93b3ab0 100644
--- a/fs/xfs/xfs_dir2_readdir.c
+++ b/fs/xfs/xfs_dir2_readdir.c
@@ -665,7 +665,7 @@ xfs_readdir(
 	if (XFS_FORCED_SHUTDOWN(dp->i_mount))
 		return -EIO;
 
-	ASSERT(S_ISDIR(dp->i_d.di_mode));
+	ASSERT(S_ISDIR(VFS_I(dp)->i_mode));
 	XFS_STATS_INC(dp->i_mount, xs_dir_getdents);
 
 	args.dp = dp;
diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index ebe9b82..20eb6e5 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -156,9 +156,9 @@ xfs_update_prealloc_flags(
 	xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL);
 
 	if (!(flags & XFS_PREALLOC_INVISIBLE)) {
-		ip->i_d.di_mode &= ~S_ISUID;
-		if (ip->i_d.di_mode & S_IXGRP)
-			ip->i_d.di_mode &= ~S_ISGID;
+		VFS_I(ip)->i_mode &= ~S_ISUID;
+		if (VFS_I(ip)->i_mode & S_IXGRP)
+			VFS_I(ip)->i_mode &= ~S_ISGID;
 		xfs_trans_ichgtime(tp, ip, XFS_ICHGTIME_MOD | XFS_ICHGTIME_CHG);
 	}
 
diff --git a/fs/xfs/xfs_filestream.c b/fs/xfs/xfs_filestream.c
index c4c130f..a51353a 100644
--- a/fs/xfs/xfs_filestream.c
+++ b/fs/xfs/xfs_filestream.c
@@ -151,7 +151,7 @@ xfs_filestream_pick_ag(
 	xfs_agnumber_t		ag, max_ag = NULLAGNUMBER;
 	int			err, trylock, nscan;
 
-	ASSERT(S_ISDIR(ip->i_d.di_mode));
+	ASSERT(S_ISDIR(VFS_I(ip)->i_mode));
 
 	/* 2% of an AG's blocks must be free for it to be chosen. */
 	minfree = mp->m_sb.sb_agblocks / 50;
@@ -319,7 +319,7 @@ xfs_filestream_lookup_ag(
 	xfs_agnumber_t		startag, ag = NULLAGNUMBER;
 	struct xfs_mru_cache_elem *mru;
 
-	ASSERT(S_ISREG(ip->i_d.di_mode));
+	ASSERT(S_ISREG(VFS_I(ip)->i_mode));
 
 	pip = xfs_filestream_get_parent(ip);
 	if (!pip)
diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c
index 7c26f86..c1edb0f 100644
--- a/fs/xfs/xfs_icache.c
+++ b/fs/xfs/xfs_icache.c
@@ -63,6 +63,9 @@ xfs_inode_alloc(
 		return NULL;
 	}
 
+	/* VFS doesn't initialise i_mode! */
+	VFS_I(ip)->i_mode = 0;
+
 	XFS_STATS_INC(mp, vn_active);
 	ASSERT(atomic_read(&ip->i_pincount) == 0);
 	ASSERT(!spin_is_locked(&ip->i_flags_lock));
@@ -98,7 +101,7 @@ void
 xfs_inode_free(
 	struct xfs_inode	*ip)
 {
-	switch (ip->i_d.di_mode & S_IFMT) {
+	switch (VFS_I(ip)->i_mode & S_IFMT) {
 	case S_IFREG:
 	case S_IFDIR:
 	case S_IFLNK:
@@ -135,6 +138,35 @@ xfs_inode_free(
 }
 
 /*
+ * When we recycle a reclaimable inode, we need to re-initialise the VFS inode
+ * part of the structure. This is made more complex by the fact we store
+ * information about the on-disk values in the VFS inode and so we can't just
+ * overwrite it's values unconditionally. Hence we save the parameters we
+ * need to retain across reinitialisation, and rewrite them into the VFS inode
+ * after resetting it's state even if resetting fails.
+ */
+static int
+xfs_reinit_inode(
+	struct xfs_mount	*mp,
+	struct inode		*inode)
+{
+	uint32_t	nlink = inode->i_nlink;
+	umode_t		mode = inode->i_mode;
+	uint32_t	generation = inode->i_generation;
+	uint64_t	version = inode->i_version;
+	int		error;
+
+	error = inode_init_always(mp->m_super, inode);
+
+	set_nlink(inode, nlink);
+	inode->i_mode = mode;
+	inode->i_generation = generation;
+	inode->i_version = version;
+
+	return error;
+}
+
+/*
  * Check the validity of the inode we just found it the cache
  */
 static int
@@ -185,7 +217,7 @@ xfs_iget_cache_hit(
 	/*
 	 * If lookup is racing with unlink return an error immediately.
 	 */
-	if (ip->i_d.di_mode == 0 && !(flags & XFS_IGET_CREATE)) {
+	if (VFS_I(ip)->i_mode == 0 && !(flags & XFS_IGET_CREATE)) {
 		error = -ENOENT;
 		goto out_error;
 	}
@@ -208,7 +240,7 @@ xfs_iget_cache_hit(
 		spin_unlock(&ip->i_flags_lock);
 		rcu_read_unlock();
 
-		error = inode_init_always(mp->m_super, inode);
+		error = xfs_reinit_inode(mp, inode);
 		if (error) {
 			/*
 			 * Re-initializing the inode failed, and we are in deep
@@ -295,7 +327,7 @@ xfs_iget_cache_miss(
 
 	trace_xfs_iget_miss(ip);
 
-	if ((ip->i_d.di_mode == 0) && !(flags & XFS_IGET_CREATE)) {
+	if ((VFS_I(ip)->i_mode == 0) && !(flags & XFS_IGET_CREATE)) {
 		error = -ENOENT;
 		goto out_destroy;
 	}
@@ -444,7 +476,7 @@ again:
 	 * If we have a real type for an on-disk inode, we can setup the inode
 	 * now.	 If it's a new inode being created, xfs_ialloc will handle it.
 	 */
-	if (xfs_iflags_test(ip, XFS_INEW) && ip->i_d.di_mode != 0)
+	if (xfs_iflags_test(ip, XFS_INEW) && VFS_I(ip)->i_mode != 0)
 		xfs_setup_existing_inode(ip);
 	return 0;
 
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index 96aee23..a0e7cdf 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -793,7 +793,7 @@ xfs_ialloc(
 	if (ip->i_d.di_version == 1)
 		ip->i_d.di_version = 2;
 
-	ip->i_d.di_mode = mode;
+	inode->i_mode = mode;
 	set_nlink(inode, nlink);
 	ip->i_d.di_uid = xfs_kuid_to_uid(current_fsuid());
 	ip->i_d.di_gid = xfs_kgid_to_gid(current_fsgid());
@@ -801,9 +801,8 @@ xfs_ialloc(
 
 	if (pip && XFS_INHERIT_GID(pip)) {
 		ip->i_d.di_gid = pip->i_d.di_gid;
-		if ((pip->i_d.di_mode & S_ISGID) && S_ISDIR(mode)) {
-			ip->i_d.di_mode |= S_ISGID;
-		}
+		if ((VFS_I(pip)->i_mode & S_ISGID) && S_ISDIR(mode))
+			inode->i_mode |= S_ISGID;
 	}
 
 	/*
@@ -812,10 +811,9 @@ xfs_ialloc(
 	 * (and only if the irix_sgid_inherit compatibility variable is set).
 	 */
 	if ((irix_sgid_inherit) &&
-	    (ip->i_d.di_mode & S_ISGID) &&
-	    (!in_group_p(xfs_gid_to_kgid(ip->i_d.di_gid)))) {
-		ip->i_d.di_mode &= ~S_ISGID;
-	}
+	    (inode->i_mode & S_ISGID) &&
+	    (!in_group_p(xfs_gid_to_kgid(ip->i_d.di_gid))))
+		inode->i_mode &= ~S_ISGID;
 
 	ip->i_d.di_size = 0;
 	ip->i_d.di_nextents = 0;
@@ -1407,7 +1405,7 @@ xfs_link(
 
 	trace_xfs_link(tdp, target_name);
 
-	ASSERT(!S_ISDIR(sip->i_d.di_mode));
+	ASSERT(!S_ISDIR(VFS_I(sip)->i_mode));
 
 	if (XFS_FORCED_SHUTDOWN(mp))
 		return -EIO;
@@ -1614,7 +1612,7 @@ xfs_release(
 	xfs_mount_t	*mp = ip->i_mount;
 	int		error;
 
-	if (!S_ISREG(ip->i_d.di_mode) || (ip->i_d.di_mode == 0))
+	if (!S_ISREG(VFS_I(ip)->i_mode) || (VFS_I(ip)->i_mode == 0))
 		return 0;
 
 	/* If this is a read-only mount, don't do this (would generate I/O) */
@@ -1849,7 +1847,7 @@ xfs_inactive(
 	 * If the inode is already free, then there can be nothing
 	 * to clean up here.
 	 */
-	if (ip->i_d.di_mode == 0) {
+	if (VFS_I(ip)->i_mode == 0) {
 		ASSERT(ip->i_df.if_real_bytes == 0);
 		ASSERT(ip->i_df.if_broot_bytes == 0);
 		return;
@@ -1873,7 +1871,7 @@ xfs_inactive(
 		return;
 	}
 
-	if (S_ISREG(ip->i_d.di_mode) &&
+	if (S_ISREG(VFS_I(ip)->i_mode) &&
 	    (ip->i_d.di_size != 0 || XFS_ISIZE(ip) != 0 ||
 	     ip->i_d.di_nextents > 0 || ip->i_delayed_blks > 0))
 		truncate = 1;
@@ -1882,7 +1880,7 @@ xfs_inactive(
 	if (error)
 		return;
 
-	if (S_ISLNK(ip->i_d.di_mode))
+	if (S_ISLNK(VFS_I(ip)->i_mode))
 		error = xfs_inactive_symlink(ip);
 	else if (truncate)
 		error = xfs_inactive_truncate(ip);
@@ -1944,7 +1942,7 @@ xfs_iunlink(
 	int		error;
 
 	ASSERT(VFS_I(ip)->i_nlink == 0 || ignore_linkcount);
-	ASSERT(ip->i_d.di_mode != 0);
+	ASSERT(VFS_I(ip)->i_mode != 0);
 
 	mp = tp->t_mountp;
 
@@ -2387,7 +2385,7 @@ xfs_ifree(
 	ASSERT(VFS_I(ip)->i_nlink == 0);
 	ASSERT(ip->i_d.di_nextents == 0);
 	ASSERT(ip->i_d.di_anextents == 0);
-	ASSERT(ip->i_d.di_size == 0 || !S_ISREG(ip->i_d.di_mode));
+	ASSERT(ip->i_d.di_size == 0 || !S_ISREG(VFS_I(ip)->i_mode));
 	ASSERT(ip->i_d.di_nblocks == 0);
 
 	/*
@@ -2401,7 +2399,7 @@ xfs_ifree(
 	if (error)
 		return error;
 
-	ip->i_d.di_mode = 0;		/* mark incore inode as free */
+	VFS_I(ip)->i_mode = 0;		/* mark incore inode as free */
 	ip->i_d.di_flags = 0;
 	ip->i_d.di_dmevmask = 0;
 	ip->i_d.di_forkoff = 0;		/* mark the attr fork not in use */
@@ -2498,7 +2496,7 @@ xfs_remove(
 {
 	xfs_mount_t		*mp = dp->i_mount;
 	xfs_trans_t             *tp = NULL;
-	int			is_dir = S_ISDIR(ip->i_d.di_mode);
+	int			is_dir = S_ISDIR(VFS_I(ip)->i_mode);
 	int                     error = 0;
 	xfs_bmap_free_t         free_list;
 	xfs_fsblock_t           first_block;
@@ -2743,7 +2741,7 @@ xfs_cross_rename(
 	if (dp1 != dp2) {
 		dp2_flags = XFS_ICHGTIME_MOD | XFS_ICHGTIME_CHG;
 
-		if (S_ISDIR(ip2->i_d.di_mode)) {
+		if (S_ISDIR(VFS_I(ip2)->i_mode)) {
 			error = xfs_dir_replace(tp, ip2, &xfs_name_dotdot,
 						dp1->i_ino, first_block,
 						free_list, spaceres);
@@ -2751,7 +2749,7 @@ xfs_cross_rename(
 				goto out_trans_abort;
 
 			/* transfer ip2 ".." reference to dp1 */
-			if (!S_ISDIR(ip1->i_d.di_mode)) {
+			if (!S_ISDIR(VFS_I(ip1)->i_mode)) {
 				error = xfs_droplink(tp, dp2);
 				if (error)
 					goto out_trans_abort;
@@ -2770,7 +2768,7 @@ xfs_cross_rename(
 			ip2_flags |= XFS_ICHGTIME_MOD | XFS_ICHGTIME_CHG;
 		}
 
-		if (S_ISDIR(ip1->i_d.di_mode)) {
+		if (S_ISDIR(VFS_I(ip1)->i_mode)) {
 			error = xfs_dir_replace(tp, ip1, &xfs_name_dotdot,
 						dp2->i_ino, first_block,
 						free_list, spaceres);
@@ -2778,7 +2776,7 @@ xfs_cross_rename(
 				goto out_trans_abort;
 
 			/* transfer ip1 ".." reference to dp2 */
-			if (!S_ISDIR(ip2->i_d.di_mode)) {
+			if (!S_ISDIR(VFS_I(ip2)->i_mode)) {
 				error = xfs_droplink(tp, dp1);
 				if (error)
 					goto out_trans_abort;
@@ -2875,7 +2873,7 @@ xfs_rename(
 	struct xfs_inode	*inodes[__XFS_SORT_INODES];
 	int			num_inodes = __XFS_SORT_INODES;
 	bool			new_parent = (src_dp != target_dp);
-	bool			src_is_directory = S_ISDIR(src_ip->i_d.di_mode);
+	bool			src_is_directory = S_ISDIR(VFS_I(src_ip)->i_mode);
 	int			spaceres;
 	int			error;
 
@@ -3004,7 +3002,7 @@ xfs_rename(
 		 * target and source are directories and that target can be
 		 * destroyed, or that neither is a directory.
 		 */
-		if (S_ISDIR(target_ip->i_d.di_mode)) {
+		if (S_ISDIR(VFS_I(target_ip)->i_mode)) {
 			/*
 			 * Make sure target dir is empty.
 			 */
@@ -3434,7 +3432,7 @@ xfs_iflush_int(
 			__func__, ip->i_ino, be16_to_cpu(dip->di_magic), dip);
 		goto corrupt_out;
 	}
-	if (S_ISREG(ip->i_d.di_mode)) {
+	if (S_ISREG(VFS_I(ip)->i_mode)) {
 		if (XFS_TEST_ERROR(
 		    (ip->i_d.di_format != XFS_DINODE_FMT_EXTENTS) &&
 		    (ip->i_d.di_format != XFS_DINODE_FMT_BTREE),
@@ -3444,7 +3442,7 @@ xfs_iflush_int(
 				__func__, ip->i_ino, ip);
 			goto corrupt_out;
 		}
-	} else if (S_ISDIR(ip->i_d.di_mode)) {
+	} else if (S_ISDIR(VFS_I(ip)->i_mode)) {
 		if (XFS_TEST_ERROR(
 		    (ip->i_d.di_format != XFS_DINODE_FMT_EXTENTS) &&
 		    (ip->i_d.di_format != XFS_DINODE_FMT_BTREE) &&
diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
index e74d13d..d627749 100644
--- a/fs/xfs/xfs_inode.h
+++ b/fs/xfs/xfs_inode.h
@@ -88,7 +88,7 @@ static inline struct inode *VFS_I(struct xfs_inode *ip)
  */
 static inline xfs_fsize_t XFS_ISIZE(struct xfs_inode *ip)
 {
-	if (S_ISREG(ip->i_d.di_mode))
+	if (S_ISREG(VFS_I(ip)->i_mode))
 		return i_size_read(VFS_I(ip));
 	return ip->i_d.di_size;
 }
@@ -369,7 +369,7 @@ static inline int xfs_isiflocked(struct xfs_inode *ip)
  */
 #define XFS_INHERIT_GID(pip)	\
 	(((pip)->i_mount->m_flags & XFS_MOUNT_GRPID) || \
-	 ((pip)->i_d.di_mode & S_ISGID))
+	 (VFS_I(pip)->i_mode & S_ISGID))
 
 int		xfs_release(struct xfs_inode *ip);
 void		xfs_inactive(struct xfs_inode *ip);
diff --git a/fs/xfs/xfs_inode_item.c b/fs/xfs/xfs_inode_item.c
index 334d657..bd9808f 100644
--- a/fs/xfs/xfs_inode_item.c
+++ b/fs/xfs/xfs_inode_item.c
@@ -333,7 +333,6 @@ xfs_inode_to_log_dinode(
 
 	to->di_magic = XFS_DINODE_MAGIC;
 
-	to->di_mode = from->di_mode;
 	to->di_version = from->di_version;
 	to->di_format = from->di_format;
 	to->di_uid = from->di_uid;
@@ -351,6 +350,7 @@ xfs_inode_to_log_dinode(
 	to->di_ctime.t_nsec = inode->i_ctime.tv_nsec;
 	to->di_nlink = inode->i_nlink;
 	to->di_gen = inode->i_generation;
+	to->di_mode = inode->i_mode;
 
 	to->di_size = from->di_size;
 	to->di_nblocks = from->di_nblocks;
diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
index 2c40c5f..5069bd8 100644
--- a/fs/xfs/xfs_ioctl.c
+++ b/fs/xfs/xfs_ioctl.c
@@ -962,7 +962,7 @@ xfs_set_diflags(
 		di_flags |= XFS_DIFLAG_NODEFRAG;
 	if (xflags & XFS_XFLAG_FILESTREAM)
 		di_flags |= XFS_DIFLAG_FILESTREAM;
-	if (S_ISDIR(ip->i_d.di_mode)) {
+	if (S_ISDIR(VFS_I(ip)->i_mode)) {
 		if (xflags & XFS_XFLAG_RTINHERIT)
 			di_flags |= XFS_DIFLAG_RTINHERIT;
 		if (xflags & XFS_XFLAG_NOSYMLINKS)
@@ -971,7 +971,7 @@ xfs_set_diflags(
 			di_flags |= XFS_DIFLAG_EXTSZINHERIT;
 		if (xflags & XFS_XFLAG_PROJINHERIT)
 			di_flags |= XFS_DIFLAG_PROJINHERIT;
-	} else if (S_ISREG(ip->i_d.di_mode)) {
+	} else if (S_ISREG(VFS_I(ip)->i_mode)) {
 		if (xflags & XFS_XFLAG_REALTIME)
 			di_flags |= XFS_DIFLAG_REALTIME;
 		if (xflags & XFS_XFLAG_EXTSIZE)
@@ -1112,14 +1112,14 @@ xfs_ioctl_setattr_check_extsize(
 {
 	struct xfs_mount	*mp = ip->i_mount;
 
-	if ((fa->fsx_xflags & XFS_XFLAG_EXTSIZE) && !S_ISREG(ip->i_d.di_mode))
+	if ((fa->fsx_xflags & XFS_XFLAG_EXTSIZE) && !S_ISREG(VFS_I(ip)->i_mode))
 		return -EINVAL;
 
 	if ((fa->fsx_xflags & XFS_XFLAG_EXTSZINHERIT) &&
-	    !S_ISDIR(ip->i_d.di_mode))
+	    !S_ISDIR(VFS_I(ip)->i_mode))
 		return -EINVAL;
 
-	if (S_ISREG(ip->i_d.di_mode) && ip->i_d.di_nextents &&
+	if (S_ISREG(VFS_I(ip)->i_mode) && ip->i_d.di_nextents &&
 	    ((ip->i_d.di_extsize << mp->m_sb.sb_blocklog) != fa->fsx_extsize))
 		return -EINVAL;
 
@@ -1240,9 +1240,9 @@ xfs_ioctl_setattr(
 	 * successful return from chown()
 	 */
 
-	if ((ip->i_d.di_mode & (S_ISUID|S_ISGID)) &&
+	if ((VFS_I(ip)->i_mode & (S_ISUID|S_ISGID)) &&
 	    !capable_wrt_inode_uidgid(VFS_I(ip), CAP_FSETID))
-		ip->i_d.di_mode &= ~(S_ISUID|S_ISGID);
+		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) {
diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
index 397ce85..34bff1e 100644
--- a/fs/xfs/xfs_iops.c
+++ b/fs/xfs/xfs_iops.c
@@ -454,7 +454,7 @@ xfs_vn_getattr(
 
 	stat->size = XFS_ISIZE(ip);
 	stat->dev = inode->i_sb->s_dev;
-	stat->mode = ip->i_d.di_mode;
+	stat->mode = inode->i_mode;
 	stat->nlink = inode->i_nlink;
 	stat->uid = inode->i_uid;
 	stat->gid = inode->i_gid;
@@ -501,9 +501,6 @@ xfs_setattr_mode(
 
 	ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
 
-	ip->i_d.di_mode &= S_IFMT;
-	ip->i_d.di_mode |= mode & ~S_IFMT;
-
 	inode->i_mode &= S_IFMT;
 	inode->i_mode |= mode & ~S_IFMT;
 }
@@ -647,9 +644,9 @@ xfs_setattr_nonsize(
 		 * The set-user-ID and set-group-ID bits of a file will be
 		 * cleared upon successful return from chown()
 		 */
-		if ((ip->i_d.di_mode & (S_ISUID|S_ISGID)) &&
+		if ((inode->i_mode & (S_ISUID|S_ISGID)) &&
 		    !capable(CAP_FSETID))
-			ip->i_d.di_mode &= ~(S_ISUID|S_ISGID);
+			inode->i_mode &= ~(S_ISUID|S_ISGID);
 
 		/*
 		 * Change the ownerships and register quota modifications
@@ -759,7 +756,7 @@ xfs_setattr_size(
 
 	ASSERT(xfs_isilocked(ip, XFS_IOLOCK_EXCL));
 	ASSERT(xfs_isilocked(ip, XFS_MMAPLOCK_EXCL));
-	ASSERT(S_ISREG(ip->i_d.di_mode));
+	ASSERT(S_ISREG(inode->i_mode));
 	ASSERT((iattr->ia_valid & (ATTR_UID|ATTR_GID|ATTR_ATIME|ATTR_ATIME_SET|
 		ATTR_MTIME_SET|ATTR_KILL_PRIV|ATTR_TIMES_SET)) == 0);
 
@@ -1211,7 +1208,6 @@ xfs_setup_inode(
 	/* make the inode look hashed for the writeback code */
 	hlist_add_fake(&inode->i_hash);
 
-	inode->i_mode	= ip->i_d.di_mode;
 	inode->i_uid    = xfs_uid_to_kuid(ip->i_d.di_uid);
 	inode->i_gid    = xfs_gid_to_kgid(ip->i_d.di_gid);
 
diff --git a/fs/xfs/xfs_itable.c b/fs/xfs/xfs_itable.c
index 6162e65..ce73eb3 100644
--- a/fs/xfs/xfs_itable.c
+++ b/fs/xfs/xfs_itable.c
@@ -88,7 +88,6 @@ xfs_bulkstat_one_int(
 	buf->bs_projid_lo = dic->di_projid_lo;
 	buf->bs_projid_hi = dic->di_projid_hi;
 	buf->bs_ino = ino;
-	buf->bs_mode = dic->di_mode;
 	buf->bs_uid = dic->di_uid;
 	buf->bs_gid = dic->di_gid;
 	buf->bs_size = dic->di_size;
@@ -101,6 +100,7 @@ xfs_bulkstat_one_int(
 	buf->bs_ctime.tv_sec = inode->i_ctime.tv_sec;
 	buf->bs_ctime.tv_nsec = inode->i_ctime.tv_nsec;
 	buf->bs_gen = inode->i_generation;
+	buf->bs_mode = inode->i_mode;
 
 	buf->bs_xflags = xfs_ip2xflags(ip);
 	buf->bs_extsize = dic->di_extsize << mp->m_sb.sb_blocklog;
diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
index 611c25c..bd6f23b 100644
--- a/fs/xfs/xfs_log_recover.c
+++ b/fs/xfs/xfs_log_recover.c
@@ -4338,7 +4338,7 @@ xlog_recover_process_one_iunlink(
 		goto fail_iput;
 
 	ASSERT(VFS_I(ip)->i_nlink == 0);
-	ASSERT(ip->i_d.di_mode != 0);
+	ASSERT(VFS_I(ip)->i_mode != 0);
 
 	/* setup for the next pass */
 	agino = be32_to_cpu(dip->di_next_unlinked);
diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
index bb753b3..d306105 100644
--- a/fs/xfs/xfs_mount.c
+++ b/fs/xfs/xfs_mount.c
@@ -865,7 +865,7 @@ xfs_mountfs(
 
 	ASSERT(rip != NULL);
 
-	if (unlikely(!S_ISDIR(rip->i_d.di_mode))) {
+	if (unlikely(!S_ISDIR(VFS_I(rip)->i_mode))) {
 		xfs_warn(mp, "corrupted root inode %llu: not a directory",
 			(unsigned long long)rip->i_ino);
 		xfs_iunlock(rip, XFS_ILOCK_EXCL);
-- 
2.5.0

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 8/8] xfs: mode di_mode to vfs inode

[Brian Foster]

On Thu, Jan 14, 2016 at 05:09:25PM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> Move the di_mode value from the xfs_icdinode to the VFS inode, reducing
> the xfs_icdinode byte another 2 bytes and collapsing another 2 byte hole
> in the structure.
> 
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> ---
>  fs/fs-writeback.c              |  1 +
>  fs/xfs/libxfs/xfs_bmap.c       |  6 +++---
>  fs/xfs/libxfs/xfs_dir2.c       | 12 +++++------
>  fs/xfs/libxfs/xfs_inode_buf.c  |  8 +++----
>  fs/xfs/libxfs/xfs_inode_buf.h  |  1 -
>  fs/xfs/libxfs/xfs_inode_fork.c |  2 +-
>  fs/xfs/xfs_bmap_util.c         |  4 ++--
>  fs/xfs/xfs_dir2_readdir.c      |  2 +-
>  fs/xfs/xfs_file.c              |  6 +++---
>  fs/xfs/xfs_filestream.c        |  4 ++--
>  fs/xfs/xfs_icache.c            | 42 +++++++++++++++++++++++++++++++-----
>  fs/xfs/xfs_inode.c             | 48 ++++++++++++++++++++----------------------
>  fs/xfs/xfs_inode.h             |  4 ++--
>  fs/xfs/xfs_inode_item.c        |  2 +-
>  fs/xfs/xfs_ioctl.c             | 14 ++++++------
>  fs/xfs/xfs_iops.c              | 12 ++++-------
>  fs/xfs/xfs_itable.c            |  2 +-
>  fs/xfs/xfs_log_recover.c       |  2 +-
>  fs/xfs/xfs_mount.c             |  2 +-
>  19 files changed, 100 insertions(+), 74 deletions(-)
> 
> diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
> index 023f6a1..0858457 100644
> --- a/fs/fs-writeback.c
> +++ b/fs/fs-writeback.c
> @@ -2106,6 +2106,7 @@ EXPORT_SYMBOL(__mark_inode_dirty);
>   * completed by the time we have gained the lock and waited for all IO that is
>   * in progress regardless of the order callers are granted the lock.
>   */
> +#include "linux/delay.h"
>  static void wait_sb_inodes(struct super_block *sb)
>  {
>  	struct inode *inode, *old_inode = NULL;

What's this hunk about?

...
> diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c
> index 7c26f86..c1edb0f 100644
> --- a/fs/xfs/xfs_icache.c
> +++ b/fs/xfs/xfs_icache.c
...
> @@ -135,6 +138,35 @@ xfs_inode_free(
>  }
>  
>  /*
> + * When we recycle a reclaimable inode, we need to re-initialise the VFS inode
> + * part of the structure. This is made more complex by the fact we store
> + * information about the on-disk values in the VFS inode and so we can't just
> + * overwrite it's values unconditionally. Hence we save the parameters we

Nit:		its

> + * need to retain across reinitialisation, and rewrite them into the VFS inode
> + * after resetting it's state even if resetting fails.

... and above as well.

That aside, it looks like xfs_reinit_inode() addresses my previous
questions around reclaimable inodes. Could we introduce this function
properly when it is first needed (the nlink patch) and update it
appropriately?

Brian

> + */
> +static int
> +xfs_reinit_inode(
> +	struct xfs_mount	*mp,
> +	struct inode		*inode)
> +{
> +	uint32_t	nlink = inode->i_nlink;
> +	umode_t		mode = inode->i_mode;
> +	uint32_t	generation = inode->i_generation;
> +	uint64_t	version = inode->i_version;
> +	int		error;
> +
> +	error = inode_init_always(mp->m_super, inode);
> +
> +	set_nlink(inode, nlink);
> +	inode->i_mode = mode;
> +	inode->i_generation = generation;
> +	inode->i_version = version;
> +
> +	return error;
> +}
> +
> +/*
>   * Check the validity of the inode we just found it the cache
>   */
>  static int
> @@ -185,7 +217,7 @@ xfs_iget_cache_hit(
>  	/*
>  	 * If lookup is racing with unlink return an error immediately.
>  	 */
> -	if (ip->i_d.di_mode == 0 && !(flags & XFS_IGET_CREATE)) {
> +	if (VFS_I(ip)->i_mode == 0 && !(flags & XFS_IGET_CREATE)) {
>  		error = -ENOENT;
>  		goto out_error;
>  	}
> @@ -208,7 +240,7 @@ xfs_iget_cache_hit(
>  		spin_unlock(&ip->i_flags_lock);
>  		rcu_read_unlock();
>  
> -		error = inode_init_always(mp->m_super, inode);
> +		error = xfs_reinit_inode(mp, inode);
>  		if (error) {
>  			/*
>  			 * Re-initializing the inode failed, and we are in deep
> @@ -295,7 +327,7 @@ xfs_iget_cache_miss(
>  
>  	trace_xfs_iget_miss(ip);
>  
> -	if ((ip->i_d.di_mode == 0) && !(flags & XFS_IGET_CREATE)) {
> +	if ((VFS_I(ip)->i_mode == 0) && !(flags & XFS_IGET_CREATE)) {
>  		error = -ENOENT;
>  		goto out_destroy;
>  	}
> @@ -444,7 +476,7 @@ again:
>  	 * If we have a real type for an on-disk inode, we can setup the inode
>  	 * now.	 If it's a new inode being created, xfs_ialloc will handle it.
>  	 */
> -	if (xfs_iflags_test(ip, XFS_INEW) && ip->i_d.di_mode != 0)
> +	if (xfs_iflags_test(ip, XFS_INEW) && VFS_I(ip)->i_mode != 0)
>  		xfs_setup_existing_inode(ip);
>  	return 0;
>  
> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> index 96aee23..a0e7cdf 100644
> --- a/fs/xfs/xfs_inode.c
> +++ b/fs/xfs/xfs_inode.c
> @@ -793,7 +793,7 @@ xfs_ialloc(
>  	if (ip->i_d.di_version == 1)
>  		ip->i_d.di_version = 2;
>  
> -	ip->i_d.di_mode = mode;
> +	inode->i_mode = mode;
>  	set_nlink(inode, nlink);
>  	ip->i_d.di_uid = xfs_kuid_to_uid(current_fsuid());
>  	ip->i_d.di_gid = xfs_kgid_to_gid(current_fsgid());
> @@ -801,9 +801,8 @@ xfs_ialloc(
>  
>  	if (pip && XFS_INHERIT_GID(pip)) {
>  		ip->i_d.di_gid = pip->i_d.di_gid;
> -		if ((pip->i_d.di_mode & S_ISGID) && S_ISDIR(mode)) {
> -			ip->i_d.di_mode |= S_ISGID;
> -		}
> +		if ((VFS_I(pip)->i_mode & S_ISGID) && S_ISDIR(mode))
> +			inode->i_mode |= S_ISGID;
>  	}
>  
>  	/*
> @@ -812,10 +811,9 @@ xfs_ialloc(
>  	 * (and only if the irix_sgid_inherit compatibility variable is set).
>  	 */
>  	if ((irix_sgid_inherit) &&
> -	    (ip->i_d.di_mode & S_ISGID) &&
> -	    (!in_group_p(xfs_gid_to_kgid(ip->i_d.di_gid)))) {
> -		ip->i_d.di_mode &= ~S_ISGID;
> -	}
> +	    (inode->i_mode & S_ISGID) &&
> +	    (!in_group_p(xfs_gid_to_kgid(ip->i_d.di_gid))))
> +		inode->i_mode &= ~S_ISGID;
>  
>  	ip->i_d.di_size = 0;
>  	ip->i_d.di_nextents = 0;
> @@ -1407,7 +1405,7 @@ xfs_link(
>  
>  	trace_xfs_link(tdp, target_name);
>  
> -	ASSERT(!S_ISDIR(sip->i_d.di_mode));
> +	ASSERT(!S_ISDIR(VFS_I(sip)->i_mode));
>  
>  	if (XFS_FORCED_SHUTDOWN(mp))
>  		return -EIO;
> @@ -1614,7 +1612,7 @@ xfs_release(
>  	xfs_mount_t	*mp = ip->i_mount;
>  	int		error;
>  
> -	if (!S_ISREG(ip->i_d.di_mode) || (ip->i_d.di_mode == 0))
> +	if (!S_ISREG(VFS_I(ip)->i_mode) || (VFS_I(ip)->i_mode == 0))
>  		return 0;
>  
>  	/* If this is a read-only mount, don't do this (would generate I/O) */
> @@ -1849,7 +1847,7 @@ xfs_inactive(
>  	 * If the inode is already free, then there can be nothing
>  	 * to clean up here.
>  	 */
> -	if (ip->i_d.di_mode == 0) {
> +	if (VFS_I(ip)->i_mode == 0) {
>  		ASSERT(ip->i_df.if_real_bytes == 0);
>  		ASSERT(ip->i_df.if_broot_bytes == 0);
>  		return;
> @@ -1873,7 +1871,7 @@ xfs_inactive(
>  		return;
>  	}
>  
> -	if (S_ISREG(ip->i_d.di_mode) &&
> +	if (S_ISREG(VFS_I(ip)->i_mode) &&
>  	    (ip->i_d.di_size != 0 || XFS_ISIZE(ip) != 0 ||
>  	     ip->i_d.di_nextents > 0 || ip->i_delayed_blks > 0))
>  		truncate = 1;
> @@ -1882,7 +1880,7 @@ xfs_inactive(
>  	if (error)
>  		return;
>  
> -	if (S_ISLNK(ip->i_d.di_mode))
> +	if (S_ISLNK(VFS_I(ip)->i_mode))
>  		error = xfs_inactive_symlink(ip);
>  	else if (truncate)
>  		error = xfs_inactive_truncate(ip);
> @@ -1944,7 +1942,7 @@ xfs_iunlink(
>  	int		error;
>  
>  	ASSERT(VFS_I(ip)->i_nlink == 0 || ignore_linkcount);
> -	ASSERT(ip->i_d.di_mode != 0);
> +	ASSERT(VFS_I(ip)->i_mode != 0);
>  
>  	mp = tp->t_mountp;
>  
> @@ -2387,7 +2385,7 @@ xfs_ifree(
>  	ASSERT(VFS_I(ip)->i_nlink == 0);
>  	ASSERT(ip->i_d.di_nextents == 0);
>  	ASSERT(ip->i_d.di_anextents == 0);
> -	ASSERT(ip->i_d.di_size == 0 || !S_ISREG(ip->i_d.di_mode));
> +	ASSERT(ip->i_d.di_size == 0 || !S_ISREG(VFS_I(ip)->i_mode));
>  	ASSERT(ip->i_d.di_nblocks == 0);
>  
>  	/*
> @@ -2401,7 +2399,7 @@ xfs_ifree(
>  	if (error)
>  		return error;
>  
> -	ip->i_d.di_mode = 0;		/* mark incore inode as free */
> +	VFS_I(ip)->i_mode = 0;		/* mark incore inode as free */
>  	ip->i_d.di_flags = 0;
>  	ip->i_d.di_dmevmask = 0;
>  	ip->i_d.di_forkoff = 0;		/* mark the attr fork not in use */
> @@ -2498,7 +2496,7 @@ xfs_remove(
>  {
>  	xfs_mount_t		*mp = dp->i_mount;
>  	xfs_trans_t             *tp = NULL;
> -	int			is_dir = S_ISDIR(ip->i_d.di_mode);
> +	int			is_dir = S_ISDIR(VFS_I(ip)->i_mode);
>  	int                     error = 0;
>  	xfs_bmap_free_t         free_list;
>  	xfs_fsblock_t           first_block;
> @@ -2743,7 +2741,7 @@ xfs_cross_rename(
>  	if (dp1 != dp2) {
>  		dp2_flags = XFS_ICHGTIME_MOD | XFS_ICHGTIME_CHG;
>  
> -		if (S_ISDIR(ip2->i_d.di_mode)) {
> +		if (S_ISDIR(VFS_I(ip2)->i_mode)) {
>  			error = xfs_dir_replace(tp, ip2, &xfs_name_dotdot,
>  						dp1->i_ino, first_block,
>  						free_list, spaceres);
> @@ -2751,7 +2749,7 @@ xfs_cross_rename(
>  				goto out_trans_abort;
>  
>  			/* transfer ip2 ".." reference to dp1 */
> -			if (!S_ISDIR(ip1->i_d.di_mode)) {
> +			if (!S_ISDIR(VFS_I(ip1)->i_mode)) {
>  				error = xfs_droplink(tp, dp2);
>  				if (error)
>  					goto out_trans_abort;
> @@ -2770,7 +2768,7 @@ xfs_cross_rename(
>  			ip2_flags |= XFS_ICHGTIME_MOD | XFS_ICHGTIME_CHG;
>  		}
>  
> -		if (S_ISDIR(ip1->i_d.di_mode)) {
> +		if (S_ISDIR(VFS_I(ip1)->i_mode)) {
>  			error = xfs_dir_replace(tp, ip1, &xfs_name_dotdot,
>  						dp2->i_ino, first_block,
>  						free_list, spaceres);
> @@ -2778,7 +2776,7 @@ xfs_cross_rename(
>  				goto out_trans_abort;
>  
>  			/* transfer ip1 ".." reference to dp2 */
> -			if (!S_ISDIR(ip2->i_d.di_mode)) {
> +			if (!S_ISDIR(VFS_I(ip2)->i_mode)) {
>  				error = xfs_droplink(tp, dp1);
>  				if (error)
>  					goto out_trans_abort;
> @@ -2875,7 +2873,7 @@ xfs_rename(
>  	struct xfs_inode	*inodes[__XFS_SORT_INODES];
>  	int			num_inodes = __XFS_SORT_INODES;
>  	bool			new_parent = (src_dp != target_dp);
> -	bool			src_is_directory = S_ISDIR(src_ip->i_d.di_mode);
> +	bool			src_is_directory = S_ISDIR(VFS_I(src_ip)->i_mode);
>  	int			spaceres;
>  	int			error;
>  
> @@ -3004,7 +3002,7 @@ xfs_rename(
>  		 * target and source are directories and that target can be
>  		 * destroyed, or that neither is a directory.
>  		 */
> -		if (S_ISDIR(target_ip->i_d.di_mode)) {
> +		if (S_ISDIR(VFS_I(target_ip)->i_mode)) {
>  			/*
>  			 * Make sure target dir is empty.
>  			 */
> @@ -3434,7 +3432,7 @@ xfs_iflush_int(
>  			__func__, ip->i_ino, be16_to_cpu(dip->di_magic), dip);
>  		goto corrupt_out;
>  	}
> -	if (S_ISREG(ip->i_d.di_mode)) {
> +	if (S_ISREG(VFS_I(ip)->i_mode)) {
>  		if (XFS_TEST_ERROR(
>  		    (ip->i_d.di_format != XFS_DINODE_FMT_EXTENTS) &&
>  		    (ip->i_d.di_format != XFS_DINODE_FMT_BTREE),
> @@ -3444,7 +3442,7 @@ xfs_iflush_int(
>  				__func__, ip->i_ino, ip);
>  			goto corrupt_out;
>  		}
> -	} else if (S_ISDIR(ip->i_d.di_mode)) {
> +	} else if (S_ISDIR(VFS_I(ip)->i_mode)) {
>  		if (XFS_TEST_ERROR(
>  		    (ip->i_d.di_format != XFS_DINODE_FMT_EXTENTS) &&
>  		    (ip->i_d.di_format != XFS_DINODE_FMT_BTREE) &&
> diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
> index e74d13d..d627749 100644
> --- a/fs/xfs/xfs_inode.h
> +++ b/fs/xfs/xfs_inode.h
> @@ -88,7 +88,7 @@ static inline struct inode *VFS_I(struct xfs_inode *ip)
>   */
>  static inline xfs_fsize_t XFS_ISIZE(struct xfs_inode *ip)
>  {
> -	if (S_ISREG(ip->i_d.di_mode))
> +	if (S_ISREG(VFS_I(ip)->i_mode))
>  		return i_size_read(VFS_I(ip));
>  	return ip->i_d.di_size;
>  }
> @@ -369,7 +369,7 @@ static inline int xfs_isiflocked(struct xfs_inode *ip)
>   */
>  #define XFS_INHERIT_GID(pip)	\
>  	(((pip)->i_mount->m_flags & XFS_MOUNT_GRPID) || \
> -	 ((pip)->i_d.di_mode & S_ISGID))
> +	 (VFS_I(pip)->i_mode & S_ISGID))
>  
>  int		xfs_release(struct xfs_inode *ip);
>  void		xfs_inactive(struct xfs_inode *ip);
> diff --git a/fs/xfs/xfs_inode_item.c b/fs/xfs/xfs_inode_item.c
> index 334d657..bd9808f 100644
> --- a/fs/xfs/xfs_inode_item.c
> +++ b/fs/xfs/xfs_inode_item.c
> @@ -333,7 +333,6 @@ xfs_inode_to_log_dinode(
>  
>  	to->di_magic = XFS_DINODE_MAGIC;
>  
> -	to->di_mode = from->di_mode;
>  	to->di_version = from->di_version;
>  	to->di_format = from->di_format;
>  	to->di_uid = from->di_uid;
> @@ -351,6 +350,7 @@ xfs_inode_to_log_dinode(
>  	to->di_ctime.t_nsec = inode->i_ctime.tv_nsec;
>  	to->di_nlink = inode->i_nlink;
>  	to->di_gen = inode->i_generation;
> +	to->di_mode = inode->i_mode;
>  
>  	to->di_size = from->di_size;
>  	to->di_nblocks = from->di_nblocks;
> diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
> index 2c40c5f..5069bd8 100644
> --- a/fs/xfs/xfs_ioctl.c
> +++ b/fs/xfs/xfs_ioctl.c
> @@ -962,7 +962,7 @@ xfs_set_diflags(
>  		di_flags |= XFS_DIFLAG_NODEFRAG;
>  	if (xflags & XFS_XFLAG_FILESTREAM)
>  		di_flags |= XFS_DIFLAG_FILESTREAM;
> -	if (S_ISDIR(ip->i_d.di_mode)) {
> +	if (S_ISDIR(VFS_I(ip)->i_mode)) {
>  		if (xflags & XFS_XFLAG_RTINHERIT)
>  			di_flags |= XFS_DIFLAG_RTINHERIT;
>  		if (xflags & XFS_XFLAG_NOSYMLINKS)
> @@ -971,7 +971,7 @@ xfs_set_diflags(
>  			di_flags |= XFS_DIFLAG_EXTSZINHERIT;
>  		if (xflags & XFS_XFLAG_PROJINHERIT)
>  			di_flags |= XFS_DIFLAG_PROJINHERIT;
> -	} else if (S_ISREG(ip->i_d.di_mode)) {
> +	} else if (S_ISREG(VFS_I(ip)->i_mode)) {
>  		if (xflags & XFS_XFLAG_REALTIME)
>  			di_flags |= XFS_DIFLAG_REALTIME;
>  		if (xflags & XFS_XFLAG_EXTSIZE)
> @@ -1112,14 +1112,14 @@ xfs_ioctl_setattr_check_extsize(
>  {
>  	struct xfs_mount	*mp = ip->i_mount;
>  
> -	if ((fa->fsx_xflags & XFS_XFLAG_EXTSIZE) && !S_ISREG(ip->i_d.di_mode))
> +	if ((fa->fsx_xflags & XFS_XFLAG_EXTSIZE) && !S_ISREG(VFS_I(ip)->i_mode))
>  		return -EINVAL;
>  
>  	if ((fa->fsx_xflags & XFS_XFLAG_EXTSZINHERIT) &&
> -	    !S_ISDIR(ip->i_d.di_mode))
> +	    !S_ISDIR(VFS_I(ip)->i_mode))
>  		return -EINVAL;
>  
> -	if (S_ISREG(ip->i_d.di_mode) && ip->i_d.di_nextents &&
> +	if (S_ISREG(VFS_I(ip)->i_mode) && ip->i_d.di_nextents &&
>  	    ((ip->i_d.di_extsize << mp->m_sb.sb_blocklog) != fa->fsx_extsize))
>  		return -EINVAL;
>  
> @@ -1240,9 +1240,9 @@ xfs_ioctl_setattr(
>  	 * successful return from chown()
>  	 */
>  
> -	if ((ip->i_d.di_mode & (S_ISUID|S_ISGID)) &&
> +	if ((VFS_I(ip)->i_mode & (S_ISUID|S_ISGID)) &&
>  	    !capable_wrt_inode_uidgid(VFS_I(ip), CAP_FSETID))
> -		ip->i_d.di_mode &= ~(S_ISUID|S_ISGID);
> +		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) {
> diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
> index 397ce85..34bff1e 100644
> --- a/fs/xfs/xfs_iops.c
> +++ b/fs/xfs/xfs_iops.c
> @@ -454,7 +454,7 @@ xfs_vn_getattr(
>  
>  	stat->size = XFS_ISIZE(ip);
>  	stat->dev = inode->i_sb->s_dev;
> -	stat->mode = ip->i_d.di_mode;
> +	stat->mode = inode->i_mode;
>  	stat->nlink = inode->i_nlink;
>  	stat->uid = inode->i_uid;
>  	stat->gid = inode->i_gid;
> @@ -501,9 +501,6 @@ xfs_setattr_mode(
>  
>  	ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
>  
> -	ip->i_d.di_mode &= S_IFMT;
> -	ip->i_d.di_mode |= mode & ~S_IFMT;
> -
>  	inode->i_mode &= S_IFMT;
>  	inode->i_mode |= mode & ~S_IFMT;
>  }
> @@ -647,9 +644,9 @@ xfs_setattr_nonsize(
>  		 * The set-user-ID and set-group-ID bits of a file will be
>  		 * cleared upon successful return from chown()
>  		 */
> -		if ((ip->i_d.di_mode & (S_ISUID|S_ISGID)) &&
> +		if ((inode->i_mode & (S_ISUID|S_ISGID)) &&
>  		    !capable(CAP_FSETID))
> -			ip->i_d.di_mode &= ~(S_ISUID|S_ISGID);
> +			inode->i_mode &= ~(S_ISUID|S_ISGID);
>  
>  		/*
>  		 * Change the ownerships and register quota modifications
> @@ -759,7 +756,7 @@ xfs_setattr_size(
>  
>  	ASSERT(xfs_isilocked(ip, XFS_IOLOCK_EXCL));
>  	ASSERT(xfs_isilocked(ip, XFS_MMAPLOCK_EXCL));
> -	ASSERT(S_ISREG(ip->i_d.di_mode));
> +	ASSERT(S_ISREG(inode->i_mode));
>  	ASSERT((iattr->ia_valid & (ATTR_UID|ATTR_GID|ATTR_ATIME|ATTR_ATIME_SET|
>  		ATTR_MTIME_SET|ATTR_KILL_PRIV|ATTR_TIMES_SET)) == 0);
>  
> @@ -1211,7 +1208,6 @@ xfs_setup_inode(
>  	/* make the inode look hashed for the writeback code */
>  	hlist_add_fake(&inode->i_hash);
>  
> -	inode->i_mode	= ip->i_d.di_mode;
>  	inode->i_uid    = xfs_uid_to_kuid(ip->i_d.di_uid);
>  	inode->i_gid    = xfs_gid_to_kgid(ip->i_d.di_gid);
>  
> diff --git a/fs/xfs/xfs_itable.c b/fs/xfs/xfs_itable.c
> index 6162e65..ce73eb3 100644
> --- a/fs/xfs/xfs_itable.c
> +++ b/fs/xfs/xfs_itable.c
> @@ -88,7 +88,6 @@ xfs_bulkstat_one_int(
>  	buf->bs_projid_lo = dic->di_projid_lo;
>  	buf->bs_projid_hi = dic->di_projid_hi;
>  	buf->bs_ino = ino;
> -	buf->bs_mode = dic->di_mode;
>  	buf->bs_uid = dic->di_uid;
>  	buf->bs_gid = dic->di_gid;
>  	buf->bs_size = dic->di_size;
> @@ -101,6 +100,7 @@ xfs_bulkstat_one_int(
>  	buf->bs_ctime.tv_sec = inode->i_ctime.tv_sec;
>  	buf->bs_ctime.tv_nsec = inode->i_ctime.tv_nsec;
>  	buf->bs_gen = inode->i_generation;
> +	buf->bs_mode = inode->i_mode;
>  
>  	buf->bs_xflags = xfs_ip2xflags(ip);
>  	buf->bs_extsize = dic->di_extsize << mp->m_sb.sb_blocklog;
> diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
> index 611c25c..bd6f23b 100644
> --- a/fs/xfs/xfs_log_recover.c
> +++ b/fs/xfs/xfs_log_recover.c
> @@ -4338,7 +4338,7 @@ xlog_recover_process_one_iunlink(
>  		goto fail_iput;
>  
>  	ASSERT(VFS_I(ip)->i_nlink == 0);
> -	ASSERT(ip->i_d.di_mode != 0);
> +	ASSERT(VFS_I(ip)->i_mode != 0);
>  
>  	/* setup for the next pass */
>  	agino = be32_to_cpu(dip->di_next_unlinked);
> diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
> index bb753b3..d306105 100644
> --- a/fs/xfs/xfs_mount.c
> +++ b/fs/xfs/xfs_mount.c
> @@ -865,7 +865,7 @@ xfs_mountfs(
>  
>  	ASSERT(rip != NULL);
>  
> -	if (unlikely(!S_ISDIR(rip->i_d.di_mode))) {
> +	if (unlikely(!S_ISDIR(VFS_I(rip)->i_mode))) {
>  		xfs_warn(mp, "corrupted root inode %llu: not a directory",
>  			(unsigned long long)rip->i_ino);
>  		xfs_iunlock(rip, XFS_ILOCK_EXCL);
> -- 
> 2.5.0
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs