From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752941AbbCZVLt (ORCPT ); Thu, 26 Mar 2015 17:11:49 -0400 Received: from mx1.redhat.com ([209.132.183.28]:59841 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752191AbbCZVLr (ORCPT ); Thu, 26 Mar 2015 17:11:47 -0400 Date: Thu, 26 Mar 2015 17:11:46 -0400 From: Don Zickus To: David Ahern Cc: acme@kernel.org, linux-kernel@vger.kernel.org, Joe Mario , Jiri Olsa Subject: Re: [PATCH v2] perf tool: Fix ppid for synthesized fork events Message-ID: <20150326211146.GZ162412@redhat.com> References: <1427302270-10178-1-git-send-email-dsahern@gmail.com> <20150325191526.GX162412@redhat.com> <551312C0.4060706@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <551312C0.4060706@gmail.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Mar 25, 2015 at 01:55:44PM -0600, David Ahern wrote: > >Hmm, I am not entirely sure this is correct. You made an optimization that > >hides the negative impact your patch does. I would prefer you split this > >patch into two pieces. One with the read loop optimization (which I think > >is great) and the second is your ppid change. > > > >I would then like to redo our test with the first patch applied and then > >both patches applied. > > > > From your other response I take it you understand the patch now? It > is a matter of semantics to break this single into 2 -- optimize the > existing code and then add the ppid. End result will be what this > patch shows. Before I do that can you /Joe confirm the performance > is acceptable? Hi David, Sorry for drawing this out. Originally the performance still seemed off. But as we split the patch up to see where the perf impact was, the problem seemed to have disappeared. So we are testing the original patch again. The only difference now is we were playing with the -BN option in perf based on your changelog, just because we never used it before. :-) One last test without the -BN option and if that looks fine, then we have no objections. Again sorry for dragging this out. I will let you know tomorrow EST. Cheers, Don