[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v2 01/12] xen/trace: Don't over-read trace objects


  • To: Jan Beulich <JBeulich@xxxxxxxx>
  • From: Dario Faggioli <dfaggioli@xxxxxxxx>
  • Date: Thu, 30 Sep 2021 08:07:12 +0000
  • Accept-language: en-US
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=suse.com; dmarc=pass action=none header.from=suse.com; dkim=pass header.d=suse.com; arc=none
  • 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; bh=kD/3FSSuo+0Qv8jCUpQXYHGDFD27mwPLJVLhJ2SwDvo=; b=b9jA8ENMX1dKkvS1h03kyY4tvZlkjJFGA4y6X4aush3wQtqPJwkEn0tUmsCY+7RHhKk2S7z6Bq6ETN4ypDK7CH65Mlou+c3Fk6mCx/0MI9JlkOoeXR2bHRtsSUzs4BZE44jCOzmNOfHkfWh1CCKmXY8KOMBGmc/wc5dm81MBghLoKtM1UbEMVqyicZRZF6tRV/ltm/b24D3IbZ737bkY0Ldh+GZjtwEIyWi57CjFWqjFHDo3n7C7oEqWf1g5fYctutlPVDQnAnpVPcb7EQCYf/tVBdvAKMP05L4s6+ZFJdskIYsO4FFoLPr33Vnf6a3+sMhjNzJ87fc1feCF7RaSQA==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=hqHqtDjhbt02FQv3laidogZOGG96DBf1zOzjM/YeGVDH/7cNxOjIooPRI9WVDkfOAPkxFryeitKRDny6OvRh0esnotFmdBG9TNUoX9z5MVym/4to+unTcD7sYPK8Yo/YM+NT1rRdUHuU8JI8Au3HpKCoVgEkiQBA3mQV4sMtjsPQCQ6HlGEbtmr17zdcjpm8c0A1I7jvQqsyODq+1AOJVOON2AKHQlXChpbbzPy3KAaPiuSF2t7nmUR5ZMmpmRAnsKaun0+YqgiGZnRGj/y35tlENCS2kSq0sLMR3Z0XN9RVyST+q2eglAmO453EtG6v0gtKGHpVVHZS1r6D8jnySw==
  • Authentication-results: suse.com; dkim=none (message not signed) header.d=none;suse.com; dmarc=none action=none header.from=suse.com;
  • Cc: "julien@xxxxxxx" <julien@xxxxxxx>, "wl@xxxxxxx" <wl@xxxxxxx>, "iwj@xxxxxxxxxxxxxx" <iwj@xxxxxxxxxxxxxx>, "sstabellini@xxxxxxxxxx" <sstabellini@xxxxxxxxxx>, "mengxu@xxxxxxxxxxxxx" <mengxu@xxxxxxxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>, "andrew.cooper3@xxxxxxxxxx" <andrew.cooper3@xxxxxxxxxx>, "George.Dunlap@xxxxxxxxxxxxx" <George.Dunlap@xxxxxxxxxxxxx>
  • Delivery-date: Thu, 30 Sep 2021 08:07:20 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Thread-index: AQHXrkSLbMGUvlBXkkK2FQ1vuFXyFauzSsKAgARBtICABLtYgA==
  • Thread-topic: [PATCH v2 01/12] xen/trace: Don't over-read trace objects

On Mon, 2021-09-27 at 09:51 +0200, Jan Beulich wrote:
> On 24.09.2021 16:51, Dario Faggioli wrote:
> > On Mon, 2021-09-20 at 18:25 +0100, Andrew Cooper wrote:
> > 
> > > There is one buggy race record, TRC_RTDS_BUDGET_BURN.  As it must
> > > remain
> > > __packed (as cur_budget is misaligned), change bool has_extratime
> > > to
> > > uint32_t
> > > to compensate.
> > > 
> > Mmm... maybe my understanding of data alignment inside structs is a
> > bit
> > lacking, but what the actual issue here, and what would we need to
> > do
> > to fix it (where, by fix, I mean us being able to get rid of the
> > `__packed`)?
> > 
> > If rearranging fields is not enough, we can think about making
> > priority_level and has_extratime smaller, or even combining them in
> > just one field and decode the information in xentrace.
> 
> I guess Andrew has tried to avoid re-arranging field order so that
> the consumer side doesn't need to also change.
> 
Right, but is it really worth it, in this case?

Aren't we (very very likely) in control, by having them here in the
tree, of all the consumers? And is is this a stable ABI?

Also, both xentrace_format and xenalyze are being modified in this
series anyway...

Maybe there's still something I'm missing, but if we've getting rid of
those ugly bitfields and __packed attributes, it seems to me that doing
it completely --i.e., including in this case-- is worth the small
change in the tools.

Regards
-- 
Dario Faggioli, Ph.D
http://about.me/dario.faggioli
Virtualization Software Engineer
SUSE Labs, SUSE https://www.suse.com/
-------------------------------------------------------------------
<<This happens because _I_ choose it to happen!>> (Raistlin Majere)

Attachment: signature.asc
Description: This is a digitally signed message part


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.