All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/10] decouple the in-memory from the on-disk log format V2
@ 2013-11-29  8:39 Christoph Hellwig
  2013-11-29  8:39 ` [PATCH 01/10] xfs: remove duplicate code in xlog_cil_insert_format_items Christoph Hellwig
                   ` (9 more replies)
  0 siblings, 10 replies; 29+ messages in thread
From: Christoph Hellwig @ 2013-11-29  8:39 UTC (permalink / raw)
  To: xfs

Since the introduction of the CIL we already have a layer of indirection
between the physical log format and the data structure tracking the
changes in memory.  But due to the way iop_format works we are still
forced to keep a copy of everything that goes out to the log in memory
even before copying it into the CIL.

The first patch in this series changes iop_format so that the log items
are free to store their in-memory data however they want before formatting
them into the CIL, and the other patches take advantage of that by not
keeping most log formats in memory all the time.  Especially the EFI and
EFD related ones at the end start to show the benefit.

What's missing from this series are larger changes to the in-core inode
layout.  No needing the full struct icdinode at all times will be the
biggest benefit of this change, but it will be large enough series of it's
own.

Changes from V1:
 - split into more patches
 - added the xlog_copy_iovec helper

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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH 01/10] xfs: remove duplicate code in xlog_cil_insert_format_items
  2013-11-29  8:39 [PATCH 00/10] decouple the in-memory from the on-disk log format V2 Christoph Hellwig
@ 2013-11-29  8:39 ` Christoph Hellwig
  2013-12-03  0:58   ` Dave Chinner
  2013-12-09 19:45   ` Ben Myers
  2013-11-29  8:39 ` [PATCH 02/10] xfs: refactor xfs_buf_item_format_segment Christoph Hellwig
                   ` (8 subsequent siblings)
  9 siblings, 2 replies; 29+ messages in thread
From: Christoph Hellwig @ 2013-11-29  8:39 UTC (permalink / raw)
  To: xfs

[-- Attachment #1: 0001-xfs-remove-duplicate-code-in-xlog_cil_insert_format_.patch --]
[-- Type: text/plain, Size: 1806 bytes --]

Share code that was previously duplicated in two branches.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_log_cil.c |   33 +++++++++++++--------------------
 1 file changed, 13 insertions(+), 20 deletions(-)

diff --git a/fs/xfs/xfs_log_cil.c b/fs/xfs/xfs_log_cil.c
index 5eb51fc..0a7a8ce 100644
--- a/fs/xfs/xfs_log_cil.c
+++ b/fs/xfs/xfs_log_cil.c
@@ -254,29 +254,22 @@ xlog_cil_insert_format_items(
 			 */
 			*diff_iovecs -= lv->lv_niovecs;
 			*diff_len -= lv->lv_buf_len;
-
-			/* Ensure the lv is set up according to ->iop_size */
-			lv->lv_niovecs = niovecs;
-			lv->lv_buf = (char *)lv + buf_size - nbytes;
-
-			lv->lv_buf_len = xlog_cil_lv_item_format(lip, lv);
-			goto insert;
+		} else {
+			/* allocate new data chunk */
+			lv = kmem_zalloc(buf_size, KM_SLEEP|KM_NOFS);
+			lv->lv_item = lip;
+			lv->lv_size = buf_size;
+			if (ordered) {
+				/* track as an ordered logvec */
+				ASSERT(lip->li_lv == NULL);
+				lv->lv_buf_len = XFS_LOG_VEC_ORDERED;
+				goto insert;
+			}
+			lv->lv_iovecp = (struct xfs_log_iovec *)&lv[1];
 		}
 
-		/* allocate new data chunk */
-		lv = kmem_zalloc(buf_size, KM_SLEEP|KM_NOFS);
-		lv->lv_item = lip;
-		lv->lv_size = buf_size;
+		/* Ensure the lv is set up according to ->iop_size */
 		lv->lv_niovecs = niovecs;
-		if (ordered) {
-			/* track as an ordered logvec */
-			ASSERT(lip->li_lv == NULL);
-			lv->lv_buf_len = XFS_LOG_VEC_ORDERED;
-			goto insert;
-		}
-
-		/* The allocated iovec region lies beyond the log vector. */
-		lv->lv_iovecp = (struct xfs_log_iovec *)&lv[1];
 
 		/* The allocated data region lies beyond the iovec region */
 		lv->lv_buf = (char *)lv + buf_size - nbytes;
-- 
1.7.10.4


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

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 02/10] xfs: refactor xfs_buf_item_format_segment
  2013-11-29  8:39 [PATCH 00/10] decouple the in-memory from the on-disk log format V2 Christoph Hellwig
  2013-11-29  8:39 ` [PATCH 01/10] xfs: remove duplicate code in xlog_cil_insert_format_items Christoph Hellwig
@ 2013-11-29  8:39 ` Christoph Hellwig
  2013-12-03  1:03   ` Dave Chinner
  2013-11-29  8:39 ` [PATCH 03/10] xfs: refactor xfs_inode_item_size Christoph Hellwig
                   ` (7 subsequent siblings)
  9 siblings, 1 reply; 29+ messages in thread
From: Christoph Hellwig @ 2013-11-29  8:39 UTC (permalink / raw)
  To: xfs

