Re: [PATCH 6/8] bdi: add a new writeback list for sync

[Brian Foster <bfoster@redhat.com>]

On Tue, Jun 23, 2015 at 08:24:00PM -0700, Josef Bacik wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> wait_sb_inodes() current does a walk of all inodes in the filesystem
> to find dirty one to wait on during sync. This is highly
> inefficient and wastes a lot of CPU when there are lots of clean
> cached inodes that we don't need to wait on.
> 
> To avoid this "all inode" walk, we need to track inodes that are
> currently under writeback that we need to wait for. We do this by
> adding inodes to a writeback list on the bdi when the mapping is
> first tagged as having pages under writeback.  wait_sb_inodes() can
> then walk this list of "inodes under IO" and wait specifically just
> for the inodes that the current sync(2) needs to wait for.
> 
> To avoid needing all the realted locking to be safe against
> interrupts, Jan Kara suggested that we be lazy about removal from
> the writeback list. That is, we don't remove inodes from the
> writeback list on IO completion, but do it directly during a
> wait_sb_inodes() walk.
> 
> This means that the a rare sync(2) call will have some work to do
> skipping clean inodes However, in the current problem case of
> concurrent sync workloads, concurrent wait_sb_inodes() calls only
> walk the very recently dispatched inodes and hence should have very
> little work to do.
> 
> This also means that we have to remove the inodes from the writeback
> list during eviction. Do this in inode_wait_for_writeback() once
> all writeback on the inode is complete.
> 
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> Signed-off-by: Josef Bacik <jbacik@fb.com>
> Tested-by: Dave Chinner <dchinner@redhat.com>
> Reviewed-by: Jan Kara <jack@suse.cz>
> ---

Hi all,

My understanding is this (and the subsequent) patch were never merged
due to conflicts with the cgroup aware writeback stuff that had been
merged. I think the fundamental approach of this patch still solves the
original problem, the writeback inodes are just always tracked on the
root bdi_writeback rather than the per-cgroup bdi_writeback the inode
was associated with before writeback.

Any thoughts on that are appreciated. Otherwise, I'm trying to rebase
this and I've hit a lockdep splat [1] that suggests the locking for the
writeback list needs to be tweaked one way or another. Note that lockdep
fires with this original version as well, so I don't think this is a new
issue.

I _think_ this means that list_lock either needs to be taken outside of
mapping->tree_lock or we need to create a new irq safe spinlock specific
to the b_writeback list. The former looks like it requires some ugliness
in __test_set_page_writeback() to acquire list_lock first, but only when
list_empty(&inode->i_wb_list) so we don't grab it for every page. The
latter obviously means a new irq-disabling lock (which is already done
in parts of wait_sb_inode(), fwiw). The latter might also facilitate
further cleanups like pulling b_writeback out of bdi_writeback and up
into backing_dev_info since we really only need one instance of the
list, but I'm not sure if that's preferable..

Thoughts?

Brian

[1] lockdep splat:

[ INFO: possible irq lock inversion dependency detected ]
4.4.0-rc4+ #54 Not tainted
---------------------------------------------------------
swapper/0/0 just changed the state of lock:
(&(&mapping->tree_lock)->rlock){-.....}, at: [<ffffffff811d25ed>] test_clear_page_writeback+0x5d/0x210
but this lock took another, HARDIRQ-unsafe lock in the past:
(&(&wb->list_lock)->rlock){+.+...}#012#012and interrupts could create inverse lock ordering between them.
#012other info that might help us debug this:
Possible interrupt unsafe locking scenario:
      CPU0                    CPU1
      ----                    ----
 lock(&(&wb->list_lock)->rlock);
                              local_irq_disable();
                              lock(&(&mapping->tree_lock)->rlock);
                              lock(&(&wb->list_lock)->rlock);
 <Interrupt>
   lock(&(&mapping->tree_lock)->rlock);
