* [PATCH v2 0/2] xentrace: fix xentrace for smt=0 @ 2018-10-04 10:51 Juergen Gross 2018-10-04 10:51 ` [PATCH v2 1/2] xentrace: allow sparse cpu list Juergen Gross 2018-10-04 10:51 ` [PATCH v2 2/2] xentrace: handle sparse cpu ids correctly in xen trace buffer handling Juergen Gross 0 siblings, 2 replies; 6+ messages in thread From: Juergen Gross @ 2018-10-04 10:51 UTC (permalink / raw) To: xen-devel; +Cc: George.Dunlap, wei.liu2, ian.jackson, Juergen Gross When hyperthreads are disabled via smt=0 Xen boot parameter xentrace is no longer working, but crashing the system. Patch 1 makes the xentrace user tool work on systems when not all possible cpu ids have an associated trace buffer allocated. Patch 2 corrects xentrace handling by sizing the control structures according to the physical cpus instead of taking online cpus into account only. Juergen Gross (2): xentrace: allow sparse cpu list xentrace: handle sparse cpu ids correctly in xen trace buffer handling tools/xentrace/xentrace.c | 18 ++++++++++++------ xen/common/trace.c | 6 +++--- 2 files changed, 15 insertions(+), 9 deletions(-) -- 2.16.4 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 1/2] xentrace: allow sparse cpu list 2018-10-04 10:51 [PATCH v2 0/2] xentrace: fix xentrace for smt=0 Juergen Gross @ 2018-10-04 10:51 ` Juergen Gross 2018-10-04 11:22 ` George Dunlap 2018-10-04 10:51 ` [PATCH v2 2/2] xentrace: handle sparse cpu ids correctly in xen trace buffer handling Juergen Gross 1 sibling, 1 reply; 6+ messages in thread From: Juergen Gross @ 2018-10-04 10:51 UTC (permalink / raw) To: xen-devel; +Cc: George.Dunlap, wei.liu2, ian.jackson, Juergen Gross Modify the xentrace utility to allow sparse cpu list resulting in not all possible cpus having a trace buffer allocated. Signed-off-by: Juergen Gross <jgross@suse.com> --- tools/xentrace/xentrace.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/tools/xentrace/xentrace.c b/tools/xentrace/xentrace.c index 364a6fdad5..590a91e091 100644 --- a/tools/xentrace/xentrace.c +++ b/tools/xentrace/xentrace.c @@ -500,12 +500,14 @@ static struct t_struct *map_tbufs(unsigned long tbufs_mfn, unsigned int num, for(i=0; i<num; i++) { - - const uint32_t *mfn_list = (const uint32_t *)tbufs.t_info - + tbufs.t_info->mfn_offset[i]; + const uint32_t *mfn_list; int j; xen_pfn_t pfn_list[tbufs.t_info->tbuf_size]; + if ( !tbufs.t_info->mfn_offset[i] ) + continue; + + mfn_list = (const uint32_t *)tbufs.t_info + tbufs.t_info->mfn_offset[i]; for ( j=0; j<tbufs.t_info->tbuf_size; j++) pfn_list[j] = (xen_pfn_t)mfn_list[j]; @@ -702,7 +704,8 @@ static int monitor_tbufs(void) if ( opts.discard ) for ( i = 0; i < num; i++ ) - meta[i]->cons = meta[i]->prod; + if ( meta[i] ) + meta[i]->cons = meta[i]->prod; /* now, scan buffers for events */ while ( 1 ) @@ -710,7 +713,10 @@ static int monitor_tbufs(void) for ( i = 0; i < num; i++ ) { unsigned long start_offset, end_offset, window_size, cons, prod; - + + if ( !meta[i] ) + continue; + /* Read window information only once. */ cons = meta[i]->cons; prod = meta[i]->prod; -- 2.16.4 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/2] xentrace: allow sparse cpu list 2018-10-04 10:51 ` [PATCH v2 1/2] xentrace: allow sparse cpu list Juergen Gross @ 2018-10-04 11:22 ` George Dunlap 2018-10-04 11:25 ` Juergen Gross 0 siblings, 1 reply; 6+ messages in thread From: George Dunlap @ 2018-10-04 11:22 UTC (permalink / raw) To: Juergen Gross, xen-devel; +Cc: George.Dunlap, wei.liu2, ian.jackson [-- Attachment #1: Type: text/plain, Size: 2059 bytes --] On 10/04/2018 11:51 AM, Juergen Gross wrote: > Modify the xentrace utility to allow sparse cpu list resulting in not > all possible cpus having a trace buffer allocated. > > Signed-off-by: Juergen Gross <jgross@suse.com> This looks good: Reviewed-by: George Dunlap <george.dunlap@citrix.com> Would you mind if I fold in the attached comment when committing? -George > --- > tools/xentrace/xentrace.c | 16 +++++++++++----- > 1 file changed, 11 insertions(+), 5 deletions(-) > > diff --git a/tools/xentrace/xentrace.c b/tools/xentrace/xentrace.c > index 364a6fdad5..590a91e091 100644 > --- a/tools/xentrace/xentrace.c > +++ b/tools/xentrace/xentrace.c > @@ -500,12 +500,14 @@ static struct t_struct *map_tbufs(unsigned long tbufs_mfn, unsigned int num, > > for(i=0; i<num; i++) > { > - > - const uint32_t *mfn_list = (const uint32_t *)tbufs.t_info > - + tbufs.t_info->mfn_offset[i]; > + const uint32_t *mfn_list; > int j; > xen_pfn_t pfn_list[tbufs.t_info->tbuf_size]; > > + if ( !tbufs.t_info->mfn_offset[i] ) > + continue; > + > + mfn_list = (const uint32_t *)tbufs.t_info + tbufs.t_info->mfn_offset[i]; > for ( j=0; j<tbufs.t_info->tbuf_size; j++) > pfn_list[j] = (xen_pfn_t)mfn_list[j]; > > @@ -702,7 +704,8 @@ static int monitor_tbufs(void) > > if ( opts.discard ) > for ( i = 0; i < num; i++ ) > - meta[i]->cons = meta[i]->prod; > + if ( meta[i] ) > + meta[i]->cons = meta[i]->prod; > > /* now, scan buffers for events */ > while ( 1 ) > @@ -710,7 +713,10 @@ static int monitor_tbufs(void) > for ( i = 0; i < num; i++ ) > { > unsigned long start_offset, end_offset, window_size, cons, prod; > - > + > + if ( !meta[i] ) > + continue; > + > /* Read window information only once. */ > cons = meta[i]->cons; > prod = meta[i]->prod; > [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: sparse-cpu-comment.diff --] [-- Type: text/x-patch; name="sparse-cpu-comment.diff", Size: 763 bytes --] diff --git a/tools/xentrace/xentrace.c b/tools/xentrace/xentrace.c index 590a91e091..a897223592 100644 --- a/tools/xentrace/xentrace.c +++ b/tools/xentrace/xentrace.c @@ -489,7 +489,11 @@ static struct t_struct *map_tbufs(unsigned long tbufs_mfn, unsigned int num, exit(EXIT_FAILURE); } - /* Map per-cpu buffers */ + /* + * Map per-cpu buffers. NB that if a cpus is offline, it may have + * no trace buffers. In this case, the the respective mfn_offset + * will be 0, and the index should be ignored. + */ tbufs.meta = (struct t_buf **)calloc(num, sizeof(struct t_buf *)); tbufs.data = (unsigned char **)calloc(num, sizeof(unsigned char *)); if ( tbufs.meta == NULL || tbufs.data == NULL ) [-- Attachment #3: Type: text/plain, Size: 157 bytes --] _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/2] xentrace: allow sparse cpu list 2018-10-04 11:22 ` George Dunlap @ 2018-10-04 11:25 ` Juergen Gross 0 siblings, 0 replies; 6+ messages in thread From: Juergen Gross @ 2018-10-04 11:25 UTC (permalink / raw) To: George Dunlap, xen-devel; +Cc: George.Dunlap, ian.jackson, wei.liu2 On 04/10/2018 13:22, George Dunlap wrote: > On 10/04/2018 11:51 AM, Juergen Gross wrote: >> Modify the xentrace utility to allow sparse cpu list resulting in not >> all possible cpus having a trace buffer allocated. >> >> Signed-off-by: Juergen Gross <jgross@suse.com> > > This looks good: > > Reviewed-by: George Dunlap <george.dunlap@citrix.com> > > Would you mind if I fold in the attached comment when committing? With some adaptions, please: - /* Map per-cpu buffers */ + /* + * Map per-cpu buffers. NB that if a cpus is offline, it may have s/ cpus / cpu / + * no trace buffers. In this case, the the respective mfn_offset s/the the/the/ + * will be 0, and the index should be ignored. + */ Juergen _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 2/2] xentrace: handle sparse cpu ids correctly in xen trace buffer handling 2018-10-04 10:51 [PATCH v2 0/2] xentrace: fix xentrace for smt=0 Juergen Gross 2018-10-04 10:51 ` [PATCH v2 1/2] xentrace: allow sparse cpu list Juergen Gross @ 2018-10-04 10:51 ` Juergen Gross 2018-10-04 11:39 ` George Dunlap 1 sibling, 1 reply; 6+ messages in thread From: Juergen Gross @ 2018-10-04 10:51 UTC (permalink / raw) To: xen-devel; +Cc: George.Dunlap, wei.liu2, ian.jackson, Juergen Gross The per-cpu buffers for Xentrace are addressed by cpu-id, but the info array for the buffers is sized only by number of online cpus. This might lead to crashes when using Xentrace with smt=0. The t_info structure has to be sized based on nr_cpu_ids. Signed-off-by: Juergen Gross <jgross@suse.com> --- tools/xentrace/xentrace.c | 2 +- xen/common/trace.c | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tools/xentrace/xentrace.c b/tools/xentrace/xentrace.c index 590a91e091..12497a16b4 100644 --- a/tools/xentrace/xentrace.c +++ b/tools/xentrace/xentrace.c @@ -596,7 +596,7 @@ static unsigned int get_num_cpus(void) exit(EXIT_FAILURE); } - return physinfo.nr_cpus; + return physinfo.max_cpu_id + 1; } /** diff --git a/xen/common/trace.c b/xen/common/trace.c index 8cdc17b731..c079454c6a 100644 --- a/xen/common/trace.c +++ b/xen/common/trace.c @@ -113,7 +113,7 @@ 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 = num_online_cpus(); + unsigned int max_cpus = nr_cpu_ids; unsigned int t_info_words; /* force maximum value for an unsigned type */ @@ -151,11 +151,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, not bytes */ - t_info_words = num_online_cpus() * pages + t_info_first_offset; + t_info_words = nr_cpu_ids * 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, num_online_cpus()); + t_info_pages, pages, nr_cpu_ids); return pages; } -- 2.16.4 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 2/2] xentrace: handle sparse cpu ids correctly in xen trace buffer handling 2018-10-04 10:51 ` [PATCH v2 2/2] xentrace: handle sparse cpu ids correctly in xen trace buffer handling Juergen Gross @ 2018-10-04 11:39 ` George Dunlap 0 siblings, 0 replies; 6+ messages in thread From: George Dunlap @ 2018-10-04 11:39 UTC (permalink / raw) To: Juergen Gross, xen-devel; +Cc: George.Dunlap, wei.liu2, ian.jackson On 10/04/2018 11:51 AM, Juergen Gross wrote: > The per-cpu buffers for Xentrace are addressed by cpu-id, but the info > array for the buffers is sized only by number of online cpus. This > might lead to crashes when using Xentrace with smt=0. > > The t_info structure has to be sized based on nr_cpu_ids. > > Signed-off-by: Juergen Gross <jgross@suse.com> > --- > tools/xentrace/xentrace.c | 2 +- > xen/common/trace.c | 6 +++--- > 2 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/tools/xentrace/xentrace.c b/tools/xentrace/xentrace.c > index 590a91e091..12497a16b4 100644 > --- a/tools/xentrace/xentrace.c > +++ b/tools/xentrace/xentrace.c > @@ -596,7 +596,7 @@ static unsigned int get_num_cpus(void) > exit(EXIT_FAILURE); > } > > - return physinfo.nr_cpus; > + return physinfo.max_cpu_id + 1; > } > > /** > diff --git a/xen/common/trace.c b/xen/common/trace.c > index 8cdc17b731..c079454c6a 100644 > --- a/xen/common/trace.c > +++ b/xen/common/trace.c > @@ -113,7 +113,7 @@ 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 = num_online_cpus(); > + unsigned int max_cpus = nr_cpu_ids; > unsigned int t_info_words; > > /* force maximum value for an unsigned type */ > @@ -151,11 +151,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, not bytes > */ This sounds a bit like song lyrics. But that's not your fault, not your fault: Reviewed-by: George Dunlap <george.dunlap@citrix.com> I'll fix the comment on check-in. -George _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-10-04 11:40 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-10-04 10:51 [PATCH v2 0/2] xentrace: fix xentrace for smt=0 Juergen Gross 2018-10-04 10:51 ` [PATCH v2 1/2] xentrace: allow sparse cpu list Juergen Gross 2018-10-04 11:22 ` George Dunlap 2018-10-04 11:25 ` Juergen Gross 2018-10-04 10:51 ` [PATCH v2 2/2] xentrace: handle sparse cpu ids correctly in xen trace buffer handling Juergen Gross 2018-10-04 11:39 ` George Dunlap
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.