[-- Attachment #1: 0002-xfs-refactor-xfs_buf_item_format_segment.patch --]
[-- Type: text/plain, Size: 3829 bytes --]

Add two helpers to make the code more readable.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_buf_item.c |   72 ++++++++++++++++++++++++++-----------------------
 1 file changed, 39 insertions(+), 33 deletions(-)

diff --git a/fs/xfs/xfs_buf_item.c b/fs/xfs/xfs_buf_item.c
index a64f67b..a30c1fb 100644
--- a/fs/xfs/xfs_buf_item.c
+++ b/fs/xfs/xfs_buf_item.c
@@ -182,6 +182,34 @@ xfs_buf_item_size(
 	trace_xfs_buf_item_size(bip);
 }
 
+static inline struct xfs_log_iovec *
+xfs_buf_item_copy_iovec(
+	struct xfs_log_iovec	*vecp,
+	struct xfs_buf		*bp,
+	uint			offset,
+	int			first_bit,
+	uint			nbits)
+{
+	offset += first_bit * XFS_BLF_CHUNK;
+
+	vecp->i_type = XLOG_REG_TYPE_BCHUNK;
+	vecp->i_addr = xfs_buf_offset(bp, offset);
+	vecp->i_len = nbits * XFS_BLF_CHUNK;
+	return vecp + 1;
+}
+
+static inline bool
+xfs_buf_item_straddle(
+	struct xfs_buf		*bp,
+	uint			offset,
+	int			next_bit,
+	int			last_bit)
+{
+	return xfs_buf_offset(bp, offset + (next_bit << XFS_BLF_SHIFT)) !=
+		(xfs_buf_offset(bp, offset + (last_bit << XFS_BLF_SHIFT)) +
+		 XFS_BLF_CHUNK);
+}
+
 static struct xfs_log_iovec *
 xfs_buf_item_format_segment(
 	struct xfs_buf_log_item	*bip,
@@ -196,7 +224,6 @@ xfs_buf_item_format_segment(
 	int		last_bit;
 	int		next_bit;
 	uint		nbits;
-	uint		buffer_offset;
 
 	/* copy the flags across from the base format item */
 	blfp->blf_flags = bip->__bli_format.blf_flags;
@@ -239,7 +266,6 @@ xfs_buf_item_format_segment(
 	/*
 	 * Fill in an iovec for each set of contiguous chunks.
 	 */
-
 	last_bit = first_bit;
 	nbits = 1;
 	for (;;) {
@@ -252,42 +278,22 @@ xfs_buf_item_format_segment(
 		next_bit = xfs_next_bit(blfp->blf_data_map, blfp->blf_map_size,
 					(uint)last_bit + 1);
 		/*
-		 * If we run out of bits fill in the last iovec and get
-		 * out of the loop.
-		 * Else if we start a new set of bits then fill in the
-		 * iovec for the series we were looking at and start
-		 * counting the bits in the new one.
-		 * Else we're still in the same set of bits so just
-		 * keep counting and scanning.
+		 * If we run out of bits fill in the last iovec and get out of
+		 * the loop.  Else if we start a new set of bits then fill in
+		 * the iovec for the series we were looking at and start
+		 * counting the bits in the new one.  Else we're still in the
+		 * same set of bits so just keep counting and scanning.
 		 */
 		if (next_bit == -1) {
-			buffer_offset = offset + first_bit * XFS_BLF_CHUNK;
-			vecp->i_addr = xfs_buf_offset(bp, buffer_offset);
-			vecp->i_len = nbits * XFS_BLF_CHUNK;
-			vecp->i_type = XLOG_REG_TYPE_BCHUNK;
+			xfs_buf_item_copy_iovec(vecp, bp, offset,
+						first_bit, nbits);
 			nvecs++;
 			break;
-		} else if (next_bit != last_bit + 1) {
-			buffer_offset = offset + first_bit * XFS_BLF_CHUNK;
-			vecp->i_addr = xfs_buf_offset(bp, buffer_offset);
-			vecp->i_len = nbits * XFS_BLF_CHUNK;
-			vecp->i_type = XLOG_REG_TYPE_BCHUNK;
-			nvecs++;
-			vecp++;
-			first_bit = next_bit;
-			last_bit = next_bit;
-			nbits = 1;
-		} else if (xfs_buf_offset(bp, offset +
-					      (next_bit << XFS_BLF_SHIFT)) !=
-			   (xfs_buf_offset(bp, offset +
-					       (last_bit << XFS_BLF_SHIFT)) +
-			    XFS_BLF_CHUNK)) {
-			buffer_offset = offset + first_bit * XFS_BLF_CHUNK;
-			vecp->i_addr = xfs_buf_offset(bp, buffer_offset);
-			vecp->i_len = nbits * XFS_BLF_CHUNK;
-			vecp->i_type = XLOG_REG_TYPE_BCHUNK;
+		} else if (next_bit != last_bit + 1 ||
+		           xfs_buf_item_straddle(bp, offset, next_bit, last_bit)) {
+			vecp = xfs_buf_item_copy_iovec(vecp, bp, offset,
+						       first_bit, nbits);
 			nvecs++;
-			vecp++;
 			first_bit = next_bit;
 			last_bit = next_bit;
 			nbits = 1;
-- 
1.7.10.4


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

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 03/10] xfs: refactor xfs_inode_item_size
  2013-11-29  8:39 [PATCH 00/10] decouple the in-memory from the on-disk log format V2 Christoph Hellwig
  2013-11-29  8:39 ` [PATCH 01/10] xfs: remove duplicate code in xlog_cil_insert_format_items Christoph Hellwig
  2013-11-29  8:39 ` [PATCH 02/10] xfs: refactor xfs_buf_item_format_segment Christoph Hellwig
@ 2013-11-29  8:39 ` Christoph Hellwig
  2013-12-03  1:06   ` Dave Chinner
  2013-11-29  8:39 ` [PATCH 04/10] xfs: refactor xfs_inode_item_format Christoph Hellwig
                   ` (6 subsequent siblings)
  9 siblings, 1 reply; 29+ messages in thread
From: Christoph Hellwig @ 2013-11-29  8:39 UTC (permalink / raw)
  To: xfs

[-- Attachment #1: 0003-xfs-refactor-xfs_inode_item_size.patch --]
[-- Type: text/plain, Size: 3798 bytes --]

Split out two helpers to size the data and attribute to make the
function more readable.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_inode_item.c |   62 ++++++++++++++++++++++++++---------------------
 1 file changed, 35 insertions(+), 27 deletions(-)

diff --git a/fs/xfs/xfs_inode_item.c b/fs/xfs/xfs_inode_item.c
index 7c0d391f..050d254 100644
--- a/fs/xfs/xfs_inode_item.c
+++ b/fs/xfs/xfs_inode_item.c
@@ -39,27 +39,14 @@ static inline struct xfs_inode_log_item *INODE_ITEM(struct xfs_log_item *lip)
 	return container_of(lip, struct xfs_inode_log_item, ili_item);
 }
 
-
-/*
- * This returns the number of iovecs needed to log the given inode item.
- *
- * We need one iovec for the inode log format structure, one for the
- * inode core, and possibly one for the inode data/extents/b-tree root
- * and one for the inode attribute data/extents/b-tree root.
- */
 STATIC void
-xfs_inode_item_size(
-	struct xfs_log_item	*lip,
+xfs_inode_item_data_fork_size(
+	struct xfs_inode_log_item *iip,
 	int			*nvecs,
 	int			*nbytes)
 {
-	struct xfs_inode_log_item *iip = INODE_ITEM(lip);
 	struct xfs_inode	*ip = iip->ili_inode;
 
-	*nvecs += 2;
-	*nbytes += sizeof(struct xfs_inode_log_format) +
-		   xfs_icdinode_size(ip->i_d.di_version);
-
 	switch (ip->i_d.di_format) {
 	case XFS_DINODE_FMT_EXTENTS:
 		if ((iip->ili_fields & XFS_ILOG_DEXT) &&
@@ -70,7 +57,6 @@ xfs_inode_item_size(
 			*nvecs += 1;
 		}
 		break;
-
 	case XFS_DINODE_FMT_BTREE:
 		if ((iip->ili_fields & XFS_ILOG_DBROOT) &&
 		    ip->i_df.if_broot_bytes > 0) {
@@ -78,7 +64,6 @@ xfs_inode_item_size(
 			*nvecs += 1;
 		}
 		break;
-
 	case XFS_DINODE_FMT_LOCAL:
 		if ((iip->ili_fields & XFS_ILOG_DDATA) &&
 		    ip->i_df.if_bytes > 0) {
@@ -90,19 +75,20 @@ xfs_inode_item_size(
 	case XFS_DINODE_FMT_DEV:
 	case XFS_DINODE_FMT_UUID:
 		break;
-
 	default:
 		ASSERT(0);
 		break;
 	}
+}
 
-	if (!XFS_IFORK_Q(ip))
-		return;
-
+STATIC void
+xfs_inode_item_attr_fork_size(
+	struct xfs_inode_log_item *iip,
+	int			*nvecs,
+	int			*nbytes)
+{
+	struct xfs_inode	*ip = iip->ili_inode;
 
-	/*
-	 * Log any necessary attribute data.
-	 */
 	switch (ip->i_d.di_aformat) {
 	case XFS_DINODE_FMT_EXTENTS:
 		if ((iip->ili_fields & XFS_ILOG_AEXT) &&
@@ -113,7 +99,6 @@ xfs_inode_item_size(
 			*nvecs += 1;
 		}
 		break;
-
 	case XFS_DINODE_FMT_BTREE:
 		if ((iip->ili_fields & XFS_ILOG_ABROOT) &&
 		    ip->i_afp->if_broot_bytes > 0) {
@@ -121,7 +106,6 @@ xfs_inode_item_size(
 			*nvecs += 1;
 		}
 		break;
-
 	case XFS_DINODE_FMT_LOCAL:
 		if ((iip->ili_fields & XFS_ILOG_ADATA) &&
 		    ip->i_afp->if_bytes > 0) {
@@ -129,7 +113,6 @@ xfs_inode_item_size(
 			*nvecs += 1;
 		}
 		break;
-
 	default:
 		ASSERT(0);
 		break;
@@ -137,6 +120,31 @@ xfs_inode_item_size(
 }
 
 /*
+ * This returns the number of iovecs needed to log the given inode item.
+ *
+ * We need one iovec for the inode log format structure, one for the
+ * inode core, and possibly one for the inode data/extents/b-tree root
+ * and one for the inode attribute data/extents/b-tree root.
+ */
+STATIC void
+xfs_inode_item_size(
+	struct xfs_log_item	*lip,
+	int			*nvecs,
+	int			*nbytes)
+{
+	struct xfs_inode_log_item *iip = INODE_ITEM(lip);
+	struct xfs_inode	*ip = iip->ili_inode;
+
+	*nvecs += 2;
+	*nbytes += sizeof(struct xfs_inode_log_format) +
+		   xfs_icdinode_size(ip->i_d.di_version);
+
+	xfs_inode_item_data_fork_size(iip, nvecs, nbytes);
+	if (XFS_IFORK_Q(ip))
+		xfs_inode_item_attr_fork_size(iip, nvecs, nbytes);
+}
+
+/*
  * xfs_inode_item_format_extents - convert in-core extents to on-disk form
  *
  * For either the data or attr fork in extent format, we need to endian convert
-- 
1.7.10.4


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

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 04/10] xfs: refactor xfs_inode_item_format
  2013-11-29  8:39 [PATCH 00/10] decouple the in-memory from the on-disk log format V2 Christoph Hellwig
                   ` (2 preceding siblings ...)
  2013-11-29  8:39 ` [PATCH 03/10] xfs: refactor xfs_inode_item_size Christoph Hellwig
@ 2013-11-29  8:39 ` Christoph Hellwig
  2013-12-03  1:10   ` Dave Chinner
  2013-11-29  8:39 ` [PATCH 05/10] xfs: introduce xlog_copy_iovec Christoph Hellwig
                   ` (5 subsequent siblings)
  9 siblings, 1 reply; 29+ messages in thread
From: Christoph Hellwig @ 2013-11-29  8:39 UTC (permalink / raw)
  To: xfs

[-- Attachment #1: 0004-xfs-refactor-xfs_inode_item_format.patch --]
[-- Type: text/plain, Size: 8044 bytes --]

Split out a function to handle the data and attr fork, as well as a helper
for the really old v1 inodes.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_inode_item.c |  165 +++++++++++++++++++++++++----------------------
 1 file changed, 89 insertions(+), 76 deletions(-)

diff --git a/fs/xfs/xfs_inode_item.c b/fs/xfs/xfs_inode_item.c
index 050d254..96554a3 100644
--- a/fs/xfs/xfs_inode_item.c
+++ b/fs/xfs/xfs_inode_item.c
@@ -180,63 +180,41 @@ xfs_inode_item_format_extents(
 }
 
 /*
- * 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, the second with the on-disk inode structure,
- * and a possible third and/or fourth with the inode data/extents/b-tree
- * root and inode attributes data/extents/b-tree root.
+ * If this is a v1 format inode, then we need to log it as such.  This means
+ * that we have to copy the link count from the new field to the old.  We
+ * don't have to worry about the new fields, because nothing trusts them as
+ * long as the old inode version number is there.
  */
 STATIC void
-xfs_inode_item_format(
-	struct xfs_log_item	*lip,
-	struct xfs_log_iovec	*vecp)
+xfs_inode_item_format_v1_inode(
+	struct xfs_inode	*ip)
+{
+	if (!xfs_sb_version_hasnlink(&ip->i_mount->m_sb)) {
+		/*
+		 * Convert it back.
+		 */
+		ASSERT(ip->i_d.di_nlink <= XFS_MAXLINK_1);
+		ip->i_d.di_onlink = ip->i_d.di_nlink;
+	} else {
+		/*
+		 * The superblock version has already been bumped,
+		 * so just make the conversion to the new inode
+		 * format permanent.
+		 */
+		ip->i_d.di_version = 2;
+		ip->i_d.di_onlink = 0;
+		memset(&(ip->i_d.di_pad[0]), 0, sizeof(ip->i_d.di_pad));
+	}
+}
+	
+STATIC struct xfs_log_iovec *
+xfs_inode_item_format_data_fork(
+	struct xfs_inode_log_item *iip,
+	struct xfs_log_iovec	*vecp,
+	int			*nvecs)
 {
-	struct xfs_inode_log_item *iip = INODE_ITEM(lip);
 	struct xfs_inode	*ip = iip->ili_inode;
-	uint			nvecs;
 	size_t			data_bytes;
-	xfs_mount_t		*mp;
-
-	vecp->i_addr = &iip->ili_format;
-	vecp->i_len  = sizeof(xfs_inode_log_format_t);
-	vecp->i_type = XLOG_REG_TYPE_IFORMAT;
-	vecp++;
-	nvecs	     = 1;
-
-	vecp->i_addr = &ip->i_d;
-	vecp->i_len  = xfs_icdinode_size(ip->i_d.di_version);
-	vecp->i_type = XLOG_REG_TYPE_ICORE;
-	vecp++;
-	nvecs++;
-
-	/*
-	 * If this is really an old format inode, then we need to
-	 * log it as such.  This means that we have to copy the link
-	 * count from the new field to the old.  We don't have to worry
-	 * about the new fields, because nothing trusts them as long as
-	 * the old inode version number is there.  If the superblock already
-	 * has a new version number, then we don't bother converting back.
-	 */
-	mp = ip->i_mount;
-	ASSERT(ip->i_d.di_version == 1 || xfs_sb_version_hasnlink(&mp->m_sb));
-	if (ip->i_d.di_version == 1) {
-		if (!xfs_sb_version_hasnlink(&mp->m_sb)) {
-			/*
-			 * Convert it back.
-			 */
-			ASSERT(ip->i_d.di_nlink <= XFS_MAXLINK_1);
-			ip->i_d.di_onlink = ip->i_d.di_nlink;
-		} else {
-			/*
-			 * The superblock version has already been bumped,
-			 * so just make the conversion to the new inode
-			 * format permanent.
-			 */
-			ip->i_d.di_version = 2;
-			ip->i_d.di_onlink = 0;
-			memset(&(ip->i_d.di_pad[0]), 0, sizeof(ip->i_d.di_pad));
-		}
-	}
 
 	switch (ip->i_d.di_format) {
 	case XFS_DINODE_FMT_EXTENTS:
@@ -271,12 +249,11 @@ xfs_inode_item_format(
 			ASSERT(vecp->i_len <= ip->i_df.if_bytes);
 			iip->ili_format.ilf_dsize = vecp->i_len;
 			vecp++;
-			nvecs++;
+			(*nvecs)++;
 		} else {
 			iip->ili_fields &= ~XFS_ILOG_DEXT;
 		}
 		break;
-
 	case XFS_DINODE_FMT_BTREE:
 		iip->ili_fields &=
 			~(XFS_ILOG_DDATA | XFS_ILOG_DEXT |
@@ -289,7 +266,7 @@ xfs_inode_item_format(
 			vecp->i_len = ip->i_df.if_broot_bytes;
 			vecp->i_type = XLOG_REG_TYPE_IBROOT;
 			vecp++;
-			nvecs++;
+			(*nvecs)++;
 			iip->ili_format.ilf_dsize = ip->i_df.if_broot_bytes;
 		} else {
 			ASSERT(!(iip->ili_fields &
@@ -297,7 +274,6 @@ xfs_inode_item_format(
 			iip->ili_fields &= ~XFS_ILOG_DBROOT;
 		}
 		break;
-
 	case XFS_DINODE_FMT_LOCAL:
 		iip->ili_fields &=
 			~(XFS_ILOG_DEXT | XFS_ILOG_DBROOT |
@@ -319,13 +295,12 @@ xfs_inode_item_format(
 			vecp->i_len = (int)data_bytes;
 			vecp->i_type = XLOG_REG_TYPE_ILOCAL;
 			vecp++;
-			nvecs++;
+			(*nvecs)++;
 			iip->ili_format.ilf_dsize = (unsigned)data_bytes;
 		} else {
 			iip->ili_fields &= ~XFS_ILOG_DDATA;
 		}
 		break;
-
 	case XFS_DINODE_FMT_DEV:
 		iip->ili_fields &=
 			~(XFS_ILOG_DDATA | XFS_ILOG_DBROOT |
@@ -335,7 +310,6 @@ xfs_inode_item_format(
 				ip->i_df.if_u2.if_rdev;
 		}
 		break;
-
 	case XFS_DINODE_FMT_UUID:
 		iip->ili_fields &=
 			~(XFS_ILOG_DDATA | XFS_ILOG_DBROOT |
@@ -345,20 +319,22 @@ xfs_inode_item_format(
 				ip->i_df.if_u2.if_uuid;
 		}
 		break;
-
 	default:
 		ASSERT(0);
 		break;
 	}
 
-	/*
-	 * If there are no attributes associated with the file, then we're done.
-	 */
-	if (!XFS_IFORK_Q(ip)) {
-		iip->ili_fields &=
-			~(XFS_ILOG_ADATA | XFS_ILOG_ABROOT | XFS_ILOG_AEXT);
-		goto out;
-	}
+	return vecp;
+}
+
+STATIC struct xfs_log_iovec *
+xfs_inode_item_format_attr_fork(
+	struct xfs_inode_log_item *iip,
+	struct xfs_log_iovec	*vecp,
+	int			*nvecs)
+{
+	struct xfs_inode	*ip = iip->ili_inode;
+	size_t			data_bytes;
 
 	switch (ip->i_d.di_aformat) {
 	case XFS_DINODE_FMT_EXTENTS:
@@ -386,12 +362,11 @@ xfs_inode_item_format(
 #endif
 			iip->ili_format.ilf_asize = vecp->i_len;
 			vecp++;
-			nvecs++;
+			(*nvecs)++;
 		} else {
 			iip->ili_fields &= ~XFS_ILOG_AEXT;
 		}
 		break;
-
 	case XFS_DINODE_FMT_BTREE:
 		iip->ili_fields &=
 			~(XFS_ILOG_ADATA | XFS_ILOG_AEXT);
@@ -404,13 +379,12 @@ xfs_inode_item_format(
 			vecp->i_len = ip->i_afp->if_broot_bytes;
 			vecp->i_type = XLOG_REG_TYPE_IATTR_BROOT;
 			vecp++;
-			nvecs++;
+			(*nvecs)++;
 			iip->ili_format.ilf_asize = ip->i_afp->if_broot_bytes;
 		} else {
 			iip->ili_fields &= ~XFS_ILOG_ABROOT;
 		}
 		break;
-
 	case XFS_DINODE_FMT_LOCAL:
 		iip->ili_fields &=
 			~(XFS_ILOG_AEXT | XFS_ILOG_ABROOT);
@@ -431,19 +405,59 @@ xfs_inode_item_format(
 			vecp->i_len = (int)data_bytes;
 			vecp->i_type = XLOG_REG_TYPE_IATTR_LOCAL;
 			vecp++;
-			nvecs++;
+			(*nvecs)++;
 			iip->ili_format.ilf_asize = (unsigned)data_bytes;
 		} else {
 			iip->ili_fields &= ~XFS_ILOG_ADATA;
 		}
 		break;
-
 	default:
 		ASSERT(0);
 		break;
 	}
 
-out:
+	return vecp;
+}
+
+/*
+ * 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,
+ * the second with the on-disk inode structure, and a possible third and/or
+ * fourth with the inode data/extents/b-tree root and inode attributes
+ * data/extents/b-tree root.
+ */
+STATIC void
+xfs_inode_item_format(
+	struct xfs_log_item	*lip,
+	struct xfs_log_iovec	*vecp)
+{
+	struct xfs_inode_log_item *iip = INODE_ITEM(lip);
+	struct xfs_inode	*ip = iip->ili_inode;
+	uint			nvecs;
+
+	vecp->i_addr = &iip->ili_format;
+	vecp->i_len  = sizeof(xfs_inode_log_format_t);
+	vecp->i_type = XLOG_REG_TYPE_IFORMAT;
+	vecp++;
+	nvecs	     = 1;
+
+	vecp->i_addr = &ip->i_d;
+	vecp->i_len  = xfs_icdinode_size(ip->i_d.di_version);
+	vecp->i_type = XLOG_REG_TYPE_ICORE;
+	vecp++;
+	nvecs++;
+
+	if (ip->i_d.di_version == 1)
+		xfs_inode_item_format_v1_inode(ip);
+
+	vecp = xfs_inode_item_format_data_fork(iip, vecp, &nvecs);
+	if (XFS_IFORK_Q(ip)) {
+		vecp = xfs_inode_item_format_attr_fork(iip, vecp, &nvecs);
+	} else {
+		iip->ili_fields &=
+			~(XFS_ILOG_ADATA | XFS_ILOG_ABROOT | XFS_ILOG_AEXT);
+	}
+
 	/*
 	 * Now update the log format that goes out to disk from the in-core
 	 * values.  We always write the inode core to make the arithmetic
@@ -455,7 +469,6 @@ out:
 	iip->ili_format.ilf_size = nvecs;
 }
 
-
 /*
  * This is called to pin the inode associated with the inode log
  * item in memory so it cannot be written out.
-- 
1.7.10.4


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

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 05/10] xfs: introduce xlog_copy_iovec
  2013-11-29  8:39 [PATCH 00/10] decouple the in-memory from the on-disk log format V2 Christoph Hellwig
                   ` (3 preceding siblings ...)
  2013-11-29  8:39 ` [PATCH 04/10] xfs: refactor xfs_inode_item_format Christoph Hellwig
@ 2013-11-29  8:39 ` Christoph Hellwig
  2013-12-03  1:21   ` Dave Chinner
  2013-11-29  8:39 ` [PATCH 06/10] xfs: format log items write directly into the linear CIL buffer Christoph Hellwig
                   ` (4 subsequent siblings)
  9 siblings, 1 reply; 29+ messages in thread
From: Christoph Hellwig @ 2013-11-29  8:39 UTC (permalink / raw)
  To: xfs

[-- Attachment #1: 0005-xfs-introduce-xlog_copy_iovec.patch --]
[-- Type: text/plain, Size: 15139 bytes --]

Add a helper to abstract out filling the log iovecs in the log item format
handlers.  This will allow us to change the way we do the log item
formatting more easily.

The copy in the name is a bit confusing for now as it just assigns a
pointer and lets the CIL code perform the copy, but that will change
soon.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_buf_item.c     |   30 +++++-------
 fs/xfs/xfs_dquot_item.c   |   25 +++++-----
 fs/xfs/xfs_extfree_item.c |   19 ++++----
 fs/xfs/xfs_icreate_item.c |    9 ++--
 fs/xfs/xfs_inode_item.c   |  115 ++++++++++++++++++++-------------------------
 fs/xfs/xfs_log.h          |   13 +++++
 6 files changed, 103 insertions(+), 108 deletions(-)

diff --git a/fs/xfs/xfs_buf_item.c b/fs/xfs/xfs_buf_item.c
index a30c1fb..d49419d 100644
--- a/fs/xfs/xfs_buf_item.c
+++ b/fs/xfs/xfs_buf_item.c
@@ -182,20 +182,18 @@ xfs_buf_item_size(
 	trace_xfs_buf_item_size(bip);
 }
 
-static inline struct xfs_log_iovec *
+static inline void
 xfs_buf_item_copy_iovec(
-	struct xfs_log_iovec	*vecp,
+	struct xfs_log_iovec	**vecp,
 	struct xfs_buf		*bp,
 	uint			offset,
 	int			first_bit,
 	uint			nbits)
 {
 	offset += first_bit * XFS_BLF_CHUNK;
-
-	vecp->i_type = XLOG_REG_TYPE_BCHUNK;
-	vecp->i_addr = xfs_buf_offset(bp, offset);
-	vecp->i_len = nbits * XFS_BLF_CHUNK;
-	return vecp + 1;
+	xlog_copy_iovec(vecp, XLOG_REG_TYPE_BCHUNK,
+			xfs_buf_offset(bp, offset),
+			nbits * XFS_BLF_CHUNK);
 }
 
 static inline bool
@@ -210,10 +208,10 @@ xfs_buf_item_straddle(
 		 XFS_BLF_CHUNK);
 }
 
-static struct xfs_log_iovec *
+static void
 xfs_buf_item_format_segment(
 	struct xfs_buf_log_item	*bip,
-	struct xfs_log_iovec	*vecp,
+	struct xfs_log_iovec	**vecp,
 	uint			offset,
 	struct xfs_buf_log_format *blfp)
 {
@@ -245,10 +243,7 @@ xfs_buf_item_format_segment(
 		goto out;
 	}
 
-	vecp->i_addr = blfp;
-	vecp->i_len = base_size;
-	vecp->i_type = XLOG_REG_TYPE_BFORMAT;
-	vecp++;
+	xlog_copy_iovec(vecp, XLOG_REG_TYPE_BFORMAT, blfp, base_size);
 	nvecs = 1;
 
 	if (bip->bli_flags & XFS_BLI_STALE) {
@@ -291,8 +286,8 @@ xfs_buf_item_format_segment(
 			break;
 		} else if (next_bit != last_bit + 1 ||
 		           xfs_buf_item_straddle(bp, offset, next_bit, last_bit)) {
-			vecp = xfs_buf_item_copy_iovec(vecp, bp, offset,
-						       first_bit, nbits);
+			xfs_buf_item_copy_iovec(vecp, bp, offset,
+						first_bit, nbits);
 			nvecs++;
 			first_bit = next_bit;
 			last_bit = next_bit;
@@ -304,7 +299,6 @@ xfs_buf_item_format_segment(
 	}
 out:
 	blfp->blf_size = nvecs;
-	return vecp;
 }
 
 /*
@@ -360,8 +354,8 @@ xfs_buf_item_format(
 	}
 
 	for (i = 0; i < bip->bli_format_count; i++) {
-		vecp = xfs_buf_item_format_segment(bip, vecp, offset,
-						&bip->bli_formats[i]);
+		xfs_buf_item_format_segment(bip, &vecp, offset,
+					    &bip->bli_formats[i]);
 		offset += bp->b_maps[i].bm_len;
 	}
 
diff --git a/fs/xfs/xfs_dquot_item.c b/fs/xfs/xfs_dquot_item.c
index 92e5f62..ca354a8 100644
--- a/fs/xfs/xfs_dquot_item.c
+++ b/fs/xfs/xfs_dquot_item.c
@@ -57,20 +57,18 @@ xfs_qm_dquot_logitem_size(
 STATIC void
 xfs_qm_dquot_logitem_format(
 	struct xfs_log_item	*lip,
-	struct xfs_log_iovec	*logvec)
+	struct xfs_log_iovec	*vecp)
 {
 	struct xfs_dq_logitem	*qlip = DQUOT_ITEM(lip);
 
-	logvec->i_addr = &qlip->qli_format;
-	logvec->i_len  = sizeof(xfs_dq_logformat_t);
-	logvec->i_type = XLOG_REG_TYPE_QFORMAT;
-	logvec++;
-	logvec->i_addr = &qlip->qli_dquot->q_core;
-	logvec->i_len  = sizeof(xfs_disk_dquot_t);
-	logvec->i_type = XLOG_REG_TYPE_DQUOT;
+	xlog_copy_iovec(&vecp, XLOG_REG_TYPE_QFORMAT,
+			&qlip->qli_format,
+			sizeof(struct xfs_dq_logformat));
+	xlog_copy_iovec(&vecp, XLOG_REG_TYPE_DQUOT,
+			&qlip->qli_dquot->q_core,
+			sizeof(struct xfs_disk_dquot));
 
 	qlip->qli_format.qlf_size = 2;
-
 }
 
 /*
@@ -304,15 +302,16 @@ xfs_qm_qoff_logitem_size(
 STATIC void
 xfs_qm_qoff_logitem_format(
 	struct xfs_log_item	*lip,
-	struct xfs_log_iovec	*log_vector)
+	struct xfs_log_iovec	*vecp)
 {
 	struct xfs_qoff_logitem	*qflip = QOFF_ITEM(lip);
 
 	ASSERT(qflip->qql_format.qf_type == XFS_LI_QUOTAOFF);
 
-	log_vector->i_addr = &qflip->qql_format;
-	log_vector->i_len = sizeof(xfs_qoff_logitem_t);
-	log_vector->i_type = XLOG_REG_TYPE_QUOTAOFF;
+	xlog_copy_iovec(&vecp, XLOG_REG_TYPE_QUOTAOFF,
+			&qflip->qql_format,
+			sizeof(struct xfs_qoff_logitem));
+
 	qflip->qql_format.qf_size = 1;
 }
 
diff --git a/fs/xfs/xfs_extfree_item.c b/fs/xfs/xfs_extfree_item.c
index 3680d04..08823ec 100644
--- a/fs/xfs/xfs_extfree_item.c
+++ b/fs/xfs/xfs_extfree_item.c
@@ -26,6 +26,7 @@
 #include "xfs_trans_priv.h"
 #include "xfs_buf_item.h"
 #include "xfs_extfree_item.h"
+#include "xfs_log.h"
 
 
 kmem_zone_t	*xfs_efi_zone;
@@ -101,7 +102,7 @@ xfs_efi_item_size(
 STATIC void
 xfs_efi_item_format(
 	struct xfs_log_item	*lip,
-	struct xfs_log_iovec	*log_vector)
+	struct xfs_log_iovec	*vecp)
 {
 	struct xfs_efi_log_item	*efip = EFI_ITEM(lip);
 
@@ -111,10 +112,9 @@ xfs_efi_item_format(
 	efip->efi_format.efi_type = XFS_LI_EFI;
 	efip->efi_format.efi_size = 1;
 
-	log_vector->i_addr = &efip->efi_format;
-	log_vector->i_len = xfs_efi_item_sizeof(efip);
-	log_vector->i_type = XLOG_REG_TYPE_EFI_FORMAT;
-	ASSERT(log_vector->i_len >= sizeof(xfs_efi_log_format_t));
+	xlog_copy_iovec(&vecp, XLOG_REG_TYPE_EFI_FORMAT,
+			&efip->efi_format,
+			xfs_efi_item_sizeof(efip));
 }
 
 
@@ -368,7 +368,7 @@ xfs_efd_item_size(
 STATIC void
 xfs_efd_item_format(
 	struct xfs_log_item	*lip,
-	struct xfs_log_iovec	*log_vector)
+	struct xfs_log_iovec	*vecp)
 {
 	struct xfs_efd_log_item	*efdp = EFD_ITEM(lip);
 
@@ -377,10 +377,9 @@ xfs_efd_item_format(
 	efdp->efd_format.efd_type = XFS_LI_EFD;
 	efdp->efd_format.efd_size = 1;
 
-	log_vector->i_addr = &efdp->efd_format;
-	log_vector->i_len = xfs_efd_item_sizeof(efdp);
-	log_vector->i_type = XLOG_REG_TYPE_EFD_FORMAT;
-	ASSERT(log_vector->i_len >= sizeof(xfs_efd_log_format_t));
+	xlog_copy_iovec(&vecp, XLOG_REG_TYPE_EFD_FORMAT,
+			&efdp->efd_format,
+			xfs_efd_item_sizeof(efdp));
 }
 
 /*
diff --git a/fs/xfs/xfs_icreate_item.c b/fs/xfs/xfs_icreate_item.c
index d2eaccf..5751fa8 100644
--- a/fs/xfs/xfs_icreate_item.c
+++ b/fs/xfs/xfs_icreate_item.c
@@ -28,6 +28,7 @@
 #include "xfs_trans_priv.h"
 #include "xfs_error.h"
 #include "xfs_icreate_item.h"
+#include "xfs_log.h"
 
 kmem_zone_t	*xfs_icreate_zone;		/* inode create item zone */
 
@@ -58,13 +59,13 @@ xfs_icreate_item_size(
 STATIC void
 xfs_icreate_item_format(
 	struct xfs_log_item	*lip,
-	struct xfs_log_iovec	*log_vector)
+	struct xfs_log_iovec	*vecp)
 {
 	struct xfs_icreate_item	*icp = ICR_ITEM(lip);
 
-	log_vector->i_addr = (xfs_caddr_t)&icp->ic_format;
-	log_vector->i_len  = sizeof(struct xfs_icreate_log);
-	log_vector->i_type = XLOG_REG_TYPE_ICREATE;
+	xlog_copy_iovec(&vecp, XLOG_REG_TYPE_ICREATE,
+			&icp->ic_format,
+			sizeof(struct xfs_icreate_log));
 }
 
 
diff --git a/fs/xfs/xfs_inode_item.c b/fs/xfs/xfs_inode_item.c
index 96554a3..73002db 100644
--- a/fs/xfs/xfs_inode_item.c
+++ b/fs/xfs/xfs_inode_item.c
@@ -30,6 +30,7 @@
 #include "xfs_trace.h"
 #include "xfs_trans_priv.h"
 #include "xfs_dinode.h"
+#include "xfs_log.h"
 
 
 kmem_zone_t	*xfs_ili_zone;		/* inode log item zone */
@@ -159,14 +160,15 @@ xfs_inode_item_size(
  * here, so always use the physical fork size to determine the size of the
  * buffer we need to allocate.
  */
-STATIC void
+STATIC int
 xfs_inode_item_format_extents(
 	struct xfs_inode	*ip,
-	struct xfs_log_iovec	*vecp,
+	struct xfs_log_iovec	**vecp,
 	int			whichfork,
 	int			type)
 {
 	xfs_bmbt_rec_t		*ext_buffer;
+	int			len;
 
 	ext_buffer = kmem_alloc(XFS_IFORK_SIZE(ip, whichfork), KM_SLEEP);
 	if (whichfork == XFS_DATA_FORK)
@@ -174,9 +176,9 @@ xfs_inode_item_format_extents(
 	else
 		ip->i_itemp->ili_aextents_buf = ext_buffer;
 
-	vecp->i_addr = ext_buffer;
-	vecp->i_len = xfs_iextents_copy(ip, ext_buffer, whichfork);
-	vecp->i_type = type;
+	len = xfs_iextents_copy(ip, ext_buffer, whichfork);
+	xlog_copy_iovec(vecp, type, ext_buffer, len);
+	return len;
 }
 
 /*
@@ -207,10 +209,10 @@ xfs_inode_item_format_v1_inode(
 	}
 }
 	
-STATIC struct xfs_log_iovec *
+STATIC void
 xfs_inode_item_format_data_fork(
 	struct xfs_inode_log_item *iip,
-	struct xfs_log_iovec	*vecp,
+	struct xfs_log_iovec	**vecp,
 	int			*nvecs)
 {
 	struct xfs_inode	*ip = iip->ili_inode;
@@ -237,18 +239,18 @@ xfs_inode_item_format_data_fork(
 				 * extents, so just point to the
 				 * real extents array.
 				 */
-				vecp->i_addr = ip->i_df.if_u1.if_extents;
-				vecp->i_len = ip->i_df.if_bytes;
-				vecp->i_type = XLOG_REG_TYPE_IEXT;
+				xlog_copy_iovec(vecp, XLOG_REG_TYPE_IEXT,
+						ip->i_df.if_u1.if_extents,
+						ip->i_df.if_bytes);
+				iip->ili_format.ilf_dsize = ip->i_df.if_bytes;
 			} else
 #endif
 			{
-				xfs_inode_item_format_extents(ip, vecp,
-					XFS_DATA_FORK, XLOG_REG_TYPE_IEXT);
+				iip->ili_format.ilf_dsize =
+					xfs_inode_item_format_extents(ip, vecp,
+						XFS_DATA_FORK, XLOG_REG_TYPE_IEXT);
+				ASSERT(iip->ili_format.ilf_dsize <= ip->i_df.if_bytes);
 			}
-			ASSERT(vecp->i_len <= ip->i_df.if_bytes);
-			iip->ili_format.ilf_dsize = vecp->i_len;
-			vecp++;
 			(*nvecs)++;
 		} else {
 			iip->ili_fields &= ~XFS_ILOG_DEXT;
@@ -262,10 +264,9 @@ xfs_inode_item_format_data_fork(
 		if ((iip->ili_fields & XFS_ILOG_DBROOT) &&
 		    ip->i_df.if_broot_bytes > 0) {
 			ASSERT(ip->i_df.if_broot != NULL);
-			vecp->i_addr = ip->i_df.if_broot;
-			vecp->i_len = ip->i_df.if_broot_bytes;
-			vecp->i_type = XLOG_REG_TYPE_IBROOT;
-			vecp++;
+			xlog_copy_iovec(vecp, XLOG_REG_TYPE_IBROOT,
+					ip->i_df.if_broot,
+					ip->i_df.if_broot_bytes);
 			(*nvecs)++;
 			iip->ili_format.ilf_dsize = ip->i_df.if_broot_bytes;
 		} else {
@@ -280,21 +281,18 @@ xfs_inode_item_format_data_fork(
 			  XFS_ILOG_DEV | XFS_ILOG_UUID);
 		if ((iip->ili_fields & XFS_ILOG_DDATA) &&
 		    ip->i_df.if_bytes > 0) {
-			ASSERT(ip->i_df.if_u1.if_data != NULL);
-			ASSERT(ip->i_d.di_size > 0);
-
-			vecp->i_addr = ip->i_df.if_u1.if_data;
 			/*
 			 * Round i_bytes up to a word boundary.
 			 * The underlying memory is guaranteed to
 			 * to be there by xfs_idata_realloc().
 			 */
 			data_bytes = roundup(ip->i_df.if_bytes, 4);
-			ASSERT((ip->i_df.if_real_bytes == 0) ||
-			       (ip->i_df.if_real_bytes == data_bytes));
-			vecp->i_len = (int)data_bytes;
-			vecp->i_type = XLOG_REG_TYPE_ILOCAL;
-			vecp++;
+			ASSERT(ip->i_df.if_real_bytes == 0 ||
+			       ip->i_df.if_real_bytes == data_bytes);
+			ASSERT(ip->i_df.if_u1.if_data != NULL);
+			ASSERT(ip->i_d.di_size > 0);
+			xlog_copy_iovec(vecp, XLOG_REG_TYPE_ILOCAL,
+					ip->i_df.if_u1.if_data, data_bytes);
 			(*nvecs)++;
 			iip->ili_format.ilf_dsize = (unsigned)data_bytes;
 		} else {
@@ -323,14 +321,12 @@ xfs_inode_item_format_data_fork(
 		ASSERT(0);
 		break;
 	}
-
-	return vecp;
 }
 
-STATIC struct xfs_log_iovec *
+STATIC void
 xfs_inode_item_format_attr_fork(
 	struct xfs_inode_log_item *iip,
-	struct xfs_log_iovec	*vecp,
+	struct xfs_log_iovec	**vecp,
 	int			*nvecs)
 {
 	struct xfs_inode	*ip = iip->ili_inode;
@@ -352,16 +348,16 @@ xfs_inode_item_format_attr_fork(
 			 * There are not delayed allocation extents
 			 * for attributes, so just point at the array.
 			 */
-			vecp->i_addr = ip->i_afp->if_u1.if_extents;
-			vecp->i_len = ip->i_afp->if_bytes;
-			vecp->i_type = XLOG_REG_TYPE_IATTR_EXT;
+			xlog_copy_iovec(vecp, XLOG_REG_TYPE_IATTR_EXT,
+					ip->i_afp->if_u1.if_extents,
+					ip->i_afp->if_bytes);
+			iip->ili_format.ilf_asize = ip->i_afp->if_bytes;
 #else
 			ASSERT(iip->ili_aextents_buf == NULL);
-			xfs_inode_item_format_extents(ip, vecp,
+			iip->ili_format.ilf_asize =
+				xfs_inode_item_format_extents(ip, vecp,
 					XFS_ATTR_FORK, XLOG_REG_TYPE_IATTR_EXT);
 #endif
-			iip->ili_format.ilf_asize = vecp->i_len;
-			vecp++;
 			(*nvecs)++;
 		} else {
 			iip->ili_fields &= ~XFS_ILOG_AEXT;
@@ -375,10 +371,9 @@ xfs_inode_item_format_attr_fork(
 		    ip->i_afp->if_broot_bytes > 0) {
 			ASSERT(ip->i_afp->if_broot != NULL);
 
-			vecp->i_addr = ip->i_afp->if_broot;
-			vecp->i_len = ip->i_afp->if_broot_bytes;
-			vecp->i_type = XLOG_REG_TYPE_IATTR_BROOT;
-			vecp++;
+			xlog_copy_iovec(vecp, XLOG_REG_TYPE_IATTR_BROOT,
+					ip->i_afp->if_broot,
+					ip->i_afp->if_broot_bytes);
 			(*nvecs)++;
 			iip->ili_format.ilf_asize = ip->i_afp->if_broot_bytes;
 		} else {
@@ -391,20 +386,18 @@ xfs_inode_item_format_attr_fork(
 
 		if ((iip->ili_fields & XFS_ILOG_ADATA) &&
 		    ip->i_afp->if_bytes > 0) {
-			ASSERT(ip->i_afp->if_u1.if_data != NULL);
-
-			vecp->i_addr = ip->i_afp->if_u1.if_data;
 			/*
 			 * Round i_bytes up to a word boundary.
 			 * The underlying memory is guaranteed to
 			 * to be there by xfs_idata_realloc().
 			 */
 			data_bytes = roundup(ip->i_afp->if_bytes, 4);
-			ASSERT((ip->i_afp->if_real_bytes == 0) ||
-			       (ip->i_afp->if_real_bytes == data_bytes));
-			vecp->i_len = (int)data_bytes;
-			vecp->i_type = XLOG_REG_TYPE_IATTR_LOCAL;
-			vecp++;
+			ASSERT(ip->i_afp->if_real_bytes == 0 ||
+			       ip->i_afp->if_real_bytes == data_bytes);
+			ASSERT(ip->i_afp->if_u1.if_data != NULL);
+			xlog_copy_iovec(vecp, XLOG_REG_TYPE_IATTR_LOCAL,
+					ip->i_afp->if_u1.if_data,
+					data_bytes);
 			(*nvecs)++;
 			iip->ili_format.ilf_asize = (unsigned)data_bytes;
 		} else {
@@ -415,8 +408,6 @@ xfs_inode_item_format_attr_fork(
 		ASSERT(0);
 		break;
 	}
-
-	return vecp;
 }
 
 /*
@@ -435,24 +426,22 @@ xfs_inode_item_format(
 	struct xfs_inode	*ip = iip->ili_inode;
 	uint			nvecs;
 
-	vecp->i_addr = &iip->ili_format;
-	vecp->i_len  = sizeof(xfs_inode_log_format_t);
-	vecp->i_type = XLOG_REG_TYPE_IFORMAT;
-	vecp++;
-	nvecs	     = 1;
+	xlog_copy_iovec(&vecp, XLOG_REG_TYPE_IFORMAT,
+			&iip->ili_format,
+			sizeof(struct xfs_inode_log_format));
+	nvecs = 1;
 
-	vecp->i_addr = &ip->i_d;
-	vecp->i_len  = xfs_icdinode_size(ip->i_d.di_version);
-	vecp->i_type = XLOG_REG_TYPE_ICORE;
-	vecp++;
+	xlog_copy_iovec(&vecp, XLOG_REG_TYPE_ICORE,
+			&ip->i_d,
+			xfs_icdinode_size(ip->i_d.di_version));
 	nvecs++;
 
 	if (ip->i_d.di_version == 1)
 		xfs_inode_item_format_v1_inode(ip);
 
-	vecp = xfs_inode_item_format_data_fork(iip, vecp, &nvecs);
+	xfs_inode_item_format_data_fork(iip, &vecp, &nvecs);
 	if (XFS_IFORK_Q(ip)) {
-		vecp = xfs_inode_item_format_attr_fork(iip, vecp, &nvecs);
+		xfs_inode_item_format_attr_fork(iip, &vecp, &nvecs);
 	} else {
 		iip->ili_fields &=
 			~(XFS_ILOG_ADATA | XFS_ILOG_ABROOT | XFS_ILOG_AEXT);
diff --git a/fs/xfs/xfs_log.h b/fs/xfs/xfs_log.h
index e148719..384c6c4 100644
--- a/fs/xfs/xfs_log.h
+++ b/fs/xfs/xfs_log.h
@@ -30,6 +30,19 @@ struct xfs_log_vec {
 
 #define XFS_LOG_VEC_ORDERED	(-1)
 
+static inline void *
+xlog_copy_iovec(struct xfs_log_iovec **vecp, uint type, void *data, int len)
+{
+	struct xfs_log_iovec *vec = *vecp;
+
+	vec->i_type = type;
+	vec->i_addr = data;
+	vec->i_len = len;
+
+	*vecp = vec + 1;
+	return vec->i_addr;
+}
+
 /*
  * Structure used to pass callback function and the function's argument
  * to the log manager.
-- 
1.7.10.4


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

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 06/10] xfs: format log items write directly into the linear CIL buffer
  2013-11-29  8:39 [PATCH 00/10] decouple the in-memory from the on-disk log format V2 Christoph Hellwig
                   ` (4 preceding siblings ...)
  2013-11-29  8:39 ` [PATCH 05/10] xfs: introduce xlog_copy_iovec Christoph Hellwig
@ 2013-11-29  8:39 ` Christoph Hellwig
  2013-12-04  0:37   ` Dave Chinner
  2013-12-11 12:03   ` [PATCH 06/10 v2] " Christoph Hellwig
  2013-11-29  8:39 ` [PATCH 07/10] xfs: format logged extents directly into the CIL Christoph Hellwig
                   ` (3 subsequent siblings)
  9 siblings, 2 replies; 29+ messages in thread
From: Christoph Hellwig @ 2013-11-29  8:39 UTC (permalink / raw)
  To: xfs

[-- Attachment #1: 0006-xfs-format-log-items-write-directly-into-the-linear-.patch --]
[-- Type: text/plain, Size: 18011 bytes --]

Instead of setting up pointers to memory locations in iop_format which then
get copied into the CIL linear buffer after return move the copy into
the individual inode items.  This avoids the need to always have a memory
block in the exact same layout that gets written into the log around, and
allow the log items to be much more flexible in their in-memory layouts.

Note that all log item format routines now need to be careful to modify
the copy of the item that was placed into the CIL after calls to
xlog_copy_iovec instead of the in-memory copy.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_buf_item.c     |   29 +++++++-------
 fs/xfs/xfs_dquot_item.c   |   19 +++++-----
 fs/xfs/xfs_extfree_item.c |   10 +++--
 fs/xfs/xfs_icreate_item.c |    5 ++-
 fs/xfs/xfs_inode_item.c   |   92 ++++++++++++++++++++++-----------------------
 fs/xfs/xfs_log.h          |   34 +++++++++++++++--
 fs/xfs/xfs_log_cil.c      |   34 +----------------
 fs/xfs/xfs_trans.h        |    2 +-
 8 files changed, 111 insertions(+), 114 deletions(-)

diff --git a/fs/xfs/xfs_buf_item.c b/fs/xfs/xfs_buf_item.c
index d49419d..7641173 100644
--- a/fs/xfs/xfs_buf_item.c
+++ b/fs/xfs/xfs_buf_item.c
@@ -184,6 +184,7 @@ xfs_buf_item_size(
 
 static inline void
 xfs_buf_item_copy_iovec(
+	struct xfs_log_vec	*lv,
 	struct xfs_log_iovec	**vecp,
 	struct xfs_buf		*bp,
 	uint			offset,
@@ -191,7 +192,7 @@ xfs_buf_item_copy_iovec(
 	uint			nbits)
 {
 	offset += first_bit * XFS_BLF_CHUNK;
-	xlog_copy_iovec(vecp, XLOG_REG_TYPE_BCHUNK,
+	xlog_copy_iovec(lv, vecp, XLOG_REG_TYPE_BCHUNK,
 			xfs_buf_offset(bp, offset),
 			nbits * XFS_BLF_CHUNK);
 }
@@ -211,13 +212,13 @@ xfs_buf_item_straddle(
 static void
 xfs_buf_item_format_segment(
 	struct xfs_buf_log_item	*bip,
+	struct xfs_log_vec	*lv,
 	struct xfs_log_iovec	**vecp,
 	uint			offset,
 	struct xfs_buf_log_format *blfp)
 {
 	struct xfs_buf	*bp = bip->bli_buf;
 	uint		base_size;
-	uint		nvecs;
 	int		first_bit;
 	int		last_bit;
 	int		next_bit;
@@ -233,18 +234,17 @@ xfs_buf_item_format_segment(
 	 */
 	base_size = xfs_buf_log_format_size(blfp);
 
-	nvecs = 0;
 	first_bit = xfs_next_bit(blfp->blf_data_map, blfp->blf_map_size, 0);
 	if (!(bip->bli_flags & XFS_BLI_STALE) && first_bit == -1) {
 		/*
 		 * If the map is not be dirty in the transaction, mark
 		 * the size as zero and do not advance the vector pointer.
 		 */
-		goto out;
+		return;
 	}
 
-	xlog_copy_iovec(vecp, XLOG_REG_TYPE_BFORMAT, blfp, base_size);
-	nvecs = 1;
+	blfp = xlog_copy_iovec(lv, vecp, XLOG_REG_TYPE_BFORMAT, blfp, base_size);
+	blfp->blf_size = 1;
 
 	if (bip->bli_flags & XFS_BLI_STALE) {
 		/*
@@ -254,7 +254,7 @@ xfs_buf_item_format_segment(
 		 */
 		trace_xfs_buf_item_format_stale(bip);
 		ASSERT(blfp->blf_flags & XFS_BLF_CANCEL);
-		goto out;
+		return;
 	}
 
 
@@ -280,15 +280,15 @@ xfs_buf_item_format_segment(
 		 * same set of bits so just keep counting and scanning.
 		 */
 		if (next_bit == -1) {
-			xfs_buf_item_copy_iovec(vecp, bp, offset,
+			xfs_buf_item_copy_iovec(lv, vecp, bp, offset,
 						first_bit, nbits);
-			nvecs++;
+			blfp->blf_size++;
 			break;
 		} else if (next_bit != last_bit + 1 ||
 		           xfs_buf_item_straddle(bp, offset, next_bit, last_bit)) {
-			xfs_buf_item_copy_iovec(vecp, bp, offset,
+			xfs_buf_item_copy_iovec(lv, vecp, bp, offset,
 						first_bit, nbits);
-			nvecs++;
+			blfp->blf_size++;
 			first_bit = next_bit;
 			last_bit = next_bit;
 			nbits = 1;
@@ -297,8 +297,6 @@ xfs_buf_item_format_segment(
 			nbits++;
 		}
 	}
-out:
-	blfp->blf_size = nvecs;
 }
 
 /*
@@ -310,10 +308,11 @@ out:
 STATIC void
 xfs_buf_item_format(
 	struct xfs_log_item	*lip,
-	struct xfs_log_iovec	*vecp)
+	struct xfs_log_vec	*lv)
 {
 	struct xfs_buf_log_item	*bip = BUF_ITEM(lip);
 	struct xfs_buf		*bp = bip->bli_buf;
+	struct xfs_log_iovec	*vecp = NULL;
 	uint			offset = 0;
 	int			i;
 
@@ -354,7 +353,7 @@ xfs_buf_item_format(
 	}
 
 	for (i = 0; i < bip->bli_format_count; i++) {
-		xfs_buf_item_format_segment(bip, &vecp, offset,
+		xfs_buf_item_format_segment(bip, lv, &vecp, offset,
 					    &bip->bli_formats[i]);
 		offset += bp->b_maps[i].bm_len;
 	}
diff --git a/fs/xfs/xfs_dquot_item.c b/fs/xfs/xfs_dquot_item.c
index ca354a8..946d588 100644
--- a/fs/xfs/xfs_dquot_item.c
+++ b/fs/xfs/xfs_dquot_item.c
@@ -57,18 +57,19 @@ xfs_qm_dquot_logitem_size(
 STATIC void
 xfs_qm_dquot_logitem_format(
 	struct xfs_log_item	*lip,
-	struct xfs_log_iovec	*vecp)
+	struct xfs_log_vec	*lv)
 {
 	struct xfs_dq_logitem	*qlip = DQUOT_ITEM(lip);
+	struct xfs_log_iovec	*vecp = NULL;
 
-	xlog_copy_iovec(&vecp, XLOG_REG_TYPE_QFORMAT,
+	qlip->qli_format.qlf_size = 2;
+
+	xlog_copy_iovec(lv, &vecp, XLOG_REG_TYPE_QFORMAT,
 			&qlip->qli_format,
 			sizeof(struct xfs_dq_logformat));
-	xlog_copy_iovec(&vecp, XLOG_REG_TYPE_DQUOT,
+	xlog_copy_iovec(lv, &vecp, XLOG_REG_TYPE_DQUOT,
 			&qlip->qli_dquot->q_core,
 			sizeof(struct xfs_disk_dquot));
-
-	qlip->qli_format.qlf_size = 2;
 }
 
 /*
@@ -302,17 +303,17 @@ xfs_qm_qoff_logitem_size(
 STATIC void
 xfs_qm_qoff_logitem_format(
 	struct xfs_log_item	*lip,
-	struct xfs_log_iovec	*vecp)
+	struct xfs_log_vec	*lv)
 {
 	struct xfs_qoff_logitem	*qflip = QOFF_ITEM(lip);
+	struct xfs_log_iovec	*vecp = NULL;
 
 	ASSERT(qflip->qql_format.qf_type == XFS_LI_QUOTAOFF);
+	qflip->qql_format.qf_size = 1;
 
-	xlog_copy_iovec(&vecp, XLOG_REG_TYPE_QUOTAOFF,
+	xlog_copy_iovec(lv, &vecp, XLOG_REG_TYPE_QUOTAOFF,
 			&qflip->qql_format,
 			sizeof(struct xfs_qoff_logitem));
-
-	qflip->qql_format.qf_size = 1;
 }
 
 /*
diff --git a/fs/xfs/xfs_extfree_item.c b/fs/xfs/xfs_extfree_item.c
index 08823ec..fb7a4c1 100644
--- a/fs/xfs/xfs_extfree_item.c
+++ b/fs/xfs/xfs_extfree_item.c
@@ -102,9 +102,10 @@ xfs_efi_item_size(
 STATIC void
 xfs_efi_item_format(
 	struct xfs_log_item	*lip,
-	struct xfs_log_iovec	*vecp)
+	struct xfs_log_vec	*lv)
 {
 	struct xfs_efi_log_item	*efip = EFI_ITEM(lip);
+	struct xfs_log_iovec	*vecp = NULL;
 
 	ASSERT(atomic_read(&efip->efi_next_extent) ==
 				efip->efi_format.efi_nextents);
@@ -112,7 +113,7 @@ xfs_efi_item_format(
 	efip->efi_format.efi_type = XFS_LI_EFI;
 	efip->efi_format.efi_size = 1;
 
-	xlog_copy_iovec(&vecp, XLOG_REG_TYPE_EFI_FORMAT,
+	xlog_copy_iovec(lv, &vecp, XLOG_REG_TYPE_EFI_FORMAT,
 			&efip->efi_format,
 			xfs_efi_item_sizeof(efip));
 }
@@ -368,16 +369,17 @@ xfs_efd_item_size(
 STATIC void
 xfs_efd_item_format(
 	struct xfs_log_item	*lip,
-	struct xfs_log_iovec	*vecp)
+	struct xfs_log_vec	*lv)
 {
 	struct xfs_efd_log_item	*efdp = EFD_ITEM(lip);
+	struct xfs_log_iovec	*vecp = NULL;
 
 	ASSERT(efdp->efd_next_extent == efdp->efd_format.efd_nextents);
 
 	efdp->efd_format.efd_type = XFS_LI_EFD;
 	efdp->efd_format.efd_size = 1;
 
-	xlog_copy_iovec(&vecp, XLOG_REG_TYPE_EFD_FORMAT,
+	xlog_copy_iovec(lv, &vecp, XLOG_REG_TYPE_EFD_FORMAT,
 			&efdp->efd_format,
 			xfs_efd_item_sizeof(efdp));
 }
diff --git a/fs/xfs/xfs_icreate_item.c b/fs/xfs/xfs_icreate_item.c
index 5751fa8..7e45492 100644
--- a/fs/xfs/xfs_icreate_item.c
+++ b/fs/xfs/xfs_icreate_item.c
@@ -59,11 +59,12 @@ xfs_icreate_item_size(
 STATIC void
 xfs_icreate_item_format(
 	struct xfs_log_item	*lip,
-	struct xfs_log_iovec	*vecp)
+	struct xfs_log_vec	*lv)
 {
 	struct xfs_icreate_item	*icp = ICR_ITEM(lip);
+	struct xfs_log_iovec	*vecp = NULL;
 
-	xlog_copy_iovec(&vecp, XLOG_REG_TYPE_ICREATE,
+	xlog_copy_iovec(lv, &vecp, XLOG_REG_TYPE_ICREATE,
 			&icp->ic_format,
 			sizeof(struct xfs_icreate_log));
 }
diff --git a/fs/xfs/xfs_inode_item.c b/fs/xfs/xfs_inode_item.c
index 73002db..35dd24a 100644
--- a/fs/xfs/xfs_inode_item.c
+++ b/fs/xfs/xfs_inode_item.c
@@ -163,6 +163,7 @@ xfs_inode_item_size(
 STATIC int
 xfs_inode_item_format_extents(
 	struct xfs_inode	*ip,
+	struct xfs_log_vec	*lv,
 	struct xfs_log_iovec	**vecp,
 	int			whichfork,
 	int			type)
@@ -177,7 +178,7 @@ xfs_inode_item_format_extents(
 		ip->i_itemp->ili_aextents_buf = ext_buffer;
 
 	len = xfs_iextents_copy(ip, ext_buffer, whichfork);
-	xlog_copy_iovec(vecp, type, ext_buffer, len);
+	xlog_copy_iovec(lv, vecp, type, ext_buffer, len);
 	return len;
 }
 
@@ -212,8 +213,9 @@ xfs_inode_item_format_v1_inode(
 STATIC void
 xfs_inode_item_format_data_fork(
 	struct xfs_inode_log_item *iip,
-	struct xfs_log_iovec	**vecp,
-	int			*nvecs)
+	struct xfs_inode_log_format *ilf,
+	struct xfs_log_vec	*lv,
+	struct xfs_log_iovec	**vecp)
 {
 	struct xfs_inode	*ip = iip->ili_inode;
 	size_t			data_bytes;
@@ -239,19 +241,19 @@ xfs_inode_item_format_data_fork(
 				 * extents, so just point to the
 				 * real extents array.
 				 */
-				xlog_copy_iovec(vecp, XLOG_REG_TYPE_IEXT,
+				xlog_copy_iovec(lv, vecp, XLOG_REG_TYPE_IEXT,
 						ip->i_df.if_u1.if_extents,
 						ip->i_df.if_bytes);
-				iip->ili_format.ilf_dsize = ip->i_df.if_bytes;
+				ilf->ilf_dsize = ip->i_df.if_bytes;
 			} else
 #endif
 			{
-				iip->ili_format.ilf_dsize =
-					xfs_inode_item_format_extents(ip, vecp,
+				ilf->ilf_dsize =
+					xfs_inode_item_format_extents(ip, lv, vecp,
 						XFS_DATA_FORK, XLOG_REG_TYPE_IEXT);
 				ASSERT(iip->ili_format.ilf_dsize <= ip->i_df.if_bytes);
 			}
-			(*nvecs)++;
+			ilf->ilf_size++;
 		} else {
 			iip->ili_fields &= ~XFS_ILOG_DEXT;
 		}
@@ -264,11 +266,11 @@ xfs_inode_item_format_data_fork(
 		if ((iip->ili_fields & XFS_ILOG_DBROOT) &&
 		    ip->i_df.if_broot_bytes > 0) {
 			ASSERT(ip->i_df.if_broot != NULL);
-			xlog_copy_iovec(vecp, XLOG_REG_TYPE_IBROOT,
+			xlog_copy_iovec(lv, vecp, XLOG_REG_TYPE_IBROOT,
 					ip->i_df.if_broot,
 					ip->i_df.if_broot_bytes);
-			(*nvecs)++;
-			iip->ili_format.ilf_dsize = ip->i_df.if_broot_bytes;
+			ilf->ilf_dsize = ip->i_df.if_broot_bytes;
+			ilf->ilf_size++;
 		} else {
 			ASSERT(!(iip->ili_fields &
 				 XFS_ILOG_DBROOT));
@@ -291,10 +293,10 @@ xfs_inode_item_format_data_fork(
 			       ip->i_df.if_real_bytes == data_bytes);
 			ASSERT(ip->i_df.if_u1.if_data != NULL);
 			ASSERT(ip->i_d.di_size > 0);
-			xlog_copy_iovec(vecp, XLOG_REG_TYPE_ILOCAL,
+			xlog_copy_iovec(lv, vecp, XLOG_REG_TYPE_ILOCAL,
 					ip->i_df.if_u1.if_data, data_bytes);
-			(*nvecs)++;
-			iip->ili_format.ilf_dsize = (unsigned)data_bytes;
+			ilf->ilf_dsize = (unsigned)data_bytes;
+			ilf->ilf_size++;
 		} else {
 			iip->ili_fields &= ~XFS_ILOG_DDATA;
 		}
@@ -303,19 +305,15 @@ xfs_inode_item_format_data_fork(
 		iip->ili_fields &=
 			~(XFS_ILOG_DDATA | XFS_ILOG_DBROOT |
 			  XFS_ILOG_DEXT | XFS_ILOG_UUID);
-		if (iip->ili_fields & XFS_ILOG_DEV) {
-			iip->ili_format.ilf_u.ilfu_rdev =
-				ip->i_df.if_u2.if_rdev;
-		}
+		if (iip->ili_fields & XFS_ILOG_DEV)
+			ilf->ilf_u.ilfu_rdev = ip->i_df.if_u2.if_rdev;
 		break;
 	case XFS_DINODE_FMT_UUID:
 		iip->ili_fields &=
 			~(XFS_ILOG_DDATA | XFS_ILOG_DBROOT |
 			  XFS_ILOG_DEXT | XFS_ILOG_DEV);
-		if (iip->ili_fields & XFS_ILOG_UUID) {
-			iip->ili_format.ilf_u.ilfu_uuid =
-				ip->i_df.if_u2.if_uuid;
-		}
+		if (iip->ili_fields & XFS_ILOG_UUID)
+			ilf->ilf_u.ilfu_uuid = ip->i_df.if_u2.if_uuid;
 		break;
 	default:
 		ASSERT(0);
@@ -326,8 +324,9 @@ xfs_inode_item_format_data_fork(
 STATIC void
 xfs_inode_item_format_attr_fork(
 	struct xfs_inode_log_item *iip,
-	struct xfs_log_iovec	**vecp,
-	int			*nvecs)
+	struct xfs_inode_log_format *ilf,
+	struct xfs_log_vec	*lv,
+	struct xfs_log_iovec	**vecp)
 {
 	struct xfs_inode	*ip = iip->ili_inode;
 	size_t			data_bytes;
@@ -348,17 +347,17 @@ xfs_inode_item_format_attr_fork(
 			 * There are not delayed allocation extents
 			 * for attributes, so just point at the array.
 			 */
-			xlog_copy_iovec(vecp, XLOG_REG_TYPE_IATTR_EXT,
+			xlog_copy_iovec(lv, vecp, XLOG_REG_TYPE_IATTR_EXT,
 					ip->i_afp->if_u1.if_extents,
 					ip->i_afp->if_bytes);
-			iip->ili_format.ilf_asize = ip->i_afp->if_bytes;
+			ilf->ilf_asize = ip->i_afp->if_bytes;
 #else
 			ASSERT(iip->ili_aextents_buf == NULL);
-			iip->ili_format.ilf_asize =
-				xfs_inode_item_format_extents(ip, vecp,
+			ilf->ilf_asize =
+				xfs_inode_item_format_extents(ip, lv, vecp,
 					XFS_ATTR_FORK, XLOG_REG_TYPE_IATTR_EXT);
 #endif
-			(*nvecs)++;
+			ilf->ilf_size++;
 		} else {
 			iip->ili_fields &= ~XFS_ILOG_AEXT;
 		}
@@ -371,11 +370,11 @@ xfs_inode_item_format_attr_fork(
 		    ip->i_afp->if_broot_bytes > 0) {
 			ASSERT(ip->i_afp->if_broot != NULL);
 
-			xlog_copy_iovec(vecp, XLOG_REG_TYPE_IATTR_BROOT,
+			xlog_copy_iovec(lv, vecp, XLOG_REG_TYPE_IATTR_BROOT,
 					ip->i_afp->if_broot,
 					ip->i_afp->if_broot_bytes);
-			(*nvecs)++;
-			iip->ili_format.ilf_asize = ip->i_afp->if_broot_bytes;
+			ilf->ilf_asize = ip->i_afp->if_broot_bytes;
+			ilf->ilf_size++;
 		} else {
 			iip->ili_fields &= ~XFS_ILOG_ABROOT;
 		}
@@ -395,11 +394,11 @@ xfs_inode_item_format_attr_fork(
 			ASSERT(ip->i_afp->if_real_bytes == 0 ||
 			       ip->i_afp->if_real_bytes == data_bytes);
 			ASSERT(ip->i_afp->if_u1.if_data != NULL);
-			xlog_copy_iovec(vecp, XLOG_REG_TYPE_IATTR_LOCAL,
+			xlog_copy_iovec(lv, vecp, XLOG_REG_TYPE_IATTR_LOCAL,
 					ip->i_afp->if_u1.if_data,
 					data_bytes);
-			(*nvecs)++;
-			iip->ili_format.ilf_asize = (unsigned)data_bytes;
+			ilf->ilf_asize = (unsigned)data_bytes;
+			ilf->ilf_size++;
 		} else {
 			iip->ili_fields &= ~XFS_ILOG_ADATA;
 		}
@@ -420,28 +419,28 @@ xfs_inode_item_format_attr_fork(
 STATIC void
 xfs_inode_item_format(
 	struct xfs_log_item	*lip,
-	struct xfs_log_iovec	*vecp)
+	struct xfs_log_vec	*lv)
 {
 	struct xfs_inode_log_item *iip = INODE_ITEM(lip);
 	struct xfs_inode	*ip = iip->ili_inode;
-	uint			nvecs;
+	struct xfs_inode_log_format *ilf;
+	struct xfs_log_iovec	*vecp = NULL;
 
-	xlog_copy_iovec(&vecp, XLOG_REG_TYPE_IFORMAT,
+	ilf = xlog_copy_iovec(lv, &vecp, XLOG_REG_TYPE_IFORMAT,
 			&iip->ili_format,
 			sizeof(struct xfs_inode_log_format));
-	nvecs = 1;
-
-	xlog_copy_iovec(&vecp, XLOG_REG_TYPE_ICORE,
-			&ip->i_d,
-			xfs_icdinode_size(ip->i_d.di_version));
-	nvecs++;
+	ilf->ilf_size = 1;
 
 	if (ip->i_d.di_version == 1)
 		xfs_inode_item_format_v1_inode(ip);
+	xlog_copy_iovec(lv, &vecp, XLOG_REG_TYPE_ICORE,
+			&ip->i_d,
+			xfs_icdinode_size(ip->i_d.di_version));
+	ilf->ilf_size++;
 
-	xfs_inode_item_format_data_fork(iip, &vecp, &nvecs);
+	xfs_inode_item_format_data_fork(iip, ilf, lv, &vecp);
 	if (XFS_IFORK_Q(ip)) {
-		xfs_inode_item_format_attr_fork(iip, &vecp, &nvecs);
+		xfs_inode_item_format_attr_fork(iip, ilf, lv, &vecp);
 	} else {
 		iip->ili_fields &=
 			~(XFS_ILOG_ADATA | XFS_ILOG_ABROOT | XFS_ILOG_AEXT);
@@ -455,7 +454,6 @@ xfs_inode_item_format(
 	 */
 	iip->ili_format.ilf_fields = XFS_ILOG_CORE |
 		(iip->ili_fields & ~XFS_ILOG_TIMESTAMP);
-	iip->ili_format.ilf_size = nvecs;
 }
 
 /*
diff --git a/fs/xfs/xfs_log.h b/fs/xfs/xfs_log.h
index 384c6c4..e04bd0c 100644
--- a/fs/xfs/xfs_log.h
+++ b/fs/xfs/xfs_log.h
@@ -31,18 +31,44 @@ struct xfs_log_vec {
 #define XFS_LOG_VEC_ORDERED	(-1)
 
 static inline void *
-xlog_copy_iovec(struct xfs_log_iovec **vecp, uint type, void *data, int len)
+xlog_prepare_iovec(struct xfs_log_vec *lv, struct xfs_log_iovec **vecp,
+		uint type)
 {
 	struct xfs_log_iovec *vec = *vecp;
 
+	if (vec) {
+		ASSERT(vec - lv->lv_iovecp < lv->lv_niovecs);
+		vec++;
+	} else {
+		vec = &lv->lv_iovecp[0];
+	}
+
 	vec->i_type = type;
-	vec->i_addr = data;
-	vec->i_len = len;
+	vec->i_addr = lv->lv_buf + lv->lv_buf_len;
 
-	*vecp = vec + 1;
+	*vecp = vec;
 	return vec->i_addr;
 }
 
+static inline void
+xlog_finish_iovec(struct xfs_log_vec *lv, struct xfs_log_iovec *vec, int len)
+{
+	lv->lv_buf_len += len;
+	vec->i_len = len;
+}
+
+static inline void *
+xlog_copy_iovec(struct xfs_log_vec *lv, struct xfs_log_iovec **vecp,
+		uint type, void *data, int len)
+{
+	void *buf;
+
+	buf = xlog_prepare_iovec(lv, vecp, type);
+	memcpy(buf, data, len);
+	xlog_finish_iovec(lv, *vecp, len);
+	return buf;
+}
+
 /*
  * Structure used to pass callback function and the function's argument
  * to the log manager.
diff --git a/fs/xfs/xfs_log_cil.c b/fs/xfs/xfs_log_cil.c
index 0a7a8ce..28192d6 100644
--- a/fs/xfs/xfs_log_cil.c
+++ b/fs/xfs/xfs_log_cil.c
@@ -82,36 +82,6 @@ xlog_cil_init_post_recovery(
 								log->l_curr_block);
 }
 
-STATIC int
-xlog_cil_lv_item_format(
-	struct xfs_log_item	*lip,
-	struct xfs_log_vec	*lv)
-{
-	int	index;
-	char	*ptr;
-
-	/* format new vectors into array */
-	lip->li_ops->iop_format(lip, lv->lv_iovecp);
-
-	/* copy data into existing array */
-	ptr = lv->lv_buf;
-	for (index = 0; index < lv->lv_niovecs; index++) {
-		struct xfs_log_iovec *vec = &lv->lv_iovecp[index];
-
-		memcpy(ptr, vec->i_addr, vec->i_len);
-		vec->i_addr = ptr;
-		ptr += vec->i_len;
-	}
-
-	/*
-	 * some size calculations for log vectors over-estimate, so the caller
-	 * doesn't know the amount of space actually used by the item. Return
-	 * the byte count to the caller so they can check and store it
-	 * appropriately.
-	 */
-	return ptr - lv->lv_buf;
-}
-
 /*
  * Prepare the log item for insertion into the CIL. Calculate the difference in
  * log space and vectors it will consume, and if it is a new item pin it as
@@ -272,9 +242,9 @@ xlog_cil_insert_format_items(
 		lv->lv_niovecs = niovecs;
 
 		/* The allocated data region lies beyond the iovec region */
+		lv->lv_buf_len = 0;
 		lv->lv_buf = (char *)lv + buf_size - nbytes;
-
-		lv->lv_buf_len = xlog_cil_lv_item_format(lip, lv);
+		lip->li_ops->iop_format(lip, lv);
 insert:
 		ASSERT(lv->lv_buf_len <= nbytes);
 		xfs_cil_prepare_item(log, lv, old_lv, diff_len, diff_iovecs);
diff --git a/fs/xfs/xfs_trans.h b/fs/xfs/xfs_trans.h
index 9b96d35..b5bc1ab 100644
--- a/fs/xfs/xfs_trans.h
+++ b/fs/xfs/xfs_trans.h
@@ -64,7 +64,7 @@ typedef struct xfs_log_item {
 
 struct xfs_item_ops {
 	void (*iop_size)(xfs_log_item_t *, int *, int *);
-	void (*iop_format)(xfs_log_item_t *, struct xfs_log_iovec *);
+	void (*iop_format)(xfs_log_item_t *, struct xfs_log_vec *);
 	void (*iop_pin)(xfs_log_item_t *);
 	void (*iop_unpin)(xfs_log_item_t *, int remove);
 	uint (*iop_push)(struct xfs_log_item *, struct list_head *);
-- 
1.7.10.4


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

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 07/10] xfs: format logged extents directly into the CIL
  2013-11-29  8:39 [PATCH 00/10] decouple the in-memory from the on-disk log format V2 Christoph Hellwig
                   ` (5 preceding siblings ...)
  2013-11-29  8:39 ` [PATCH 06/10] xfs: format log items write directly into the linear CIL buffer Christoph Hellwig
@ 2013-11-29  8:39 ` Christoph Hellwig
  2013-12-04  0:40   ` Dave Chinner
  2013-11-29  8:39 ` [PATCH 08/10] xfs: remove the inode log format from the inode log item Christoph Hellwig
                   ` (2 subsequent siblings)
  9 siblings, 1 reply; 29+ messages in thread
From: Christoph Hellwig @ 2013-11-29  8:39 UTC (permalink / raw)
  To: xfs

[-- Attachment #1: 0007-xfs-format-logged-extents-directly-into-the-CIL.patch --]
[-- Type: text/plain, Size: 7555 bytes --]

With the new iop_format scheme there is no need to have a temporary buffer
to format logged extents into, we can do so directly into the CIL.  This
also allows to remove the shortcut for big endian systems that probably
hasn't gotten a lot of test coverage for a long time.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_inode_fork.c |   15 ++++---
 fs/xfs/xfs_inode_item.c |  113 ++++++++---------------------------------------
 fs/xfs/xfs_inode_item.h |    4 --
 3 files changed, 26 insertions(+), 106 deletions(-)

diff --git a/fs/xfs/xfs_inode_fork.c b/fs/xfs/xfs_inode_fork.c
index cfee14a..06abaee 100644
--- a/fs/xfs/xfs_inode_fork.c
+++ b/fs/xfs/xfs_inode_fork.c
@@ -721,15 +721,16 @@ xfs_idestroy_fork(
 }
 
 /*
- * xfs_iextents_copy()
+ * Convert in-core extents to on-disk form
  *
- * This is called to copy the REAL extents (as opposed to the delayed
- * allocation extents) from the inode into the given buffer.  It
- * returns the number of bytes copied into the buffer.
+ * For either the data or attr fork in extent format, we need to endian convert
+ * the in-core extent as we place them into the on-disk inode.
  *
- * If there are no delayed allocation extents, then we can just
- * memcpy() the extents into the buffer.  Otherwise, we need to
- * examine each extent in turn and skip those which are delayed.
+ * In the case of the data fork, the in-core and on-disk fork sizes can be
+ * different due to delayed allocation extents. We only copy on-disk extents
+ * here, so callers must always use the physical fork size to determine the
+ * size of the buffer passed to this routine.  We will return the size actually
+ * used.
  */
 int
 xfs_iextents_copy(
diff --git a/fs/xfs/xfs_inode_item.c b/fs/xfs/xfs_inode_item.c
index 35dd24a..f103dfd 100644
--- a/fs/xfs/xfs_inode_item.c
+++ b/fs/xfs/xfs_inode_item.c
@@ -146,43 +146,6 @@ xfs_inode_item_size(
 }
 
 /*
- * xfs_inode_item_format_extents - convert in-core extents to on-disk form
- *
- * For either the data or attr fork in extent format, we need to endian convert
- * the in-core extent as we place them into the on-disk inode. In this case, we
- * need to do this conversion before we write the extents into the log. Because
- * we don't have the disk inode to write into here, we allocate a buffer and
- * format the extents into it via xfs_iextents_copy(). We free the buffer in
- * the unlock routine after the copy for the log has been made.
- *
- * In the case of the data fork, the in-core and on-disk fork sizes can be
- * different due to delayed allocation extents. We only log on-disk extents
- * here, so always use the physical fork size to determine the size of the
- * buffer we need to allocate.
- */
-STATIC int
-xfs_inode_item_format_extents(
-	struct xfs_inode	*ip,
-	struct xfs_log_vec	*lv,
-	struct xfs_log_iovec	**vecp,
-	int			whichfork,
-	int			type)
-{
-	xfs_bmbt_rec_t		*ext_buffer;
-	int			len;
-
-	ext_buffer = kmem_alloc(XFS_IFORK_SIZE(ip, whichfork), KM_SLEEP);
-	if (whichfork == XFS_DATA_FORK)
-		ip->i_itemp->ili_extents_buf = ext_buffer;
-	else
-		ip->i_itemp->ili_aextents_buf = ext_buffer;
-
-	len = xfs_iextents_copy(ip, ext_buffer, whichfork);
-	xlog_copy_iovec(lv, vecp, type, ext_buffer, len);
-	return len;
-}
-
-/*
  * If this is a v1 format inode, then we need to log it as such.  This means
  * that we have to copy the link count from the new field to the old.  We
  * don't have to worry about the new fields, because nothing trusts them as
@@ -229,30 +192,18 @@ xfs_inode_item_format_data_fork(
 		if ((iip->ili_fields & XFS_ILOG_DEXT) &&
 		    ip->i_d.di_nextents > 0 &&
 		    ip->i_df.if_bytes > 0) {
+			struct xfs_bmbt_rec *p;
+
 			ASSERT(ip->i_df.if_u1.if_extents != NULL);
 			ASSERT(ip->i_df.if_bytes / sizeof(xfs_bmbt_rec_t) > 0);
-			ASSERT(iip->ili_extents_buf == NULL);
-
-#ifdef XFS_NATIVE_HOST
-                       if (ip->i_d.di_nextents == ip->i_df.if_bytes /
-                                               (uint)sizeof(xfs_bmbt_rec_t)) {
-				/*
-				 * There are no delayed allocation
-				 * extents, so just point to the
-				 * real extents array.
-				 */
-				xlog_copy_iovec(lv, vecp, XLOG_REG_TYPE_IEXT,
-						ip->i_df.if_u1.if_extents,
-						ip->i_df.if_bytes);
-				ilf->ilf_dsize = ip->i_df.if_bytes;
-			} else
-#endif
-			{
-				ilf->ilf_dsize =
-					xfs_inode_item_format_extents(ip, lv, vecp,
-						XFS_DATA_FORK, XLOG_REG_TYPE_IEXT);
-				ASSERT(iip->ili_format.ilf_dsize <= ip->i_df.if_bytes);
-			}
+
+			p = xlog_prepare_iovec(lv, vecp, XLOG_REG_TYPE_IEXT);
+			data_bytes = xfs_iextents_copy(ip, p, XFS_DATA_FORK);
+			xlog_finish_iovec(lv, *vecp, data_bytes);
+
+			ASSERT(data_bytes <= ip->i_df.if_bytes);
+
+			ilf->ilf_dsize = data_bytes;
 			ilf->ilf_size++;
 		} else {
 			iip->ili_fields &= ~XFS_ILOG_DEXT;
@@ -339,24 +290,17 @@ xfs_inode_item_format_attr_fork(
 		if ((iip->ili_fields & XFS_ILOG_AEXT) &&
 		    ip->i_d.di_anextents > 0 &&
 		    ip->i_afp->if_bytes > 0) {
+			struct xfs_bmbt_rec *p;
+
 			ASSERT(ip->i_afp->if_bytes / sizeof(xfs_bmbt_rec_t) ==
 				ip->i_d.di_anextents);
 			ASSERT(ip->i_afp->if_u1.if_extents != NULL);
-#ifdef XFS_NATIVE_HOST
-			/*
-			 * There are not delayed allocation extents
-			 * for attributes, so just point at the array.
-			 */
-			xlog_copy_iovec(lv, vecp, XLOG_REG_TYPE_IATTR_EXT,
-					ip->i_afp->if_u1.if_extents,
-					ip->i_afp->if_bytes);
-			ilf->ilf_asize = ip->i_afp->if_bytes;
-#else
-			ASSERT(iip->ili_aextents_buf == NULL);
-			ilf->ilf_asize =
-				xfs_inode_item_format_extents(ip, lv, vecp,
-					XFS_ATTR_FORK, XLOG_REG_TYPE_IATTR_EXT);
-#endif
+
+			p = xlog_prepare_iovec(lv, vecp, XLOG_REG_TYPE_IATTR_EXT);
+			data_bytes = xfs_iextents_copy(ip, p, XFS_ATTR_FORK);
+			xlog_finish_iovec(lv, *vecp, data_bytes);
+
+			ilf->ilf_asize = data_bytes;
 			ilf->ilf_size++;
 		} else {
 			iip->ili_fields &= ~XFS_ILOG_AEXT;
@@ -571,27 +515,6 @@ xfs_inode_item_unlock(
 	ASSERT(ip->i_itemp != NULL);
 	ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
 
-	/*
-	 * If the inode needed a separate buffer with which to log
-	 * its extents, then free it now.
-	 */
-	if (iip->ili_extents_buf != NULL) {
-		ASSERT(ip->i_d.di_format == XFS_DINODE_FMT_EXTENTS);
-		ASSERT(ip->i_d.di_nextents > 0);
-		ASSERT(iip->ili_fields & XFS_ILOG_DEXT);
-		ASSERT(ip->i_df.if_bytes > 0);
-		kmem_free(iip->ili_extents_buf);
-		iip->ili_extents_buf = NULL;
-	}
-	if (iip->ili_aextents_buf != NULL) {
-		ASSERT(ip->i_d.di_aformat == XFS_DINODE_FMT_EXTENTS);
-		ASSERT(ip->i_d.di_anextents > 0);
-		ASSERT(iip->ili_fields & XFS_ILOG_AEXT);
-		ASSERT(ip->i_afp->if_bytes > 0);
-		kmem_free(iip->ili_aextents_buf);
-		iip->ili_aextents_buf = NULL;
-	}
-
 	lock_flags = iip->ili_lock_flags;
 	iip->ili_lock_flags = 0;
 	if (lock_flags)
diff --git a/fs/xfs/xfs_inode_item.h b/fs/xfs/xfs_inode_item.h
index dce4d65..29b5f2b 100644
--- a/fs/xfs/xfs_inode_item.h
+++ b/fs/xfs/xfs_inode_item.h
@@ -34,10 +34,6 @@ typedef struct xfs_inode_log_item {
 	unsigned short		ili_logged;	   /* flushed logged data */
 	unsigned int		ili_last_fields;   /* fields when flushed */
 	unsigned int		ili_fields;	   /* fields to be logged */
-	struct xfs_bmbt_rec	*ili_extents_buf;  /* array of logged
-						      data exts */
-	struct xfs_bmbt_rec	*ili_aextents_buf; /* array of logged
-						      attr exts */
 	xfs_inode_log_format_t	ili_format;	   /* logged structure */
 } xfs_inode_log_item_t;
 
-- 
1.7.10.4


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

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 08/10] xfs: remove the inode log format from the inode log item
  2013-11-29  8:39 [PATCH 00/10] decouple the in-memory from the on-disk log format V2 Christoph Hellwig
                   ` (6 preceding siblings ...)
  2013-11-29  8:39 ` [PATCH 07/10] xfs: format logged extents directly into the CIL Christoph Hellwig
@ 2013-11-29  8:39 ` Christoph Hellwig
  2013-12-04  0:44   ` Dave Chinner
  2013-11-29  8:39 ` [PATCH 09/10] xfs: remove the dquot log format from the dquot " Christoph Hellwig
  2013-11-29  8:39 ` [PATCH 10/10] xfs: remove the quotaoff log format from the quotaoff " Christoph Hellwig
  9 siblings, 1 reply; 29+ messages in thread
From: Christoph Hellwig @ 2013-11-29  8:39 UTC (permalink / raw)
  To: xfs

[-- Attachment #1: 0008-xfs-remove-the-inode-log-format-from-the-inode-log-i.patch --]
[-- Type: text/plain, Size: 3039 bytes --]

No need to keep the inode log format around all the time, we can easily
generate it at iop_format time.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_inode_item.c |   29 +++++++++++------------------
 fs/xfs/xfs_inode_item.h |    1 -
 2 files changed, 11 insertions(+), 19 deletions(-)

diff --git a/fs/xfs/xfs_inode_item.c b/fs/xfs/xfs_inode_item.c
index f103dfd..346d592 100644
--- a/fs/xfs/xfs_inode_item.c
+++ b/fs/xfs/xfs_inode_item.c
@@ -370,17 +370,21 @@ xfs_inode_item_format(
 	struct xfs_inode_log_format *ilf;
 	struct xfs_log_iovec	*vecp = NULL;
 
-	ilf = xlog_copy_iovec(lv, &vecp, XLOG_REG_TYPE_IFORMAT,
-			&iip->ili_format,
-			sizeof(struct xfs_inode_log_format));
-	ilf->ilf_size = 1;
+	ilf = xlog_prepare_iovec(lv, &vecp, XLOG_REG_TYPE_IFORMAT);
+	ilf->ilf_type = XFS_LI_INODE;
+	ilf->ilf_ino = ip->i_ino;
+	ilf->ilf_blkno = ip->i_imap.im_blkno;
+	ilf->ilf_len = ip->i_imap.im_len;
+	ilf->ilf_boffset = ip->i_imap.im_boffset;
+	ilf->ilf_fields = XFS_ILOG_CORE;
+	ilf->ilf_size = 2; /* format + core */
+	xlog_finish_iovec(lv, vecp, sizeof(struct xfs_inode_log_format));
 
 	if (ip->i_d.di_version == 1)
 		xfs_inode_item_format_v1_inode(ip);
 	xlog_copy_iovec(lv, &vecp, XLOG_REG_TYPE_ICORE,
 			&ip->i_d,
 			xfs_icdinode_size(ip->i_d.di_version));
-	ilf->ilf_size++;
 
 	xfs_inode_item_format_data_fork(iip, ilf, lv, &vecp);
 	if (XFS_IFORK_Q(ip)) {
@@ -390,14 +394,8 @@ xfs_inode_item_format(
 			~(XFS_ILOG_ADATA | XFS_ILOG_ABROOT | XFS_ILOG_AEXT);
 	}
 
-	/*
-	 * Now update the log format that goes out to disk from the in-core
-	 * values.  We always write the inode core to make the arithmetic
-	 * games in recovery easier, which isn't a big deal as just about any
-	 * transaction would dirty it anyway.
-	 */
-	iip->ili_format.ilf_fields = XFS_ILOG_CORE |
-		(iip->ili_fields & ~XFS_ILOG_TIMESTAMP);
+	/* update the format with the exact fields we actually logged */
+	ilf->ilf_fields |= (iip->ili_fields & ~XFS_ILOG_TIMESTAMP);
 }
 
 /*
@@ -601,11 +599,6 @@ xfs_inode_item_init(
 	iip->ili_inode = ip;
 	xfs_log_item_init(mp, &iip->ili_item, XFS_LI_INODE,
 						&xfs_inode_item_ops);
-	iip->ili_format.ilf_type = XFS_LI_INODE;
-	iip->ili_format.ilf_ino = ip->i_ino;
-	iip->ili_format.ilf_blkno = ip->i_imap.im_blkno;
-	iip->ili_format.ilf_len = ip->i_imap.im_len;
-	iip->ili_format.ilf_boffset = ip->i_imap.im_boffset;
 }
 
 /*
diff --git a/fs/xfs/xfs_inode_item.h b/fs/xfs/xfs_inode_item.h
index 29b5f2b..488d812 100644
--- a/fs/xfs/xfs_inode_item.h
+++ b/fs/xfs/xfs_inode_item.h
@@ -34,7 +34,6 @@ typedef struct xfs_inode_log_item {
 	unsigned short		ili_logged;	   /* flushed logged data */
 	unsigned int		ili_last_fields;   /* fields when flushed */
 	unsigned int		ili_fields;	   /* fields to be logged */
-	xfs_inode_log_format_t	ili_format;	   /* logged structure */
 } xfs_inode_log_item_t;
 
 static inline int xfs_inode_clean(xfs_inode_t *ip)
-- 
1.7.10.4


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

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 09/10] xfs: remove the dquot log format from the dquot log item
  2013-11-29  8:39 [PATCH 00/10] decouple the in-memory from the on-disk log format V2 Christoph Hellwig
                   ` (7 preceding siblings ...)
  2013-11-29  8:39 ` [PATCH 08/10] xfs: remove the inode log format from the inode log item Christoph Hellwig
@ 2013-11-29  8:39 ` Christoph Hellwig
  2013-12-04  0:47   ` Dave Chinner
  2013-11-29  8:39 ` [PATCH 10/10] xfs: remove the quotaoff log format from the quotaoff " Christoph Hellwig
  9 siblings, 1 reply; 29+ messages in thread
From: Christoph Hellwig @ 2013-11-29  8:39 UTC (permalink / raw)
  To: xfs

[-- Attachment #1: 0009-xfs-remove-the-dquot-log-format-from-the-dquot-log-i.patch --]
[-- Type: text/plain, Size: 2623 bytes --]

No need to keep the dquot log format around all the time, we can easily
generate it at iop_format time.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_dquot_item.c |   25 +++++++++----------------
 fs/xfs/xfs_dquot_item.h |    1 -
 2 files changed, 9 insertions(+), 17 deletions(-)

diff --git a/fs/xfs/xfs_dquot_item.c b/fs/xfs/xfs_dquot_item.c
index 946d588..d4fffa9 100644
--- a/fs/xfs/xfs_dquot_item.c
+++ b/fs/xfs/xfs_dquot_item.c
@@ -61,12 +61,17 @@ xfs_qm_dquot_logitem_format(
 {
 	struct xfs_dq_logitem	*qlip = DQUOT_ITEM(lip);
 	struct xfs_log_iovec	*vecp = NULL;
+	struct xfs_dq_logformat	*qlf;
 
-	qlip->qli_format.qlf_size = 2;
+	qlf = xlog_prepare_iovec(lv, &vecp, XLOG_REG_TYPE_QFORMAT);
+	qlf->qlf_type = XFS_LI_DQUOT;
+	qlf->qlf_size = 2;
+	qlf->qlf_id = be32_to_cpu(qlip->qli_dquot->q_core.d_id);
+	qlf->qlf_blkno = qlip->qli_dquot->q_blkno;
+	qlf->qlf_len = 1;
+	qlf->qlf_boffset = qlip->qli_dquot->q_bufoffset;
+	xlog_finish_iovec(lv, vecp, sizeof(struct xfs_dq_logformat));
 
-	xlog_copy_iovec(lv, &vecp, XLOG_REG_TYPE_QFORMAT,
-			&qlip->qli_format,
-			sizeof(struct xfs_dq_logformat));
 	xlog_copy_iovec(lv, &vecp, XLOG_REG_TYPE_DQUOT,
 			&qlip->qli_dquot->q_core,
 			sizeof(struct xfs_disk_dquot));
@@ -256,18 +261,6 @@ xfs_qm_dquot_logitem_init(
 	xfs_log_item_init(dqp->q_mount, &lp->qli_item, XFS_LI_DQUOT,
 					&xfs_dquot_item_ops);
 	lp->qli_dquot = dqp;
-	lp->qli_format.qlf_type = XFS_LI_DQUOT;
-	lp->qli_format.qlf_id = be32_to_cpu(dqp->q_core.d_id);
-	lp->qli_format.qlf_blkno = dqp->q_blkno;
-	lp->qli_format.qlf_len = 1;
-	/*
-	 * This is just the offset of this dquot within its buffer
-	 * (which is currently 1 FSB and probably won't change).
-	 * Hence 32 bits for this offset should be just fine.
-	 * Alternatively, we can store (bufoffset / sizeof(xfs_dqblk_t))
-	 * here, and recompute it at recovery time.
-	 */
-	lp->qli_format.qlf_boffset = (__uint32_t)dqp->q_bufoffset;
 }
 
 /*------------------  QUOTAOFF LOG ITEMS  -------------------*/
diff --git a/fs/xfs/xfs_dquot_item.h b/fs/xfs/xfs_dquot_item.h
index 5acae2a..925cbe9 100644
--- a/fs/xfs/xfs_dquot_item.h
+++ b/fs/xfs/xfs_dquot_item.h
@@ -27,7 +27,6 @@ typedef struct xfs_dq_logitem {
 	xfs_log_item_t		 qli_item;	   /* common portion */
 	struct xfs_dquot	*qli_dquot;	   /* dquot ptr */
 	xfs_lsn_t		 qli_flush_lsn;	   /* lsn at last flush */
-	xfs_dq_logformat_t	 qli_format;	   /* logged structure */
 } xfs_dq_logitem_t;
 
 typedef struct xfs_qoff_logitem {
-- 
1.7.10.4


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

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 10/10] xfs: remove the quotaoff log format from the quotaoff log item
  2013-11-29  8:39 [PATCH 00/10] decouple the in-memory from the on-disk log format V2 Christoph Hellwig
                   ` (8 preceding siblings ...)
  2013-11-29  8:39 ` [PATCH 09/10] xfs: remove the dquot log format from the dquot " Christoph Hellwig
@ 2013-11-29  8:39 ` Christoph Hellwig
  2013-12-04  0:49   ` Dave Chinner
  9 siblings, 1 reply; 29+ messages in thread
From: Christoph Hellwig @ 2013-11-29  8:39 UTC (permalink / raw)
  To: xfs

[-- Attachment #1: 0010-xfs-remove-the-quotaoff-log-format-from-the-quotaoff.patch --]
[-- Type: text/plain, Size: 2488 bytes --]

This one doesn't save a whole lot of memory, but still makes the code
simpler.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_dquot_item.c |   22 +++++++---------------
 fs/xfs/xfs_dquot_item.h |    2 +-
 2 files changed, 8 insertions(+), 16 deletions(-)

diff --git a/fs/xfs/xfs_dquot_item.c b/fs/xfs/xfs_dquot_item.c
index d4fffa9..f33fbaa 100644
--- a/fs/xfs/xfs_dquot_item.c
+++ b/fs/xfs/xfs_dquot_item.c
@@ -286,13 +286,6 @@ xfs_qm_qoff_logitem_size(
 	*nbytes += sizeof(struct xfs_qoff_logitem);
 }
 
-/*
- * This is called to fill in the vector of log iovecs for the
- * given quotaoff log item. We use only 1 iovec, and we point that
- * at the quotaoff_log_format structure embedded in the quotaoff item.
- * It is at this point that we assert that all of the extent
- * slots in the quotaoff item have been filled.
- */
 STATIC void
 xfs_qm_qoff_logitem_format(
 	struct xfs_log_item	*lip,
@@ -300,13 +293,13 @@ xfs_qm_qoff_logitem_format(
 {
 	struct xfs_qoff_logitem	*qflip = QOFF_ITEM(lip);
 	struct xfs_log_iovec	*vecp = NULL;
+	struct xfs_qoff_logformat *qlf;
 
-	ASSERT(qflip->qql_format.qf_type == XFS_LI_QUOTAOFF);
-	qflip->qql_format.qf_size = 1;
-
-	xlog_copy_iovec(lv, &vecp, XLOG_REG_TYPE_QUOTAOFF,
-			&qflip->qql_format,
-			sizeof(struct xfs_qoff_logitem));
+	qlf = xlog_prepare_iovec(lv, &vecp, XLOG_REG_TYPE_QUOTAOFF);
+	qlf->qf_type = XFS_LI_QUOTAOFF;
+	qlf->qf_size = 1;
+	qlf->qf_flags = qflip->qql_flags;
+	xlog_finish_iovec(lv, vecp, sizeof(struct xfs_qoff_logitem));
 }
 
 /*
@@ -446,8 +439,7 @@ xfs_qm_qoff_logitem_init(
 	xfs_log_item_init(mp, &qf->qql_item, XFS_LI_QUOTAOFF, start ?
 			&xfs_qm_qoffend_logitem_ops : &xfs_qm_qoff_logitem_ops);
 	qf->qql_item.li_mountp = mp;
-	qf->qql_format.qf_type = XFS_LI_QUOTAOFF;
-	qf->qql_format.qf_flags = flags;
 	qf->qql_start_lip = start;
+	qf->qql_flags = flags;
 	return qf;
 }
diff --git a/fs/xfs/xfs_dquot_item.h b/fs/xfs/xfs_dquot_item.h
index 925cbe9..502e946 100644
--- a/fs/xfs/xfs_dquot_item.h
+++ b/fs/xfs/xfs_dquot_item.h
@@ -32,7 +32,7 @@ typedef struct xfs_dq_logitem {
 typedef struct xfs_qoff_logitem {
 	xfs_log_item_t		 qql_item;	/* common portion */
 	struct xfs_qoff_logitem *qql_start_lip; /* qoff-start logitem, if any */
-	xfs_qoff_logformat_t	 qql_format;	/* logged structure */
+	unsigned int		qql_flags;
 } xfs_qoff_logitem_t;
 
 
-- 
1.7.10.4


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

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* Re: [PATCH 01/10] xfs: remove duplicate code in xlog_cil_insert_format_items
  2013-11-29  8:39 ` [PATCH 01/10] xfs: remove duplicate code in xlog_cil_insert_format_items Christoph Hellwig
@ 2013-12-03  0:58   ` Dave Chinner
  2013-12-09 19:45   ` Ben Myers
  1 sibling, 0 replies; 29+ messages in thread
From: Dave Chinner @ 2013-12-03  0:58 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: xfs

On Fri, Nov 29, 2013 at 12:39:20AM -0800, Christoph Hellwig wrote:
> Share code that was previously duplicated in two branches.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

looks good.

Reviewed-by: Dave Chinner <dchinner@redhat.com>

-- 
Dave Chinner
david@fromorbit.com

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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 02/10] xfs: refactor xfs_buf_item_format_segment
  2013-11-29  8:39 ` [PATCH 02/10] xfs: refactor xfs_buf_item_format_segment Christoph Hellwig
@ 2013-12-03  1:03   ` Dave Chinner
  0 siblings, 0 replies; 29+ messages in thread
From: Dave Chinner @ 2013-12-03  1:03 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: xfs

On Fri, Nov 29, 2013 at 12:39:21AM -0800, Christoph Hellwig wrote:
> Add two helpers to make the code more readable.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks good. Much easier to understand split out like this, so thanks
for doing that.

Reviewed-by: Dave Chinner <dchinner@redhat.com>

-- 
Dave Chinner
david@fromorbit.com

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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 03/10] xfs: refactor xfs_inode_item_size
  2013-11-29  8:39 ` [PATCH 03/10] xfs: refactor xfs_inode_item_size Christoph Hellwig
@ 2013-12-03  1:06   ` Dave Chinner
  0 siblings, 0 replies; 29+ messages in thread
From: Dave Chinner @ 2013-12-03  1:06 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: xfs

On Fri, Nov 29, 2013 at 12:39:22AM -0800, Christoph Hellwig wrote:
> Split out two helpers to size the data and attribute to make the
> function more readable.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks good.

Reviewed-by: Dave Chinner <dchinner@redhat.com>

-- 
Dave Chinner
david@fromorbit.com

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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 04/10] xfs: refactor xfs_inode_item_format
  2013-11-29  8:39 ` [PATCH 04/10] xfs: refactor xfs_inode_item_format Christoph Hellwig
@ 2013-12-03  1:10   ` Dave Chinner
  0 siblings, 0 replies; 29+ messages in thread
From: Dave Chinner @ 2013-12-03  1:10 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: xfs

On Fri, Nov 29, 2013 at 12:39:23AM -0800, Christoph Hellwig wrote:
> Split out a function to handle the data and attr fork, as well as a helper
> for the really old v1 inodes.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Much nicer!

Reviewed-by: Dave Chinner <dchinner@redhat.com>

-- 
Dave Chinner
david@fromorbit.com

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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 05/10] xfs: introduce xlog_copy_iovec
  2013-11-29  8:39 ` [PATCH 05/10] xfs: introduce xlog_copy_iovec Christoph Hellwig
@ 2013-12-03  1:21   ` Dave Chinner
  2013-12-03  9:43     ` Christoph Hellwig
  0 siblings, 1 reply; 29+ messages in thread
From: Dave Chinner @ 2013-12-03  1:21 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: xfs

On Fri, Nov 29, 2013 at 12:39:24AM -0800, Christoph Hellwig wrote:
> Add a helper to abstract out filling the log iovecs in the log item format
> handlers.  This will allow us to change the way we do the log item
> formatting more easily.
> 
> The copy in the name is a bit confusing for now as it just assigns a
> pointer and lets the CIL code perform the copy, but that will change
> soon.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks good - I think the helper was worth the effort. ;)

Reviewed-by: Dave Chinner <dchinner@redhat.com>
-- 
Dave Chinner
david@fromorbit.com

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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 05/10] xfs: introduce xlog_copy_iovec
  2013-12-03  1:21   ` Dave Chinner
@ 2013-12-03  9:43     ` Christoph Hellwig
  0 siblings, 0 replies; 29+ messages in thread
From: Christoph Hellwig @ 2013-12-03  9:43 UTC (permalink / raw)
  To: Dave Chinner; +Cc: Christoph Hellwig, xfs

On Tue, Dec 03, 2013 at 12:21:20PM +1100, Dave Chinner wrote:
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> 
> Looks good - I think the helper was worth the effort. ;)

This one is very different from the version I had earlier.  Took me a
long rainy Saturday to arrive at this version..

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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 06/10] xfs: format log items write directly into the linear CIL buffer
  2013-11-29  8:39 ` [PATCH 06/10] xfs: format log items write directly into the linear CIL buffer Christoph Hellwig
@ 2013-12-04  0:37   ` Dave Chinner
  2013-12-09 19:00     ` Ben Myers
  2013-12-11 12:03   ` [PATCH 06/10 v2] " Christoph Hellwig
  1 sibling, 1 reply; 29+ messages in thread
From: Dave Chinner @ 2013-12-04  0:37 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: xfs

On Fri, Nov 29, 2013 at 12:39:25AM -0800, Christoph Hellwig wrote:
> Instead of setting up pointers to memory locations in iop_format which then
> get copied into the CIL linear buffer after return move the copy into
> the individual inode items.  This avoids the need to always have a memory
> block in the exact same layout that gets written into the log around, and
> allow the log items to be much more flexible in their in-memory layouts.
> 
> Note that all log item format routines now need to be careful to modify
> the copy of the item that was placed into the CIL after calls to
> xlog_copy_iovec instead of the in-memory copy.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
>  	 */
>  	base_size = xfs_buf_log_format_size(blfp);
>  
> -	nvecs = 0;
>  	first_bit = xfs_next_bit(blfp->blf_data_map, blfp->blf_map_size, 0);
>  	if (!(bip->bli_flags & XFS_BLI_STALE) && first_bit == -1) {
>  		/*
>  		 * If the map is not be dirty in the transaction, mark
>  		 * the size as zero and do not advance the vector pointer.
>  		 */
> -		goto out;
> +		return;
>  	}
>  
> -	xlog_copy_iovec(vecp, XLOG_REG_TYPE_BFORMAT, blfp, base_size);
> -	nvecs = 1;
> +	blfp = xlog_copy_iovec(lv, vecp, XLOG_REG_TYPE_BFORMAT, blfp, base_size);
> +	blfp->blf_size = 1;

Hmmmm. What guarantee do we have blf_size is now natuarally aligned?
We've returned a pointer that could have any offset into the logvec
buffer, and so some platforms are going to have problems if blfp is
at an address that is not a multiple of 4 or 8, right?

>  xfs_inode_item_format_data_fork(
>  	struct xfs_inode_log_item *iip,
> -	struct xfs_log_iovec	**vecp,
> -	int			*nvecs)
> +	struct xfs_inode_log_format *ilf,
> +	struct xfs_log_vec	*lv,
> +	struct xfs_log_iovec	**vecp)
>  {
>  	struct xfs_inode	*ip = iip->ili_inode;
>  	size_t			data_bytes;
> @@ -239,19 +241,19 @@ xfs_inode_item_format_data_fork(
>  				 * extents, so just point to the
>  				 * real extents array.
>  				 */
> -				xlog_copy_iovec(vecp, XLOG_REG_TYPE_IEXT,
> +				xlog_copy_iovec(lv, vecp, XLOG_REG_TYPE_IEXT,
>  						ip->i_df.if_u1.if_extents,
>  						ip->i_df.if_bytes);
> -				iip->ili_format.ilf_dsize = ip->i_df.if_bytes;
> +				ilf->ilf_dsize = ip->i_df.if_bytes;

And by the looks of it, we could have the same problems here?

> diff --git a/fs/xfs/xfs_log.h b/fs/xfs/xfs_log.h
> index 384c6c4..e04bd0c 100644
> --- a/fs/xfs/xfs_log.h
> +++ b/fs/xfs/xfs_log.h
> @@ -31,18 +31,44 @@ struct xfs_log_vec {
>  #define XFS_LOG_VEC_ORDERED	(-1)
>  
>  static inline void *
> -xlog_copy_iovec(struct xfs_log_iovec **vecp, uint type, void *data, int len)
> +xlog_prepare_iovec(struct xfs_log_vec *lv, struct xfs_log_iovec **vecp,
> +		uint type)
>  {
>  	struct xfs_log_iovec *vec = *vecp;
>  
> +	if (vec) {
> +		ASSERT(vec - lv->lv_iovecp < lv->lv_niovecs);
> +		vec++;
> +	} else {
> +		vec = &lv->lv_iovecp[0];
> +	}
> +
>  	vec->i_type = type;
> -	vec->i_addr = data;
> -	vec->i_len = len;
> +	vec->i_addr = lv->lv_buf + lv->lv_buf_len;

We could at least check here that the alignment is good...

>  
> -	*vecp = vec + 1;
> +	*vecp = vec;
>  	return vec->i_addr;
>  }
>  
> +static inline void
> +xlog_finish_iovec(struct xfs_log_vec *lv, struct xfs_log_iovec *vec, int len)
> +{
> +	lv->lv_buf_len += len;

And if we need to guarantee alignment, then maybe roundup here to
ensure we don't end up with bad offsets?  That would require padding
the allocation of the buffer to take it into account, too....

Other than this concern, the code looks fine.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 07/10] xfs: format logged extents directly into the CIL
  2013-11-29  8:39 ` [PATCH 07/10] xfs: format logged extents directly into the CIL Christoph Hellwig
@ 2013-12-04  0:40   ` Dave Chinner
  0 siblings, 0 replies; 29+ messages in thread
From: Dave Chinner @ 2013-12-04  0:40 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: xfs

On Fri, Nov 29, 2013 at 12:39:26AM -0800, Christoph Hellwig wrote:
> With the new iop_format scheme there is no need to have a temporary buffer
> to format logged extents into, we can do so directly into the CIL.  This
> also allows to remove the shortcut for big endian systems that probably
> hasn't gotten a lot of test coverage for a long time.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Very nice! Lots of cruft just disappears here :)

Reviewed-by: Dave Chinner <dchinner@redhat.com>

-- 
Dave Chinner
david@fromorbit.com

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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 08/10] xfs: remove the inode log format from the inode log item
  2013-11-29  8:39 ` [PATCH 08/10] xfs: remove the inode log format from the inode log item Christoph Hellwig
@ 2013-12-04  0:44   ` Dave Chinner
  0 siblings, 0 replies; 29+ messages in thread
From: Dave Chinner @ 2013-12-04  0:44 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: xfs

On Fri, Nov 29, 2013 at 12:39:27AM -0800, Christoph Hellwig wrote:
> No need to keep the inode log format around all the time, we can easily
> generate it at iop_format time.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Ignoring the previously mentioned structure alignment issue, this
change looks good. Overall, the inode log item is reduced in size by
about 72 bytes. :)

Reviewed-by: Dave Chinner <dchinner@redhat.com>

-- 
Dave Chinner
david@fromorbit.com

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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 09/10] xfs: remove the dquot log format from the dquot log item
  2013-11-29  8:39 ` [PATCH 09/10] xfs: remove the dquot log format from the dquot " Christoph Hellwig
@ 2013-12-04  0:47   ` Dave Chinner
  0 siblings, 0 replies; 29+ messages in thread
From: Dave Chinner @ 2013-12-04  0:47 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: xfs

On Fri, Nov 29, 2013 at 12:39:28AM -0800, Christoph Hellwig wrote:
> No need to keep the dquot log format around all the time, we can easily
> generate it at iop_format time.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

looks good.

Reviewed-by: Dave Chinner <dchinner@redhat.com>
-- 
Dave Chinner
david@fromorbit.com

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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 10/10] xfs: remove the quotaoff log format from the quotaoff log item
  2013-11-29  8:39 ` [PATCH 10/10] xfs: remove the quotaoff log format from the quotaoff " Christoph Hellwig
@ 2013-12-04  0:49   ` Dave Chinner
  0 siblings, 0 replies; 29+ messages in thread
From: Dave Chinner @ 2013-12-04  0:49 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: xfs

On Fri, Nov 29, 2013 at 12:39:29AM -0800, Christoph Hellwig wrote:
> This one doesn't save a whole lot of memory, but still makes the code
> simpler.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks good,

Reviewed-by: Dave Chinner <dchinner@redhat.com>
-- 
Dave Chinner
david@fromorbit.com

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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 06/10] xfs: format log items write directly into the linear CIL buffer
  2013-12-04  0:37   ` Dave Chinner
@ 2013-12-09 19:00     ` Ben Myers
  2013-12-10 16:12       ` Christoph Hellwig
  0 siblings, 1 reply; 29+ messages in thread
From: Ben Myers @ 2013-12-09 19:00 UTC (permalink / raw)
  To: Christoph Hellwig, Dave Chinner; +Cc: xfs

On Wed, Dec 04, 2013 at 11:37:12AM +1100, Dave Chinner wrote:
> On Fri, Nov 29, 2013 at 12:39:25AM -0800, Christoph Hellwig wrote:
> > Instead of setting up pointers to memory locations in iop_format which then
> > get copied into the CIL linear buffer after return move the copy into
> > the individual inode items.  This avoids the need to always have a memory
> > block in the exact same layout that gets written into the log around, and
> > allow the log items to be much more flexible in their in-memory layouts.
> > 
> > Note that all log item format routines now need to be careful to modify
> > the copy of the item that was placed into the CIL after calls to
> > xlog_copy_iovec instead of the in-memory copy.
> > 
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> >  	 */
> >  	base_size = xfs_buf_log_format_size(blfp);
> >  
> > -	nvecs = 0;
> >  	first_bit = xfs_next_bit(blfp->blf_data_map, blfp->blf_map_size, 0);
> >  	if (!(bip->bli_flags & XFS_BLI_STALE) && first_bit == -1) {
> >  		/*
> >  		 * If the map is not be dirty in the transaction, mark
> >  		 * the size as zero and do not advance the vector pointer.
> >  		 */
> > -		goto out;
> > +		return;
> >  	}
> >  
> > -	xlog_copy_iovec(vecp, XLOG_REG_TYPE_BFORMAT, blfp, base_size);
> > -	nvecs = 1;
> > +	blfp = xlog_copy_iovec(lv, vecp, XLOG_REG_TYPE_BFORMAT, blfp, base_size);
> > +	blfp->blf_size = 1;
> 
> Hmmmm. What guarantee do we have blf_size is now natuarally aligned?
> We've returned a pointer that could have any offset into the logvec
> buffer, and so some platforms are going to have problems if blfp is
> at an address that is not a multiple of 4 or 8, right?
> 
> >  xfs_inode_item_format_data_fork(
> >  	struct xfs_inode_log_item *iip,
> > -	struct xfs_log_iovec	**vecp,
> > -	int			*nvecs)
> > +	struct xfs_inode_log_format *ilf,
> > +	struct xfs_log_vec	*lv,
> > +	struct xfs_log_iovec	**vecp)
> >  {
> >  	struct xfs_inode	*ip = iip->ili_inode;
> >  	size_t			data_bytes;
> > @@ -239,19 +241,19 @@ xfs_inode_item_format_data_fork(
> >  				 * extents, so just point to the
> >  				 * real extents array.
> >  				 */
> > -				xlog_copy_iovec(vecp, XLOG_REG_TYPE_IEXT,
> > +				xlog_copy_iovec(lv, vecp, XLOG_REG_TYPE_IEXT,
> >  						ip->i_df.if_u1.if_extents,
> >  						ip->i_df.if_bytes);
> > -				iip->ili_format.ilf_dsize = ip->i_df.if_bytes;
> > +				ilf->ilf_dsize = ip->i_df.if_bytes;
> 
> And by the looks of it, we could have the same problems here?
> 
> > diff --git a/fs/xfs/xfs_log.h b/fs/xfs/xfs_log.h
> > index 384c6c4..e04bd0c 100644
> > --- a/fs/xfs/xfs_log.h
> > +++ b/fs/xfs/xfs_log.h
> > @@ -31,18 +31,44 @@ struct xfs_log_vec {
> >  #define XFS_LOG_VEC_ORDERED	(-1)
> >  
> >  static inline void *
> > -xlog_copy_iovec(struct xfs_log_iovec **vecp, uint type, void *data, int len)
> > +xlog_prepare_iovec(struct xfs_log_vec *lv, struct xfs_log_iovec **vecp,
> > +		uint type)
> >  {
> >  	struct xfs_log_iovec *vec = *vecp;
> >  
> > +	if (vec) {
> > +		ASSERT(vec - lv->lv_iovecp < lv->lv_niovecs);
> > +		vec++;
> > +	} else {
> > +		vec = &lv->lv_iovecp[0];
> > +	}
> > +
> >  	vec->i_type = type;
> > -	vec->i_addr = data;
> > -	vec->i_len = len;
> > +	vec->i_addr = lv->lv_buf + lv->lv_buf_len;
> 
> We could at least check here that the alignment is good...
> 
> >  
> > -	*vecp = vec + 1;
> > +	*vecp = vec;
> >  	return vec->i_addr;
> >  }
> >  
> > +static inline void
> > +xlog_finish_iovec(struct xfs_log_vec *lv, struct xfs_log_iovec *vec, int len)
> > +{
> > +	lv->lv_buf_len += len;
> 
> And if we need to guarantee alignment, then maybe roundup here to
> ensure we don't end up with bad offsets?  That would require padding
> the allocation of the buffer to take it into account, too....
> 
> Other than this concern, the code looks fine.

Christoph, what about this alignment issue?

Thanks,
	Ben

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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 01/10] xfs: remove duplicate code in xlog_cil_insert_format_items
  2013-11-29  8:39 ` [PATCH 01/10] xfs: remove duplicate code in xlog_cil_insert_format_items Christoph Hellwig
  2013-12-03  0:58   ` Dave Chinner
@ 2013-12-09 19:45   ` Ben Myers
  2013-12-10 16:18     ` Christoph Hellwig
  1 sibling, 1 reply; 29+ messages in thread
From: Ben Myers @ 2013-12-09 19:45 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: xfs

Christoph,

On Fri, Nov 29, 2013 at 12:39:20AM -0800, Christoph Hellwig wrote:
> Share code that was previously duplicated in two branches.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/xfs_log_cil.c |   33 +++++++++++++--------------------
>  1 file changed, 13 insertions(+), 20 deletions(-)
> 
> diff --git a/fs/xfs/xfs_log_cil.c b/fs/xfs/xfs_log_cil.c
> index 5eb51fc..0a7a8ce 100644
> --- a/fs/xfs/xfs_log_cil.c
> +++ b/fs/xfs/xfs_log_cil.c
> @@ -254,29 +254,22 @@ xlog_cil_insert_format_items(
>  			 */
>  			*diff_iovecs -= lv->lv_niovecs;
>  			*diff_len -= lv->lv_buf_len;
> -
> -			/* Ensure the lv is set up according to ->iop_size */
> -			lv->lv_niovecs = niovecs;
> -			lv->lv_buf = (char *)lv + buf_size - nbytes;
> -
> -			lv->lv_buf_len = xlog_cil_lv_item_format(lip, lv);
> -			goto insert;
> +		} else {
> +			/* allocate new data chunk */
> +			lv = kmem_zalloc(buf_size, KM_SLEEP|KM_NOFS);
> +			lv->lv_item = lip;
> +			lv->lv_size = buf_size;
> +			if (ordered) {
> +				/* track as an ordered logvec */
> +				ASSERT(lip->li_lv == NULL);
> +				lv->lv_buf_len = XFS_LOG_VEC_ORDERED;
> +				goto insert;

It looks like lv->lv_niovecs is no longer being set in this case.  Was that intentional?

Thanks,
Ben

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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 06/10] xfs: format log items write directly into the linear CIL buffer
  2013-12-09 19:00     ` Ben Myers
@ 2013-12-10 16:12       ` Christoph Hellwig
  0 siblings, 0 replies; 29+ messages in thread
From: Christoph Hellwig @ 2013-12-10 16:12 UTC (permalink / raw)
  To: Ben Myers; +Cc: Christoph Hellwig, xfs

On Mon, Dec 09, 2013 at 01:00:28PM -0600, Ben Myers wrote:
> > And if we need to guarantee alignment, then maybe roundup here to
> > ensure we don't end up with bad offsets?  That would require padding
> > the allocation of the buffer to take it into account, too....
> > 
> > Other than this concern, the code looks fine.
> 
> Christoph, what about this alignment issue?

I think Dave is right, but I haven't had time to look at the issue in
detail and respin the patchset yet.

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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 01/10] xfs: remove duplicate code in xlog_cil_insert_format_items
  2013-12-09 19:45   ` Ben Myers
@ 2013-12-10 16:18     ` Christoph Hellwig
  0 siblings, 0 replies; 29+ messages in thread
From: Christoph Hellwig @ 2013-12-10 16:18 UTC (permalink / raw)
  To: Ben Myers; +Cc: Christoph Hellwig, xfs

On Mon, Dec 09, 2013 at 01:45:48PM -0600, Ben Myers wrote:
> > +			if (ordered) {
> > +				/* track as an ordered logvec */
> > +				ASSERT(lip->li_lv == NULL);
> > +				lv->lv_buf_len = XFS_LOG_VEC_ORDERED;
> > +				goto insert;
> 
> It looks like lv->lv_niovecs is no longer being set in this case.  Was that intentional?

Yes, ordered log vecs don't have iovecs attached, so the implicit zero
from the kmem_zalloc is the right value.

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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH 06/10 v2] xfs: format log items write directly into the linear CIL buffer
  2013-11-29  8:39 ` [PATCH 06/10] xfs: format log items write directly into the linear CIL buffer Christoph Hellwig
  2013-12-04  0:37   ` Dave Chinner
@ 2013-12-11 12:03   ` Christoph Hellwig
  2013-12-12  0:05     ` Dave Chinner
  2013-12-12 16:25     ` [PATCH 06/10 v3] " Christoph Hellwig
  1 sibling, 2 replies; 29+ messages in thread
From: Christoph Hellwig @ 2013-12-11 12:03 UTC (permalink / raw)
  To: xfs

Instead of setting up pointers to memory locations in iop_format which then
get copied into the CIL linear buffer after return move the copy into
the individual inode items.  This avoids the need to always have a memory
block in the exact same layout that gets written into the log around, and
allow the log items to be much more flexible in their in-memory layouts.

The only caveat is that we need to properly align the data for each
iovec so that don't have structures misaligned in subsequent iovecs.

Note that all log item format routines now need to be careful to modify
the copy of the item that was placed into the CIL after calls to
xlog_copy_iovec instead of the in-memory copy.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_buf_item.c     |   29 +++++++-------
 fs/xfs/xfs_dquot_item.c   |   19 +++++-----
 fs/xfs/xfs_extfree_item.c |   10 +++--
 fs/xfs/xfs_icreate_item.c |    5 ++-
 fs/xfs/xfs_inode_item.c   |   92 ++++++++++++++++++++++-----------------------
 fs/xfs/xfs_log.h          |   39 +++++++++++++++++--
 fs/xfs/xfs_log_cil.c      |   41 +++++---------------
 fs/xfs/xfs_trans.h        |    2 +-
 8 files changed, 123 insertions(+), 114 deletions(-)

diff --git a/fs/xfs/xfs_buf_item.c b/fs/xfs/xfs_buf_item.c
index d49419d..7641173 100644
--- a/fs/xfs/xfs_buf_item.c
+++ b/fs/xfs/xfs_buf_item.c
@@ -184,6 +184,7 @@ xfs_buf_item_size(
 
 static inline void
 xfs_buf_item_copy_iovec(
+	struct xfs_log_vec	*lv,
 	struct xfs_log_iovec	**vecp,
 	struct xfs_buf		*bp,
 	uint			offset,
@@ -191,7 +192,7 @@ xfs_buf_item_copy_iovec(
 	uint			nbits)
 {
 	offset += first_bit * XFS_BLF_CHUNK;
-	xlog_copy_iovec(vecp, XLOG_REG_TYPE_BCHUNK,
+	xlog_copy_iovec(lv, vecp, XLOG_REG_TYPE_BCHUNK,
 			xfs_buf_offset(bp, offset),
 			nbits * XFS_BLF_CHUNK);
 }
@@ -211,13 +212,13 @@ xfs_buf_item_straddle(
 static void
 xfs_buf_item_format_segment(
 	struct xfs_buf_log_item	*bip,
+	struct xfs_log_vec	*lv,
 	struct xfs_log_iovec	**vecp,
 	uint			offset,
 	struct xfs_buf_log_format *blfp)
 {
 	struct xfs_buf	*bp = bip->bli_buf;
 	uint		base_size;
-	uint		nvecs;
 	int		first_bit;
 	int		last_bit;
 	int		next_bit;
@@ -233,18 +234,17 @@ xfs_buf_item_format_segment(
 	 */
 	base_size = xfs_buf_log_format_size(blfp);
 
-	nvecs = 0;
 	first_bit = xfs_next_bit(blfp->blf_data_map, blfp->blf_map_size, 0);
 	if (!(bip->bli_flags & XFS_BLI_STALE) && first_bit == -1) {
 		/*
 		 * If the map is not be dirty in the transaction, mark
 		 * the size as zero and do not advance the vector pointer.
 		 */
-		goto out;
+		return;
 	}
 
-	xlog_copy_iovec(vecp, XLOG_REG_TYPE_BFORMAT, blfp, base_size);
-	nvecs = 1;
+	blfp = xlog_copy_iovec(lv, vecp, XLOG_REG_TYPE_BFORMAT, blfp, base_size);
+	blfp->blf_size = 1;
 
 	if (bip->bli_flags & XFS_BLI_STALE) {
 		/*
@@ -254,7 +254,7 @@ xfs_buf_item_format_segment(
 		 */
 		trace_xfs_buf_item_format_stale(bip);
 		ASSERT(blfp->blf_flags & XFS_BLF_CANCEL);
-		goto out;
+		return;
 	}
 
 
@@ -280,15 +280,15 @@ xfs_buf_item_format_segment(
 		 * same set of bits so just keep counting and scanning.
 		 */
 		if (next_bit == -1) {
-			xfs_buf_item_copy_iovec(vecp, bp, offset,
+			xfs_buf_item_copy_iovec(lv, vecp, bp, offset,
 						first_bit, nbits);
-			nvecs++;
+			blfp->blf_size++;
 			break;
 		} else if (next_bit != last_bit + 1 ||
 		           xfs_buf_item_straddle(bp, offset, next_bit, last_bit)) {
-			xfs_buf_item_copy_iovec(vecp, bp, offset,
+			xfs_buf_item_copy_iovec(lv, vecp, bp, offset,
 						first_bit, nbits);
-			nvecs++;
+			blfp->blf_size++;
 			first_bit = next_bit;
 			last_bit = next_bit;
 			nbits = 1;
@@ -297,8 +297,6 @@ xfs_buf_item_format_segment(
 			nbits++;
 		}
 	}
-out:
-	blfp->blf_size = nvecs;
 }
 
 /*
@@ -310,10 +308,11 @@ out:
 STATIC void
 xfs_buf_item_format(
 	struct xfs_log_item	*lip,
-	struct xfs_log_iovec	*vecp)
+	struct xfs_log_vec	*lv)
 {
 	struct xfs_buf_log_item	*bip = BUF_ITEM(lip);
 	struct xfs_buf		*bp = bip->bli_buf;
+	struct xfs_log_iovec	*vecp = NULL;
 	uint			offset = 0;
 	int			i;
 
@@ -354,7 +353,7 @@ xfs_buf_item_format(
 	}
 
 	for (i = 0; i < bip->bli_format_count; i++) {
-		xfs_buf_item_format_segment(bip, &vecp, offset,
+		xfs_buf_item_format_segment(bip, lv, &vecp, offset,
 					    &bip->bli_formats[i]);
 		offset += bp->b_maps[i].bm_len;
 	}
diff --git a/fs/xfs/xfs_dquot_item.c b/fs/xfs/xfs_dquot_item.c
index ca354a8..946d588 100644
--- a/fs/xfs/xfs_dquot_item.c
+++ b/fs/xfs/xfs_dquot_item.c
@@ -57,18 +57,19 @@ xfs_qm_dquot_logitem_size(
 STATIC void
 xfs_qm_dquot_logitem_format(
 	struct xfs_log_item	*lip,
-	struct xfs_log_iovec	*vecp)
+	struct xfs_log_vec	*lv)
 {
 	struct xfs_dq_logitem	*qlip = DQUOT_ITEM(lip);
+	struct xfs_log_iovec	*vecp = NULL;
 
-	xlog_copy_iovec(&vecp, XLOG_REG_TYPE_QFORMAT,
+	qlip->qli_format.qlf_size = 2;
+
+	xlog_copy_iovec(lv, &vecp, XLOG_REG_TYPE_QFORMAT,
 			&qlip->qli_format,
 			sizeof(struct xfs_dq_logformat));
-	xlog_copy_iovec(&vecp, XLOG_REG_TYPE_DQUOT,
+	xlog_copy_iovec(lv, &vecp, XLOG_REG_TYPE_DQUOT,
 			&qlip->qli_dquot->q_core,
 			sizeof(struct xfs_disk_dquot));
-
-	qlip->qli_format.qlf_size = 2;
 }
 
 /*
@@ -302,17 +303,17 @@ xfs_qm_qoff_logitem_size(
 STATIC void
 xfs_qm_qoff_logitem_format(
 	struct xfs_log_item	*lip,
-	struct xfs_log_iovec	*vecp)
+	struct xfs_log_vec	*lv)
 {
 	struct xfs_qoff_logitem	*qflip = QOFF_ITEM(lip);
+	struct xfs_log_iovec	*vecp = NULL;
 
 	ASSERT(qflip->qql_format.qf_type == XFS_LI_QUOTAOFF);
+	qflip->qql_format.qf_size = 1;
 
-	xlog_copy_iovec(&vecp, XLOG_REG_TYPE_QUOTAOFF,
+	xlog_copy_iovec(lv, &vecp, XLOG_REG_TYPE_QUOTAOFF,
 			&qflip->qql_format,
 			sizeof(struct xfs_qoff_logitem));
-
-	qflip->qql_format.qf_size = 1;
 }
 
 /*
diff --git a/fs/xfs/xfs_extfree_item.c b/fs/xfs/xfs_extfree_item.c
index 08823ec..fb7a4c1 100644
--- a/fs/xfs/xfs_extfree_item.c
+++ b/fs/xfs/xfs_extfree_item.c
@@ -102,9 +102,10 @@ xfs_efi_item_size(
 STATIC void
 xfs_efi_item_format(
 	struct xfs_log_item	*lip,
-	struct xfs_log_iovec	*vecp)
+	struct xfs_log_vec	*lv)
 {
 	struct xfs_efi_log_item	*efip = EFI_ITEM(lip);
+	struct xfs_log_iovec	*vecp = NULL;
 
 	ASSERT(atomic_read(&efip->efi_next_extent) ==
 				efip->efi_format.efi_nextents);
@@ -112,7 +113,7 @@ xfs_efi_item_format(
 	efip->efi_format.efi_type = XFS_LI_EFI;
 	efip->efi_format.efi_size = 1;
 
-	xlog_copy_iovec(&vecp, XLOG_REG_TYPE_EFI_FORMAT,
+	xlog_copy_iovec(lv, &vecp, XLOG_REG_TYPE_EFI_FORMAT,
 			&efip->efi_format,
 			xfs_efi_item_sizeof(efip));
 }
@@ -368,16 +369,17 @@ xfs_efd_item_size(
 STATIC void
 xfs_efd_item_format(
 	struct xfs_log_item	*lip,
-	struct xfs_log_iovec	*vecp)
+	struct xfs_log_vec	*lv)
 {
 	struct xfs_efd_log_item	*efdp = EFD_ITEM(lip);
+	struct xfs_log_iovec	*vecp = NULL;
 
 	ASSERT(efdp->efd_next_extent == efdp->efd_format.efd_nextents);
 
 	efdp->efd_format.efd_type = XFS_LI_EFD;
 	efdp->efd_format.efd_size = 1;
 
-	xlog_copy_iovec(&vecp, XLOG_REG_TYPE_EFD_FORMAT,
+	xlog_copy_iovec(lv, &vecp, XLOG_REG_TYPE_EFD_FORMAT,
 			&efdp->efd_format,
 			xfs_efd_item_sizeof(efdp));
 }
diff --git a/fs/xfs/xfs_icreate_item.c b/fs/xfs/xfs_icreate_item.c
index 5751fa8..7e45492 100644
--- a/fs/xfs/xfs_icreate_item.c
+++ b/fs/xfs/xfs_icreate_item.c
@@ -59,11 +59,12 @@ xfs_icreate_item_size(
 STATIC void
 xfs_icreate_item_format(
 	struct xfs_log_item	*lip,
-	struct xfs_log_iovec	*vecp)
+	struct xfs_log_vec	*lv)
 {
 	struct xfs_icreate_item	*icp = ICR_ITEM(lip);
+	struct xfs_log_iovec	*vecp = NULL;
 
-	xlog_copy_iovec(&vecp, XLOG_REG_TYPE_ICREATE,
+	xlog_copy_iovec(lv, &vecp, XLOG_REG_TYPE_ICREATE,
 			&icp->ic_format,
 			sizeof(struct xfs_icreate_log));
 }
diff --git a/fs/xfs/xfs_inode_item.c b/fs/xfs/xfs_inode_item.c
index 73002db..35dd24a 100644
--- a/fs/xfs/xfs_inode_item.c
+++ b/fs/xfs/xfs_inode_item.c
@@ -163,6 +163,7 @@ xfs_inode_item_size(
 STATIC int
 xfs_inode_item_format_extents(
 	struct xfs_inode	*ip,
+	struct xfs_log_vec	*lv,
 	struct xfs_log_iovec	**vecp,
 	int			whichfork,
 	int			type)
@@ -177,7 +178,7 @@ xfs_inode_item_format_extents(
 		ip->i_itemp->ili_aextents_buf = ext_buffer;
 
 	len = xfs_iextents_copy(ip, ext_buffer, whichfork);
-	xlog_copy_iovec(vecp, type, ext_buffer, len);
+	xlog_copy_iovec(lv, vecp, type, ext_buffer, len);
 	return len;
 }
 
@@ -212,8 +213,9 @@ xfs_inode_item_format_v1_inode(
 STATIC void
 xfs_inode_item_format_data_fork(
 	struct xfs_inode_log_item *iip,
-	struct xfs_log_iovec	**vecp,
-	int			*nvecs)
+	struct xfs_inode_log_format *ilf,
+	struct xfs_log_vec	*lv,
+	struct xfs_log_iovec	**vecp)
 {
 	struct xfs_inode	*ip = iip->ili_inode;
 	size_t			data_bytes;
@@ -239,19 +241,19 @@ xfs_inode_item_format_data_fork(
 				 * extents, so just point to the
 				 * real extents array.
 				 */
-				xlog_copy_iovec(vecp, XLOG_REG_TYPE_IEXT,
+				xlog_copy_iovec(lv, vecp, XLOG_REG_TYPE_IEXT,
 						ip->i_df.if_u1.if_extents,
 						ip->i_df.if_bytes);
-				iip->ili_format.ilf_dsize = ip->i_df.if_bytes;
+				ilf->ilf_dsize = ip->i_df.if_bytes;
 			} else
 #endif
 			{
-				iip->ili_format.ilf_dsize =
-					xfs_inode_item_format_extents(ip, vecp,
+				ilf->ilf_dsize =
+					xfs_inode_item_format_extents(ip, lv, vecp,
 						XFS_DATA_FORK, XLOG_REG_TYPE_IEXT);
 				ASSERT(iip->ili_format.ilf_dsize <= ip->i_df.if_bytes);
 			}
-			(*nvecs)++;
+			ilf->ilf_size++;
 		} else {
 			iip->ili_fields &= ~XFS_ILOG_DEXT;
 		}
@@ -264,11 +266,11 @@ xfs_inode_item_format_data_fork(
 		if ((iip->ili_fields & XFS_ILOG_DBROOT) &&
 		    ip->i_df.if_broot_bytes > 0) {
 			ASSERT(ip->i_df.if_broot != NULL);
-			xlog_copy_iovec(vecp, XLOG_REG_TYPE_IBROOT,
+			xlog_copy_iovec(lv, vecp, XLOG_REG_TYPE_IBROOT,
 					ip->i_df.if_broot,
 					ip->i_df.if_broot_bytes);
-			(*nvecs)++;
-			iip->ili_format.ilf_dsize = ip->i_df.if_broot_bytes;
+			ilf->ilf_dsize = ip->i_df.if_broot_bytes;
+			ilf->ilf_size++;
 		} else {
 			ASSERT(!(iip->ili_fields &
 				 XFS_ILOG_DBROOT));
@@ -291,10 +293,10 @@ xfs_inode_item_format_data_fork(
 			       ip->i_df.if_real_bytes == data_bytes);
 			ASSERT(ip->i_df.if_u1.if_data != NULL);
 			ASSERT(ip->i_d.di_size > 0);
-			xlog_copy_iovec(vecp, XLOG_REG_TYPE_ILOCAL,
+			xlog_copy_iovec(lv, vecp, XLOG_REG_TYPE_ILOCAL,
 					ip->i_df.if_u1.if_data, data_bytes);
-			(*nvecs)++;
-			iip->ili_format.ilf_dsize = (unsigned)data_bytes;
+			ilf->ilf_dsize = (unsigned)data_bytes;
+			ilf->ilf_size++;
 		} else {
 			iip->ili_fields &= ~XFS_ILOG_DDATA;
 		}
@@ -303,19 +305,15 @@ xfs_inode_item_format_data_fork(
 		iip->ili_fields &=
 			~(XFS_ILOG_DDATA | XFS_ILOG_DBROOT |
 			  XFS_ILOG_DEXT | XFS_ILOG_UUID);
-		if (iip->ili_fields & XFS_ILOG_DEV) {
-			iip->ili_format.ilf_u.ilfu_rdev =
-				ip->i_df.if_u2.if_rdev;
-		}
+		if (iip->ili_fields & XFS_ILOG_DEV)
+			ilf->ilf_u.ilfu_rdev = ip->i_df.if_u2.if_rdev;
 		break;
 	case XFS_DINODE_FMT_UUID:
 		iip->ili_fields &=
 			~(XFS_ILOG_DDATA | XFS_ILOG_DBROOT |
 			  XFS_ILOG_DEXT | XFS_ILOG_DEV);
-		if (iip->ili_fields & XFS_ILOG_UUID) {
-			iip->ili_format.ilf_u.ilfu_uuid =
-				ip->i_df.if_u2.if_uuid;
-		}
+		if (iip->ili_fields & XFS_ILOG_UUID)
+			ilf->ilf_u.ilfu_uuid = ip->i_df.if_u2.if_uuid;
 		break;
 	default:
 		ASSERT(0);
@@ -326,8 +324,9 @@ xfs_inode_item_format_data_fork(
 STATIC void
 xfs_inode_item_format_attr_fork(
 	struct xfs_inode_log_item *iip,
-	struct xfs_log_iovec	**vecp,
-	int			*nvecs)
+	struct xfs_inode_log_format *ilf,
+	struct xfs_log_vec	*lv,
+	struct xfs_log_iovec	**vecp)
 {
 	struct xfs_inode	*ip = iip->ili_inode;
 	size_t			data_bytes;
@@ -348,17 +347,17 @@ xfs_inode_item_format_attr_fork(
 			 * There are not delayed allocation extents
 			 * for attributes, so just point at the array.
 			 */
-			xlog_copy_iovec(vecp, XLOG_REG_TYPE_IATTR_EXT,
+			xlog_copy_iovec(lv, vecp, XLOG_REG_TYPE_IATTR_EXT,
 					ip->i_afp->if_u1.if_extents,
 					ip->i_afp->if_bytes);
-			iip->ili_format.ilf_asize = ip->i_afp->if_bytes;
+			ilf->ilf_asize = ip->i_afp->if_bytes;
 #else
 			ASSERT(iip->ili_aextents_buf == NULL);
-			iip->ili_format.ilf_asize =
-				xfs_inode_item_format_extents(ip, vecp,
+			ilf->ilf_asize =
+				xfs_inode_item_format_extents(ip, lv, vecp,
 					XFS_ATTR_FORK, XLOG_REG_TYPE_IATTR_EXT);
 #endif
-			(*nvecs)++;
+			ilf->ilf_size++;
 		} else {
 			iip->ili_fields &= ~XFS_ILOG_AEXT;
 		}
@@ -371,11 +370,11 @@ xfs_inode_item_format_attr_fork(
 		    ip->i_afp->if_broot_bytes > 0) {
 			ASSERT(ip->i_afp->if_broot != NULL);
 
-			xlog_copy_iovec(vecp, XLOG_REG_TYPE_IATTR_BROOT,
+			xlog_copy_iovec(lv, vecp, XLOG_REG_TYPE_IATTR_BROOT,
 					ip->i_afp->if_broot,
 					ip->i_afp->if_broot_bytes);
-			(*nvecs)++;
-			iip->ili_format.ilf_asize = ip->i_afp->if_broot_bytes;
+			ilf->ilf_asize = ip->i_afp->if_broot_bytes;
+			ilf->ilf_size++;
 		} else {
 			iip->ili_fields &= ~XFS_ILOG_ABROOT;
 		}
@@ -395,11 +394,11 @@ xfs_inode_item_format_attr_fork(
 			ASSERT(ip->i_afp->if_real_bytes == 0 ||
 			       ip->i_afp->if_real_bytes == data_bytes);
 			ASSERT(ip->i_afp->if_u1.if_data != NULL);
-			xlog_copy_iovec(vecp, XLOG_REG_TYPE_IATTR_LOCAL,
+			xlog_copy_iovec(lv, vecp, XLOG_REG_TYPE_IATTR_LOCAL,
 					ip->i_afp->if_u1.if_data,
 					data_bytes);
-			(*nvecs)++;
-			iip->ili_format.ilf_asize = (unsigned)data_bytes;
+			ilf->ilf_asize = (unsigned)data_bytes;
+			ilf->ilf_size++;
 		} else {
 			iip->ili_fields &= ~XFS_ILOG_ADATA;
 		}
@@ -420,28 +419,28 @@ xfs_inode_item_format_attr_fork(
 STATIC void
 xfs_inode_item_format(
 	struct xfs_log_item	*lip,
-	struct xfs_log_iovec	*vecp)
+	struct xfs_log_vec	*lv)
 {
 	struct xfs_inode_log_item *iip = INODE_ITEM(lip);
 	struct xfs_inode	*ip = iip->ili_inode;
-	uint			nvecs;
+	struct xfs_inode_log_format *ilf;
+	struct xfs_log_iovec	*vecp = NULL;
 
-	xlog_copy_iovec(&vecp, XLOG_REG_TYPE_IFORMAT,
+	ilf = xlog_copy_iovec(lv, &vecp, XLOG_REG_TYPE_IFORMAT,
 			&iip->ili_format,
 			sizeof(struct xfs_inode_log_format));
-	nvecs = 1;
-
-	xlog_copy_iovec(&vecp, XLOG_REG_TYPE_ICORE,
-			&ip->i_d,
-			xfs_icdinode_size(ip->i_d.di_version));
-	nvecs++;
+	ilf->ilf_size = 1;
 
 	if (ip->i_d.di_version == 1)
 		xfs_inode_item_format_v1_inode(ip);
+	xlog_copy_iovec(lv, &vecp, XLOG_REG_TYPE_ICORE,
+			&ip->i_d,
+			xfs_icdinode_size(ip->i_d.di_version));
+	ilf->ilf_size++;
 
-	xfs_inode_item_format_data_fork(iip, &vecp, &nvecs);
+	xfs_inode_item_format_data_fork(iip, ilf, lv, &vecp);
 	if (XFS_IFORK_Q(ip)) {
-		xfs_inode_item_format_attr_fork(iip, &vecp, &nvecs);
+		xfs_inode_item_format_attr_fork(iip, ilf, lv, &vecp);
 	} else {
 		iip->ili_fields &=
 			~(XFS_ILOG_ADATA | XFS_ILOG_ABROOT | XFS_ILOG_AEXT);
@@ -455,7 +454,6 @@ xfs_inode_item_format(
 	 */
 	iip->ili_format.ilf_fields = XFS_ILOG_CORE |
 		(iip->ili_fields & ~XFS_ILOG_TIMESTAMP);
-	iip->ili_format.ilf_size = nvecs;
 }
 
 /*
diff --git a/fs/xfs/xfs_log.h b/fs/xfs/xfs_log.h
index 384c6c4..65e054a 100644
--- a/fs/xfs/xfs_log.h
+++ b/fs/xfs/xfs_log.h
@@ -31,18 +31,49 @@ struct xfs_log_vec {
 #define XFS_LOG_VEC_ORDERED	(-1)
 
 static inline void *
-xlog_copy_iovec(struct xfs_log_iovec **vecp, uint type, void *data, int len)
+xlog_prepare_iovec(struct xfs_log_vec *lv, struct xfs_log_iovec **vecp,
+		uint type)
 {
 	struct xfs_log_iovec *vec = *vecp;
 
+	if (vec) {
+		ASSERT(vec - lv->lv_iovecp < lv->lv_niovecs);
+		vec++;
+	} else {
+		vec = &lv->lv_iovecp[0];
+	}
+
 	vec->i_type = type;
-	vec->i_addr = data;
-	vec->i_len = len;
+	vec->i_addr = lv->lv_buf + lv->lv_buf_len;
 
-	*vecp = vec + 1;
+	*vecp = vec;
 	return vec->i_addr;
 }
 
+static inline void
+xlog_finish_iovec(struct xfs_log_vec *lv, struct xfs_log_iovec *vec, int len)
+{
+	/*
+	 * We need to make sure the next buffer is naturally aligned for the
+	 * biggest basic data type we put into it.  We already accounted for
+	 * this when sizing the buffer.
+	 */
+	lv->lv_buf_len += round_up(len, sizeof(uint64_t));
+	vec->i_len = len;
+}
+
+static inline void *
+xlog_copy_iovec(struct xfs_log_vec *lv, struct xfs_log_iovec **vecp,
+		uint type, void *data, int len)
+{
+	void *buf;
+
+	buf = xlog_prepare_iovec(lv, vecp, type);
+	memcpy(buf, data, len);
+	xlog_finish_iovec(lv, *vecp, len);
+	return buf;
+}
+
 /*
  * Structure used to pass callback function and the function's argument
  * to the log manager.
diff --git a/fs/xfs/xfs_log_cil.c b/fs/xfs/xfs_log_cil.c
index 0a7a8ce..cdebd83 100644
--- a/fs/xfs/xfs_log_cil.c
+++ b/fs/xfs/xfs_log_cil.c
@@ -82,36 +82,6 @@ xlog_cil_init_post_recovery(
 								log->l_curr_block);
 }
 
-STATIC int
-xlog_cil_lv_item_format(
-	struct xfs_log_item	*lip,
-	struct xfs_log_vec	*lv)
-{
-	int	index;
-	char	*ptr;
-
-	/* format new vectors into array */
-	lip->li_ops->iop_format(lip, lv->lv_iovecp);
-
-	/* copy data into existing array */
-	ptr = lv->lv_buf;
-	for (index = 0; index < lv->lv_niovecs; index++) {
-		struct xfs_log_iovec *vec = &lv->lv_iovecp[index];
-
-		memcpy(ptr, vec->i_addr, vec->i_len);
-		vec->i_addr = ptr;
-		ptr += vec->i_len;
-	}
-
-	/*
-	 * some size calculations for log vectors over-estimate, so the caller
-	 * doesn't know the amount of space actually used by the item. Return
-	 * the byte count to the caller so they can check and store it
-	 * appropriately.
-	 */
-	return ptr - lv->lv_buf;
-}
-
 /*
  * Prepare the log item for insertion into the CIL. Calculate the difference in
  * log space and vectors it will consume, and if it is a new item pin it as
@@ -232,6 +202,13 @@ xlog_cil_insert_format_items(
 			nbytes = 0;
 		}
 
+		/*
+		 * We 64-bit align the length of each iovec so that the start
+		 * of the next one is naturally aligned.  We'll need to
+		 * account for that slack space here.
+		 */
+		nbytes += niovecs * sizeof(uint64_t);
+
 		/* grab the old item if it exists for reservation accounting */
 		old_lv = lip->li_lv;
 
@@ -272,9 +249,9 @@ xlog_cil_insert_format_items(
 		lv->lv_niovecs = niovecs;
 
 		/* The allocated data region lies beyond the iovec region */
+		lv->lv_buf_len = 0;
 		lv->lv_buf = (char *)lv + buf_size - nbytes;
-
-		lv->lv_buf_len = xlog_cil_lv_item_format(lip, lv);
+		lip->li_ops->iop_format(lip, lv);
 insert:
 		ASSERT(lv->lv_buf_len <= nbytes);
 		xfs_cil_prepare_item(log, lv, old_lv, diff_len, diff_iovecs);
diff --git a/fs/xfs/xfs_trans.h b/fs/xfs/xfs_trans.h
index 9b96d35..b5bc1ab 100644
--- a/fs/xfs/xfs_trans.h
+++ b/fs/xfs/xfs_trans.h
@@ -64,7 +64,7 @@ typedef struct xfs_log_item {
 
 struct xfs_item_ops {
 	void (*iop_size)(xfs_log_item_t *, int *, int *);
-	void (*iop_format)(xfs_log_item_t *, struct xfs_log_iovec *);
+	void (*iop_format)(xfs_log_item_t *, struct xfs_log_vec *);
 	void (*iop_pin)(xfs_log_item_t *);
 	void (*iop_unpin)(xfs_log_item_t *, int remove);
 	uint (*iop_push)(struct xfs_log_item *, struct list_head *);
-- 
1.7.10.4

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

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* Re: [PATCH 06/10 v2] xfs: format log items write directly into the linear CIL buffer
  2013-12-11 12:03   ` [PATCH 06/10 v2] " Christoph Hellwig
@ 2013-12-12  0:05     ` Dave Chinner
  2013-12-12 16:25     ` [PATCH 06/10 v3] " Christoph Hellwig
  1 sibling, 0 replies; 29+ messages in thread
From: Dave Chinner @ 2013-12-12  0:05 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: xfs

On Wed, Dec 11, 2013 at 04:03:41AM -0800, Christoph Hellwig wrote:
> Instead of setting up pointers to memory locations in iop_format which then
> get copied into the CIL linear buffer after return move the copy into
> the individual inode items.  This avoids the need to always have a memory
> block in the exact same layout that gets written into the log around, and
> allow the log items to be much more flexible in their in-memory layouts.
> 
> The only caveat is that we need to properly align the data for each
> iovec so that don't have structures misaligned in subsequent iovecs.

.....

> diff --git a/fs/xfs/xfs_log.h b/fs/xfs/xfs_log.h
> index 384c6c4..65e054a 100644
> --- a/fs/xfs/xfs_log.h
> +++ b/fs/xfs/xfs_log.h
> @@ -31,18 +31,49 @@ struct xfs_log_vec {
>  #define XFS_LOG_VEC_ORDERED	(-1)
>  
>  static inline void *
> -xlog_copy_iovec(struct xfs_log_iovec **vecp, uint type, void *data, int len)
> +xlog_prepare_iovec(struct xfs_log_vec *lv, struct xfs_log_iovec **vecp,
> +		uint type)
>  {
>  	struct xfs_log_iovec *vec = *vecp;
>  
> +	if (vec) {
> +		ASSERT(vec - lv->lv_iovecp < lv->lv_niovecs);
> +		vec++;
> +	} else {
> +		vec = &lv->lv_iovecp[0];
> +	}
> +
>  	vec->i_type = type;
> -	vec->i_addr = data;
> -	vec->i_len = len;
> +	vec->i_addr = lv->lv_buf + lv->lv_buf_len;
>  
> -	*vecp = vec + 1;
> +	*vecp = vec;
>  	return vec->i_addr;

Can you add an assert here like this:

	ASSERT(IS_ALIGNED(vec->i_addr, sizeof(uint64_t));

So we catch any situation where the alignment ends up wrong?

Otherwise, it looks good, so consider it:

Reviewed-by: Dave Chinner <dchinner@redhat.com>

-- 
Dave Chinner
david@fromorbit.com

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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH 06/10 v3] xfs: format log items write directly into the linear CIL buffer
  2013-12-11 12:03   ` [PATCH 06/10 v2] " Christoph Hellwig
  2013-12-12  0:05     ` Dave Chinner
@ 2013-12-12 16:25     ` Christoph Hellwig
  1 sibling, 0 replies; 29+ messages in thread
From: Christoph Hellwig @ 2013-12-12 16:25 UTC (permalink / raw)
  To: xfs

Instead of setting up pointers to memory locations in iop_format which then
get copied into the CIL linear buffer after return move the copy into
the individual inode items.  This avoids the need to always have a memory
block in the exact same layout that gets written into the log around, and
allow the log items to be much more flexible in their in-memory layouts.

The only caveat is that we need to properly align the data for each
iovec so that don't have structures misaligned in subsequent iovecs.

Note that all log item format routines now need to be careful to modify
the copy of the item that was placed into the CIL after calls to
xlog_copy_iovec instead of the in-memory copy.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Dave Chinner <dchinner@redhat.com>
---
 fs/xfs/xfs_buf_item.c     |   29 +++++++-------
 fs/xfs/xfs_dquot_item.c   |   19 +++++-----
 fs/xfs/xfs_extfree_item.c |   10 +++--
 fs/xfs/xfs_icreate_item.c |    5 ++-
 fs/xfs/xfs_inode_item.c   |   92 ++++++++++++++++++++++-----------------------
 fs/xfs/xfs_log.h          |   41 ++++++++++++++++++--
 fs/xfs/xfs_log_cil.c      |   41 +++++---------------
 fs/xfs/xfs_trans.h        |    2 +-
 8 files changed, 125 insertions(+), 114 deletions(-)

diff --git a/fs/xfs/xfs_buf_item.c b/fs/xfs/xfs_buf_item.c
index d49419d..7641173 100644
--- a/fs/xfs/xfs_buf_item.c
+++ b/fs/xfs/xfs_buf_item.c
@@ -184,6 +184,7 @@ xfs_buf_item_size(
 
 static inline void
 xfs_buf_item_copy_iovec(
+	struct xfs_log_vec	*lv,
 	struct xfs_log_iovec	**vecp,
 	struct xfs_buf		*bp,
 	uint			offset,
@@ -191,7 +192,7 @@ xfs_buf_item_copy_iovec(
 	uint			nbits)
 {
 	offset += first_bit * XFS_BLF_CHUNK;
-	xlog_copy_iovec(vecp, XLOG_REG_TYPE_BCHUNK,
+	xlog_copy_iovec(lv, vecp, XLOG_REG_TYPE_BCHUNK,
 			xfs_buf_offset(bp, offset),
 			nbits * XFS_BLF_CHUNK);
 }
@@ -211,13 +212,13 @@ xfs_buf_item_straddle(
 static void
 xfs_buf_item_format_segment(
 	struct xfs_buf_log_item	*bip,
+	struct xfs_log_vec	*lv,
 	struct xfs_log_iovec	**vecp,
 	uint			offset,
 	struct xfs_buf_log_format *blfp)
 {
 	struct xfs_buf	*bp = bip->bli_buf;
 	uint		base_size;
-	uint		nvecs;
 	int		first_bit;
 	int		last_bit;
 	int		next_bit;
@@ -233,18 +234,17 @@ xfs_buf_item_format_segment(
 	 */
 	base_size = xfs_buf_log_format_size(blfp);
 
-	nvecs = 0;
 	first_bit = xfs_next_bit(blfp->blf_data_map, blfp->blf_map_size, 0);
 	if (!(bip->bli_flags & XFS_BLI_STALE) && first_bit == -1) {
 		/*
 		 * If the map is not be dirty in the transaction, mark
 		 * the size as zero and do not advance the vector pointer.
 		 */
-		goto out;
+		return;
 	}
 
-	xlog_copy_iovec(vecp, XLOG_REG_TYPE_BFORMAT, blfp, base_size);
-	nvecs = 1;
+	blfp = xlog_copy_iovec(lv, vecp, XLOG_REG_TYPE_BFORMAT, blfp, base_size);
+	blfp->blf_size = 1;
 
 	if (bip->bli_flags & XFS_BLI_STALE) {
 		/*
@@ -254,7 +254,7 @@ xfs_buf_item_format_segment(
 		 */
 		trace_xfs_buf_item_format_stale(bip);
 		ASSERT(blfp->blf_flags & XFS_BLF_CANCEL);
-		goto out;
+		return;
 	}
 
 
@@ -280,15 +280,15 @@ xfs_buf_item_format_segment(
 		 * same set of bits so just keep counting and scanning.
 		 */
 		if (next_bit == -1) {
-			xfs_buf_item_copy_iovec(vecp, bp, offset,
+			xfs_buf_item_copy_iovec(lv, vecp, bp, offset,
 						first_bit, nbits);
-			nvecs++;
+			blfp->blf_size++;
 			break;
 		} else if (next_bit != last_bit + 1 ||
 		           xfs_buf_item_straddle(bp, offset, next_bit, last_bit)) {
-			xfs_buf_item_copy_iovec(vecp, bp, offset,
+			xfs_buf_item_copy_iovec(lv, vecp, bp, offset,
 						first_bit, nbits);
-			nvecs++;
+			blfp->blf_size++;
 			first_bit = next_bit;
 			last_bit = next_bit;
 			nbits = 1;
@@ -297,8 +297,6 @@ xfs_buf_item_format_segment(
 			nbits++;
 		}
 	}
-out:
-	blfp->blf_size = nvecs;
 }
 
 /*
@@ -310,10 +308,11 @@ out:
 STATIC void
 xfs_buf_item_format(
 	struct xfs_log_item	*lip,
-	struct xfs_log_iovec	*vecp)
+	struct xfs_log_vec	*lv)
 {
 	struct xfs_buf_log_item	*bip = BUF_ITEM(lip);
 	struct xfs_buf		*bp = bip->bli_buf;
+	struct xfs_log_iovec	*vecp = NULL;
 	uint			offset = 0;
 	int			i;
 
@@ -354,7 +353,7 @@ xfs_buf_item_format(
 	}
 
 	for (i = 0; i < bip->bli_format_count; i++) {
-		xfs_buf_item_format_segment(bip, &vecp, offset,
+		xfs_buf_item_format_segment(bip, lv, &vecp, offset,
 					    &bip->bli_formats[i]);
 		offset += bp->b_maps[i].bm_len;
 	}
diff --git a/fs/xfs/xfs_dquot_item.c b/fs/xfs/xfs_dquot_item.c
index ca354a8..946d588 100644
--- a/fs/xfs/xfs_dquot_item.c
+++ b/fs/xfs/xfs_dquot_item.c
@@ -57,18 +57,19 @@ xfs_qm_dquot_logitem_size(
 STATIC void
 xfs_qm_dquot_logitem_format(
 	struct xfs_log_item	*lip,
-	struct xfs_log_iovec	*vecp)
+	struct xfs_log_vec	*lv)
 {
 	struct xfs_dq_logitem	*qlip = DQUOT_ITEM(lip);
+	struct xfs_log_iovec	*vecp = NULL;
 
-	xlog_copy_iovec(&vecp, XLOG_REG_TYPE_QFORMAT,
+	qlip->qli_format.qlf_size = 2;
+
+	xlog_copy_iovec(lv, &vecp, XLOG_REG_TYPE_QFORMAT,
 			&qlip->qli_format,
 			sizeof(struct xfs_dq_logformat));
-	xlog_copy_iovec(&vecp, XLOG_REG_TYPE_DQUOT,
+	xlog_copy_iovec(lv, &vecp, XLOG_REG_TYPE_DQUOT,
 			&qlip->qli_dquot->q_core,
 			sizeof(struct xfs_disk_dquot));
-
-	qlip->qli_format.qlf_size = 2;
 }
 
 /*
@@ -302,17 +303,17 @@ xfs_qm_qoff_logitem_size(
 STATIC void
 xfs_qm_qoff_logitem_format(
 	struct xfs_log_item	*lip,
-	struct xfs_log_iovec	*vecp)
+	struct xfs_log_vec	*lv)
 {
 	struct xfs_qoff_logitem	*qflip = QOFF_ITEM(lip);
+	struct xfs_log_iovec	*vecp = NULL;
 
 	ASSERT(qflip->qql_format.qf_type == XFS_LI_QUOTAOFF);
+	qflip->qql_format.qf_size = 1;
 
-	xlog_copy_iovec(&vecp, XLOG_REG_TYPE_QUOTAOFF,
+	xlog_copy_iovec(lv, &vecp, XLOG_REG_TYPE_QUOTAOFF,
 			&qflip->qql_format,
 			sizeof(struct xfs_qoff_logitem));
-
-	qflip->qql_format.qf_size = 1;
 }
 
 /*
diff --git a/fs/xfs/xfs_extfree_item.c b/fs/xfs/xfs_extfree_item.c
index 08823ec..fb7a4c1 100644
--- a/fs/xfs/xfs_extfree_item.c
+++ b/fs/xfs/xfs_extfree_item.c
@@ -102,9 +102,10 @@ xfs_efi_item_size(
 STATIC void
 xfs_efi_item_format(
 	struct xfs_log_item	*lip,
-	struct xfs_log_iovec	*vecp)
+	struct xfs_log_vec	*lv)
 {
 	struct xfs_efi_log_item	*efip = EFI_ITEM(lip);
+	struct xfs_log_iovec	*vecp = NULL;
 
 	ASSERT(atomic_read(&efip->efi_next_extent) ==
 				efip->efi_format.efi_nextents);
@@ -112,7 +113,7 @@ xfs_efi_item_format(
 	efip->efi_format.efi_type = XFS_LI_EFI;
 	efip->efi_format.efi_size = 1;
 
-	xlog_copy_iovec(&vecp, XLOG_REG_TYPE_EFI_FORMAT,
+	xlog_copy_iovec(lv, &vecp, XLOG_REG_TYPE_EFI_FORMAT,
 			&efip->efi_format,
 			xfs_efi_item_sizeof(efip));
 }
@@ -368,16 +369,17 @@ xfs_efd_item_size(
 STATIC void
 xfs_efd_item_format(
 	struct xfs_log_item	*lip,
-	struct xfs_log_iovec	*vecp)
+	struct xfs_log_vec	*lv)
 {
 	struct xfs_efd_log_item	*efdp = EFD_ITEM(lip);
+	struct xfs_log_iovec	*vecp = NULL;
 
 	ASSERT(efdp->efd_next_extent == efdp->efd_format.efd_nextents);
 
 	efdp->efd_format.efd_type = XFS_LI_EFD;
 	efdp->efd_format.efd_size = 1;
 
-	xlog_copy_iovec(&vecp, XLOG_REG_TYPE_EFD_FORMAT,
+	xlog_copy_iovec(lv, &vecp, XLOG_REG_TYPE_EFD_FORMAT,
 			&efdp->efd_format,
 			xfs_efd_item_sizeof(efdp));
 }
diff --git a/fs/xfs/xfs_icreate_item.c b/fs/xfs/xfs_icreate_item.c
index 5751fa8..7e45492 100644
--- a/fs/xfs/xfs_icreate_item.c
+++ b/fs/xfs/xfs_icreate_item.c
@@ -59,11 +59,12 @@ xfs_icreate_item_size(
 STATIC void
 xfs_icreate_item_format(
 	struct xfs_log_item	*lip,
-	struct xfs_log_iovec	*vecp)
+	struct xfs_log_vec	*lv)
 {
 	struct xfs_icreate_item	*icp = ICR_ITEM(lip);
+	struct xfs_log_iovec	*vecp = NULL;
 
-	xlog_copy_iovec(&vecp, XLOG_REG_TYPE_ICREATE,
+	xlog_copy_iovec(lv, &vecp, XLOG_REG_TYPE_ICREATE,
 			&icp->ic_format,
 			sizeof(struct xfs_icreate_log));
 }
diff --git a/fs/xfs/xfs_inode_item.c b/fs/xfs/xfs_inode_item.c
index 73002db..35dd24a 100644
--- a/fs/xfs/xfs_inode_item.c
+++ b/fs/xfs/xfs_inode_item.c
@@ -163,6 +163,7 @@ xfs_inode_item_size(
 STATIC int
 xfs_inode_item_format_extents(
 	struct xfs_inode	*ip,
+	struct xfs_log_vec	*lv,
 	struct xfs_log_iovec	**vecp,
 	int			whichfork,
 	int			type)
@@ -177,7 +178,7 @@ xfs_inode_item_format_extents(
 		ip->i_itemp->ili_aextents_buf = ext_buffer;
 
 	len = xfs_iextents_copy(ip, ext_buffer, whichfork);
-	xlog_copy_iovec(vecp, type, ext_buffer, len);
+	xlog_copy_iovec(lv, vecp, type, ext_buffer, len);
 	return len;
 }
 
@@ -212,8 +213,9 @@ xfs_inode_item_format_v1_inode(
 STATIC void
 xfs_inode_item_format_data_fork(
 	struct xfs_inode_log_item *iip,
-	struct xfs_log_iovec	**vecp,
-	int			*nvecs)
+	struct xfs_inode_log_format *ilf,
+	struct xfs_log_vec	*lv,
+	struct xfs_log_iovec	**vecp)
 {
 	struct xfs_inode	*ip = iip->ili_inode;
 	size_t			data_bytes;
@@ -239,19 +241,19 @@ xfs_inode_item_format_data_fork(
 				 * extents, so just point to the
 				 * real extents array.
 				 */
-				xlog_copy_iovec(vecp, XLOG_REG_TYPE_IEXT,
+				xlog_copy_iovec(lv, vecp, XLOG_REG_TYPE_IEXT,
 						ip->i_df.if_u1.if_extents,
 						ip->i_df.if_bytes);
-				iip->ili_format.ilf_dsize = ip->i_df.if_bytes;
+				ilf->ilf_dsize = ip->i_df.if_bytes;
 			} else
 #endif
 			{
-				iip->ili_format.ilf_dsize =
-					xfs_inode_item_format_extents(ip, vecp,
+				ilf->ilf_dsize =
+					xfs_inode_item_format_extents(ip, lv, vecp,
 						XFS_DATA_FORK, XLOG_REG_TYPE_IEXT);
 				ASSERT(iip->ili_format.ilf_dsize <= ip->i_df.if_bytes);
 			}
-			(*nvecs)++;
+			ilf->ilf_size++;
 		} else {
 			iip->ili_fields &= ~XFS_ILOG_DEXT;
 		}
@@ -264,11 +266,11 @@ xfs_inode_item_format_data_fork(
 		if ((iip->ili_fields & XFS_ILOG_DBROOT) &&
 		    ip->i_df.if_broot_bytes > 0) {
 			ASSERT(ip->i_df.if_broot != NULL);
-			xlog_copy_iovec(vecp, XLOG_REG_TYPE_IBROOT,
+			xlog_copy_iovec(lv, vecp, XLOG_REG_TYPE_IBROOT,
 					ip->i_df.if_broot,
 					ip->i_df.if_broot_bytes);
-			(*nvecs)++;
-			iip->ili_format.ilf_dsize = ip->i_df.if_broot_bytes;
+			ilf->ilf_dsize = ip->i_df.if_broot_bytes;
+			ilf->ilf_size++;
 		} else {
 			ASSERT(!(iip->ili_fields &
 				 XFS_ILOG_DBROOT));
@@ -291,10 +293,10 @@ xfs_inode_item_format_data_fork(
 			       ip->i_df.if_real_bytes == data_bytes);
 			ASSERT(ip->i_df.if_u1.if_data != NULL);
 			ASSERT(ip->i_d.di_size > 0);
-			xlog_copy_iovec(vecp, XLOG_REG_TYPE_ILOCAL,
+			xlog_copy_iovec(lv, vecp, XLOG_REG_TYPE_ILOCAL,
 					ip->i_df.if_u1.if_data, data_bytes);
-			(*nvecs)++;
-			iip->ili_format.ilf_dsize = (unsigned)data_bytes;
+			ilf->ilf_dsize = (unsigned)data_bytes;
+			ilf->ilf_size++;
 		} else {
 			iip->ili_fields &= ~XFS_ILOG_DDATA;
 		}
@@ -303,19 +305,15 @@ xfs_inode_item_format_data_fork(
 		iip->ili_fields &=
 			~(XFS_ILOG_DDATA | XFS_ILOG_DBROOT |
 			  XFS_ILOG_DEXT | XFS_ILOG_UUID);
-		if (iip->ili_fields & XFS_ILOG_DEV) {
-			iip->ili_format.ilf_u.ilfu_rdev =
-				ip->i_df.if_u2.if_rdev;
-		}
+		if (iip->ili_fields & XFS_ILOG_DEV)
+			ilf->ilf_u.ilfu_rdev = ip->i_df.if_u2.if_rdev;
 		break;
 	case XFS_DINODE_FMT_UUID:
 		iip->ili_fields &=
 			~(XFS_ILOG_DDATA | XFS_ILOG_DBROOT |
 			  XFS_ILOG_DEXT | XFS_ILOG_DEV);
-		if (iip->ili_fields & XFS_ILOG_UUID) {
-			iip->ili_format.ilf_u.ilfu_uuid =
-				ip->i_df.if_u2.if_uuid;
-		}
+		if (iip->ili_fields & XFS_ILOG_UUID)
+			ilf->ilf_u.ilfu_uuid = ip->i_df.if_u2.if_uuid;
 		break;
 	default:
 		ASSERT(0);
@@ -326,8 +324,9 @@ xfs_inode_item_format_data_fork(
 STATIC void
 xfs_inode_item_format_attr_fork(
 	struct xfs_inode_log_item *iip,
-	struct xfs_log_iovec	**vecp,
-	int			*nvecs)
+	struct xfs_inode_log_format *ilf,
+	struct xfs_log_vec	*lv,
+	struct xfs_log_iovec	**vecp)
 {
 	struct xfs_inode	*ip = iip->ili_inode;
 	size_t			data_bytes;
@@ -348,17 +347,17 @@ xfs_inode_item_format_attr_fork(
 			 * There are not delayed allocation extents
 			 * for attributes, so just point at the array.
 			 */
-			xlog_copy_iovec(vecp, XLOG_REG_TYPE_IATTR_EXT,
+			xlog_copy_iovec(lv, vecp, XLOG_REG_TYPE_IATTR_EXT,
 					ip->i_afp->if_u1.if_extents,
 					ip->i_afp->if_bytes);
-			iip->ili_format.ilf_asize = ip->i_afp->if_bytes;
+			ilf->ilf_asize = ip->i_afp->if_bytes;
 #else
 			ASSERT(iip->ili_aextents_buf == NULL);
-			iip->ili_format.ilf_asize =
-				xfs_inode_item_format_extents(ip, vecp,
+			ilf->ilf_asize =
+				xfs_inode_item_format_extents(ip, lv, vecp,
 					XFS_ATTR_FORK, XLOG_REG_TYPE_IATTR_EXT);
 #endif
-			(*nvecs)++;
+			ilf->ilf_size++;
 		} else {
 			iip->ili_fields &= ~XFS_ILOG_AEXT;
 		}
@@ -371,11 +370,11 @@ xfs_inode_item_format_attr_fork(
 		    ip->i_afp->if_broot_bytes > 0) {
 			ASSERT(ip->i_afp->if_broot != NULL);
 
-			xlog_copy_iovec(vecp, XLOG_REG_TYPE_IATTR_BROOT,
+			xlog_copy_iovec(lv, vecp, XLOG_REG_TYPE_IATTR_BROOT,
 					ip->i_afp->if_broot,
 					ip->i_afp->if_broot_bytes);
-			(*nvecs)++;
-			iip->ili_format.ilf_asize = ip->i_afp->if_broot_bytes;
+			ilf->ilf_asize = ip->i_afp->if_broot_bytes;
+			ilf->ilf_size++;
 		} else {
 			iip->ili_fields &= ~XFS_ILOG_ABROOT;
 		}
@@ -395,11 +394,11 @@ xfs_inode_item_format_attr_fork(
 			ASSERT(ip->i_afp->if_real_bytes == 0 ||
 			       ip->i_afp->if_real_bytes == data_bytes);
 			ASSERT(ip->i_afp->if_u1.if_data != NULL);
-			xlog_copy_iovec(vecp, XLOG_REG_TYPE_IATTR_LOCAL,
+			xlog_copy_iovec(lv, vecp, XLOG_REG_TYPE_IATTR_LOCAL,
 					ip->i_afp->if_u1.if_data,
 					data_bytes);
-			(*nvecs)++;
-			iip->ili_format.ilf_asize = (unsigned)data_bytes;
+			ilf->ilf_asize = (unsigned)data_bytes;
+			ilf->ilf_size++;
 		} else {
 			iip->ili_fields &= ~XFS_ILOG_ADATA;
 		}
@@ -420,28 +419,28 @@ xfs_inode_item_format_attr_fork(
 STATIC void
 xfs_inode_item_format(
 	struct xfs_log_item	*lip,
-	struct xfs_log_iovec	*vecp)
+	struct xfs_log_vec	*lv)
 {
 	struct xfs_inode_log_item *iip = INODE_ITEM(lip);
 	struct xfs_inode	*ip = iip->ili_inode;
-	uint			nvecs;
+	struct xfs_inode_log_format *ilf;
+	struct xfs_log_iovec	*vecp = NULL;
 
-	xlog_copy_iovec(&vecp, XLOG_REG_TYPE_IFORMAT,
+	ilf = xlog_copy_iovec(lv, &vecp, XLOG_REG_TYPE_IFORMAT,
 			&iip->ili_format,
 			sizeof(struct xfs_inode_log_format));
-	nvecs = 1;
-
-	xlog_copy_iovec(&vecp, XLOG_REG_TYPE_ICORE,
-			&ip->i_d,
-			xfs_icdinode_size(ip->i_d.di_version));
-	nvecs++;
+	ilf->ilf_size = 1;
 
 	if (ip->i_d.di_version == 1)
 		xfs_inode_item_format_v1_inode(ip);
+	xlog_copy_iovec(lv, &vecp, XLOG_REG_TYPE_ICORE,
+			&ip->i_d,
+			xfs_icdinode_size(ip->i_d.di_version));
+	ilf->ilf_size++;
 
-	xfs_inode_item_format_data_fork(iip, &vecp, &nvecs);
+	xfs_inode_item_format_data_fork(iip, ilf, lv, &vecp);
 	if (XFS_IFORK_Q(ip)) {
-		xfs_inode_item_format_attr_fork(iip, &vecp, &nvecs);
+		xfs_inode_item_format_attr_fork(iip, ilf, lv, &vecp);
 	} else {
 		iip->ili_fields &=
 			~(XFS_ILOG_ADATA | XFS_ILOG_ABROOT | XFS_ILOG_AEXT);
@@ -455,7 +454,6 @@ xfs_inode_item_format(
 	 */
 	iip->ili_format.ilf_fields = XFS_ILOG_CORE |
 		(iip->ili_fields & ~XFS_ILOG_TIMESTAMP);
-	iip->ili_format.ilf_size = nvecs;
 }
 
 /*
diff --git a/fs/xfs/xfs_log.h b/fs/xfs/xfs_log.h
index 384c6c4..b0f4ef7 100644
--- a/fs/xfs/xfs_log.h
+++ b/fs/xfs/xfs_log.h
@@ -31,18 +31,51 @@ struct xfs_log_vec {
 #define XFS_LOG_VEC_ORDERED	(-1)
 
 static inline void *
-xlog_copy_iovec(struct xfs_log_iovec **vecp, uint type, void *data, int len)
+xlog_prepare_iovec(struct xfs_log_vec *lv, struct xfs_log_iovec **vecp,
+		uint type)
 {
 	struct xfs_log_iovec *vec = *vecp;
 
+	if (vec) {
+		ASSERT(vec - lv->lv_iovecp < lv->lv_niovecs);
+		vec++;
+	} else {
+		vec = &lv->lv_iovecp[0];
+	}
+
 	vec->i_type = type;
-	vec->i_addr = data;
-	vec->i_len = len;
+	vec->i_addr = lv->lv_buf + lv->lv_buf_len;
+
+	ASSERT(IS_ALIGNED((unsigned long)vec->i_addr, sizeof(uint64_t)));
 
-	*vecp = vec + 1;
+	*vecp = vec;
 	return vec->i_addr;
 }
 
+static inline void
+xlog_finish_iovec(struct xfs_log_vec *lv, struct xfs_log_iovec *vec, int len)
+{
+	/*
+	 * We need to make sure the next buffer is naturally aligned for the
+	 * biggest basic data type we put into it.  We already accounted for
+	 * this when sizing the buffer.
+	 */
+	lv->lv_buf_len += round_up(len, sizeof(uint64_t));
+	vec->i_len = len;
+}
+
+static inline void *
+xlog_copy_iovec(struct xfs_log_vec *lv, struct xfs_log_iovec **vecp,
+		uint type, void *data, int len)
+{
+	void *buf;
+
+	buf = xlog_prepare_iovec(lv, vecp, type);
+	memcpy(buf, data, len);
+	xlog_finish_iovec(lv, *vecp, len);
+	return buf;
+}
+
 /*
  * Structure used to pass callback function and the function's argument
  * to the log manager.
diff --git a/fs/xfs/xfs_log_cil.c b/fs/xfs/xfs_log_cil.c
index 0a7a8ce..cdebd83 100644
--- a/fs/xfs/xfs_log_cil.c
+++ b/fs/xfs/xfs_log_cil.c
@@ -82,36 +82,6 @@ xlog_cil_init_post_recovery(
 								log->l_curr_block);
 }
 
-STATIC int
-xlog_cil_lv_item_format(
-	struct xfs_log_item	*lip,
-	struct xfs_log_vec	*lv)
-{
-	int	index;
-	char	*ptr;
-
-	/* format new vectors into array */
-	lip->li_ops->iop_format(lip, lv->lv_iovecp);
-
-	/* copy data into existing array */
-	ptr = lv->lv_buf;
-	for (index = 0; index < lv->lv_niovecs; index++) {
-		struct xfs_log_iovec *vec = &lv->lv_iovecp[index];
-
-		memcpy(ptr, vec->i_addr, vec->i_len);
-		vec->i_addr = ptr;
-		ptr += vec->i_len;
-	}
-
-	/*
-	 * some size calculations for log vectors over-estimate, so the caller
-	 * doesn't know the amount of space actually used by the item. Return
-	 * the byte count to the caller so they can check and store it
-	 * appropriately.
-	 */
-	return ptr - lv->lv_buf;
-}
-
 /*
  * Prepare the log item for insertion into the CIL. Calculate the difference in
  * log space and vectors it will consume, and if it is a new item pin it as
@@ -232,6 +202,13 @@ xlog_cil_insert_format_items(
 			nbytes = 0;
 		}
 
+		/*
+		 * We 64-bit align the length of each iovec so that the start
+		 * of the next one is naturally aligned.  We'll need to
+		 * account for that slack space here.
+		 */
+		nbytes += niovecs * sizeof(uint64_t);
+
 		/* grab the old item if it exists for reservation accounting */
 		old_lv = lip->li_lv;
 
@@ -272,9 +249,9 @@ xlog_cil_insert_format_items(
 		lv->lv_niovecs = niovecs;
 
 		/* The allocated data region lies beyond the iovec region */
+		lv->lv_buf_len = 0;
 		lv->lv_buf = (char *)lv + buf_size - nbytes;
-
-		lv->lv_buf_len = xlog_cil_lv_item_format(lip, lv);
+		lip->li_ops->iop_format(lip, lv);
 insert:
 		ASSERT(lv->lv_buf_len <= nbytes);
 		xfs_cil_prepare_item(log, lv, old_lv, diff_len, diff_iovecs);
diff --git a/fs/xfs/xfs_trans.h b/fs/xfs/xfs_trans.h
index 9b96d35..b5bc1ab 100644
--- a/fs/xfs/xfs_trans.h
+++ b/fs/xfs/xfs_trans.h
@@ -64,7 +64,7 @@ typedef struct xfs_log_item {
 
 struct xfs_item_ops {
 	void (*iop_size)(xfs_log_item_t *, int *, int *);
-	void (*iop_format)(xfs_log_item_t *, struct xfs_log_iovec *);
+	void (*iop_format)(xfs_log_item_t *, struct xfs_log_vec *);
 	void (*iop_pin)(xfs_log_item_t *);
 	void (*iop_unpin)(xfs_log_item_t *, int remove);
 	uint (*iop_push)(struct xfs_log_item *, struct list_head *);
-- 
1.7.10.4

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

^ permalink raw reply related	[flat|nested] 29+ messages in thread

end of thread, other threads:[~2013-12-12 16:25 UTC | newest]

Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-11-29  8:39 [PATCH 00/10] decouple the in-memory from the on-disk log format V2 Christoph Hellwig
2013-11-29  8:39 ` [PATCH 01/10] xfs: remove duplicate code in xlog_cil_insert_format_items Christoph Hellwig
2013-12-03  0:58   ` Dave Chinner
2013-12-09 19:45   ` Ben Myers
2013-12-10 16:18     ` Christoph Hellwig
2013-11-29  8:39 ` [PATCH 02/10] xfs: refactor xfs_buf_item_format_segment Christoph Hellwig
2013-12-03  1:03   ` Dave Chinner
2013-11-29  8:39 ` [PATCH 03/10] xfs: refactor xfs_inode_item_size Christoph Hellwig
2013-12-03  1:06   ` Dave Chinner
2013-11-29  8:39 ` [PATCH 04/10] xfs: refactor xfs_inode_item_format Christoph Hellwig
2013-12-03  1:10   ` Dave Chinner
2013-11-29  8:39 ` [PATCH 05/10] xfs: introduce xlog_copy_iovec Christoph Hellwig
2013-12-03  1:21   ` Dave Chinner
2013-12-03  9:43     ` Christoph Hellwig
2013-11-29  8:39 ` [PATCH 06/10] xfs: format log items write directly into the linear CIL buffer Christoph Hellwig
2013-12-04  0:37   ` Dave Chinner
2013-12-09 19:00     ` Ben Myers
2013-12-10 16:12       ` Christoph Hellwig
2013-12-11 12:03   ` [PATCH 06/10 v2] " Christoph Hellwig
2013-12-12  0:05     ` Dave Chinner
2013-12-12 16:25     ` [PATCH 06/10 v3] " Christoph Hellwig
2013-11-29  8:39 ` [PATCH 07/10] xfs: format logged extents directly into the CIL Christoph Hellwig
2013-12-04  0:40   ` Dave Chinner
2013-11-29  8:39 ` [PATCH 08/10] xfs: remove the inode log format from the inode log item Christoph Hellwig
2013-12-04  0:44   ` Dave Chinner
2013-11-29  8:39 ` [PATCH 09/10] xfs: remove the dquot log format from the dquot " Christoph Hellwig
2013-12-04  0:47   ` Dave Chinner
2013-11-29  8:39 ` [PATCH 10/10] xfs: remove the quotaoff log format from the quotaoff " Christoph Hellwig
2013-12-04  0:49   ` Dave Chinner

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.