From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 106790 invoked by alias); 3 May 2017 14:13: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 Received: (qmail 106736 invoked by uid 89); 3 May 2017 14:13:09 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.4 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_NONE,RCVD_IN_SORBS_SPAM,SPF_PASS autolearn=no version=3.3.2 spammy=exploited, client's, Hx-languages-length:4158, kindly X-HELO: mail-wm0-f41.google.com Received: from mail-wm0-f41.google.com (HELO mail-wm0-f41.google.com) (74.125.82.41) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 03 May 2017 14:13:06 +0000 Received: by mail-wm0-f41.google.com with SMTP id w64so148419988wma.0 for ; Wed, 03 May 2017 07:13:08 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=9/fbx1fy5pkka7U1kxwRYsX6E4jU98Q3+bu5R+Wd3+s=; b=OKhaVa+vY0LkD0pyF+U4tK2bGV4t/tvK1qVRWfesGKvrlyl7LlwNwrYkks6DslbfSr ehpEtNAt6P8qfgd0lOlmLBStTMWuaV6AOvdzc96ZDONOPUbRzYx6Cw2XPlBw7Mj3Czdi a1/q2TuLTqPw3+2TJ2jL1K8MZJSF6PDlpGAv6rpLZX70CI3nqNhjm2/SJdVaIti0lXOW RWJWE/SwDPycwGXPVIUOtLQ3iYmXqWS8F2nBEL7s+g0nwhe0ecaqGD9KxwgiDyw5XGiA cbk2NZrUz/pD0zIKbrseS6KoMRYCr7ziT6qZ5KHA2kU6Yb+RRulT/1KTa5PKqPKos0Ox f7xw== X-Gm-Message-State: AN3rC/7bXXd85V3GPTA5+yEmJ08fMcJ+mDpoFK6+7jPMDZuqqbrqxiRD gcniLDXFX1S322p+0bcc4LztMvnjZgjk X-Received: by 10.25.163.134 with SMTP id m128mr11866439lfe.1.1493820786621; Wed, 03 May 2017 07:13:06 -0700 (PDT) MIME-Version: 1.0 Received: by 10.25.233.8 with HTTP; Wed, 3 May 2017 07:12:36 -0700 (PDT) In-Reply-To: References: <20170316165739.88524-1-prudo@linux.vnet.ibm.com> <20170316165739.88524-4-prudo@linux.vnet.ibm.com> From: Omair Javaid Date: Wed, 03 May 2017 14:13:00 -0000 Message-ID: Subject: Re: [RFC v3 3/8] Add basic Linux kernel support To: Andreas Arnez Cc: Philipp Rudo , "gdb-patches@sourceware.org" , Yao Qi , Peter Griffin , Lee Jones , Russell Wayman Content-Type: text/plain; charset=UTF-8 X-IsSubscribed: yes X-SW-Source: 2017-05/txt/msg00054.txt.bz2 On 24 April 2017 at 20:24, Andreas Arnez wrote: > On Thu, Apr 20 2017, Omair Javaid wrote: > >> Hi Philipp and Andreas, >> >> I have some further comments on this patch specifically about copying >> task_struct->pid into ptid->lwp and using task_struct address as tid. >> >> I see that we are overriding lwp, tid which any target beneath might >> be using differently. >> >> So suggestion about storing task_struct->pid or task_struct address is >> to use private_thread_info in binutils-gdb/gdb/gdbthread.h for this >> information. > > The current version of the patch series is mainly focused on dump > targets. Remote targets require some additional changes. We've > discussed the use of private_thread_info before, and the last I've seen > is that it is not suitable either, because remote.c uses it already: > > https://sourceware.org/ml/gdb-patches/2017-02/msg00543.html > > In my view, the private_thread_info field really is a hack, and we are > now facing its limits. It provides some space for a single thread layer > to store information into, but not for multiple such layers. In the > case of the Linux kernel we at least have two different thread layers: > the CPU layer (each "thread" is a CPU), and the kernel task layer on top > of that. > > I think we need to allow a target to maintain its *own* thread list. > The CPU "thread list" would be maintained by the target beneath > (remote/dump), and the kernel task list would be maintained by the LK > target. The ptid namespaces could be completely separate. Hi Philip and Andreas, Further more on this topic, remote stub assigns a common pid to all CPU threads and uses LWP as CPU number. While tid is left zero. I think we ll have to rework the old to new ptid mapping mechanism a little bit in order to adjust all types of targets beneath. In your implementation of lk_init_ptid_map() you are testing pid from task_struct against lwp of set by target target beneath. In case of remote this will never be equal to pid as it is marked as the cpu number. Also in your implementation of lk_update_running_tasks lwp is being updated with pid read from task_struct and tid is the task struct address. We are doing this not only for linux thread layer tasks but also for CPU tasks in lk_update_running_tasks. This causes some sync issues while on live targets as every time we halt a new thread is reported by remote. I think linux thread layer should only update tasks it has created itself i-e tasks created by parsing task_struct. We can devise a mechanism to map CPU tasks to curr task_struct and leave CPU tasks as they were created by target beneath with same ptids. Whats your take on this? > >> I also have reservation about use of old_ptid naming in struct >> lk_private and struct lk_ptid_map. >> >> old_ptid naming is a little confusing kindly choose a distinguishable >> name for old_ptid varibles in both lk_private and lk_ptid_map. >> >> Further Here's an implementation of bitmap_weight function from linux >> kernel. Kindly see if your implementation can be improved and moved to >> a generic area in gdb. >> >> 10 int __bitmap_weight(const unsigned long *bitmap, int bits) >> 11 { >> 12 int k, w = 0, lim = bits/BITS_PER_LONG; >> 13 >> 14 for (k = 0; k < lim; k++) >> 15 w += hweight_long(bitmap[k]); >> 16 >> 17 if (bits % BITS_PER_LONG) >> 18 w += hweight_long(bitmap[k] & BITMAP_LAST_WORD_MASK(bits)); >> 19 >> 20 return w; >> 21 } > > The __bitmap_weight function is specific to Linux, so I'm not sure we > want to move it to a generic area. For big-endian targets the function > depends on the width of Linux' "unsigned long" type, because > BITMAP_LAST_WORD_MASK builds a mask for the *least significant* bits > instead of the *lowest-addressed* ones. > > It's probably true that the performance of lk_bitmap_hweight could be > improved. For instance, with some care a function like popcount_hwi() > in GCC's hwint.h could be exploited, even if the target's word width and > byte order may not match the GDB client's. This would not make the > function simpler, though. > > -- > Andreas >