[PATCH 00/11] remove xfs_iomap

[PATCH 00/11] remove xfs_iomap

[Christoph Hellwig]

This series gets rid of the xfs_iomap structure which we use to
communicate between xfs_aops.c and xfs_iomap.c.  It's not a very
useful structure - we basically need block offsets above and below
it, but it's in terms of bytes.  Removing it and using the xfs_bmbt_irec
structure instead thus simplifies the code and reduces the stack footprint
of the writeback code.

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

[PATCH 01/11] xfs: limit xfs_imap_to_bmap to a single mapping

[Christoph Hellwig]

[-- Attachment #1: xfs-iomap-cleanup-1 --]
[-- Type: text/plain, Size: 3404 bytes --]

We only call xfs_iomap for single mappings anyway, so remove all code
dealing with multiple mappings from xfs_imap_to_bmap and add asserts
that we never get results that we do not expect.

Signed-off-by: Christoph Hellwig <hch@lst.de>

Index: xfs/fs/xfs/xfs_iomap.c
===================================================================
--- xfs.orig/fs/xfs/xfs_iomap.c	2010-04-28 14:06:53.000000000 +0200
+++ xfs/fs/xfs/xfs_iomap.c	2010-04-28 14:14:38.708003332 +0200
@@ -55,49 +55,41 @@
 #define XFS_STRAT_WRITE_IMAPS	2
 #define XFS_WRITE_IMAPS		XFS_BMAP_MAX_NMAP
 
-STATIC int
+STATIC void
 xfs_imap_to_bmap(
 	xfs_inode_t	*ip,
 	xfs_off_t	offset,
 	xfs_bmbt_irec_t *imap,
 	xfs_iomap_t	*iomapp,
 	int		imaps,			/* Number of imap entries */
-	int		iomaps,			/* Number of iomap entries */
 	int		flags)
 {
 	xfs_mount_t	*mp = ip->i_mount;
-	int		pbm;
 	xfs_fsblock_t	start_block;
 
-
-	for (pbm = 0; imaps && pbm < iomaps; imaps--, iomapp++, imap++, pbm++) {
-		iomapp->iomap_offset = XFS_FSB_TO_B(mp, imap->br_startoff);
-		iomapp->iomap_delta = offset - iomapp->iomap_offset;
-		iomapp->iomap_bsize = XFS_FSB_TO_B(mp, imap->br_blockcount);
-		iomapp->iomap_flags = flags;
-
-		if (XFS_IS_REALTIME_INODE(ip)) {
-			iomapp->iomap_flags |= IOMAP_REALTIME;
-			iomapp->iomap_target = mp->m_rtdev_targp;
-		} else {
-			iomapp->iomap_target = mp->m_ddev_targp;
-		}
-		start_block = imap->br_startblock;
-		if (start_block == HOLESTARTBLOCK) {
-			iomapp->iomap_bn = IOMAP_DADDR_NULL;
-			iomapp->iomap_flags |= IOMAP_HOLE;
-		} else if (start_block == DELAYSTARTBLOCK) {
-			iomapp->iomap_bn = IOMAP_DADDR_NULL;
-			iomapp->iomap_flags |= IOMAP_DELAY;
-		} else {
-			iomapp->iomap_bn = xfs_fsb_to_db(ip, start_block);
-			if (ISUNWRITTEN(imap))
-				iomapp->iomap_flags |= IOMAP_UNWRITTEN;
-		}
-
-		offset += iomapp->iomap_bsize - iomapp->iomap_delta;
+	iomapp->iomap_offset = XFS_FSB_TO_B(mp, imap->br_startoff);
+	iomapp->iomap_delta = offset - iomapp->iomap_offset;
+	iomapp->iomap_bsize = XFS_FSB_TO_B(mp, imap->br_blockcount);
+	iomapp->iomap_flags = flags;
+
+	if (XFS_IS_REALTIME_INODE(ip)) {
+		iomapp->iomap_flags |= IOMAP_REALTIME;
+		iomapp->iomap_target = mp->m_rtdev_targp;
+	} else {
+		iomapp->iomap_target = mp->m_ddev_targp;
+	}
+	start_block = imap->br_startblock;
+	if (start_block == HOLESTARTBLOCK) {
+		iomapp->iomap_bn = IOMAP_DADDR_NULL;
+		iomapp->iomap_flags |= IOMAP_HOLE;
+	} else if (start_block == DELAYSTARTBLOCK) {
+		iomapp->iomap_bn = IOMAP_DADDR_NULL;
+		iomapp->iomap_flags |= IOMAP_DELAY;
+	} else {
+		iomapp->iomap_bn = xfs_fsb_to_db(ip, start_block);
+		if (ISUNWRITTEN(imap))
+			iomapp->iomap_flags |= IOMAP_UNWRITTEN;
 	}
-	return pbm;	/* Return the number filled */
 }
 
 int
@@ -119,6 +111,7 @@ xfs_iomap(
 	int		iomap_flags = 0;
 
 	ASSERT((ip->i_d.di_mode & S_IFMT) == S_IFREG);
+	ASSERT(niomaps && *niomaps == 1);
 
 	if (XFS_FORCED_SHUTDOWN(mp))
 		return XFS_ERROR(EIO);
@@ -203,12 +196,11 @@ xfs_iomap(
 		break;
 	}
 
-	if (nimaps) {
-		*niomaps = xfs_imap_to_bmap(ip, offset, &imap,
-					    iomapp, nimaps, *niomaps, iomap_flags);
-	} else if (niomaps) {
-		*niomaps = 0;
-	}
+	ASSERT(nimaps <= 1);
+
+	if (nimaps)
+		xfs_imap_to_bmap(ip, offset, &imap, iomapp, nimaps, iomap_flags);
+	*niomaps = nimaps;
 
 out:
 	if (lockmode)

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

Re: [PATCH 01/11] xfs: limit xfs_imap_to_bmap to a single mapping

[Alex Elder]

On Wed, 2010-04-28 at 08:28 -0400, Christoph Hellwig wrote:
> plain text document attachment (xfs-iomap-cleanup-1)
> We only call xfs_iomap for single mappings anyway, so remove all code
> dealing with multiple mappings from xfs_imap_to_bmap and add asserts
> that we never get results that we do not expect.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks good.

Reviewed-by: Alex Elder <aelder@sgi.com>

> Index: xfs/fs/xfs/xfs_iomap.c
> ===================================================================
> --- xfs.orig/fs/xfs/xfs_iomap.c	2010-04-28 14:06:53.000000000 +0200
> +++ xfs/fs/xfs/xfs_iomap.c	2010-04-28 14:14:38.708003332 +0200

. . .

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

[PATCH 02/11] xfs: remove iomap_target

[Christoph Hellwig]

[-- Attachment #1: xfs-iomap-cleanup-2 --]
[-- Type: text/plain, Size: 5917 bytes --]

Instead of using the iomap_target field in struct xfs_iomap and the
IOMAP_REALTIME flag just use the already existing xfs_find_bdev_for_inode
helper.  There's some fallout as we need to pass the inode in a few more
places, which we also use to sanitize some calling conventions.

Signed-off-by: Christoph Hellwig <hch@lst.de>

Index: xfs/fs/xfs/linux-2.6/xfs_aops.c
===================================================================
--- xfs.orig/fs/xfs/linux-2.6/xfs_aops.c	2010-04-28 14:14:17.050004169 +0200
+++ xfs/fs/xfs/linux-2.6/xfs_aops.c	2010-04-28 14:14:43.975254833 +0200
@@ -102,8 +102,9 @@ xfs_count_page_state(
 
 STATIC struct block_device *
 xfs_find_bdev_for_inode(
-	struct xfs_inode	*ip)
+	struct inode		*inode)
 {
+	struct xfs_inode	*ip = XFS_I(inode);
 	struct xfs_mount	*mp = ip->i_mount;
 
 	if (XFS_IS_REALTIME_INODE(ip))
@@ -553,19 +554,19 @@ xfs_add_to_ioend(
 
 STATIC void
 xfs_map_buffer(
+	struct inode		*inode,
 	struct buffer_head	*bh,
 	xfs_iomap_t		*mp,
-	xfs_off_t		offset,
-	uint			block_bits)
+	xfs_off_t		offset)
 {
 	sector_t		bn;
 
 	ASSERT(mp->iomap_bn != IOMAP_DADDR_NULL);
 
-	bn = (mp->iomap_bn >> (block_bits - BBSHIFT)) +
-	      ((offset - mp->iomap_offset) >> block_bits);
+	bn = (mp->iomap_bn >> (inode->i_blkbits - BBSHIFT)) +
+	      ((offset - mp->iomap_offset) >> inode->i_blkbits);
 
-	ASSERT(bn || (mp->iomap_flags & IOMAP_REALTIME));
+	ASSERT(bn || XFS_IS_REALTIME_INODE(XFS_I(inode)));
 
 	bh->b_blocknr = bn;
 	set_buffer_mapped(bh);
@@ -573,17 +574,17 @@ xfs_map_buffer(
 
 STATIC void
 xfs_map_at_offset(
+	struct inode		*inode,
 	struct buffer_head	*bh,
-	loff_t			offset,
-	int			block_bits,
-	xfs_iomap_t		*iomapp)
+	xfs_iomap_t		*iomapp,
+	xfs_off_t		offset)
 {
 	ASSERT(!(iomapp->iomap_flags & IOMAP_HOLE));
 	ASSERT(!(iomapp->iomap_flags & IOMAP_DELAY));
 
 	lock_buffer(bh);
-	xfs_map_buffer(bh, iomapp, offset, block_bits);
-	bh->b_bdev = iomapp->iomap_target->bt_bdev;
+	xfs_map_buffer(inode, bh, iomapp, offset);
+	bh->b_bdev = xfs_find_bdev_for_inode(inode);
 	set_buffer_mapped(bh);
 	clear_buffer_delay(bh);
 	clear_buffer_unwritten(bh);
@@ -749,7 +750,6 @@ xfs_convert_page(
 	xfs_off_t		end_offset;
 	unsigned long		p_offset;
 	unsigned int		type;
-	int			bbits = inode->i_blkbits;
 	int			len, page_dirty;
 	int			count = 0, done = 0, uptodate = 1;
  	xfs_off_t		offset = page_offset(page);
@@ -813,7 +813,7 @@ xfs_convert_page(
 			ASSERT(!(mp->iomap_flags & IOMAP_HOLE));
 			ASSERT(!(mp->iomap_flags & IOMAP_DELAY));
 
-			xfs_map_at_offset(bh, offset, bbits, mp);
+			xfs_map_at_offset(inode, bh, mp, offset);
 			if (startio) {
 				xfs_add_to_ioend(inode, bh, offset,
 						type, ioendp, done);
@@ -1173,8 +1173,7 @@ xfs_page_state_convert(
 				iomap_valid = xfs_iomap_valid(&iomap, offset);
 			}
 			if (iomap_valid) {
-				xfs_map_at_offset(bh, offset,
-						inode->i_blkbits, &iomap);
+				xfs_map_at_offset(inode, bh, &iomap, offset);
 				if (startio) {
 					xfs_add_to_ioend(inode, bh, offset,
 							type, &ioend,
@@ -1472,10 +1471,8 @@ __xfs_get_blocks(
 		 * For unwritten extents do not report a disk address on
 		 * the read case (treat as if we're reading into a hole).
 		 */
-		if (create || !(iomap.iomap_flags & IOMAP_UNWRITTEN)) {
-			xfs_map_buffer(bh_result, &iomap, offset,
-				       inode->i_blkbits);
-		}
+		if (create || !(iomap.iomap_flags & IOMAP_UNWRITTEN))
+			xfs_map_buffer(inode, bh_result, &iomap, offset);
 		if (create && (iomap.iomap_flags & IOMAP_UNWRITTEN)) {
 			if (direct)
 				bh_result->b_private = inode;
@@ -1487,7 +1484,7 @@ __xfs_get_blocks(
 	 * If this is a realtime file, data may be on a different device.
 	 * to that pointed to from the buffer_head b_bdev currently.
 	 */
-	bh_result->b_bdev = iomap.iomap_target->bt_bdev;
+	bh_result->b_bdev = xfs_find_bdev_for_inode(inode);
 
 	/*
 	 * If we previously allocated a block out beyond eof and we are now
@@ -1611,7 +1608,7 @@ xfs_vm_direct_IO(
 	struct block_device *bdev;
 	ssize_t		ret;
 
-	bdev = xfs_find_bdev_for_inode(XFS_I(inode));
+	bdev = xfs_find_bdev_for_inode(inode);
 
 	iocb->private = xfs_alloc_ioend(inode, rw == WRITE ?
 					IOMAP_UNWRITTEN : IOMAP_READ);
Index: xfs/fs/xfs/xfs_iomap.c
===================================================================
--- xfs.orig/fs/xfs/xfs_iomap.c	2010-04-28 14:14:38.708003332 +0200
+++ xfs/fs/xfs/xfs_iomap.c	2010-04-28 14:14:43.976254414 +0200
@@ -72,12 +72,6 @@ xfs_imap_to_bmap(
 	iomapp->iomap_bsize = XFS_FSB_TO_B(mp, imap->br_blockcount);
 	iomapp->iomap_flags = flags;
 
-	if (XFS_IS_REALTIME_INODE(ip)) {
-		iomapp->iomap_flags |= IOMAP_REALTIME;
-		iomapp->iomap_target = mp->m_rtdev_targp;
-	} else {
-		iomapp->iomap_target = mp->m_ddev_targp;
-	}
 	start_block = imap->br_startblock;
 	if (start_block == HOLESTARTBLOCK) {
 		iomapp->iomap_bn = IOMAP_DADDR_NULL;
Index: xfs/fs/xfs/xfs_iomap.h
===================================================================
--- xfs.orig/fs/xfs/xfs_iomap.h	2010-04-28 14:14:17.430004169 +0200
+++ xfs/fs/xfs/xfs_iomap.h	2010-04-28 14:14:43.982253995 +0200
@@ -25,7 +25,6 @@ typedef enum {				/* iomap_flags values
 	IOMAP_READ =		0,	/* mapping for a read */
 	IOMAP_HOLE =		0x02,	/* mapping covers a hole  */
 	IOMAP_DELAY =		0x04,	/* mapping covers delalloc region  */
-	IOMAP_REALTIME =	0x10,	/* mapping on the realtime device  */
 	IOMAP_UNWRITTEN =	0x20,	/* mapping covers allocated */
 					/* but uninitialized file data  */
 	IOMAP_NEW =		0x40	/* just allocate */
@@ -71,7 +70,6 @@ typedef enum {
 
 typedef struct xfs_iomap {
 	xfs_daddr_t		iomap_bn;	/* first 512B blk of mapping */
-	xfs_buftarg_t		*iomap_target;
 	xfs_off_t		iomap_offset;	/* offset of mapping, bytes */
 	xfs_off_t		iomap_bsize;	/* size of mapping, bytes */
 	xfs_off_t		iomap_delta;	/* offset into mapping, bytes */

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

Re: [PATCH 02/11] xfs: remove iomap_target

[Alex Elder]

On Wed, 2010-04-28 at 08:28 -0400, Christoph Hellwig wrote:
> plain text document attachment (xfs-iomap-cleanup-2)
> Instead of using the iomap_target field in struct xfs_iomap and the
> IOMAP_REALTIME flag just use the already existing xfs_find_bdev_for_inode
> helper.  There's some fallout as we need to pass the inode in a few more
> places, which we also use to sanitize some calling conventions.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

There are other places the xfs_find_bdev_for_inode() helper ought
to be used also.

Looks good.

Reviewed-by: Alex Elder <aelder@sgi.com>


> Index: xfs/fs/xfs/linux-2.6/xfs_aops.c
> ===================================================================
. . .

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

[PATCH 03/11] xfs: remove iomap_delta

[Christoph Hellwig]

[-- Attachment #1: xfs-iomap-cleanup-3 --]
[-- Type: text/plain, Size: 2125 bytes --]

The iomap_delta field in struct xfs_iomap just contains the difference
between the offset passed to xfs_iomap and the iomap_offset.  Just calculate
it in the only caller that cares.


Signed-off-by: Christoph Hellwig <hch@lst.de>

Index: xfs/fs/xfs/linux-2.6/xfs_aops.c
===================================================================
--- xfs.orig/fs/xfs/linux-2.6/xfs_aops.c	2010-04-28 14:14:43.975254833 +0200
+++ xfs/fs/xfs/linux-2.6/xfs_aops.c	2010-04-28 14:14:47.332253927 +0200
@@ -1511,9 +1511,11 @@ __xfs_get_blocks(
 	}
 
 	if (direct || size > (1 << inode->i_blkbits)) {
-		ASSERT(iomap.iomap_bsize - iomap.iomap_delta > 0);
+		xfs_off_t iomap_delta = offset - iomap.iomap_offset;
+
+		ASSERT(iomap.iomap_bsize - iomap_delta > 0);
 		offset = min_t(xfs_off_t,
-				iomap.iomap_bsize - iomap.iomap_delta, size);
+				iomap.iomap_bsize - iomap_delta, size);
 		bh_result->b_size = (ssize_t)min_t(xfs_off_t, LONG_MAX, offset);
 	}
 
Index: xfs/fs/xfs/xfs_iomap.c
===================================================================
--- xfs.orig/fs/xfs/xfs_iomap.c	2010-04-28 14:14:43.976254414 +0200
+++ xfs/fs/xfs/xfs_iomap.c	2010-04-28 14:14:47.333254206 +0200
@@ -68,7 +68,6 @@ xfs_imap_to_bmap(
 	xfs_fsblock_t	start_block;
 
 	iomapp->iomap_offset = XFS_FSB_TO_B(mp, imap->br_startoff);
-	iomapp->iomap_delta = offset - iomapp->iomap_offset;
 	iomapp->iomap_bsize = XFS_FSB_TO_B(mp, imap->br_blockcount);
 	iomapp->iomap_flags = flags;
 
Index: xfs/fs/xfs/xfs_iomap.h
===================================================================
--- xfs.orig/fs/xfs/xfs_iomap.h	2010-04-28 14:14:43.982253995 +0200
+++ xfs/fs/xfs/xfs_iomap.h	2010-04-28 14:14:47.337253577 +0200
@@ -72,7 +72,6 @@ typedef struct xfs_iomap {
 	xfs_daddr_t		iomap_bn;	/* first 512B blk of mapping */
 	xfs_off_t		iomap_offset;	/* offset of mapping, bytes */
 	xfs_off_t		iomap_bsize;	/* size of mapping, bytes */
-	xfs_off_t		iomap_delta;	/* offset into mapping, bytes */
 	iomap_flags_t		iomap_flags;
 } xfs_iomap_t;
 

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

Re: [PATCH 03/11] xfs: remove iomap_delta

[Alex Elder]

On Wed, 2010-04-28 at 08:28 -0400, Christoph Hellwig wrote:
> plain text document attachment (xfs-iomap-cleanup-3)
> The iomap_delta field in struct xfs_iomap just contains the difference
> between the offset passed to xfs_iomap and the iomap_offset.  Just calculate
> it in the only caller that cares.
> 
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks good.

Reviewed-by: Alex Elder <aelder@sgi.com>

> Index: xfs/fs/xfs/linux-2.6/xfs_aops.c
> ===================================================================
. . .

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

[PATCH 04/11] xfs: report iomap_offset and iomap_bsize in block base

[Christoph Hellwig]

[-- Attachment #1: xfs-iomap-cleanup-4 --]
[-- Type: text/plain, Size: 4471 bytes --]

Report the iomap_offset and iomap_bsize fields of struct xfs_iomap
in terms of fsblocks instead of in terms of bytes.  Shift the byte
conversions into the callers temporarily, but they will disappear
or get cleaned up later.

Signed-off-by: Christoph Hellwig <hch@lst.de>

Index: xfs/fs/xfs/linux-2.6/xfs_aops.c
===================================================================
--- xfs.orig/fs/xfs/linux-2.6/xfs_aops.c	2010-04-27 17:07:53.000000000 +0200
+++ xfs/fs/xfs/linux-2.6/xfs_aops.c	2010-04-27 17:13:15.434005637 +0200
@@ -319,11 +319,16 @@ xfs_map_blocks(
 
 STATIC int
 xfs_iomap_valid(
+	struct inode		*inode,
 	xfs_iomap_t		*iomapp,
 	loff_t			offset)
 {
-	return offset >= iomapp->iomap_offset &&
-		offset < iomapp->iomap_offset + iomapp->iomap_bsize;
+	struct xfs_mount	*mp = XFS_I(inode)->i_mount;
+	xfs_off_t		iomap_offset = XFS_FSB_TO_B(mp, iomapp->iomap_offset);
+	xfs_off_t		iomap_bsize = XFS_FSB_TO_B(mp, iomapp->iomap_bsize);
+
+	return offset >= iomap_offset &&
+		offset < iomap_offset + iomap_bsize;
 }
 
 /*
@@ -560,11 +565,13 @@ xfs_map_buffer(
 	xfs_off_t		offset)
 {
 	sector_t		bn;
+	struct xfs_mount	*m = XFS_I(inode)->i_mount;
+	xfs_off_t		iomap_offset = XFS_FSB_TO_B(m, mp->iomap_offset);
 
 	ASSERT(mp->iomap_bn != IOMAP_DADDR_NULL);
 
 	bn = (mp->iomap_bn >> (inode->i_blkbits - BBSHIFT)) +
-	      ((offset - mp->iomap_offset) >> inode->i_blkbits);
+	      ((offset - iomap_offset) >> inode->i_blkbits);
 
 	ASSERT(bn || XFS_IS_REALTIME_INODE(XFS_I(inode)));
 
@@ -805,7 +812,7 @@ xfs_convert_page(
 			else
 				type = IOMAP_DELAY;
 
-			if (!xfs_iomap_valid(mp, offset)) {
+			if (!xfs_iomap_valid(inode, mp, offset)) {
 				done = 1;
 				continue;
 			}
@@ -1115,7 +1122,7 @@ xfs_page_state_convert(
 		}
 
 		if (iomap_valid)
-			iomap_valid = xfs_iomap_valid(&iomap, offset);
+			iomap_valid = xfs_iomap_valid(inode, &iomap, offset);
 
 		/*
 		 * First case, map an unwritten extent and prepare for
@@ -1170,7 +1177,7 @@ xfs_page_state_convert(
 						&iomap, flags);
 				if (err)
 					goto error;
-				iomap_valid = xfs_iomap_valid(&iomap, offset);
+				iomap_valid = xfs_iomap_valid(inode, &iomap, offset);
 			}
 			if (iomap_valid) {
 				xfs_map_at_offset(inode, bh, &iomap, offset);
@@ -1200,7 +1207,7 @@ xfs_page_state_convert(
 						&iomap, flags);
 				if (err)
 					goto error;
-				iomap_valid = xfs_iomap_valid(&iomap, offset);
+				iomap_valid = xfs_iomap_valid(inode, &iomap, offset);
 			}
 
 			/*
@@ -1240,7 +1247,11 @@ xfs_page_state_convert(
 		xfs_start_page_writeback(page, 1, count);
 
 	if (ioend && iomap_valid) {
-		offset = (iomap.iomap_offset + iomap.iomap_bsize - 1) >>
+		struct xfs_mount	*m = XFS_I(inode)->i_mount;
+		xfs_off_t		iomap_offset = XFS_FSB_TO_B(m, iomap.iomap_offset);
+		xfs_off_t		iomap_bsize = XFS_FSB_TO_B(m, iomap.iomap_bsize);
+
+		offset = (iomap_offset + iomap_bsize - 1) >>
 					PAGE_CACHE_SHIFT;
 		tlast = min_t(pgoff_t, offset, last_index);
 		xfs_cluster_write(inode, page->index + 1, &iomap, &ioend,
@@ -1511,11 +1522,14 @@ __xfs_get_blocks(
 	}
 
 	if (direct || size > (1 << inode->i_blkbits)) {
-		xfs_off_t iomap_delta = offset - iomap.iomap_offset;
+		struct xfs_mount	*mp = XFS_I(inode)->i_mount;
+		xfs_off_t		iomap_offset = XFS_FSB_TO_B(mp, iomap.iomap_offset);
+		xfs_off_t		iomap_delta = offset - iomap_offset;
+		xfs_off_t		iomap_bsize = XFS_FSB_TO_B(mp, iomap.iomap_bsize);
 
-		ASSERT(iomap.iomap_bsize - iomap_delta > 0);
+		ASSERT(iomap_bsize - iomap_delta > 0);
 		offset = min_t(xfs_off_t,
-				iomap.iomap_bsize - iomap_delta, size);
+				iomap_bsize - iomap_delta, size);
 		bh_result->b_size = (ssize_t)min_t(xfs_off_t, LONG_MAX, offset);
 	}
 
Index: xfs/fs/xfs/xfs_iomap.c
===================================================================
--- xfs.orig/fs/xfs/xfs_iomap.c	2010-04-27 17:08:03.000000000 +0200
+++ xfs/fs/xfs/xfs_iomap.c	2010-04-27 17:12:58.858068081 +0200
@@ -64,11 +64,10 @@ xfs_imap_to_bmap(
 	int		imaps,			/* Number of imap entries */
 	int		flags)
 {
-	xfs_mount_t	*mp = ip->i_mount;
 	xfs_fsblock_t	start_block;
 
-	iomapp->iomap_offset = XFS_FSB_TO_B(mp, imap->br_startoff);
-	iomapp->iomap_bsize = XFS_FSB_TO_B(mp, imap->br_blockcount);
+	iomapp->iomap_offset = imap->br_startoff;
+	iomapp->iomap_bsize = imap->br_blockcount;
 	iomapp->iomap_flags = flags;
 
 	start_block = imap->br_startblock;

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

Re: [PATCH 04/11] xfs: report iomap_offset and iomap_bsize in block base

[Alex Elder]

On Wed, 2010-04-28 at 08:28 -0400, Christoph Hellwig wrote:
> plain text document attachment (xfs-iomap-cleanup-4)
> Report the iomap_offset and iomap_bsize fields of struct xfs_iomap
> in terms of fsblocks instead of in terms of bytes.  Shift the byte
> conversions into the callers temporarily, but they will disappear
> or get cleaned up later.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks good.

Reviewed-by: Alex Elder <aelder@sgi.com>


> Index: xfs/fs/xfs/linux-2.6/xfs_aops.c
> ===================================================================
> --- xfs.orig/fs/xfs/linux-2.6/xfs_aops.c	2010-04-27 17:07:53.000000000 +0200
> +++ xfs/fs/xfs/linux-2.6/xfs_aops.c	2010-04-27 17:13:15.434005637 +0200
. . .


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

[PATCH 05/11] xfs: report iomap_bn in block base

[Christoph Hellwig]

[-- Attachment #1: xfs-iomap-cleanup-5 --]
[-- Type: text/plain, Size: 4302 bytes --]

Report the iomap_bn field of struct xfs_iomap in terms of filesystem blocks
instead of in terms of bytes.  Shift the byte conversions into the caller,
and replace the IOMAP_DELAY and IOMAP_HOLE flag checks with checks for
HOLESTARTBLOCK and DELAYSTARTBLOCK.


Signed-off-by: Christoph Hellwig <hch@lst.de>

Index: xfs/fs/xfs/linux-2.6/xfs_aops.c
===================================================================
--- xfs.orig/fs/xfs/linux-2.6/xfs_aops.c	2010-04-27 17:39:18.329254483 +0200
+++ xfs/fs/xfs/linux-2.6/xfs_aops.c	2010-04-27 17:39:20.699003542 +0200
@@ -567,10 +567,12 @@ xfs_map_buffer(
 	sector_t		bn;
 	struct xfs_mount	*m = XFS_I(inode)->i_mount;
 	xfs_off_t		iomap_offset = XFS_FSB_TO_B(m, mp->iomap_offset);
+	xfs_daddr_t		iomap_bn = xfs_fsb_to_db(XFS_I(inode), mp->iomap_bn);
 
-	ASSERT(mp->iomap_bn != IOMAP_DADDR_NULL);
+	ASSERT(mp->iomap_bn != HOLESTARTBLOCK);
+	ASSERT(mp->iomap_bn != DELAYSTARTBLOCK);
 
-	bn = (mp->iomap_bn >> (inode->i_blkbits - BBSHIFT)) +
+	bn = (iomap_bn >> (inode->i_blkbits - BBSHIFT)) +
 	      ((offset - iomap_offset) >> inode->i_blkbits);
 
 	ASSERT(bn || XFS_IS_REALTIME_INODE(XFS_I(inode)));
@@ -586,8 +588,8 @@ xfs_map_at_offset(
 	xfs_iomap_t		*iomapp,
 	xfs_off_t		offset)
 {
-	ASSERT(!(iomapp->iomap_flags & IOMAP_HOLE));
-	ASSERT(!(iomapp->iomap_flags & IOMAP_DELAY));
+	ASSERT(iomapp->iomap_bn != HOLESTARTBLOCK);
+	ASSERT(iomapp->iomap_bn != DELAYSTARTBLOCK);
 
 	lock_buffer(bh);
 	xfs_map_buffer(inode, bh, iomapp, offset);
@@ -817,8 +819,8 @@ xfs_convert_page(
 				continue;
 			}
 
-			ASSERT(!(mp->iomap_flags & IOMAP_HOLE));
-			ASSERT(!(mp->iomap_flags & IOMAP_DELAY));
+			ASSERT(mp->iomap_bn != HOLESTARTBLOCK);
+			ASSERT(mp->iomap_bn != DELAYSTARTBLOCK);
 
 			xfs_map_at_offset(inode, bh, mp, offset);
 			if (startio) {
@@ -1477,7 +1479,8 @@ __xfs_get_blocks(
 	if (niomap == 0)
 		return 0;
 
-	if (iomap.iomap_bn != IOMAP_DADDR_NULL) {
+	if (iomap.iomap_bn != HOLESTARTBLOCK &&
+	    iomap.iomap_bn != DELAYSTARTBLOCK) {
 		/*
 		 * For unwritten extents do not report a disk address on
 		 * the read case (treat as if we're reading into a hole).
@@ -1512,7 +1515,7 @@ __xfs_get_blocks(
 	     (iomap.iomap_flags & (IOMAP_NEW|IOMAP_UNWRITTEN))))
 		set_buffer_new(bh_result);
 
-	if (iomap.iomap_flags & IOMAP_DELAY) {
+	if (iomap.iomap_bn == DELAYSTARTBLOCK) {
 		BUG_ON(direct);
 		if (create) {
 			set_buffer_uptodate(bh_result);
Index: xfs/fs/xfs/xfs_iomap.c
===================================================================
--- xfs.orig/fs/xfs/xfs_iomap.c	2010-04-27 17:39:18.313003820 +0200
+++ xfs/fs/xfs/xfs_iomap.c	2010-04-27 17:39:20.700004101 +0200
@@ -64,24 +64,15 @@ xfs_imap_to_bmap(
 	int		imaps,			/* Number of imap entries */
 	int		flags)
 {
-	xfs_fsblock_t	start_block;
-
 	iomapp->iomap_offset = imap->br_startoff;
 	iomapp->iomap_bsize = imap->br_blockcount;
 	iomapp->iomap_flags = flags;
+	iomapp->iomap_bn = imap->br_startblock;
 
-	start_block = imap->br_startblock;
-	if (start_block == HOLESTARTBLOCK) {
-		iomapp->iomap_bn = IOMAP_DADDR_NULL;
-		iomapp->iomap_flags |= IOMAP_HOLE;
-	} else if (start_block == DELAYSTARTBLOCK) {
-		iomapp->iomap_bn = IOMAP_DADDR_NULL;
-		iomapp->iomap_flags |= IOMAP_DELAY;
-	} else {
-		iomapp->iomap_bn = xfs_fsb_to_db(ip, start_block);
-		if (ISUNWRITTEN(imap))
-			iomapp->iomap_flags |= IOMAP_UNWRITTEN;
-	}
+	if (imap->br_startblock != HOLESTARTBLOCK &&
+	    imap->br_startblock != DELAYSTARTBLOCK &&
+	    ISUNWRITTEN(imap))
+		iomapp->iomap_flags |= IOMAP_UNWRITTEN;
 }
 
 int
Index: xfs/fs/xfs/xfs_iomap.h
===================================================================
--- xfs.orig/fs/xfs/xfs_iomap.h	2010-04-27 17:39:18.339273969 +0200
+++ xfs/fs/xfs/xfs_iomap.h	2010-04-27 17:39:27.783067167 +0200
@@ -18,12 +18,8 @@
 #ifndef __XFS_IOMAP_H__
 #define __XFS_IOMAP_H__
 
-#define IOMAP_DADDR_NULL ((xfs_daddr_t) (-1LL))
-
-
 typedef enum {				/* iomap_flags values */
 	IOMAP_READ =		0,	/* mapping for a read */
-	IOMAP_HOLE =		0x02,	/* mapping covers a hole  */
 	IOMAP_DELAY =		0x04,	/* mapping covers delalloc region  */
 	IOMAP_UNWRITTEN =	0x20,	/* mapping covers allocated */
 					/* but uninitialized file data  */

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

Re: [PATCH 05/11] xfs: report iomap_bn in block base

[Alex Elder]

On Wed, 2010-04-28 at 08:28 -0400, Christoph Hellwig wrote:
> plain text document attachment (xfs-iomap-cleanup-5)
> Report the iomap_bn field of struct xfs_iomap in terms of filesystem blocks
> instead of in terms of bytes.  Shift the byte conversions into the caller,
> and replace the IOMAP_DELAY and IOMAP_HOLE flag checks with checks for
> HOLESTARTBLOCK and DELAYSTARTBLOCK.
> 
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks good, although iomap_bn was previously a disk address,
not a byte offset (as you suggest in your summary).  I can
fix it if you like.

Reviewed-by: Alex Elder <aelder@sgi.com>


> Index: xfs/fs/xfs/linux-2.6/xfs_aops.c
> ===================================================================
. . .

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

[PATCH 06/11] xfs: kill struct xfs_iomap

[Christoph Hellwig]

[-- Attachment #1: xfs-iomap-cleanup-6 --]
[-- Type: text/plain, Size: 13960 bytes --]

Now that struct xfs_iomap contains exactly the same units as
struct xfs_bmbt_irec we can just use the latter directly in the aops
code.  Replace the missing IOMAP_NEW flag with a new boolean output
parameter to xfs_iomap.

Signed-off-by: Christoph Hellwig <hch@lst.de>

Index: xfs/fs/xfs/linux-2.6/xfs_aops.c
===================================================================
--- xfs.orig/fs/xfs/linux-2.6/xfs_aops.c	2010-04-27 17:39:20.699003542 +0200
+++ xfs/fs/xfs/linux-2.6/xfs_aops.c	2010-04-27 18:12:14.223025612 +0200
@@ -309,23 +309,24 @@ xfs_map_blocks(
 	struct inode		*inode,
 	loff_t			offset,
 	ssize_t			count,
-	xfs_iomap_t		*mapp,
+	struct xfs_bmbt_irec	*imap,
 	int			flags)
 {
 	int			nmaps = 1;
+	int			new = 0;
 
-	return -xfs_iomap(XFS_I(inode), offset, count, flags, mapp, &nmaps);
+	return -xfs_iomap(XFS_I(inode), offset, count, flags, imap, &nmaps, &new);
 }
 
 STATIC int
 xfs_iomap_valid(
 	struct inode		*inode,
-	xfs_iomap_t		*iomapp,
+	struct xfs_bmbt_irec	*imap,
 	loff_t			offset)
 {
 	struct xfs_mount	*mp = XFS_I(inode)->i_mount;
-	xfs_off_t		iomap_offset = XFS_FSB_TO_B(mp, iomapp->iomap_offset);
-	xfs_off_t		iomap_bsize = XFS_FSB_TO_B(mp, iomapp->iomap_bsize);
+	xfs_off_t		iomap_offset = XFS_FSB_TO_B(mp, imap->br_startoff);
+	xfs_off_t		iomap_bsize = XFS_FSB_TO_B(mp, imap->br_blockcount);
 
 	return offset >= iomap_offset &&
 		offset < iomap_offset + iomap_bsize;
@@ -561,16 +562,16 @@ STATIC void
 xfs_map_buffer(
 	struct inode		*inode,
 	struct buffer_head	*bh,
-	xfs_iomap_t		*mp,
+	struct xfs_bmbt_irec	*imap,
 	xfs_off_t		offset)
 {
 	sector_t		bn;
 	struct xfs_mount	*m = XFS_I(inode)->i_mount;
-	xfs_off_t		iomap_offset = XFS_FSB_TO_B(m, mp->iomap_offset);
-	xfs_daddr_t		iomap_bn = xfs_fsb_to_db(XFS_I(inode), mp->iomap_bn);
+	xfs_off_t		iomap_offset = XFS_FSB_TO_B(m, imap->br_startoff);
+	xfs_daddr_t		iomap_bn = xfs_fsb_to_db(XFS_I(inode), imap->br_startblock);
 
-	ASSERT(mp->iomap_bn != HOLESTARTBLOCK);
-	ASSERT(mp->iomap_bn != DELAYSTARTBLOCK);
+	ASSERT(imap->br_startblock != HOLESTARTBLOCK);
+	ASSERT(imap->br_startblock != DELAYSTARTBLOCK);
 
 	bn = (iomap_bn >> (inode->i_blkbits - BBSHIFT)) +
 	      ((offset - iomap_offset) >> inode->i_blkbits);
@@ -585,14 +586,14 @@ STATIC void
 xfs_map_at_offset(
 	struct inode		*inode,
 	struct buffer_head	*bh,
-	xfs_iomap_t		*iomapp,
+	struct xfs_bmbt_irec	*imap,
 	xfs_off_t		offset)
 {
-	ASSERT(iomapp->iomap_bn != HOLESTARTBLOCK);
-	ASSERT(iomapp->iomap_bn != DELAYSTARTBLOCK);
+	ASSERT(imap->br_startblock != HOLESTARTBLOCK);
+	ASSERT(imap->br_startblock != DELAYSTARTBLOCK);
 
 	lock_buffer(bh);
-	xfs_map_buffer(inode, bh, iomapp, offset);
+	xfs_map_buffer(inode, bh, imap, offset);
 	bh->b_bdev = xfs_find_bdev_for_inode(inode);
 	set_buffer_mapped(bh);
 	clear_buffer_delay(bh);
@@ -749,7 +750,7 @@ xfs_convert_page(
 	struct inode		*inode,
 	struct page		*page,
 	loff_t			tindex,
-	xfs_iomap_t		*mp,
+	struct xfs_bmbt_irec	*imap,
 	xfs_ioend_t		**ioendp,
 	struct writeback_control *wbc,
 	int			startio,
@@ -814,15 +815,15 @@ xfs_convert_page(
 			else
 				type = IOMAP_DELAY;
 
-			if (!xfs_iomap_valid(inode, mp, offset)) {
+			if (!xfs_iomap_valid(inode, imap, offset)) {
 				done = 1;
 				continue;
 			}
 
-			ASSERT(mp->iomap_bn != HOLESTARTBLOCK);
-			ASSERT(mp->iomap_bn != DELAYSTARTBLOCK);
+			ASSERT(imap->br_startblock != HOLESTARTBLOCK);
+			ASSERT(imap->br_startblock != DELAYSTARTBLOCK);
 
-			xfs_map_at_offset(inode, bh, mp, offset);
+			xfs_map_at_offset(inode, bh, imap, offset);
 			if (startio) {
 				xfs_add_to_ioend(inode, bh, offset,
 						type, ioendp, done);
@@ -874,7 +875,7 @@ STATIC void
 xfs_cluster_write(
 	struct inode		*inode,
 	pgoff_t			tindex,
-	xfs_iomap_t		*iomapp,
+	struct xfs_bmbt_irec	*imap,
 	xfs_ioend_t		**ioendp,
 	struct writeback_control *wbc,
 	int			startio,
@@ -893,7 +894,7 @@ xfs_cluster_write(
 
 		for (i = 0; i < pagevec_count(&pvec); i++) {
 			done = xfs_convert_page(inode, pvec.pages[i], tindex++,
-					iomapp, ioendp, wbc, startio, all_bh);
+					imap, ioendp, wbc, startio, all_bh);
 			if (done)
 				break;
 		}
@@ -1050,7 +1051,7 @@ xfs_page_state_convert(
 	int		unmapped) /* also implies page uptodate */
 {
 	struct buffer_head	*bh, *head;
-	xfs_iomap_t		iomap;
+	struct xfs_bmbt_irec	imap;
 	xfs_ioend_t		*ioend = NULL, *iohead = NULL;
 	loff_t			offset;
 	unsigned long           p_offset = 0;
@@ -1124,7 +1125,7 @@ xfs_page_state_convert(
 		}
 
 		if (iomap_valid)
-			iomap_valid = xfs_iomap_valid(inode, &iomap, offset);
+			iomap_valid = xfs_iomap_valid(inode, &imap, offset);
 
 		/*
 		 * First case, map an unwritten extent and prepare for
@@ -1176,13 +1177,13 @@ xfs_page_state_convert(
 				}
 
 				err = xfs_map_blocks(inode, offset, size,
-						&iomap, flags);
+						&imap, flags);
 				if (err)
 					goto error;
-				iomap_valid = xfs_iomap_valid(inode, &iomap, offset);
+				iomap_valid = xfs_iomap_valid(inode, &imap, offset);
 			}
 			if (iomap_valid) {
-				xfs_map_at_offset(inode, bh, &iomap, offset);
+				xfs_map_at_offset(inode, bh, &imap, offset);
 				if (startio) {
 					xfs_add_to_ioend(inode, bh, offset,
 							type, &ioend,
@@ -1206,10 +1207,10 @@ xfs_page_state_convert(
 				size = xfs_probe_cluster(inode, page, bh,
 								head, 1);
 				err = xfs_map_blocks(inode, offset, size,
-						&iomap, flags);
+						&imap, flags);
 				if (err)
 					goto error;
-				iomap_valid = xfs_iomap_valid(inode, &iomap, offset);
+				iomap_valid = xfs_iomap_valid(inode, &imap, offset);
 			}
 
 			/*
@@ -1250,13 +1251,13 @@ xfs_page_state_convert(
 
 	if (ioend && iomap_valid) {
 		struct xfs_mount	*m = XFS_I(inode)->i_mount;
-		xfs_off_t		iomap_offset = XFS_FSB_TO_B(m, iomap.iomap_offset);
-		xfs_off_t		iomap_bsize = XFS_FSB_TO_B(m, iomap.iomap_bsize);
+		xfs_off_t		iomap_offset = XFS_FSB_TO_B(m, imap.br_startoff);
+		xfs_off_t		iomap_bsize = XFS_FSB_TO_B(m, imap.br_blockcount);
 
 		offset = (iomap_offset + iomap_bsize - 1) >>
 					PAGE_CACHE_SHIFT;
 		tlast = min_t(pgoff_t, offset, last_index);
-		xfs_cluster_write(inode, page->index + 1, &iomap, &ioend,
+		xfs_cluster_write(inode, page->index + 1, &imap, &ioend,
 					wbc, startio, all_bh, tlast);
 	}
 
@@ -1459,10 +1460,11 @@ __xfs_get_blocks(
 	int			direct,
 	bmapi_flags_t		flags)
 {
-	xfs_iomap_t		iomap;
+	struct xfs_bmbt_irec	imap;
 	xfs_off_t		offset;
 	ssize_t			size;
-	int			niomap = 1;
+	int			nimap = 1;
+	int			new = 0;
 	int			error;
 
 	offset = (xfs_off_t)iblock << inode->i_blkbits;
@@ -1473,21 +1475,21 @@ __xfs_get_blocks(
 		return 0;
 
 	error = xfs_iomap(XFS_I(inode), offset, size,
-			     create ? flags : BMAPI_READ, &iomap, &niomap);
+			     create ? flags : BMAPI_READ, &imap, &nimap, &new);
 	if (error)
 		return -error;
-	if (niomap == 0)
+	if (nimap == 0)
 		return 0;
 
-	if (iomap.iomap_bn != HOLESTARTBLOCK &&
-	    iomap.iomap_bn != DELAYSTARTBLOCK) {
+	if (imap.br_startblock != HOLESTARTBLOCK &&
+	    imap.br_startblock != DELAYSTARTBLOCK) {
 		/*
 		 * For unwritten extents do not report a disk address on
 		 * the read case (treat as if we're reading into a hole).
 		 */
-		if (create || !(iomap.iomap_flags & IOMAP_UNWRITTEN))
-			xfs_map_buffer(inode, bh_result, &iomap, offset);
-		if (create && (iomap.iomap_flags & IOMAP_UNWRITTEN)) {
+		if (create || !ISUNWRITTEN(&imap))
+			xfs_map_buffer(inode, bh_result, &imap, offset);
+		if (create && ISUNWRITTEN(&imap)) {
 			if (direct)
 				bh_result->b_private = inode;
 			set_buffer_unwritten(bh_result);
@@ -1512,10 +1514,10 @@ __xfs_get_blocks(
 	if (create &&
 	    ((!buffer_mapped(bh_result) && !buffer_uptodate(bh_result)) ||
 	     (offset >= i_size_read(inode)) ||
-	     (iomap.iomap_flags & (IOMAP_NEW|IOMAP_UNWRITTEN))))
+	     (new || ISUNWRITTEN(&imap))))
 		set_buffer_new(bh_result);
 
-	if (iomap.iomap_bn == DELAYSTARTBLOCK) {
+	if (imap.br_startblock == DELAYSTARTBLOCK) {
 		BUG_ON(direct);
 		if (create) {
 			set_buffer_uptodate(bh_result);
@@ -1526,9 +1528,9 @@ __xfs_get_blocks(
 
 	if (direct || size > (1 << inode->i_blkbits)) {
 		struct xfs_mount	*mp = XFS_I(inode)->i_mount;
-		xfs_off_t		iomap_offset = XFS_FSB_TO_B(mp, iomap.iomap_offset);
+		xfs_off_t		iomap_offset = XFS_FSB_TO_B(mp, imap.br_startoff);
 		xfs_off_t		iomap_delta = offset - iomap_offset;
-		xfs_off_t		iomap_bsize = XFS_FSB_TO_B(mp, iomap.iomap_bsize);
+		xfs_off_t		iomap_bsize = XFS_FSB_TO_B(mp, imap.br_blockcount);
 
 		ASSERT(iomap_bsize - iomap_delta > 0);
 		offset = min_t(xfs_off_t,
Index: xfs/fs/xfs/xfs_iomap.c
===================================================================
--- xfs.orig/fs/xfs/xfs_iomap.c	2010-04-27 17:39:20.700004101 +0200
+++ xfs/fs/xfs/xfs_iomap.c	2010-04-27 18:12:21.135027288 +0200
@@ -55,46 +55,25 @@
 #define XFS_STRAT_WRITE_IMAPS	2
 #define XFS_WRITE_IMAPS		XFS_BMAP_MAX_NMAP
 
-STATIC void
-xfs_imap_to_bmap(
-	xfs_inode_t	*ip,
-	xfs_off_t	offset,
-	xfs_bmbt_irec_t *imap,
-	xfs_iomap_t	*iomapp,
-	int		imaps,			/* Number of imap entries */
-	int		flags)
-{
-	iomapp->iomap_offset = imap->br_startoff;
-	iomapp->iomap_bsize = imap->br_blockcount;
-	iomapp->iomap_flags = flags;
-	iomapp->iomap_bn = imap->br_startblock;
-
-	if (imap->br_startblock != HOLESTARTBLOCK &&
-	    imap->br_startblock != DELAYSTARTBLOCK &&
-	    ISUNWRITTEN(imap))
-		iomapp->iomap_flags |= IOMAP_UNWRITTEN;
-}
-
 int
 xfs_iomap(
-	xfs_inode_t	*ip,
-	xfs_off_t	offset,
-	ssize_t		count,
-	int		flags,
-	xfs_iomap_t	*iomapp,
-	int		*niomaps)
+	struct xfs_inode	*ip,
+	xfs_off_t		offset,
+	ssize_t			count,
+	int			flags,
+	struct xfs_bmbt_irec	*imap,
+	int			*nimaps,
+	int			*new)
 {
-	xfs_mount_t	*mp = ip->i_mount;
-	xfs_fileoff_t	offset_fsb, end_fsb;
-	int		error = 0;
-	int		lockmode = 0;
-	xfs_bmbt_irec_t	imap;
-	int		nimaps = 1;
-	int		bmapi_flags = 0;
-	int		iomap_flags = 0;
+	struct xfs_mount	*mp = ip->i_mount;
+	xfs_fileoff_t		offset_fsb, end_fsb;
+	int			error = 0;
+	int			lockmode = 0;
+	int			bmapi_flags = 0;
 
 	ASSERT((ip->i_d.di_mode & S_IFMT) == S_IFREG);
-	ASSERT(niomaps && *niomaps == 1);
+
+	*new = 0;
 
 	if (XFS_FORCED_SHUTDOWN(mp))
 		return XFS_ERROR(EIO);
@@ -136,8 +115,8 @@ xfs_iomap(
 
 	error = xfs_bmapi(NULL, ip, offset_fsb,
 			(xfs_filblks_t)(end_fsb - offset_fsb),
-			bmapi_flags,  NULL, 0, &imap,
-			&nimaps, NULL, NULL);
+			bmapi_flags,  NULL, 0, imap,
+			nimaps, NULL, NULL);
 
 	if (error)
 		goto out;
@@ -145,45 +124,41 @@ xfs_iomap(
 	switch (flags & (BMAPI_WRITE|BMAPI_ALLOCATE)) {
 	case BMAPI_WRITE:
 		/* If we found an extent, return it */
-		if (nimaps &&
-		    (imap.br_startblock != HOLESTARTBLOCK) &&
-		    (imap.br_startblock != DELAYSTARTBLOCK)) {
-			trace_xfs_iomap_found(ip, offset, count, flags, &imap);
+		if (*nimaps &&
+		    (imap->br_startblock != HOLESTARTBLOCK) &&
+		    (imap->br_startblock != DELAYSTARTBLOCK)) {
+			trace_xfs_iomap_found(ip, offset, count, flags, imap);
 			break;
 		}
 
 		if (flags & (BMAPI_DIRECT|BMAPI_MMAP)) {
 			error = xfs_iomap_write_direct(ip, offset, count, flags,
-						       &imap, &nimaps, nimaps);
+						       imap, nimaps, *nimaps);
 		} else {
 			error = xfs_iomap_write_delay(ip, offset, count, flags,
-						      &imap, &nimaps);
+						      imap, nimaps);
 		}
 		if (!error) {
-			trace_xfs_iomap_alloc(ip, offset, count, flags, &imap);
+			trace_xfs_iomap_alloc(ip, offset, count, flags, imap);
 		}
-		iomap_flags = IOMAP_NEW;
+		*new = 1;
 		break;
 	case BMAPI_ALLOCATE:
 		/* If we found an extent, return it */
 		xfs_iunlock(ip, lockmode);
 		lockmode = 0;
 
-		if (nimaps && !isnullstartblock(imap.br_startblock)) {
-			trace_xfs_iomap_found(ip, offset, count, flags, &imap);
+		if (*nimaps && !isnullstartblock(imap->br_startblock)) {
+			trace_xfs_iomap_found(ip, offset, count, flags, imap);
 			break;
 		}
 
 		error = xfs_iomap_write_allocate(ip, offset, count,
-						 &imap, &nimaps);
+						 imap, nimaps);
 		break;
 	}
 
-	ASSERT(nimaps <= 1);
-
-	if (nimaps)
-		xfs_imap_to_bmap(ip, offset, &imap, iomapp, nimaps, iomap_flags);
-	*niomaps = nimaps;
+	ASSERT(*nimaps <= 1);
 
 out:
 	if (lockmode)
@@ -191,7 +166,6 @@ out:
 	return XFS_ERROR(error);
 }
 
-
 STATIC int
 xfs_iomap_eof_align_last_fsb(
 	xfs_mount_t	*mp,
Index: xfs/fs/xfs/xfs_iomap.h
===================================================================
--- xfs.orig/fs/xfs/xfs_iomap.h	2010-04-27 17:39:27.783067167 +0200
+++ xfs/fs/xfs/xfs_iomap.h	2010-04-27 18:12:14.232003612 +0200
@@ -47,35 +47,11 @@ typedef enum {
 	{ BMAPI_MMAP,		"MMAP" }, \
 	{ BMAPI_TRYLOCK,	"TRYLOCK" }
 
-/*
- * xfs_iomap_t:  File system I/O map
- *
- * The iomap_bn field is expressed in 512-byte blocks, and is where the
- * mapping starts on disk.
- *
- * The iomap_offset, iomap_bsize and iomap_delta fields are in bytes.
- * iomap_offset is the offset of the mapping in the file itself.
- * iomap_bsize is the size of the mapping,  iomap_delta is the
- * desired data's offset into the mapping, given the offset supplied
- * to the file I/O map routine.
- *
- * When a request is made to read beyond the logical end of the object,
- * iomap_size may be set to 0, but iomap_offset and iomap_length should be set
- * to the actual amount of underlying storage that has been allocated, if any.
- */
-
-typedef struct xfs_iomap {
-	xfs_daddr_t		iomap_bn;	/* first 512B blk of mapping */
-	xfs_off_t		iomap_offset;	/* offset of mapping, bytes */
-	xfs_off_t		iomap_bsize;	/* size of mapping, bytes */
-	iomap_flags_t		iomap_flags;
-} xfs_iomap_t;
-
 struct xfs_inode;
 struct xfs_bmbt_irec;
 
 extern int xfs_iomap(struct xfs_inode *, xfs_off_t, ssize_t, int,
-		     struct xfs_iomap *, int *);
+		     struct xfs_bmbt_irec *, int *, int *);
 extern int xfs_iomap_write_direct(struct xfs_inode *, xfs_off_t, size_t,
 				  int, struct xfs_bmbt_irec *, int *, int);
 extern int xfs_iomap_write_delay(struct xfs_inode *, xfs_off_t, size_t, int,

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

Re: [PATCH 06/11] xfs: kill struct xfs_iomap

[Alex Elder]

On Wed, 2010-04-28 at 08:28 -0400, Christoph Hellwig wrote:
> plain text document attachment (xfs-iomap-cleanup-6)
> Now that struct xfs_iomap contains exactly the same units as
> struct xfs_bmbt_irec we can just use the latter directly in the aops
> code.  Replace the missing IOMAP_NEW flag with a new boolean output
> parameter to xfs_iomap.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks good.  My only suggestion was going to be having xfs_iomap()
support passing a null pointer for "new" if it's a don't-care in
the caller, but in the end it's called in only two spots and the
result would not be an improvement.

Reviewed-by: Alex Elder <aelder@sgi.com>

> Index: xfs/fs/xfs/linux-2.6/xfs_aops.c
> ===================================================================
> --- xfs.orig/fs/xfs/linux-2.6/xfs_aops.c	2010-04-27 17:39:20.699003542 +0200
> +++ xfs/fs/xfs/linux-2.6/xfs_aops.c	2010-04-27 18:12:14.223025612 +0200
> @@ -309,23 +309,24 @@ xfs_map_blocks(
>  	struct inode		*inode,
>  	loff_t			offset,
>  	ssize_t			count,
> -	xfs_iomap_t		*mapp,
> +	struct xfs_bmbt_irec	*imap,
>  	int			flags)
>  {
>  	int			nmaps = 1;
> +	int			new = 0;
>  
> -	return -xfs_iomap(XFS_I(inode), offset, count, flags, mapp, &nmaps);
> +	return -xfs_iomap(XFS_I(inode), offset, count, flags, imap, &nmaps, &new);
>  }
>  
>  STATIC int
>  xfs_iomap_valid(
>  	struct inode		*inode,
> -	xfs_iomap_t		*iomapp,
> +	struct xfs_bmbt_irec	*imap,
>  	loff_t			offset)
>  {
>  	struct xfs_mount	*mp = XFS_I(inode)->i_mount;
> -	xfs_off_t		iomap_offset = XFS_FSB_TO_B(mp, iomapp->iomap_offset);
> -	xfs_off_t		iomap_bsize = XFS_FSB_TO_B(mp, iomapp->iomap_bsize);
> +	xfs_off_t		iomap_offset = XFS_FSB_TO_B(mp, imap->br_startoff);
> +	xfs_off_t		iomap_bsize = XFS_FSB_TO_B(mp, imap->br_blockcount);
>  
>  	return offset >= iomap_offset &&
>  		offset < iomap_offset + iomap_bsize;
> @@ -561,16 +562,16 @@ STATIC void
>  xfs_map_buffer(
>  	struct inode		*inode,
>  	struct buffer_head	*bh,
> -	xfs_iomap_t		*mp,
> +	struct xfs_bmbt_irec	*imap,
>  	xfs_off_t		offset)
>  {
>  	sector_t		bn;
>  	struct xfs_mount	*m = XFS_I(inode)->i_mount;
> -	xfs_off_t		iomap_offset = XFS_FSB_TO_B(m, mp->iomap_offset);
> -	xfs_daddr_t		iomap_bn = xfs_fsb_to_db(XFS_I(inode), mp->iomap_bn);
> +	xfs_off_t		iomap_offset = XFS_FSB_TO_B(m, imap->br_startoff);
> +	xfs_daddr_t		iomap_bn = xfs_fsb_to_db(XFS_I(inode), imap->br_startblock);
>  
> -	ASSERT(mp->iomap_bn != HOLESTARTBLOCK);
> -	ASSERT(mp->iomap_bn != DELAYSTARTBLOCK);
> +	ASSERT(imap->br_startblock != HOLESTARTBLOCK);
> +	ASSERT(imap->br_startblock != DELAYSTARTBLOCK);
>  
>  	bn = (iomap_bn >> (inode->i_blkbits - BBSHIFT)) +
>  	      ((offset - iomap_offset) >> inode->i_blkbits);
> @@ -585,14 +586,14 @@ STATIC void
>  xfs_map_at_offset(
>  	struct inode		*inode,
>  	struct buffer_head	*bh,
> -	xfs_iomap_t		*iomapp,
> +	struct xfs_bmbt_irec	*imap,
>  	xfs_off_t		offset)
>  {
> -	ASSERT(iomapp->iomap_bn != HOLESTARTBLOCK);
> -	ASSERT(iomapp->iomap_bn != DELAYSTARTBLOCK);
> +	ASSERT(imap->br_startblock != HOLESTARTBLOCK);
> +	ASSERT(imap->br_startblock != DELAYSTARTBLOCK);
>  
>  	lock_buffer(bh);
> -	xfs_map_buffer(inode, bh, iomapp, offset);
> +	xfs_map_buffer(inode, bh, imap, offset);
>  	bh->b_bdev = xfs_find_bdev_for_inode(inode);
>  	set_buffer_mapped(bh);
>  	clear_buffer_delay(bh);
> @@ -749,7 +750,7 @@ xfs_convert_page(
>  	struct inode		*inode,
>  	struct page		*page,
>  	loff_t			tindex,
> -	xfs_iomap_t		*mp,
> +	struct xfs_bmbt_irec	*imap,
>  	xfs_ioend_t		**ioendp,
>  	struct writeback_control *wbc,
>  	int			startio,
> @@ -814,15 +815,15 @@ xfs_convert_page(
>  			else
>  				type = IOMAP_DELAY;
>  
> -			if (!xfs_iomap_valid(inode, mp, offset)) {
> +			if (!xfs_iomap_valid(inode, imap, offset)) {
>  				done = 1;
>  				continue;
>  			}
>  
> -			ASSERT(mp->iomap_bn != HOLESTARTBLOCK);
> -			ASSERT(mp->iomap_bn != DELAYSTARTBLOCK);
> +			ASSERT(imap->br_startblock != HOLESTARTBLOCK);
> +			ASSERT(imap->br_startblock != DELAYSTARTBLOCK);
>  
> -			xfs_map_at_offset(inode, bh, mp, offset);
> +			xfs_map_at_offset(inode, bh, imap, offset);
>  			if (startio) {
>  				xfs_add_to_ioend(inode, bh, offset,
>  						type, ioendp, done);
> @@ -874,7 +875,7 @@ STATIC void
>  xfs_cluster_write(
>  	struct inode		*inode,
>  	pgoff_t			tindex,
> -	xfs_iomap_t		*iomapp,
> +	struct xfs_bmbt_irec	*imap,
>  	xfs_ioend_t		**ioendp,
>  	struct writeback_control *wbc,
>  	int			startio,
> @@ -893,7 +894,7 @@ xfs_cluster_write(
>  
>  		for (i = 0; i < pagevec_count(&pvec); i++) {
>  			done = xfs_convert_page(inode, pvec.pages[i], tindex++,
> -					iomapp, ioendp, wbc, startio, all_bh);
> +					imap, ioendp, wbc, startio, all_bh);
>  			if (done)
>  				break;
>  		}
> @@ -1050,7 +1051,7 @@ xfs_page_state_convert(
>  	int		unmapped) /* also implies page uptodate */
>  {
>  	struct buffer_head	*bh, *head;
> -	xfs_iomap_t		iomap;
> +	struct xfs_bmbt_irec	imap;
>  	xfs_ioend_t		*ioend = NULL, *iohead = NULL;
>  	loff_t			offset;
>  	unsigned long           p_offset = 0;
> @@ -1124,7 +1125,7 @@ xfs_page_state_convert(
>  		}
>  
>  		if (iomap_valid)
> -			iomap_valid = xfs_iomap_valid(inode, &iomap, offset);
> +			iomap_valid = xfs_iomap_valid(inode, &imap, offset);
>  
>  		/*
>  		 * First case, map an unwritten extent and prepare for
> @@ -1176,13 +1177,13 @@ xfs_page_state_convert(
>  				}
>  
>  				err = xfs_map_blocks(inode, offset, size,
> -						&iomap, flags);
> +						&imap, flags);
>  				if (err)
>  					goto error;
> -				iomap_valid = xfs_iomap_valid(inode, &iomap, offset);
> +				iomap_valid = xfs_iomap_valid(inode, &imap, offset);
>  			}
>  			if (iomap_valid) {
> -				xfs_map_at_offset(inode, bh, &iomap, offset);
> +				xfs_map_at_offset(inode, bh, &imap, offset);
>  				if (startio) {
>  					xfs_add_to_ioend(inode, bh, offset,
>  							type, &ioend,
> @@ -1206,10 +1207,10 @@ xfs_page_state_convert(
>  				size = xfs_probe_cluster(inode, page, bh,
>  								head, 1);
>  				err = xfs_map_blocks(inode, offset, size,
> -						&iomap, flags);
> +						&imap, flags);
>  				if (err)
>  					goto error;
> -				iomap_valid = xfs_iomap_valid(inode, &iomap, offset);
> +				iomap_valid = xfs_iomap_valid(inode, &imap, offset);
>  			}
>  
>  			/*
> @@ -1250,13 +1251,13 @@ xfs_page_state_convert(
>  
>  	if (ioend && iomap_valid) {
>  		struct xfs_mount	*m = XFS_I(inode)->i_mount;
> -		xfs_off_t		iomap_offset = XFS_FSB_TO_B(m, iomap.iomap_offset);
> -		xfs_off_t		iomap_bsize = XFS_FSB_TO_B(m, iomap.iomap_bsize);
> +		xfs_off_t		iomap_offset = XFS_FSB_TO_B(m, imap.br_startoff);
> +		xfs_off_t		iomap_bsize = XFS_FSB_TO_B(m, imap.br_blockcount);
>  
>  		offset = (iomap_offset + iomap_bsize - 1) >>
>  					PAGE_CACHE_SHIFT;
>  		tlast = min_t(pgoff_t, offset, last_index);
> -		xfs_cluster_write(inode, page->index + 1, &iomap, &ioend,
> +		xfs_cluster_write(inode, page->index + 1, &imap, &ioend,
>  					wbc, startio, all_bh, tlast);
>  	}
>  
> @@ -1459,10 +1460,11 @@ __xfs_get_blocks(
>  	int			direct,
>  	bmapi_flags_t		flags)
>  {
> -	xfs_iomap_t		iomap;
> +	struct xfs_bmbt_irec	imap;
>  	xfs_off_t		offset;
>  	ssize_t			size;
> -	int			niomap = 1;
> +	int			nimap = 1;
> +	int			new = 0;
>  	int			error;
>  
>  	offset = (xfs_off_t)iblock << inode->i_blkbits;
> @@ -1473,21 +1475,21 @@ __xfs_get_blocks(
>  		return 0;
>  
>  	error = xfs_iomap(XFS_I(inode), offset, size,
> -			     create ? flags : BMAPI_READ, &iomap, &niomap);
> +			     create ? flags : BMAPI_READ, &imap, &nimap, &new);
>  	if (error)
>  		return -error;
> -	if (niomap == 0)
> +	if (nimap == 0)
>  		return 0;
>  
> -	if (iomap.iomap_bn != HOLESTARTBLOCK &&
> -	    iomap.iomap_bn != DELAYSTARTBLOCK) {
> +	if (imap.br_startblock != HOLESTARTBLOCK &&
> +	    imap.br_startblock != DELAYSTARTBLOCK) {
>  		/*
>  		 * For unwritten extents do not report a disk address on
>  		 * the read case (treat as if we're reading into a hole).
>  		 */
> -		if (create || !(iomap.iomap_flags & IOMAP_UNWRITTEN))
> -			xfs_map_buffer(inode, bh_result, &iomap, offset);
> -		if (create && (iomap.iomap_flags & IOMAP_UNWRITTEN)) {
> +		if (create || !ISUNWRITTEN(&imap))
> +			xfs_map_buffer(inode, bh_result, &imap, offset);
> +		if (create && ISUNWRITTEN(&imap)) {
>  			if (direct)
>  				bh_result->b_private = inode;
>  			set_buffer_unwritten(bh_result);
> @@ -1512,10 +1514,10 @@ __xfs_get_blocks(
>  	if (create &&
>  	    ((!buffer_mapped(bh_result) && !buffer_uptodate(bh_result)) ||
>  	     (offset >= i_size_read(inode)) ||
> -	     (iomap.iomap_flags & (IOMAP_NEW|IOMAP_UNWRITTEN))))
> +	     (new || ISUNWRITTEN(&imap))))
>  		set_buffer_new(bh_result);
>  
> -	if (iomap.iomap_bn == DELAYSTARTBLOCK) {
> +	if (imap.br_startblock == DELAYSTARTBLOCK) {
>  		BUG_ON(direct);
>  		if (create) {
>  			set_buffer_uptodate(bh_result);
> @@ -1526,9 +1528,9 @@ __xfs_get_blocks(
>  
>  	if (direct || size > (1 << inode->i_blkbits)) {
>  		struct xfs_mount	*mp = XFS_I(inode)->i_mount;
> -		xfs_off_t		iomap_offset = XFS_FSB_TO_B(mp, iomap.iomap_offset);
> +		xfs_off_t		iomap_offset = XFS_FSB_TO_B(mp, imap.br_startoff);
>  		xfs_off_t		iomap_delta = offset - iomap_offset;
> -		xfs_off_t		iomap_bsize = XFS_FSB_TO_B(mp, iomap.iomap_bsize);
> +		xfs_off_t		iomap_bsize = XFS_FSB_TO_B(mp, imap.br_blockcount);
>  
>  		ASSERT(iomap_bsize - iomap_delta > 0);
>  		offset = min_t(xfs_off_t,
> Index: xfs/fs/xfs/xfs_iomap.c
> ===================================================================
> --- xfs.orig/fs/xfs/xfs_iomap.c	2010-04-27 17:39:20.700004101 +0200
> +++ xfs/fs/xfs/xfs_iomap.c	2010-04-27 18:12:21.135027288 +0200
> @@ -55,46 +55,25 @@
>  #define XFS_STRAT_WRITE_IMAPS	2
>  #define XFS_WRITE_IMAPS		XFS_BMAP_MAX_NMAP
>  
> -STATIC void
> -xfs_imap_to_bmap(
> -	xfs_inode_t	*ip,
> -	xfs_off_t	offset,
> -	xfs_bmbt_irec_t *imap,
> -	xfs_iomap_t	*iomapp,
> -	int		imaps,			/* Number of imap entries */
> -	int		flags)
> -{
> -	iomapp->iomap_offset = imap->br_startoff;
> -	iomapp->iomap_bsize = imap->br_blockcount;
> -	iomapp->iomap_flags = flags;
> -	iomapp->iomap_bn = imap->br_startblock;
> -
> -	if (imap->br_startblock != HOLESTARTBLOCK &&
> -	    imap->br_startblock != DELAYSTARTBLOCK &&
> -	    ISUNWRITTEN(imap))
> -		iomapp->iomap_flags |= IOMAP_UNWRITTEN;
> -}
> -
>  int
>  xfs_iomap(
> -	xfs_inode_t	*ip,
> -	xfs_off_t	offset,
> -	ssize_t		count,
> -	int		flags,
> -	xfs_iomap_t	*iomapp,
> -	int		*niomaps)
> +	struct xfs_inode	*ip,
> +	xfs_off_t		offset,
> +	ssize_t			count,
> +	int			flags,
> +	struct xfs_bmbt_irec	*imap,
> +	int			*nimaps,
> +	int			*new)
>  {
> -	xfs_mount_t	*mp = ip->i_mount;
> -	xfs_fileoff_t	offset_fsb, end_fsb;
> -	int		error = 0;
> -	int		lockmode = 0;
> -	xfs_bmbt_irec_t	imap;
> -	int		nimaps = 1;
> -	int		bmapi_flags = 0;
> -	int		iomap_flags = 0;
> +	struct xfs_mount	*mp = ip->i_mount;
> +	xfs_fileoff_t		offset_fsb, end_fsb;
> +	int			error = 0;
> +	int			lockmode = 0;
> +	int			bmapi_flags = 0;
>  
>  	ASSERT((ip->i_d.di_mode & S_IFMT) == S_IFREG);
> -	ASSERT(niomaps && *niomaps == 1);
> +
> +	*new = 0;
>  
>  	if (XFS_FORCED_SHUTDOWN(mp))
>  		return XFS_ERROR(EIO);
> @@ -136,8 +115,8 @@ xfs_iomap(
>  
>  	error = xfs_bmapi(NULL, ip, offset_fsb,
>  			(xfs_filblks_t)(end_fsb - offset_fsb),
> -			bmapi_flags,  NULL, 0, &imap,
> -			&nimaps, NULL, NULL);
> +			bmapi_flags,  NULL, 0, imap,
> +			nimaps, NULL, NULL);
>  
>  	if (error)
>  		goto out;
> @@ -145,45 +124,41 @@ xfs_iomap(
>  	switch (flags & (BMAPI_WRITE|BMAPI_ALLOCATE)) {
>  	case BMAPI_WRITE:
>  		/* If we found an extent, return it */
> -		if (nimaps &&
> -		    (imap.br_startblock != HOLESTARTBLOCK) &&
> -		    (imap.br_startblock != DELAYSTARTBLOCK)) {
> -			trace_xfs_iomap_found(ip, offset, count, flags, &imap);
> +		if (*nimaps &&
> +		    (imap->br_startblock != HOLESTARTBLOCK) &&
> +		    (imap->br_startblock != DELAYSTARTBLOCK)) {
> +			trace_xfs_iomap_found(ip, offset, count, flags, imap);
>  			break;
>  		}
>  
>  		if (flags & (BMAPI_DIRECT|BMAPI_MMAP)) {
>  			error = xfs_iomap_write_direct(ip, offset, count, flags,
> -						       &imap, &nimaps, nimaps);
> +						       imap, nimaps, *nimaps);
>  		} else {
>  			error = xfs_iomap_write_delay(ip, offset, count, flags,
> -						      &imap, &nimaps);
> +						      imap, nimaps);
>  		}
>  		if (!error) {
> -			trace_xfs_iomap_alloc(ip, offset, count, flags, &imap);
> +			trace_xfs_iomap_alloc(ip, offset, count, flags, imap);
>  		}
> -		iomap_flags = IOMAP_NEW;
> +		*new = 1;
>  		break;
>  	case BMAPI_ALLOCATE:
>  		/* If we found an extent, return it */
>  		xfs_iunlock(ip, lockmode);
>  		lockmode = 0;
>  
> -		if (nimaps && !isnullstartblock(imap.br_startblock)) {
> -			trace_xfs_iomap_found(ip, offset, count, flags, &imap);
> +		if (*nimaps && !isnullstartblock(imap->br_startblock)) {
> +			trace_xfs_iomap_found(ip, offset, count, flags, imap);
>  			break;
>  		}
>  
>  		error = xfs_iomap_write_allocate(ip, offset, count,
> -						 &imap, &nimaps);
> +						 imap, nimaps);
>  		break;
>  	}
>  
> -	ASSERT(nimaps <= 1);
> -
> -	if (nimaps)
> -		xfs_imap_to_bmap(ip, offset, &imap, iomapp, nimaps, iomap_flags);
> -	*niomaps = nimaps;
> +	ASSERT(*nimaps <= 1);
>  
>  out:
>  	if (lockmode)
> @@ -191,7 +166,6 @@ out:
>  	return XFS_ERROR(error);
>  }
>  
> -
>  STATIC int
>  xfs_iomap_eof_align_last_fsb(
>  	xfs_mount_t	*mp,
> Index: xfs/fs/xfs/xfs_iomap.h
> ===================================================================
> --- xfs.orig/fs/xfs/xfs_iomap.h	2010-04-27 17:39:27.783067167 +0200
> +++ xfs/fs/xfs/xfs_iomap.h	2010-04-27 18:12:14.232003612 +0200
> @@ -47,35 +47,11 @@ typedef enum {
>  	{ BMAPI_MMAP,		"MMAP" }, \
>  	{ BMAPI_TRYLOCK,	"TRYLOCK" }
>  
> -/*
> - * xfs_iomap_t:  File system I/O map
> - *
> - * The iomap_bn field is expressed in 512-byte blocks, and is where the
> - * mapping starts on disk.
> - *
> - * The iomap_offset, iomap_bsize and iomap_delta fields are in bytes.
> - * iomap_offset is the offset of the mapping in the file itself.
> - * iomap_bsize is the size of the mapping,  iomap_delta is the
> - * desired data's offset into the mapping, given the offset supplied
> - * to the file I/O map routine.
> - *
> - * When a request is made to read beyond the logical end of the object,
> - * iomap_size may be set to 0, but iomap_offset and iomap_length should be set
> - * to the actual amount of underlying storage that has been allocated, if any.
> - */
> -
> -typedef struct xfs_iomap {
> -	xfs_daddr_t		iomap_bn;	/* first 512B blk of mapping */
> -	xfs_off_t		iomap_offset;	/* offset of mapping, bytes */
> -	xfs_off_t		iomap_bsize;	/* size of mapping, bytes */
> -	iomap_flags_t		iomap_flags;
> -} xfs_iomap_t;
> -
>  struct xfs_inode;
>  struct xfs_bmbt_irec;
>  
>  extern int xfs_iomap(struct xfs_inode *, xfs_off_t, ssize_t, int,
> -		     struct xfs_iomap *, int *);
> +		     struct xfs_bmbt_irec *, int *, int *);
>  extern int xfs_iomap_write_direct(struct xfs_inode *, xfs_off_t, size_t,
>  				  int, struct xfs_bmbt_irec *, int *, int);
>  extern int xfs_iomap_write_delay(struct xfs_inode *, xfs_off_t, size_t, int,
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs



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

[PATCH 07/11] xfs: move I/O type flags into xfs_aops.c

[Christoph Hellwig]

[-- Attachment #1: xfs-iomap-cleanup-7 --]
[-- Type: text/plain, Size: 6570 bytes --]

The IOMAP_ flags are now only used inside xfs_aops.c for extent probing
and I/O completion tracking, so more them here, and rename them to IO_*
as there's no mapping involved at all.

Signed-off-by: Christoph Hellwig <hch@lst.de>

Index: xfs/fs/xfs/linux-2.6/xfs_aops.c
===================================================================
--- xfs.orig/fs/xfs/linux-2.6/xfs_aops.c	2010-04-27 18:45:11.776004100 +0200
+++ xfs/fs/xfs/linux-2.6/xfs_aops.c	2010-04-27 20:34:06.333011852 +0200
@@ -44,6 +44,15 @@
 #include <linux/pagevec.h>
 #include <linux/writeback.h>
 
+/*
+ * Types of I/O for bmap clustering and I/O completion tracking.
+ */
+enum {
+	IO_READ,	/* mapping for a read */
+	IO_DELAY,	/* mapping covers delalloc region */
+	IO_UNWRITTEN,	/* mapping covers allocated but uninitialized data */
+	IO_NEW		/* just allocated */
+};
 
 /*
  * Prime number of hash buckets since address is used as the key.
@@ -183,7 +192,7 @@ xfs_setfilesize(
 	xfs_fsize_t		isize;
 
 	ASSERT((ip->i_d.di_mode & S_IFMT) == S_IFREG);
-	ASSERT(ioend->io_type != IOMAP_READ);
+	ASSERT(ioend->io_type != IO_READ);
 
 	if (unlikely(ioend->io_error))
 		return 0;
@@ -214,7 +223,7 @@ xfs_finish_ioend(
 	if (atomic_dec_and_test(&ioend->io_remaining)) {
 		struct workqueue_struct *wq;
 
-		wq = (ioend->io_type == IOMAP_UNWRITTEN) ?
+		wq = (ioend->io_type == IO_UNWRITTEN) ?
 			xfsconvertd_workqueue : xfsdatad_workqueue;
 		queue_work(wq, &ioend->io_work);
 		if (wait)
@@ -237,7 +246,7 @@ xfs_end_io(
 	 * For unwritten extents we need to issue transactions to convert a
 	 * range to normal written extens after the data I/O has finished.
 	 */
-	if (ioend->io_type == IOMAP_UNWRITTEN &&
+	if (ioend->io_type == IO_UNWRITTEN &&
 	    likely(!ioend->io_error && !XFS_FORCED_SHUTDOWN(ip->i_mount))) {
 
 		error = xfs_iomap_write_unwritten(ip, ioend->io_offset,
@@ -250,7 +259,7 @@ xfs_end_io(
 	 * We might have to update the on-disk file size after extending
 	 * writes.
 	 */
-	if (ioend->io_type != IOMAP_READ) {
+	if (ioend->io_type != IO_READ) {
 		error = xfs_setfilesize(ioend);
 		ASSERT(!error || error == EAGAIN);
 	}
@@ -723,11 +732,11 @@ xfs_is_delayed_page(
 		bh = head = page_buffers(page);
 		do {
 			if (buffer_unwritten(bh))
-				acceptable = (type == IOMAP_UNWRITTEN);
+				acceptable = (type == IO_UNWRITTEN);
 			else if (buffer_delay(bh))
-				acceptable = (type == IOMAP_DELAY);
+				acceptable = (type == IO_DELAY);
 			else if (buffer_dirty(bh) && buffer_mapped(bh))
-				acceptable = (type == IOMAP_NEW);
+				acceptable = (type == IO_NEW);
 			else
 				break;
 		} while ((bh = bh->b_this_page) != head);
@@ -811,9 +820,9 @@ xfs_convert_page(
 
 		if (buffer_unwritten(bh) || buffer_delay(bh)) {
 			if (buffer_unwritten(bh))
-				type = IOMAP_UNWRITTEN;
+				type = IO_UNWRITTEN;
 			else
-				type = IOMAP_DELAY;
+				type = IO_DELAY;
 
 			if (!xfs_iomap_valid(inode, imap, offset)) {
 				done = 1;
@@ -835,7 +844,7 @@ xfs_convert_page(
 			page_dirty--;
 			count++;
 		} else {
-			type = IOMAP_NEW;
+			type = IO_NEW;
 			if (buffer_mapped(bh) && all_bh && startio) {
 				lock_buffer(bh);
 				xfs_add_to_ioend(inode, bh, offset,
@@ -939,7 +948,7 @@ xfs_aops_discard_page(
 	loff_t			offset = page_offset(page);
 	ssize_t			len = 1 << inode->i_blkbits;
 
-	if (!xfs_is_delayed_page(page, IOMAP_DELAY))
+	if (!xfs_is_delayed_page(page, IO_DELAY))
 		goto out_invalidate;
 
 	if (XFS_FORCED_SHUTDOWN(ip->i_mount))
@@ -1106,7 +1115,7 @@ xfs_page_state_convert(
 	bh = head = page_buffers(page);
 	offset = page_offset(page);
 	flags = BMAPI_READ;
-	type = IOMAP_NEW;
+	type = IO_NEW;
 
 	/* TODO: cleanup count and page_dirty */
 
@@ -1149,13 +1158,13 @@ xfs_page_state_convert(
 				iomap_valid = 0;
 
 			if (buffer_unwritten(bh)) {
-				type = IOMAP_UNWRITTEN;
+				type = IO_UNWRITTEN;
 				flags = BMAPI_WRITE | BMAPI_IGNSTATE;
 			} else if (buffer_delay(bh)) {
-				type = IOMAP_DELAY;
+				type = IO_DELAY;
 				flags = BMAPI_ALLOCATE | trylock;
 			} else {
-				type = IOMAP_NEW;
+				type = IO_NEW;
 				flags = BMAPI_WRITE | BMAPI_MMAP;
 			}
 
@@ -1169,7 +1178,7 @@ xfs_page_state_convert(
 				 * for unwritten extent conversion.
 				 */
 				new_ioend = 1;
-				if (type == IOMAP_NEW) {
+				if (type == IO_NEW) {
 					size = xfs_probe_cluster(inode,
 							page, bh, head, 0);
 				} else {
@@ -1214,14 +1223,14 @@ xfs_page_state_convert(
 			}
 
 			/*
-			 * We set the type to IOMAP_NEW in case we are doing a
+			 * We set the type to IO_NEW in case we are doing a
 			 * small write at EOF that is extending the file but
 			 * without needing an allocation. We need to update the
 			 * file size on I/O completion in this case so it is
 			 * the same case as having just allocated a new extent
 			 * that we are writing into for the first time.
 			 */
-			type = IOMAP_NEW;
+			type = IO_NEW;
 			if (trylock_buffer(bh)) {
 				ASSERT(buffer_mapped(bh));
 				if (iomap_valid)
@@ -1593,7 +1602,7 @@ xfs_end_io_direct(
 	 */
 	ioend->io_offset = offset;
 	ioend->io_size = size;
-	if (ioend->io_type == IOMAP_READ) {
+	if (ioend->io_type == IO_READ) {
 		xfs_finish_ioend(ioend, 0);
 	} else if (private && size > 0) {
 		xfs_finish_ioend(ioend, is_sync_kiocb(iocb));
@@ -1604,7 +1613,7 @@ xfs_end_io_direct(
 		 * didn't map an unwritten extent so switch it's completion
 		 * handler.
 		 */
-		ioend->io_type = IOMAP_NEW;
+		ioend->io_type = IO_NEW;
 		xfs_finish_ioend(ioend, 0);
 	}
 
@@ -1632,7 +1641,7 @@ xfs_vm_direct_IO(
 	bdev = xfs_find_bdev_for_inode(inode);
 
 	iocb->private = xfs_alloc_ioend(inode, rw == WRITE ?
-					IOMAP_UNWRITTEN : IOMAP_READ);
+					IO_UNWRITTEN : IO_READ);
 
 	ret = blockdev_direct_IO_no_locking(rw, iocb, inode, bdev, iov,
 					    offset, nr_segs,
Index: xfs/fs/xfs/xfs_iomap.h
===================================================================
--- xfs.orig/fs/xfs/xfs_iomap.h	2010-04-27 18:45:11.784022678 +0200
+++ xfs/fs/xfs/xfs_iomap.h	2010-04-27 20:20:17.308010875 +0200
@@ -18,14 +18,6 @@
 #ifndef __XFS_IOMAP_H__
 #define __XFS_IOMAP_H__
 
-typedef enum {				/* iomap_flags values */
-	IOMAP_READ =		0,	/* mapping for a read */
-	IOMAP_DELAY =		0x04,	/* mapping covers delalloc region  */
-	IOMAP_UNWRITTEN =	0x20,	/* mapping covers allocated */
-					/* but uninitialized file data  */
-	IOMAP_NEW =		0x40	/* just allocate */
-} iomap_flags_t;
-
 typedef enum {
 	/* base extent manipulation calls */
 	BMAPI_READ = (1 << 0),		/* read extents */

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

Re: [PATCH 07/11] xfs: move I/O type flags into xfs_aops.c

[Alex Elder]

On Wed, 2010-04-28 at 08:28 -0400, Christoph Hellwig wrote:
> plain text document attachment (xfs-iomap-cleanup-7)
> The IOMAP_ flags are now only used inside xfs_aops.c for extent probing
> and I/O completion tracking, so more them here, and rename them to IO_*
> as there's no mapping involved at all.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks good.

Reviewed-by: Alex Elder <aelder@sgi.com>

> Index: xfs/fs/xfs/linux-2.6/xfs_aops.c
> ===================================================================


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

[PATCH 08/11] xfs: clean up xfs_iomap_valid

[Christoph Hellwig]

[-- Attachment #1: xfs-iomap-cleanup-9 --]
[-- Type: text/plain, Size: 4591 bytes --]

Rename all iomap_valid identifiers to imap_valid to fit the new world order,
and clean up xfs_iomap_valid to convert the passed in offset to blocks
instead of the imap values to bytes.  Use the simpler inode->i_blkbits
instead of the XFS macros for this.

Signed-off-by: Christoph Hellwig <hch@lst.de>

Index: xfs/fs/xfs/linux-2.6/xfs_aops.c
===================================================================
--- xfs.orig/fs/xfs/linux-2.6/xfs_aops.c	2010-04-28 12:59:42.130254345 +0200
+++ xfs/fs/xfs/linux-2.6/xfs_aops.c	2010-04-28 12:59:49.296003890 +0200
@@ -328,17 +328,15 @@ xfs_map_blocks(
 }
 
 STATIC int
-xfs_iomap_valid(
+xfs_imap_valid(
 	struct inode		*inode,
 	struct xfs_bmbt_irec	*imap,
-	loff_t			offset)
+	xfs_off_t		offset)
 {
-	struct xfs_mount	*mp = XFS_I(inode)->i_mount;
-	xfs_off_t		iomap_offset = XFS_FSB_TO_B(mp, imap->br_startoff);
-	xfs_off_t		iomap_bsize = XFS_FSB_TO_B(mp, imap->br_blockcount);
+	offset >>= inode->i_blkbits;
 
-	return offset >= iomap_offset &&
-		offset < iomap_offset + iomap_bsize;
+	return offset >= imap->br_startoff &&
+		offset < imap->br_startoff + imap->br_blockcount;
 }
 
 /*
@@ -824,7 +822,7 @@ xfs_convert_page(
 			else
 				type = IO_DELAY;
 
-			if (!xfs_iomap_valid(inode, imap, offset)) {
+			if (!xfs_imap_valid(inode, imap, offset)) {
 				done = 1;
 				continue;
 			}
@@ -1068,7 +1066,7 @@ xfs_page_state_convert(
 	__uint64_t              end_offset;
 	pgoff_t                 end_index, last_index, tlast;
 	ssize_t			size, len;
-	int			flags, err, iomap_valid = 0, uptodate = 1;
+	int			flags, err, imap_valid = 0, uptodate = 1;
 	int			page_dirty, count = 0;
 	int			trylock = 0;
 	int			all_bh = unmapped;
@@ -1129,12 +1127,12 @@ xfs_page_state_convert(
 			 * the iomap is actually still valid, but the ioend
 			 * isn't.  shouldn't happen too often.
 			 */
-			iomap_valid = 0;
+			imap_valid = 0;
 			continue;
 		}
 
-		if (iomap_valid)
-			iomap_valid = xfs_iomap_valid(inode, &imap, offset);
+		if (imap_valid)
+			imap_valid = xfs_imap_valid(inode, &imap, offset);
 
 		/*
 		 * First case, map an unwritten extent and prepare for
@@ -1155,7 +1153,7 @@ xfs_page_state_convert(
 			 * Make sure we don't use a read-only iomap
 			 */
 			if (flags == BMAPI_READ)
-				iomap_valid = 0;
+				imap_valid = 0;
 
 			if (buffer_unwritten(bh)) {
 				type = IO_UNWRITTEN;
@@ -1168,7 +1166,7 @@ xfs_page_state_convert(
 				flags = BMAPI_WRITE | BMAPI_MMAP;
 			}
 
-			if (!iomap_valid) {
+			if (!imap_valid) {
 				/*
 				 * if we didn't have a valid mapping then we
 				 * need to ensure that we put the new mapping
@@ -1189,9 +1187,10 @@ xfs_page_state_convert(
 						&imap, flags);
 				if (err)
 					goto error;
-				iomap_valid = xfs_iomap_valid(inode, &imap, offset);
+				imap_valid = xfs_imap_valid(inode, &imap,
+							    offset);
 			}
-			if (iomap_valid) {
+			if (imap_valid) {
 				xfs_map_at_offset(inode, bh, &imap, offset);
 				if (startio) {
 					xfs_add_to_ioend(inode, bh, offset,
@@ -1211,7 +1210,7 @@ xfs_page_state_convert(
 			 * That means it must already have extents allocated
 			 * underneath it. Map the extent by reading it.
 			 */
-			if (!iomap_valid || flags != BMAPI_READ) {
+			if (!imap_valid || flags != BMAPI_READ) {
 				flags = BMAPI_READ;
 				size = xfs_probe_cluster(inode, page, bh,
 								head, 1);
@@ -1219,7 +1218,8 @@ xfs_page_state_convert(
 						&imap, flags);
 				if (err)
 					goto error;
-				iomap_valid = xfs_iomap_valid(inode, &imap, offset);
+				imap_valid = xfs_imap_valid(inode, &imap,
+							    offset);
 			}
 
 			/*
@@ -1233,18 +1233,18 @@ xfs_page_state_convert(
 			type = IO_NEW;
 			if (trylock_buffer(bh)) {
 				ASSERT(buffer_mapped(bh));
-				if (iomap_valid)
+				if (imap_valid)
 					all_bh = 1;
 				xfs_add_to_ioend(inode, bh, offset, type,
-						&ioend, !iomap_valid);
+						&ioend, !imap_valid);
 				page_dirty--;
 				count++;
 			} else {
-				iomap_valid = 0;
+				imap_valid = 0;
 			}
 		} else if ((buffer_uptodate(bh) || PageUptodate(page)) &&
 			   (unmapped || startio)) {
-			iomap_valid = 0;
+			imap_valid = 0;
 		}
 
 		if (!iohead)
@@ -1258,7 +1258,7 @@ xfs_page_state_convert(
 	if (startio)
 		xfs_start_page_writeback(page, 1, count);
 
-	if (ioend && iomap_valid) {
+	if (ioend && imap_valid) {
 		struct xfs_mount	*m = XFS_I(inode)->i_mount;
 		xfs_off_t		iomap_offset = XFS_FSB_TO_B(m, imap.br_startoff);
 		xfs_off_t		iomap_bsize = XFS_FSB_TO_B(m, imap.br_blockcount);

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

Re: [PATCH 08/11] xfs: clean up xfs_iomap_valid

[Alex Elder]

On Wed, 2010-04-28 at 08:28 -0400, Christoph Hellwig wrote:
> plain text document attachment (xfs-iomap-cleanup-9)
> Rename all iomap_valid identifiers to imap_valid to fit the new world order,
> and clean up xfs_iomap_valid to convert the passed in offset to blocks
> instead of the imap values to bytes.  Use the simpler inode->i_blkbits
> instead of the XFS macros for this.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks good.

Reviewed-by: Alex Elder <aelder@sgi.com>

> Index: xfs/fs/xfs/linux-2.6/xfs_aops.c
> ===================================================================


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

[PATCH 09/11] xfs: clean up mapping size calculation in __xfs_get_blocks

[Christoph Hellwig]

[-- Attachment #1: xfs-iomap-cleanup-10 --]
[-- Type: text/plain, Size: 1424 bytes --]

Signed-off-by: Christoph Hellwig <hch@lst.de>

Index: xfs/fs/xfs/linux-2.6/xfs_aops.c
===================================================================
--- xfs.orig/fs/xfs/linux-2.6/xfs_aops.c	2010-04-28 12:59:49.296003890 +0200
+++ xfs/fs/xfs/linux-2.6/xfs_aops.c	2010-04-28 12:59:52.857024355 +0200
@@ -1535,16 +1535,23 @@ __xfs_get_blocks(
 		}
 	}
 
+	/*
+	 * If this is O_DIRECT or the mpage code calling tell them how large
+	 * the mapping is, so that we can avoid repeated get_blocks calls.
+	 */
 	if (direct || size > (1 << inode->i_blkbits)) {
-		struct xfs_mount	*mp = XFS_I(inode)->i_mount;
-		xfs_off_t		iomap_offset = XFS_FSB_TO_B(mp, imap.br_startoff);
-		xfs_off_t		iomap_delta = offset - iomap_offset;
-		xfs_off_t		iomap_bsize = XFS_FSB_TO_B(mp, imap.br_blockcount);
+		xfs_off_t		mapping_size;
 
-		ASSERT(iomap_bsize - iomap_delta > 0);
-		offset = min_t(xfs_off_t,
-				iomap_bsize - iomap_delta, size);
-		bh_result->b_size = (ssize_t)min_t(xfs_off_t, LONG_MAX, offset);
+		mapping_size = imap.br_startoff + imap.br_blockcount - iblock;
+		mapping_size <<= inode->i_blkbits;
+
+		ASSERT(mapping_size > 0);
+		if (mapping_size > size)
+			mapping_size = size;
+		if (mapping_size > LONG_MAX)
+			mapping_size = LONG_MAX;
+
+		bh_result->b_size = mapping_size;
 	}
 
 	return 0;

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

Re: [PATCH 09/11] xfs: clean up mapping size calculation in __xfs_get_blocks

[Alex Elder]

On Wed, 2010-04-28 at 08:28 -0400, Christoph Hellwig wrote:
> plain text document attachment (xfs-iomap-cleanup-10)
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks good.

Reviewed-by: Alex Elder <aelder@sgi.com>

> Index: xfs/fs/xfs/linux-2.6/xfs_aops.c
> ===================================================================


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

Re: [PATCH 09/11] xfs: clean up mapping size calculation in __xfs_get_blocks

[Alex Elder]

On Wed, 2010-04-28 at 08:28 -0400, Christoph Hellwig wrote:
> plain text document attachment (xfs-iomap-cleanup-10)
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks good.  I do think the result of your cleanups is
cleaner looking code.  I worry (a little bit) that
some of the information embedded in some things (like
macro names, e.g. XFS_FSB_TO_B()) going away makes
the code less understandable by someone unfamiliar
with it.  Nevertheless...

Reviewed-by: Alex Elder <aelder@sgi.com>

> Index: xfs/fs/xfs/linux-2.6/xfs_aops.c
> ===================================================================


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

[PATCH 10/11] xfs: clean up end index calculation in xfs_page_state_convert

[Christoph Hellwig]

[-- Attachment #1: xfs-iomap-cleanup-11 --]
[-- Type: text/plain, Size: 1662 bytes --]

Signed-off-by: Christoph Hellwig <hch@lst.de>

Index: xfs/fs/xfs/linux-2.6/xfs_aops.c
===================================================================
--- xfs.orig/fs/xfs/linux-2.6/xfs_aops.c	2010-04-28 12:59:52.857024355 +0200
+++ xfs/fs/xfs/linux-2.6/xfs_aops.c	2010-04-28 12:59:54.989003542 +0200
@@ -1064,7 +1064,7 @@ xfs_page_state_convert(
 	unsigned long           p_offset = 0;
 	unsigned int		type;
 	__uint64_t              end_offset;
-	pgoff_t                 end_index, last_index, tlast;
+	pgoff_t                 end_index, last_index;
 	ssize_t			size, len;
 	int			flags, err, imap_valid = 0, uptodate = 1;
 	int			page_dirty, count = 0;
@@ -1259,15 +1259,22 @@ xfs_page_state_convert(
 		xfs_start_page_writeback(page, 1, count);
 
 	if (ioend && imap_valid) {
-		struct xfs_mount	*m = XFS_I(inode)->i_mount;
-		xfs_off_t		iomap_offset = XFS_FSB_TO_B(m, imap.br_startoff);
-		xfs_off_t		iomap_bsize = XFS_FSB_TO_B(m, imap.br_blockcount);
-
-		offset = (iomap_offset + iomap_bsize - 1) >>
-					PAGE_CACHE_SHIFT;
-		tlast = min_t(pgoff_t, offset, last_index);
+		xfs_off_t		end_index;
+
+		end_index = imap.br_startoff + imap.br_blockcount;
+
+		/* to bytes */
+		end_index <<= inode->i_blkbits;
+
+		/* to pages */
+		end_index = (end_index - 1) >> PAGE_CACHE_SHIFT;
+
+		/* check against file size */
+		if (end_index > last_index)
+			end_index = last_index;
+
 		xfs_cluster_write(inode, page->index + 1, &imap, &ioend,
-					wbc, startio, all_bh, tlast);
+					wbc, startio, all_bh, end_index);
 	}
 
 	if (iohead)

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

Re: [PATCH 10/11] xfs: clean up end index calculation in xfs_page_state_convert

[Alex Elder]

On Wed, 2010-04-28 at 08:29 -0400, Christoph Hellwig wrote:
> plain text document attachment (xfs-iomap-cleanup-11)
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Whoops, I think I responded twice to patch 9.

The whole series of 11 looks good.

Reviewed-by: Alex Elder <aelder@sgi.com>


> Index: xfs/fs/xfs/linux-2.6/xfs_aops.c
> ===================================================================
> --- xfs.orig/fs/xfs/linux-2.6/xfs_aops.c	2010-04-28 12:59:52.857024355 +0200
> +++ xfs/fs/xfs/linux-2.6/xfs_aops.c	2010-04-28 12:59:54.989003542 +0200
> @@ -1064,7 +1064,7 @@ xfs_page_state_convert(
>  	unsigned long           p_offset = 0;
>  	unsigned int		type;
>  	__uint64_t              end_offset;
> -	pgoff_t                 end_index, last_index, tlast;
> +	pgoff_t                 end_index, last_index;
>  	ssize_t			size, len;
>  	int			flags, err, imap_valid = 0, uptodate = 1;
>  	int			page_dirty, count = 0;
> @@ -1259,15 +1259,22 @@ xfs_page_state_convert(
>  		xfs_start_page_writeback(page, 1, count);
>  
>  	if (ioend && imap_valid) {
> -		struct xfs_mount	*m = XFS_I(inode)->i_mount;
> -		xfs_off_t		iomap_offset = XFS_FSB_TO_B(m, imap.br_startoff);
> -		xfs_off_t		iomap_bsize = XFS_FSB_TO_B(m, imap.br_blockcount);
> -
> -		offset = (iomap_offset + iomap_bsize - 1) >>
> -					PAGE_CACHE_SHIFT;
> -		tlast = min_t(pgoff_t, offset, last_index);
> +		xfs_off_t		end_index;
> +
> +		end_index = imap.br_startoff + imap.br_blockcount;
> +
> +		/* to bytes */
> +		end_index <<= inode->i_blkbits;
> +
> +		/* to pages */
> +		end_index = (end_index - 1) >> PAGE_CACHE_SHIFT;
> +
> +		/* check against file size */
> +		if (end_index > last_index)
> +			end_index = last_index;
> +
>  		xfs_cluster_write(inode, page->index + 1, &imap, &ioend,
> -					wbc, startio, all_bh, tlast);
> +					wbc, startio, all_bh, end_index);
>  	}
>  
>  	if (iohead)
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs



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

[PATCH 11/11] xfs: mark xfs_iomap_write_ helpers static

[Christoph Hellwig]

[-- Attachment #1: xfs-iomap-cleanup-12 --]
[-- Type: text/plain, Size: 3145 bytes --]

And also drop a useless argument to xfs_iomap_write_direct.

Signed-off-by: Christoph Hellwig <hch@lst.de>

Index: xfs/fs/xfs/xfs_iomap.c
===================================================================
--- xfs.orig/fs/xfs/xfs_iomap.c	2010-04-27 18:24:42.775004380 +0200
+++ xfs/fs/xfs/xfs_iomap.c	2010-04-27 18:32:10.114024284 +0200
@@ -55,6 +55,13 @@
 #define XFS_STRAT_WRITE_IMAPS	2
 #define XFS_WRITE_IMAPS		XFS_BMAP_MAX_NMAP
 
+STATIC int xfs_iomap_write_direct(struct xfs_inode *, xfs_off_t, size_t,
+				  int, struct xfs_bmbt_irec *, int *);
+STATIC int xfs_iomap_write_delay(struct xfs_inode *, xfs_off_t, size_t, int,
+				 struct xfs_bmbt_irec *, int *);
+STATIC int xfs_iomap_write_allocate(struct xfs_inode *, xfs_off_t, size_t,
+				struct xfs_bmbt_irec *, int *);
+
 int
 xfs_iomap(
 	struct xfs_inode	*ip,
@@ -133,7 +140,7 @@ xfs_iomap(
 
 		if (flags & (BMAPI_DIRECT|BMAPI_MMAP)) {
 			error = xfs_iomap_write_direct(ip, offset, count, flags,
-						       imap, nimaps, *nimaps);
+						       imap, nimaps);
 		} else {
 			error = xfs_iomap_write_delay(ip, offset, count, flags,
 						      imap, nimaps);
@@ -234,15 +241,14 @@ xfs_cmn_err_fsblock_zero(
 	return EFSCORRUPTED;
 }
 
-int
+STATIC int
 xfs_iomap_write_direct(
 	xfs_inode_t	*ip,
 	xfs_off_t	offset,
 	size_t		count,
 	int		flags,
 	xfs_bmbt_irec_t *ret_imap,
-	int		*nmaps,
-	int		found)
+	int		*nmaps)
 {
 	xfs_mount_t	*mp = ip->i_mount;
 	xfs_fileoff_t	offset_fsb;
@@ -279,7 +285,7 @@ xfs_iomap_write_direct(
 		if (error)
 			goto error_out;
 	} else {
-		if (found && (ret_imap->br_startblock == HOLESTARTBLOCK))
+		if (*nmaps && (ret_imap->br_startblock == HOLESTARTBLOCK))
 			last_fsb = MIN(last_fsb, (xfs_fileoff_t)
 					ret_imap->br_blockcount +
 					ret_imap->br_startoff);
@@ -434,7 +440,7 @@ xfs_iomap_eof_want_preallocate(
 	return 0;
 }
 
-int
+STATIC int
 xfs_iomap_write_delay(
 	xfs_inode_t	*ip,
 	xfs_off_t	offset,
@@ -537,7 +543,7 @@ retry:
  * We no longer bother to look at the incoming map - all we have to
  * guarantee is that whatever we allocate fills the required range.
  */
-int
+STATIC int
 xfs_iomap_write_allocate(
 	xfs_inode_t	*ip,
 	xfs_off_t	offset,
Index: xfs/fs/xfs/xfs_iomap.h
===================================================================
--- xfs.orig/fs/xfs/xfs_iomap.h	2010-04-27 18:24:42.787004729 +0200
+++ xfs/fs/xfs/xfs_iomap.h	2010-04-27 18:32:10.115023027 +0200
@@ -44,12 +44,6 @@ struct xfs_bmbt_irec;
 
 extern int xfs_iomap(struct xfs_inode *, xfs_off_t, ssize_t, int,
 		     struct xfs_bmbt_irec *, int *, int *);
-extern int xfs_iomap_write_direct(struct xfs_inode *, xfs_off_t, size_t,
-				  int, struct xfs_bmbt_irec *, int *, int);
-extern int xfs_iomap_write_delay(struct xfs_inode *, xfs_off_t, size_t, int,
-				 struct xfs_bmbt_irec *, int *);
-extern int xfs_iomap_write_allocate(struct xfs_inode *, xfs_off_t, size_t,
-				struct xfs_bmbt_irec *, int *);
 extern int xfs_iomap_write_unwritten(struct xfs_inode *, xfs_off_t, size_t);
 
 #endif /* __XFS_IOMAP_H__*/

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

Re: [PATCH 11/11] xfs: mark xfs_iomap_write_ helpers static

[Alex Elder]

On Wed, 2010-04-28 at 08:29 -0400, Christoph Hellwig wrote:
> plain text document attachment (xfs-iomap-cleanup-12)
> And also drop a useless argument to xfs_iomap_write_direct.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks good.

Reviewed-by: Alex Elder <aelder@sgi.com>

> Index: xfs/fs/xfs/xfs_iomap.c
> ===================================================================


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

Re: [PATCH 00/11] remove xfs_iomap

[Dave Chinner]

On Wed, Apr 28, 2010 at 08:28:50AM -0400, Christoph Hellwig wrote:
> This series gets rid of the xfs_iomap structure which we use to
> communicate between xfs_aops.c and xfs_iomap.c.  It's not a very
> useful structure - we basically need block offsets above and below
> it, but it's in terms of bytes.  Removing it and using the xfs_bmbt_irec
> structure instead thus simplifies the code and reduces the stack footprint
> of the writeback code.

I like the idea - anything we can remove from the writeback path is
useful in terms of stack footprint. I've had a quick look over the
patches and can't see any obvious issues, but I need to do a more
in-depth review of them over the next couple of days. In the mean
time, I'll add them to my QA stack...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

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