From: kemi <kemi.wang@intel.com>
To: Linus Torvalds <torvalds@linux-foundation.org>,
David Howells <dhowells@redhat.com>
Cc: Jeff Layton <jlayton@redhat.com>,
Ye Xiaolong <xiaolong.ye@intel.com>, LKP <lkp@01.org>,
LKML <linux-kernel@vger.kernel.org>
Subject: Re: [LKP] [lkp-robot] [iversion] c0cef30e4f: aim7.jobs-per-min -18.0% regression
Date: Fri, 2 Mar 2018 13:54:29 +0800 [thread overview]
Message-ID: <7f1b2159-edc0-57de-eed0-f544d638e8c0@intel.com> (raw)
In-Reply-To: <CA+55aFx+oUB_H4KFTkuT+goMRABkjbdS211Zy2fG96RHuqbhBA@mail.gmail.com>
On 2018年02月28日 01:04, Linus Torvalds wrote:
> On Tue, Feb 27, 2018 at 5:43 AM, David Howells <dhowells@redhat.com> wrote:
>> Is it possible there's a stall between the load of RCX and the subsequent
>> instructions because they all have to wait for RCX to become available?
>
> No. Modern Intel big-core CPU's simply aren't that fragile. All these
> instructions should do OoO fine for trivial sequences like this, and
> as far as I can tell, the new code sequence should be better.
>
> And even if it were worse for some odd reason, it would be worse by a cycle.
>
> This kind of 18% change is something else, it is definitely not about
> instruction scheduling.
>
> Now, if the change to inode_cmp_iversion() causes some actual
> _behavioral_ changes, and we get more IO, that's more like it. But the
> code really does seem to be equivalent. In both cases it is simply
> comparing 63 bits: the high 63 bits of 0x150(%rbp) - inode->i_version
> - with the low 63 bits of 0x20(%rax) - iint->version.
>
> The only issue would be if the high bit of 0x20(%rax) was somehow set.
> The new code doesn't shift that bit away an more, but it should never
> be set since it comes from
>
> i_version = inode_query_iversion(inode);
> ...
> iint->version = i_version;
>
> and that inode_query_iversion() will have done the version shift.
>
>> The interleaving between operating on RSI and RCX in the older code might
>> alleviate that.
>>
>> In addition, the load if the 20(%rax) value is now done in the CMP instruction
>> rather than earlier, so it might not get speculatively loaded in time, whereas
>> the earlier code explicitly loads it up front.
>
> No again, OoO cores will generally hide details like that.
>
> You can see effects of it, but it's hard, and it can go both ways.
>
> Anyway, I think the _real_ change has nothing to with instruction
> scheduling, and everything to do with this:
>
> 107.62 ± 37% +139.1% 257.38 ± 16% vmstat.io.bo
> 48740 ± 36% +191.4% 142047 ± 16% proc-vmstat.pgpgout
>
> (There's fairly big variation in those numbers, but the changes are
> even bigger) or this:
>
> 258.12 -100.0% 0.00 turbostat.Avg_MHz
> 21.48 -21.5 0.00 turbostat.Busy%
>
This is caused by a limitation in current turbostat parse script of lkp. It
treats a string including wildcard character (e.g. 30.**) in the output of turbostat
monitor as an error and set all the stats value as 0.
Turbostat monitor runs successfully during these tests.
> or this:
>
> 27397 ±194% +43598.3% 11972338 ±139%
> latency_stats.max.io_schedule.nfs_lock_and_join_requests.nfs_updatepage.nfs_write_end.generic_perform_write.nfs_file_write.__vfs_write.vfs_write.SyS_write.entry_SYSCALL_64_fastpath
> 27942 ±189% +96489.5% 26989044 ±139%
> latency_stats.sum.io_schedule.nfs_lock_and_join_requests.nfs_updatepage.nfs_write_end.generic_perform_write.nfs_file_write.__vfs_write.vfs_write.SyS_write.entry_SYSCALL_64_fastpath
>
> but those all sound like something changed in the setup, not in the kernel.
>
> Odd.
>
> Linus
>
next prev parent reply other threads:[~2018-03-02 5:56 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-02-25 15:05 kernel test robot
2018-02-25 15:41 ` Jeff Layton
2018-02-26 8:38 ` Ye Xiaolong
2018-02-26 11:43 ` Jeff Layton
2018-02-26 12:33 ` Jeff Layton
2018-02-27 7:42 ` [LKP] " kemi
2018-02-27 13:29 ` Jeff Layton
2018-02-27 13:43 ` David Howells
2018-02-27 15:27 ` Jeff Layton
2018-02-27 17:04 ` Linus Torvalds
2018-03-02 5:54 ` kemi [this message]
2018-03-15 7:33 ` kemi
2018-03-15 17:46 ` Linus Torvalds
2018-02-25 21:18 ` Linus Torvalds
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=7f1b2159-edc0-57de-eed0-f544d638e8c0@intel.com \
--to=kemi.wang@intel.com \
--cc=dhowells@redhat.com \
--cc=jlayton@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=lkp@01.org \
--cc=torvalds@linux-foundation.org \
--cc=xiaolong.ye@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®