From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.7 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, HEADER_FROM_DIFFERENT_DOMAINS,HK_RANDOM_FROM,HTML_MESSAGE,INCLUDES_PATCH, MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 58011C3A5A6 for ; Thu, 19 Sep 2019 19:37:47 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 107A821907 for ; Thu, 19 Sep 2019 19:37:47 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=bushare.onmicrosoft.com header.i=@bushare.onmicrosoft.com header.b="5VukyyGy" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 107A821907 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=bu.edu Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Received: from localhost ([::1]:48182 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1iB2FO-00057N-5l for qemu-devel@archiver.kernel.org; Thu, 19 Sep 2019 15:37:46 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:42769) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1iB2EL-0004hN-QI for qemu-devel@nongnu.org; Thu, 19 Sep 2019 15:36:45 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1iB2EJ-0003PD-8R for qemu-devel@nongnu.org; Thu, 19 Sep 2019 15:36:41 -0400 Received: from mail-eopbgr760133.outbound.protection.outlook.com ([40.107.76.133]:25597 helo=NAM02-CY1-obe.outbound.protection.outlook.com) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1iB2EG-0003NX-21 for qemu-devel@nongnu.org; Thu, 19 Sep 2019 15:36:37 -0400 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=O1LrnVoNKDjHETXxMek0lScBdEDmmK1iQ8lFwP94iJ1KSnARffsnprSNXP0Qpo6L5BlBJbW+EniLFbifmdOtwNE9UQ27KzugbzEZXdzCM3XkRwx4RMIqC28WFh0HWONSY0QGgDzAQRadMMj/j4jUSRjj6SposdJiRupPSPLp5z59p30drL0MlzXceLcfc5Xygcez/XzjyTudBphB9eu/p6hNBI1RRixo92xZgux4jBJYhPBMdAy6e7E49/E5W/q5iR/sAWLPwjUzjnJniP5G6LelBlCY/o/2OBbxT4VHbhi+b918ljkkNMi65MYTSGkxm331KaK9pNLvotp2g7Yw0g== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=YO0H6UD2UfwXcv/vjRCKs+1BrCh5JLfdJi40zaZuKmo=; b=U1ZDhLehWZJrc3lJ9i0O8tvHmj7MhD7YNcrReCt+3vtxzOOGkiawMMiR03omSf8K1S+8v+tDE1XMfyBf5sA1QV4Ybr3QCRP2MKccJTspiLJfubI3NVR+huVDOgx0nyVEX39zx4OFANT4FThkGX7TWbhAUSdJoCO2Ad2fxlCgapu0dFMAtcFDCfY4fXifCagYlinQ/2sW4kt1eUBWF01Bn7vLj1SWFvDFhdhD8no5/KYKzamtR3AQ4Fs7Y9pV2PGdXXaImAc4w5PyuJ5YSBJlVjAAvnuV4c7ufp5eevDvzds3d9wmzB6+xWtExqsoE+E2RwXj1dESh4lwUmt4z3eCCQ== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=bu.edu; dmarc=pass action=none header.from=bu.edu; dkim=pass header.d=bu.edu; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bushare.onmicrosoft.com; s=selector2-bushare-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=YO0H6UD2UfwXcv/vjRCKs+1BrCh5JLfdJi40zaZuKmo=; b=5VukyyGylrmXy4XaAaSNQUQaRy59BP+C0UKoHFjzyngWLYk4TRTuGx7hsO+Q3O+Y4qjnl6vw7UqDE416HFEcApSKNOQ8vqLNkgGn0K3ZQ3WZYy6vASSFjx3AYKXqA3fxi4nI3n9iAUWha19NzUY7B1Z5Nu4s36iAltZ7Dwfn3xA= Received: from CY4PR03MB2872.namprd03.prod.outlook.com (10.175.118.17) by CY4PR03MB3109.namprd03.prod.outlook.com (10.171.247.145) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2284.18; Thu, 19 Sep 2019 19:36:32 +0000 Received: from CY4PR03MB2872.namprd03.prod.outlook.com ([fe80::6d88:c5bd:41ad:b107]) by CY4PR03MB2872.namprd03.prod.outlook.com ([fe80::6d88:c5bd:41ad:b107%3]) with mapi id 15.20.2284.009; Thu, 19 Sep 2019 19:36:32 +0000 From: "Oleinik, Alexander" To: John Snow , Stefan Hajnoczi Subject: Re: [PATCH v3 13/22] libqtest: make qtest_bufwrite send "atomic" Thread-Topic: [PATCH v3 13/22] libqtest: make qtest_bufwrite send "atomic" Thread-Index: AQHVbneLvcrhrDR470+fVESmeJqVGacyz5eAgACLYoCAAAsJgA== Date: Thu, 19 Sep 2019 19:36:31 +0000 Message-ID: References: <20190918231846.22538-1-alxndr@bu.edu> <20190918231846.22538-14-alxndr@bu.edu> <20190919103741.GO3606@stefanha-x1.localdomain> In-Reply-To: Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: spf=none (sender IP is ) smtp.mailfrom=alxndr@bu.edu; x-originating-ip: [128.197.127.33] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: c19c5a22-c37f-44f5-f047-08d73d38acc2 x-microsoft-antispam: BCL:0; PCL:0; RULEID:(2390118)(7020095)(4652040)(8989299)(5600167)(711020)(4605104)(1401327)(4534185)(4627221)(201703031133081)(201702281549075)(8990200)(2017052603328)(7193020); SRVR:CY4PR03MB3109; x-ms-traffictypediagnostic: CY4PR03MB3109: x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:1284; x-forefront-prvs: 016572D96D x-forefront-antispam-report: SFV:NSPM; SFS:(10019020)(979002)(4636009)(136003)(39860400002)(396003)(366004)(346002)(376002)(199004)(189003)(6246003)(75432002)(7736002)(99286004)(6436002)(110136005)(54906003)(76176011)(5660300002)(25786009)(86362001)(256004)(88552002)(71190400001)(4326008)(71200400001)(2906002)(81166006)(81156014)(478600001)(8936002)(8676002)(229853002)(14454004)(66476007)(66066001)(6486002)(36756003)(118296001)(66946007)(66556008)(102836004)(64756008)(66446008)(53546011)(486006)(54896002)(76116006)(236005)(26005)(6512007)(786003)(446003)(105004)(19627405001)(11346002)(14444005)(476003)(6506007)(186003)(6116002)(3846002)(2616005)(316002)(969003)(989001)(999001)(1009001)(1019001); DIR:OUT; SFP:1102; SCL:1; SRVR:CY4PR03MB3109; H:CY4PR03MB2872.namprd03.prod.outlook.com; FPR:; SPF:None; LANG:en; PTR:InfoNoRecords; A:1; MX:1; received-spf: None (protection.outlook.com: bu.edu does not designate permitted sender hosts) x-ms-exchange-senderadcheck: 1 x-microsoft-antispam-message-info: Jf4AhiMxEuX4AEWi4ETwzjDzvTtdOJ/N0OvTJihsoCIRXdEV450O+5+QN8rniHhYxHqb2WcPpetVybtR+cDKGi/5n1BXIuoV8h6O4bVllLrnyLfnMisTKZl+0zJVIN0M4A+CrH2N1mro3KqbpshkhIaBbiWTFPZqOLk5MOy3Q8UYvs5cQdIywRM5gqF6cRAoVR/ry8cqatTeaLgEexD+v9DCbFY0ZyzP5l+ygb4UaMhfdGjjkd6ACZ9b2+SCRADY/AVbcsLZbkjL9gEir0OtPolyAc4/QReCyaM/xYQo72ax7ZiJZpFuhenpHVJBrTGqoXpAosv8e0VHmEuSmEQ65/9t0YKr7zwMHz9cf5oOgnaDh8zF3NR/lS4VD7Hj04iBnBJK3ii3YGllt85dGiCGjCqUICbQTrnszDkMlVyxBF0= x-ms-exchange-transport-forked: True Content-Type: multipart/alternative; boundary="_000_b416ebcc5d9a0b09a0ad7ef78ba0cd19d13f9f2dcamelbuedu_" MIME-Version: 1.0 X-OriginatorOrg: bu.edu X-MS-Exchange-CrossTenant-Network-Message-Id: c19c5a22-c37f-44f5-f047-08d73d38acc2 X-MS-Exchange-CrossTenant-originalarrivaltime: 19 Sep 2019 19:36:31.9115 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: d57d32cc-c121-488f-b07b-dfe705680c71 X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: MaeCJ6t5rPniD6DVKnIQXdzLn0WHQPHez5kwCDuwRLcX+lOPX5BNLm1VfsSZGKwy X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY4PR03MB3109 X-detected-operating-system: by eggs.gnu.org: Windows 7 or 8 [fuzzy] X-Received-From: 40.107.76.133 X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Thomas Huth , "pbonzini@redhat.com" , "bsd@redhat.com" , "qemu-devel@nongnu.org" , Laurent Vivier Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" --_000_b416ebcc5d9a0b09a0ad7ef78ba0cd19d13f9f2dcamelbuedu_ Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable On Thu, 2019-09-19 at 14:56 -0400, John Snow wrote: > > On 9/19/19 6:37 AM, Stefan Hajnoczi wrote: > > On Wed, Sep 18, 2019 at 11:19:40PM +0000, Oleinik, Alexander wrote: > > > When using qtest "in-process" communication, qtest_sendf directly > > > calls > > > a function in the server (qtest.c). Combining the contents of the > > > subsequent socket_sends into the qtest_sendf, makes it so the > > > server can > > > immediately handle the command, without building a local buffer > > > and > > > waiting for a newline. > > > > > > Signed-off-by: Alexander Oleinik = > > > > --- > > > tests/libqtest.c | 4 +--- > > > 1 file changed, 1 insertion(+), 3 deletions(-) > > > > > > diff --git a/tests/libqtest.c b/tests/libqtest.c > > > index 19feea9e17..d770462869 100644 > > > --- a/tests/libqtest.c > > > +++ b/tests/libqtest.c > > > @@ -1086,9 +1086,7 @@ void qtest_bufwrite(QTestState *s, uint64_t > > > addr, const void *data, size_t size) > > > gchar *bdata; > > > > > > bdata =3D g_base64_encode(data, size); > > > - qtest_sendf(s, "b64write 0x%" PRIx64 " 0x%zx ", addr, size); > > > - socket_send(s->fd, bdata, strlen(bdata)); > > > - socket_send(s->fd, "\n", 1); > > > + qtest_sendf(s, "b64write 0x%" PRIx64 " 0x%zx %s\n", addr, > > > size, bdata); > > > qtest_rsp(s, 0); > > > g_free(bdata); > > > } > > > -- > > > 2.23.0 > > > > Cc John Snow, who added the b64write command. > > > > The downside to doing this is that sprintf-formatting needs to be > > performed on the entire base64 buffer. This slows things down > > slightly > > and a larger temporary buffer needs to be allocated, but I'm not > > sure it > > matters. > > > > *struggles to remember* > > I guess I wanted something that had some space savings while > maintaining > some semblance of debuggability. This is almost certainly meant for > AHCI > tests where it's writing various patterns to large blocks of memory. > > I doubt I really measured the performance of it, but it seemed like > the > way to go for transferring medium amounts of data at the time via the > qtest protocol. > > Looks like I am the only user of it, still: > > tests/ahci-test.c: qtest_bufwrite(ahci->parent->qts, ptr, tx, > bufsize); > tests/ahci-test.c: qtest_bufwrite(ahci->parent->qts, ptr, tx, > bufsize); > tests/libqos/ahci.c: qtest_bufwrite(ahci->parent->qts, ptr, > buffer, bufsize); > > The buffers can be quite large, so you might be re-buffering a decent > amount of data from the sender now. > > 1, Are large transfers like this guaranteed to be atomic anyway? What > kind of socket is it? we're probably eclipsing frame and packet sizes > here. > > 2, I am not sure what being "atomic" affords us in terms of allowing > the server to not wait for newlines, how does this change help? > > --js I'm modifying qtest to allow the server and client to co-exist within the same process (facilitating coverage-guided fuzzing). One of the modifications is making qtest_sendf directly call a function in qtest.c. All the other qtest commands are sent with a single qtest_sendf call, so the qtest.c function could immediately call qtest_process_command. This breaks if the command is sent with different qtest_send/socket_send calls, as in b64write. It should be simple to change qtest_server_inproc_recv (the qtest.c receive= r) to wait for an "\n" prior to qtest_process_command, so I will probably d= o that and then normal(socket) qtest will keep the memory-reduction benefit= s of the non-"atomic" approach. As a side note, would qtest_memwrite, also benefit from splitting up the se= nd command? for (i =3D 0; i < size; i++) { sprintf(&enc[i * 2], "%02x", ptr[i]); } qtest_sendf(s, "write 0x%" PRIx64 " 0x%zx 0x%s\n", addr, size, enc); qtest_rsp(s, 0); g_free(enc); --_000_b416ebcc5d9a0b09a0ad7ef78ba0cd19d13f9f2dcamelbuedu_ Content-Type: text/html; charset="iso-8859-1" Content-ID: Content-Transfer-Encoding: quoted-printable
On Thu, 2019-09-19 at 14:56 -0400, Joh= n Snow wrote:
>
> On 9/19/19 6:37 AM, Stefan Hajnoczi wrote:
> > On Wed, Sep 18, 2019 at 11:19:40PM +0000, Oleinik, Alexan= der wrote:
> > > When using qtest "in-process" communication, qtest_= sendf directly
> > > calls
> > > a function in the server (qtest.c). Combining the contents of= the
> > > subsequent socket_sends into the qtest_sendf, makes it so the=
> > > server can
> > > immediately handle the command, without building a local buff= er
> > > and
> > > waiting for a newline.
> > >
> > > Signed-off-by: Alexander Oleinik <alxndr@bu.edu>
> > > ---
> > >  tests/libqtest.c | 4 +---
> > >  1 file changed, 1 insertion(+), 3 deletions(-)
> > >
> > > diff --git a/tests/libqtest.c b/tests/libqtest.c
> > > index 19feea9e17..d770462869 100644
> > > --- a/tests/libqtest.c
> > > +++ b/tests/libqtest.c
> > > @@ -1086,9 +1086,7 @@ void qtest_bufwrite(QTestState *s, = uint64_t
> > > addr, const void *data, size_t size)
> > >      gchar *bdata;
> > >  
> > >      bdata =3D g_base64_encode(data,= size);
> > > -    qtest_sendf(s, "b64write 0x%&qu= ot; PRIx64 " 0x%zx ", addr, size);
> > > -    socket_send(s->fd, bdata, strlen(= bdata));
> > > -    socket_send(s->fd, "\n"= , 1);
> > > +    qtest_sendf(s, "b64write 0x= %" PRIx64 " 0x%zx %s\n", addr,
> > > size, bdata);
> > >      qtest_rsp(s, 0);
> > >      g_free(bdata);
> > >  }
> > > -- 
> > > 2.23.0
> >
> > Cc John Snow, who added the b64write command.
> >
> > The downside to doing this is that sprintf-formatting needs t= o be
> > performed on the entire base64 buffer.  This slows = things down
> > slightly
> > and a larger temporary buffer needs to be allocated, but I'm = not
> > sure it
> > matters.
> >
>
> *struggles to remember*
>
> I guess I wanted something that had some space savings while<= span>
> maintaining
> some semblance of debuggability. This is almost certainly mea= nt for
> AHCI
> tests where it's writing various patterns to large blocks of = memory.
>
> I doubt I really measured the performance of it, but it seeme= d like
> the
> way to go for transferring medium amounts of data at the time= via the
> qtest protocol.
>
> Looks like I am the only user of it, still:
>
> tests/ahci-test.c:    qtest_bufwrite(ahci= ->parent->qts, ptr, tx,
> bufsize);
> tests/ahci-test.c:    qtest_bufwrite(ahci= ->parent->qts, ptr, tx,
> bufsize);
> tests/libqos/ahci.c:       = ; qtest_bufwrite(ahci->parent->qts, ptr,
> buffer, bufsize);
>
> The buffers can be quite large, so you might be re-buffering = a decent
> amount of data from the sender now.
>
> 1, Are large transfers like this guaranteed to be atomic anyw= ay? What
> kind of socket is it? we're probably eclipsing frame and pack= et sizes
> here.
>
> 2, I am not sure what being "atomic" affords us in = terms of allowing
> the server to not wait for newlines, how does this change hel= p?
>
> --js

I'm modifying qtest to allow the serve= r and client to co-exist within
the same process (facilitating coverage-guided fuzzing). One of the
modifications is making qtest_sendf directly call a function in
qtest.c. All the other qtest commands are sent with a single qtest_sendf call, so the qtest.c function could immediately call
qtest_process_command. This breaks if the command is sent with=
different qtest_send/socket_send calls, as in b64write.

It should be simple to change qtest_se= rver_inproc_recv (the qtest.c receiver) to wait for an "\n" prior= to qtest_process_command, so I will probably do that and then normal(socke= t) qtest will keep the memory-reduction benefits of the non-"atomic" approach.

As a side note, would qtest_memwrite, = also benefit from splitting up the send command?

    for (i =3D 0; = i < size; i++) {
      &n= bsp; sprintf(&enc[i * 2], "%02x", ptr[i]);
    }

    qtest_sendf(s,= "write 0x%" PRIx64 " 0x%zx 0x%s\n", addr, size, enc);<= /div>
    qtest_rsp(s, 0= );
    g_free(enc);
--_000_b416ebcc5d9a0b09a0ad7ef78ba0cd19d13f9f2dcamelbuedu_--