From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 32284 invoked by alias); 1 Jul 2015 08:39:10 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 32270 invoked by uid 89); 1 Jul 2015 08:39:09 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.9 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mga14.intel.com Received: from mga14.intel.com (HELO mga14.intel.com) (192.55.52.115) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 01 Jul 2015 08:39:08 +0000 Received: from orsmga002.jf.intel.com ([10.7.209.21]) by fmsmga103.fm.intel.com with ESMTP; 01 Jul 2015 01:39:06 -0700 X-ExtLoop1: 1 Received: from irsmsx109.ger.corp.intel.com ([163.33.3.23]) by orsmga002.jf.intel.com with ESMTP; 01 Jul 2015 01:39:05 -0700 Received: from irsmsx104.ger.corp.intel.com ([169.254.5.171]) by IRSMSX109.ger.corp.intel.com ([169.254.13.200]) with mapi id 14.03.0224.002; Wed, 1 Jul 2015 09:39:05 +0100 From: "Metzger, Markus T" To: Pedro Alves CC: "gdb-patches@sourceware.org" Subject: RE: [PATCH 2/5] btrace: support Intel(R) Processor Trace Date: Wed, 01 Jul 2015 08:39:00 -0000 Message-ID: References: <1435047418-21611-1-git-send-email-markus.t.metzger@intel.com> <1435047418-21611-3-git-send-email-markus.t.metzger@intel.com> <55929205.6080907@redhat.com> <5592B100.803@redhat.com> In-Reply-To: <5592B100.803@redhat.com> Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2015-07/txt/msg00001.txt.bz2 > -----Original Message----- > From: Pedro Alves [mailto:palves@redhat.com] > Sent: Tuesday, June 30, 2015 5:09 PM > To: Metzger, Markus T > Cc: gdb-patches@sourceware.org > Subject: Re: [PATCH 2/5] btrace: support Intel(R) Processor Trace >=20 > On 06/30/2015 03:54 PM, Metzger, Markus T wrote: >=20 > >>> + decoder =3D pt_insn_alloc_decoder (&config); > >>> + if (decoder =3D=3D NULL) > >>> + error (_("Failed to allocate the Intel(R) Processor Trace decode= r.")); > >>> + > >>> + TRY > >>> + { > >>> + struct pt_image *image; > >>> + > >>> + image =3D pt_insn_get_image(decoder); > >>> + if (image =3D=3D NULL) > >>> + error (_("Failed to configure the Intel(R) Processor Trace decoder.= ")); > >>> + > >>> + errcode =3D pt_image_set_callback(image, > >> btrace_pt_readmem_callback, NULL); > >>> + if (errcode < 0) > >>> + error (_("Failed to configure the Intel(R) Processor Trace decoder:= " > >>> + "%s."), pt_errstr (pt_errcode (errcode))); > >>> + > >>> + ftrace_add_pt (decoder, &btinfo->begin, &btinfo->end, &level, > >>> + &btinfo->ngaps); > >>> + } > >>> + CATCH (error, RETURN_MASK_ALL) > >>> + { > >>> + /* Indicate a gap in the trace if we quit trace processing. E= rrors were > >>> + already logged before. */ > >> > >> What does this "already logged before" mean? AFAICS, the errors thrown > >> in the TRY branch are just swallowed here. Did you mean to rethrow > them? > >> Otherwise I'm not seeing the point in throwing them in the first place. > > > > This means that decode errors are already represented as gaps in the tr= ace. > > When the trace is printed, the error at a trace gap is printed. > > > > This code is now handling a user interrupt, which is also represented > > as a gap at the very end of the trace. > > > > This reference to decode errors is maybe more confusing than helpful. > > I'll remove it. >=20 > I still don't get why throw the errors in the TRY branch: >=20 > if (image =3D=3D NULL) > error (_("Failed to configure the Intel(R) Processor Trace decoder.")); >=20 > errcode =3D pt_image_set_callback(image, btrace_pt_readmem_callback, > NULL); > if (errcode < 0) > error (_("Failed to configure the Intel(R) Processor Trace decoder: " > "%s."), pt_errstr (pt_errcode (errcode))); >=20 > ... if they're just dropped on the catch brock. Shouldn't those > be rethrown? The CATCH block you had does not do that. And it > neither rethrows the ctrl-c that generates the RETURN_QUIT: >=20 > + CATCH (error, RETURN_MASK_ALL) > + { > + /* Indicate a gap in the trace if we quit trace processing. Error= s were > + already logged before. */ > + if (error.reason =3D=3D RETURN_QUIT && btinfo->end !=3D NULL) > + { > + btinfo->end =3D ftrace_new_gap (btinfo->end, BDE_PT_USER_QUIT); > + btinfo->ngaps++; > + } > + } >=20 > So shouldn't that be: >=20 > CATCH (error, RETURN_MASK_ALL) > { > /* Indicate a gap in the trace if we quit trace processing. Errors= were > already logged before. */ > if (error.reason =3D=3D RETURN_QUIT && btinfo->end !=3D NULL) > { > btinfo->end =3D ftrace_new_gap (btinfo->end, BDE_PT_USER_QUIT); > btinfo->ngaps++; > } >=20 > + throw_exception (error); > } >=20 > ? You're right. We should rethrow. Thanks. Regards, Markus. Intel GmbH Dornacher Strasse 1 85622 Feldkirchen/Muenchen, Deutschland Sitz der Gesellschaft: Feldkirchen bei Muenchen Geschaeftsfuehrer: Christian Lamprechter, Hannes Schwaderer, Douglas Lusk Registergericht: Muenchen HRB 47456 Ust.-IdNr./VAT Registration No.: DE129385895 Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052