#012 *** DEADLOCK ***
no locks held by swapper/0/0.
#012the shortest dependencies between 2nd lock and 1st lock:
-> (&(&wb->list_lock)->rlock){+.+...} ops: 194 {
   HARDIRQ-ON-W at:
                     [<ffffffff810f9959>] __lock_acquire+0x819/0x1a50
                     [<ffffffff810fb54e>] lock_acquire+0xce/0x1c0
                     [<ffffffff817e3df8>] _raw_spin_lock+0x38/0x50
                     [<ffffffff8126b7a9>] __mark_inode_dirty+0x3d9/0x5e0
                     [<ffffffff8125a859>] generic_update_time+0x79/0xd0
                     [<ffffffff8125c638>] touch_atime+0xa8/0xd0
                     [<ffffffff812519c3>] iterate_dir+0xe3/0x130
                     [<ffffffff81251ecd>] SyS_getdents+0x9d/0x130
                     [<ffffffff817e4a72>] entry_SYSCALL_64_fastpath+0x12/0x76
   SOFTIRQ-ON-W at:
                     [<ffffffff810f998a>] __lock_acquire+0x84a/0x1a50
                     [<ffffffff810fb54e>] lock_acquire+0xce/0x1c0
                     [<ffffffff817e3df8>] _raw_spin_lock+0x38/0x50
                     [<ffffffff8126b7a9>] __mark_inode_dirty+0x3d9/0x5e0
                     [<ffffffff8125a859>] generic_update_time+0x79/0xd0
                     [<ffffffff8125c638>] touch_atime+0xa8/0xd0
                     [<ffffffff812519c3>] iterate_dir+0xe3/0x130
                     [<ffffffff81251ecd>] SyS_getdents+0x9d/0x130
                     [<ffffffff817e4a72>] entry_SYSCALL_64_fastpath+0x12/0x76
   INITIAL USE at:
                    [<ffffffff810f9538>] __lock_acquire+0x3f8/0x1a50
                    [<ffffffff810fb54e>] lock_acquire+0xce/0x1c0
                    [<ffffffff817e3df8>] _raw_spin_lock+0x38/0x50
                    [<ffffffff8126b7a9>] __mark_inode_dirty+0x3d9/0x5e0
                    [<ffffffff8125a859>] generic_update_time+0x79/0xd0
                    [<ffffffff8125c638>] touch_atime+0xa8/0xd0
                    [<ffffffff812519c3>] iterate_dir+0xe3/0x130
                    [<ffffffff81251ecd>] SyS_getdents+0x9d/0x130
                    [<ffffffff817e4a72>] entry_SYSCALL_64_fastpath+0x12/0x76
 }
 ... key      at: [<ffffffff82c7faa0>] __key.37388+0x0/0x8
 ... acquired at:
  [<ffffffff810fb54e>] lock_acquire+0xce/0x1c0
  [<ffffffff817e3df8>] _raw_spin_lock+0x38/0x50
  [<ffffffff812703a7>] bdi_mark_inode_writeback+0x67/0xd0
  [<ffffffff811d19b2>] __test_set_page_writeback+0x112/0x200
  [<ffffffff8127bcdc>] __block_write_full_page.constprop.44+0xec/0x400
  [<ffffffff8127c0ee>] block_write_full_page+0xfe/0x190
  [<ffffffff8127cec8>] blkdev_writepage+0x18/0x20
  [<ffffffff811cede6>] __writepage+0x16/0x40
  [<ffffffff811cfc65>] write_cache_pages+0x225/0x5f0
  [<ffffffff811d0084>] generic_writepages+0x54/0x80
  [<ffffffff811d24e1>] do_writepages+0x21/0x30
  [<ffffffff811c4b40>] __filemap_fdatawrite_range+0x80/0xb0
  [<ffffffff811c4c7d>] filemap_write_and_wait_range+0x2d/0x70
  [<ffffffff8127ca8b>] blkdev_fsync+0x1b/0x50
  [<ffffffff81274a9b>] vfs_fsync_range+0x4b/0xb0
  [<ffffffff81274b5d>] do_fsync+0x3d/0x70
  [<ffffffff81274e10>] SyS_fsync+0x10/0x20
  [<ffffffff817e4a72>] entry_SYSCALL_64_fastpath+0x12/0x76

