Fri, 16 Jun 2023 15:22:24 +0100 George Dunlap <george.dun...@cloud.com>:
> 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.
I have tested the patch below with this cmdline:
conring_size=32M loglvl=all guest_loglvl=all crashkernel=264M,below=4G
console=com1 com1=115200 dom0_mem=16G dom0_max_vcpus=8 dom0_vcpus_pin
maxcpus=10 console_timestamps=datems tbuf_size=-1 smt=0
It appears to work as expected. One downside is that xenalyze reports
each cpu it finds based on index, instead of the real smp_processor_id.
That is apparently a limitation of the interface. In the end this detail
may not matter.
I think this change should go on top of a revert of
commit 74584a367051bc0d6f4b96fd360fa7bc6538fc39.
Regarding coding style: can this_cpu and per_cpu be used as array index,
or is a temporary variable required? This would affect the number of
lines changed in next_record.
Olaf
--- a/xen/common/trace.c
+++ b/xen/common/trace.c
@@ -53,6 +53,7 @@ integer_param("tevt_mask", opt_tevt_mask);
static struct t_info *t_info;
static unsigned int t_info_pages;
+static DEFINE_PER_CPU_READ_MOSTLY(uint16_t, t_info_mfn_offset);
static DEFINE_PER_CPU_READ_MOSTLY(struct t_buf *, t_bufs);
static DEFINE_PER_CPU_READ_MOSTLY(spinlock_t, t_lock);
static u32 data_size __read_mostly;
@@ -110,7 +111,8 @@ static int calculate_tbuf_size(unsigned int pages, uint16_t
t_info_first_offset)
struct t_info dummy_pages;
typeof(dummy_pages.tbuf_size) max_pages;
typeof(dummy_pages.mfn_offset[0]) max_mfn_offset;
- unsigned int max_cpus = nr_cpu_ids;
+ unsigned int nr_cpus = num_online_cpus();
+ unsigned int max_cpus = nr_cpus;
unsigned int t_info_words;
/* force maximum value for an unsigned type */
@@ -148,11 +150,11 @@ static int calculate_tbuf_size(unsigned int pages,
uint16_t t_info_first_offset)
* NB this calculation is correct, because t_info_first_offset is
* in words, not bytes
*/
- t_info_words = nr_cpu_ids * pages + t_info_first_offset;
+ t_info_words = nr_cpus * pages + t_info_first_offset;
t_info_pages = PFN_UP(t_info_words * sizeof(uint32_t));
printk(XENLOG_INFO "xentrace: requesting %u t_info pages "
"for %u trace pages on %u cpus\n",
- t_info_pages, pages, nr_cpu_ids);
+ t_info_pages, pages, nr_cpus);
return pages;
}
@@ -173,6 +175,7 @@ static int alloc_trace_bufs(unsigned int pages)
uint32_t *t_info_mfn_list;
uint16_t t_info_first_offset;
uint16_t offset;
+ uint16_t index = 0;
if ( t_info )
return -EBUSY;
@@ -201,8 +204,9 @@ static int alloc_trace_bufs(unsigned int pages)
*/
for_each_online_cpu(cpu)
{
- offset = t_info_first_offset + (cpu * pages);
- t_info->mfn_offset[cpu] = offset;
+ per_cpu(t_info_mfn_offset, cpu) = index;
+ offset = t_info_first_offset + (index * pages);
+ t_info->mfn_offset[index] = offset;
for ( i = 0; i < pages; i++ )
{
@@ -216,6 +220,7 @@ static int alloc_trace_bufs(unsigned int pages)
}
t_info_mfn_list[offset + i] = virt_to_mfn(p);
}
+ index++;
}
/*
@@ -227,7 +232,8 @@ static int alloc_trace_bufs(unsigned int pages)
spin_lock_init(&per_cpu(t_lock, cpu));
- offset = t_info->mfn_offset[cpu];
+ index = per_cpu(t_info_mfn_offset, cpu);
+ offset = t_info->mfn_offset[index];
/* Initialize the buffer metadata */
per_cpu(t_bufs, cpu) = buf = mfn_to_virt(t_info_mfn_list[offset]);
@@ -260,7 +266,8 @@ static int alloc_trace_bufs(unsigned int pages)
out_dealloc:
for_each_online_cpu(cpu)
{
- offset = t_info->mfn_offset[cpu];
+ index = per_cpu(t_info_mfn_offset, cpu);
+ offset = t_info->mfn_offset[index];
if ( !offset )
continue;
for ( i = 0; i < pages; i++ )
@@ -509,6 +516,7 @@ static unsigned char *next_record(const struct t_buf *buf,
uint32_t *next,
uint32_t per_cpu_mfn_nr;
uint32_t *mfn_list;
uint32_t mfn;
+ uint16_t index;
unsigned char *this_page;
barrier(); /* must read buf->prod and buf->cons only once */
@@ -527,7 +535,8 @@ static unsigned char *next_record(const struct t_buf *buf,
uint32_t *next,
/* offset into array of mfns */
per_cpu_mfn_nr = x >> PAGE_SHIFT;
- per_cpu_mfn_offset = t_info->mfn_offset[smp_processor_id()];
+ index = this_cpu(t_info_mfn_offset);
+ per_cpu_mfn_offset = t_info->mfn_offset[index];
mfn_list = (uint32_t *)t_info;
mfn = mfn_list[per_cpu_mfn_offset + per_cpu_mfn_nr];
this_page = mfn_to_virt(mfn);
pgpXKqUgxi4k4.pgp
Description: Digitale Signatur von OpenPGP
