Working on -e trace

[haris iqbal]

On Thu, Mar 10, 2016 at 5:24 AM, Dmitry V. Levin <ldv at altlinux.org> wrote:
> On Wed, Mar 09, 2016 at 11:02:52AM +0530, haris iqbal wrote:
> [...]
>> Sorry for the multiple commits. My vim is giving trouble.
>
> Please don't send incremental commits, just amend and send the one you
> already have.

I did not quite understand this. So, I make changes and send the older
commit log? How will the changes be seen by you guys then?

>
>> >> @@ -264,8 +264,8 @@
>> >>  [271] = { 6,    0,        SEN(process_vm_writev),
>> >> "process_vm_writev"    },
>> >>  [272] = { 5,    0,        SEN(kcmp),            "kcmp"            },
>> >>  [273] = { 3,    TD,        SEN(finit_module),        "finit_module"        },
>> >> -[274] = { 3,    0,        SEN(sched_setattr),        "sched_setattr"        },
>> >> -[275] = { 4,    0,        SEN(sched_getattr),        "sched_getattr"        },
>> >> +[274] = { 3,    TSC,        SEN(sched_setattr),
>> >> "sched_setattr"        },
>> >> +[275] = { 4,    TSC,        SEN(sched_getattr),
>> >> "sched_getattr"        },
>> >>  [276] = { 5,    TD|TF,        SEN(renameat2),            "renameat2"        },
>> >>  [277] = { 3,    0,        SEN(seccomp),            "seccomp",        },
>> >>  [278] = { 3,    0,        SEN(getrandom),            "getrandom",        },
>> >
>> > I assume all changes to syscallent files were made by some simple script.
>> > Please include this script into commit message.
>>
>> The python script that I used was
>>
>> import os
>> rootdir = '/home/student/junk/test_strace'
>>
>> seperator = ','
>> for subdir, dirs, files in os.walk(rootdir):
>>     for file in files:
>>         temp = os.path.join(subdir, file)
>>         if(temp.find("syscallent") != -1):
>>             print
>> "------------------------------------------------------------------\n"
>>             print temp
>>
>>             with open(temp, 'r') as file:
>>                 lines = file.readlines()
>>
>>             for i in range(len(lines)):
>>                 if(lines[i].find("sched") != -1):
>>                     print lines[i]
>>                     parts = lines[i].split(seperator)
>>
>>                     parts[1] = '\tTSC'
>>
>>                     lines[i] = seperator.join(parts)
>>                     print lines[i]
>>
>>
>>             with open(temp, 'w') as file:
>>                 file.writelines( lines )
>
> I've checked that
> git grep -Fl 'SEN(sched_' linux/ |xargs sed -i 's/0\(,[[:space:]]*SEN(sched_\)/TSC\1/'
> produces exactly the same result as your patch.
>
> I'd say my sed script is a bit simpler than your python script, though. ;)

Yes, But for me, python is simpler. :)

>
>> >> --- a/syscall.c
>> >> +++ b/syscall.c
>> >> @@ -78,6 +78,7 @@
>> >>  #define TP TRACE_PROCESS
>> >>  #define TS TRACE_SIGNAL
>> >>  #define TM TRACE_MEMORY
>> >> +#define TSC TRACE_SCHED
>> >>  #define NF SYSCALL_NEVER_FAILS
>> >>  #define MA MAX_ARGS
>> >>  #define SI STACKTRACE_INVALIDATE_CACHE
>> >> @@ -500,6 +501,8 @@ lookup_class(const char *s)
>> >>          return TRACE_DESC;
>> >>      if (strcmp(s, "memory") == 0)
>> >>          return TRACE_MEMORY;
>> >> +    if (strcmp(s, "scheduler") == 0)
>> >> +        return TRACE_SCHED;
>> >>      return -1;
>> >>  }
>> >
>> > Please follow the indentation style.
>> > BTW, why "scheduler"?  Could you give a rationale for this name?
>>
>> I thought as all the system calls are one or the other related to the
>> scheduler. We can also consider the description "syscalls relate to
>> scheduling.
>
> There is a subtle difference between a scheduler (an actor) and a set
> of syscalls related to scheduling (an action) than might lead to confusion
> in the future.  That's why I still think that "%sched" would be better in
> long run than "scheduler" or "scheduling".

True, but my concern was we cannot change the name of this new one to
"%sched", while the older ones are still following some other naming
convention. What can be done is, we can decide on the names for all
the existing options (for file, memory, ..) and then change all in one
commit.

>
> Anyway, a user-visible change like this has to be properly documented with
> an entry in strace.1 and a line in NEWS file.

Done.

commit 78b22b92e775372e8cf21afaf66ae4aa84f4fedc
Author: student <haris.phnx at gmail.com>
Date:   Thu Mar 10 23:38:21 2016 +0530

    Added documentation in strace.1 and NEWS files

diff --git a/NEWS b/NEWS
index 7779f65..6e71b99 100644
--- a/NEWS
+++ b/NEWS
@@ -12,6 +12,7 @@ Noteworthy changes in release ?.?? (????-??-??)
     spent in decoding socket details in -yy mode.
   * Implemented decoding of copy_file_range syscall.
   * Implemented dumping of vmsplice syscall.
+  * Added feature to trace scheduler related system calls with option -e.

 * Bug fixes
   * Fixed build on arc, metag, nios2, or1k, and tile architectures.
diff --git a/strace.1 b/strace.1
index 429cdfc..5a2bdf5 100644
--- a/strace.1
+++ b/strace.1
@@ -423,6 +423,9 @@ Trace all file descriptor related system calls.
 .BR "\-e\ trace" = memory
 Trace all memory mapping related system calls.
 .TP
+.BR "\-e\ trace" = scheduler
+Trace all scheduler related system calls.
+.TP
 \fB\-e\ abbrev\fR=\,\fIset\fR
 Abbreviate the output from printing each member of large structures.
 The default is


>
>
> --
> ldv
>
> ------------------------------------------------------------------------------
> Transform Data into Opportunity.
> Accelerate data analysis in your applications with
> Intel Data Analytics Acceleration Library.
> Click to learn more.
> http://pubads.g.doubleclick.net/gampad/clk?id=278785111&iu=/4140
> _______________________________________________
> Strace-devel mailing list
> Strace-devel at lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/strace-devel
>



-- 

With regards,

Md Haris Iqbal,
Placement Coordinator, MTech IT
NITK Surathkal,
Contact: +91 8861996962