From: Dave Chinner <david@fromorbit.com>
To: xfs@oss.sgi.com
Subject: [PATCH 5/6] xfs: factor mapping out of xfs_do_writepage
Date: Thu, 11 Feb 2016 17:45:45 +1100 [thread overview]
Message-ID: <1455173146-19535-6-git-send-email-david@fromorbit.com> (raw)
In-Reply-To: <1455173146-19535-1-git-send-email-david@fromorbit.com>
From: Dave Chinner <dchinner@redhat.com>
Separate out the bufferhead based mapping from the writepage code so
that we have a clear separation of the page operations and the
bufferhead state.
Signed-off-by: Dave Chinner <dchinner@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Dave Chinner <david@fromorbit.com>
---
fs/xfs/xfs_aops.c | 232 ++++++++++++++++++++++++++++--------------------------
1 file changed, 122 insertions(+), 110 deletions(-)
diff --git a/fs/xfs/xfs_aops.c b/fs/xfs/xfs_aops.c
index 01e1663..691cdf9 100644
--- a/fs/xfs/xfs_aops.c
+++ b/fs/xfs/xfs_aops.c
@@ -753,6 +753,127 @@ xfs_writepage_submit(
return status;
}
+static int
+xfs_writepage_map(
+ struct xfs_writepage_ctx *wpc,
+ struct inode *inode,
+ struct page *page,
+ loff_t offset,
+ __uint64_t end_offset)
+{
+ struct buffer_head *bh, *head;
+ ssize_t len = 1 << inode->i_blkbits;
+ int error = 0;
+ int uptodate = 1;
+ int count = 0;
+
+ bh = head = page_buffers(page);
+ offset = page_offset(page);
+
+ do {
+ if (offset >= end_offset)
+ break;
+ if (!buffer_uptodate(bh))
+ uptodate = 0;
+
+ /*
+ * set_page_dirty dirties all buffers in a page, independent
+ * of their state. The dirty state however is entirely
+ * meaningless for holes (!mapped && uptodate), so skip
+ * buffers covering holes here.
+ */
+ if (!buffer_mapped(bh) && buffer_uptodate(bh)) {
+ wpc->imap_valid = false;
+ continue;
+ }
+
+ if (buffer_unwritten(bh)) {
+ if (wpc->io_type != XFS_IO_UNWRITTEN) {
+ wpc->io_type = XFS_IO_UNWRITTEN;
+ wpc->imap_valid = false;
+ }
+ } else if (buffer_delay(bh)) {
+ if (wpc->io_type != XFS_IO_DELALLOC) {
+ wpc->io_type = XFS_IO_DELALLOC;
+ wpc->imap_valid = false;
+ }
+ } else if (buffer_uptodate(bh)) {
+ if (wpc->io_type != XFS_IO_OVERWRITE) {
+ wpc->io_type = XFS_IO_OVERWRITE;
+ wpc->imap_valid = false;
+ }
+ } else {
+ if (PageUptodate(page))
+ ASSERT(buffer_mapped(bh));
+ /*
+ * This buffer is not uptodate and will not be
+ * written to disk. Ensure that we will put any
+ * subsequent writeable buffers into a new
+ * ioend.
+ */
+ wpc->imap_valid = false;
+ continue;
+ }
+
+ if (wpc->imap_valid)
+ wpc->imap_valid = xfs_imap_valid(inode, &wpc->imap,
+ offset);
+ if (!wpc->imap_valid) {
+ error = xfs_map_blocks(inode, offset, &wpc->imap,
+ wpc->io_type);
+ if (error)
+ goto out_error;
+ wpc->imap_valid = xfs_imap_valid(inode, &wpc->imap,
+ offset);
+ }
+ if (wpc->imap_valid) {
+ lock_buffer(bh);
+ if (wpc->io_type != XFS_IO_OVERWRITE)
+ xfs_map_at_offset(inode, bh, &wpc->imap, offset);
+ xfs_add_to_ioend(inode, bh, offset, wpc);
+ count++;
+ }
+
+ if (!wpc->iohead)
+ wpc->iohead = wpc->ioend;
+
+ } while (offset += len, ((bh = bh->b_this_page) != head));
+
+ if (uptodate && bh == head)
+ SetPageUptodate(page);
+
+ xfs_start_page_writeback(page, 1, count);
+ ASSERT(wpc->iohead || !count);
+ return 0;
+
+out_error:
+ /*
+ * On error, we have to fail the iohead here because we locked buffers
+ * in the ioend chain. If we don't do this, we'll deadlock invalidating
+ * the page as that tries to lock the buffers on the page. Also, because
+ * we may have set pages under writeback, we have to make sure we run
+ * IO completion to mark the error state of the IO appropriately, so we
+ * can't cancel the ioend directly here. That means we have to mark this
+ * page as under writeback if we included any buffers from it in the
+ * ioend chain so that completion treats it correctly.
+ *
+ * If we didn't include the page in the ioend, then we can simply
+ * discard and unlock it as there are no other users of the page or it's
+ * buffers right now. The caller will still need to trigger submission
+ * of outstanding ioends on the writepage context so they are treated
+ * correctly on error.
+ */
+ if (count)
+ xfs_start_page_writeback(page, 0, count);
+ else {
+ xfs_aops_discard_page(page);
+ ClearPageUptodate(page);
+ unlock_page(page);
+ }
+ mapping_set_error(page->mapping, error);
+ return error;
+}
+
/*
* Write out a dirty page.
*
@@ -769,13 +890,9 @@ xfs_do_writepage(
{
struct xfs_writepage_ctx *wpc = data;
struct inode *inode = page->mapping->host;
- struct buffer_head *bh, *head;
loff_t offset;
__uint64_t end_offset;
pgoff_t end_index;
- ssize_t len;
- int err, uptodate = 1;
- int count = 0;
trace_xfs_writepage(inode, page, 0, 0);
@@ -868,112 +985,7 @@ xfs_do_writepage(
end_offset = offset;
}
- len = 1 << inode->i_blkbits;
-
- bh = head = page_buffers(page);
- offset = page_offset(page);
-
- do {
- if (offset >= end_offset)
- break;
- if (!buffer_uptodate(bh))
- uptodate = 0;
-
- /*
- * set_page_dirty dirties all buffers in a page, independent
- * of their state. The dirty state however is entirely
- * meaningless for holes (!mapped && uptodate), so skip
- * buffers covering holes here.
- */
- if (!buffer_mapped(bh) && buffer_uptodate(bh)) {
- wpc->imap_valid = false;
- continue;
- }
-
- if (buffer_unwritten(bh)) {
- if (wpc->io_type != XFS_IO_UNWRITTEN) {
- wpc->io_type = XFS_IO_UNWRITTEN;
- wpc->imap_valid = false;
- }
- } else if (buffer_delay(bh)) {
- if (wpc->io_type != XFS_IO_DELALLOC) {
- wpc->io_type = XFS_IO_DELALLOC;
- wpc->imap_valid = false;
- }
- } else if (buffer_uptodate(bh)) {
- if (wpc->io_type != XFS_IO_OVERWRITE) {
- wpc->io_type = XFS_IO_OVERWRITE;
- wpc->imap_valid = false;
- }
- } else {
- if (PageUptodate(page))
- ASSERT(buffer_mapped(bh));
- /*
- * This buffer is not uptodate and will not be
- * written to disk. Ensure that we will put any
- * subsequent writeable buffers into a new
- * ioend.
- */
- wpc->imap_valid = 0;
- continue;
- }
-
- if (wpc->imap_valid)
- wpc->imap_valid = xfs_imap_valid(inode, &wpc->imap, offset);
- if (!wpc->imap_valid) {
- err = xfs_map_blocks(inode, offset, &wpc->imap,
- wpc->io_type);
- if (err)
- goto error;
- wpc->imap_valid = xfs_imap_valid(inode, &wpc->imap, offset);
- }
- if (wpc->imap_valid) {
- lock_buffer(bh);
- if (wpc->io_type != XFS_IO_OVERWRITE)
- xfs_map_at_offset(inode, bh, &wpc->imap, offset);
- xfs_add_to_ioend(inode, bh, offset, wpc);
- count++;
- }
-
- if (!wpc->iohead)
- wpc->iohead = wpc->ioend;
-
- } while (offset += len, ((bh = bh->b_this_page) != head));
-
- if (uptodate && bh == head)
- SetPageUptodate(page);
-
- xfs_start_page_writeback(page, 1, count);
-
- ASSERT(wpc->iohead || !count);
- return 0;
-
-error:
- /*
- * On error, we have to fail the iohead here because we buffers locked
- * in the ioend chain. If we don't do this, we'll deadlock invalidating
- * the page as that tries to lock the buffers on the page. Also, because
- * we may have set pages under writeback, we have to make sure we run
- * IO completion to mark the error state of the IO appropriately, so we
- * can't cancel the ioend directly here. That means we have to mark this
- * page as under writeback if we included any buffers from it in the
- * ioend chain so that completion treats it correctly.
- *
- * If we didn't include the page in the ioend, then we can simply
- * discard and unlock it as there are no other users of the page or it's
- * buffers right now. The caller will still need to trigger submission
- * of outstanding ioends on the writepage context so they are treated
- * correctly on error.
- */
- if (count)
- xfs_start_page_writeback(page, 0, count);
- else {
- xfs_aops_discard_page(page);
- ClearPageUptodate(page);
- unlock_page(page);
- }
- mapping_set_error(page->mapping, err);
- return err;
+ return xfs_writepage_map(wpc, inode, page, offset, end_offset);
redirty:
redirty_page_for_writepage(wbc, page);
--
2.5.0
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
next prev parent reply other threads:[~2016-02-11 6:45 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-02-11 6:45 [PATCH 0/6 v5] xfs: get rid of xfs_cluster_write() Dave Chinner
2016-02-11 6:45 ` [PATCH 1/6] xfs: remove nonblocking mode from xfs_vm_writepage Dave Chinner
2016-02-11 6:45 ` [PATCH 2/6] xfs: remove xfs_cancel_ioend Dave Chinner
2016-02-12 9:30 ` Christoph Hellwig
2016-02-11 6:45 ` [PATCH 3/6] xfs: Introduce writeback context for writepages Dave Chinner
2016-02-11 6:45 ` [PATCH 4/6] xfs: xfs_cluster_write is redundant Dave Chinner
2016-02-11 6:45 ` Dave Chinner [this message]
2016-02-11 6:45 ` [PATCH 6/6] xfs: don't chain ioends during writepage submission Dave Chinner
2016-02-12 9:36 ` Christoph Hellwig
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1455173146-19535-6-git-send-email-david@fromorbit.com \
--to=david@fromorbit.com \
--cc=xfs@oss.sgi.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox