Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1739157 > unrolled thread

Re: [PATCH v3 00/20] Speculative page faults

Back to article view | Back to linux.kernel

Contents

  Re: [PATCH v3 00/20] Speculative page faults Alexei Starovoitov <alexei.starovoitov@gmail.com> - 2017-09-25 18:30 +0200
    Re: [PATCH v3 00/20] Speculative page faults Andrew Morton <akpm@linux-foundation.org> - 2017-09-26 01:40 +0200
      Re: [PATCH v3 00/20] Speculative page faults Laurent Dufour <ldufour@linux.vnet.ibm.com> - 2017-09-28 14:20 +0200
      Re: [PATCH v3 00/20] Speculative page faults Laurent Dufour <ldufour@linux.vnet.ibm.com> - 2017-09-28 14:30 +0200
        Re: [PATCH v3 00/20] Speculative page faults Andrew Morton <akpm@linux-foundation.org> - 2017-09-28 22:40 +0200
          Re: [PATCH v3 00/20] Speculative page faults Laurent Dufour <ldufour@linux.vnet.ibm.com> - 2017-09-29 17:30 +0200
    Re: [PATCH v3 00/20] Speculative page faults Laurent Dufour <ldufour@linux.vnet.ibm.com> - 2017-09-26 10:40 +0200

#1739157 — Re: [PATCH v3 00/20] Speculative page faults

On Mon, Sep 18, 2017 at 12:15 AM, Laurent Dufour
<ldufour@linux.vnet.ibm.com> wrote:
> Despite the unprovable lockdep warning raised by Sergey, I didn't get any
> feedback on this series.
>
> Is there a chance to get it moved upstream ?

what is the status ?
We're eagerly looking forward for this set to land,
since we have several use cases for tracing that
will build on top of this set as discussed at Plumbers.

[toc] | [next] | [standalone]

#1739362

On Mon, 25 Sep 2017 09:27:43 -0700 Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:

> On Mon, Sep 18, 2017 at 12:15 AM, Laurent Dufour
> <ldufour@linux.vnet.ibm.com> wrote:
> > Despite the unprovable lockdep warning raised by Sergey, I didn't get any
> > feedback on this series.
> >
> > Is there a chance to get it moved upstream ?
> 
> what is the status ?
> We're eagerly looking forward for this set to land,
> since we have several use cases for tracing that
> will build on top of this set as discussed at Plumbers.

