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 33FFD3858C52 for ; Thu, 21 Sep 2023 14:03:14 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 33FFD3858C52 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1695304993; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=OFIKaHaz5FxXcowodQcYxKH7hwPsck4wbkL3Eqv0i1g=; b=e6CC2QCc8MimAAXkjJJnFa7+GgsZdQffsL4isxiNY4/l/MTFtLPE6xrx6xjW+0gXa31bIU +YFUVTH7i6b32tcQsuGmgb0vZWbKjIMbTr5YanmK31+1ZfQ0OSlisOIyRCUKzg5RCD+kt8 sGkbEQcYsHhKYkn4ekmJZL9tHlUdLIc= Received: from mail-ej1-f70.google.com (mail-ej1-f70.google.com [209.85.218.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-466-YKSjwzd6PWejL3ay1xtRXQ-1; Thu, 21 Sep 2023 10:03:07 -0400 X-MC-Unique: YKSjwzd6PWejL3ay1xtRXQ-1 Received: by mail-ej1-f70.google.com with SMTP id a640c23a62f3a-99bcb13d8ddso76131366b.0 for ; Thu, 21 Sep 2023 07:03:01 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1695304980; x=1695909780; h=mime-version:message-id:date:references:in-reply-to:subject:to:from :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=OFIKaHaz5FxXcowodQcYxKH7hwPsck4wbkL3Eqv0i1g=; b=OEwl37i8LO1E8ZEqRZIuC0dU+A8NG3ltCoXod1SrVx+Duvwx8YVajTrRzW2g6XYkvU SAFLJpE6CaLXqr9lJnOpMxjj0qkumt8CEnoER6Dg9SwblTpoMtbxDqLEDIZ70EeOPiBe f4CEVk+qv5YnWZeeeWVQ+gGF9uKyslaMFGv4rfmLTEcBcII+spCKfl0YOig0/IIo1m0K UoMcQTVgfkGIl8zAzmmHjxPgEIJJYuqdyK7dr7iwPGN0KOjr0hSIsZtfPUOQ4GfFFB7g /goQx/XH0gG+oFiqSS0GCV96qlRwoEHugN6jd1eltkQiuVSwkt6PxIXMN/cRnLoVGjhL QKOg== X-Gm-Message-State: AOJu0YyaMCfho/T2Yv6j9oNcyrpHaAbnZbGuT3/yNLxyClw3KRbcVFSr 5FvMh/WLXMeN0Z2SONnCBY9/WsADAfNOAFOz9eL7YkdWsUqrGvLI7rzoPi5oFtH8fVTeuFr5VJN D/brvSfy7/wnRqG35ZdihqLjaJWcu5w== X-Received: by 2002:a17:906:24e:b0:9ae:4492:df47 with SMTP id 14-20020a170906024e00b009ae4492df47mr4085798ejl.54.1695304980217; Thu, 21 Sep 2023 07:03:00 -0700 (PDT) X-Google-Smtp-Source: AGHT+IGptSaMiaOO89cjuZ76Mw5oSnosX0Ec2ZGT0tHoyNb2aC9SUNR7ukRQR+lprk0Ro/lSS3Jptg== X-Received: by 2002:a17:906:24e:b0:9ae:4492:df47 with SMTP id 14-20020a170906024e00b009ae4492df47mr4085743ejl.54.1695304979127; Thu, 21 Sep 2023 07:02:59 -0700 (PDT) Received: from localhost ([31.111.84.209]) by smtp.gmail.com with ESMTPSA id bg1-20020a170906a04100b009adce1c97ccsm1109268ejb.53.2023.09.21.07.02.58 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 21 Sep 2023 07:02:58 -0700 (PDT) From: Andrew Burgess To: "Gerlicher, Klaus" , Pedro Alves , Simon Marchi , "gdb-patches@sourceware.org" Subject: RE: [PATCH 1/1] gdb, gdbserver: replace PBUFSIZ with a target op In-Reply-To: References: <20230919054511.17998-1-klaus.gerlicher@intel.com> <20230919054511.17998-2-klaus.gerlicher@intel.com> <3d0d3efc-f802-4a3a-a602-dc3e59c99c94@polymtl.ca> <87sf79xanx.fsf@redhat.com> <24534cd7-9c80-19da-5a9c-17962ae46fb3@palves.net> Date: Thu, 21 Sep 2023 15:02:57 +0100 Message-ID: <87ediry67i.fsf@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain X-Spam-Status: No, score=-5.7 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: "Gerlicher, Klaus" writes: > Hi, > > I'm unsure why I'm confusing you about what this does. This is a patch for remote target packet buffer, so for gdbserver. > > From original gdbserver/server.h: > > /* Buffer sizes for transferring memory, registers, etc. The target decides > how big this needs to be but this value must be at least as large as the > largest register set supported by gdbserver. */ > > PBUFSIZ is defined as a constant( #define) and is used for allocating the buffer that gdbserver writes into for communication with GDB. > > This will remain the same size for targets not aware of the > process_stratum_pbufsiz::query_pbuf_size override. I called this a > target op but maybe that's the wrong term and that is the source of > confusion? No target_ops is the correct name (so target ops is fine). For me, I think you should drop all of the changes in the gdb/ tree, leaving just the gdbserver/ changes. GDB should not need to know about PBUFSIZ, and indeed, there's just one reference to it, in a comment, and this should be rewritten to indicate that the maximum size being referenced is the one that the server passed us in the qSupported packet. > Targets can override this to any size they see fit for what register > size or memory transfer sizes they require. It doesn't make sense to > have big allocation for targets that don't need that much but some > newer accelerator/SIMD/GPU device (for a lack of a better term) > targets need a much bigger buffer. Agreed. It would be interesting to see if we could derive the required size from the tdesc as Pedro suggested. Thanks, Andrew > > Thanks > Klaus > > -----Original Message----- > From: Pedro Alves > Sent: Wednesday, September 20, 2023 6:32 PM > To: Andrew Burgess ; Gerlicher, Klaus ; Simon Marchi ; gdb-patches@sourceware.org > Subject: Re: [PATCH 1/1] gdb, gdbserver: replace PBUFSIZ with a target op > > Hi! > > On 2023-09-20 13:59, Andrew Burgess via Gdb-patches wrote: >> "Gerlicher, Klaus via Gdb-patches" writes: >> >>> Hi Simon, >>> >>> Thanks for the quick response. >>> >>> At least the initial buffer size needs to be fixed since now most >>> clients aren't aware of any dynamic behavior here and therefore we >>> need at least something pre-allocated for these clients. >> >> I don't understand your concerns here. For this patch we're only >> talking about the gdbserver client, right? And your patch (rightly) >> doesn't change things on the GDB side. > > The client is the GDB side, the server side is, well, gdbserver. :-) > >> >> GDB already uses a dynamic packet buffer size. > > It is dynamic, but not in the sense that we just append to the buffer with push_back and let the buffer grow unbounded. Instead, GDB tries to guess a sufficient packet size, but if the server tells it explicitly what packet size it supports, then GDB will grow its buffer to that size, no questions asked. > > /* If we increased the packet size, make sure to increase the global > buffer size also. We delay this until after parsing the entire > qSupported packet, because this is the same buffer we were > parsing. */ > if (rs->buf.size () < rs->explicit_packet_size) > rs->buf.resize (rs->explicit_packet_size); > > >> So the only initial >> buffer I think you can be talking about here is the gdbserver buffer, >> which I think could be made dynamic, just as GDB's is. > > I think he was really talking about the GDB side. Or even other clients, like LLDB, etc. > >> >> We could hard-code gdbserver to return some stupidly large number for >> the PacketSize in the qSupported reply, say MAX_INT? Or (MAX_INT / >> 4), you pick, this could be anything really, just something huge. > > I don't think it can, due to the immediate resize mentioned above. > > I don't understand why the patch added a target method on the gdb side. > > Also, do we really need the new target method on the gdbserver side? > We assert that the buffer size is bigger than the tdesc's register size plus a slack, but how about flipping that around and make the buffer size be dependent on the register size? Maybe the packet size decision is done earlier than we know which tdesc we are using, though, that's something to check. > > Pedro Alves > > Intel Deutschland GmbH > Registered Address: Am Campeon 10, 85579 Neubiberg, Germany > Tel: +49 89 99 8853-0, www.intel.de > Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva > Chairperson of the Supervisory Board: Nicole Lau > Registered Office: Munich > Commercial Register: Amtsgericht Muenchen HRB 186928