1
linux/fs/xfs/linux-2.6
Fengguang Wu 1f7decf6d9 writeback: remove pages_skipped accounting in __block_write_full_page()
Miklos Szeredi <miklos@szeredi.hu> and me identified a writeback bug:

> The following strange behavior can be observed:
>
> 1. large file is written
> 2. after 30 seconds, nr_dirty goes down by 1024
> 3. then for some time (< 30 sec) nothing happens (disk idle)
> 4. then nr_dirty again goes down by 1024
> 5. repeat from 3. until whole file is written
>
> So basically a 4Mbyte chunk of the file is written every 30 seconds.
> I'm quite sure this is not the intended behavior.

It can be produced by the following test scheme:

# cat bin/test-writeback.sh
grep nr_dirty /proc/vmstat
echo 1 > /proc/sys/fs/inode_debug
dd if=/dev/zero of=/var/x bs=1K count=204800&
while true; do grep nr_dirty /proc/vmstat; sleep 1; done

# bin/test-writeback.sh
nr_dirty 19207
nr_dirty 19207
nr_dirty 30924
204800+0 records in
204800+0 records out
209715200 bytes (210 MB) copied, 1.58363 seconds, 132 MB/s
nr_dirty 47150
nr_dirty 47141
nr_dirty 47142
nr_dirty 47142
nr_dirty 47142
nr_dirty 47142
nr_dirty 47205
nr_dirty 47214
nr_dirty 47214
nr_dirty 47214
nr_dirty 47214
nr_dirty 47214
nr_dirty 47215
nr_dirty 47216
nr_dirty 47216
nr_dirty 47216
nr_dirty 47154
nr_dirty 47143
nr_dirty 47143
nr_dirty 47143
nr_dirty 47143
nr_dirty 47143
nr_dirty 47142
nr_dirty 47142
nr_dirty 47142
nr_dirty 47142
nr_dirty 47134
nr_dirty 47134
nr_dirty 47135
nr_dirty 47135
nr_dirty 47135
nr_dirty 46097 <== -1038
nr_dirty 46098
nr_dirty 46098
nr_dirty 46098
[...]
nr_dirty 46091
nr_dirty 46092
nr_dirty 46092
nr_dirty 45069 <== -1023
nr_dirty 45056
nr_dirty 45056
nr_dirty 45056
[...]
nr_dirty 37822
nr_dirty 36799 <== -1023
[...]
nr_dirty 36781
nr_dirty 35758 <== -1023
[...]
nr_dirty 34708
nr_dirty 33672 <== -1024
[...]
nr_dirty 33692
nr_dirty 32669 <== -1023

% ls -li /var/x
847824 -rw-r--r-- 1 root root 200M 2007-08-12 04:12 /var/x

% dmesg|grep 847824  # generated by a debug printk
[  529.263184] redirtied inode 847824 line 548
[  564.250872] redirtied inode 847824 line 548
[  594.272797] redirtied inode 847824 line 548
[  629.231330] redirtied inode 847824 line 548
[  659.224674] redirtied inode 847824 line 548
[  689.219890] redirtied inode 847824 line 548
[  724.226655] redirtied inode 847824 line 548
[  759.198568] redirtied inode 847824 line 548

# line 548 in fs/fs-writeback.c:
543                 if (wbc->pages_skipped != pages_skipped) {
544                         /*
545                          * writeback is not making progress due to locked
546                          * buffers.  Skip this inode for now.
547                          */
548                         redirty_tail(inode);
549                 }

More debug efforts show that __block_write_full_page()
never has the chance to call submit_bh() for that big dirty file:
the buffer head is *clean*. So basicly no page io is issued by
__block_write_full_page(), hence pages_skipped goes up.

Also the comment in generic_sync_sb_inodes():

544                         /*
545                          * writeback is not making progress due to locked
546                          * buffers.  Skip this inode for now.
547                          */

and the comment in __block_write_full_page():

1713                 /*
1714                  * The page was marked dirty, but the buffers were
1715                  * clean.  Someone wrote them back by hand with
1716                  * ll_rw_block/submit_bh.  A rare case.
1717                  */

do not quite agree with each other. The page writeback should be skipped for
'locked buffer', but here it is 'clean buffer'!

This patch fixes this bug. Though I'm not sure why __block_write_full_page()
is called only to do nothing and who actually issued the writeback for us.