-> (&(&mapping->tree_lock)->rlock){-.....} ops: 16821 {
  IN-HARDIRQ-W at:
                   [<ffffffff810f9ad9>] __lock_acquire+0x999/0x1a50
                   [<ffffffff810fb54e>] lock_acquire+0xce/0x1c0
                   [<ffffffff817e4810>] _raw_spin_lock_irqsave+0x50/0x70
                   [<ffffffff811d25ed>] test_clear_page_writeback+0x5d/0x210
                   [<ffffffff811c2b2f>] end_page_writeback+0x1f/0xa0
                   [<ffffffff81278df3>] end_buffer_async_write+0xd3/0x1a0
                   [<ffffffff81277828>] end_bio_bh_io_sync+0x28/0x40
                   [<ffffffff813a329f>] bio_endio+0x3f/0x60
                   [<ffffffff8162d4d9>] dec_pending+0x149/0x310
                   [<ffffffff8162e1d6>] clone_endio+0x76/0xe0
                   [<ffffffff813a329f>] bio_endio+0x3f/0x60
                   [<ffffffff813ab512>] blk_update_request+0xb2/0x3a0
                   [<ffffffff813b63ca>] blk_mq_end_request+0x1a/0x70
                   [<ffffffffa009e82f>] virtblk_request_done+0x3f/0x70 [virtio_blk]
                   [<ffffffff813b4ea3>] __blk_mq_complete_request_remote+0x13/0x20
                   [<ffffffff81137d9f>] flush_smp_call_function_queue+0x5f/0x160
                   [<ffffffff811388d3>] generic_smp_call_function_single_interrupt+0x13/0x60
                   [<ffffffff810530a7>] smp_call_function_single_interrupt+0x27/0x40
                   [<ffffffff817e604c>] call_function_single_interrupt+0x8c/0xa0
                   [<ffffffff81026703>] default_idle+0x23/0x150
                   [<ffffffff8102702f>] arch_cpu_idle+0xf/0x20
                   [<ffffffff810efd8a>] default_idle_call+0x2a/0x40
                   [<ffffffff810f0184>] cpu_startup_entry+0x384/0x3f0
                   [<ffffffff817d6aaa>] rest_init+0x13a/0x140
                   [<ffffffff81d6b01f>] start_kernel+0x49e/0x4bf
                   [<ffffffff81d6a339>] x86_64_start_reservations+0x2a/0x2c
                   [<ffffffff81d6a480>] x86_64_start_kernel+0x145/0x168
  INITIAL USE at:
                  [<ffffffff810f9538>] __lock_acquire+0x3f8/0x1a50
                  [<ffffffff810fb54e>] lock_acquire+0xce/0x1c0
                  [<ffffffff817e3fa4>] _raw_spin_lock_irq+0x44/0x60
                  [<ffffffff811c303c>] __add_to_page_cache_locked+0xbc/0x340
                  [<ffffffff811c3317>] add_to_page_cache_lru+0x37/0x90
                  [<ffffffff811c3dc7>] pagecache_get_page+0xb7/0x200
                  [<ffffffff811c4139>] grab_cache_page_write_begin+0x29/0x40
                  [<ffffffff81269848>] simple_write_begin+0x28/0x1c0
                  [<ffffffff811c2a01>] generic_perform_write+0xd1/0x1e0
                  [<ffffffff811c55f2>] __generic_file_write_iter+0x1a2/0x1e0
                  [<ffffffff811c5718>] generic_file_write_iter+0xe8/0x1e0
                  [<ffffffff8123c409>] __vfs_write+0xc9/0x110
                  [<ffffffff8123ca99>] vfs_write+0xa9/0x1a0
                  [<ffffffff8123d778>] SyS_write+0x58/0xd0
                  [<ffffffff81d6c3b1>] xwrite+0x29/0x5c
                  [<ffffffff81d6c40e>] do_copy+0x2a/0xb8
                  [<ffffffff81d6c151>] write_buffer+0x23/0x34
                  [<ffffffff81d6c9e6>] unpack_to_rootfs+0xfd/0x294
                  [<ffffffff81d6cbd9>] populate_rootfs+0x5c/0x108
                  [<ffffffff81002123>] do_one_initcall+0xb3/0x200
                  [<ffffffff81d6b22e>] kernel_init_freeable+0x1ee/0x28d
                  [<ffffffff817d6abe>] kernel_init+0xe/0xe0
                  [<ffffffff817e4ddf>] ret_from_fork+0x3f/0x70
}
... key      at: [<ffffffff82ca2760>] __key.39560+0x0/0x8
... acquired at:
  [<ffffffff810f7c1b>] check_usage_forwards+0x15b/0x160
  [<ffffffff810f88c3>] mark_lock+0x333/0x610
  [<ffffffff810f9ad9>] __lock_acquire+0x999/0x1a50
  [<ffffffff810fb54e>] lock_acquire+0xce/0x1c0
  [<ffffffff817e4810>] _raw_spin_lock_irqsave+0x50/0x70
  [<ffffffff811d25ed>] test_clear_page_writeback+0x5d/0x210
  [<ffffffff811c2b2f>] end_page_writeback+0x1f/0xa0
  [<ffffffff81278df3>] end_buffer_async_write+0xd3/0x1a0
  [<ffffffff81277828>] end_bio_bh_io_sync+0x28/0x40
  [<ffffffff813a329f>] bio_endio+0x3f/0x60
  [<ffffffff8162d4d9>] dec_pending+0x149/0x310
  [<ffffffff8162e1d6>] clone_endio+0x76/0xe0
  [<ffffffff813a329f>] bio_endio+0x3f/0x60
  [<ffffffff813ab512>] blk_update_request+0xb2/0x3a0
  [<ffffffff813b63ca>] blk_mq_end_request+0x1a/0x70
  [<ffffffffa009e82f>] virtblk_request_done+0x3f/0x70 [virtio_blk]
  [<ffffffff813b4ea3>] __blk_mq_complete_request_remote+0x13/0x20
  [<ffffffff81137d9f>] flush_smp_call_function_queue+0x5f/0x160
  [<ffffffff811388d3>] generic_smp_call_function_single_interrupt+0x13/0x60
  [<ffffffff810530a7>] smp_call_function_single_interrupt+0x27/0x40
  [<ffffffff817e604c>] call_function_single_interrupt+0x8c/0xa0
  [<ffffffff81026703>] default_idle+0x23/0x150
  [<ffffffff8102702f>] arch_cpu_idle+0xf/0x20
  [<ffffffff810efd8a>] default_idle_call+0x2a/0x40
  [<ffffffff810f0184>] cpu_startup_entry+0x384/0x3f0
  [<ffffffff817d6aaa>] rest_init+0x13a/0x140
  [<ffffffff81d6b01f>] start_kernel+0x49e/0x4bf
  [<ffffffff81d6a339>] x86_64_start_reservations+0x2a/0x2c
  [<ffffffff81d6a480>] x86_64_start_kernel+0x145/0x168

