All of lore.kernel.org
 help / color / mirror / Atom feed
* [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
* [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 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
* 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.