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

Re: [PATCH v2 03/13] libxenguest: deal with log-dirty op stats overflow


  • To: Juergen Gross <jgross@xxxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Thu, 19 Aug 2021 13:53:07 +0200
  • 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:X-MS-Exchange-SenderADCheck; bh=NDFOGwrf/co0cRw12K7EfTghWh+uwqSNo0qfEp5rEbw=; b=D25paQ8hQEX7f/qhGpXjKOULj70XF10ORfgGfqssLfZsoNOzJ8K2HBksdWaHEgYMW23ciU3hhbLEh3MsyRdHV0HfZPOmO0FiCsbN9PcqaWPT758VH2UagJklv8MyU6dI9jT/MhsUn2j1A3QtESxiPBv4CBfXW2GWINU1n2kBrEeaJwEZeuTDKsiSaL238D8T0XuC1tvzVWuBo+ch+aF0vgdiIEi8/fTZ8jz8aB/lGhOB4RSND/QlVDZEj5G4PRh+1ixtXtY5N8WIuh7NuBbLW0wZFkgAvoOwb9sGn2/FIv2ONFMRWwsqF2InJg7WHuParJkUFQqjAwCJRDQsc4dnEQ==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=Kl+D6HhC6+TxWjYAD9SBHnf8yD1C08DnqOfVAqOswBmKQIOY/OQlubtzEGaM0Yy3r5PGUVTMjMcCCbcCtsgI1SxqnISoua/GK14g5wfbZoSbz2sTFtXmZjRX+p5IBo6uaFvsSC/KoxHSpAEPlFI2+eRmBeJWJ+rXJ0g8aso0/d30O7EdwNGWzY/lfSsyRtLE5PT4tmt0pcibJoP8/J2x0IrAslfYoqs06ziAFuAz4npEqFwrZhYZ1RO1shmG3OFsEGT05+e40SfVKT2QtnsPBgvYrml+FksSj+oiXUcs8B69S9uuJ7UAJfhYTdPoCU9t+WRYfU6f+MqQXvYAecSd9w==
  • Authentication-results: lists.xenproject.org; dkim=none (message not signed) header.d=none;lists.xenproject.org; dmarc=none action=none header.from=suse.com;
  • Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>, George Dunlap <george.dunlap@xxxxxxxxxx>, Ian Jackson <iwj@xxxxxxxxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Thu, 19 Aug 2021 11:53:16 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 19.08.2021 13:51, Jan Beulich wrote:
> On 19.08.2021 13:25, Juergen Gross wrote:
>> On 19.08.21 13:06, Jan Beulich wrote:
>>> On 19.08.2021 12:20, Juergen Gross wrote:
>>>> On 05.07.21 17:13, Jan Beulich wrote:
>>>>> In send_memory_live() the precise value the dirty_count struct field
>>>>> gets initialized to doesn't matter much (apart from the triggering of
>>>>> the log message in send_dirty_pages(), see below), but it is important
>>>>> that it not be zero on the first iteration (or else send_dirty_pages()
>>>>> won't get called at all). Saturate the initializer value at the maximum
>>>>> value the field can hold.
>>>>>
>>>>> While there also initialize struct precopy_stats' respective field to a
>>>>> more sane value: We don't really know how many dirty pages there are at
>>>>> that point.
>>>>>
>>>>> In suspend_and_send_dirty() and verify_frames() the local variables
>>>>> don't need initializing at all, as they're only an output from the
>>>>> hypercall which gets invoked first thing.
>>>>>
>>>>> In send_checkpoint_dirty_pfn_list() the local variable can be dropped
>>>>> altogether: It's optional to xc_logdirty_control() and not used anywhere
>>>>> else.
>>>>>
>>>>> Note that in case the clipping actually takes effect, the "Bitmap
>>>>> contained more entries than expected..." log message will trigger. This
>>>>> being just an informational message, I don't think this is overly
>>>>> concerning.
>>>>
>>>> Is there any real reason why the width of the stats fields can't be
>>>> expanded to avoid clipping? This could avoid the need to set the
>>>> initial value to -1, which seems one of the more controversial changes.
>>>
>>> While not impossible, it comes with a price tag, as we'd either need
>>> to decouple xc_shadow_op_stats_t from struct xen_domctl_shadow_op_stats
>>> or alter the underlying domctl. Neither of which looked either
>>
>> I was thinking about the domctl.
>>
>> Apart of struct xen_sysctl_page_offline_op this seems to be the only
>> left domctl/sysctl structure limiting guest or host size to values
>> being relevant. Changing those would be a sensible thing to do IMO.
> 
> Yet in the context of v1 of this series, which included "x86/paging:
> deal with log-dirty stats overflow" (now commit 17e91570c5a4) we
> settled on these fields not needing widening. This doesn't prevent
> us doing what you suggest, but it would look pretty odd to me at
> least.

And - just to make it very explicit - even if I went this route to
address a controversial point, I'd still like to understand the
reason for the original objection - if only for my own education.

Jan




 


Rackspace

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