From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id INhgKcFPMmeRdS4AWB0awg (envelope-from ) for ; Mon, 11 Nov 2024 13:41:05 -0500 Received: by simark.ca (Postfix, from userid 112) id 872AB1E110; Mon, 11 Nov 2024 13:41:05 -0500 (EST) X-Spam-Checker-Version: SpamAssassin 4.0.0 (2022-12-13) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-5.3 required=5.0 tests=ARC_SIGNED,ARC_VALID,BAYES_00, MAILING_LIST_MULTI,RCVD_IN_DNSWL_MED autolearn=ham autolearn_force=no version=4.0.0 Received: from server2.sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 85E4F1E0C1 for ; Mon, 11 Nov 2024 13:41:02 -0500 (EST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 222FE3858C5F for ; Mon, 11 Nov 2024 18:41:02 +0000 (GMT) Received: from mail-wm1-f48.google.com (mail-wm1-f48.google.com [209.85.128.48]) by sourceware.org (Postfix) with ESMTPS id 01AA83858D35 for ; Mon, 11 Nov 2024 18:40:40 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 01AA83858D35 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=palves.net Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 01AA83858D35 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=209.85.128.48 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1731350442; cv=none; b=KHn43KCw4buQpoGCPAepD3LUKFGboSF2fRUt4jadHanKFHfSZxns4d5csiBVdfSB53p1BiFE37vtH1j684ltsV0QZJfNz2y+e5o9BcXzDIZabF5exSApnF2WAg0L6KRvmQ+3A6ZF1sXUOP3foOxJ1OjLQQJk/TUqyOXiCnY39g8= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1731350442; c=relaxed/simple; bh=xciZabEcac7D3dS7+B2/8nvuS3a/Ov6/AuycaaNhalU=; h=Message-ID:Date:MIME-Version:Subject:To:From; b=oEpkSk5K++ydhPaJ43j77NnK5ZvO+iwZTOeLPG8qf7DUhBieeWYMRnXgvnlajGvKeaQ8A0tNxv75yJFc0q4zE78B3H2U+Ur5CSdzbHP/YizYyj7TxKGQIXrOYNGeYY5ifk6G6jhsQpcpd/WQNP0xpumQSfs2+UUDbtkGXILxHAc= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-wm1-f48.google.com with SMTP id 5b1f17b1804b1-431481433bdso42307945e9.3 for ; Mon, 11 Nov 2024 10:40:39 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1731350439; x=1731955239; h=content-transfer-encoding:in-reply-to:content-language:from :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=WWFY3FBIADS52R219F1KyXOoOg95/ZmbobAkcTuxXVw=; b=KeAbUh58kivf8Ov4KvH2y5363uRBV9igjeRezMBvlmkmhC2ddtLxnmOuI6KBeSPIyj izuHnDhajiDeevQYdRkgrFkSU5L/uDMKVRz2bcOMHWcZ9ABYhdvUhlJYD7TiPZf2H0lN RIrRR7i/5VF1+d4bG9gG0PdhAHRz9j/qgV7PxNXtXVSoxShMebgKmOrpzYRQPx02fQxk u2jTyEwCj79+9LFVN5na3KBCRAQZIbjo32zQn9YvF8LzR6RZbk5Jv2YSbZ8eC/AJKzXX sxBHNNtFv1t7/QRqfFJPYTDWp4/mA8AC1yInxsAj4DfS72BEYJmf7gTxTI7tl6OipMAf kMHA== X-Gm-Message-State: AOJu0YzMILjrYAxqKb9cLH4Ikpff0rvFKkSm1knN6p/SruEO/ZacHdqq WhBcmRubYov5uTdjt+kUN1WM+VdB+aNIpvWYqFpIk+fJ1v9aGWV91nyfYJ7w X-Google-Smtp-Source: AGHT+IFDj2WvkktpPntmOHuV7RZGKN+oJy96MgazvvneZ3Osbs3+p6rR7tVHLVN6F9Q7NtDJdWAtlw== X-Received: by 2002:a05:600c:1c82:b0:431:5632:448b with SMTP id 5b1f17b1804b1-432b751b70amr122515635e9.25.1731350438722; Mon, 11 Nov 2024 10:40:38 -0800 (PST) Received: from ?IPV6:2001:8a0:f912:ec00:5321:9692:323b:1a87? ([2001:8a0:f912:ec00:5321:9692:323b:1a87]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-432aa709f3fsm225957055e9.32.2024.11.11.10.40.38 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 11 Nov 2024 10:40:38 -0800 (PST) Message-ID: <334e39d6-bdb3-4aad-8524-55b99958d6fc@palves.net> Date: Mon, 11 Nov 2024 18:40:33 +0000 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v4 1/4] Make linux checkpoints work with multiple inferiors To: Kevin Buettner Cc: gdb-patches@sourceware.org References: <20240626020148.68109-1-kevinb@redhat.com> <20240626020148.68109-2-kevinb@redhat.com> <20241030200434.46738394@f40-zbm-amd> From: Pedro Alves Content-Language: en-US In-Reply-To: <20241030200434.46738394@f40-zbm-amd> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces~public-inbox=simark.ca@sourceware.org Hi Kevin, On 2024-10-31 03:04, Kevin Buettner wrote: > On Wed, 23 Oct 2024 21:10:53 +0100 > Pedro Alves wrote: >> Now that I look at the code, I see that the predicate used to >> determine whether to show the inferior number is checking if >> there are multiple inferiors with checkpoints, so it seems >> intentional. But FYI, coming at this with a lot of context >> from previous discussion swapped out, I found it confusing. > > As you say, it was intentional, but it's easy enough to change it > to always print fully-qualified ids when there are multiple inferiors. > I'll make that change (and will update the test case). > Thanks! >> On the "R" state, thinking some more (since our last discussion), >> I wonder if we really need it. In "info threads", we show that >> the thread is running in the "frame" column, like: >> >> (gdb) info threads >> Id Target Id Frame >> * 1 Thread 0x7ffff7f8e740 (LWP 439463) "infloop" (running) >> ^^^^^^^^^ >> >> Maybe we should just reuse the frame-printing code from "info threads". > > The current linux-fork.c code prints . In the v1 series, I > had used '*' and '+' as the first character to indicate the active > forks, with '*' also indicating the active inferior. You recommended > that I instead use a state character "A" and dispense with the '+' > indicators. Well, since we have one state character, why not another, > hence "R". (It makes the output more compact.) If we change "R" to > "(running)", the state character "A" seems a little odd to me; why not > say "(active)" or, instead, use a compact method of indicating active > forks as I did in my v1 patch. The '*' and '+' in the same column compact form was really confusing to me when I tried using this in an earlier version. I really think it makes sense to keep it in a separate column. We shouldn't put "(active)" in the "frame" column (I mean, the column that is equivalent to the "Frame" column in "info threads"), as an active fork has a real frame to print, of course. Putting "(active)" where "A" is being put looks like too much wasted space to me: * 1.0 (active) process 439463 at 0x7ffff7ce578a, file ../sysdeps/unix/sysv/linux/clock_nanosleep.c, line 78 1.1 process 439827 at 0x7ffff7ce578a, file ../sysdeps/unix/sysv/linux/clock_nanosleep.c, line 78 2.0 process 439463 at 0x7ffff7ce578a, file ../sysdeps/unix/sysv/linux/clock_nanosleep.c, line 78 2.1 (active) process 439827 at 0x7ffff7ce578a, file ../sysdeps/unix/sysv/linux/clock_nanosleep.c, line 78 In downstream rocgdb (for AMDGPU), we have a new "info lanes" command with a "State" column, and one of the states is A (for active), which is I guess why "A" and state column looked so obvious to me. E.g.: (gdb) info lanes Id State Target Id Frame * 0 A AMDGPU Lane 1:1:1:1/0 (0,0,0)[0,0,0] bit_extract_kernel (C_d=0x7fffe3a00000, A_d=0x7fffe8400000, N=1000000) at /home/pedro/rocm/bit_extract/bit_extract.cpp:62 1 A AMDGPU Lane 1:1:1:1/1 (0,0,0)[1,0,0] bit_extract_kernel (C_d=0x7fffe3a00000, A_d=0x7fffe8400000, N=1000000) at /home/pedro/rocm/bit_extract/bit_extract.cpp:62 ... > > But perhaps this is needless bike-shedding. If you really want it to > be "(running)", I'll change it. Well, you have to change _something_. If the fork is running, then there is no frame to print, so if you don't print (running), what else could you print, other than leaving it blank, like: * 1.0 A process 439463 1.1 process 439827 at 0x7ffff7ce578a, file ../sysdeps/unix/sysv/linux/clock_nanosleep.c, line 78 I now think that: * 1.0 A process 439463 (running) 1.1 process 439827 at 0x7ffff7ce578a, file ../sysdeps/unix/sysv/linux/clock_nanosleep.c, line 78 ... is better than: * 1.0 AR process 439463 1.1 process 439827 at 0x7ffff7ce578a, file ../sysdeps/unix/sysv/linux/clock_nanosleep.c, line 78 ... because: #1 - it's what you get with "info threads" as well, and consistency is good. #2 - running vs not-running is not really a property of the fork wrt to checkpointing, unlike active/not-active. > But do give some thought about whether the state character "A" still makes sense. Yeah, if there aren't other obvious states, we could also consider other representations. Like, make it a yes/no column? This only works nicely if we add a table header row, but we should do that anyhow. E.g.: Id Active Target Id Frame * 1.0 y process 439463 at 0x7ffff7ce578a, file ../sysdeps/unix/sysv/linux/clock_nanosleep.c, line 78 1.1 n process 439827 at 0x7ffff7ce578a, file ../sysdeps/unix/sysv/linux/clock_nanosleep.c, line 78 2.0 n process 484476 at 0x7ffff7ce578a, file ../sysdeps/unix/sysv/linux/clock_nanosleep.c, line 78 2.1 y process 454312 at 0x7ffff7ce578a, file ../sysdeps/unix/sysv/linux/clock_nanosleep.c, line 78 This is probably the most gdb-natural output? It's also useful/interesting to consider how all these different options would be exposed to MI. (Why that was never done, I don't know. I find it strange, especially since we know of frontends like ARM DDT which use checkpoints. But I think they relied on annotations instead of MI. But I hope they moved along by now.) Representing the "active" state in MI in the same "column" as the selected thread more obviously doesn't work there, as the "selected" item is not normally a property of a row. E.g., for threads listing, we get a current-thread-id="1" attribute, separate from the thread list: (gdb) interpreter-exec mi "-thread-info" ^done,threads=[{id="1",target-id="Thread 0xf7fbf600 (LWP 255870)",name="step",frame={level="0",addr="0x08048385",func="main",args=[],file="step.c",fullname="/home/pedro/pedro/gdb/tests/step.c",line="27",arch="i386"},state="stopped",core="2"}],current-thread-id="1" An MI "-checkpoints-info" command would most probably want to do the same, and add an active=y/n attribute. Pedro Alves