xfs
[Top] [All Lists]

Re: [PATCH 8/9] xfs: introduce xfs_buf_submit[_wait]

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH 8/9] xfs: introduce xfs_buf_submit[_wait]
From: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Date: Fri, 15 Aug 2014 21:55:31 -0700
Cc: xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <20140815233743.GX26465@dastard>
References: <1408084747-4540-1-git-send-email-david@xxxxxxxxxxxxx> <1408084747-4540-9-git-send-email-david@xxxxxxxxxxxxx> <20140815131020.GB27856@xxxxxxxxxxxxx> <20140815233743.GX26465@dastard>
User-agent: Mutt/1.5.23 (2014-03-12)
On Sat, Aug 16, 2014 at 09:37:43AM +1000, Dave Chinner wrote:
> xfs_zero_remaining_bytes is uses an uncached buffer, so we're not
> using the buffer cache at all for the blocks being zeroed. That is
> why it does the flag twiddling dance it does. However, consolidation
> all the different block zeroing functions we have is an exercise for
> a different day....

Well, we're using buffers.  Anyway, below is what I think it should
look like when using buffers.   Although I wonder how either the old
or new variant pass the verifier check in _xfs_buf_ioapply for v5
filesystems given that we don't pass any ops in.

diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
index 2f1e30d..c495dce 100644
--- a/fs/xfs/xfs_bmap_util.c
+++ b/fs/xfs/xfs_bmap_util.c
@@ -1122,14 +1122,6 @@ xfs_zero_remaining_bytes(
        if (endoff > XFS_ISIZE(ip))
                endoff = XFS_ISIZE(ip);
 
-       bp = xfs_buf_get_uncached(XFS_IS_REALTIME_INODE(ip) ?
-                                       mp->m_rtdev_targp : mp->m_ddev_targp,
-                                 BTOBB(mp->m_sb.sb_blocksize), 0);
-       if (!bp)
-               return -ENOMEM;
-
-       xfs_buf_unlock(bp);
-
        for (offset = startoff; offset <= endoff; offset = lastoffset + 1) {
                uint lock_mode;
 
@@ -1152,42 +1144,26 @@ xfs_zero_remaining_bytes(
                ASSERT(imap.br_startblock != DELAYSTARTBLOCK);
                if (imap.br_state == XFS_EXT_UNWRITTEN)
                        continue;
-               XFS_BUF_UNDONE(bp);
-               XFS_BUF_UNWRITE(bp);
-               XFS_BUF_READ(bp);
-               XFS_BUF_SET_ADDR(bp, xfs_fsb_to_db(ip, imap.br_startblock));
 
-               if (XFS_FORCED_SHUTDOWN(mp)) {
-                       error = -EIO;
-                       break;
-               }
-               xfs_buf_iorequest(bp);
-               error = xfs_buf_iowait(bp);
-               if (error) {
-                       xfs_buf_ioerror_alert(bp,
-                                       "xfs_zero_remaining_bytes(read)");
-                       break;
+               bp = xfs_buf_read_uncached(XFS_IS_REALTIME_INODE(ip) ?
+                               mp->m_rtdev_targp : mp->m_ddev_targp,
+                               xfs_fsb_to_db(ip, imap.br_startblock),
+                               BTOBB(mp->m_sb.sb_blocksize),
+                               0, NULL);
+               if (!bp)
+                       return -ENOMEM;
+               if (bp->b_error) {
+                       error = bp->b_error;
+                       xfs_buf_relse(bp);
+                       return error;
                }
+
                memset(bp->b_addr +
                        (offset - XFS_FSB_TO_B(mp, imap.br_startoff)),
                      0, lastoffset - offset + 1);
-               XFS_BUF_UNDONE(bp);
-               XFS_BUF_UNREAD(bp);
-               XFS_BUF_WRITE(bp);
 
-               if (XFS_FORCED_SHUTDOWN(mp)) {
-                       error = -EIO;
-                       break;
-               }
-               xfs_buf_iorequest(bp);
-               error = xfs_buf_iowait(bp);
-               if (error) {
-                       xfs_buf_ioerror_alert(bp,
-                                       "xfs_zero_remaining_bytes(write)");
-                       break;
-               }
+               xfs_bwrite(bp);
        }
-       xfs_buf_free(bp);
        return error;
 }
 

<Prev in Thread] Current Thread [Next in Thread>