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

Groups > linux.kernel > #1613648 > unrolled thread

linux-next: manual merge of the akpm tree with the tip tree

Back to article view | Back to linux.kernel

Contents

  linux-next: manual merge of the akpm tree with the tip tree Stephen Rothwell <sfr@canb.auug.org.au> - 2017-03-31 07:50 +0200
    Re: linux-next: manual merge of the akpm tree with the tip tree Peter Zijlstra <peterz@infradead.org> - 2017-03-31 08:50 +0200
      Re: linux-next: manual merge of the akpm tree with the tip tree Andi Kleen <ak@linux.intel.com> - 2017-03-31 16:00 +0200
        Re: linux-next: manual merge of the akpm tree with the tip tree Peter Zijlstra <peterz@infradead.org> - 2017-03-31 16:50 +0200
          Re: linux-next: manual merge of the akpm tree with the tip tree Andi Kleen <ak@linux.intel.com> - 2017-03-31 18:10 +0200
            Re: linux-next: manual merge of the akpm tree with the tip tree Peter Zijlstra <peterz@infradead.org> - 2017-03-31 19:50 +0200

#1613648 — linux-next: manual merge of the akpm tree with the tip tree

Hi all,

Today's linux-next merge of the akpm tree got a conflict in:

  kernel/sched/fair.c

between commit:

  0ccb977f4c80 ("sched/fair: Explicitly generate __update_load_avg() instances")

from the tip tree and patch:

   "kernel/sched/fair.c: uninline __update_load_avg()"

from the akpm tree.

I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging.  You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.

-- 
Cheers,
Stephen Rothwell

