Re: xentrace buffer size, maxcpus and online cpus

[George Dunlap <george.dunlap@xxxxxxxxx>]

On Fri, Jun 16, 2023 at 12:52 PM Jan Beulich <jbeulich@xxxxxxxx> wrote:

On 16.06.2023 13:47, Olaf Hering wrote:
> Wed, 31 May 2023 11:05:52 +0200 Jan Beulich <jbeulich@xxxxxxxx>:
>
>> As said before, num_online_cpus() will under-report for the purpose
>> here, as CPUs may have been brought offline, and may be brought online
>> again later (independent of the use of "maxcpus=").
>
> It turned out, commit 74584a367051bc0d6f4b96fd360fa7bc6538fc39 broke
> the expected behavior. But to me it is unclear what bug was fixed by
> this commit.

Hmm, I find title and description quite clear there.

[SNIP] 

> To me it looks like commit 74584a367051bc0d6f4b96fd360fa7bc6538fc39
> could be reverted.

I don't think so. I'll add George to Cc as well, as he's the maintainer
of this stuff.

I agree; the clear implication is that with smt=0, you might have num_online_cpus() return 4, but cpuids that look like {1, 3, 5, 7}; so you need the trace buffer array to be of size 8.

 

> If I read alloc_trace_bufs correctly, it already operates on online
> cpus. And __trace_var will do nothing if called on a cpu which was
> not online, t_bufs will likely be NULL.

Yielding an incomplete overall trace, at best.

Historically we've avoided the thorny problems of synchronization wrt the trace buffers by saying that you get one chance to set them up the way you want, and that's what you get until you reboot the host.  If someone felt like sorting all that out I wouldn't oppose it.  But in the meantime, the current "policy" is that it's OK if *tracing* breaks when changing the number of cpus, as long as it doesn't cause the *hypervisor* to break.

The proposed change to calculate_tbuf_size() wouldn't work as-is, because it looks like at the moment alloc_trace_bufs() leaves a gap in the mfn list for "offline" CPUs.  But it looks like maybe the "interface" would allow those "holes" to be compacted?

 -George