headers
Johannes Weiner | 1 Nov 2011 11:52
Picon
Favicon

Re: [patch 3/5] mm: try to distribute dirty pages fairly across zones

On Sat, Oct 29, 2011 at 04:39:44AM +0800, Wu Fengguang wrote:
> [restore CC list]
> 
> > > I'm trying to understand where the performance gain comes from.
> > > 
> > > I noticed that in all cases, before/after patchset, nr_vmscan_write are all zero.
> > > 
> > > nr_vmscan_immediate_reclaim is significantly reduced though:
> > 
> > That's a good thing, it means we burn less CPU time on skipping
> > through dirty pages on the LRU.
> > 
> > Until a certain priority level, the dirty pages encountered on the LRU
> > list are marked PageReclaim and put back on the list, this is the
> > nr_vmscan_immediate_reclaim number.  And only below that priority, we
> > actually ask the FS to write them, which is nr_vmscan_write.
> 
> Yes, it is.
> 
> > I suspect this is where the performance improvement comes from: we
> > find clean pages for reclaim much faster.
> 
> That explains how it could reduce CPU overheads. However the dd's are
> throttled anyway, so I still don't understand how the speedup of dd page
> allocations improve the _IO_ performance.

They are throttled in balance_dirty_pages() when there are too many
dirty pages.  But they are also 'throttled' in direct reclaim when
there are too many clean + dirty pages.  Wild guess: speeding up
direct reclaim allows dirty pages to be generated faster and the
(Continue reading)

Permalink | Reply | 0 comments |
headers
Johannes Weiner | 1 Nov 2011 11:55
Picon
Favicon

Re: [patch 3/5] mm: try to distribute dirty pages fairly across zones

On Mon, Oct 31, 2011 at 07:33:21PM +0800, Wu Fengguang wrote:
> > //regression
> > 3) much increased cpu %user and %system for btrfs
> 
> Sorry I find out that the CPU time regressions for btrfs are caused by
> some additional trace events enabled on btrfs (for debugging an
> unrelated btrfs hang bug) which results in 7 times more trace event
> lines:
> 
>  2701238 /export/writeback/thresh=1000M/btrfs-1dd-4k-8p-2941M-1000M:10-3.1.0-rc9-ioless-full-nfs-wq5-next-20111014+
> 19054054 /export/writeback/thresh=1000M/btrfs-1dd-4k-8p-2941M-1000M:10-3.1.0-rc9-ioless-full-per-zone-dirty-next-20111014+
> 
> So no real regressions.

Phew :-)

> Besides, the patchset also performs good on random writes:
> 
> 3.1.0-rc9-ioless-full-nfs-wq5-next-20111014+ 
3.1.0-rc9-ioless-full-per-zone-dirty-next-20111014+  
> ------------------------  ------------------------  
>                     1.65        -5.1%         1.57  MMAP-RANDWRITE-4K/btrfs-fio_fat_mmap_randwrite_4k-4k-8p-4096M-20:10-X
>                    18.65        -6.4%        17.46  MMAP-RANDWRITE-4K/ext3-fio_fat_mmap_randwrite_4k-4k-8p-4096M-20:10-X
>                     2.09        +1.2%         2.12  MMAP-RANDWRITE-4K/ext4-fio_fat_mmap_randwrite_4k-4k-8p-4096M-20:10-X
>                     2.49        -0.3%         2.48  MMAP-RANDWRITE-4K/xfs-fio_fat_mmap_randwrite_4k-4k-8p-4096M-20:10-X
>                    51.35        +0.0%        51.36  MMAP-RANDWRITE-64K/btrfs-fio_fat_mmap_randwrite_64k-64k-8p-4096M-20:10-X
>                    45.20        +0.5%        45.43  MMAP-RANDWRITE-64K/ext3-fio_fat_mmap_randwrite_64k-64k-8p-4096M-20:10-X
>                    44.77        +0.7%        45.10  MMAP-RANDWRITE-64K/ext4-fio_fat_mmap_randwrite_64k-64k-8p-4096M-20:10-X
>                    45.11        +2.5%        46.23  MMAP-RANDWRITE-64K/xfs-fio_fat_mmap_randwrite_64k-64k-8p-4096M-20:10-X
>                   211.31        +0.2%       211.74  TOTAL write_bw
(Continue reading)

Permalink | Reply | 0 comments |
headers
Ben Hutchings | 1 Nov 2011 15:14
Picon

Re: [PATCH] Fix possible memory corruption in xfs_readlink

