linux-kernel - Re: [PATCH] fs: sync: fixed performance regression

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 10 Jul 2013 16:45:42 -0700
From:	Paul Taysom <taysom@...gle.com>
To:	Paul Taysom <taysom@...omium.org>
Cc:	Alexander Viro <viro@...iv.linux.org.uk>,
	linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
	jack@...e.cz, sonnyrao@...omium.org
Subject: Re: [PATCH] fs: sync: fixed performance regression

On Wed, Jul 10, 2013 at 4:12 PM, Paul Taysom <taysom@...omium.org> wrote:
> The following commit introduced a 10x regression for
> syncing inodes in ext4 with relatime enabled where just
> the atime had been modified.
>
>     commit 4ea425b63a3dfeb7707fc7cc7161c11a51e871ed
>     Author: Jan Kara <jack@...e.cz>
>     Date:   Tue Jul 3 16:45:34 2012 +0200
>     vfs: Avoid unnecessary WB_SYNC_NONE writeback during sys_sync and reorder sync passes
>
>     See also: http://www.kernelhub.org/?msg=93100&p=2
>
> Fixed by putting back in the call to writeback_inodes_sb.
>
> I'll attach the test in a reply to this e-mail.
>
> The test starts by creating 512 files, syncing, reading one byte
> from each of those files, syncing, and then deleting each file
> and syncing. The time to do each sync is printed. The process
> is then repeated for 1024 files and then the next power of
> two up to 262144 files.
>
> Note, when running the test, the slow down doesn't always happen
> but most of the tests will show a slow down.
>
> In response to crbug.com/240422
>
> Signed-off-by: Paul Taysom <taysom@...omium.org>
> ---
>  fs/sync.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/fs/sync.c b/fs/sync.c
> index 905f3f6..55c3316 100644
> --- a/fs/sync.c
> +++ b/fs/sync.c
> @@ -73,6 +73,12 @@ static void sync_inodes_one_sb(struct super_block *sb, void *arg)
>                 sync_inodes_sb(sb);
>  }
>
> +static void writeback_inodes_one_sb(struct super_block *sb, void *arg)
> +{
> +       if (!(sb->s_flags & MS_RDONLY))
> +               writeback_inodes_sb(sb, WB_REASON_SYNC);
> +}
> +
>  static void sync_fs_one_sb(struct super_block *sb, void *arg)
>  {
>         if (!(sb->s_flags & MS_RDONLY) && sb->s_op->sync_fs)
> @@ -104,6 +110,7 @@ SYSCALL_DEFINE0(sync)
>         int nowait = 0, wait = 1;
>
>         wakeup_flusher_threads(0, WB_REASON_SYNC);
> +       iterate_supers(writeback_inodes_one_sb, NULL);
>         iterate_supers(sync_inodes_one_sb, NULL);
>         iterate_supers(sync_fs_one_sb, &nowait);
>         iterate_supers(sync_fs_one_sb, &wait);
> --
> 1.8.3
>
Try this again but in plaintext mode. Attaching test results and test
program. Tests were run on a Pixel x86 with SSD storage.

View attachment "syncperf.c" of type "text/x-csrc" (3102 bytes)

Download attachment "syncperf.results" of type "application/octet-stream" (6043 bytes)