* [RFC/PATCH 0/3] dwarves print: Cacheline accounting fixes
@ 2015-12-08 22:47 Jiri Olsa
[not found] ` <1449614826-2278-1-git-send-email-jolsa-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
0 siblings, 1 reply; 11+ messages in thread
From: Jiri Olsa @ 2015-12-08 22:47 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo; +Cc: Joe Mario, dwarves-u79uwXL29TY76Z2rM5mHXA
hi,
hit 2 issues when used pahole
- sometimes it did not account the hole between variables
- when using -E to extract inner struct, cacheline boundaries were wrong
with this patchset I was able to get it work on structs
that I was investigating, but I'm not sure I covered all
parts or fixed it in proper way.
thanks,
jirka
---
Jiri Olsa (3):
dwarves print: Fix holes accounting
dwarves print: Pass into struct conf_fprintf class__fprintf_cacheline_boundary
dwarves print: Keep global cacheline in struct conf_fprintf
dwarves.h | 1 +
dwarves_fprintf.c | 45 ++++++++++++++++++++++++++-------------------
2 files changed, 27 insertions(+), 19 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe dwarves" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/3] dwarves print: Fix holes accounting
[not found] ` <1449614826-2278-1-git-send-email-jolsa-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
@ 2015-12-08 22:47 ` Jiri Olsa
[not found] ` <1449614826-2278-2-git-send-email-jolsa-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2015-12-08 22:47 ` [PATCH 2/3] dwarves print: Pass into struct conf_fprintf class__fprintf_cacheline_boundary Jiri Olsa
` (2 subsequent siblings)
3 siblings, 1 reply; 11+ messages in thread
From: Jiri Olsa @ 2015-12-08 22:47 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo; +Cc: Joe Mario, dwarves-u79uwXL29TY76Z2rM5mHXA
Sometimes the hole could be missing, try to bypass
this issue by comparing last and current offsets.
Signed-off-by: Jiri Olsa <jolsa-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
dwarves_fprintf.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/dwarves_fprintf.c b/dwarves_fprintf.c
index 71d8ce9f982b..df80af255c67 100644
--- a/dwarves_fprintf.c
+++ b/dwarves_fprintf.c
@@ -1268,6 +1268,17 @@ size_t class__fprintf(struct class *class, const struct cu *cu,
}
pos = tag__class_member(tag_pos);
+ /*
+ * Sometimes the hole could be missing, try to bypass
+ * this issue by comparing last and current offsets.
+ */
+ if (last) {
+ uint32_t tmp = last->byte_offset + last->byte_size + last->hole;
+
+ if (pos->byte_offset > tmp)
+ sum_holes += pos->byte_offset - tmp;
+ }
+
if (last != NULL &&
pos->byte_offset != last->byte_offset &&
!cconf.suppress_comments)
--
2.4.3
--
To unsubscribe from this list: send the line "unsubscribe dwarves" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/3] dwarves print: Pass into struct conf_fprintf class__fprintf_cacheline_boundary
[not found] ` <1449614826-2278-1-git-send-email-jolsa-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2015-12-08 22:47 ` [PATCH 1/3] dwarves print: Fix holes accounting Jiri Olsa
@ 2015-12-08 22:47 ` Jiri Olsa
[not found] ` <1449614826-2278-3-git-send-email-jolsa-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2015-12-08 22:47 ` [PATCH 3/3] dwarves print: Keep global cacheline in struct conf_fprintf Jiri Olsa
2015-12-16 16:57 ` [RFC/PATCH 0/3] dwarves print: Cacheline accounting fixes Jiri Olsa
3 siblings, 1 reply; 11+ messages in thread
From: Jiri Olsa @ 2015-12-08 22:47 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo; +Cc: Joe Mario, dwarves-u79uwXL29TY76Z2rM5mHXA
Passing into struct conf_fprintf class__fprintf_cacheline_boundary,
so the next patch is more easy to read.
Signed-off-by: Jiri Olsa <jolsa-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
dwarves_fprintf.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/dwarves_fprintf.c b/dwarves_fprintf.c
index df80af255c67..2d114210831a 100644
--- a/dwarves_fprintf.c
+++ b/dwarves_fprintf.c
@@ -1122,7 +1122,8 @@ static size_t class__fprintf_cacheline_boundary(uint32_t last_cacheline,
size_t sum, size_t sum_holes,
uint8_t *newline,
uint32_t *cacheline,
- int indent, FILE *fp)
+ struct conf_fprintf *cconf,
+ FILE *fp)
{
const size_t real_sum = sum + sum_holes;
size_t printed = 0;
@@ -1139,7 +1140,7 @@ static size_t class__fprintf_cacheline_boundary(uint32_t last_cacheline,
++printed;
}
- printed += fprintf(fp, "%.*s", indent, tabs);
+ printed += fprintf(fp, "%.*s", cconf->indent, tabs);
if (cacheline_pos == 0)
printed += fprintf(fp, "/* --- cacheline %u boundary "
@@ -1287,7 +1288,7 @@ size_t class__fprintf(struct class *class, const struct cu *cu,
sum, sum_holes,
&newline,
&last_cacheline,
- cconf.indent,
+ &cconf,
fp);
/*
* These paranoid checks doesn't make much sense on
@@ -1477,7 +1478,7 @@ size_t class__fprintf(struct class *class, const struct cu *cu,
sum, sum_holes,
&newline,
&last_cacheline,
- cconf.indent, fp);
+ &cconf, fp);
if (!cconf.show_only_data_members)
class__vtable_fprintf(class, cu, &cconf, fp);
--
2.4.3
--
To unsubscribe from this list: send the line "unsubscribe dwarves" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 3/3] dwarves print: Keep global cacheline in struct conf_fprintf
[not found] ` <1449614826-2278-1-git-send-email-jolsa-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2015-12-08 22:47 ` [PATCH 1/3] dwarves print: Fix holes accounting Jiri Olsa
2015-12-08 22:47 ` [PATCH 2/3] dwarves print: Pass into struct conf_fprintf class__fprintf_cacheline_boundary Jiri Olsa
@ 2015-12-08 22:47 ` Jiri Olsa
[not found] ` <1449614826-2278-4-git-send-email-jolsa-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2015-12-16 16:57 ` [RFC/PATCH 0/3] dwarves print: Cacheline accounting fixes Jiri Olsa
3 siblings, 1 reply; 11+ messages in thread
From: Jiri Olsa @ 2015-12-08 22:47 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo; +Cc: Joe Mario, dwarves-u79uwXL29TY76Z2rM5mHXA
Together with adding cconf->base_offset to sums used
for cacheline computation, this ensures proper cacheline
number to be printed for nested struct. Note possitions
of 'cacheline' lines in following outputs.
Before:
$ pahole -C task_struct -E ~/kernel/linux-perf-local/vmlinux
struct task_struct {
volatile long int state; /* 0 8 */
--- First cacheline is ok
struct task_struct * last_wakee; /* 56 8 */
/* --- cacheline 1 boundary (64 bytes) --- */
int wake_cpu; /* 64 4 */
...
const struct sched_class * sched_class; /* 88 8 */
struct sched_entity {
struct load_weight {
long unsigned int weight; /* 96 8 */
/* typedef u32 */ unsigned int inv_weight; /* 104 4 */
} load; /* 96 16 */
--- Second one is still in relative mode and gives wrong
alignment in wrt task_struct start
unsigned int on_rq; /* 152 4 */
/* --- cacheline 1 boundary (64 bytes) --- */
/* typedef u64 */ long long unsigned int exec_start; /* 160 8 */
/* typedef u64 */ long long unsigned int sum_exec_runtime; /* 168 8 */
...
After:
$ pahole -C task_struct -E ~/kernel/linux-perf-local/vmlinux
struct task_struct {
volatile long int state; /* 0 8 */
--- First cacheline is ok
struct task_struct * last_wakee; /* 56 8 */
/* --- cacheline 1 boundary (64 bytes) --- */
int wake_cpu; /* 64 4 */
...
const struct sched_class * sched_class; /* 88 8 */
struct sched_entity {
struct load_weight {
long unsigned int weight; /* 96 8 */
/* typedef u32 */ unsigned int inv_weight; /* 104 4 */
} load; /* 96 16 */
struct rb_node {
long unsigned int __rb_parent_color; /* 112 8 */
struct rb_node * rb_right; /* 120 8 */
/* --- cacheline 2 boundary (128 bytes) --- */
struct rb_node * rb_left; /* 128 8 */
} run_node; /* 112 24 */
--- Second one is still in relative mode and gives wrong
alignment in wrt task_struct start
/* --- cacheline 2 boundary (128 bytes) was 8 bytes ago --- */
struct list_head {
struct list_head * next; /* 136 8 */
struct list_head * prev; /* 144 8 */
} group_node; /* 136 16 */
...
Signed-off-by: Jiri Olsa <jolsa-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
dwarves.h | 1 +
dwarves_fprintf.c | 25 ++++++++++---------------
2 files changed, 11 insertions(+), 15 deletions(-)
diff --git a/dwarves.h b/dwarves.h
index 73fec8ac614a..44ebf8384b9c 100644
--- a/dwarves.h
+++ b/dwarves.h
@@ -56,6 +56,7 @@ struct conf_fprintf {
int32_t type_spacing;
int32_t name_spacing;
uint32_t base_offset;
+ uint32_t base_cacheline;
uint8_t indent;
uint8_t expand_types:1;
uint8_t expand_pointers:1;
diff --git a/dwarves_fprintf.c b/dwarves_fprintf.c
index 2d114210831a..72d8b08bf235 100644
--- a/dwarves_fprintf.c
+++ b/dwarves_fprintf.c
@@ -1118,22 +1118,21 @@ size_t function__fprintf_stats(const struct tag *tag, const struct cu *cu,
return printed + fprintf(fp, " */\n");
}
-static size_t class__fprintf_cacheline_boundary(uint32_t last_cacheline,
- size_t sum, size_t sum_holes,
+static size_t class__fprintf_cacheline_boundary(size_t sum, size_t sum_holes,
uint8_t *newline,
- uint32_t *cacheline,
struct conf_fprintf *cconf,
FILE *fp)
{
- const size_t real_sum = sum + sum_holes;
+ const size_t real_sum = sum + sum_holes + cconf->base_offset;
size_t printed = 0;
+ uint32_t cacheline = real_sum / cacheline_size;
- *cacheline = real_sum / cacheline_size;
-
- if (*cacheline > last_cacheline) {
+ if (cacheline != cconf->base_cacheline) {
const uint32_t cacheline_pos = real_sum % cacheline_size;
const uint32_t cacheline_in_bytes = real_sum - cacheline_pos;
+ cconf->base_cacheline = cacheline;
+
if (*newline) {
fputc('\n', fp);
*newline = 0;
@@ -1144,12 +1143,12 @@ static size_t class__fprintf_cacheline_boundary(uint32_t last_cacheline,
if (cacheline_pos == 0)
printed += fprintf(fp, "/* --- cacheline %u boundary "
- "(%u bytes) --- */\n", *cacheline,
+ "(%u bytes) --- */\n", cacheline,
cacheline_in_bytes);
else
printed += fprintf(fp, "/* --- cacheline %u boundary "
"(%u bytes) was %u bytes ago --- "
- "*/\n", *cacheline,
+ "*/\n", cacheline,
cacheline_in_bytes, cacheline_pos);
}
return printed;
@@ -1284,10 +1283,8 @@ size_t class__fprintf(struct class *class, const struct cu *cu,
pos->byte_offset != last->byte_offset &&
!cconf.suppress_comments)
printed +=
- class__fprintf_cacheline_boundary(last_cacheline,
- sum, sum_holes,
+ class__fprintf_cacheline_boundary(sum, sum_holes,
&newline,
- &last_cacheline,
&cconf,
fp);
/*
@@ -1474,10 +1471,8 @@ size_t class__fprintf(struct class *class, const struct cu *cu,
}
if (!cconf.suppress_comments)
- printed += class__fprintf_cacheline_boundary(last_cacheline,
- sum, sum_holes,
+ printed += class__fprintf_cacheline_boundary(sum, sum_holes,
&newline,
- &last_cacheline,
&cconf, fp);
if (!cconf.show_only_data_members)
class__vtable_fprintf(class, cu, &cconf, fp);
--
2.4.3
--
To unsubscribe from this list: send the line "unsubscribe dwarves" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [RFC/PATCH 0/3] dwarves print: Cacheline accounting fixes
[not found] ` <1449614826-2278-1-git-send-email-jolsa-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
` (2 preceding siblings ...)
2015-12-08 22:47 ` [PATCH 3/3] dwarves print: Keep global cacheline in struct conf_fprintf Jiri Olsa
@ 2015-12-16 16:57 ` Jiri Olsa
3 siblings, 0 replies; 11+ messages in thread
From: Jiri Olsa @ 2015-12-16 16:57 UTC (permalink / raw)
To: Jiri Olsa
Cc: Arnaldo Carvalho de Melo, Joe Mario, dwarves-u79uwXL29TY76Z2rM5mHXA
On Tue, Dec 08, 2015 at 11:47:03PM +0100, Jiri Olsa wrote:
> hi,
> hit 2 issues when used pahole
> - sometimes it did not account the hole between variables
> - when using -E to extract inner struct, cacheline boundaries were wrong
>
> with this patchset I was able to get it work on structs
> that I was investigating, but I'm not sure I covered all
> parts or fixed it in proper way.
any feedback?
thanks,
jirka
--
To unsubscribe from this list: send the line "unsubscribe dwarves" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] dwarves print: Fix holes accounting
[not found] ` <1449614826-2278-2-git-send-email-jolsa-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
@ 2015-12-16 17:58 ` Arnaldo Carvalho de Melo
[not found] ` <20151216175839.GB6843-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
0 siblings, 1 reply; 11+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-12-16 17:58 UTC (permalink / raw)
To: Jiri Olsa; +Cc: Joe Mario, dwarves-u79uwXL29TY76Z2rM5mHXA
Em Tue, Dec 08, 2015 at 11:47:04PM +0100, Jiri Olsa escreveu:
> Sometimes the hole could be missing, try to bypass
> this issue by comparing last and current offsets.
Can you provide an example of before and after this change? What real
case triggered this?
- Arnaldo
> Signed-off-by: Jiri Olsa <jolsa-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
> dwarves_fprintf.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/dwarves_fprintf.c b/dwarves_fprintf.c
> index 71d8ce9f982b..df80af255c67 100644
> --- a/dwarves_fprintf.c
> +++ b/dwarves_fprintf.c
> @@ -1268,6 +1268,17 @@ size_t class__fprintf(struct class *class, const struct cu *cu,
> }
> pos = tag__class_member(tag_pos);
>
> + /*
> + * Sometimes the hole could be missing, try to bypass
> + * this issue by comparing last and current offsets.
> + */
> + if (last) {
> + uint32_t tmp = last->byte_offset + last->byte_size + last->hole;
> +
> + if (pos->byte_offset > tmp)
> + sum_holes += pos->byte_offset - tmp;
> + }
> +
> if (last != NULL &&
> pos->byte_offset != last->byte_offset &&
> !cconf.suppress_comments)
> --
> 2.4.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe dwarves" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe dwarves" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] dwarves print: Pass into struct conf_fprintf class__fprintf_cacheline_boundary
[not found] ` <1449614826-2278-3-git-send-email-jolsa-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
@ 2015-12-16 18:00 ` Arnaldo Carvalho de Melo
[not found] ` <20151216180017.GC6843-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
0 siblings, 1 reply; 11+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-12-16 18:00 UTC (permalink / raw)
To: Jiri Olsa; +Cc: Joe Mario, dwarves-u79uwXL29TY76Z2rM5mHXA
Em Tue, Dec 08, 2015 at 11:47:05PM +0100, Jiri Olsa escreveu:
> Passing into struct conf_fprintf class__fprintf_cacheline_boundary,
> so the next patch is more easy to read.
"Passing struct conf_fprintf to class__fprintf_cacheline_boundary(), so
the next patch is more easy to read."
?
> Signed-off-by: Jiri Olsa <jolsa-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
> dwarves_fprintf.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/dwarves_fprintf.c b/dwarves_fprintf.c
> index df80af255c67..2d114210831a 100644
> --- a/dwarves_fprintf.c
> +++ b/dwarves_fprintf.c
> @@ -1122,7 +1122,8 @@ static size_t class__fprintf_cacheline_boundary(uint32_t last_cacheline,
> size_t sum, size_t sum_holes,
> uint8_t *newline,
> uint32_t *cacheline,
> - int indent, FILE *fp)
> + struct conf_fprintf *cconf,
> + FILE *fp)
> {
> const size_t real_sum = sum + sum_holes;
> size_t printed = 0;
> @@ -1139,7 +1140,7 @@ static size_t class__fprintf_cacheline_boundary(uint32_t last_cacheline,
> ++printed;
> }
>
> - printed += fprintf(fp, "%.*s", indent, tabs);
> + printed += fprintf(fp, "%.*s", cconf->indent, tabs);
>
> if (cacheline_pos == 0)
> printed += fprintf(fp, "/* --- cacheline %u boundary "
> @@ -1287,7 +1288,7 @@ size_t class__fprintf(struct class *class, const struct cu *cu,
> sum, sum_holes,
> &newline,
> &last_cacheline,
> - cconf.indent,
> + &cconf,
> fp);
> /*
> * These paranoid checks doesn't make much sense on
> @@ -1477,7 +1478,7 @@ size_t class__fprintf(struct class *class, const struct cu *cu,
> sum, sum_holes,
> &newline,
> &last_cacheline,
> - cconf.indent, fp);
> + &cconf, fp);
> if (!cconf.show_only_data_members)
> class__vtable_fprintf(class, cu, &cconf, fp);
>
> --
> 2.4.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe dwarves" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe dwarves" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] dwarves print: Fix holes accounting
[not found] ` <20151216175839.GB6843-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
@ 2015-12-16 18:05 ` Arnaldo Carvalho de Melo
[not found] ` <20151216180532.GD6843-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
0 siblings, 1 reply; 11+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-12-16 18:05 UTC (permalink / raw)
To: Jiri Olsa; +Cc: Joe Mario, dwarves-u79uwXL29TY76Z2rM5mHXA
Em Wed, Dec 16, 2015 at 02:58:39PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Tue, Dec 08, 2015 at 11:47:04PM +0100, Jiri Olsa escreveu:
> > Sometimes the hole could be missing, try to bypass
> > this issue by comparing last and current offsets.
>
> Can you provide an example of before and after this change? What real
> case triggered this?
>
> - Arnaldo
>
> > Signed-off-by: Jiri Olsa <jolsa-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> > ---
> > dwarves_fprintf.c | 11 +++++++++++
> > 1 file changed, 11 insertions(+)
> >
> > diff --git a/dwarves_fprintf.c b/dwarves_fprintf.c
> > index 71d8ce9f982b..df80af255c67 100644
> > --- a/dwarves_fprintf.c
> > +++ b/dwarves_fprintf.c
> > @@ -1268,6 +1268,17 @@ size_t class__fprintf(struct class *class, const struct cu *cu,
> > }
> > pos = tag__class_member(tag_pos);
> >
> > + /*
> > + * Sometimes the hole could be missing, try to bypass
> > + * this issue by comparing last and current offsets.
> > + */
> > + if (last) {
> > + uint32_t tmp = last->byte_offset + last->byte_size + last->hole;
> > +
> > + if (pos->byte_offset > tmp)
> > + sum_holes += pos->byte_offset - tmp;
> > + }
> > +
I mean, how could this be? last->hole then would have been wrong,
miscalculated, looking at where it is calculated now...
- arnaldo
> > if (last != NULL &&
> > pos->byte_offset != last->byte_offset &&
> > !cconf.suppress_comments)
> > --
> > 2.4.3
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe dwarves" in
> > the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe dwarves" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] dwarves print: Fix holes accounting
[not found] ` <20151216180532.GD6843-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
@ 2015-12-16 18:10 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 11+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-12-16 18:10 UTC (permalink / raw)
To: Jiri Olsa; +Cc: Joe Mario, dwarves-u79uwXL29TY76Z2rM5mHXA
Em Wed, Dec 16, 2015 at 03:05:32PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Wed, Dec 16, 2015 at 02:58:39PM -0300, Arnaldo Carvalho de Melo escreveu:
> > Em Tue, Dec 08, 2015 at 11:47:04PM +0100, Jiri Olsa escreveu:
> > > Sometimes the hole could be missing, try to bypass
> > > this issue by comparing last and current offsets.
> > Can you provide an example of before and after this change? What real
> > case triggered this?
<SNIP>
> > > +++ b/dwarves_fprintf.c
> > > @@ -1268,6 +1268,17 @@ size_t class__fprintf(struct class *class, const struct cu *cu,
> > > }
> > > pos = tag__class_member(tag_pos);
> > >
> > > + /*
> > > + * Sometimes the hole could be missing, try to bypass
> > > + * this issue by comparing last and current offsets.
> > > + */
> > > + if (last) {
> > > + uint32_t tmp = last->byte_offset + last->byte_size + last->hole;
> > > +
> > > + if (pos->byte_offset > tmp)
> > > + sum_holes += pos->byte_offset - tmp;
> > > + }
> I mean, how could this be? last->hole then would have been wrong,
> miscalculated, looking at where it is calculated now...
Yeah, class__find_holes() should have properly calculated last->hole so
that (last->byte_offset + last->byte_size + last->hole) ==
pos->byte_offset.
- Arnaldo
--
To unsubscribe from this list: send the line "unsubscribe dwarves" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/3] dwarves print: Keep global cacheline in struct conf_fprintf
[not found] ` <1449614826-2278-4-git-send-email-jolsa-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
@ 2015-12-16 18:29 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 11+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-12-16 18:29 UTC (permalink / raw)
To: Jiri Olsa; +Cc: Joe Mario, dwarves-u79uwXL29TY76Z2rM5mHXA
Em Tue, Dec 08, 2015 at 11:47:06PM +0100, Jiri Olsa escreveu:
> Together with adding cconf->base_offset to sums used
> for cacheline computation, this ensures proper cacheline
> number to be printed for nested struct. Note possitions
> of 'cacheline' lines in following outputs.
>
> Before:
> $ pahole -C task_struct -E ~/kernel/linux-perf-local/vmlinux
> struct task_struct {
> volatile long int state; /* 0 8 */
>
> --- First cacheline is ok
>
> struct task_struct * last_wakee; /* 56 8 */
> /* --- cacheline 1 boundary (64 bytes) --- */
> int wake_cpu; /* 64 4 */
>
> ...
>
> const struct sched_class * sched_class; /* 88 8 */
> struct sched_entity {
> struct load_weight {
> long unsigned int weight; /* 96 8 */
> /* typedef u32 */ unsigned int inv_weight; /* 104 4 */
> } load; /* 96 16 */
>
> --- Second one is still in relative mode and gives wrong
> alignment in wrt task_struct start
>
> unsigned int on_rq; /* 152 4 */
> /* --- cacheline 1 boundary (64 bytes) --- */
Ok, this is a bug...
> /* typedef u64 */ long long unsigned int exec_start; /* 160 8 */
> /* typedef u64 */ long long unsigned int sum_exec_runtime; /* 168 8 */
> ...
>
> After:
> $ pahole -C task_struct -E ~/kernel/linux-perf-local/vmlinux
>
> struct task_struct {
> volatile long int state; /* 0 8 */
>
> --- First cacheline is ok
>
> struct task_struct * last_wakee; /* 56 8 */
> /* --- cacheline 1 boundary (64 bytes) --- */
> int wake_cpu; /* 64 4 */
>
> ...
>
> const struct sched_class * sched_class; /* 88 8 */
> struct sched_entity {
> struct load_weight {
> long unsigned int weight; /* 96 8 */
> /* typedef u32 */ unsigned int inv_weight; /* 104 4 */
> } load; /* 96 16 */
> struct rb_node {
> long unsigned int __rb_parent_color; /* 112 8 */
> struct rb_node * rb_right; /* 120 8 */
> /* --- cacheline 2 boundary (128 bytes) --- */
Which seems fixed here, but...
> struct rb_node * rb_left; /* 128 8 */
> } run_node; /* 112 24 */
>
> --- Second one is still in relative mode and gives wrong
> alignment in wrt task_struct start
... why this one is buggy still? I.e. it should be fixed in such a way
that when we go recursively expanding structs, we know what is the
current cacheline we're on.
- Arnaldo
>
> /* --- cacheline 2 boundary (128 bytes) was 8 bytes ago --- */
> struct list_head {
> struct list_head * next; /* 136 8 */
> struct list_head * prev; /* 144 8 */
> } group_node; /* 136 16 */
> ...
>
> Signed-off-by: Jiri Olsa <jolsa-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
> dwarves.h | 1 +
> dwarves_fprintf.c | 25 ++++++++++---------------
> 2 files changed, 11 insertions(+), 15 deletions(-)
>
> diff --git a/dwarves.h b/dwarves.h
> index 73fec8ac614a..44ebf8384b9c 100644
> --- a/dwarves.h
> +++ b/dwarves.h
> @@ -56,6 +56,7 @@ struct conf_fprintf {
> int32_t type_spacing;
> int32_t name_spacing;
> uint32_t base_offset;
> + uint32_t base_cacheline;
> uint8_t indent;
> uint8_t expand_types:1;
> uint8_t expand_pointers:1;
> diff --git a/dwarves_fprintf.c b/dwarves_fprintf.c
> index 2d114210831a..72d8b08bf235 100644
> --- a/dwarves_fprintf.c
> +++ b/dwarves_fprintf.c
> @@ -1118,22 +1118,21 @@ size_t function__fprintf_stats(const struct tag *tag, const struct cu *cu,
> return printed + fprintf(fp, " */\n");
> }
>
> -static size_t class__fprintf_cacheline_boundary(uint32_t last_cacheline,
> - size_t sum, size_t sum_holes,
> +static size_t class__fprintf_cacheline_boundary(size_t sum, size_t sum_holes,
> uint8_t *newline,
> - uint32_t *cacheline,
> struct conf_fprintf *cconf,
> FILE *fp)
> {
> - const size_t real_sum = sum + sum_holes;
> + const size_t real_sum = sum + sum_holes + cconf->base_offset;
> size_t printed = 0;
> + uint32_t cacheline = real_sum / cacheline_size;
>
> - *cacheline = real_sum / cacheline_size;
> -
> - if (*cacheline > last_cacheline) {
> + if (cacheline != cconf->base_cacheline) {
> const uint32_t cacheline_pos = real_sum % cacheline_size;
> const uint32_t cacheline_in_bytes = real_sum - cacheline_pos;
>
> + cconf->base_cacheline = cacheline;
> +
> if (*newline) {
> fputc('\n', fp);
> *newline = 0;
> @@ -1144,12 +1143,12 @@ static size_t class__fprintf_cacheline_boundary(uint32_t last_cacheline,
>
> if (cacheline_pos == 0)
> printed += fprintf(fp, "/* --- cacheline %u boundary "
> - "(%u bytes) --- */\n", *cacheline,
> + "(%u bytes) --- */\n", cacheline,
> cacheline_in_bytes);
> else
> printed += fprintf(fp, "/* --- cacheline %u boundary "
> "(%u bytes) was %u bytes ago --- "
> - "*/\n", *cacheline,
> + "*/\n", cacheline,
> cacheline_in_bytes, cacheline_pos);
> }
> return printed;
> @@ -1284,10 +1283,8 @@ size_t class__fprintf(struct class *class, const struct cu *cu,
> pos->byte_offset != last->byte_offset &&
> !cconf.suppress_comments)
> printed +=
> - class__fprintf_cacheline_boundary(last_cacheline,
> - sum, sum_holes,
> + class__fprintf_cacheline_boundary(sum, sum_holes,
> &newline,
> - &last_cacheline,
> &cconf,
> fp);
> /*
> @@ -1474,10 +1471,8 @@ size_t class__fprintf(struct class *class, const struct cu *cu,
> }
>
> if (!cconf.suppress_comments)
> - printed += class__fprintf_cacheline_boundary(last_cacheline,
> - sum, sum_holes,
> + printed += class__fprintf_cacheline_boundary(sum, sum_holes,
> &newline,
> - &last_cacheline,
> &cconf, fp);
> if (!cconf.show_only_data_members)
> class__vtable_fprintf(class, cu, &cconf, fp);
> --
> 2.4.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe dwarves" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe dwarves" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] dwarves print: Pass into struct conf_fprintf class__fprintf_cacheline_boundary
[not found] ` <20151216180017.GC6843-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
@ 2015-12-17 9:17 ` Jiri Olsa
0 siblings, 0 replies; 11+ messages in thread
From: Jiri Olsa @ 2015-12-17 9:17 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Jiri Olsa, Joe Mario, dwarves-u79uwXL29TY76Z2rM5mHXA
On Wed, Dec 16, 2015 at 03:00:17PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Tue, Dec 08, 2015 at 11:47:05PM +0100, Jiri Olsa escreveu:
> > Passing into struct conf_fprintf class__fprintf_cacheline_boundary,
> > so the next patch is more easy to read.
>
> "Passing struct conf_fprintf to class__fprintf_cacheline_boundary(), so
> the next patch is more easy to read."
>
> ?
exactly ;-)
jirka
--
To unsubscribe from this list: send the line "unsubscribe dwarves" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2015-12-17 9:17 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-08 22:47 [RFC/PATCH 0/3] dwarves print: Cacheline accounting fixes Jiri Olsa
[not found] ` <1449614826-2278-1-git-send-email-jolsa-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2015-12-08 22:47 ` [PATCH 1/3] dwarves print: Fix holes accounting Jiri Olsa
[not found] ` <1449614826-2278-2-git-send-email-jolsa-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2015-12-16 17:58 ` Arnaldo Carvalho de Melo
[not found] ` <20151216175839.GB6843-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2015-12-16 18:05 ` Arnaldo Carvalho de Melo
[not found] ` <20151216180532.GD6843-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2015-12-16 18:10 ` Arnaldo Carvalho de Melo
2015-12-08 22:47 ` [PATCH 2/3] dwarves print: Pass into struct conf_fprintf class__fprintf_cacheline_boundary Jiri Olsa
[not found] ` <1449614826-2278-3-git-send-email-jolsa-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2015-12-16 18:00 ` Arnaldo Carvalho de Melo
[not found] ` <20151216180017.GC6843-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2015-12-17 9:17 ` Jiri Olsa
2015-12-08 22:47 ` [PATCH 3/3] dwarves print: Keep global cacheline in struct conf_fprintf Jiri Olsa
[not found] ` <1449614826-2278-4-git-send-email-jolsa-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2015-12-16 18:29 ` Arnaldo Carvalho de Melo
2015-12-16 16:57 ` [RFC/PATCH 0/3] dwarves print: Cacheline accounting fixes Jiri Olsa
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).