diff --cc kernel/sched/fair.c
index 359dbc05a3b4,28a2bd8bfb67..000000000000
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@@ -2929,11 -2848,14 +2929,11 @@@ accumulate_sum(u64 delta, int cpu, stru
   *   load_avg = u_0` + y*(u_0 + u_1*y + u_2*y^2 + ... )
   *            = u_0 + u_1*y + u_2*y^2 + ... [re-labeling u_i --> u_{i+1}]
   */
- static __always_inline int
+ static int
 -__update_load_avg(u64 now, int cpu, struct sched_avg *sa,
 +___update_load_avg(u64 now, int cpu, struct sched_avg *sa,
  		  unsigned long weight, int running, struct cfs_rq *cfs_rq)
  {
 -	u64 delta, scaled_delta, periods;
 -	u32 contrib;
 -	unsigned int delta_w, scaled_delta_w, decayed = 0;
 -	unsigned long scale_freq, scale_cpu;
 +	u64 delta;
  
  	delta = now - sa->last_update_time;
  	/*

[toc] | [next] | [standalone]

#1613663

On Fri, Mar 31, 2017 at 04:44:51PM +1100, Stephen Rothwell wrote:
> Hi all,
> 
> Today's linux-next merge of the akpm tree got a conflict in:
> 
>   kernel/sched/fair.c
> 
> between commit:
> 
>   0ccb977f4c80 ("sched/fair: Explicitly generate __update_load_avg() instances")
> 
> from the tip tree and patch:
> 
>    "kernel/sched/fair.c: uninline __update_load_avg()"
> 
> from the akpm tree.
> 
> I fixed it up (see below) and can carry the fix as necessary. This
> is now fixed as far as linux-next is concerned, but any non trivial
> conflicts should be mentioned to your upstream maintainer when your tree
> is submitted for merging.  You may also want to consider cooperating
> with the maintainer of the conflicting tree to minimise any particularly
> complex conflicts.

Argh!

Andrew, please drop that patch. And the x86 out-of-line of __atomic_add_unless().

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

#1614045

> Argh!
> 
> Andrew, please drop that patch. And the x86 out-of-line of __atomic_add_unless().

Why dropping the second?  Do you have something better?

On the first there were no 0day regressions, so at least basic performance
checking has been done.

-Andi

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

#1614081

On Fri, Mar 31, 2017 at 06:54:48AM -0700, Andi Kleen wrote:
> > Argh!
> > 
> > Andrew, please drop that patch. And the x86 out-of-line of __atomic_add_unless().
> 
> Why dropping the second?  Do you have something better?

The try_cmpxchg() patches save about half the text, and do not have the
out-of-line penalty as shown here:

   https://lkml.kernel.org/r/20170322165144.dtidvvbxey7w5pbd@hirez.programming.kicks-ass.net

> On the first there were no 0day regressions, so at least basic performance
> checking has been done.

The first is superseded by much better patches in the scheduler tree.

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

#1614130

On Fri, Mar 31, 2017 at 04:45:46PM +0200, Peter Zijlstra wrote:
> On Fri, Mar 31, 2017 at 06:54:48AM -0700, Andi Kleen wrote:
> > > Argh!
> > > 
> > > Andrew, please drop that patch. And the x86 out-of-line of __atomic_add_unless().
> > 
> > Why dropping the second?  Do you have something better?
> 
> The try_cmpxchg() patches save about half the text, and do not have the
> out-of-line penalty as shown here:
> 
>    https://lkml.kernel.org/r/20170322165144.dtidvvbxey7w5pbd@hirez.programming.kicks-ass.net

Where is the source for the benchmark?

Based on the description it sounds like it's testing atomic_inc(), which my patches
don't change.

BTW testing such things in tight loops is bad practice. If you run
them back to back the CPU pipeline has to do much more serialization,
which is usually not realistic and drastically overestimates
the overhead.

A better practice is to run some real workload. If you want to see
cycle counts you can look at LBR cycles, or PT cycles from sampling or tracing.

> > On the first there were no 0day regressions, so at least basic performance
> > checking has been done.
> 
> The first is superseded by much better patches in the scheduler tree.

Which patches exactly?  The new patches shrink the text too?

-Andi

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

#1614195

On Fri, Mar 31, 2017 at 09:02:42AM -0700, Andi Kleen wrote:
> On Fri, Mar 31, 2017 at 04:45:46PM +0200, Peter Zijlstra wrote:
> > On Fri, Mar 31, 2017 at 06:54:48AM -0700, Andi Kleen wrote:
> > > > Argh!
> > > > 
> > > > Andrew, please drop that patch. And the x86 out-of-line of __atomic_add_unless().
> > > 
> > > Why dropping the second?  Do you have something better?
> > 
> > The try_cmpxchg() patches save about half the text, and do not have the
> > out-of-line penalty as shown here:
> > 
> >    https://lkml.kernel.org/r/20170322165144.dtidvvbxey7w5pbd@hirez.programming.kicks-ass.net
> 
> Where is the source for the benchmark?

In that email; heck marc.info even provides a downloadable link, you
don't even have to go find it in your local lkml archives.

> Based on the description it sounds like it's testing atomic_inc(),
> which my patches don't change.

Yes, reading is hard.

It tests:

 lock incl

vs

 call refcount_inc

vs

 $inlined refcount_inc

And refcount_inc() is more complex than add_unless().

> BTW testing such things in tight loops is bad practice. If you run
> them back to back the CPU pipeline has to do much more serialization,
> which is usually not realistic and drastically overestimates
> the overhead.
> 
> A better practice is to run some real workload. If you want to see
> cycle counts you can look at LBR cycles, or PT cycles from sampling or tracing.

Hey, at least I did benchmark it. You just waved your hands and are
causing extra work for other people.

> > > On the first there were no 0day regressions, so at least basic performance
> > > checking has been done.
> > 
> > The first is superseded by much better patches in the scheduler tree.
> 
> Which patches exactly?  The new patches shrink the text too?

Try your local google foo; or look at the patch that conflicted, its
that one and the next.

In the end it comes down to -mm carrying patches against trees that are
maintained elsewhere without acks from said maintainers. I don't feel
bad about causing conflicts.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web