* [PATCH 01/10] xfs: improve mapping type check in xfs_vm_writepage
2010-11-22 13:05 [PATCH 00/10] writeback updates Christoph Hellwig
@ 2010-11-22 13:05 ` Christoph Hellwig
2010-12-01 4:01 ` Dave Chinner
2010-11-22 13:05 ` [PATCH 02/10] xfs: remove some dead bio handling code Christoph Hellwig
` (8 subsequent siblings)
9 siblings, 1 reply; 22+ messages in thread
From: Christoph Hellwig @ 2010-11-22 13:05 UTC (permalink / raw)
To: xfs
[-- 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
^ permalink raw reply [flat|nested] 22+ messages in thread* [PATCH 02/10] xfs: remove some dead bio handling code
2010-11-22 13:05 [PATCH 00/10] writeback updates Christoph Hellwig
2010-11-22 13:05 ` [PATCH 01/10] xfs: improve mapping type check in xfs_vm_writepage Christoph Hellwig
@ 2010-11-22 13:05 ` Christoph Hellwig
2010-12-01 4:02 ` Dave Chinner
2010-11-22 13:05 ` [PATCH 03/10] xfs: a few small tweaks for overwrites in xfs_vm_writepage Christoph Hellwig
` (7 subsequent siblings)
9 siblings, 1 reply; 22+ messages in thread
From: Christoph Hellwig @ 2010-11-22 13:05 UTC (permalink / raw)
To: xfs
[-- 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
^ permalink raw reply [flat|nested] 22+ messages in thread* [PATCH 03/10] xfs: a few small tweaks for overwrites in xfs_vm_writepage
2010-11-22 13:05 [PATCH 00/10] writeback updates Christoph Hellwig
2010-11-22 13:05 ` [PATCH 01/10] xfs: improve mapping type check in xfs_vm_writepage Christoph Hellwig
2010-11-22 13:05 ` [PATCH 02/10] xfs: remove some dead bio handling code Christoph Hellwig
@ 2010-11-22 13:05 ` Christoph Hellwig
2010-12-01 4:07 ` Dave Chinner
2010-11-22 13:05 ` [PATCH 04/10] xfs: cleanup the xfs_iomap_write_* helpers Christoph Hellwig
` (6 subsequent siblings)
9 siblings, 1 reply; 22+ messages in thread
From: Christoph Hellwig @ 2010-11-22 13:05 UTC (permalink / raw)
To: xfs
[-- 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
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH 03/10] xfs: a few small tweaks for overwrites in xfs_vm_writepage
2010-11-22 13:05 ` [PATCH 03/10] xfs: a few small tweaks for overwrites in xfs_vm_writepage Christoph Hellwig
@ 2010-12-01 4:07 ` Dave Chinner
0 siblings, 0 replies; 22+ messages in thread
From: Dave Chinner @ 2010-12-01 4:07 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: xfs
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
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 04/10] xfs: cleanup the xfs_iomap_write_* helpers
2010-11-22 13:05 [PATCH 00/10] writeback updates Christoph Hellwig
` (2 preceding siblings ...)
2010-11-22 13:05 ` [PATCH 03/10] xfs: a few small tweaks for overwrites in xfs_vm_writepage Christoph Hellwig
@ 2010-11-22 13:05 ` Christoph Hellwig
2010-12-01 4:11 ` Dave Chinner
2010-11-22 13:05 ` [PATCH 05/10] xfs: kill xfs_iomap Christoph Hellwig
` (5 subsequent siblings)
9 siblings, 1 reply; 22+ messages in thread
From: Christoph Hellwig @ 2010-11-22 13:05 UTC (permalink / raw)
To: xfs
[-- 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
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH 04/10] xfs: cleanup the xfs_iomap_write_* helpers
2010-11-22 13:05 ` [PATCH 04/10] xfs: cleanup the xfs_iomap_write_* helpers Christoph Hellwig
@ 2010-12-01 4:11 ` Dave Chinner
0 siblings, 0 replies; 22+ messages in thread
From: Dave Chinner @ 2010-12-01 4:11 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: xfs
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
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 05/10] xfs: kill xfs_iomap
2010-11-22 13:05 [PATCH 00/10] writeback updates Christoph Hellwig
` (3 preceding siblings ...)
2010-11-22 13:05 ` [PATCH 04/10] xfs: cleanup the xfs_iomap_write_* helpers Christoph Hellwig
@ 2010-11-22 13:05 ` Christoph Hellwig
2010-12-01 4:32 ` Dave Chinner
2010-11-22 13:05 ` [PATCH 06/10] xfs: simplify xfs_map_blocks Christoph Hellwig
` (4 subsequent siblings)
9 siblings, 1 reply; 22+ messages in thread
From: Christoph Hellwig @ 2010-11-22 13:05 UTC (permalink / raw)
To: xfs
[-- 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
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH 05/10] xfs: kill xfs_iomap
2010-11-22 13:05 ` [PATCH 05/10] xfs: kill xfs_iomap Christoph Hellwig
@ 2010-12-01 4:32 ` Dave Chinner
0 siblings, 0 replies; 22+ messages in thread
From: Dave Chinner @ 2010-12-01 4:32 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: xfs
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
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 06/10] xfs: simplify xfs_map_blocks
2010-11-22 13:05 [PATCH 00/10] writeback updates Christoph Hellwig
` (4 preceding siblings ...)
2010-11-22 13:05 ` [PATCH 05/10] xfs: kill xfs_iomap Christoph Hellwig
@ 2010-11-22 13:05 ` Christoph Hellwig
2010-12-01 4:41 ` Dave Chinner
2010-11-22 13:05 ` [PATCH 07/10] xfs: remove xfs_probe_cluster Christoph Hellwig
` (3 subsequent siblings)
9 siblings, 1 reply; 22+ messages in thread
From: Christoph Hellwig @ 2010-11-22 13:05 UTC (permalink / raw)
To: xfs
[-- 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
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH 06/10] xfs: simplify xfs_map_blocks
2010-11-22 13:05 ` [PATCH 06/10] xfs: simplify xfs_map_blocks Christoph Hellwig
@ 2010-12-01 4:41 ` Dave Chinner
0 siblings, 0 replies; 22+ messages in thread
From: Dave Chinner @ 2010-12-01 4:41 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: xfs
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
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 07/10] xfs: remove xfs_probe_cluster
2010-11-22 13:05 [PATCH 00/10] writeback updates Christoph Hellwig
` (5 preceding siblings ...)
2010-11-22 13:05 ` [PATCH 06/10] xfs: simplify xfs_map_blocks Christoph Hellwig
@ 2010-11-22 13:05 ` Christoph Hellwig
2010-12-01 4:43 ` Dave Chinner
2010-11-22 13:05 ` [PATCH 08/10] xfs: remove the all_bh flag from xfs_convert_page Christoph Hellwig
` (2 subsequent siblings)
9 siblings, 1 reply; 22+ messages in thread
From: Christoph Hellwig @ 2010-11-22 13:05 UTC (permalink / raw)
To: xfs
[-- 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
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH 07/10] xfs: remove xfs_probe_cluster
2010-11-22 13:05 ` [PATCH 07/10] xfs: remove xfs_probe_cluster Christoph Hellwig
@ 2010-12-01 4:43 ` Dave Chinner
0 siblings, 0 replies; 22+ messages in thread
From: Dave Chinner @ 2010-12-01 4:43 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: xfs
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
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 08/10] xfs: remove the all_bh flag from xfs_convert_page
2010-11-22 13:05 [PATCH 00/10] writeback updates Christoph Hellwig
` (6 preceding siblings ...)
2010-11-22 13:05 ` [PATCH 07/10] xfs: remove xfs_probe_cluster Christoph Hellwig
@ 2010-11-22 13:05 ` Christoph Hellwig
2010-12-01 4:45 ` Dave Chinner
2010-11-22 13:05 ` [PATCH 09/10] xfs: refactor xfs_vm_writepage Christoph Hellwig
2010-11-22 13:05 ` [PATCH 10/10] xfs: simplify xfs_map_at_offset Christoph Hellwig
9 siblings, 1 reply; 22+ messages in thread
From: Christoph Hellwig @ 2010-11-22 13:05 UTC (permalink / raw)
To: xfs
[-- 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
^ permalink raw reply [flat|nested] 22+ messages in thread* [PATCH 09/10] xfs: refactor xfs_vm_writepage
2010-11-22 13:05 [PATCH 00/10] writeback updates Christoph Hellwig
` (7 preceding siblings ...)
2010-11-22 13:05 ` [PATCH 08/10] xfs: remove the all_bh flag from xfs_convert_page Christoph Hellwig
@ 2010-11-22 13:05 ` Christoph Hellwig
2010-12-01 4:49 ` Dave Chinner
2010-11-22 13:05 ` [PATCH 10/10] xfs: simplify xfs_map_at_offset Christoph Hellwig
9 siblings, 1 reply; 22+ messages in thread
From: Christoph Hellwig @ 2010-11-22 13:05 UTC (permalink / raw)
To: xfs
[-- 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
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH 09/10] xfs: refactor xfs_vm_writepage
2010-11-22 13:05 ` [PATCH 09/10] xfs: refactor xfs_vm_writepage Christoph Hellwig
@ 2010-12-01 4:49 ` Dave Chinner
0 siblings, 0 replies; 22+ messages in thread
From: Dave Chinner @ 2010-12-01 4:49 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: xfs
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
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 10/10] xfs: simplify xfs_map_at_offset
2010-11-22 13:05 [PATCH 00/10] writeback updates Christoph Hellwig
` (8 preceding siblings ...)
2010-11-22 13:05 ` [PATCH 09/10] xfs: refactor xfs_vm_writepage Christoph Hellwig
@ 2010-11-22 13:05 ` Christoph Hellwig
2010-12-01 4:50 ` Dave Chinner
9 siblings, 1 reply; 22+ messages in thread
From: Christoph Hellwig @ 2010-11-22 13:05 UTC (permalink / raw)
To: xfs
[-- 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
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH 10/10] xfs: simplify xfs_map_at_offset
2010-11-22 13:05 ` [PATCH 10/10] xfs: simplify xfs_map_at_offset Christoph Hellwig
@ 2010-12-01 4:50 ` Dave Chinner
0 siblings, 0 replies; 22+ messages in thread
From: Dave Chinner @ 2010-12-01 4:50 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: xfs
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
^ permalink raw reply [flat|nested] 22+ messages in thread