#012stack backtrace:
CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.4.0-rc4+ #54
Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
0000000000000000 06776aa479e380f6 ffff88011fc03ad0 ffffffff813dd019
ffffffff82a0f120 ffff88011fc03b10 ffffffff811c184d ffffffff81c10be8
ffffffff81c10be8 ffffffff81c10500 ffffffff81a56510 0000000000000000
Call Trace:
<IRQ>  [<ffffffff813dd019>] dump_stack+0x4b/0x72
[<ffffffff811c184d>] print_irq_inversion_bug.part.38+0x1a4/0x1b0
[<ffffffff810f7c1b>] check_usage_forwards+0x15b/0x160
[<ffffffff810f88c3>] mark_lock+0x333/0x610
[<ffffffff810f7ac0>] ? check_usage_backwards+0x160/0x160
[<ffffffff810f9ad9>] __lock_acquire+0x999/0x1a50
[<ffffffff810fb54e>] lock_acquire+0xce/0x1c0
[<ffffffff811d25ed>] ? test_clear_page_writeback+0x5d/0x210
[<ffffffff817e4810>] _raw_spin_lock_irqsave+0x50/0x70
[<ffffffff811d25ed>] ? test_clear_page_writeback+0x5d/0x210
[<ffffffff811d25ed>] test_clear_page_writeback+0x5d/0x210
[<ffffffff811c2b2f>] end_page_writeback+0x1f/0xa0
[<ffffffff81278df3>] end_buffer_async_write+0xd3/0x1a0
[<ffffffff81277828>] end_bio_bh_io_sync+0x28/0x40
[<ffffffff813a329f>] bio_endio+0x3f/0x60
[<ffffffff8162d4d9>] dec_pending+0x149/0x310
[<ffffffff811c6809>] ? mempool_free+0x29/0x80
[<ffffffff8162e1d6>] clone_endio+0x76/0xe0
[<ffffffff813a329f>] bio_endio+0x3f/0x60
[<ffffffff813ab512>] blk_update_request+0xb2/0x3a0
[<ffffffff813b4e90>] ? blkdev_issue_zeroout+0xf0/0xf0
[<ffffffff813b63ca>] blk_mq_end_request+0x1a/0x70
[<ffffffffa009e82f>] virtblk_request_done+0x3f/0x70 [virtio_blk]
[<ffffffff813b4ea3>] __blk_mq_complete_request_remote+0x13/0x20
[<ffffffff81137d9f>] flush_smp_call_function_queue+0x5f/0x160
[<ffffffff811388d3>] generic_smp_call_function_single_interrupt+0x13/0x60
[<ffffffff810530a7>] smp_call_function_single_interrupt+0x27/0x40
[<ffffffff817e604c>] call_function_single_interrupt+0x8c/0xa0
<EOI>  [<ffffffff810653a6>] ? native_safe_halt+0x6/0x10
[<ffffffff81026703>] default_idle+0x23/0x150
[<ffffffff8102702f>] arch_cpu_idle+0xf/0x20
[<ffffffff810efd8a>] default_idle_call+0x2a/0x40
[<ffffffff810f0184>] cpu_startup_entry+0x384/0x3f0
[<ffffffff817d6aaa>] rest_init+0x13a/0x140
[<ffffffff81d6b01f>] start_kernel+0x49e/0x4bf
[<ffffffff81d6a120>] ? early_idt_handler_array+0x120/0x120
[<ffffffff81d6a339>] x86_64_start_reservations+0x2a/0x2c
[<ffffffff81d6a480>] x86_64_start_kernel+0x145/0x168

