All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dave Hansen <dave.hansen@linux.intel.com>
To: Linus Torvalds <torvalds@linux-foundation.org>,
	Peter Anvin <hpa@zytor.com>
Cc: Theodore Ts'o <tytso@mit.edu>,
	Andrew Morton <akpm@linux-foundation.org>,
	"linux-ext4@vger.kernel.org" <linux-ext4@vger.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [REGRESSION] 998ef75ddb and aio-dio-invalidate-failure w/ data=journal
Date: Mon, 5 Oct 2015 13:48:59 -0700	[thread overview]
Message-ID: <5612E23B.7070606@linux.intel.com> (raw)
In-Reply-To: <CA+55aFw1AcOL7+ZUKL=bC9GLJ3iMehQyqLWThAa=O7p1YdoEAQ@mail.gmail.com>

On 10/05/2015 01:22 PM, Linus Torvalds wrote:
> On Mon, Oct 5, 2015 at 5:23 PM, Dave Hansen <dave.hansen@linux.intel.com> wrote:
>> One thing I've been noticing on Skylake is that barriers (implicit and
>> explicit) are showing up more in profiles.
> 
> Ahh, you're on skylake?

Yup.

> It's entirely possible that the issue is that the whole
> "stac/mov/clac" is much more expensive because skylake actually ends
> up supporting those AC instructions. That would make sense.
> 
> We could probably do them outside the loop, rather than tightly around
> the actual move instructions. Peter (hpa), is there some sane
> interface to try to do that?

iov_iter_fault_in_readable() is just going and touching a single word in
the page so that it is faulted in, or a pair of words if it manages to
cross a page boundary (which isn't happening here).  I'm not sure
there's a loop to move them out of here (for the prefaulting part).

We could theoretically expand the stac/clac to be around the pair of
__get_user()s in fault_in_pages_readable() but that would only help the
case where we are crossing a page boundary.

Although I was probably wrong about the source of the overhead, the
point still remains that the prefaulting is eating cycles for no
practical benefit.

>>  What we're seeing here
>> probably isn't actually stac/clac overhead, but the cost of finishing
>> some other operations that are outstanding before we can proceed through
>> here.
> 
> I suspect it actually _is_ stac/clac overhead. It might well be that
> clac/stac ends up serializing loads some way. Last I heard, they were
> reasonably cheap but certainly not free - and when we're talking about
> something that just loops over bringing the line into cache, it might
> be relatively expensive.
> 
> How did you do the profile? Use "-e cycles:pp" to get the precise
> profile information, which should actually attribute the cost to the
> instruction that really causes it.

It reduced the skid a bit.

Plain (no -e"):
>        │      stac
>  24.57 │      mov    (%rcx),%sil
>  15.70 │      clac
>  28.77 │      test   %eax,%eax
>   2.15 │      mov    %sil,-0x1(%rbp)
>   8.93 │    ↓ jne    66
>   2.31 │      movslq %edx,%rdx

With "-e cycles:pp":
>        │      sub    $0x8,%rsp
>  24.57 │      stac
>  15.49 │      mov    (%rcx),%sil
>  29.06 │      clac
>   2.24 │      test   %eax,%eax
>   8.77 │      mov    %sil,-0x1(%rbp)
>   2.22 │    ↓ jne    66
>        │      movslq %edx,%rdx

  reply	other threads:[~2015-10-05 20:48 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-10-05 15:22 [REGRESSION] 998ef75ddb and aio-dio-invalidate-failure w/ data=journal Theodore Ts'o
2015-10-05 15:58 ` Linus Torvalds
2015-10-05 16:23   ` Dave Hansen
2015-10-05 20:22     ` Linus Torvalds
2015-10-05 20:48       ` Dave Hansen [this message]
2015-10-05 21:18         ` Linus Torvalds
2015-10-05 21:55           ` Linus Torvalds
2015-10-05 23:33             ` Dave Hansen
2015-10-06  9:01               ` Linus Torvalds
2015-10-05 20:49       ` H. Peter Anvin
2015-10-06  7:56         ` Ingo Molnar
2015-10-06  9:10           ` Linus Torvalds
2015-10-06  9:27             ` Ingo Molnar
2015-10-06 13:29               ` Linus Torvalds
2015-10-06 13:42                 ` Ingo Molnar
2015-10-05 16:03 ` Dave Hansen
2015-10-05 18:04 ` Dave Hansen
2015-10-07  3:34   ` Theodore Ts'o
2015-10-07  7:32     ` Linus Torvalds
2015-10-07 15:43       ` Theodore Ts'o
2015-10-09  4:01         ` [PATCH] ext4: use private version of page_zero_new_buffers() for data=journal mode Theodore Ts'o
2015-10-13  6:06           ` Leonid V. Fedorenchik
2015-10-15 11:17           ` Jan Kara
2025-01-26 17:01           ` Mateusz Guzik
2025-01-26 18:48             ` Linus Torvalds
2025-01-26 19:49               ` Mateusz Guzik
2025-01-26 22:03                 ` Linus Torvalds
2025-01-26 22:45                   ` Mateusz Guzik
2025-01-27 20:52                     ` Dave Hansen
2025-01-27 21:46                       ` Dave Chinner

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=5612E23B.7070606@linux.intel.com \
    --to=dave.hansen@linux.intel.com \
    --cc=akpm@linux-foundation.org \
    --cc=hpa@zytor.com \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=torvalds@linux-foundation.org \
    --cc=tytso@mit.edu \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.