| Message ID | 20190109130945.28519-7-ykaradzhov@vmware.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Modifications toward KS 1.0 | expand |
On Wed, 9 Jan 2019 15:09:45 +0200 Yordan Karadzhov <ykaradzhov@vmware.com> wrote: > Plugin-provided actions are executed when loading the data. These > actions can be used to modify the contain of the kshark_entries "modify the contain of" ? > generated by a given event type and we consider the case of having > more than one plugin-provided actions per event type. However, the code "more than one plugin-provided actions per event type." also doesn't make sense. > that handles the case of multiple actions per-event has a bug. The "if" > was introduced with the idea that only the last per-event action will > modify the KS_PLUGIN_UNTOUCHED flag of the entry, but it misbehaves in > the case of a single per-event action in the list, followed by actions > for other events. Patch looks fine, but can you resend with a cleaner change log? Thanks! -- Steve > > Signed-off-by: Yordan Karadzhov <ykaradzhov@vmware.com> > --- > kernel-shark-qt/src/libkshark.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/kernel-shark-qt/src/libkshark.c b/kernel-shark-qt/src/libkshark.c > index 598ea52..9ab2d57 100644 > --- a/kernel-shark-qt/src/libkshark.c > +++ b/kernel-shark-qt/src/libkshark.c > @@ -750,8 +750,7 @@ static size_t get_records(struct kshark_context *kshark_ctx, > entry->event_id))) { > evt_handler->event_func(kshark_ctx, rec, entry); > evt_handler = evt_handler->next; > - if (!evt_handler) > - entry->visible &= ~KS_PLUGIN_UNTOUCHED_MASK; > + entry->visible &= ~KS_PLUGIN_UNTOUCHED_MASK; > } > > pid = entry->pid;
Ping? -- Steve On Wed, 9 Jan 2019 11:52:53 -0500 Steven Rostedt <rostedt@goodmis.org> wrote: > On Wed, 9 Jan 2019 15:09:45 +0200 > Yordan Karadzhov <ykaradzhov@vmware.com> wrote: > > > Plugin-provided actions are executed when loading the data. These > > actions can be used to modify the contain of the kshark_entries > > "modify the contain of" ? > > > generated by a given event type and we consider the case of having > > more than one plugin-provided actions per event type. However, the code > > "more than one plugin-provided actions per event type." also doesn't > make sense. > > > that handles the case of multiple actions per-event has a bug. The "if" > > was introduced with the idea that only the last per-event action will > > modify the KS_PLUGIN_UNTOUCHED flag of the entry, but it misbehaves in > > the case of a single per-event action in the list, followed by actions > > for other events. > > Patch looks fine, but can you resend with a cleaner change log? > > Thanks! > > -- Steve > > > > > Signed-off-by: Yordan Karadzhov <ykaradzhov@vmware.com> > > --- > > kernel-shark-qt/src/libkshark.c | 3 +-- > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > diff --git a/kernel-shark-qt/src/libkshark.c b/kernel-shark-qt/src/libkshark.c > > index 598ea52..9ab2d57 100644 > > --- a/kernel-shark-qt/src/libkshark.c > > +++ b/kernel-shark-qt/src/libkshark.c > > @@ -750,8 +750,7 @@ static size_t get_records(struct kshark_context *kshark_ctx, > > entry->event_id))) { > > evt_handler->event_func(kshark_ctx, rec, entry); > > evt_handler = evt_handler->next; > > - if (!evt_handler) > > - entry->visible &= ~KS_PLUGIN_UNTOUCHED_MASK; > > + entry->visible &= ~KS_PLUGIN_UNTOUCHED_MASK; > > } > > > > pid = entry->pid; >
On Thu, 14 Mar 2019 20:01:14 -0400 Steven Rostedt <rostedt@goodmis.org> wrote: > Ping? Actually this is marked as superseded in patchwork. Is it? -- Steve > > On Wed, 9 Jan 2019 11:52:53 -0500 > Steven Rostedt <rostedt@goodmis.org> wrote: > > > On Wed, 9 Jan 2019 15:09:45 +0200 > > Yordan Karadzhov <ykaradzhov@vmware.com> wrote: > > > > > Plugin-provided actions are executed when loading the data. These > > > actions can be used to modify the contain of the kshark_entries > > > > "modify the contain of" ? > > > > > generated by a given event type and we consider the case of having > > > more than one plugin-provided actions per event type. However, the code > > > > "more than one plugin-provided actions per event type." also doesn't > > make sense. > > > > > that handles the case of multiple actions per-event has a bug. The "if" > > > was introduced with the idea that only the last per-event action will > > > modify the KS_PLUGIN_UNTOUCHED flag of the entry, but it misbehaves in > > > the case of a single per-event action in the list, followed by actions > > > for other events. > > > > Patch looks fine, but can you resend with a cleaner change log? > > > > Thanks! > > > > -- Steve > > > > > > > > Signed-off-by: Yordan Karadzhov <ykaradzhov@vmware.com> > > > --- > > > kernel-shark-qt/src/libkshark.c | 3 +-- > > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > > > diff --git a/kernel-shark-qt/src/libkshark.c b/kernel-shark-qt/src/libkshark.c > > > index 598ea52..9ab2d57 100644 > > > --- a/kernel-shark-qt/src/libkshark.c > > > +++ b/kernel-shark-qt/src/libkshark.c > > > @@ -750,8 +750,7 @@ static size_t get_records(struct kshark_context *kshark_ctx, > > > entry->event_id))) { > > > evt_handler->event_func(kshark_ctx, rec, entry); > > > evt_handler = evt_handler->next; > > > - if (!evt_handler) > > > - entry->visible &= ~KS_PLUGIN_UNTOUCHED_MASK; > > > + entry->visible &= ~KS_PLUGIN_UNTOUCHED_MASK; > > > } > > > > > > pid = entry->pid; > >
On 15.03.19 г. 2:05 ч., Steven Rostedt wrote: > On Thu, 14 Mar 2019 20:01:14 -0400 > Steven Rostedt <rostedt@goodmis.org> wrote: > >> Ping? > > Actually this is marked as superseded in patchwork. Is it? > I think this one was been pushed https://git.kernel.org/pub/scm/utils/trace-cmd/trace-cmd.git/commit/?id=fbba6a95533db0d42e6e9a9241fda3616b8f03a0 Thanks! Yordan > -- Steve > >> >> On Wed, 9 Jan 2019 11:52:53 -0500 >> Steven Rostedt <rostedt@goodmis.org> wrote: >> >>> On Wed, 9 Jan 2019 15:09:45 +0200 >>> Yordan Karadzhov <ykaradzhov@vmware.com> wrote: >>> >>>> Plugin-provided actions are executed when loading the data. These >>>> actions can be used to modify the contain of the kshark_entries >>> >>> "modify the contain of" ? >>> >>>> generated by a given event type and we consider the case of having >>>> more than one plugin-provided actions per event type. However, the code >>> >>> "more than one plugin-provided actions per event type." also doesn't >>> make sense. >>> >>>> that handles the case of multiple actions per-event has a bug. The "if" >>>> was introduced with the idea that only the last per-event action will >>>> modify the KS_PLUGIN_UNTOUCHED flag of the entry, but it misbehaves in >>>> the case of a single per-event action in the list, followed by actions >>>> for other events. >>> >>> Patch looks fine, but can you resend with a cleaner change log? >>> >>> Thanks! >>> >>> -- Steve >>> >>>> >>>> Signed-off-by: Yordan Karadzhov <ykaradzhov@vmware.com> >>>> --- >>>> kernel-shark-qt/src/libkshark.c | 3 +-- >>>> 1 file changed, 1 insertion(+), 2 deletions(-) >>>> >>>> diff --git a/kernel-shark-qt/src/libkshark.c b/kernel-shark-qt/src/libkshark.c >>>> index 598ea52..9ab2d57 100644 >>>> --- a/kernel-shark-qt/src/libkshark.c >>>> +++ b/kernel-shark-qt/src/libkshark.c >>>> @@ -750,8 +750,7 @@ static size_t get_records(struct kshark_context *kshark_ctx, >>>> entry->event_id))) { >>>> evt_handler->event_func(kshark_ctx, rec, entry); >>>> evt_handler = evt_handler->next; >>>> - if (!evt_handler) >>>> - entry->visible &= ~KS_PLUGIN_UNTOUCHED_MASK; >>>> + entry->visible &= ~KS_PLUGIN_UNTOUCHED_MASK; >>>> } >>>> >>>> pid = entry->pid; >>> >
diff --git a/kernel-shark-qt/src/libkshark.c b/kernel-shark-qt/src/libkshark.c index 598ea52..9ab2d57 100644 --- a/kernel-shark-qt/src/libkshark.c +++ b/kernel-shark-qt/src/libkshark.c @@ -750,8 +750,7 @@ static size_t get_records(struct kshark_context *kshark_ctx, entry->event_id))) { evt_handler->event_func(kshark_ctx, rec, entry); evt_handler = evt_handler->next; - if (!evt_handler) - entry->visible &= ~KS_PLUGIN_UNTOUCHED_MASK; + entry->visible &= ~KS_PLUGIN_UNTOUCHED_MASK; } pid = entry->pid;
Plugin-provided actions are executed when loading the data. These actions can be used to modify the contain of the kshark_entries generated by a given event type and we consider the case of having more than one plugin-provided actions per event type. However, the code that handles the case of multiple actions per-event has a bug. The "if" was introduced with the idea that only the last per-event action will modify the KS_PLUGIN_UNTOUCHED flag of the entry, but it misbehaves in the case of a single per-event action in the list, followed by actions for other events. Signed-off-by: Yordan Karadzhov <ykaradzhov@vmware.com> --- kernel-shark-qt/src/libkshark.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)