From: Maynard J. <may...@us...> - 2013-01-04 21:23:28
|
*Andi* or *Suravee*, could one of you please review this patch? Thanks. -Maynard --------------------------------------------------------------------- Fix bug where some invalid unit mask values are accepted as valid Under certain conditions for events that use a bitmask of unit mask values, an invalid unit mask value specified by the user may be incorrectly accepted as valid. Example: i386/sandybridge: event specification l2_rqsts:200000:2 is accepted as valid, whereas there is no unit mask value of '2'. See bug: https://sourceforge.net/tracker/?func=detail&aid=3599359&group_id=16191&atid=116191 for more details. Signed-off-by: Maynard Johnson <may...@us...> --- libop/op_events.c | 15 +++++++-------- 1 files changed, 7 insertions(+), 8 deletions(-) diff --git a/libop/op_events.c b/libop/op_events.c index 6b02fcc..63a7084 100644 --- a/libop/op_events.c +++ b/libop/op_events.c @@ -1015,7 +1015,7 @@ int op_check_events(int ctr, u32 nr, u32 um, op_cpu cpu_type) { int ret = OP_INVALID_EVENT; size_t i; - u32 ctr_mask = 1 << ctr; + u32 masked_val, ctr_mask = 1 << ctr; struct list_head * pos; load_events(cpu_type); @@ -1031,21 +1031,20 @@ int op_check_events(int ctr, u32 nr, u32 um, op_cpu cpu_type) ret |= OP_INVALID_COUNTER; if (event->unit->unit_type_mask == utm_bitmask) { - for (i = 0; i < event->unit->num; ++i) - um &= ~(event->unit->um[i].value); - - if (um) + masked_val = 0; + for (i = 0; i < event->unit->num; i++) { + if ((event->unit->um[i].value & um) == event->unit->um[i].value) + masked_val |= event->unit->um[i].value; + } + if (!(masked_val == um && um != 0)) ret |= OP_INVALID_UM; - } else { for (i = 0; i < event->unit->num; ++i) { if (event->unit->um[i].value == um) break; } - if (i == event->unit->num) ret |= OP_INVALID_UM; - } if (ret == OP_OK_EVENT) -- 1.7.1 |
From: Maynard J. <may...@us...> - 2013-01-14 16:00:55
|
On 01/04/2013 03:23 PM, Maynard Johnson wrote: > *Andi* or *Suravee*, could one of you please review this patch? > No responses after 10 days of waiting for feedback, so I've committed this patch now. -Maynard > Thanks. > > -Maynard > > --------------------------------------------------------------------- > Fix bug where some invalid unit mask values are accepted as valid > > Under certain conditions for events that use a bitmask of unit mask > values, an invalid unit mask value specified by the user may be > incorrectly accepted as valid. > > Example: > i386/sandybridge: event specification l2_rqsts:200000:2 > is accepted as valid, whereas there is no unit mask value of '2'. > > See bug: > https://sourceforge.net/tracker/?func=detail&aid=3599359&group_id=16191&atid=116191 > for more details. > > Signed-off-by: Maynard Johnson <may...@us...> > --- > libop/op_events.c | 15 +++++++-------- > 1 files changed, 7 insertions(+), 8 deletions(-) > > diff --git a/libop/op_events.c b/libop/op_events.c > index 6b02fcc..63a7084 100644 > --- a/libop/op_events.c > +++ b/libop/op_events.c > @@ -1015,7 +1015,7 @@ int op_check_events(int ctr, u32 nr, u32 um, op_cpu cpu_type) > { > int ret = OP_INVALID_EVENT; > size_t i; > - u32 ctr_mask = 1 << ctr; > + u32 masked_val, ctr_mask = 1 << ctr; > struct list_head * pos; > > load_events(cpu_type); > @@ -1031,21 +1031,20 @@ int op_check_events(int ctr, u32 nr, u32 um, op_cpu cpu_type) > ret |= OP_INVALID_COUNTER; > > if (event->unit->unit_type_mask == utm_bitmask) { > - for (i = 0; i < event->unit->num; ++i) > - um &= ~(event->unit->um[i].value); > - > - if (um) > + masked_val = 0; > + for (i = 0; i < event->unit->num; i++) { > + if ((event->unit->um[i].value & um) == event->unit->um[i].value) > + masked_val |= event->unit->um[i].value; > + } > + if (!(masked_val == um && um != 0)) > ret |= OP_INVALID_UM; > - > } else { > for (i = 0; i < event->unit->num; ++i) { > if (event->unit->um[i].value == um) > break; > } > - > if (i == event->unit->num) > ret |= OP_INVALID_UM; > - > } > > if (ret == OP_OK_EVENT) |
From: Andi K. <an...@fi...> - 2013-01-14 16:07:49
|
On Mon, Jan 14, 2013 at 09:58:15AM -0600, Maynard Johnson wrote: > On 01/04/2013 03:23 PM, Maynard Johnson wrote: > > *Andi* or *Suravee*, could one of you please review this patch? > > > No responses after 10 days of waiting for feedback, so I've committed this patch now. Patch is ok for me. -Andi > > -Maynard > > Thanks. > > > > -Maynard > > > > --------------------------------------------------------------------- > > Fix bug where some invalid unit mask values are accepted as valid > > > > Under certain conditions for events that use a bitmask of unit mask > > values, an invalid unit mask value specified by the user may be > > incorrectly accepted as valid. > > > > Example: > > i386/sandybridge: event specification l2_rqsts:200000:2 > > is accepted as valid, whereas there is no unit mask value of '2'. > > > > See bug: > > https://sourceforge.net/tracker/?func=detail&aid=3599359&group_id=16191&atid=116191 > > for more details. > > > > Signed-off-by: Maynard Johnson <may...@us...> > > --- > > libop/op_events.c | 15 +++++++-------- > > 1 files changed, 7 insertions(+), 8 deletions(-) > > > > diff --git a/libop/op_events.c b/libop/op_events.c > > index 6b02fcc..63a7084 100644 > > --- a/libop/op_events.c > > +++ b/libop/op_events.c > > @@ -1015,7 +1015,7 @@ int op_check_events(int ctr, u32 nr, u32 um, op_cpu cpu_type) > > { > > int ret = OP_INVALID_EVENT; > > size_t i; > > - u32 ctr_mask = 1 << ctr; > > + u32 masked_val, ctr_mask = 1 << ctr; > > struct list_head * pos; > > > > load_events(cpu_type); > > @@ -1031,21 +1031,20 @@ int op_check_events(int ctr, u32 nr, u32 um, op_cpu cpu_type) > > ret |= OP_INVALID_COUNTER; > > > > if (event->unit->unit_type_mask == utm_bitmask) { > > - for (i = 0; i < event->unit->num; ++i) > > - um &= ~(event->unit->um[i].value); > > - > > - if (um) > > + masked_val = 0; > > + for (i = 0; i < event->unit->num; i++) { > > + if ((event->unit->um[i].value & um) == event->unit->um[i].value) > > + masked_val |= event->unit->um[i].value; > > + } > > + if (!(masked_val == um && um != 0)) > > ret |= OP_INVALID_UM; > > - > > } else { > > for (i = 0; i < event->unit->num; ++i) { > > if (event->unit->um[i].value == um) > > break; > > } > > - > > if (i == event->unit->num) > > ret |= OP_INVALID_UM; > > - > > } > > > > if (ret == OP_OK_EVENT) > -- ak...@li... -- Speaking for myself only. |
From: Suthikulpanit, S. <Sur...@am...> - 2013-01-14 17:10:53
|
Could you please simplify the logic: if ( !(masked_val == um && um != 0)) to: if ((masked_val != um) || (um == 0))) Suravee. -----Original Message----- From: Maynard Johnson [mailto:may...@us...] Sent: Monday, January 14, 2013 9:58 AM To: oprofile-list; Suthikulpanit, Suravee; Andi Kleen Subject: Re: [PATCH] Fix bug where some invalid unit mask values are accepted as valid On 01/04/2013 03:23 PM, Maynard Johnson wrote: > *Andi* or *Suravee*, could one of you please review this patch? > No responses after 10 days of waiting for feedback, so I've committed this patch now. -Maynard > Thanks. > > -Maynard > > --------------------------------------------------------------------- > Fix bug where some invalid unit mask values are accepted as valid > > Under certain conditions for events that use a bitmask of unit mask > values, an invalid unit mask value specified by the user may be > incorrectly accepted as valid. > > Example: > i386/sandybridge: event specification l2_rqsts:200000:2 is accepted > as valid, whereas there is no unit mask value of '2'. > > See bug: > https://sourceforge.net/tracker/?func=detail&aid=3599359&group_id=1619 > 1&atid=116191 > for more details. > > Signed-off-by: Maynard Johnson <may...@us...> > --- > libop/op_events.c | 15 +++++++-------- > 1 files changed, 7 insertions(+), 8 deletions(-) > > diff --git a/libop/op_events.c b/libop/op_events.c index > 6b02fcc..63a7084 100644 > --- a/libop/op_events.c > +++ b/libop/op_events.c > @@ -1015,7 +1015,7 @@ int op_check_events(int ctr, u32 nr, u32 um, > op_cpu cpu_type) { > int ret = OP_INVALID_EVENT; > size_t i; > - u32 ctr_mask = 1 << ctr; > + u32 masked_val, ctr_mask = 1 << ctr; > struct list_head * pos; > > load_events(cpu_type); > @@ -1031,21 +1031,20 @@ int op_check_events(int ctr, u32 nr, u32 um, op_cpu cpu_type) > ret |= OP_INVALID_COUNTER; > > if (event->unit->unit_type_mask == utm_bitmask) { > - for (i = 0; i < event->unit->num; ++i) > - um &= ~(event->unit->um[i].value); > - > - if (um) > + masked_val = 0; > + for (i = 0; i < event->unit->num; i++) { > + if ((event->unit->um[i].value & um) == event->unit->um[i].value) > + masked_val |= event->unit->um[i].value; > + } > + if (!(masked_val == um && um != 0)) > ret |= OP_INVALID_UM; > - > } else { > for (i = 0; i < event->unit->num; ++i) { > if (event->unit->um[i].value == um) > break; > } > - > if (i == event->unit->num) > ret |= OP_INVALID_UM; > - > } > > if (ret == OP_OK_EVENT) ------------------------------------------------------------------------------ Master Visual Studio, SharePoint, SQL, ASP.NET, C# 2012, HTML5, CSS, MVC, Windows 8 Apps, JavaScript and much more. Keep your skills current with LearnDevNow - 3,200 step-by-step video tutorials by Microsoft MVPs and experts. SALE $99.99 this month only -- learn more at: http://p.sf.net/sfu/learnmore_122412 _______________________________________________ oprofile-list mailing list opr...@li... https://lists.sourceforge.net/lists/listinfo/oprofile-list |