Re: [PATCH 2/2] tools/migration: Fix potential overflow in send_checkpoint_dirty_pfn_list()

2021-07-06 Thread Andrew Cooper
On 06/07/2021 16:22, Jan Beulich wrote: > On 06.07.2021 17:13, Jan Beulich wrote: >> On 06.07.2021 16:11, Olaf Hering wrote: >>> Am Tue, 6 Jul 2021 14:58:04 +0200 >>> schrieb Olaf Hering : >>> the reporting is broken since a while >>> A quick check on a Dell T320 with E5-2430L, which does not

Re: [PATCH 2/2] tools/migration: Fix potential overflow in send_checkpoint_dirty_pfn_list()

2021-07-06 Thread Jan Beulich
On 06.07.2021 17:13, Jan Beulich wrote: > On 06.07.2021 16:11, Olaf Hering wrote: >> Am Tue, 6 Jul 2021 14:58:04 +0200 >> schrieb Olaf Hering : >> >>> the reporting is broken since a while >> >> A quick check on a Dell T320 with E5-2430L, which does not have "Page >> Modification Logging", indicat

Re: [PATCH 2/2] tools/migration: Fix potential overflow in send_checkpoint_dirty_pfn_list()

2021-07-06 Thread Jan Beulich
On 06.07.2021 16:11, Olaf Hering wrote: > Am Tue, 6 Jul 2021 14:58:04 +0200 > schrieb Olaf Hering : > >> the reporting is broken since a while > > A quick check on a Dell T320 with E5-2430L, which does not have "Page > Modification Logging", indicates that staging-4.5 appears to work, but > rep

Re: [PATCH 2/2] tools/migration: Fix potential overflow in send_checkpoint_dirty_pfn_list()

2021-07-06 Thread Olaf Hering
Am Tue, 6 Jul 2021 14:58:04 +0200 schrieb Olaf Hering : > the reporting is broken since a while A quick check on a Dell T320 with E5-2430L, which does not have "Page Modification Logging", indicates that staging-4.5 appears to work, but reporting in staging-4.6 is broken. Olaf pgpvrugZq6Cre

Re: [PATCH 2/2] tools/migration: Fix potential overflow in send_checkpoint_dirty_pfn_list()

2021-07-06 Thread Jan Beulich
On 06.07.2021 15:34, Andrew Cooper wrote: > On 06/07/2021 13:03, Jan Beulich wrote: >> On 06.07.2021 13:23, Andrew Cooper wrote: >>> 'count * sizeof(*pfns)' can in principle overflow, but is implausible in >>> practice as the time between checkpoints is typically sub-second. >>> Nevertheless, simpl

Re: [PATCH 2/2] tools/migration: Fix potential overflow in send_checkpoint_dirty_pfn_list()

2021-07-06 Thread Jan Beulich
On 06.07.2021 15:19, Andrew Cooper wrote: > On 06/07/2021 13:58, Olaf Hering wrote: >> Am Tue, 6 Jul 2021 12:23:32 +0100 >> schrieb Andrew Cooper : >> >>> +count = stats.dirty_count; >> Is this accurate? > > The live loop relies on it, and it worked correctly the last time I > tested it. When

Re: [PATCH 2/2] tools/migration: Fix potential overflow in send_checkpoint_dirty_pfn_list()

2021-07-06 Thread Andrew Cooper
On 06/07/2021 14:39, Olaf Hering wrote: > Am Tue, 6 Jul 2021 14:22:58 +0100 > schrieb Andrew Cooper : > >> What hardware is this on?  i.e. is the Page Modification Logging feature >> in use? > At least it gets reported as VMX feature during boot, this is a CoyotePass > system. That logging is pro

Re: [PATCH 2/2] tools/migration: Fix potential overflow in send_checkpoint_dirty_pfn_list()

2021-07-06 Thread Olaf Hering
Am Tue, 6 Jul 2021 14:22:58 +0100 schrieb Andrew Cooper : > What hardware is this on?  i.e. is the Page Modification Logging feature > in use? At least it gets reported as VMX feature during boot, this is a CoyotePass system. Olaf pgpwQDzeiSuN6.pgp Description: Digitale Signatur von OpenPGP

Re: [PATCH 2/2] tools/migration: Fix potential overflow in send_checkpoint_dirty_pfn_list()

2021-07-06 Thread Andrew Cooper
On 06/07/2021 13:03, Jan Beulich wrote: > On 06.07.2021 13:23, Andrew Cooper wrote: >> 'count * sizeof(*pfns)' can in principle overflow, but is implausible in >> practice as the time between checkpoints is typically sub-second. >> Nevertheless, simplify the code and remove the risk. >> >> There is

Re: [PATCH 2/2] tools/migration: Fix potential overflow in send_checkpoint_dirty_pfn_list()

2021-07-06 Thread Olaf Hering
Am Tue, 6 Jul 2021 14:19:21 +0100 schrieb Andrew Cooper : > > 20: faults= 0 dirty= 80 > > What is this showing, other than (unsurprisingly) faults doesn't work > for an HVM guest? The dirty count goes down after a while for a domU that constantly touches as many pages as it can. But yes, the

Re: [PATCH 2/2] tools/migration: Fix potential overflow in send_checkpoint_dirty_pfn_list()

2021-07-06 Thread Andrew Cooper
On 06/07/2021 14:19, Andrew Cooper wrote: > On 06/07/2021 13:58, Olaf Hering wrote: >> Am Tue, 6 Jul 2021 12:23:32 +0100 >> schrieb Andrew Cooper : >> >>> +count = stats.dirty_count; >> Is this accurate? > The live loop relies on it, and it worked correctly the last time I > tested it. > >> I r

Re: [PATCH 2/2] tools/migration: Fix potential overflow in send_checkpoint_dirty_pfn_list()

2021-07-06 Thread Andrew Cooper
On 06/07/2021 13:58, Olaf Hering wrote: > Am Tue, 6 Jul 2021 12:23:32 +0100 > schrieb Andrew Cooper : > >> +count = stats.dirty_count; > Is this accurate? The live loop relies on it, and it worked correctly the last time I tested it. > I remember the reporting is broken since a while, and tes

Re: [PATCH 2/2] tools/migration: Fix potential overflow in send_checkpoint_dirty_pfn_list()

2021-07-06 Thread Olaf Hering
Am Tue, 6 Jul 2021 12:23:32 +0100 schrieb Andrew Cooper : > +count = stats.dirty_count; Is this accurate? I remember the reporting is broken since a while, and testing a busy domU indicates it is still the case. # xen-logdirty `xl domid domU` 0: faults= 0 dirty= 258050 1: faults= 0 dirty=

Re: [PATCH 2/2] tools/migration: Fix potential overflow in send_checkpoint_dirty_pfn_list()

2021-07-06 Thread Jan Beulich
On 06.07.2021 13:23, Andrew Cooper wrote: > 'count * sizeof(*pfns)' can in principle overflow, but is implausible in > practice as the time between checkpoints is typically sub-second. > Nevertheless, simplify the code and remove the risk. > > There is no need to loop over the bitmap to calculate

[PATCH 2/2] tools/migration: Fix potential overflow in send_checkpoint_dirty_pfn_list()

2021-07-06 Thread Andrew Cooper
'count * sizeof(*pfns)' can in principle overflow, but is implausible in practice as the time between checkpoints is typically sub-second. Nevertheless, simplify the code and remove the risk. There is no need to loop over the bitmap to calculate count. The number of set bits is returned in xc_sha