xfs
[Top] [All Lists]

Re: [patch] Fix xfs_iunpin() sets I_DIRTY_SYNC after clear_inode().

To: Takenori Nagano <t-nagano@xxxxxxxxxxxxx>
Subject: Re: [patch] Fix xfs_iunpin() sets I_DIRTY_SYNC after clear_inode().
From: David Chinner <dgc@xxxxxxx>
Date: Fri, 13 Oct 2006 11:46:51 +1000
Cc: xfs@xxxxxxxxxxx
In-reply-to: <452E32FF.8010109@xxxxxxxxxxxxx>
References: <45237CCE.4010007@xxxxxxxxxxxxx> <20061006032617.GC11034@xxxxxxxxxxxxxxxxx> <20061011064357.GN19345@xxxxxxxxxxxxxxxxx> <452E32FF.8010109@xxxxxxxxxxxxx>
Sender: xfs-bounce@xxxxxxxxxxx
User-agent: Mutt/1.4.2.1i
On Thu, Oct 12, 2006 at 09:20:15PM +0900, Takenori Nagano wrote:
> Hi David,
> 
> I tried those patches, but they caused degradation.
> These are results of "vmstat 10" while my test program was running.
> 
> - Before applying those patches:
> procs -----------memory---------- ---swap-- -----io---- -system-- 
> -----cpu------
>  r  b   swpd   free   buff  cache   si   so    bi    bo   in   cs us sy id wa 
> st
>  7  0      0 2768240  37632 210512    0    0     7 43367  268  676  1 49 50  
> 0  0
>  9  0      0 2716352  37632 210672    0    0     0 362864 2154 47915  1 51 48 
>  0  0
>  9  0      0 2663136  37664 210048    0    0     0 361745 2154 48258  1 50 49 
>  0  0
> 10  0      0 2610688  37664 211184    0    0     0 360908 2152 48068  1 51 49 
>  0  0
>  9  0      0 2557904  37680 210512    0    0     0 360254 2154 49036  1 51 48 
>  0  0
> 10  0      0 2504832  37696 210304    0    0     0 362525 2153 48460  1 50 49 
>  0  0
> 
> 
> - After applying those patches:
> procs -----------memory---------- ---swap-- -----io---- -system-- 
> -----cpu------
>  r  b   swpd   free   buff  cache   si   so    bi    bo   in   cs us sy id wa 
> st
>  0  0      0 15584608  21776 153072    0    0    69   403  256  394  1  3 95  
> 1  0
>  0  0      0 15586032  21824 153024    0    0     1  2319 2161 2944  0  2 98  
> 0  0
>  1  0      0 15585920  21824 153104    0    0     0  2342 2161 2951  0  2 98  
> 0  0
>  0  0      0 15585696  21824 152976    0    0     0  2364 2160 2978  0  2 98  
> 0  0
>  1  0      0 15585360  21824 153168    0    0     0  2380 2161 3027  0  2 98  
> 0  0
>  0  0      0 15585248  21824 152976    0    0     0  2348 2161 2983  0  2 98  
> 0  0
> 
> 
> Block I/O performance degradation was very serious.

That was unexpected. :/

> Now, I am trying to ease the degradation.
> Do you have any idea for resolving the degradation?

Did you see a degradation with your original fix? I suspect
not.

I think that the lsn based flush is tripping over a sync
transaction "optimisation" - if the previous log buffer needs
syncing or is currently being synced, then we don't try to flush the
active log buffer straight away - we wait for the previous log
buffer to complete it's I/O in the hope that we get more
transactions into the current log buffer.

IOWs, we introduce a pipeline bubble where we don't force the
current log buffer until the I/O on the previous log buffer has
completed and this effectively serialises these log forces. I
suspect that this is not needed anymore, but I'll look
inot this separately.

When flushing the entire log, (using 0 as the lsn as your
original patch did), we simply close off the current buffer
and flush it out, waiting on it completion if we need a sync
flush. IOWs, no pipeline bubble is introduced and we continue
to issue concurrent log I/O.

Can you test this patch (on top of the last patch I sent)
and see if it fixes the degradation?

Regards,

Dave.
-- 
Dave Chinner
Principal Engineer
SGI Australian Software Group

---
 fs/xfs/xfs_inode.c |   12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)

Index: 2.6.x-xfs-new/fs/xfs/xfs_inode.c
===================================================================
--- 2.6.x-xfs-new.orig/fs/xfs/xfs_inode.c       2006-10-11 17:09:12.000000000 
+1000
+++ 2.6.x-xfs-new/fs/xfs/xfs_inode.c    2006-10-13 11:43:38.236562541 +1000
@@ -2773,26 +2773,16 @@ void
 xfs_iunpin_wait(
        xfs_inode_t     *ip)
 {
-       xfs_inode_log_item_t    *iip;
-       xfs_lsn_t       lsn;
-
        ASSERT(ismrlocked(&ip->i_lock, MR_UPDATE | MR_ACCESS));
 
        if (atomic_read(&ip->i_pincount) == 0) {
                return;
        }
 
-       iip = ip->i_itemp;
-       if (iip && iip->ili_last_lsn) {
-               lsn = iip->ili_last_lsn;
-       } else {
-               lsn = (xfs_lsn_t)0;
-       }
-
        /*
         * Give the log a push so we don't wait here too long.
         */
-       xfs_log_force(ip->i_mount, lsn, XFS_LOG_FORCE);
+       xfs_log_force(ip->i_mount, 0, XFS_LOG_FORCE);
 
        wait_event(ip->i_ipin_wait, (atomic_read(&ip->i_pincount) == 0));
 }


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