All of lore.kernel.org
 help / color / mirror / Atom feed
From: Gregory Haskins <ghaskins@novell.com>
To: Ingo Molnar <mingo@elte.hu>
Cc: Mike Galbraith <efault@gmx.de>,
	LKML <linux-kernel@vger.kernel.org>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>
Subject: Re: [revert] mysql+oltp regression
Date: Mon, 11 Aug 2008 09:03:34 -0400	[thread overview]
Message-ID: <48A038A6.4010104@novell.com> (raw)
In-Reply-To: <20080811124857.GD10082@elte.hu>

[-- Attachment #1: Type: text/plain, Size: 3876 bytes --]

Ingo Molnar wrote:
> * Gregory Haskins <ghaskins@novell.com> wrote:
>
>   
>> Ingo Molnar wrote:
>>     
>>> * Mike Galbraith <efault@gmx.de> wrote:
>>>
>>>   
>>>       
>>>> Greetings,
>>>>
>>>> During regression testing of tip/sched/clock fixes, a regression in  
>>>> low client count throughput turned up, which I traced this back to 
>>>> the commit below.  I don't see anything wrong with it, but suspect 
>>>> that it is preventing client/server pairs from staying together on 
>>>> the same CPU as buddies, which mysql definitely likes quite a lot.  
>>>> (I suspect that this is the case, because I've seen this same 
>>>> performance curve while tinkering with wakeup affinity and breaking 
>>>> it all to pieces;)
>>>>
>>>> Changelog and test results below in case nobody sees a problem with  
>>>> the commit itself.
>>>>     
>>>>         
>>> i've applied your fix to tip/sched/urgent for the time being, thanks  
>>> Mike for tracking it down. We can re-try newer iterations of Greg's  
>>> patch in tip/sched/devel.
>>>
>>>   
>>>       
>> Hmm..  The patch still looks correct afaict.  I fear we are just 
>> papering over some other issue by reverting it, but I will try to see 
>> if I can track this down.  We will, of course, now be skipping trying 
>> to balance the (effectively random) last task in the queue which may 
>> or may not result in better performance on sheer luck instead of 
>> algorithmic intelligence.  This makes me nervous.
>>     
>
> yeah - but we had that behavior for quite some time.
>
> This is how the patch cycle works normally: we had a fair chance to 
> discover this problem in your testing then in -tip testing and then in 
> linux-next or -mm but we didnt find it at any stage.
>
> Now we are in the upstream release cycle so unless there's some 
> immediate fix available (or there are _really_ strong reasons against 
> the revert) doing the revert is the right approach.
>
> A revert is not necessarily the indicator of the quality of the change 
> in question, it is a tester-driven exception event that guarantees that 
> the kernel improves in a monotonic way. (for all testers who opt to help 
> us in doing so)
>
> And given that the problem was readily reproducible for Mike, it should 
> be reproducible for you as well - so we dont actually make the bug 
> harder to fix by doing the revert.
>
> Perhaps we should introduce the notion of "Defer-to-next-release" 
> reverts - which this really is - in contrast to "Revert-because-bad", 
> which your change definitely is not.
>   

Hi Ingo,
  Understood, and a totally reasonable stance.  I mostly wanted to make 
sure it was understood that I don't think I can "fix" that particular 
patch since I think it was already correct.  Rather, I will have to try 
to identify some other area (presumably the load balancer) to harmonize 
with it.  I think we are on the same page, though. :)


>   
>> Speaking of this: Another patch I submitted to you Ingo (had to do 
>> with updating the load_weight inside task_setprio) seems to also have 
>> this phenomenon: e.g. its technically correct but further testing has 
>> revealed negative repercussions elsewhere.  So please ignore that 
>> patch (or revert if you already pulled in, but I don't think you 
>> have).  Ill try to look into this issue as well.
>>     
>
> ok, under which thread/subject is that? Not queued in tip/sched/* yet, 
> correct?
>   
Here is the original thread:

http://lkml.org/lkml/2008/7/3/416

I do not believe you have queued it anywhere (public anyway) yet.

Note I have already invalidated 1/2, and now I am retracting 2/2 as 
well.  (1/2 is actually a bogus patch, 2/2 is "technically correct" but 
causes ripples in the load balancer that need to be sorted out first.

Thanks!
-Greg



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]

  parent reply	other threads:[~2008-08-11 13:06 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-08-11 11:32 [revert] mysql+oltp regression Mike Galbraith
2008-08-11 11:43 ` Ingo Molnar
2008-08-11 12:27   ` Gregory Haskins
2008-08-11 12:48     ` Ingo Molnar
2008-08-11 12:51       ` Ingo Molnar
2008-08-11 13:03       ` Gregory Haskins [this message]
2008-08-11 13:14         ` Ingo Molnar
2008-08-11 13:19           ` Gregory Haskins
2008-08-11 13:27             ` Gregory Haskins
2008-08-11 13:31               ` Ingo Molnar
2008-08-11 13:29             ` Ingo Molnar

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=48A038A6.4010104@novell.com \
    --to=ghaskins@novell.com \
    --cc=a.p.zijlstra@chello.nl \
    --cc=efault@gmx.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    /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.