On Tue, 2011-10-18 at 02:18 -0200, Carlos Maiolino wrote:
> Fixes a possible memory corruption when the link is larger than
> MAXPATHLEN and XFS_DEBUG is not enabled. This also remove the
> S_ISLNK assert, since the inode mode is checked previously in
> xfs_readlink_by_handle() and via VFS.
> 
> Signed-off-by: Carlos Maiolino <cmaiolino <at> redhat.com>
> ---
>  fs/xfs/xfs_vnodeops.c |   11 ++++++++---
>  1 files changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/xfs/xfs_vnodeops.c b/fs/xfs/xfs_vnodeops.c
> index 51fc429..c3288be 100644
> --- a/fs/xfs/xfs_vnodeops.c
> +++ b/fs/xfs/xfs_vnodeops.c
>  <at>  <at>  -123,13 +123,18  <at>  <at>  xfs_readlink(
>  
>  	xfs_ilock(ip, XFS_ILOCK_SHARED);
>  
> -	ASSERT(S_ISLNK(ip->i_d.di_mode));
> -	ASSERT(ip->i_d.di_size <= MAXPATHLEN);
> -
>  	pathlen = ip->i_d.di_size;

pathlen is a signed int (32-bit) and di_size has signed 64-bit type.
So, even if di_size was verified to be non-negative earlier (is it?)...

>  	if (!pathlen)
>  		goto out;
>
(Continue reading)

Permalink | Reply | 6 comments |
headers
Bill Kendall | 1 Nov 2011 20:53
Picon
Favicon

[PATCH 2/4] xfstests: refactor xfsdump quota checking

Tests can enable/disable quota checking by passing -q or -Q to the
various dump and restore helper routines. But -q and -Q are valid
xfsdump/xfsrestore options, so in addition to being confusing, tests
cannot use these options. Use --check-quota and --no-check-quota
instead.

Signed-off-by: Bill Kendall <wkendall <at> sgi.com>
---
 061         |    3 ++-
 common.dump |    4 ++--
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/061 b/061
index 414675f..2b08e11 100755
--- a/061
+++ b/061
 <at>  <at>  -48,7 +48,8  <at>  <at>  dump_file=src/dumpfile # override dump_file to checked-in dumpfile
 session_label="stress_056"
 # we have no quotas to restore
 # if we happen to run this on crackle then put the hostname back
-_do_restore_file -Q  | sed -e 's/HOSTNAME/crackle/g' -e 's#SCRATCH_DEV#/dev/dsk/dks0d2s1#'
+_do_restore_file --no-check-quota |
+sed -e 's/HOSTNAME/crackle/g' -e 's#SCRATCH_DEV#/dev/dsk/dks0d2s1#'
 _diff_compare_sub
 _ls_nodate_compare_sub

diff --git a/common.dump b/common.dump
index 56b348a..09f1a91 100644
--- a/common.dump
+++ b/common.dump
(Continue reading)

Permalink | Reply | 0 comments |
headers
Bill Kendall | 1 Nov 2011 20:53
Picon
Favicon

[PATCH 4/4] xfstests: refactor dump test argument parsing

The dump and restore helper functions all call the same _parse_args()
function. xfsdump and xfsrestore have some common options, but others
have different meanings/syntaxes or only exist in one program or the
other. Split _parse_args() into dump and restore variants.

Further, a test cannot pass most options to xfsrestore because many of
the parsed args are not used by the restore helper functions.  Change
the helpers so that the parsed options are available (to be used in
future tests).

Also add a check for a missing --multi argument, and change a couple of
callers to be consistent and use $* instead of "$ <at> ".

Signed-off-by: Bill Kendall <wkendall <at> sgi.com>
---
 common.dump |   95 ++++++++++++++++++++++++++++++++++++++++++++---------------
 1 files changed, 71 insertions(+), 24 deletions(-)

diff --git a/common.dump b/common.dump
index 552085f..076517d 100644
--- a/common.dump
+++ b/common.dump
 <at>  <at>  -876,9 +876,10  <at>  <at>  _dir_filter()
 }

 #
+# Parse xfsdump arguments.
 # Note: requires a space between option letter and argument
 #
-_parse_args()
(Continue reading)

Permalink | Reply | 0 comments |
headers
Bill Kendall | 1 Nov 2011 20:53
Picon
Favicon

[PATCH 3/4] xfstests: allow dump file name to be passed as arg

xfsdump tests using tapes (rather than files) can pass the tape pathname
to the dump/restore helper functions using the -f option. Change
_parse_args() so that this can be done for file-based tests as well, so
that they don't have to set the global 'dump_file' variable before doing
the dump or restore.

Signed-off-by: Bill Kendall <wkendall <at> sgi.com>
---
 061         |    5 ++---
 064         |    9 +++------
 065         |    9 +++------
 264         |   12 ++++--------
 common.dump |    3 +++
 5 files changed, 15 insertions(+), 23 deletions(-)

diff --git a/061 b/061
index 2b08e11..8f1d149 100755
--- a/061
+++ b/061
 <at>  <at>  -44,11 +44,10  <at>  <at>  _supported_os IRIX Linux
 # _create_dumpdir_fill_perm (small dump)

 _create_dumpdir_fill_perm
-dump_file=src/dumpfile # override dump_file to checked-in dumpfile
-session_label="stress_056"
+# override dump_file to checked-in dumpfile
 # we have no quotas to restore
 # if we happen to run this on crackle then put the hostname back
