From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 36588 invoked by alias); 8 May 2017 10:23:28 -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 36575 invoked by uid 89); 8 May 2017 10:23:28 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-6.8 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_2,MIME_BASE64_BLANKS,RP_MATCHES_RCVD,SPF_PASS autolearn=ham version=3.3.2 spammy=supervisory, 1012, Deutschland, deutschland X-HELO: mga03.intel.com Received: from mga03.intel.com (HELO mga03.intel.com) (134.134.136.65) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 08 May 2017 10:23:24 +0000 Received: from orsmga001.jf.intel.com ([10.7.209.18]) by orsmga103.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 08 May 2017 03:23:25 -0700 X-ExtLoop1: 1 Received: from irsmsx106.ger.corp.intel.com ([163.33.3.31]) by orsmga001.jf.intel.com with ESMTP; 08 May 2017 03:23:24 -0700 Received: from irsmsx105.ger.corp.intel.com ([169.254.7.163]) by IRSMSX106.ger.corp.intel.com ([169.254.8.202]) with mapi id 14.03.0319.002; Mon, 8 May 2017 11:23:23 +0100 From: "Wiederhake, Tim" To: Yao Qi CC: "gdb-patches@sourceware.org" , "Metzger, Markus T" Subject: RE: [PATCH v2 00/11] btrace: Turn linked list of function call segments into vector Date: Mon, 08 May 2017 10:23:00 -0000 Message-ID: <9676A094AF46E14E8265E7A3F4CCE9AF3C14C9E7@irsmsx105.ger.corp.intel.com> References: <1494231185-4709-1-git-send-email-tim.wiederhake@intel.com> <86h90v3elw.fsf@gmail.com> In-Reply-To: <86h90v3elw.fsf@gmail.com> dlp-product: dlpe-windows dlp-version: 10.0.102.7 dlp-reaction: no-action Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 X-IsSubscribed: yes X-SW-Source: 2017-05/txt/msg00182.txt.bz2 SGkgWWFvIQ0KDQpUaGUgb25lIHBsYWNlIEkgcmVtZW1iZXJlZCBmcm9tIHRo ZSB0b3Agb2YgbXkgaGVhZCAobmV3X3RocmVhZCBpbiB0aHJlYWQuYykgd2Fz IEMrK2lmaWVkIGluIHRoZSBtZWFudGltZS4gIEkgYmVsaWV2ZSB0aGVyZSB3 YXMgc29tZSBwbGFjZSBpbiByZW1vdGUuYyBhcyB3ZWxsLCBidXQgSSBjYW4n dCBmaW5kIGFueSBldmlkZW5jZSBhdCB0aGUgbW9tZW50Lg0KDQpJIHRyaWVk IGFkZGluZyBhIG5vbi10cml2aWFsIGNvbnN0cnVjdG9yIHRvIHN0cnVjdCBi dHJhY2VfdGhyZWFkX2luZm8gdG8gc2VlIGlmIGFueSBvZiB0aGUgbm9uLXBv ZC1wb2lzb25pbmctcGF0Y2hlcyBraWNrZWQgaW4gLS0gd2hpY2ggZGlkIG5v dCBoYXBwZW4uDQoNClNvIGFsbCBpbiBhbGwsIHRoYW5rcyBmb3Igbm90aWNp bmcgbWUsIEknbGwgcmV3cml0ZSB0aGlzIHNlcmllcyB0byB1c2Ugc3RkOjp2 ZWN0b3IuDQoNClRJbQ0KDQo+IC0tLS0tT3JpZ2luYWwgTWVzc2FnZS0tLS0t DQo+IEZyb206IFlhbyBRaSBbbWFpbHRvOnFpeWFvbHRjQGdtYWlsLmNvbV0N Cj4gU2VudDogTW9uZGF5LCBNYXkgOCwgMjAxNyAxMTowMyBBTQ0KPiBUbzog V2llZGVyaGFrZSwgVGltIDx0aW0ud2llZGVyaGFrZUBpbnRlbC5jb20+DQo+ IENjOiBnZGItcGF0Y2hlc0Bzb3VyY2V3YXJlLm9yZzsgTWV0emdlciwgTWFy a3VzIFQNCj4gPG1hcmt1cy50Lm1ldHpnZXJAaW50ZWwuY29tPg0KPiBTdWJq ZWN0OiBSZTogW1BBVENIIHYyIDAwLzExXSBidHJhY2U6IFR1cm4gbGlua2Vk IGxpc3Qgb2YgZnVuY3Rpb24gY2FsbA0KPiBzZWdtZW50cyBpbnRvIHZlY3Rv cg0KPiANCj4gVGltIFdpZWRlcmhha2UgPHRpbS53aWVkZXJoYWtlQGludGVs LmNvbT4gd3JpdGVzOg0KPiANCj4gPiAgIEluIHNvbWUNCj4gPiBpbnN0YW5j ZXMsIHN0cnVjdCBidHJhY2VfdGhyZWFkX2luZm8gaXMgaW5pdGlhbGl6ZWQg YnkgbWVtc2V0J2luZyBpdCB0bw0KPiAweDAwLA0KPiA+IHNvIHdlIGNhbid0 IHVzZSBzdGQ6OnZlY3RvciAoeWV0KS4NCj4gDQo+IFdoYXQgYXJlIHRoZXNl IGluc3RhbmNlcz8gIEkgY2FuJ3QgZmluZCB0aGVtLg0KPiANCj4gLS0NCj4g WWFvICjpvZDlsKcpDQpJbnRlbCBEZXV0c2NobGFuZCBHbWJIClJlZ2lzdGVy ZWQgQWRkcmVzczogQW0gQ2FtcGVvbiAxMC0xMiwgODU1NzkgTmV1YmliZXJn LCBHZXJtYW55ClRlbDogKzQ5IDg5IDk5IDg4NTMtMCwgd3d3LmludGVsLmRl Ck1hbmFnaW5nIERpcmVjdG9yczogQ2hyaXN0aW4gRWlzZW5zY2htaWQsIENo cmlzdGlhbiBMYW1wcmVjaHRlcgpDaGFpcnBlcnNvbiBvZiB0aGUgU3VwZXJ2 aXNvcnkgQm9hcmQ6IE5pY29sZSBMYXUKUmVnaXN0ZXJlZCBPZmZpY2U6IE11 bmljaApDb21tZXJjaWFsIFJlZ2lzdGVyOiBBbXRzZ2VyaWNodCBNdWVuY2hl biBIUkIgMTg2OTI4Cg== >From gdb-patches-return-138923-listarch-gdb-patches=sources.redhat.com@sourceware.org Mon May 08 11:22:13 2017 Return-Path: Delivered-To: listarch-gdb-patches@sources.redhat.com Received: (qmail 25462 invoked by alias); 8 May 2017 11:22:12 -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 Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 24517 invoked by uid 89); 8 May 2017 11:22:12 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-25.8 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_LAZY_DOMAIN_SECURITY,RCVD_IN_DNSWL_LOW autolearn=ham version=3.3.2 spammy= X-HELO: mx0a-001b2d01.pphosted.com Received: from mx0a-001b2d01.pphosted.com (HELO mx0a-001b2d01.pphosted.com) (148.163.156.1) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 08 May 2017 11:22:09 +0000 Received: from pps.filterd (m0098399.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.20/8.16.0.20) with SMTP id v48BBFab072759 for ; Mon, 8 May 2017 07:22:10 -0400 Received: from e06smtp13.uk.ibm.com (e06smtp13.uk.ibm.com [195.75.94.109]) by mx0a-001b2d01.pphosted.com with ESMTP id 2aajkvy1w3-1 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT) for ; Mon, 08 May 2017 07:22:10 -0400 Received: from localhost by e06smtp13.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 8 May 2017 12:22:07 +0100 Received: from b06cxnps3074.portsmouth.uk.ibm.com (9.149.109.194) by e06smtp13.uk.ibm.com (192.168.101.143) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Mon, 8 May 2017 12:22:05 +0100 Received: from d06av23.portsmouth.uk.ibm.com (d06av23.portsmouth.uk.ibm.com [9.149.105.59]) by b06cxnps3074.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id v48BM5O711272448; Mon, 8 May 2017 11:22:05 GMT Received: from d06av23.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 7C4DEA405D; Mon, 8 May 2017 12:20:47 +0100 (BST) Received: from d06av23.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 42794A405B; Mon, 8 May 2017 12:20:47 +0100 (BST) Received: from ThinkPad (unknown [9.152.212.148]) by d06av23.portsmouth.uk.ibm.com (Postfix) with ESMTP; Mon, 8 May 2017 12:20:47 +0100 (BST) Date: Mon, 08 May 2017 11:22:00 -0000 From: Philipp Rudo To: Omair Javaid Cc: Yao Qi , "gdb-patches@sourceware.org" , Yao Qi , Peter Griffin , Andreas Arnez Subject: Re: [RFC v3 3/8] Add basic Linux kernel support In-Reply-To: References: <20170316165739.88524-1-prudo@linux.vnet.ibm.com> <20170316165739.88524-4-prudo@linux.vnet.ibm.com> <86d1br8q8v.fsf@gmail.com> <20170503173626.15e61941@ThinkPad> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: quoted-printable X-TM-AS-GCONF: 00 x-cbid: 17050811-0012-0000-0000-00000524927E X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 17050811-0013-0000-0000-0000185F50C8 Message-Id: <20170508132204.7a733dc2@ThinkPad> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2017-05-08_07:,, signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 suspectscore=2 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1703280000 definitions=main-1705080063 X-IsSubscribed: yes X-SW-Source: 2017-05/txt/msg00183.txt.bz2 Content-length: 11823 Hi Omair, On Mon, 8 May 2017 04:54:16 +0500 Omair Javaid wrote: > Hi Phillip, >=20 > Thanks for writing back. I hope you are feeling better now. Thanks. It will take some more time for me to get 100% fit again but at lea= st the worst is over ... > I am trying to manage our basic live thread implementation within the > limits you have set out in your patches. >=20 > However I am interested in knowing what are your plans for immediate > future like next couple of weeks. > > If you are not planning on making any particular design changes to the > current version of your patches then probably I will continue working > using your patches as base. My current plan is to finish off the work that has piled up during the two weeks I was sick. After that I will clean up my kernel stack unwinder for s390 so I have that finally gone (it already took way too much time). >From then I don't have a fixed plan. On my bucket list there are some ite= ms without particular order and different impact to the interfaces. They are * Rebase to current master. With all the C++-yfication this will most likely lead to some minor chang= es. * C++-fy the target itself. As Yao mentioned in his mail [1] it would be better to classify struct lk_private to better fit the direction GDB is currently going to. In this process I would also get rid of some cleanups and further adept t= he new C++ features. Overall this will change some (but hopefully not many) interfaces. The biggest change will most likely be from function hooks (in struct lk_private_hooks) to virtual class methods (in lk_privat= e). * Make LK_DECLARE_* macros more flexible. Currently lk_init_private_data aborts once any declared symbol cannot be found. This also makes the whole target unusable if e.g. the kernel is compiled without CONFIG_MODULES as then some symbols needed for module support cannot be found. My idea is to assign symbols to GDB-features e.= g. module support and only turn off those features if a symbol could not be found. * Design a proper CLI (i.e. functions like dmesg etc.). This will be needed if we want others to actually use the feature. Shoul= dn't have any impact on you. * Implement separate thread_lists. Allow every target to manage its own thread_list. Heavy impact for you a= nd a lot work for me... * Implement different target views. Allow the user to switch between different target views (e.g. linux_kernel and core/remote) and thus define the wanted level of abstraction. Even w= orse then the separate thread_lists... Long story short you don't have to divert away from my patches. Even if I start working on the separate thread_lists next it will definitely take qui= te a lot of time to implement. So no matter what you will most likely have a wo= rking patch before me ;) I hope I answered all your questions. Philipp [1] https://sourceware.org/ml/gdb-patches/2017-05/msg00004.html > Otherwise if you plan to make any further changes like going for a > separate thread list implementation for all layers of targets then i > can also divert away from your patches for a while untill next update > is posted. >=20 > I am already diverting away from Peter's original implementation > because of some basic limitations pointed out during previous reviews. > I dont have reliable solution right now but trying to find one lets > see if i can manage to upgrade this current hack for live threads as > well. >=20 > -- > Omair. >=20 > On 3 May 2017 at 20:36, Philipp Rudo wrote:r > > Hi Yao, > > > > > > On Tue, 02 May 2017 12:14:40 +0100 > > Yao Qi wrote: > >=20=20 > >> Philipp Rudo writes: > >> > >> Hi Philipp, > >>=20=20 > >> > +/* Initialize architecture independent private data. Must be called > >> > + _after_ symbol tables were initialized. */ > >> > + > >> > +static void > >> > +lk_init_private_data () > >> > +{ > >> > + if (LK_PRIVATE->data !=3D NULL) > >> > + htab_empty (LK_PRIVATE->data); > >> > + > >> > + LK_DECLARE_FIELD (task_struct, tasks); > >> > + LK_DECLARE_FIELD (task_struct, pid); > >> > + LK_DECLARE_FIELD (task_struct, tgid); > >> > + LK_DECLARE_FIELD (task_struct, thread_group); > >> > + LK_DECLARE_FIELD (task_struct, comm); > >> > + LK_DECLARE_FIELD (task_struct, thread); > >> > + > >> > + LK_DECLARE_FIELD (list_head, next); > >> > + LK_DECLARE_FIELD (list_head, prev); > >> > + > >> > + LK_DECLARE_FIELD (rq, curr); > >> > + > >> > + LK_DECLARE_FIELD (cpumask, bits); > >> > + > >> > + LK_DECLARE_ADDR (init_task); > >> > + LK_DECLARE_ADDR (runqueues); > >> > + LK_DECLARE_ADDR (__per_cpu_offset); > >> > + LK_DECLARE_ADDR (init_mm); > >> > + > >> > + LK_DECLARE_ADDR_ALIAS (__cpu_online_mask, cpu_online_mask); = /* > >> > linux 4.5+ */ > >> > + LK_DECLARE_ADDR_ALIAS (cpu_online_bits, cpu_online_mask); = /* > >> > linux -4.4 */ > >> > + if (LK_ADDR (cpu_online_mask) =3D=3D -1) > >> > + error (_("Could not find address cpu_online_mask. Aborting.")); > >> > +} > >> > +=20=20 > >>=20=20 > >> > + > >> > +/* Initialize linux kernel target. */ > >> > + > >> > +static void > >> > +init_linux_kernel_ops (void) > >> > +{ > >> > + struct target_ops *t; > >> > + > >> > + if (linux_kernel_ops !=3D NULL) > >> > + return; > >> > + > >> > + t =3D XCNEW (struct target_ops); > >> > + t->to_shortname =3D "linux-kernel"; > >> > + t->to_longname =3D "linux kernel support"; > >> > + t->to_doc =3D "Adds support to debug the Linux kernel"; > >> > + > >> > + /* set t->to_data =3D struct lk_private in lk_init_private. */ > >> > + > >> > + t->to_open =3D lk_open; > >> > + t->to_close =3D lk_close; > >> > + t->to_detach =3D lk_detach; > >> > + t->to_fetch_registers =3D lk_fetch_registers; > >> > + t->to_update_thread_list =3D lk_update_thread_list; > >> > + t->to_pid_to_str =3D lk_pid_to_str; > >> > + t->to_thread_name =3D lk_thread_name; > >> > + > >> > + t->to_stratum =3D thread_stratum; > >> > + t->to_magic =3D OPS_MAGIC; > >> > + > >> > + linux_kernel_ops =3D t; > >> > + > >> > + add_target (t); > >> > +} > >> > + > >> > +/* Provide a prototype to silence -Wmissing-prototypes. */ > >> > +extern initialize_file_ftype _initialize_linux_kernel; > >> > + > >> > +void > >> > +_initialize_linux_kernel (void) > >> > +{ > >> > + init_linux_kernel_ops (); > >> > + > >> > + observer_attach_new_objfile (lk_observer_new_objfile); > >> > + observer_attach_inferior_created (lk_observer_inferior_created); > >> > +} > >> > diff --git a/gdb/lk-low.h b/gdb/lk-low.h > >> > new file mode 100644 > >> > index 0000000..292ef97 > >> > --- /dev/null > >> > +++ b/gdb/lk-low.h > >> > @@ -0,0 +1,310 @@ > >> > +/* Basic Linux kernel support, architecture independent. > >> > + > >> > + Copyright (C) 2016 Free Software Foundation, Inc. > >> > + > >> > + This file is part of GDB. > >> > + > >> > + This program is free software; you can redistribute it and/or mo= dify > >> > + it under the terms of the GNU General Public License as publishe= d by > >> > + the Free Software Foundation; either version 3 of the License, or > >> > + (at your option) any later version. > >> > + > >> > + This program is distributed in the hope that it will be useful, > >> > + but WITHOUT ANY WARRANTY; without even the implied warranty of > >> > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > >> > + GNU General Public License for more details. > >> > + > >> > + You should have received a copy of the GNU General Public License > >> > + along with this program. If not, see . > >> > */ + > >> > +#ifndef __LK_LOW_H__ > >> > +#define __LK_LOW_H__ > >> > + > >> > +#include "target.h" > >> > + > >> > +extern struct target_ops *linux_kernel_ops; > >> > + > >> > +/* Copy constants defined in Linux kernel. */ > >> > +#define LK_TASK_COMM_LEN 16 > >> > +#define LK_BITS_PER_BYTE 8 > >> > + > >> > +/* Definitions used in linux kernel target. */ > >> > +#define LK_CPU_INVAL -1U > >> > + > >> > +/* Private data structs for this target. */ > >> > +/* Forward declarations. */ > >> > +struct lk_private_hooks; > >> > +struct lk_ptid_map; > >> > + > >> > +/* Short hand access to private data. */ > >> > +#define LK_PRIVATE ((struct lk_private *) linux_kernel_ops->to_data) > >> > +#define LK_HOOK (LK_PRIVATE->hooks) > >> > + > >> > +struct lk_private=20=20 > >> > >> "private" here is a little confusing. How about rename it to > >> "linux_kernel"?=20=20 > > > > I called it "private" as it is the targets private data stored in its > > to_data hook. But I don't mind renaming it. Especially ... > >=20=20 > >> > +{ > >> > + /* Hashtab for needed addresses, structs and fields. */ > >> > + htab_t data; > >> > + > >> > + /* Linked list to map between cpu number and original ptid from t= arget > >> > + beneath. */ > >> > + struct lk_ptid_map *old_ptid; > >> > + > >> > + /* Hooks for architecture dependent functions. */ > >> > + struct lk_private_hooks *hooks; > >> > +}; > >> > +=20=20 > >> > >> Secondly, can we change it to a class and function pointers in > >> lk_private_hooks become virtual functions. gdbarch_lk_init_private > >> returns a pointer to an instance of sub-class of "linux_kernel". > >> > >> lk_init_private_data can be put the constructor of base class, to add > >> entries to "data", and sub-class (in each gdbarch) can add their own > >> specific stuff.=20=20 > > > > ... when classifying the struct, which already is on my long ToDo-list. > > This struct is a left over from when I started working on the project > > shortly before gdb-7.12 was released. I didn't think that the > > C++-yfication would kick off that fast and started with plain C ... > > > > Thanks > > Philipp > >=20=20 > >> > + > >> > +/* Functions to initialize private data. Do not use directly, use = the > >> > + macros below instead. */ > >> > + > >> > +extern struct lk_private_data *lk_init_addr (const char *name, > >> > + const char *alias, int > >> > silent); +extern struct lk_private_data *lk_init_struct (const char > >> > *name, > >> > + const char *alias, int > >> > silent);=20=20 > >>=20=20 > >> > + > >> > +/* Definitions for architecture dependent hooks. */ > >> > +/* Hook to read registers from the target and supply their content > >> > + to the regcache. */ > >> > +typedef void (*lk_hook_get_registers) (CORE_ADDR task, > >> > + struct target_ops *target, > >> > + struct regcache *regcache, > >> > + int regnum); > >> > + > >> > +/* Hook to return the per_cpu_offset of cpu CPU. Only architectures > >> > that > >> > + do not use the __per_cpu_offset array to determin the offset hav= e to > >> > + supply this hook. */ > >> > +typedef CORE_ADDR (*lk_hook_get_percpu_offset) (unsigned int cpu); > >> > + > >> > +/* Hook to map a running task to a logical CPU. Required if the ta= rget > >> > + beneath uses a different PID as struct rq. */ > >> > +typedef unsigned int (*lk_hook_map_running_task_to_cpu) (struct > >> > thread_info *ti); + > >> > +struct lk_private_hooks > >> > +{ > >> > + /* required */ > >> > + lk_hook_get_registers get_registers; > >> > + > >> > + /* optional, required if __per_cpu_offset array is not used to > >> > determine > >> > + offset. */ > >> > + lk_hook_get_percpu_offset get_percpu_offset; > >> > + > >> > + /* optional, required if the target beneath uses a different PID = as > >> > struct > >> > + rq. */ > >> > + lk_hook_map_running_task_to_cpu map_running_task_to_cpu; > >> > +};=20=20 > >>=20=20 > >=20=20 >=20