This is the two possible new behaviors after the patch:

1) pretty nice: wait 30s and write ALL:)
2) not so good:
	- during the dd: ~16M
	- after 30s:      ~4M
	- after 5s:       ~4M
	- after 5s:     ~176M

The next patch will fix case (2).

Cc: David Chinner <dgc@sgi.com>
Cc: Ken Chen <kenchen@google.com>
Signed-off-by: Fengguang Wu <wfg@mail.ustc.edu.cn>
Signed-off-by: David Chinner <dgc@sgi.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
2007-10-17 08:43:02 -07:00
..
kmem.c [PATCH] remove many unneeded #includes of sched.h 2007-02-14 08:09:54 -08:00
kmem.h Slab API: remove useless ctor parameter and reorder parameters 2007-10-17 08:42:45 -07:00
mrlock.h [XFS] Add lockdep support for XFS 2007-05-08 13:50:19 +10:00
mutex.h
sema.h [XFS] standardize on one sema init macro 2006-09-28 11:05:46 +10:00
spin.h
sv.h [XFS] Collapse sv_init and init_sv into just the one interface. 2006-09-28 11:05:52 +10:00
time.h
xfs_aops.c writeback: remove pages_skipped accounting in __block_write_full_page() 2007-10-17 08:43:02 -07:00
xfs_aops.h [PATCH] mark address_space_operations const 2006-06-28 14:59:04 -07:00
xfs_buf.c Fix up more bio fallout 2007-10-12 00:29:50 -07:00
xfs_buf.h [XFS] Only use refcounted pages for I/O 2007-07-14 15:21:14 +10:00
xfs_cred.h
xfs_dmapi_priv.h [XFS] Remove KERNEL_VERSION macros from xfs_dmapi.h 2006-11-11 18:05:06 +11:00
xfs_export.c [XFS] Keep stack usage down for 4k stacks by using noinline. 2007-02-10 18:34:56 +11:00
xfs_export.h
xfs_file.c Merge branch 'for-linus' of git://oss.sgi.com:8090/xfs/xfs-2.6 2007-07-19 14:41:33 -07:00
xfs_fs_subr.c [XFS] propogate return codes from flush routines 2007-05-08 13:49:27 +10:00
xfs_fs_subr.h [XFS] propogate return codes from flush routines 2007-05-08 13:49:27 +10:00
xfs_globals.c [XFS] Set filestreams object timeout to something sane. 2007-09-05 14:47:10 +10:00
xfs_globals.h
xfs_ioctl32.c xfs ioctl __user annotations 2007-07-26 11:11:57 -07:00
xfs_ioctl32.h
xfs_ioctl.c [XFS] Fix XFS_IOC_FSBULKSTAT{,_SINGLE} & XFS_IOC_FSINUMBERS in compat mode 2007-07-14 15:42:50 +10:00
xfs_iops.c [PATCH] mark struct inode_operations const 3 2007-02-12 09:48:46 -08:00
xfs_iops.h [PATCH] mark struct inode_operations const 3 2007-02-12 09:48:46 -08:00
xfs_linux.h [XFS] Concurrent Multi-File Data Streams 2007-07-14 15:40:53 +10:00
xfs_lrw.c xfs: convert to new aops 2007-10-16 09:42:55 -07:00
xfs_lrw.h sendfile: remove .sendfile from filesystems that use generic_file_sendfile() 2007-07-10 08:04:13 +02:00
xfs_stats.c
xfs_stats.h
xfs_super.c Slab API: remove useless ctor parameter and reorder parameters 2007-10-17 08:42:45 -07:00
xfs_super.h knfsd: exportfs: add exportfs.h header 2007-07-17 10:23:06 -07:00
xfs_sysctl.c [XFS] Concurrent Multi-File Data Streams 2007-07-14 15:40:53 +10:00
xfs_sysctl.h [XFS] Concurrent Multi-File Data Streams 2007-07-14 15:40:53 +10:00
xfs_version.h
xfs_vfs.c
xfs_vfs.h [XFS] Fix remount,readonly path to flush everything correctly. 2007-07-14 15:35:58 +10:00
xfs_vnode.c [XFS] Keep stack usage down for 4k stacks by using noinline. 2007-02-10 18:34:56 +11:00
xfs_vnode.h [XFS] XFS should not be looking at filp reference counts 2007-07-14 15:37:37 +10:00