Re: [PATCH 2/2] writeback: Replace some redirty_tail() calls with requeue_io()

[Wu Fengguang <fengguang.wu@intel.com>]

On Mon, Oct 10, 2011 at 07:21:33PM +0800, Jan Kara wrote:
>   Hi Fengguang,
> 
> On Sat 08-10-11 12:00:36, Wu Fengguang wrote:
> > The test results look not good: btrfs is heavily impacted and the
> > other filesystems are slightly impacted.
> >
> > I'll send you the detailed logs in private emails (too large for the
> > mailing list).  Basically I noticed many writeback_wait traces that never
> > appear w/o this patch.
>   OK, thanks for running these tests. I'll have a look at detailed logs.
> I guess the difference can be caused by changes in redirty/requeue logic in
> the second patch (the changes in the first patch could possibly make
> several writeback_wait events from one event but never could introduce new
> events).
> 
> I guess I'll also try to reproduce the problem since it should be pretty
> easy when you see such a huge regression even with 1 dd process on btrfs
> filesystem.
> 
> > In the btrfs cases that see larger regressions, I see large fluctuations
> > in the writeout bandwidth and long disk idle periods. It's still a bit
> > puzzling how all these happen..
>   Yes, I don't understand it yet either...

Jan, it's obviously caused by this chunk, which is not really
necessary for fixing Christoph's problem. So the easy way is to go
ahead without this chunk.

The remaining problems is, the simple dd tests may not be the suitable
workloads to demonstrate the patches' usefulness to XFS.

Thanks,
Fengguang
---

                if ((inode->i_state & I_DIRTY) &&
-                   (wbc->sync_mode == WB_SYNC_ALL || wbc->tagged_writepages))
+                   (wbc->sync_mode == WB_SYNC_ALL || wbc->tagged_writepages)) {
                        inode->dirtied_when = jiffies;
-
-               if (mapping_tagged(mapping, PAGECACHE_TAG_DIRTY)) {
+                       redirty_tail(inode, wb);
+               } else if (mapping_tagged(mapping, PAGECACHE_TAG_DIRTY)) {
                        /*
-                        * We didn't write back all the pages.  nfs_writepages()
-                        * sometimes bales out without doing anything.
+                        * We didn't write back all the pages. nfs_writepages()
+                        * sometimes bales out without doing anything or we
+                        * just run our of our writeback slice.
                         */
                        inode->i_state |= I_DIRTY_PAGES;
-                       if (wbc->nr_to_write <= 0) {
-                               /*
-                                * slice used up: queue for next turn
-                                */
-                               requeue_io(inode, wb);
-                       } else {
-                               /*
-                                * Writeback blocked by something other than
-                                * congestion. Delay the inode for some time to
-                                * avoid spinning on the CPU (100% iowait)
-                                * retrying writeback of the dirty page/inode
-                                * that cannot be performed immediately.
-                                */
-                               redirty_tail(inode, wb);
-                       }
+                       requeue_io(inode, wb);
                } else if (inode->i_state & I_DIRTY) {

Thanks,
Fengguang