>  fs/block_dev.c              |   2 +
>  fs/fs-writeback.c           | 148 +++++++++++++++++++++++++++++++++++---------
>  fs/inode.c                  |   1 +
>  include/linux/backing-dev.h |   5 ++
>  include/linux/fs.h          |   1 +
>  mm/backing-dev.c            |   1 +
>  mm/page-writeback.c         |  14 +++++
>  7 files changed, 144 insertions(+), 28 deletions(-)
> 
> diff --git a/fs/block_dev.c b/fs/block_dev.c
> index f2a89be..2726ff1 100644
> --- a/fs/block_dev.c
> +++ b/fs/block_dev.c
> @@ -64,6 +64,8 @@ void kill_bdev(struct block_device *bdev)
>  {
>  	struct address_space *mapping = bdev->bd_inode->i_mapping;
>  
> +	bdi_clear_inode_writeback(inode_to_bdi(bdev->bd_inode),
> +				  bdev->bd_inode);
>  	if (mapping->nrpages == 0 && mapping->nrshadows == 0)
>  		return;
>  
> diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
> index aa72536..5c005ad 100644
> --- a/fs/fs-writeback.c
> +++ b/fs/fs-writeback.c
> @@ -210,6 +210,34 @@ void inode_wb_list_del(struct inode *inode)
>  }
>  
>  /*
> + * mark an inode as under writeback on the given bdi
> + */
> +void bdi_mark_inode_writeback(struct backing_dev_info *bdi, struct inode *inode)
> +{
> +	WARN_ON_ONCE(bdi != inode_to_bdi(inode));
> +	if (list_empty(&inode->i_wb_list)) {
> +		spin_lock(&bdi->wb.list_lock);
> +		if (list_empty(&inode->i_wb_list))
> +			list_add_tail(&inode->i_wb_list, &bdi->wb.b_writeback);
> +		spin_unlock(&bdi->wb.list_lock);
> +	}
> +}
> +
> +/*
> + * clear an inode as under writeback on the given bdi
> + */
> +void bdi_clear_inode_writeback(struct backing_dev_info *bdi,
> +			       struct inode *inode)
> +{
> +	WARN_ON_ONCE(bdi != inode_to_bdi(inode));
> +	if (!list_empty(&inode->i_wb_list)) {
> +		spin_lock(&bdi->wb.list_lock);
> +		list_del_init(&inode->i_wb_list);
> +		spin_unlock(&bdi->wb.list_lock);
> +	}
> +}
> +
> +/*
>   * Redirty an inode: set its when-it-was dirtied timestamp and move it to the
>   * furthest end of its superblock's dirty-inode list.
>   *
> @@ -383,13 +411,28 @@ static void __inode_wait_for_writeback(struct inode *inode)
>  }
>  
>  /*
> - * Wait for writeback on an inode to complete. Caller must have inode pinned.
> + * Wait for writeback on an inode to complete during eviction. Caller must have
> + * inode pinned.
>   */
>  void inode_wait_for_writeback(struct inode *inode)
>  {
> +	BUG_ON(!(inode->i_state & I_FREEING));
> +
>  	spin_lock(&inode->i_lock);
>  	__inode_wait_for_writeback(inode);
>  	spin_unlock(&inode->i_lock);
> +
> +	/*
> +	 * For a bd_inode when we do inode_to_bdi we'll want to get the bdev for
> +	 * the inode and then deref bdev->bd_disk, which at this point has been
> +	 * set to NULL, so we would panic.  At the point we are dropping our
> +	 * bd_inode we won't have any pages under writeback on the device so
> +	 * this is safe.  But just in case we'll assert to make sure we don't
> +	 * screw this up.
> +	 */
> +	if (!sb_is_blkdev_sb(inode->i_sb))
> +		bdi_clear_inode_writeback(inode_to_bdi(inode), inode);
> +	BUG_ON(!list_empty(&inode->i_wb_list));
>  }
>  
>  /*
> @@ -1372,7 +1415,9 @@ EXPORT_SYMBOL(__mark_inode_dirty);
>   */
>  static void wait_sb_inodes(struct super_block *sb)
>  {
> -	struct inode *inode, *old_inode = NULL;
> +	struct backing_dev_info *bdi = sb->s_bdi;
> +	LIST_HEAD(sync_list);
> +	struct inode *iput_inode = NULL;
>  
>  	/*
>  	 * We need to be protected against the filesystem going from
> @@ -1380,48 +1425,95 @@ static void wait_sb_inodes(struct super_block *sb)
>  	 */
>  	WARN_ON(!rwsem_is_locked(&sb->s_umount));
>  
> -	mutex_lock(&sb->s_sync_lock);
> -	spin_lock(&sb->s_inode_list_lock);
> -
>  	/*
> -	 * Data integrity sync. Must wait for all pages under writeback,
> -	 * because there may have been pages dirtied before our sync
> -	 * call, but which had writeout started before we write it out.
> -	 * In which case, the inode may not be on the dirty list, but
> -	 * we still have to wait for that writeout.
> +	 * Data integrity sync. Must wait for all pages under writeback, because
> +	 * there may have been pages dirtied before our sync call, but which had
> +	 * writeout started before we write it out.  In which case, the inode
> +	 * may not be on the dirty list, but we still have to wait for that
> +	 * writeout.
> +	 *
> +	 * To avoid syncing inodes put under IO after we have started here,
> +	 * splice the io list to a temporary list head and walk that. Newly
> +	 * dirtied inodes will go onto the primary list so we won't wait for
> +	 * them. This is safe to do as we are serialised by the s_sync_lock,
> +	 * so we'll complete processing the complete list before the next
> +	 * sync operation repeats the splice-and-walk process.
> +	 *
> +	 * Inodes that have pages under writeback after we've finished the wait
> +	 * may or may not be on the primary list. They had pages under put IO
> +	 * after we started our wait, so we need to make sure the next sync or
> +	 * IO completion treats them correctly, Move them back to the primary
> +	 * list and restart the walk.
> +	 *
> +	 * Inodes that are clean after we have waited for them don't belong on
> +	 * any list, so simply remove them from the sync list and move onwards.
>  	 */
> -	list_for_each_entry(inode, &sb->s_inodes, i_sb_list) {
> +	mutex_lock(&sb->s_sync_lock);
> +	spin_lock(&bdi->wb.list_lock);
> +	list_splice_init(&bdi->wb.b_writeback, &sync_list);
> +
> +	while (!list_empty(&sync_list)) {
> +		struct inode *inode = list_first_entry(&sync_list, struct inode,
> +						       i_wb_list);
>  		struct address_space *mapping = inode->i_mapping;
>  
> +		/*
> +		 * We are lazy on IO completion and don't remove inodes from the
> +		 * list when they are clean. Detect that immediately and skip
> +		 * inodes we don't ahve to wait on.
> +		 */
> +		if (!mapping_tagged(mapping, PAGECACHE_TAG_WRITEBACK)) {
> +			list_del_init(&inode->i_wb_list);
> +			cond_resched_lock(&bdi->wb.list_lock);
> +			continue;
> +		}
> +
>  		spin_lock(&inode->i_lock);
> -		if ((inode->i_state & (I_FREEING|I_WILL_FREE|I_NEW)) ||
> -		    (mapping->nrpages == 0)) {
> +		if (inode->i_state & (I_FREEING|I_WILL_FREE|I_NEW)) {
> +			list_move(&inode->i_wb_list, &bdi->wb.b_writeback);
>  			spin_unlock(&inode->i_lock);
> +			cond_resched_lock(&bdi->wb.list_lock);
>  			continue;
>  		}
>  		__iget(inode);
>  		spin_unlock(&inode->i_lock);
> -		spin_unlock(&sb->s_inode_list_lock);
> +		spin_unlock(&bdi->wb.list_lock);
>  
> -		/*
> -		 * We hold a reference to 'inode' so it couldn't have been
> -		 * removed from s_inodes list while we dropped the
> -		 * s_inode_list_lock.  We cannot iput the inode now as we can
> -		 * be holding the last reference and we cannot iput it under
> -		 * s_inode_list_lock. So we keep the reference and iput it
> -		 * later.
> -		 */
> -		iput(old_inode);
> -		old_inode = inode;
> +		if (iput_inode)
> +			iput(iput_inode);
>  
>  		filemap_fdatawait(mapping);
> -
>  		cond_resched();
>  
> -		spin_lock(&sb->s_inode_list_lock);
> +		/*
> +		 * the inode has been written back now, so check whether we
> +		 * still have pages under IO and move it back to the primary
> +		 * list if necessary.  We really need the mapping->tree_lock
> +		 * here because bdi_mark_inode_writeback may have not done
> +		 * anything because we were on the spliced list and we need to
> +		 * check TAG_WRITEBACK.
> +		 */
> +		spin_lock_irq(&mapping->tree_lock);
> +		spin_lock(&bdi->wb.list_lock);
> +		if (mapping_tagged(mapping, PAGECACHE_TAG_WRITEBACK)) {
> +			WARN_ON(list_empty(&inode->i_wb_list));
> +			list_move(&inode->i_wb_list, &bdi->wb.b_writeback);
> +                } else
> +			list_del_init(&inode->i_wb_list);
> +		spin_unlock_irq(&mapping->tree_lock);
> +
> +		/*
> +		 * can't iput inode while holding the wb.list_lock. Save it for
> +		 * the next time through the loop when we drop all our spin
> +		 * locks.
> +		 */
> +		iput_inode = inode;
>  	}
> -	spin_unlock(&sb->s_inode_list_lock);
> -	iput(old_inode);
> +	spin_unlock(&bdi->wb.list_lock);
> +
> +	if (iput_inode)
> +		iput(iput_inode);
> +
>  	mutex_unlock(&sb->s_sync_lock);
>  }
>  
> diff --git a/fs/inode.c b/fs/inode.c
> index 770d684..8f00557 100644
> --- a/fs/inode.c
> +++ b/fs/inode.c
> @@ -357,6 +357,7 @@ void inode_init_once(struct inode *inode)
>  	INIT_HLIST_NODE(&inode->i_hash);
>  	INIT_LIST_HEAD(&inode->i_devices);
>  	INIT_LIST_HEAD(&inode->i_io_list);
> +	INIT_LIST_HEAD(&inode->i_wb_list);
>  	INIT_LIST_HEAD(&inode->i_lru);
>  	address_space_init_once(&inode->i_data);
>  	i_size_ordered_init(inode);
> diff --git a/include/linux/backing-dev.h b/include/linux/backing-dev.h
> index d87d8ec..0d5bca2 100644
> --- a/include/linux/backing-dev.h
> +++ b/include/linux/backing-dev.h
> @@ -56,6 +56,7 @@ struct bdi_writeback {
>  	struct list_head b_io;		/* parked for writeback */
>  	struct list_head b_more_io;	/* parked for more writeback */
>  	struct list_head b_dirty_time;	/* time stamps are dirty */
> +	struct list_head b_writeback;	/* inodes under writeback */
>  	spinlock_t list_lock;		/* protects the b_* lists */
>  };
>  
> @@ -123,6 +124,10 @@ void bdi_start_background_writeback(struct backing_dev_info *bdi);
>  void bdi_writeback_workfn(struct work_struct *work);
>  int bdi_has_dirty_io(struct backing_dev_info *bdi);
>  void bdi_wakeup_thread_delayed(struct backing_dev_info *bdi);
> +void bdi_mark_inode_writeback(struct backing_dev_info *bdi,
> +			      struct inode *inode);
> +void bdi_clear_inode_writeback(struct backing_dev_info *bdi,
> +			       struct inode *inode);
>  
>  extern spinlock_t bdi_lock;
>  extern struct list_head bdi_list;
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 6dd2ab2..d3baa08 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -636,6 +636,7 @@ struct inode {
>  	struct list_head	i_io_list;	/* backing dev IO list */
>  	struct list_head	i_lru;		/* inode LRU list */
>  	struct list_head	i_sb_list;
> +	struct list_head	i_wb_list;	/* backing dev writeback list */
>  	union {
>  		struct hlist_head	i_dentry;
>  		struct rcu_head		i_rcu;
> diff --git a/mm/backing-dev.c b/mm/backing-dev.c
> index e9ed047..ea39301 100644
> --- a/mm/backing-dev.c
> +++ b/mm/backing-dev.c
> @@ -369,6 +369,7 @@ static void bdi_wb_init(struct bdi_writeback *wb, struct backing_dev_info *bdi)
>  	INIT_LIST_HEAD(&wb->b_io);
>  	INIT_LIST_HEAD(&wb->b_more_io);
>  	INIT_LIST_HEAD(&wb->b_dirty_time);
> +	INIT_LIST_HEAD(&wb->b_writeback);
>  	spin_lock_init(&wb->list_lock);
>  	INIT_DELAYED_WORK(&wb->dwork, bdi_writeback_workfn);
>  }
> diff --git a/mm/page-writeback.c b/mm/page-writeback.c
> index eb59f7e..f3751d1 100644
> --- a/mm/page-writeback.c
> +++ b/mm/page-writeback.c
> @@ -2382,11 +2382,25 @@ int __test_set_page_writeback(struct page *page, bool keep_write)
>  		spin_lock_irqsave(&mapping->tree_lock, flags);
>  		ret = TestSetPageWriteback(page);
>  		if (!ret) {
> +			bool on_wblist;
> +
> +			on_wblist = mapping_tagged(mapping,
> +						   PAGECACHE_TAG_WRITEBACK);
> +
>  			radix_tree_tag_set(&mapping->page_tree,
>  						page_index(page),
>  						PAGECACHE_TAG_WRITEBACK);
>  			if (bdi_cap_account_writeback(bdi))
>  				__inc_bdi_stat(bdi, BDI_WRITEBACK);
> +
> +			/*
> +			 * we can come through here when swapping anonymous
> +			 * pages, so we don't necessarily have an inode to
> +			 * track for sync. Inodes are remove lazily, so there is
> +			 * no equivalent in test_clear_page_writeback().
> +			 */
> +			if (!on_wblist && mapping->host)
> +				bdi_mark_inode_writeback(bdi, mapping->host);
>  		}
>  		if (!PageDirty(page))
>  			radix_tree_tag_clear(&mapping->page_tree,
> -- 
> 2.1.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html