[PATCH 00/10] writeback updates

[PATCH 00/10] writeback updates

[Christoph Hellwig]

This serious revamps a lot of the way we do the block mapping for
writeback.  It gets rid of the xfs_iomap interface and now calls
xfs_bmapi directly from __xfs_get_blocks and xfs_vm_writepage,
and simplifies the code in writepage a lot.

It removes about 200 lines of code and a superflous radix tree
lookup for already allocated blocks.

It also shows a pretty clear path to how we can get rid of buffer
heads.  The new writepage code shows that we rely very little on
buffers for writeback - I think we can relatively easy switch it
from looping over bufferheads to loop over the xfs_bmapi results
and it will still work.  We'll need some more work on the get_blocks
and truncate side, but I think we can pretty easily remove the
buffer heads in XFS without any replacement and just rely on the
extent cache in the inode underneath xfs_bmapi.

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

[PATCH 01/10] xfs: improve mapping type check in xfs_vm_writepage

[Christoph Hellwig]

[-- Attachment #1: xfs-fix-iomap-type-checking --]
[-- Type: text/plain, Size: 1745 bytes --]

Currently we only refuse a "read-only" mapping for writing out
unwritten and delayed buffers, and refuse any other for overwrites.
Improve the checks to require delalloc mappings for delayed buffers,
and unwritten extent mappings for unwritten extents.

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-11-19 14:39:37.476261771 +0100
+++ xfs/fs/xfs/linux-2.6/xfs_aops.c	2010-11-19 14:41:41.801005380 +0100
@@ -1127,17 +1127,17 @@ xfs_vm_writepage(
 		if (buffer_unwritten(bh) || buffer_delay(bh)) {
 			int new_ioend = 0;
 
-			/*
-			 * Make sure we don't use a read-only iomap
-			 */
-			if (flags == BMAPI_READ)
-				imap_valid = 0;
-
 			if (buffer_unwritten(bh)) {
-				type = IO_UNWRITTEN;
+				if (type != IO_UNWRITTEN) {
+					type = IO_UNWRITTEN;
+					imap_valid = 0;
+				}
 				flags = BMAPI_WRITE | BMAPI_IGNSTATE;
 			} else if (buffer_delay(bh)) {
-				type = IO_DELAY;
+				if (type != IO_DELAY) {
+					type = IO_DELAY;
+					imap_valid = 0;
+				}
 				flags = BMAPI_ALLOCATE;
 
 				if (wbc->sync_mode == WB_SYNC_NONE)
@@ -1173,8 +1173,11 @@ xfs_vm_writepage(
 			 * That means it must already have extents allocated
 			 * underneath it. Map the extent by reading it.
 			 */
-			if (!imap_valid || flags != BMAPI_READ) {
+			if (flags != BMAPI_READ) {
 				flags = BMAPI_READ;
+				imap_valid = 0;
+			}
+			if (!imap_valid) {
 				size = xfs_probe_cluster(inode, page, bh, head);
 				err = xfs_map_blocks(inode, offset, size,
 						&imap, flags);

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

[PATCH 02/10] xfs: remove some dead bio handling code

[Christoph Hellwig]

[-- Attachment #1: xfs-aop-cleanups --]
[-- Type: text/plain, Size: 1654 bytes --]

We'll never have BIO_EOPNOTSUPP set after calling submit_bio as
this can only happen for discards, and used to happen for barriers,
none of which is every submitted by xfs_submit_ioend_bio.  Also
remove the loop around bio_alloc as it will never fail due to it's
mempool backing.

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-11-19 14:49:28.115261980 +0100
+++ xfs/fs/xfs/linux-2.6/xfs_aops.c	2010-11-19 14:49:29.417024168 +0100
@@ -380,26 +380,18 @@ xfs_submit_ioend_bio(
 
 	submit_bio(wbc->sync_mode == WB_SYNC_ALL ?
 		   WRITE_SYNC_PLUG : WRITE, bio);
-	ASSERT(!bio_flagged(bio, BIO_EOPNOTSUPP));
-	bio_put(bio);
 }
 
 STATIC struct bio *
 xfs_alloc_ioend_bio(
 	struct buffer_head	*bh)
 {
-	struct bio		*bio;
 	int			nvecs = bio_get_nr_vecs(bh->b_bdev);
-
-	do {
-		bio = bio_alloc(GFP_NOIO, nvecs);
-		nvecs >>= 1;
-	} while (!bio);
+	struct bio		*bio = bio_alloc(GFP_NOIO, nvecs);
 
 	ASSERT(bio->bi_private == NULL);
 	bio->bi_sector = bh->b_blocknr * (bh->b_size >> 9);
 	bio->bi_bdev = bh->b_bdev;
-	bio_get(bio);
 	return bio;
 }
 
@@ -470,9 +462,8 @@ xfs_submit_ioend(
 	/* Pass 1 - start writeback */
 	do {
 		next = ioend->io_list;
-		for (bh = ioend->io_buffer_head; bh; bh = bh->b_private) {
+		for (bh = ioend->io_buffer_head; bh; bh = bh->b_private)
 			xfs_start_buffer_writeback(bh);
-		}
 	} while ((ioend = next) != NULL);
 
 	/* Pass 2 - submit I/O */

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

[PATCH 03/10] xfs: a few small tweaks for overwrites in xfs_vm_writepage

[Christoph Hellwig]

[-- Attachment #1: xfs-stop-trylocking-buffers --]
[-- Type: text/plain, Size: 2372 bytes --]

Don't trylock the buffer.  We are the only one ever locking it for
a regular file address space, and trylock was only copied from the
generic code which did it due to the old buffer based writeout in
jbd.  Also make sure to only write out the buffer if the iomap
actually is valid, because we wouldn't have a proper mapping
otherwise.  In practice we will never get an invalid mapping here
as the page lock guarantees truncate doesn't race with us, but
better be safe than sorry.  Also make sure we allocate a new ioend
when crossing boundaries between mappings, just like we do for
delalloc and unwritten extents.  Again this currently doesn't
matter as the I/O end handler only cares for the boundaries for
unwritten extents, but this makes the code fully correct and the
same as for delalloc/unwritten extents.

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-11-19 14:56:20.698273575 +0100
+++ xfs/fs/xfs/linux-2.6/xfs_aops.c	2010-11-19 14:58:28.232030174 +0100
@@ -1096,6 +1096,8 @@ xfs_vm_writepage(
 	type = IO_NEW;
 
 	do {
+		int new_ioend = 0;
+
 		if (offset >= end_offset)
 			break;
 		if (!buffer_uptodate(bh))
@@ -1116,8 +1118,6 @@ xfs_vm_writepage(
 			imap_valid = xfs_imap_valid(inode, &imap, offset);
 
 		if (buffer_unwritten(bh) || buffer_delay(bh)) {
-			int new_ioend = 0;
-
 			if (buffer_unwritten(bh)) {
 				if (type != IO_UNWRITTEN) {
 					type = IO_UNWRITTEN;
@@ -1169,6 +1169,7 @@ xfs_vm_writepage(
 				imap_valid = 0;
 			}
 			if (!imap_valid) {
+				new_ioend = 1;
 				size = xfs_probe_cluster(inode, page, bh, head);
 				err = xfs_map_blocks(inode, offset, size,
 						&imap, flags);
@@ -1187,14 +1188,12 @@ xfs_vm_writepage(
 			 * that we are writing into for the first time.
 			 */
 			type = IO_NEW;
-			if (trylock_buffer(bh)) {
-				if (imap_valid)
-					all_bh = 1;
+			if (imap_valid) {
+				all_bh = 1;
+				lock_buffer(bh);
 				xfs_add_to_ioend(inode, bh, offset, type,
-						&ioend, !imap_valid);
+						&ioend, new_ioend);
 				count++;
-			} else {
-				imap_valid = 0;
 			}
 		} else if (PageUptodate(page)) {
 			ASSERT(buffer_mapped(bh));

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

[PATCH 04/10] xfs: cleanup the xfs_iomap_write_* helpers

[Christoph Hellwig]

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

Remove passing the BMAPI_* flags to these helpers, in xfs_iomap_write_direct
the check BMAPI_DIRECT was always true, and in the xfs_iomap_write_delay
path is was never checked at all.  Remove the nmap return value as
we never make use of it.

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

Index: xfs/fs/xfs/xfs_iomap.c
===================================================================
--- xfs.orig/fs/xfs/xfs_iomap.c	2010-11-02 11:10:15.000103373 -0400
+++ xfs/fs/xfs/xfs_iomap.c	2010-11-02 11:14:14.640103375 -0400
@@ -51,11 +51,11 @@
 #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 *);
+				  struct xfs_bmbt_irec *, int);
+STATIC int xfs_iomap_write_delay(struct xfs_inode *, xfs_off_t, size_t,
+				 struct xfs_bmbt_irec *);
 STATIC int xfs_iomap_write_allocate(struct xfs_inode *, xfs_off_t, size_t,
-				struct xfs_bmbt_irec *, int *);
+				struct xfs_bmbt_irec *);
 
 int
 xfs_iomap(
@@ -134,12 +134,12 @@ xfs_iomap(
 		}
 
 		if (flags & BMAPI_DIRECT) {
-			error = xfs_iomap_write_direct(ip, offset, count, flags,
-						       imap, nimaps);
+			error = xfs_iomap_write_direct(ip, offset, count, imap,
+						       *nimaps);
 		} else {
-			error = xfs_iomap_write_delay(ip, offset, count, flags,
-						      imap, nimaps);
+			error = xfs_iomap_write_delay(ip, offset, count, imap);
 		}
+
 		if (!error) {
 			trace_xfs_iomap_alloc(ip, offset, count, flags, imap);
 		}
@@ -155,13 +155,10 @@ xfs_iomap(
 			break;
 		}
 
-		error = xfs_iomap_write_allocate(ip, offset, count,
-						 imap, nimaps);
+		error = xfs_iomap_write_allocate(ip, offset, count, imap);
 		break;
 	}
 
-	ASSERT(*nimaps <= 1);
-
 out:
 	if (lockmode)
 		xfs_iunlock(ip, lockmode);
@@ -241,9 +238,8 @@ xfs_iomap_write_direct(
 	xfs_inode_t	*ip,
 	xfs_off_t	offset,
 	size_t		count,
-	int		flags,
 	xfs_bmbt_irec_t *imap,
-	int		*nmaps)
+	int		nmaps)
 {
 	xfs_mount_t	*mp = ip->i_mount;
 	xfs_fileoff_t	offset_fsb;
@@ -279,7 +275,7 @@ xfs_iomap_write_direct(
 		if (error)
 			goto error_out;
 	} else {
-		if (*nmaps && (imap->br_startblock == HOLESTARTBLOCK))
+		if (nmaps && (imap->br_startblock == HOLESTARTBLOCK))
 			last_fsb = MIN(last_fsb, (xfs_fileoff_t)
 					imap->br_blockcount +
 					imap->br_startoff);
@@ -331,7 +327,7 @@ xfs_iomap_write_direct(
 	xfs_trans_ijoin(tp, ip);
 
 	bmapi_flag = XFS_BMAPI_WRITE;
-	if ((flags & BMAPI_DIRECT) && (offset < ip->i_size || extsz))
+	if (offset < ip->i_size || extsz)
 		bmapi_flag |= XFS_BMAPI_PREALLOC;
 
 	/*
@@ -370,7 +366,6 @@ xfs_iomap_write_direct(
 		goto error_out;
 	}
 
-	*nmaps = 1;
 	return 0;
 
 error0:	/* Cancel bmap, unlock inode, unreserve quota blocks, cancel trans */
@@ -379,7 +374,6 @@ error0:	/* Cancel bmap, unlock inode, un
 
 error1:	/* Just cancel transaction */
 	xfs_trans_cancel(tp, XFS_TRANS_RELEASE_LOG_RES | XFS_TRANS_ABORT);
-	*nmaps = 0;	/* nothing set-up here */
 
 error_out:
 	return XFS_ERROR(error);
@@ -396,7 +390,6 @@ xfs_iomap_eof_want_preallocate(
 	xfs_inode_t	*ip,
 	xfs_off_t	offset,
 	size_t		count,
-	int		ioflag,
 	xfs_bmbt_irec_t *imap,
 	int		nimaps,
 	int		*prealloc)
@@ -440,9 +433,7 @@ xfs_iomap_write_delay(
 	xfs_inode_t	*ip,
 	xfs_off_t	offset,
 	size_t		count,
-	int		ioflag,
-	xfs_bmbt_irec_t *ret_imap,
-	int		*nmaps)
+	xfs_bmbt_irec_t *ret_imap)
 {
 	xfs_mount_t	*mp = ip->i_mount;
 	xfs_fileoff_t	offset_fsb;
@@ -470,7 +461,7 @@ xfs_iomap_write_delay(
 	offset_fsb = XFS_B_TO_FSBT(mp, offset);
 
 	error = xfs_iomap_eof_want_preallocate(mp, ip, offset, count,
-				ioflag, imap, XFS_WRITE_IMAPS, &prealloc);
+				imap, XFS_WRITE_IMAPS, &prealloc);
 	if (error)
 		return error;
 
@@ -523,8 +514,6 @@ retry:
 		return xfs_cmn_err_fsblock_zero(ip, &imap[0]);
 
 	*ret_imap = imap[0];
-	*nmaps = 1;
-
 	return 0;
 }
 
@@ -543,8 +532,7 @@ xfs_iomap_write_allocate(
 	xfs_inode_t	*ip,
 	xfs_off_t	offset,
 	size_t		count,
-	xfs_bmbt_irec_t *imap,
-	int		*retmap)
+	xfs_bmbt_irec_t *imap)
 {
 	xfs_mount_t	*mp = ip->i_mount;
 	xfs_fileoff_t	offset_fsb, last_block;
@@ -557,8 +545,6 @@ xfs_iomap_write_allocate(
 	int		error = 0;
 	int		nres;
 
-	*retmap = 0;
-
 	/*
 	 * Make sure that the dquots are there.
 	 */
@@ -680,7 +666,6 @@ xfs_iomap_write_allocate(
 		if ((offset_fsb >= imap->br_startoff) &&
 		    (offset_fsb < (imap->br_startoff +
 				   imap->br_blockcount))) {
-			*retmap = 1;
 			XFS_STATS_INC(xs_xstrat_quick);
 			return 0;
 		}

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

[PATCH 05/10] xfs: kill xfs_iomap

[Christoph Hellwig]

[-- Attachment #1: xfs-kill-xfs_iomap --]
[-- Type: text/plain, Size: 19829 bytes --]

Opencode the xfs_iomap code in it's two callers.  The overlap of passed
flags already was minimal and will be further reduced in the next patch.

As a side effect the BMAPI_* flags for xfs_bmapi and the IO_* flags
for I/O end processing are merged into a single set of flags, which
should be a bit more descriptive of the operation we perform.

Also improve the tracing by giving each caller it's own type set of
tracepoints.

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

Index: xfs/fs/xfs/xfs_iomap.h
===================================================================
--- xfs.orig/fs/xfs/xfs_iomap.h	2010-11-19 14:56:26.000000000 +0100
+++ xfs/fs/xfs/xfs_iomap.h	2010-11-19 14:58:34.394254088 +0100
@@ -18,30 +18,15 @@
 #ifndef __XFS_IOMAP_H__
 #define __XFS_IOMAP_H__
 
-/* base extent manipulation calls */
-#define BMAPI_READ	(1 << 0)	/* read extents */
-#define BMAPI_WRITE	(1 << 1)	/* create extents */
-#define BMAPI_ALLOCATE	(1 << 2)	/* delayed allocate to real extents */
-
-/* modifiers */
-#define BMAPI_IGNSTATE	(1 << 4)	/* ignore unwritten state on read */
-#define BMAPI_DIRECT	(1 << 5)	/* direct instead of buffered write */
-#define BMAPI_MMA	(1 << 6)	/* allocate for mmap write */
-#define BMAPI_TRYLOCK	(1 << 7)	/* non-blocking request */
-
-#define BMAPI_FLAGS \
-	{ BMAPI_READ,		"READ" }, \
-	{ BMAPI_WRITE,		"WRITE" }, \
-	{ BMAPI_ALLOCATE,	"ALLOCATE" }, \
-	{ BMAPI_IGNSTATE,	"IGNSTATE" }, \
-	{ BMAPI_DIRECT,		"DIRECT" }, \
-	{ BMAPI_TRYLOCK,	"TRYLOCK" }
-
 struct xfs_inode;
 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,
+			struct xfs_bmbt_irec *, int);
+extern int xfs_iomap_write_delay(struct xfs_inode *, xfs_off_t, size_t,
+			struct xfs_bmbt_irec *);
+extern int xfs_iomap_write_allocate(struct xfs_inode *, xfs_off_t, size_t,
+			struct xfs_bmbt_irec *);
 extern int xfs_iomap_write_unwritten(struct xfs_inode *, xfs_off_t, size_t);
 
 #endif /* __XFS_IOMAP_H__*/
Index: xfs/fs/xfs/linux-2.6/xfs_aops.c
===================================================================
--- xfs.orig/fs/xfs/linux-2.6/xfs_aops.c	2010-11-19 14:58:28.232030174 +0100
+++ xfs/fs/xfs/linux-2.6/xfs_aops.c	2010-11-19 14:59:15.181005660 +0100
@@ -38,15 +38,6 @@
 #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.
@@ -182,9 +173,6 @@ xfs_setfilesize(
 	xfs_inode_t		*ip = XFS_I(ioend->io_inode);
 	xfs_fsize_t		isize;
 
-	ASSERT((ip->i_d.di_mode & S_IFMT) == S_IFREG);
-	ASSERT(ioend->io_type != IO_READ);
-
 	if (unlikely(ioend->io_error))
 		return 0;
 
@@ -244,10 +232,8 @@ xfs_end_io(
 	 * We might have to update the on-disk file size after extending
 	 * writes.
 	 */
-	if (ioend->io_type != IO_READ) {
-		error = xfs_setfilesize(ioend);
-		ASSERT(!error || error == EAGAIN);
-	}
+	error = xfs_setfilesize(ioend);
+	ASSERT(!error || error == EAGAIN);
 
 	/*
 	 * If we didn't complete processing of the ioend, requeue it to the
@@ -320,12 +306,88 @@ xfs_map_blocks(
 	loff_t			offset,
 	ssize_t			count,
 	struct xfs_bmbt_irec	*imap,
-	int			flags)
+	int			type,
+	int			nonblocking)
 {
-	int			nmaps = 1;
-	int			new = 0;
+	struct xfs_inode	*ip = XFS_I(inode);
+	struct xfs_mount	*mp = ip->i_mount;
+	xfs_fileoff_t		offset_fsb, end_fsb;
+	int			error = 0;
+	int			lockmode = 0;
+	int			bmapi_flags = XFS_BMAPI_ENTIRE;
+	int			nimaps = 1;
+
+	if (XFS_FORCED_SHUTDOWN(mp))
+		return -XFS_ERROR(EIO);
+
+	switch (type) {
+	case IO_OVERWRITE:
+		lockmode = xfs_ilock_map_shared(ip);
+		break;
+	case IO_UNWRITTEN:
+		lockmode = XFS_ILOCK_EXCL;
+		bmapi_flags |= XFS_BMAPI_IGSTATE;
+		xfs_ilock(ip, lockmode);
+		break;
+	case IO_DELALLOC:
+		lockmode = XFS_ILOCK_SHARED;
+
+		if (!xfs_ilock_nowait(ip, lockmode)) {
+			if (nonblocking)
+				return -XFS_ERROR(EAGAIN);
+			xfs_ilock(ip, lockmode);
+		}
+		break;
+	}
+
+	ASSERT(offset <= mp->m_maxioffset);
+	if (offset + count > mp->m_maxioffset)
+		count = mp->m_maxioffset - offset;
+	end_fsb = XFS_B_TO_FSB(mp, (xfs_ufsize_t)offset + count);
+	offset_fsb = XFS_B_TO_FSBT(mp, offset);
+
+	error = xfs_bmapi(NULL, ip, offset_fsb, end_fsb - offset_fsb,
+			  bmapi_flags,  NULL, 0, imap, &nimaps, NULL);
+	if (error)
+		goto out;
+
+	switch (type) {
+	case IO_UNWRITTEN:
+		/* If we found an extent, return it */
+		if (nimaps &&
+		    (imap->br_startblock != HOLESTARTBLOCK) &&
+		    (imap->br_startblock != DELAYSTARTBLOCK)) {
+			trace_xfs_map_blocks_found(ip, offset, count, type, imap);
+			break;
+		}
+
+		error = xfs_iomap_write_delay(ip, offset, count, imap);
+		if (!error)
+			trace_xfs_map_blocks_alloc(ip, offset, count, type, imap);
+		break;
+	case IO_DELALLOC:
+		/* If we found an extent, return it */
+		xfs_iunlock(ip, lockmode);
+		lockmode = 0;
+
+		if (nimaps && !isnullstartblock(imap->br_startblock)) {
+			trace_xfs_map_blocks_found(ip, offset, count, type, imap);
+			break;
+		}
+
+		error = xfs_iomap_write_allocate(ip, offset, count, imap);
+		if (!error)
+			trace_xfs_map_blocks_alloc(ip, offset, count, type, imap);
+		break;
+	default:
+		if (nimaps)
+			trace_xfs_map_blocks_found(ip, offset, count, type, imap);
+	}
 
-	return -xfs_iomap(XFS_I(inode), offset, count, flags, imap, &nmaps, &new);
+out:
+	if (lockmode)
+		xfs_iunlock(ip, lockmode);
+	return -XFS_ERROR(error);
 }
 
 STATIC int
@@ -722,9 +784,9 @@ xfs_is_delayed_page(
 			if (buffer_unwritten(bh))
 				acceptable = (type == IO_UNWRITTEN);
 			else if (buffer_delay(bh))
-				acceptable = (type == IO_DELAY);
+				acceptable = (type == IO_DELALLOC);
 			else if (buffer_dirty(bh) && buffer_mapped(bh))
-				acceptable = (type == IO_NEW);
+				acceptable = (type == IO_OVERWRITE);
 			else
 				break;
 		} while ((bh = bh->b_this_page) != head);
@@ -809,7 +871,7 @@ xfs_convert_page(
 			if (buffer_unwritten(bh))
 				type = IO_UNWRITTEN;
 			else
-				type = IO_DELAY;
+				type = IO_DELALLOC;
 
 			if (!xfs_imap_valid(inode, imap, offset)) {
 				done = 1;
@@ -826,7 +888,7 @@ xfs_convert_page(
 			page_dirty--;
 			count++;
 		} else {
-			type = IO_NEW;
+			type = IO_OVERWRITE;
 			if (buffer_mapped(bh) && all_bh) {
 				lock_buffer(bh);
 				xfs_add_to_ioend(inode, bh, offset,
@@ -927,7 +989,7 @@ xfs_aops_discard_page(
 	loff_t			offset = page_offset(page);
 	ssize_t			len = 1 << inode->i_blkbits;
 
-	if (!xfs_is_delayed_page(page, IO_DELAY))
+	if (!xfs_is_delayed_page(page, IO_DELALLOC))
 		goto out_invalidate;
 
 	if (XFS_FORCED_SHUTDOWN(ip->i_mount))
@@ -1039,9 +1101,10 @@ xfs_vm_writepage(
 	__uint64_t              end_offset;
 	pgoff_t                 end_index, last_index;
 	ssize_t			size, len;
-	int			flags, err, imap_valid = 0, uptodate = 1;
+	int			err, imap_valid = 0, uptodate = 1;
 	int			count = 0;
 	int			all_bh = 0;
+	int			nonblocking = 0;
 
 	trace_xfs_writepage(inode, page, 0);
 
@@ -1092,8 +1155,10 @@ xfs_vm_writepage(
 
 	bh = head = page_buffers(page);
 	offset = page_offset(page);
-	flags = BMAPI_READ;
-	type = IO_NEW;
+	type = IO_OVERWRITE;
+
+	if (wbc->sync_mode == WB_SYNC_NONE && wbc->nonblocking)
+		nonblocking = 1;
 
 	do {
 		int new_ioend = 0;
@@ -1123,16 +1188,11 @@ xfs_vm_writepage(
 					type = IO_UNWRITTEN;
 					imap_valid = 0;
 				}
-				flags = BMAPI_WRITE | BMAPI_IGNSTATE;
 			} else if (buffer_delay(bh)) {
-				if (type != IO_DELAY) {
-					type = IO_DELAY;
+				if (type != IO_DELALLOC) {
+					type = IO_DELALLOC;
 					imap_valid = 0;
 				}
-				flags = BMAPI_ALLOCATE;
-
-				if (wbc->sync_mode == WB_SYNC_NONE)
-					flags |= BMAPI_TRYLOCK;
 			}
 
 			if (!imap_valid) {
@@ -1145,8 +1205,8 @@ xfs_vm_writepage(
 				 * for unwritten extent conversion.
 				 */
 				new_ioend = 1;
-				err = xfs_map_blocks(inode, offset, len,
-						&imap, flags);
+				err = xfs_map_blocks(inode, offset, len, &imap,
+						     type, nonblocking);
 				if (err)
 					goto error;
 				imap_valid = xfs_imap_valid(inode, &imap,
@@ -1164,30 +1224,21 @@ xfs_vm_writepage(
 			 * That means it must already have extents allocated
 			 * underneath it. Map the extent by reading it.
 			 */
-			if (flags != BMAPI_READ) {
-				flags = BMAPI_READ;
+			if (type != IO_OVERWRITE) {
+				type = IO_OVERWRITE;
 				imap_valid = 0;
 			}
 			if (!imap_valid) {
 				new_ioend = 1;
 				size = xfs_probe_cluster(inode, page, bh, head);
 				err = xfs_map_blocks(inode, offset, size,
-						&imap, flags);
+						&imap, type, nonblocking);
 				if (err)
 					goto error;
 				imap_valid = xfs_imap_valid(inode, &imap,
 							    offset);
 			}
 
-			/*
-			 * 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 = IO_NEW;
 			if (imap_valid) {
 				all_bh = 1;
 				lock_buffer(bh);
@@ -1295,13 +1346,19 @@ __xfs_get_blocks(
 	int			create,
 	int			direct)
 {
-	int			flags = create ? BMAPI_WRITE : BMAPI_READ;
+	struct xfs_inode	*ip = XFS_I(inode);
+	struct xfs_mount	*mp = ip->i_mount;
+	xfs_fileoff_t		offset_fsb, end_fsb;
+	int			error = 0;
+	int			lockmode = 0;
 	struct xfs_bmbt_irec	imap;
+	int			nimaps = 1;
 	xfs_off_t		offset;
 	ssize_t			size;
-	int			nimap = 1;
 	int			new = 0;
-	int			error;
+
+	if (XFS_FORCED_SHUTDOWN(mp))
+		return -XFS_ERROR(EIO);
 
 	offset = (xfs_off_t)iblock << inode->i_blkbits;
 	ASSERT(bh_result->b_size >= (1 << inode->i_blkbits));
@@ -1310,15 +1367,45 @@ __xfs_get_blocks(
 	if (!create && direct && offset >= i_size_read(inode))
 		return 0;
 
-	if (direct && create)
-		flags |= BMAPI_DIRECT;
+	if (create) {
+		lockmode = XFS_ILOCK_EXCL;
+		xfs_ilock(ip, lockmode);
+	} else {
+		lockmode = xfs_ilock_map_shared(ip);
+	}
+
+	ASSERT(offset <= mp->m_maxioffset);
+	if (offset + size > mp->m_maxioffset)
+		size = mp->m_maxioffset - offset;
+	end_fsb = XFS_B_TO_FSB(mp, (xfs_ufsize_t)offset + size);
+	offset_fsb = XFS_B_TO_FSBT(mp, offset);
 
-	error = xfs_iomap(XFS_I(inode), offset, size, flags, &imap, &nimap,
-			  &new);
+	error = xfs_bmapi(NULL, ip, offset_fsb, end_fsb - offset_fsb,
+			  XFS_BMAPI_ENTIRE,  NULL, 0, &imap, &nimaps, NULL);
 	if (error)
-		return -error;
-	if (nimap == 0)
-		return 0;
+		goto out_unlock;
+
+	if (create &&
+	    (!nimaps ||
+	     (imap.br_startblock == HOLESTARTBLOCK ||
+	      imap.br_startblock == DELAYSTARTBLOCK))) {
+		if (direct) {
+			error = xfs_iomap_write_direct(ip, offset, size,
+						       &imap, nimaps);
+		} else {
+			error = xfs_iomap_write_delay(ip, offset, size, &imap);
+		}
+		if (error)
+			goto out_unlock;
+
+		trace_xfs_get_blocks_alloc(ip, offset, size, 0, &imap);
+	} else if (nimaps) {
+		trace_xfs_get_blocks_found(ip, offset, size, 0, &imap);
+	} else {
+		trace_xfs_get_blocks_notfound(ip, offset, size);
+		goto out_unlock;
+	}
+	xfs_iunlock(ip, lockmode);
 
 	if (imap.br_startblock != HOLESTARTBLOCK &&
 	    imap.br_startblock != DELAYSTARTBLOCK) {
@@ -1385,6 +1472,10 @@ __xfs_get_blocks(
 	}
 
 	return 0;
+
+out_unlock:
+	xfs_iunlock(ip, lockmode);
+	return -error;
 }
 
 int
@@ -1472,7 +1563,7 @@ xfs_vm_direct_IO(
 	ssize_t			ret;
 
 	if (rw & WRITE) {
-		iocb->private = xfs_alloc_ioend(inode, IO_NEW);
+		iocb->private = xfs_alloc_ioend(inode, 0);
 
 		ret = __blockdev_direct_IO(rw, iocb, inode, bdev, iov,
 					    offset, nr_segs,
Index: xfs/fs/xfs/linux-2.6/xfs_trace.h
===================================================================
--- xfs.orig/fs/xfs/linux-2.6/xfs_trace.h	2010-11-19 14:56:26.000000000 +0100
+++ xfs/fs/xfs/linux-2.6/xfs_trace.h	2010-11-19 14:58:34.400253879 +0100
@@ -935,10 +935,10 @@ DEFINE_PAGE_EVENT(xfs_writepage);
 DEFINE_PAGE_EVENT(xfs_releasepage);
 DEFINE_PAGE_EVENT(xfs_invalidatepage);
 
-DECLARE_EVENT_CLASS(xfs_iomap_class,
+DECLARE_EVENT_CLASS(xfs_imap_class,
 	TP_PROTO(struct xfs_inode *ip, xfs_off_t offset, ssize_t count,
-		 int flags, struct xfs_bmbt_irec *irec),
-	TP_ARGS(ip, offset, count, flags, irec),
+		 int type, struct xfs_bmbt_irec *irec),
+	TP_ARGS(ip, offset, count, type, irec),
 	TP_STRUCT__entry(
 		__field(dev_t, dev)
 		__field(xfs_ino_t, ino)
@@ -946,7 +946,7 @@ DECLARE_EVENT_CLASS(xfs_iomap_class,
 		__field(loff_t, new_size)
 		__field(loff_t, offset)
 		__field(size_t, count)
-		__field(int, flags)
+		__field(int, type)
 		__field(xfs_fileoff_t, startoff)
 		__field(xfs_fsblock_t, startblock)
 		__field(xfs_filblks_t, blockcount)
@@ -958,13 +958,13 @@ DECLARE_EVENT_CLASS(xfs_iomap_class,
 		__entry->new_size = ip->i_new_size;
 		__entry->offset = offset;
 		__entry->count = count;
-		__entry->flags = flags;
+		__entry->type = type;
 		__entry->startoff = irec ? irec->br_startoff : 0;
 		__entry->startblock = irec ? irec->br_startblock : 0;
 		__entry->blockcount = irec ? irec->br_blockcount : 0;
 	),
 	TP_printk("dev %d:%d ino 0x%llx size 0x%llx new_size 0x%llx "
-		  "offset 0x%llx count %zd flags %s "
+		  "offset 0x%llx count %zd type %s "
 		  "startoff 0x%llx startblock %lld blockcount 0x%llx",
 		  MAJOR(__entry->dev), MINOR(__entry->dev),
 		  __entry->ino,
@@ -972,20 +972,21 @@ DECLARE_EVENT_CLASS(xfs_iomap_class,
 		  __entry->new_size,
 		  __entry->offset,
 		  __entry->count,
-		  __print_flags(__entry->flags, "|", BMAPI_FLAGS),
+		  __print_symbolic(__entry->type, XFS_IO_TYPES),
 		  __entry->startoff,
 		  (__int64_t)__entry->startblock,
 		  __entry->blockcount)
 )
 
 #define DEFINE_IOMAP_EVENT(name)	\
-DEFINE_EVENT(xfs_iomap_class, name,	\
+DEFINE_EVENT(xfs_imap_class, name,	\
 	TP_PROTO(struct xfs_inode *ip, xfs_off_t offset, ssize_t count,	\
-		 int flags, struct xfs_bmbt_irec *irec),		\
-	TP_ARGS(ip, offset, count, flags, irec))
-DEFINE_IOMAP_EVENT(xfs_iomap_enter);
-DEFINE_IOMAP_EVENT(xfs_iomap_found);
-DEFINE_IOMAP_EVENT(xfs_iomap_alloc);
+		 int type, struct xfs_bmbt_irec *irec),		\
+	TP_ARGS(ip, offset, count, type, irec))
+DEFINE_IOMAP_EVENT(xfs_map_blocks_found);
+DEFINE_IOMAP_EVENT(xfs_map_blocks_alloc);
+DEFINE_IOMAP_EVENT(xfs_get_blocks_found);
+DEFINE_IOMAP_EVENT(xfs_get_blocks_alloc);
 
 DECLARE_EVENT_CLASS(xfs_simple_io_class,
 	TP_PROTO(struct xfs_inode *ip, xfs_off_t offset, ssize_t count),
@@ -1022,6 +1023,7 @@ DEFINE_EVENT(xfs_simple_io_class, name,
 	TP_ARGS(ip, offset, count))
 DEFINE_SIMPLE_IO_EVENT(xfs_delalloc_enospc);
 DEFINE_SIMPLE_IO_EVENT(xfs_unwritten_convert);
+DEFINE_SIMPLE_IO_EVENT(xfs_get_blocks_notfound);
 
 
 TRACE_EVENT(xfs_itruncate_start,
Index: xfs/fs/xfs/linux-2.6/xfs_aops.h
===================================================================
--- xfs.orig/fs/xfs/linux-2.6/xfs_aops.h	2010-11-19 14:56:26.000000000 +0100
+++ xfs/fs/xfs/linux-2.6/xfs_aops.h	2010-11-19 14:58:34.404262190 +0100
@@ -23,6 +23,21 @@ extern struct workqueue_struct *xfsconve
 extern mempool_t *xfs_ioend_pool;
 
 /*
+ * Types of I/O for bmap clustering and I/O completion tracking.
+ */
+enum {
+	IO_DELALLOC = 1,/* mapping covers delalloc region */
+	IO_UNWRITTEN,	/* mapping covers allocated but uninitialized data */
+	IO_OVERWRITE,	/* mapping covers already allocated extent */
+};
+
+#define XFS_IO_TYPES \
+	{ 0,			"" }, \
+	{ IO_DELALLOC,		"delalloc" }, \
+	{ IO_UNWRITTEN,		"unwritten" }, \
+	{ IO_OVERWRITE,		"overwrite" }
+
+/*
  * xfs_ioend struct manages large extent writes for XFS.
  * It can manage several multi-page bio's at once.
  */
Index: xfs/fs/xfs/xfs_iomap.c
===================================================================
--- xfs.orig/fs/xfs/xfs_iomap.c	2010-11-19 14:58:31.000000000 +0100
+++ xfs/fs/xfs/xfs_iomap.c	2010-11-19 14:58:34.406276438 +0100
@@ -47,124 +47,8 @@
 
 #define XFS_WRITEIO_ALIGN(mp,off)	(((off) >> mp->m_writeio_log) \
 						<< mp->m_writeio_log)
-#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,
-				  struct xfs_bmbt_irec *, int);
-STATIC int xfs_iomap_write_delay(struct xfs_inode *, xfs_off_t, size_t,
-				 struct xfs_bmbt_irec *);
-STATIC int xfs_iomap_write_allocate(struct xfs_inode *, xfs_off_t, size_t,
-				struct xfs_bmbt_irec *);
-
-int
-xfs_iomap(
-	struct xfs_inode	*ip,
-	xfs_off_t		offset,
-	ssize_t			count,
-	int			flags,
-	struct xfs_bmbt_irec	*imap,
-	int			*nimaps,
-	int			*new)
-{
-	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);
-
-	*new = 0;
-
-	if (XFS_FORCED_SHUTDOWN(mp))
-		return XFS_ERROR(EIO);
-
-	trace_xfs_iomap_enter(ip, offset, count, flags, NULL);
-
-	switch (flags & (BMAPI_READ | BMAPI_WRITE | BMAPI_ALLOCATE)) {
-	case BMAPI_READ:
-		lockmode = xfs_ilock_map_shared(ip);
-		bmapi_flags = XFS_BMAPI_ENTIRE;
-		break;
-	case BMAPI_WRITE:
-		lockmode = XFS_ILOCK_EXCL;
-		if (flags & BMAPI_IGNSTATE)
-			bmapi_flags |= XFS_BMAPI_IGSTATE|XFS_BMAPI_ENTIRE;
-		xfs_ilock(ip, lockmode);
-		break;
-	case BMAPI_ALLOCATE:
-		lockmode = XFS_ILOCK_SHARED;
-		bmapi_flags = XFS_BMAPI_ENTIRE;
-
-		/* Attempt non-blocking lock */
-		if (flags & BMAPI_TRYLOCK) {
-			if (!xfs_ilock_nowait(ip, lockmode))
-				return XFS_ERROR(EAGAIN);
-		} else {
-			xfs_ilock(ip, lockmode);
-		}
-		break;
-	default:
-		BUG();
-	}
-
-	ASSERT(offset <= mp->m_maxioffset);
-	if ((xfs_fsize_t)offset + count > mp->m_maxioffset)
-		count = mp->m_maxioffset - offset;
-	end_fsb = XFS_B_TO_FSB(mp, (xfs_ufsize_t)offset + count);
-	offset_fsb = XFS_B_TO_FSBT(mp, offset);
-
-	error = xfs_bmapi(NULL, ip, offset_fsb,
-			(xfs_filblks_t)(end_fsb - offset_fsb),
-			bmapi_flags,  NULL, 0, imap,
-			nimaps, NULL);
-
-	if (error)
-		goto out;
-
-	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);
-			break;
-		}
-
-		if (flags & BMAPI_DIRECT) {
-			error = xfs_iomap_write_direct(ip, offset, count, imap,
-						       *nimaps);
-		} else {
-			error = xfs_iomap_write_delay(ip, offset, count, imap);
-		}
-
-		if (!error) {
-			trace_xfs_iomap_alloc(ip, offset, count, flags, imap);
-		}
-		*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);
-			break;
-		}
-
-		error = xfs_iomap_write_allocate(ip, offset, count, imap);
-		break;
-	}
-
-out:
-	if (lockmode)
-		xfs_iunlock(ip, lockmode);
-	return XFS_ERROR(error);
-}
-
 STATIC int
 xfs_iomap_eof_align_last_fsb(
 	xfs_mount_t	*mp,
@@ -233,7 +117,7 @@ xfs_cmn_err_fsblock_zero(
 	return EFSCORRUPTED;
 }
 
-STATIC int
+int
 xfs_iomap_write_direct(
 	xfs_inode_t	*ip,
 	xfs_off_t	offset,
@@ -428,7 +312,7 @@ xfs_iomap_eof_want_preallocate(
 	return 0;
 }
 
-STATIC int
+int
 xfs_iomap_write_delay(
 	xfs_inode_t	*ip,
 	xfs_off_t	offset,
@@ -527,7 +411,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.
  */
-STATIC int
+int
 xfs_iomap_write_allocate(
 	xfs_inode_t	*ip,
 	xfs_off_t	offset,

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

[PATCH 06/10] xfs: simplify xfs_map_blocks

[Christoph Hellwig]

[-- Attachment #1: xfs-simplify-map_blocks --]
[-- Type: text/plain, Size: 3708 bytes --]

No need to lock the extent map exclusive when performing an overwrite,
we know the extent map must already have been loaded by get_blocks.
Apply the non-blocking inode semantics to all mapping types instead of
just delayed allocations.  Remove the handling of not yet allocated
blocks for the IO_UNWRITTEN case - if an extent is marked as delayed
allocated in the buffer it must already have an extent on disk.

Add asserts to verify all the assumptions above in debug builds.

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-10-31 15:10:59.000000000 -0400
+++ xfs/fs/xfs/linux-2.6/xfs_aops.c	2010-10-31 15:13:27.777719469 -0400
@@ -313,81 +313,54 @@ xfs_map_blocks(
 	struct xfs_mount	*mp = ip->i_mount;
 	xfs_fileoff_t		offset_fsb, end_fsb;
 	int			error = 0;
-	int			lockmode = 0;
 	int			bmapi_flags = XFS_BMAPI_ENTIRE;
 	int			nimaps = 1;
 
 	if (XFS_FORCED_SHUTDOWN(mp))
 		return -XFS_ERROR(EIO);
 
-	switch (type) {
-	case IO_OVERWRITE:
-		lockmode = xfs_ilock_map_shared(ip);
-		break;
-	case IO_UNWRITTEN:
-		lockmode = XFS_ILOCK_EXCL;
+	if (type == IO_UNWRITTEN)
 		bmapi_flags |= XFS_BMAPI_IGSTATE;
-		xfs_ilock(ip, lockmode);
-		break;
-	case IO_DELALLOC:
-		lockmode = XFS_ILOCK_SHARED;
-
-		if (!xfs_ilock_nowait(ip, lockmode)) {
-			if (nonblocking)
-				return -XFS_ERROR(EAGAIN);
-			xfs_ilock(ip, lockmode);
-		}
-		break;
+
+	if (!xfs_ilock_nowait(ip, XFS_ILOCK_SHARED)) {
+		if (nonblocking)
+			return -XFS_ERROR(EAGAIN);
+		xfs_ilock(ip, XFS_ILOCK_SHARED);
 	}
 
+	ASSERT(ip->i_d.di_format != XFS_DINODE_FMT_BTREE ||
+	       (ip->i_df.if_flags & XFS_IFEXTENTS));
 	ASSERT(offset <= mp->m_maxioffset);
+
 	if (offset + count > mp->m_maxioffset)
 		count = mp->m_maxioffset - offset;
 	end_fsb = XFS_B_TO_FSB(mp, (xfs_ufsize_t)offset + count);
 	offset_fsb = XFS_B_TO_FSBT(mp, offset);
-
 	error = xfs_bmapi(NULL, ip, offset_fsb, end_fsb - offset_fsb,
 			  bmapi_flags,  NULL, 0, imap, &nimaps, NULL);
-	if (error)
-		goto out;
-
-	switch (type) {
-	case IO_UNWRITTEN:
-		/* If we found an extent, return it */
-		if (nimaps &&
-		    (imap->br_startblock != HOLESTARTBLOCK) &&
-		    (imap->br_startblock != DELAYSTARTBLOCK)) {
-			trace_xfs_map_blocks_found(ip, offset, count, type, imap);
-			break;
-		}
+	xfs_iunlock(ip, XFS_ILOCK_SHARED);
 
-		error = xfs_iomap_write_delay(ip, offset, count, imap);
-		if (!error)
-			trace_xfs_map_blocks_alloc(ip, offset, count, type, imap);
-		break;
-	case IO_DELALLOC:
-		/* If we found an extent, return it */
-		xfs_iunlock(ip, lockmode);
-		lockmode = 0;
-
-		if (nimaps && !isnullstartblock(imap->br_startblock)) {
-			trace_xfs_map_blocks_found(ip, offset, count, type, imap);
-			break;
-		}
+	if (error)
+		return -XFS_ERROR(error);
 
+	if (type == IO_DELALLOC &&
+	    (!nimaps || isnullstartblock(imap->br_startblock))) {
 		error = xfs_iomap_write_allocate(ip, offset, count, imap);
 		if (!error)
 			trace_xfs_map_blocks_alloc(ip, offset, count, type, imap);
-		break;
-	default:
-		if (nimaps)
-			trace_xfs_map_blocks_found(ip, offset, count, type, imap);
+		return -XFS_ERROR(error);
 	}
 
-out:
-	if (lockmode)
-		xfs_iunlock(ip, lockmode);
-	return -XFS_ERROR(error);
+#ifdef DEBUG
+	if (type == IO_UNWRITTEN) {
+		ASSERT(nimaps);
+		ASSERT(imap->br_startblock != HOLESTARTBLOCK);
+		ASSERT(imap->br_startblock != DELAYSTARTBLOCK);
+	}
+#endif
+	if (nimaps)
+		trace_xfs_map_blocks_found(ip, offset, count, type, imap);
+	return 0;
 }
 
 STATIC int

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

[PATCH 07/10] xfs: remove xfs_probe_cluster

[Christoph Hellwig]

[-- Attachment #1: xfs-kill-xfs_probe_cluster --]
[-- Type: text/plain, Size: 4252 bytes --]

xfs_map_blocks always calls xfs_bmapi with the XFS_BMAPI_ENTIRE entire
flag, which tells it to not cap the extent at the passed in size, but
just treat the size as an minimum to map.  This means xfs_probe_cluster
is entirely useless as we'll always get the whole extent back anyway.

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-11-19 15:07:17.349281326 +0100
+++ xfs/fs/xfs/linux-2.6/xfs_aops.c	2010-11-19 15:07:33.141005552 +0100
@@ -304,13 +304,13 @@ STATIC int
 xfs_map_blocks(
 	struct inode		*inode,
 	loff_t			offset,
-	ssize_t			count,
 	struct xfs_bmbt_irec	*imap,
 	int			type,
 	int			nonblocking)
 {
 	struct xfs_inode	*ip = XFS_I(inode);
 	struct xfs_mount	*mp = ip->i_mount;
+	ssize_t			count = 1 << inode->i_blkbits;
 	xfs_fileoff_t		offset_fsb, end_fsb;
 	int			error = 0;
 	int			bmapi_flags = XFS_BMAPI_ENTIRE;
@@ -635,108 +635,6 @@ xfs_map_at_offset(
 }
 
 /*
- * Look for a page at index that is suitable for clustering.
- */
-STATIC unsigned int
-xfs_probe_page(
-	struct page		*page,
-	unsigned int		pg_offset)
-{
-	struct buffer_head	*bh, *head;
-	int			ret = 0;
-
-	if (PageWriteback(page))
-		return 0;
-	if (!PageDirty(page))
-		return 0;
-	if (!page->mapping)
-		return 0;
-	if (!page_has_buffers(page))
-		return 0;
-
-	bh = head = page_buffers(page);
-	do {
-		if (!buffer_uptodate(bh))
-			break;
-		if (!buffer_mapped(bh))
-			break;
-		ret += bh->b_size;
-		if (ret >= pg_offset)
-			break;
-	} while ((bh = bh->b_this_page) != head);
-
-	return ret;
-}
-
-STATIC size_t
-xfs_probe_cluster(
-	struct inode		*inode,
-	struct page		*startpage,
-	struct buffer_head	*bh,
-	struct buffer_head	*head)
-{
-	struct pagevec		pvec;
-	pgoff_t			tindex, tlast, tloff;
-	size_t			total = 0;
-	int			done = 0, i;
-
-	/* First sum forwards in this page */
-	do {
-		if (!buffer_uptodate(bh) || !buffer_mapped(bh))
-			return total;
-		total += bh->b_size;
-	} while ((bh = bh->b_this_page) != head);
-
-	/* if we reached the end of the page, sum forwards in following pages */
-	tlast = i_size_read(inode) >> PAGE_CACHE_SHIFT;
-	tindex = startpage->index + 1;
-
-	/* Prune this back to avoid pathological behavior */
-	tloff = min(tlast, startpage->index + 64);
-
-	pagevec_init(&pvec, 0);
-	while (!done && tindex <= tloff) {
-		unsigned len = min_t(pgoff_t, PAGEVEC_SIZE, tlast - tindex + 1);
-
-		if (!pagevec_lookup(&pvec, inode->i_mapping, tindex, len))
-			break;
-
-		for (i = 0; i < pagevec_count(&pvec); i++) {
-			struct page *page = pvec.pages[i];
-			size_t pg_offset, pg_len = 0;
-
-			if (tindex == tlast) {
-				pg_offset =
-				    i_size_read(inode) & (PAGE_CACHE_SIZE - 1);
-				if (!pg_offset) {
-					done = 1;
-					break;
-				}
-			} else
-				pg_offset = PAGE_CACHE_SIZE;
-
-			if (page->index == tindex && trylock_page(page)) {
-				pg_len = xfs_probe_page(page, pg_offset);
-				unlock_page(page);
-			}
-
-			if (!pg_len) {
-				done = 1;
-				break;
-			}
-
-			total += pg_len;
-			tindex++;
-		}
-
-		pagevec_release(&pvec);
-		cond_resched();
-	}
-
-	return total;
-}
-
-/*
  * Test if a given page is suitable for writing as part of an unwritten
  * or delayed allocate extent.
  */
@@ -1073,7 +971,7 @@ xfs_vm_writepage(
 	unsigned int		type;
 	__uint64_t              end_offset;
 	pgoff_t                 end_index, last_index;
-	ssize_t			size, len;
+	ssize_t			len;
 	int			err, imap_valid = 0, uptodate = 1;
 	int			count = 0;
 	int			all_bh = 0;
@@ -1178,7 +1076,7 @@ xfs_vm_writepage(
 				 * for unwritten extent conversion.
 				 */
 				new_ioend = 1;
-				err = xfs_map_blocks(inode, offset, len, &imap,
+				err = xfs_map_blocks(inode, offset, &imap,
 						     type, nonblocking);
 				if (err)
 					goto error;
@@ -1203,8 +1101,7 @@ xfs_vm_writepage(
 			}
 			if (!imap_valid) {
 				new_ioend = 1;
-				size = xfs_probe_cluster(inode, page, bh, head);
-				err = xfs_map_blocks(inode, offset, size,
+				err = xfs_map_blocks(inode, offset,
 						&imap, type, nonblocking);
 				if (err)
 					goto error;

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

[PATCH 08/10] xfs: remove the all_bh flag from xfs_convert_page

[Christoph Hellwig]

[-- Attachment #1: xfs-kill-all-bh --]
[-- Type: text/plain, Size: 2980 bytes --]

The all_bh flag is always set when entering the page clustering machinery
with a regular written extent, which means the check for it is superflous.

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-11-19 15:07:33.000000000 +0100
+++ xfs/fs/xfs/linux-2.6/xfs_aops.c	2010-11-19 15:07:59.221005096 +0100
@@ -682,8 +682,7 @@ xfs_convert_page(
 	loff_t			tindex,
 	struct xfs_bmbt_irec	*imap,
 	xfs_ioend_t		**ioendp,
-	struct writeback_control *wbc,
-	int			all_bh)
+	struct writeback_control *wbc)
 {
 	struct buffer_head	*bh, *head;
 	xfs_off_t		end_offset;
@@ -738,11 +737,14 @@ xfs_convert_page(
 			continue;
 		}
 
-		if (buffer_unwritten(bh) || buffer_delay(bh)) {
+		if (buffer_unwritten(bh) || buffer_delay(bh) ||
+		    buffer_mapped(bh)) {
 			if (buffer_unwritten(bh))
 				type = IO_UNWRITTEN;
-			else
+			else if (buffer_delay(bh))
 				type = IO_DELALLOC;
+			else
+				type = IO_OVERWRITE;
 
 			if (!xfs_imap_valid(inode, imap, offset)) {
 				done = 1;
@@ -752,23 +754,17 @@ xfs_convert_page(
 			ASSERT(imap->br_startblock != HOLESTARTBLOCK);
 			ASSERT(imap->br_startblock != DELAYSTARTBLOCK);
 
-			xfs_map_at_offset(inode, bh, imap, offset);
+			if (type == IO_OVERWRITE)
+				lock_buffer(bh);
+			else
+				xfs_map_at_offset(inode, bh, imap, offset);
 			xfs_add_to_ioend(inode, bh, offset, type,
 					 ioendp, done);
 
 			page_dirty--;
 			count++;
 		} else {
-			type = IO_OVERWRITE;
-			if (buffer_mapped(bh) && all_bh) {
-				lock_buffer(bh);
-				xfs_add_to_ioend(inode, bh, offset,
-						type, ioendp, done);
-				count++;
-				page_dirty--;
-			} else {
-				done = 1;
-			}
+			done = 1;
 		}
 	} while (offset += len, (bh = bh->b_this_page) != head);
 
@@ -800,7 +796,6 @@ xfs_cluster_write(
 	struct xfs_bmbt_irec	*imap,
 	xfs_ioend_t		**ioendp,
 	struct writeback_control *wbc,
-	int			all_bh,
 	pgoff_t			tlast)
 {
 	struct pagevec		pvec;
@@ -815,7 +810,7 @@ xfs_cluster_write(
 
 		for (i = 0; i < pagevec_count(&pvec); i++) {
 			done = xfs_convert_page(inode, pvec.pages[i], tindex++,
-					imap, ioendp, wbc, all_bh);
+					imap, ioendp, wbc);
 			if (done)
 				break;
 		}
@@ -974,7 +969,6 @@ xfs_vm_writepage(
 	ssize_t			len;
 	int			err, imap_valid = 0, uptodate = 1;
 	int			count = 0;
-	int			all_bh = 0;
 	int			nonblocking = 0;
 
 	trace_xfs_writepage(inode, page, 0);
@@ -1110,7 +1104,6 @@ xfs_vm_writepage(
 			}
 
 			if (imap_valid) {
-				all_bh = 1;
 				lock_buffer(bh);
 				xfs_add_to_ioend(inode, bh, offset, type,
 						&ioend, new_ioend);
@@ -1147,7 +1140,7 @@ xfs_vm_writepage(
 			end_index = last_index;
 
 		xfs_cluster_write(inode, page->index + 1, &imap, &ioend,
-					wbc, all_bh, end_index);
+				  wbc, end_index);
 	}
 
 	if (iohead)

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

[PATCH 09/10] xfs: refactor xfs_vm_writepage

[Christoph Hellwig]

[-- Attachment #1: xfs-factor-writepage --]
[-- Type: text/plain, Size: 3970 bytes --]

After the last patches the code for overwrites is the same as for
delayed and unwritten extents except that it doesn't need to call
xfs_map_at_offset.  Take care of that fact to simplify xfs_vm_writepage.

The buffer loop now first checks the type of buffer and checks/sets
the ioend type, or continues to the next buffer if it's not interesting
to us.  Only after that we validate the iomap and perform the block
mapping if needed, all in common code for the cases where we have to
do work.

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-11-19 15:47:24.164253529 +0100
+++ xfs/fs/xfs/linux-2.6/xfs_aops.c	2010-11-19 17:05:03.221254018 +0100
@@ -1044,74 +1044,55 @@ xfs_vm_writepage(
 			continue;
 		}
 
-		if (imap_valid)
-			imap_valid = xfs_imap_valid(inode, &imap, offset);
-
-		if (buffer_unwritten(bh) || buffer_delay(bh)) {
-			if (buffer_unwritten(bh)) {
-				if (type != IO_UNWRITTEN) {
-					type = IO_UNWRITTEN;
-					imap_valid = 0;
-				}
-			} else if (buffer_delay(bh)) {
-				if (type != IO_DELALLOC) {
-					type = IO_DELALLOC;
-					imap_valid = 0;
-				}
-			}
-
-			if (!imap_valid) {
-				/*
-				 * If we didn't have a valid mapping then we
-				 * need to ensure that we put the new mapping
-				 * in a new ioend structure. This needs to be
-				 * done to ensure that the ioends correctly
-				 * reflect the block mappings at io completion
-				 * for unwritten extent conversion.
-				 */
-				new_ioend = 1;
-				err = xfs_map_blocks(inode, offset, &imap,
-						     type, nonblocking);
-				if (err)
-					goto error;
-				imap_valid = xfs_imap_valid(inode, &imap,
-							    offset);
+		if (buffer_unwritten(bh)) {
+			if (type != IO_UNWRITTEN) {
+				type = IO_UNWRITTEN;
+				imap_valid = 0;
 			}
-			if (imap_valid) {
-				xfs_map_at_offset(inode, bh, &imap, offset);
-				xfs_add_to_ioend(inode, bh, offset, type,
-						 &ioend, new_ioend);
-				count++;
+		} else if (buffer_delay(bh)) {
+			if (type != IO_DELALLOC) {
+				type = IO_DELALLOC;
+				imap_valid = 0;
 			}
 		} else if (buffer_uptodate(bh)) {
-			/*
-			 * we got here because the buffer is already mapped.
-			 * That means it must already have extents allocated
-			 * underneath it. Map the extent by reading it.
-			 */
 			if (type != IO_OVERWRITE) {
 				type = IO_OVERWRITE;
 				imap_valid = 0;
 			}
-			if (!imap_valid) {
-				new_ioend = 1;
-				err = xfs_map_blocks(inode, offset,
-						&imap, type, nonblocking);
-				if (err)
-					goto error;
-				imap_valid = xfs_imap_valid(inode, &imap,
-							    offset);
+		} else {
+			if (PageUptodate(page)) {
+				ASSERT(buffer_mapped(bh));
+				imap_valid = 0;
 			}
+			continue;
+		}
 
-			if (imap_valid) {
+		if (imap_valid)
+			imap_valid = xfs_imap_valid(inode, &imap, offset);
+		if (!imap_valid) {
+			/*
+			 * If we didn't have a valid mapping then we need to
+			 * put the new mapping into a separate ioend structure.
+			 * This ensures non-contiguous extents always have
+			 * separate ioends, which is particularly important
+			 * for unwritten extent conversion at I/O completion
+			 * time.
+			 */
+			new_ioend = 1;
+			err = xfs_map_blocks(inode, offset, &imap, type,
+					     nonblocking);
+			if (err)
+				goto error;
+			imap_valid = xfs_imap_valid(inode, &imap, offset);
+		}
+		if (imap_valid) {
+			if (type == IO_OVERWRITE)
 				lock_buffer(bh);
-				xfs_add_to_ioend(inode, bh, offset, type,
-						&ioend, new_ioend);
-				count++;
-			}
-		} else if (PageUptodate(page)) {
-			ASSERT(buffer_mapped(bh));
-			imap_valid = 0;
+			else
+				xfs_map_at_offset(inode, bh, &imap, offset);
+			xfs_add_to_ioend(inode, bh, offset, type, &ioend,
+					 new_ioend);
+			count++;
 		}
 
 		if (!iohead)

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

[PATCH 10/10] xfs: simplify xfs_map_at_offset

[Christoph Hellwig]

[-- Attachment #1: xfs-simplify-xfs_map_at_offset --]
[-- Type: text/plain, Size: 1783 bytes --]

Move the buffer locking into the callers as they need to do it wether they
call xfs_map_at_offset or not.  Remove the b_bdev assignment, which is
already done by get_blocks.  Remove the duplicate extent type asserts in
xfs_convert_page just before calling xfs_map_at_offset.

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-11-19 16:55:59.276019838 +0100
+++ xfs/fs/xfs/linux-2.6/xfs_aops.c	2010-11-19 16:58:45.791254577 +0100
@@ -626,9 +626,7 @@ xfs_map_at_offset(
 	ASSERT(imap->br_startblock != HOLESTARTBLOCK);
 	ASSERT(imap->br_startblock != DELAYSTARTBLOCK);
 
-	lock_buffer(bh);
 	xfs_map_buffer(inode, bh, imap, offset);
-	bh->b_bdev = xfs_find_bdev_for_inode(inode);
 	set_buffer_mapped(bh);
 	clear_buffer_delay(bh);
 	clear_buffer_unwritten(bh);
@@ -751,12 +749,8 @@ xfs_convert_page(
 				continue;
 			}
 
-			ASSERT(imap->br_startblock != HOLESTARTBLOCK);
-			ASSERT(imap->br_startblock != DELAYSTARTBLOCK);
-
-			if (type == IO_OVERWRITE)
-				lock_buffer(bh);
-			else
+			lock_buffer(bh);
+			if (type != IO_OVERWRITE)
 				xfs_map_at_offset(inode, bh, imap, offset);
 			xfs_add_to_ioend(inode, bh, offset, type,
 					 ioendp, done);
@@ -1086,9 +1080,8 @@ xfs_vm_writepage(
 			imap_valid = xfs_imap_valid(inode, &imap, offset);
 		}
 		if (imap_valid) {
-			if (type == IO_OVERWRITE)
-				lock_buffer(bh);
-			else
+			lock_buffer(bh);
+			if (type != IO_OVERWRITE)
 				xfs_map_at_offset(inode, bh, &imap, offset);
 			xfs_add_to_ioend(inode, bh, offset, type, &ioend,
 					 new_ioend);

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

Re: [PATCH 01/10] xfs: improve mapping type check in xfs_vm_writepage

[Dave Chinner]

On Mon, Nov 22, 2010 at 08:05:07AM -0500, Christoph Hellwig wrote:
> Currently we only refuse a "read-only" mapping for writing out
> unwritten and delayed buffers, and refuse any other for overwrites.
> Improve the checks to require delalloc mappings for delayed buffers,
> and unwritten extent mappings for unwritten extents.
> 
> 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

Re: [PATCH 02/10] xfs: remove some dead bio handling code

[Dave Chinner]

On Mon, Nov 22, 2010 at 08:05:08AM -0500, Christoph Hellwig wrote:
> We'll never have BIO_EOPNOTSUPP set after calling submit_bio as
> this can only happen for discards, and used to happen for barriers,
> none of which is every submitted by xfs_submit_ioend_bio.  Also
> remove the loop around bio_alloc as it will never fail due to it's
> mempool backing.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

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

Re: [PATCH 03/10] xfs: a few small tweaks for overwrites in xfs_vm_writepage

[Dave Chinner]

On Mon, Nov 22, 2010 at 08:05:09AM -0500, Christoph Hellwig wrote:
> Don't trylock the buffer.  We are the only one ever locking it for
> a regular file address space, and trylock was only copied from the
> generic code which did it due to the old buffer based writeout in
> jbd.  Also make sure to only write out the buffer if the iomap
> actually is valid, because we wouldn't have a proper mapping
> otherwise.  In practice we will never get an invalid mapping here
> as the page lock guarantees truncate doesn't race with us, but
> better be safe than sorry.  Also make sure we allocate a new ioend
> when crossing boundaries between mappings, just like we do for
> delalloc and unwritten extents.  Again this currently doesn't
> matter as the I/O end handler only cares for the boundaries for
> unwritten extents, but this makes the code fully correct and the
> same as for delalloc/unwritten extents.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

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

Re: [PATCH 04/10] xfs: cleanup the xfs_iomap_write_* helpers

[Dave Chinner]

On Mon, Nov 22, 2010 at 08:05:10AM -0500, Christoph Hellwig wrote:
> Remove passing the BMAPI_* flags to these helpers, in xfs_iomap_write_direct
> the check BMAPI_DIRECT was always true, and in the xfs_iomap_write_delay
> path is was never checked at all.  Remove the nmap return value as
> we never make use of it.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Makes sense - fewer useless parameters being passed around is 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

Re: [PATCH 05/10] xfs: kill xfs_iomap

[Dave Chinner]

On Mon, Nov 22, 2010 at 08:05:11AM -0500, Christoph Hellwig wrote:
> Opencode the xfs_iomap code in it's two callers.  The overlap of passed
> flags already was minimal and will be further reduced in the next patch.
> 
> As a side effect the BMAPI_* flags for xfs_bmapi and the IO_* flags
> for I/O end processing are merged into a single set of flags, which
> should be a bit more descriptive of the operation we perform.
> 
> Also improve the tracing by giving each caller it's own type set of
> tracepoints.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks ok. Minor comment:

> @@ -1472,7 +1563,7 @@ xfs_vm_direct_IO(
>  	ssize_t			ret;
>  
>  	if (rw & WRITE) {
> -		iocb->private = xfs_alloc_ioend(inode, IO_NEW);
> +		iocb->private = xfs_alloc_ioend(inode, 0);
>  
>  		ret = __blockdev_direct_IO(rw, iocb, inode, bdev, iov,
>  					    offset, nr_segs,

Using an ioend type to "0" is not very obvious given all the other
uses have a defined type. I know that this converted to IO_UNWRITTEN
in IO completion if neceessary, but perhaps a IO_DIRECT type might
be better just to document it? Or perhaps a comment stating why 0 is
OK to use here?

Everything else looks fine, so:

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

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

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

Re: [PATCH 06/10] xfs: simplify xfs_map_blocks

[Dave Chinner]

On Mon, Nov 22, 2010 at 08:05:12AM -0500, Christoph Hellwig wrote:
> No need to lock the extent map exclusive when performing an overwrite,
> we know the extent map must already have been loaded by get_blocks.
> Apply the non-blocking inode semantics to all mapping types instead of
> just delayed allocations.  Remove the handling of not yet allocated
> blocks for the IO_UNWRITTEN case - if an extent is marked as delayed
> allocated in the buffer it must already have an extent on disk.

I think your meant "marked as unwritten in the buffer" there.

> Add asserts to verify all the assumptions above in debug builds.

Nice - I would have asked that to be added ;)

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

Looks good. It is much simpler now.

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

Re: [PATCH 07/10] xfs: remove xfs_probe_cluster

[Dave Chinner]

On Mon, Nov 22, 2010 at 08:05:13AM -0500, Christoph Hellwig wrote:
> xfs_map_blocks always calls xfs_bmapi with the XFS_BMAPI_ENTIRE entire
> flag, which tells it to not cap the extent at the passed in size, but
> just treat the size as an minimum to map.  This means xfs_probe_cluster
> is entirely useless as we'll always get the whole extent back anyway.
> 
> 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

Re: [PATCH 08/10] xfs: remove the all_bh flag from xfs_convert_page

[Dave Chinner]

On Mon, Nov 22, 2010 at 08:05:14AM -0500, Christoph Hellwig wrote:
> The all_bh flag is always set when entering the page clustering machinery
> with a regular written extent, which means the check for it is superflous.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

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

Re: [PATCH 09/10] xfs: refactor xfs_vm_writepage

[Dave Chinner]

On Mon, Nov 22, 2010 at 08:05:15AM -0500, Christoph Hellwig wrote:
> After the last patches the code for overwrites is the same as for
> delayed and unwritten extents except that it doesn't need to call
> xfs_map_at_offset.  Take care of that fact to simplify xfs_vm_writepage.
> 
> The buffer loop now first checks the type of buffer and checks/sets
> the ioend type, or continues to the next buffer if it's not interesting
> to us.  Only after that we validate the iomap and perform the block
> mapping if needed, all in common code for the cases where we have to
> do work.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Much neater and easier to understand.

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

Re: [PATCH 10/10] xfs: simplify xfs_map_at_offset

[Dave Chinner]

On Mon, Nov 22, 2010 at 08:05:16AM -0500, Christoph Hellwig wrote:
> Move the buffer locking into the callers as they need to do it wether they
> call xfs_map_at_offset or not.  Remove the b_bdev assignment, which is
> already done by get_blocks.  Remove the duplicate extent type asserts in
> xfs_convert_page just before calling xfs_map_at_offset.
> 
> 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

[PATCH 02/10] xfs: remove some dead bio handling code

[Christoph Hellwig]

[-- Attachment #1: xfs-aop-cleanups --]
[-- Type: text/plain, Size: 1702 bytes --]

We'll never have BIO_EOPNOTSUPP set after calling submit_bio as
this can only happen for discards, and used to happen for barriers,
none of which is every submitted by xfs_submit_ioend_bio.  Also
remove the loop around bio_alloc as it will never fail due to it's
mempool backing.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Dave Chinner <dchinner@redhat.com>

Index: xfs/fs/xfs/linux-2.6/xfs_aops.c
===================================================================
--- xfs.orig/fs/xfs/linux-2.6/xfs_aops.c	2010-11-19 14:49:28.115261980 +0100
+++ xfs/fs/xfs/linux-2.6/xfs_aops.c	2010-11-19 14:49:29.417024168 +0100
@@ -380,26 +380,18 @@ xfs_submit_ioend_bio(
 
 	submit_bio(wbc->sync_mode == WB_SYNC_ALL ?
 		   WRITE_SYNC_PLUG : WRITE, bio);
-	ASSERT(!bio_flagged(bio, BIO_EOPNOTSUPP));
-	bio_put(bio);
 }
 
 STATIC struct bio *
 xfs_alloc_ioend_bio(
 	struct buffer_head	*bh)
 {
-	struct bio		*bio;
 	int			nvecs = bio_get_nr_vecs(bh->b_bdev);
-
-	do {
-		bio = bio_alloc(GFP_NOIO, nvecs);
-		nvecs >>= 1;
-	} while (!bio);
+	struct bio		*bio = bio_alloc(GFP_NOIO, nvecs);
 
 	ASSERT(bio->bi_private == NULL);
 	bio->bi_sector = bh->b_blocknr * (bh->b_size >> 9);
 	bio->bi_bdev = bh->b_bdev;
-	bio_get(bio);
 	return bio;
 }
 
@@ -470,9 +462,8 @@ xfs_submit_ioend(
 	/* Pass 1 - start writeback */
 	do {
 		next = ioend->io_list;
-		for (bh = ioend->io_buffer_head; bh; bh = bh->b_private) {
+		for (bh = ioend->io_buffer_head; bh; bh = bh->b_private)
 			xfs_start_buffer_writeback(bh);
-		}
 	} while ((ioend = next) != NULL);
 
 	/* Pass 2 - submit I/O */

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