It's a reasonable function partitioning. How about we define an interface like syslog_channel_s between note and driver? So we can plug in the different transport like syslog.
> -----Original Message----- > From: Nakamura, Yuuichi (Sony) <yuuichi.a.nakam...@sony.com> > Sent: Wednesday, July 1, 2020 3:01 PM > To: dev@nuttx.apache.org > Cc: Nakamura, Yuuichi (Sony) <yuuichi.a.nakam...@sony.com> > Subject: RE: NXView > > Hi all, > > After merging my syscall instrumentation patch into > feature/syscall-instrumentation branch, I had considered how to incorporate my > task trace support into the mainline. > > Currently sched_note.c has the codes to generate notes and buffer management > functions. > notes are generated all the time if configured to be enabled. (attached fig.1) > > In task tracer, I add the filter logic for some note types, and all notes > have to be enabled explicitly. > The buffer management functions are also used by the task tracer, but the > hardware solution doesn't require them. > > So, I propose the new configuration CONFIG_SCHED_INSTRUMENTATION_FILTER in > sched_note.c. > CONFIG_SCHED_INSTRUMENTATION_FILTER only enables the filter logic in each > note types. > And change CONFIG_SCHED_INSTRUMENTATION_BUFFER to make enable only buffer > management logic, not note generation. > (attached fig.2) > > If hardware solution needs only filter logic, by enabling > CONFIG_SCHED_INSTRUMENTATION_FILTER and disabling > CONFIG_SCHED_INSTRUMENTATION_BUFFER can realize it. > sched_note_add() (previously note_add() static function in sched_note.c) is > called when some kernel instrumentation event occured > and it can be implemented to send the note data to the external hardware > device. (attached fig.3) > > The task tracer defines both CONFIG_SCHED_INSTRUMENTATION_FILTER and > CONFIG_SCHED_INSTRUMENTATION_BUFFER. > And if only CONFIG_SCHED_INSTRUMENTATION_BUFFER is defined, the existing > sched_note specification remains. > > How about this proposal ? > I'm fixing my task trace code as the patch of existing sched_note.c and > note_driver.c. > After that, I'd like to send new pull request to > feature/syscall-instrumentation branch for the review. > > Thanks, > Yuuichi Nakamura > > > -----Original Message----- > > From: Nakamura, Yuuichi (Sony) > > Sent: Wednesday, June 17, 2020 2:43 PM > > To: dev@nuttx.apache.org > > Cc: Nakamura, Yuuichi (Sony) <yuuichi.a.nakam...@sony.com> > > Subject: RE: NXView > > > > Thanks for valuable comments. > > I have no objection to your advice that overlapping the similar > > implementation should be avoided. > > Let me make change the current implementation into the extension of > > the existing codes, and if there are any problems in extending, please let > > me discuss again. > > > > Regarding to another issue, the place of the application side code, it > > is because I have wanted to implement trace cmd "<command>" > > It gets the trace while executing the specified command line like > > "time" command of nsh. > > It requires nsh_parse() nshlib internal API. > > > > Thanks, > > Yuuichi Nakamura > > > > > -----Original Message----- > > > From: Gregory Nutt <spudan...@gmail.com> > > > Sent: Wednesday, June 17, 2020 11:13 AM > > > To: dev@nuttx.apache.org > > > Subject: Re: NXView > > > > > > > > > > The major differences are: > > > > > > > > - Different trace data format between the accumulated data in the > > > > memory and > > > /dev/tracer output > > > > It is because to reduce the trace data size in the memory. The > > > > accumulated > > > data contains packed (not aligned) values and > > > > task is recorded by its PID, not the name. The correspondence > > > > between PID > > > and task name string is hold in the separated task name buffer. > > > > On the other hand, the output from /dev/tracer contains aligned > > > > words and > > > contains the task name string for each trace entries. > > > > It is because easy to handle the data by the application code > > > > (nsh trace > > > command). > > > > > > That is a trivial difference and there are some misconceptions. > > > > > > The structures can be packed by simply adding the packed attribute > > > to the structures. That does not justify a redesign. > > > > > > The current implementation does *not* use the task name, it uses the > > > pid. The task name is provided only when the task is created. The > > > provides the associated between pid and name. Thereafter only the pid is > > > uses. > > > > > > Your implementation has two much overlap and should not come > > > upstream as a separate implementation. Extensions and improvements > > > to the existing implementation are welcome, however. > > > > > > Greg
