From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <konstantin.ananyev@intel.com>
Received: from mga02.intel.com (mga02.intel.com [134.134.136.20])
 by dpdk.org (Postfix) with ESMTP id 1C4F21B6DB
 for <dev@dpdk.org>; Mon, 26 Nov 2018 10:34:02 +0100 (CET)
X-Amp-Result: SKIPPED(no attachment in message)
X-Amp-File-Uploaded: False
Received: from orsmga002.jf.intel.com ([10.7.209.21])
 by orsmga101.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384;
 26 Nov 2018 01:34:02 -0800
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.56,281,1539673200"; d="scan'208";a="111527722"
Received: from irsmsx105.ger.corp.intel.com ([163.33.3.28])
 by orsmga002.jf.intel.com with ESMTP; 26 Nov 2018 01:34:00 -0800
Received: from irsmsx106.ger.corp.intel.com ([169.254.8.8]) by
 irsmsx105.ger.corp.intel.com ([169.254.7.144]) with mapi id 14.03.0415.000;
 Mon, 26 Nov 2018 09:33:59 +0000
From: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>
To: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>, "Varghese, Vipin"
 <vipin.varghese@intel.com>, Stephen Hemminger <stephen@networkplumber.org>
CC: "Pattan, Reshma" <reshma.pattan@intel.com>, "dev@dpdk.org" <dev@dpdk.org>, 
 "thomas@monjalon.net" <thomas@monjalon.net>, "Mcnamara, John"
 <john.mcnamara@intel.com>, "Byrne, Stephen1" <stephen1.byrne@intel.com>,
 "Glynn, Michael J" <michael.j.glynn@intel.com>, "Patel, Amol"
 <amol.patel@intel.com>
Thread-Topic: [PATCH v4 5/9] app/procinfo: add support for show tm
Thread-Index: AQHUhT7DxSutPQTiRki7qeUOKBTbk6VhycOwgAABymA=
Date: Mon, 26 Nov 2018 09:33:59 +0000
Message-ID: <2601191342CEEE43887BDE71AB977258010CEBC50B@IRSMSX106.ger.corp.intel.com>
References: <20181106124912.40700-1-vipin.varghese@intel.com>
 <20181106124912.40700-5-vipin.varghese@intel.com>
 <3AEA2BF9852C6F48A459DA490692831F2A3D58A3@irsmsx110.ger.corp.intel.com>
 <4C9E0AB70F954A408CC4ADDBF0F8FA7D4D2C1435@BGSMSX101.gar.corp.intel.com>
 <3AEA2BF9852C6F48A459DA490692831F2A3D61A6@irsmsx110.ger.corp.intel.com>
 <4C9E0AB70F954A408CC4ADDBF0F8FA7D4D2C1A45@BGSMSX101.gar.corp.intel.com>
 <3AEA2BF9852C6F48A459DA490692831F2A3D6264@irsmsx110.ger.corp.intel.com>
 <4C9E0AB70F954A408CC4ADDBF0F8FA7D4D2C1A79@BGSMSX101.gar.corp.intel.com>
 <3AEA2BF9852C6F48A459DA490692831F2A3D62FB@irsmsx110.ger.corp.intel.com>
 <4C9E0AB70F954A408CC4ADDBF0F8FA7D4D2C1B26@BGSMSX101.gar.corp.intel.com>
 <20181123092215.50d299ae@xeon-e3>
 <4C9E0AB70F954A408CC4ADDBF0F8FA7D4D2C1FD9@BGSMSX101.gar.corp.intel.com>
 <2601191342CEEE43887BDE71AB977258010CEBC4DC@IRSMSX106.ger.corp.intel.com>
In-Reply-To: <2601191342CEEE43887BDE71AB977258010CEBC4DC@IRSMSX106.ger.corp.intel.com>
Accept-Language: en-IE, en-US
Content-Language: en-US
X-MS-Has-Attach: 
X-MS-TNEF-Correlator: 
x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiZjM4MzQyYTctNmQyNy00YmE1LTk0YzYtMTRiYzU3N2IxYzVlIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiVm1CTHRsa21IOG9hUE4xMldpR09mMzJrWENPeVRoa3ZNaGY1UlZ6dmVrMHFGcmdBVXh6QUQyc1I3TlR4Z3lvKyJ9
x-ctpclassification: CTP_NT
dlp-product: dlpe-windows
dlp-version: 11.0.400.15
dlp-reaction: no-action
x-originating-ip: [163.33.239.180]
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
Subject: Re: [dpdk-dev] [PATCH v4 5/9] app/procinfo: add support for show tm
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Mon, 26 Nov 2018 09:34:03 -0000



> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ananyev, Konstantin
> Sent: Monday, November 26, 2018 9:28 AM
> To: Varghese, Vipin <vipin.varghese@intel.com>; Stephen Hemminger <stephe=
n@networkplumber.org>
> Cc: Pattan, Reshma <reshma.pattan@intel.com>; dev@dpdk.org; thomas@monjal=
on.net; Mcnamara, John <john.mcnamara@intel.com>;
> Byrne, Stephen1 <stephen1.byrne@intel.com>; Glynn, Michael J <michael.j.g=
lynn@intel.com>; Patel, Amol <amol.patel@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v4 5/9] app/procinfo: add support for show=
 tm
>=20
>=20
>=20
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Varghese, Vipin
> > Sent: Monday, November 26, 2018 4:15 AM
> > To: Stephen Hemminger <stephen@networkplumber.org>
> > Cc: Pattan, Reshma <reshma.pattan@intel.com>; dev@dpdk.org; thomas@monj=
alon.net; Mcnamara, John
> <john.mcnamara@intel.com>;
> > Byrne, Stephen1 <stephen1.byrne@intel.com>; Glynn, Michael J <michael.j=
.glynn@intel.com>; Patel, Amol <amol.patel@intel.com>
> > Subject: Re: [dpdk-dev] [PATCH v4 5/9] app/procinfo: add support for sh=
ow tm
> >
> > Hi Stephen,
> >
> > snipped
> >
> > >
> > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > > +			if ((ret) | (!is_leaf))
> > > > > > > > > > > > +
> > > > > > > > > > >
> > > > > > > > > > > Is the operator here should be || ?
> > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > Check is done for 'if either ret is not 0 or if it ret =
is
> > > > > > > > > > 0 but not leaf' we skip leaf details print. If 'ret is =
0 and is leaf'
> > > > > > > > > > we skip continue to print
> > > > > > > > > leaf details.
> > > > > > > > >
> > > > > > > > > IMO, using logical operator over bitwise operator is good
> > > > > > > > > here in if statement
> > > > > > > > .
> > > > > > > > > Like below.?
> > > > > > > > >
> > > > > > > > > If (ret || (is_leaf =3D=3D 0 ))
> > > > > > > >
> > > > > > > > Thanks for the information, if the logic is correct do I ne=
ed
> > > > > > > > to change for v6
> > > > > > > >
> > > > > > >
> > > > > > > OK in v6, but you can wait to hear more comments from others =
if
> > > > > > > any before sending v6 .
> > > > > >
> > > > > > Ok thanks Reshma, but can you tell me how the earlier logic fai=
ls
> > > > > > and runs slow compared to logical or?
> > > > >
> > > > > Not about faster or slower.
> > > >
> > > > Now I see, I was wondering the suggestion was for improvement for
> > > performance.
> > > >
> > > > >
> > > > > Logical operators are commonly used in decision making in C progr=
amming.
> > > > > Bitwise operators are used in C programming to perform bit-level =
operations.
> > > > >
> > > >
> > > > Agreed
> > > >
> > > > > Since , above if condition is for decision making here logical ||
> > > > > operator will fit , so I am suggesting to use that.
> > > > >
> > > >
> > > > But bitwise OR is not wrong right?
> > > >
> > > > > We  don't need to do any bitwise manipulation in if condition to
> > > > > make the decision, so bitwise | operator is not needed
> > > >
> > > > We can correct this in next patch set not v6 if this is only change=
 for 'show tm'
> > >
> > > It could be that compiler might optimize logical into bitwise operati=
on to avoid
> > > cost of conditional branch (if there are no side effects).
> >
> >
> > Checking with 'EXTRA_CFLAGS=3D-g' we get the code generated as
> >
> > """
> >                         if ((ret) | (!is_leaf))
> >    9a512:       8b 85 28 fd ff ff       mov    eax,DWORD PTR [rbp-0x2d8=
]
> >    9a518:       85 c0                   test   eax,eax
> >    9a51a:       0f 94 c0                sete   al
> >    9a51d:       0f b6 c0                movzx  eax,al
> >    9a520:       0b 85 3c fd ff ff       or     eax,DWORD PTR [rbp-0x2c4=
]
> >    9a526:       85 c0                   test   eax,eax
> >    9a528:       75 74                   jne    9a59e <show_tm+0x73e>
> >                                 continue;
> > """
> >
> > Looks like the is_leaf is picked assembly 'test' is done then byte is s=
et to 1 based on flags. This is then or with 'ret'. I think your observatio=
n
> is
> > correct 'compiler is remapping to or'
>=20
> If the operator is '|' what else except of 'OR' you expect it to be remap=
ped?
BTW, I am agree with Reshma it has to be '||' here.