-_do_restore_file --no-check-quota |
+_do_restore_file --no-check-quota -f src/dumpfile -L stress_056 |
(Continue reading)

Permalink | Reply | 0 comments |
headers
Bill Kendall | 1 Nov 2011 20:53
Picon
Favicon

[PATCH 0/4] xfstests: refactor arg processing in common.dump

This patch series reworks the argument processing for the
dump and restore helper routines in common.dump.

Note this series depends on the following outstanding patches,
as there are overlapping changes in _parse_args():
    xfstests: add test 264 for testing xfsdump -D
    xfstests: add 265 and 266 for multiple media

_______________________________________________
xfs mailing list
xfs <at> oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
Permalink | Reply | 4 comments |
headers
Bill Kendall | 1 Nov 2011 21:30
Picon
Favicon

[PATCH v2] xfsdump: fix metadata restore on split files

xfsrestore does not apply certain metadata until all of the file's
data has been restored. This allows, for example, files with the
immutable flag set to be restored properly.

While testing multi-stream restores, I noticed that files split
across multiple streams did not have their metadata restored.
Looking into this further, it also can occur with just a single
stream when the dump spans multiple tapes. Specifically it requires
that end-of-tape is hit just as a file is finished (before a new
file is begun), and the restore is performed as separate xfsrestore
invocations (one per tape).

The fix is to check to see if a file is completely restored whenever
we hit the end of a media file. The current code is broken because
it relies on all media files being applied during the same restore
session.

This also moves the S_ISREG() check into restore_complete_reg()
rather than relying on callers to make the check.

Signed-off-by: Bill Kendall <wkendall <at> sgi.com>
---
Changed for v2:
 - Reworded commit message to correctly characterize the existing bug.

 restore/content.c |   50 +++++++++++++++++++++++++++++++++-----------------
 1 files changed, 33 insertions(+), 17 deletions(-)

diff --git a/restore/content.c b/restore/content.c
index e5957bf..34fc4a0 100644
(Continue reading)

Permalink | Reply | 0 comments |
headers
Bill Kendall | 1 Nov 2011 20:53
Picon
Favicon

[PATCH 1/4] xfstests: refactor cumulative restore tests

The cumulative restore tests call _do_restore_file_cum(),
which requires a dump level to be passed in the -l option
in order to determine whether the restore directory needs
to be prepared or not. -l is not a valid xfsrestore option,
so doing things this way prevents tests from passing options
to xfsrestore. It's more straightforward to have the test
call _prepare_restore_dir itself prior to starting a series
of cumulative restores.

Signed-off-by: Bill Kendall <wkendall <at> sgi.com>
---
 064         |    3 ++-
 065         |    3 ++-
 264         |    5 +++--
 common.dump |    6 ++----
 4 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/064 b/064
index 16960c9..bf8a7bc 100755
--- a/064
+++ b/064
 <at>  <at>  -97,12 +97,13  <at>  <at>  while [ $i -le 9 ]; do
 done

 echo "Do the cumulative restores"
+_prepare_restore_dir
 i=0
 while [ $i -le 9 ]; do
     dump_file=$tmp.df.level$i
     echo ""
(Continue reading)

Permalink | Reply | 0 comments |
headers
Christoph Hellwig | 2 Nov 2011 10:20
Favicon

[PATCH v2] xfsprogs: fix various incorrect printf formats

Reported-by: Jakub Bogusz <qboosh <at> pld-linux.org>
Signed-off-by: Christoph Hellwig <hch <at> lst.de>

Index: xfsprogs/libxfs/freebsd.c
===================================================================
--- xfsprogs.orig/libxfs/freebsd.c	2011-10-30 05:56:36.180125966 +0100
+++ xfsprogs/libxfs/freebsd.c	2011-10-30 05:57:45.957126722 +0100
 <at>  <at>  -126,15 +126,13  <at>  <at>  platform_findsizes(char *path, int fd, l
 	}

 	if ((st.st_mode & S_IFMT) != S_IFCHR) {
-		fprintf(stderr, _("%s: "
-			"Not a device or file: \"%s\"n"),
+		fprintf(stderr, _("%s: Not a device or file: \"%s\"\n"),
 			progname, path);
 		exit(1);
 	}

 	if (ioctl(fd, DIOCGMEDIASIZE, &size) != 0) {
-		fprintf(stderr, _("%s: "
-			"DIOCGMEDIASIZE failed on \"%s\": %s\n"),
+		fprintf(stderr, _("%s: DIOCGMEDIASIZE failed on \"%s\": %s\n"),
 			progname, path, strerror(errno));
 		exit(1);
 	}
Index: xfsprogs/repair/dinode.c
===================================================================
--- xfsprogs.orig/repair/dinode.c	2011-10-30 05:56:36.132124261 +0100
+++ xfsprogs/repair/dinode.c	2011-10-30 05:57:45.957126722 +0100
 <at>  <at>  -1439,7 +1439,7  <at>  <at>  _("mismatch between format (%d) and size
(Continue reading)

Permalink | Reply | 2 comments |

Gmane