From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id 996313858004 for ; Wed, 1 Dec 2021 12:02:08 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 996313858004 Received: from mail-wm1-f70.google.com (mail-wm1-f70.google.com [209.85.128.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-408-lj9HTMU5Ps2Y3B8IjnA2Cw-1; Wed, 01 Dec 2021 07:02:05 -0500 X-MC-Unique: lj9HTMU5Ps2Y3B8IjnA2Cw-1 Received: by mail-wm1-f70.google.com with SMTP id j193-20020a1c23ca000000b003306ae8bfb7so12134923wmj.7 for ; Wed, 01 Dec 2021 04:02:04 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=qrkfBz1GhgpLyMQZ+aMQwgvEBq62l7Cro/3ppRSwfjY=; b=FecPmaUZC4nah6QTfPr8khij3dNQds2Vt7XVCb20QUdQjRJYjaS7QuF7gCFWCDm6Uy eJwric8g0gwhJ99DL9Otol6IudW4NRWKEU58283m8Y5y3Lv79BlAlUTV9qdu6f/R6X1F bhk31Ds+SAzuh/1DKjaf8JpZSIL46v95f/q7MuCzcHshXMZU6rHhq1ILf+86fhjaOEwY siXR6EllJuAEYK1SU1hvdB5YyF8M0X2lrTQ4uCMmPFhcM6J86g5dT9j33jnUq+tP/LRr FC+/XHLJmgAIxZ8rJ9Oi6YHN4hnZHHQR4JtB76n04/ZqZflRk1oSjBYMSEk6zZ482XI0 vtPA== X-Gm-Message-State: AOAM530iA1Ous/xnhpdQkoCsKPITmZRfFgfT9GHc48Ybffw8qq+Esrdq 84UgVdhcJEbA+/p0WJ5Ph+rJYwBQA1c9/FogXXU70qvw0Hr0I3ItmZxE717l0GUQidPqxDTFDNj Qoij1+B+hbl08XSNRZvE7ZA== X-Received: by 2002:a05:600c:4fc3:: with SMTP id o3mr6251235wmq.147.1638360123630; Wed, 01 Dec 2021 04:02:03 -0800 (PST) X-Google-Smtp-Source: ABdhPJzNNyH2M+iggCL3qeYZnNR5fnBnDC6/70t9hqjukLRgoBE09KK0pkkN0UtepEYhfnu/MFlPNA== X-Received: by 2002:a05:600c:4fc3:: with SMTP id o3mr6251197wmq.147.1638360123376; Wed, 01 Dec 2021 04:02:03 -0800 (PST) Received: from localhost (host86-134-238-138.range86-134.btcentralplus.com. [86.134.238.138]) by smtp.gmail.com with ESMTPSA id ay29sm831594wmb.44.2021.12.01.04.02.02 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 01 Dec 2021 04:02:02 -0800 (PST) Date: Wed, 1 Dec 2021 12:02:01 +0000 From: Andrew Burgess To: John Baldwin Cc: gdb-patches@sourceware.org Subject: Re: [PATCH v2 04/13] linux-nat: Don't enable async mode at the end of linux_nat_target::resume. Message-ID: <20211201120201.GE2662946@redhat.com> References: <20210803185000.22171-1-jhb@FreeBSD.org> <20210803185000.22171-5-jhb@FreeBSD.org> <20211124132457.GD2662946@redhat.com> MIME-Version: 1.0 In-Reply-To: X-Operating-System: Linux/5.8.18-100.fc31.x86_64 (x86_64) X-Uptime: 11:54:45 up 12 days, 53 min, X-Editor: GNU Emacs [ http://www.gnu.org/software/emacs ] X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=us-ascii Content-Disposition: inline X-Spam-Status: No, score=-4.3 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, RCVD_IN_BARRACUDACENTRAL, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=no autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 01 Dec 2021 12:02:10 -0000 * John Baldwin [2021-11-24 08:03:36 -0800]: > On 11/24/21 5:24 AM, Andrew Burgess wrote: > > * John Baldwin [2021-08-03 11:49:51 -0700]: > > > > > Commit 5b6d1e4fa4f ("Multi-target support") enabled async mode in the caller > > > (do_target_resume) after target::resume returns making this call > > > redundant. > > > > The only thing I spotted where this might result in a change of > > behaviour is in record-full.c in record_full_wait_1, there's this > > line: > > > > ops->beneath ()->resume (ptid, step, GDB_SIGNAL_0); > > > > Doesn't this mean that if the record target is sitting on top of a > > Linux target, then me could potentially see a change of behaviour > > here? > > > > I know almost nothing about the record target, so I can't really offer > > any insights into whether the situation about could actually happen or > > not. > > > > Maybe somebody else will have some thoughts, but at a minimum, it is > > probably worth mentioning that there might be a change for that > > case.... Unless you know for sure that there isn't. > > Hmm, I was not aware of that case. This is a change Pedro had suggested > and it didn't show any regressions in the test suite on a Linux/x86-64 > VM, but it very well might be that the test suite doesn't cover > record? I suspect it's not well tested. > > Hmm, looking at record_full_target::resume, it enables async right after > calling the beneath method and it also does so before returning, so I > suspect it's explicit call is safe to elide for the same reason as this > patch. That is, in the code today, linux_nat_target::resume enables async > mode, then would return to remote_full_target::resume which would enable > async mode (without any other statements in between), and then return > to do_target_resume which would enable async mode a third time (again without > any other code in between aside from any RAII destructors when returning > from the target methods). Given, that I think this should be safe, and > I can add a patch to this series to remove the call from > remote_full_target::resume. That's all true, but I wasn't commenting on record_full_target::resume, rather record_full_wait_1, which is called from record_full_base_target::wait. I'd certainly be happy for your change to go in. Having looked again, I don't think there's going to be any problems, looking at other targets, it doesn't seem like having to (re-)enable async mode is something we'd normally expect to do on the ::wait path... I only pointed this out as it seems to be a case that doesn't align with the claims in your commit message... Thanks, Andrew