There has been sadly little review and testing so far :(

I'll be taking a close look at it all over the next couple of weeks. 

One terribly important thing (especially for a patchset this large and
intrusive) is the rationale for merging it: the justification, usually
in the form of end-user benefit.

Laurent's [0/n] provides some nice-looking performance benefits for
workloads which are chosen to show performance benefits(!) but, alas,
no quantitative testing results for workloads which we may suspect will
be harmed by the changes(?).  Even things as simple as impact upon
single-threaded pagefault-intensive workloads and its effect upon
CONFIG_SMP=n .text size?

If you have additional usecases then please, spell them out for us in
full detail so we can better understand the benefits which this
patchset provides.

[toc] | [prev] | [next] | [standalone]

#1741483

Hi,

On 26/09/2017 01:34, Andrew Morton wrote:
> On Mon, 25 Sep 2017 09:27:43 -0700 Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
> 
>> On Mon, Sep 18, 2017 at 12:15 AM, Laurent Dufour
>> <ldufour@linux.vnet.ibm.com> wrote:
>>> Despite the unprovable lockdep warning raised by Sergey, I didn't get any
>>> feedback on this series.
>>>
>>> Is there a chance to get it moved upstream ?
>>
>> what is the status ?
>> We're eagerly looking forward for this set to land,
>> since we have several use cases for tracing that
>> will build on top of this set as discussed at Plumbers.
> 
> There has been sadly little review and testing so far :(

I do agree and I could just encourage people to do so :/

> I'll be taking a close look at it all over the next couple of weeks.

Thanks Andrew for giving it a close look.

> One terribly important thing (especially for a patchset this large and
> intrusive) is the rationale for merging it: the justification, usually
> in the form of end-user benefit.

The benefit is only for multi-threaded processes. But even on *small* 
systems with 16 CPUs, there is a real benefit.

> 
> Laurent's [0/n] provides some nice-looking performance benefits for
> workloads which are chosen to show performance benefits(!) but, alas,
> no quantitative testing results for workloads which we may suspect will
> be harmed by the changes(?).

I did test with kernbench, involving gcc/ld which are not 
multi-threaded, AFAIK, and I didn't see any impact.
But if you know additional test I should give a try, please advise.

Regarding ebizzy, it was designed to simulate web server's activity, so 
I guess there will be improvements when running real web servers.

>  Even things as simple as impact upon
> single-threaded pagefault-intensive workloads and its effect upon
> CONFIG_SMP=n .text size?
> 
> If you have additional usecases then please, spell them out for us in
> full detail so we can better understand the benefits which this
> patchset provides.

The other use-case I'm aware of is on memory database, where performance 
improvements is really significant, as I mentioned in the header of my 
series.

Cheers,
Laurent.

[toc] | [prev] | [next] | [standalone]

#1741495

Hi Andrew,

On 26/09/2017 01:34, Andrew Morton wrote:
> On Mon, 25 Sep 2017 09:27:43 -0700 Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
> 
>> On Mon, Sep 18, 2017 at 12:15 AM, Laurent Dufour
>> <ldufour@linux.vnet.ibm.com> wrote:
>>> Despite the unprovable lockdep warning raised by Sergey, I didn't get any
>>> feedback on this series.
>>>
>>> Is there a chance to get it moved upstream ?
>>
>> what is the status ?
>> We're eagerly looking forward for this set to land,
>> since we have several use cases for tracing that
>> will build on top of this set as discussed at Plumbers.
> 
> There has been sadly little review and testing so far :(
> 
> I'll be taking a close look at it all over the next couple of weeks.
> 
> One terribly important thing (especially for a patchset this large and
> intrusive) is the rationale for merging it: the justification, usually
> in the form of end-user benefit.
> 
> Laurent's [0/n] provides some nice-looking performance benefits for
> workloads which are chosen to show performance benefits(!) but, alas,
> no quantitative testing results for workloads which we may suspect will
> be harmed by the changes(?).  Even things as simple as impact upon
> single-threaded pagefault-intensive workloads and its effect upon
> CONFIG_SMP=n .text size?

I forgot to mention in my previous email the impact on the .text section.

Here are the metrics I got :

.text size	UP		SMP		Delta
4.13-mmotm	8444201		8964137		6.16%
'' +spf		8452041		8971929		6.15%
	Delta	0.09%		0.09%	

No major impact as you could see.

Thanks,
Laurent

> If you have additional usecases then please, spell them out for us in
> full detail so we can better understand the benefits which this
> patchset provides.
> 

[toc] | [prev] | [next] | [standalone]

#1741811

On Thu, 28 Sep 2017 14:29:02 +0200 Laurent Dufour <ldufour@linux.vnet.ibm.com> wrote:

> > Laurent's [0/n] provides some nice-looking performance benefits for
> > workloads which are chosen to show performance benefits(!) but, alas,
> > no quantitative testing results for workloads which we may suspect will
> > be harmed by the changes(?).  Even things as simple as impact upon
> > single-threaded pagefault-intensive workloads and its effect upon
> > CONFIG_SMP=n .text size?
> 
> I forgot to mention in my previous email the impact on the .text section.
> 
> Here are the metrics I got :
> 
> .text size	UP		SMP		Delta
> 4.13-mmotm	8444201		8964137		6.16%
> '' +spf		8452041		8971929		6.15%
> 	Delta	0.09%		0.09%	
> 
> No major impact as you could see.

8k text increase seems rather a lot actually.  That's a lot more
userspace cacheclines that get evicted during a fault...

Is the feature actually beneficial on uniprocessor?

[toc] | [prev] | [next] | [standalone]

#1742203

Hi Andrew,

On 28/09/2017 22:38, Andrew Morton wrote:
> On Thu, 28 Sep 2017 14:29:02 +0200 Laurent Dufour <ldufour@linux.vnet.ibm.com> wrote:
> 
>>> Laurent's [0/n] provides some nice-looking performance benefits for
>>> workloads which are chosen to show performance benefits(!) but, alas,
>>> no quantitative testing results for workloads which we may suspect will
>>> be harmed by the changes(?).  Even things as simple as impact upon
>>> single-threaded pagefault-intensive workloads and its effect upon
>>> CONFIG_SMP=n .text size?
>>
>> I forgot to mention in my previous email the impact on the .text section.
>>
>> Here are the metrics I got :
>>
>> .text size	UP		SMP		Delta
>> 4.13-mmotm	8444201		8964137		6.16%
>> '' +spf		8452041		8971929		6.15%
>> 	Delta	0.09%		0.09%	
>>
>> No major impact as you could see.
> 
> 8k text increase seems rather a lot actually.  That's a lot more
> userspace cacheclines that get evicted during a fault...
> 
> Is the feature actually beneficial on uniprocessor?

This is useless on uniprocessor, and I will disable it on x86 when !SMP 
by not defining __HAVE_ARCH_CALL_SPF.
So the speculative page fault handler will not be built but the vm 
sequence counter and the SCRU stuff will still be there. I may also make 
it disabled through macro when __HAVE_ARCH_CALL_SPF is not defined, but 
this may obfuscated the code a bit...

On ppc64, as this feature requires book3s, it can't be built without SMP 
support.

I rebuild the code on my x86 guest with the following patch applied:
--- a/arch/x86/include/asm/pgtable_types.h
+++ b/arch/x86/include/asm/pgtable_types.h
@@ -260,7 +260,7 @@ enum page_cache_mode {
  /*
   * Advertise that we call the Speculative Page Fault handler.
   */
-#ifdef CONFIG_X86_64
+#if defined(CONFIG_X86_64) && defined(CONFIG_SMP)
  #define __HAVE_ARCH_CALL_SPF
  #endif

And this time I got the following size on UP :
		UP
4.13-mmotm	8444201
'' +spf		8447945 (previously 8452041)
		  +3744

If I disable all the vm_sequence operations and the SRCU stuff this 
would lead to 0.

Thanks,
Laurent.

[toc] | [prev] | [next] | [standalone]

#1739688

Hi Alexei,

Le 25/09/2017 à 18:27, Alexei Starovoitov a écrit :
> On Mon, Sep 18, 2017 at 12:15 AM, Laurent Dufour
> <ldufour@linux.vnet.ibm.com> wrote:
>> Despite the unprovable lockdep warning raised by Sergey, I didn't get any
>> feedback on this series.
>>
>> Is there a chance to get it moved upstream ?
> 
> what is the status ?

As mentioned by Andrew this lacks review and test, what about your 
Ack/Review/Tested-By ?

> We're eagerly looking forward for this set to land,
> since we have several use cases for tracing that
> will build on top of this set as discussed at Plumbers.

Unfortunately I was not able to attend Plumbers this year, but I'll be 
pleased, as well as the MM mailing readers, to get your feedback and use 
cases of this series.

Thanks,
Laurent.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web