From: Robert R. <rob...@am...> - 2012-08-24 18:54:01
|
2 small patches sent out for review. -Robert Robert Richter (2): oprofile, s390: Fix uninitialized memory access when writing to oprofilefs oprofile: Remove 'WQ on CPUx, prefer CPUy' warning arch/s390/oprofile/init.c | 10 +++++----- drivers/oprofile/cpu_buffer.c | 11 +++-------- 2 files changed, 8 insertions(+), 13 deletions(-) -- 1.7.8.6 |
From: Robert R. <rob...@am...> - 2009-08-12 15:54:00
|
This patch series contains fixes and updates I still have in my oprofile userland patch queue. I posted some of them already some months ago. There are mostly reworks of opcontrol, some changes to make binary package creation and testing easier and some xml stuff. I find all of them useful, the patches are necessary for my regression. I think they are useful for others too. -Robert |
From: Robert R. <rob...@am...> - 2009-08-12 15:54:02
|
Output was broken due to unquoted double quotes. Signed-off-by: Robert Richter <rob...@am...> --- ChangeLog | 4 ++++ utils/opcontrol | 5 +++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/ChangeLog b/ChangeLog index 10c54cb..0da5c8d 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,9 @@ 2009-08-11 Robert Richter <rob...@am...> + * utils/opcontrol: fix help message + +2009-08-11 Robert Richter <rob...@am...> + * utils/opcontrol: fix parameter dump 2009-08-11 Robert Richter <rob...@am...> diff --git a/utils/opcontrol b/utils/opcontrol index fc92ac7..f1da6dc 100644 --- a/utils/opcontrol +++ b/utils/opcontrol @@ -106,7 +106,8 @@ is_tool_available() # print help message do_help() { - echo "opcontrol: usage: + cat >&2 <<EOF +opcontrol: usage: -l/--list-events list event types and unit masks -?/--help this message -v/--version show version @@ -162,7 +163,7 @@ do_help() --xen Xen image (for Xen only) --active-domains=<list> List of domains in profiling session (for Xen only) (list contains domain ids separated by commas) -" >&2 +EOF } -- 1.6.4 |
From: Robert R. <rob...@am...> - 2009-08-12 15:54:04
|
When using option --deinit the functions do_stop() and do_kill_daemon() are only called, if the daemon is running. This prevents the irritating message 'Daemon not running' for an init/deinit sequence. Signed-off-by: Robert Richter <rob...@am...> --- ChangeLog | 4 ++++ utils/opcontrol | 6 ++++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/ChangeLog b/ChangeLog index 0da5c8d..e0adc98 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,9 @@ 2009-08-11 Robert Richter <rob...@am...> + * utils/opcontrol: fix deinit, kill daemon only if running + +2009-08-11 Robert Richter <rob...@am...> + * utils/opcontrol: fix help message 2009-08-11 Robert Richter <rob...@am...> diff --git a/utils/opcontrol b/utils/opcontrol index f1da6dc..5a4e948 100644 --- a/utils/opcontrol +++ b/utils/opcontrol @@ -729,8 +729,10 @@ do_options() --deinit) DUMP=yes - STOP=yes - KILL_DAEMON=yes + test ! -f "$LOCK_FILE" || { + STOP=yes + KILL_DAEMON=yes + } DEINIT=yes EXCLUSIVE_ARGC=`expr $EXCLUSIVE_ARGC + 1` EXCLUSIVE_ARGV="$arg" -- 1.6.4 |
From: Robert R. <rob...@am...> - 2009-08-12 15:54:05
|
The parameter dump was located in do_init(). Values were printed as read from the setup file. Parameters that have been updated with a setup option were wrong. This patch fixes this. Now, parameters are also dumped if there are no options specified for --setup. Thus, parameters stored in the setup file can be dumped using 'opcontrol --setup --verbose'. Signed-off-by: Robert Richter <rob...@am...> --- ChangeLog | 4 ++++ utils/opcontrol | 10 ++++++++++ 2 files changed, 14 insertions(+), 0 deletions(-) diff --git a/ChangeLog b/ChangeLog index ce81dc7..10c54cb 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,9 @@ 2009-08-11 Robert Richter <rob...@am...> + * utils/opcontrol: fix parameter dump + +2009-08-11 Robert Richter <rob...@am...> + * utils/opcontrol: fix vecho output 2009-07-31 Maynard Johnson <may...@us...> diff --git a/utils/opcontrol b/utils/opcontrol index fbea34d..fc92ac7 100644 --- a/utils/opcontrol +++ b/utils/opcontrol @@ -404,6 +404,8 @@ do_save_setup() if test "$XEN_RANGE"; then echo "XEN_RANGE=$XEN_RANGE" >> $SETUP_FILE fi + + dump_parameters; } @@ -865,6 +867,10 @@ do_options() fi if test "$SETUP" = "yes" -a "$DO_SETUP" != "yes"; then + if test -n "$VERBOSE"; then + dump_parameters + exit 0 + fi echo "No options specified for --setup." >&2 exit 1 fi @@ -879,7 +885,10 @@ do_options() exit 1 fi fi +} +dump_parameters() +{ vecho "Parameters used:" vecho "SESSION_DIR $SESSION_DIR" vecho "LOCK_FILE $LOCK_FILE" @@ -1424,6 +1433,7 @@ do_start_daemon() fi fi + dump_parameters do_setup check_valid_args get_image_range "linux" -- 1.6.4 |
From: Robert R. <rob...@am...> - 2009-08-12 15:54:05
|
There are unreachable vecho commands in the code that are never executed since the options are parsed later. I added --verbose to check_options_early(). Additional I removed the check that allows the use of the --verbose option only for the --start or --start-daemon. Signed-off-by: Robert Richter <rob...@am...> --- ChangeLog | 4 ++++ utils/opcontrol | 28 ++++++++++++---------------- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/ChangeLog b/ChangeLog index 5c77afb..ce81dc7 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,7 @@ +2009-08-11 Robert Richter <rob...@am...> + + * utils/opcontrol: fix vecho output + 2009-07-31 Maynard Johnson <may...@us...> * configure.in: bump version in AM_INIT_AUTOMAKE to 0.9.6cvs diff --git a/utils/opcontrol b/utils/opcontrol index 41c8f0e..fbea34d 100644 --- a/utils/opcontrol +++ b/utils/opcontrol @@ -291,7 +291,6 @@ do_init() NOTE_SIZE=0 VMLINUX= XENIMAGE="none" - VERBOSE="" SEPARATE_LIB=0 SEPARATE_KERNEL=0 SEPARATE_THREAD=0 @@ -839,20 +838,16 @@ do_options() DO_SETUP=yes ;; - -V|--verbose) - if test -z "$val"; then - VERBOSE="all" - else - VERBOSE=$val - fi - ;; - -l|--list-events) EXCLUSIVE_ARGC=`expr $EXCLUSIVE_ARGC + 1` EXCLUSIVE_ARGV="$arg" exec $OPHELP ;; + # ignore early option + -V|--verbose) + ;; + *) echo "Unknown option \"$arg\". See opcontrol --help" >&2 exit 1 @@ -874,13 +869,6 @@ do_options() exit 1 fi - if test -n "$VERBOSE"; then - if test "$START" != "yes" -a "$START_DAEMON" != "yes"; then - echo "Option --verbose may only be used with --start or --start-daemon" >&2 - exit 1 - fi - fi - if test "$DO_SETUP" = "yes"; then SETUP="$DO_SETUP" fi @@ -1756,6 +1744,7 @@ check_options_early() { OPHELP="$OPDIR/ophelp" + VERBOSE="" for i in $@; do # added to handle arg=val parameters @@ -1772,6 +1761,13 @@ check_options_early() $OPHELP --version | cut -d' ' -f2- exit 0 ;; + -V|--verbose) + if test -z "$val"; then + VERBOSE="all" + else + VERBOSE=$val + fi + ;; --session-dir) error_if_empty $arg $val SESSION_DIR="$val" -- 1.6.4 |
From: Robert R. <rob...@am...> - 2009-08-12 15:54:09
|
I figured out that there is no way to reset setup parameters. The only way was removing the setup file in /root/.oprofile/daemonrc. So I added the defaults option that removes the file. If there is no setup file the default values will be set. The option can be combined with other setup options such as: opcontrol --defaults --no-vmlinux --buffer-size=640000 This resets all values to default except buffer size and vmlinux. Signed-off-by: Robert Richter <rob...@am...> --- ChangeLog | 6 ++++++ doc/opcontrol.1.in | 5 +++++ doc/oprofile.xml | 7 +++++++ utils/opcontrol | 13 +++++++++++++ 4 files changed, 31 insertions(+), 0 deletions(-) diff --git a/ChangeLog b/ChangeLog index 6fe6e5e..984e936 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,11 @@ 2009-08-11 Robert Richter <rob...@am...> + * doc/opcontrol.1.in: + * doc/oprofile.xml: + * utils/opcontrol: adding --defaults option + +2009-08-11 Robert Richter <rob...@am...> + * agents/Makefile.am: * configure.in: adding config option to disable oprofile user check diff --git a/doc/opcontrol.1.in b/doc/opcontrol.1.in index b9cf61e..8b68746 100644 --- a/doc/opcontrol.1.in +++ b/doc/opcontrol.1.in @@ -41,6 +41,11 @@ in ~root/.oprofile/daemonrc. Optional. Show configuration information. .br .TP +.BI "--defaults" +Reset setup values to defaults. Can be combined with the --setup +option. +.br +.TP .BI "--start-daemon" Start the oprofile daemon without starting profiling. Not available in 2.2/2.4 kernels. diff --git a/doc/oprofile.xml b/doc/oprofile.xml index 35ba48a..1a06bcd 100644 --- a/doc/oprofile.xml +++ b/doc/oprofile.xml @@ -567,6 +567,13 @@ The <command>opcontrol</command> script provides the following actions : </para></listitem> </varlistentry> <varlistentry> + <term><option>--defaults</option></term> + <listitem><para> + Reset setup values to defaults. Can be combined + with the <option>--setup</option> option. + </para></listitem> + </varlistentry> + <varlistentry> <term><option>--status</option></term> <listitem><para> Show configuration information. diff --git a/utils/opcontrol b/utils/opcontrol index 6d1bfc0..0ada540 100644 --- a/utils/opcontrol +++ b/utils/opcontrol @@ -113,6 +113,7 @@ opcontrol: usage: -v/--version show version --init loads the oprofile module and oprofilefs --setup give setup arguments (may be omitted) + --defaults reset setup values to defaults --status show configuration --start-daemon start daemon without starting profiling -s/--start start data collection @@ -241,6 +242,9 @@ do_init_daemon_vars() if test -n "$SESSION_DIR"; then SAVED=$SESSION_DIR fi + if test "$DEFAULTS" = "yes"; then + rm -f "$SETUP_FILE" + fi do_load_setup if test -n "$SAVED"; then SESSION_DIR=$SAVED @@ -665,6 +669,11 @@ do_options() SETUP=yes ;; + --defaults) + # also early option code existing + DO_SETUP=yes + ;; + --start-daemon) if test "$KERNEL_SUPPORT" != "yes"; then echo "$arg unsupported. use \"--start\"" >&2 @@ -1759,6 +1768,7 @@ check_options_early() OPHELP="$OPDIR/ophelp" VERBOSE="" + DEFAULTS=no for i in $@; do # added to handle arg=val parameters @@ -1782,6 +1792,9 @@ check_options_early() VERBOSE=$val fi ;; + --defaults) + DEFAULTS=yes + ;; --session-dir) error_if_empty $arg $val SESSION_DIR="$val" -- 1.6.4 |
From: Maynard J. <may...@us...> - 2009-12-08 21:47:32
|
Robert Richter wrote: > I figured out that there is no way to reset setup parameters. The only > way was removing the setup file in /root/.oprofile/daemonrc. So I > added the defaults option that removes the file. If there is no setup I'm generally in favor of this patch, but I have a few comments. > file the default values will be set. The option can be combined with You can't run this option by itself without getting the "No vmlinux file specified". This seems a bit counter-intuitive to the intention of the option. I realize this will complicate the code a bit, but it just seems to be common sense that one should be able to run this by itself as sort of a pre-setup step. > other setup options such as: > > opcontrol --defaults --no-vmlinux --buffer-size=640000 > > This resets all values to default except buffer size and vmlinux. > > Signed-off-by: Robert Richter <rob...@am...> > --- > ChangeLog | 6 ++++++ > doc/opcontrol.1.in | 5 +++++ > doc/oprofile.xml | 7 +++++++ > utils/opcontrol | 13 +++++++++++++ > 4 files changed, 31 insertions(+), 0 deletions(-) > > diff --git a/ChangeLog b/ChangeLog > index 6fe6e5e..984e936 100644 > --- a/ChangeLog > +++ b/ChangeLog > @@ -1,5 +1,11 @@ > 2009-08-11 Robert Richter <rob...@am...> > > + * doc/opcontrol.1.in: > + * doc/oprofile.xml: > + * utils/opcontrol: adding --defaults option > + > +2009-08-11 Robert Richter <rob...@am...> > + > * agents/Makefile.am: > * configure.in: adding config option to disable oprofile user check > > diff --git a/doc/opcontrol.1.in b/doc/opcontrol.1.in > index b9cf61e..8b68746 100644 > --- a/doc/opcontrol.1.in > +++ b/doc/opcontrol.1.in > @@ -41,6 +41,11 @@ in ~root/.oprofile/daemonrc. Optional. > Show configuration information. > .br > .TP > +.BI "--defaults" > +Reset setup values to defaults. Can be combined with the --setup > +option. > +.br > +.TP > .BI "--start-daemon" > Start the oprofile daemon without starting profiling. Not available > in 2.2/2.4 kernels. > diff --git a/doc/oprofile.xml b/doc/oprofile.xml > index 35ba48a..1a06bcd 100644 > --- a/doc/oprofile.xml > +++ b/doc/oprofile.xml > @@ -567,6 +567,13 @@ The <command>opcontrol</command> script provides the following actions : > </para></listitem> > </varlistentry> > <varlistentry> > + <term><option>--defaults</option></term> > + <listitem><para> > + Reset setup values to defaults. Can be combined > + with the <option>--setup</option> option. > + </para></listitem> > + </varlistentry> > + <varlistentry> > <term><option>--status</option></term> > <listitem><para> > Show configuration information. > diff --git a/utils/opcontrol b/utils/opcontrol > index 6d1bfc0..0ada540 100644 > --- a/utils/opcontrol > +++ b/utils/opcontrol > @@ -113,6 +113,7 @@ opcontrol: usage: > -v/--version show version > --init loads the oprofile module and oprofilefs > --setup give setup arguments (may be omitted) > + --defaults reset setup values to defaults > --status show configuration > --start-daemon start daemon without starting profiling > -s/--start start data collection > @@ -241,6 +242,9 @@ do_init_daemon_vars() > if test -n "$SESSION_DIR"; then > SAVED=$SESSION_DIR > fi > + if test "$DEFAULTS" = "yes"; then > + rm -f "$SETUP_FILE" > + fi > do_load_setup > if test -n "$SAVED"; then > SESSION_DIR=$SAVED > @@ -665,6 +669,11 @@ do_options() > SETUP=yes > ;; > > + --defaults) > + # also early option code existing > + DO_SETUP=yes You can move the above line to check_options_early and then the above case would be a NOP -- similar to the --session-dir option. I did so and that works. > + ;; > + > --start-daemon) > if test "$KERNEL_SUPPORT" != "yes"; then > echo "$arg unsupported. use \"--start\"" >&2 > @@ -1759,6 +1768,7 @@ check_options_early() > > OPHELP="$OPDIR/ophelp" > VERBOSE="" > + DEFAULTS=no > > for i in $@; do > # added to handle arg=val parameters > @@ -1782,6 +1792,9 @@ check_options_early() > VERBOSE=$val > fi > ;; > + --defaults) > + DEFAULTS=yes As I said above . . . move the "DO_SETUP=yes" from do_options to here. -Maynard > + ;; > --session-dir) > error_if_empty $arg $val > SESSION_DIR="$val" |
From: Robert R. <rob...@am...> - 2009-08-12 15:54:12
|
This adds an xsl script allowing the creation of one event per line dumps with opreport. The dump is usefull to get text formated output of all the data in the sampling buffer. The script is installed in the /usr/share/oprofile/xslt directory. Usage: opreport -X | xsltproc /usr/share/oprofile/xslt/opreport-denormalize.xsl - Signed-off-by: Robert Richter <rob...@am...> --- ChangeLog | 6 ++ pp/Makefile.am | 23 ++++++++ pp/xslt/opreport-denormalize.xsl | 114 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 143 insertions(+), 0 deletions(-) create mode 100644 pp/xslt/opreport-denormalize.xsl diff --git a/ChangeLog b/ChangeLog index 984e936..e5920c5 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,11 @@ 2009-08-11 Robert Richter <rob...@am...> + * pp/Makefile.am: + * pp/xslt/opreport-denormalize.xsl: add xsl script for + denormalized text dumps + +2009-08-11 Robert Richter <rob...@am...> + * doc/opcontrol.1.in: * doc/oprofile.xml: * utils/opcontrol: adding --defaults option diff --git a/pp/Makefile.am b/pp/Makefile.am index 1436473..adb34e9 100644 --- a/pp/Makefile.am +++ b/pp/Makefile.am @@ -43,3 +43,26 @@ oparchive_SOURCES = oparchive.cpp \ oparchive_options.h oparchive_options.cpp \ $(pp_common) oparchive_LDADD = $(common_libs) + +xslt_files = \ + xslt/opreport-denormalize.xsl + +install-data-local: + for i in ${xslt_files} ; do \ + dir=`dirname $$i` ; \ + mkdir -p $(DESTDIR)$(pkgdatadir)/$$dir ; \ + $(INSTALL_DATA) $(srcdir)/$$i $(DESTDIR)$(pkgdatadir)/$$i ; \ + done + +uninstall-local: + for i in ${xslt_files} ; do \ + dir=`dirname $$i` ; \ + if test -f $(DESTDIR)$(pkgdatadir)/$$i ; then \ + rm $(DESTDIR)$(pkgdatadir)/$$i ; \ + fi; \ + if test -d $(DESTDIR)$(pkgdatadir)/$$dir ; then \ + rmdir --ignore-fail-on-non-empty $(DESTDIR)$(pkgdatadir)/$$dir ; \ + fi; \ + done + +EXTRA_DIST = $(xslt_files) diff --git a/pp/xslt/opreport-denormalize.xsl b/pp/xslt/opreport-denormalize.xsl new file mode 100644 index 0000000..6cdc176 --- /dev/null +++ b/pp/xslt/opreport-denormalize.xsl @@ -0,0 +1,114 @@ +<xsl:stylesheet version="1.0" +xmlns:xsl="http://www.w3.org/1999/XSL/Transform"> + +<xsl:key name="get_class" match="class" use="@name" /> +<xsl:key name="get_event" match="eventsetup" use="@id" /> +<xsl:key name="get_symbol" match="symboldata" use="@id" /> + +<xsl:template match="/"> + +<xsl:text>schemaversion: </xsl:text><xsl:value-of select="profile/@schemaversion"/><xsl:text>
</xsl:text> +<xsl:text>cputype : </xsl:text><xsl:value-of select="profile/@cputype"/><xsl:text>
</xsl:text> +<xsl:text>processor : </xsl:text><xsl:value-of select="profile/@processor"/><xsl:text>
</xsl:text> +<xsl:text>separatedcpus: </xsl:text><xsl:value-of select="profile/@separatedcpus"/><xsl:text>
</xsl:text> +<xsl:text>mhz : </xsl:text><xsl:value-of select="profile/@mhz"/><xsl:text>
</xsl:text> +<xsl:text>title : </xsl:text><xsl:value-of select="profile/@title"/><xsl:text>
</xsl:text> +<xsl:text>
</xsl:text> + +<xsl:text>events:
</xsl:text> +<xsl:for-each select="profile/setup/eventsetup"> +<xsl:value-of select="@id"/> +<xsl:text>:	</xsl:text> +<xsl:apply-templates select="."/> +<xsl:text>
</xsl:text> +</xsl:for-each> +<xsl:text>
</xsl:text> + +<xsl:for-each select="profile/binary"> + +<xsl:for-each select="count"> +<xsl:apply-templates select="key('get_class', @class)"/> +<xsl:text>	</xsl:text> +<xsl:value-of select="../@name"/> +<xsl:text>	</xsl:text> +<xsl:text>(mod)	</xsl:text> +<xsl:text>(sym)	</xsl:text> +<xsl:number value="."/> +<xsl:text>
</xsl:text> +</xsl:for-each> + +<xsl:for-each select="symbol"> + +<xsl:for-each select="count"> +<xsl:apply-templates select="key('get_class', @class)"/> +<xsl:text>	</xsl:text> +<xsl:value-of select="../../@name"/> +<xsl:text>	</xsl:text> +<xsl:text>(mod)	</xsl:text> +<xsl:apply-templates select="key('get_symbol', ../@idref)"/> +<xsl:text>	</xsl:text> +<xsl:number value="."/> +<xsl:text>
</xsl:text> +</xsl:for-each> + +</xsl:for-each> + +<xsl:for-each select="module"> + +<xsl:for-each select="count"> +<xsl:apply-templates select="key('get_class', @class)"/> +<xsl:text>	</xsl:text> +<xsl:value-of select="../../@name"/> +<xsl:text>	</xsl:text> +<xsl:value-of select="../@name"/> +<xsl:text>	</xsl:text> +<xsl:text>(sym)	</xsl:text> +<xsl:number value="."/> +<xsl:text>
</xsl:text> +</xsl:for-each> + +<xsl:for-each select="symbol"> + +<xsl:for-each select="count"> +<xsl:apply-templates select="key('get_class', @class)"/> +<xsl:text>	</xsl:text> +<xsl:value-of select="../../../@name"/> +<xsl:text>	</xsl:text> +<xsl:value-of select="../../@name"/> +<xsl:text>	</xsl:text> +<xsl:apply-templates select="key('get_symbol', ../@idref)"/> +<xsl:text>	</xsl:text> +<xsl:number value="."/> +<xsl:text>
</xsl:text> +</xsl:for-each> + +</xsl:for-each> + +</xsl:for-each> + +</xsl:for-each> + +</xsl:template> + +<xsl:template match="class"> +<xsl:apply-templates select="key('get_event', @event)"/> +<xsl:text>	</xsl:text> +<xsl:value-of select="@cpu"/> +</xsl:template> + +<xsl:template match="eventsetup"> +<xsl:value-of select="@eventname"/> +<xsl:text>	</xsl:text> +<xsl:value-of select="@unitmask"/> +<xsl:text>	</xsl:text> +<xsl:value-of select="@setupcount"/> +</xsl:template> + +<xsl:template match="symboldata"> +<xsl:value-of select="@name"/> +<xsl:text>(</xsl:text> +<xsl:value-of select="@startingaddr"/> +<xsl:text>)</xsl:text> +</xsl:template> + +</xsl:stylesheet> -- 1.6.4 |
From: John L. <le...@mo...> - 2009-08-12 16:57:56
|
On Wed, Aug 12, 2009 at 05:48:35PM +0200, Robert Richter wrote: > This adds an xsl script allowing the creation of one event per line > dumps with opreport. The dump is usefull to get text formated output > of all the data in the sampling buffer. The script is installed in the > /usr/share/oprofile/xslt directory. Can you give an example? You need to update the opreport man page and the manual regards john |
From: Robert R. <rob...@am...> - 2009-08-12 15:54:17
|
For package creation and its usage on a different filesystem it becomes necessary to create copies with libtoolize instead of symlinks. This patch adds an option for this to libtoolize. Signed-off-by: Robert Richter <rob...@am...> --- ChangeLog | 4 ++++ autogen.sh | 2 +- 2 files changed, 5 insertions(+), 1 deletions(-) diff --git a/ChangeLog b/ChangeLog index ac4736d..5456ac1 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,9 @@ 2009-08-11 Robert Richter <rob...@am...> + * autogen.sh: create copies with libtoolize instead of symlinks + +2009-08-11 Robert Richter <rob...@am...> + * libop/op_xml_events.c: * libop/op_xml_out.c: * libop/op_xml_out.h: diff --git a/autogen.sh b/autogen.sh index 02e92ca..708afa3 100755 --- a/autogen.sh +++ b/autogen.sh @@ -35,7 +35,7 @@ if test -n "$1"; then exit 1 fi -libtoolize --automake +libtoolize --automake -c run "$ACLOCAL -I m4" run $AUTOHEADER run "$AUTOMAKE --foreign --add-missing --copy" -- 1.6.4 |
From: Maynard J. <may...@us...> - 2009-10-23 23:08:52
|
Robert Richter wrote: > For package creation and its usage on a different filesystem it > becomes necessary to create copies with libtoolize instead of > symlinks. This patch adds an option for this to libtoolize. Seems OK to me. Patch committed. P.S. Sorry it took me so long to get back to this. -Maynard > > Signed-off-by: Robert Richter <rob...@am...> > --- > ChangeLog | 4 ++++ > autogen.sh | 2 +- > 2 files changed, 5 insertions(+), 1 deletions(-) > > diff --git a/ChangeLog b/ChangeLog > index ac4736d..5456ac1 100644 > --- a/ChangeLog > +++ b/ChangeLog > @@ -1,5 +1,9 @@ > 2009-08-11 Robert Richter <rob...@am...> > > + * autogen.sh: create copies with libtoolize instead of symlinks > + > +2009-08-11 Robert Richter <rob...@am...> > + > * libop/op_xml_events.c: > * libop/op_xml_out.c: > * libop/op_xml_out.h: > diff --git a/autogen.sh b/autogen.sh > index 02e92ca..708afa3 100755 > --- a/autogen.sh > +++ b/autogen.sh > @@ -35,7 +35,7 @@ if test -n "$1"; then > exit 1 > fi > > -libtoolize --automake > +libtoolize --automake -c > run "$ACLOCAL -I m4" > run $AUTOHEADER > run "$AUTOMAKE --foreign --add-missing --copy" |
From: Robert R. <rob...@am...> - 2009-08-12 15:54:20
|
For testing and building binary packages on a build server it is annoying to always get a warnig the user oprofile does not exists on this system. There are two checks, during configuration and while running make. This patch adds an option what allows building packages without all this checks: ./confiure --disable-account-check ... Signed-off-by: Robert Richter <rob...@am...> --- ChangeLog | 5 +++++ agents/Makefile.am | 10 ++++++++-- configure.in | 12 ++++++++++-- 3 files changed, 23 insertions(+), 4 deletions(-) diff --git a/ChangeLog b/ChangeLog index 5456ac1..6fe6e5e 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,10 @@ 2009-08-11 Robert Richter <rob...@am...> + * agents/Makefile.am: + * configure.in: adding config option to disable oprofile user check + +2009-08-11 Robert Richter <rob...@am...> + * autogen.sh: create copies with libtoolize instead of symlinks 2009-08-11 Robert Richter <rob...@am...> diff --git a/agents/Makefile.am b/agents/Makefile.am index 10325a1..c049fa7 100644 --- a/agents/Makefile.am +++ b/agents/Makefile.am @@ -9,7 +9,13 @@ if BUILD_JVMPI_AGENT SUBDIRS += jvmpi endif -install-exec-hook: +INSTALL_EXEC_HOOK = + +if CHECK_ACCOUNT +INSTALL_EXEC_HOOK += __install-exec-hook +endif + +__install-exec-hook: @getent passwd oprofile >/dev/null || ( \ echo "****************************************************************" ; \ echo "* WARNING:" ; \ @@ -27,4 +33,4 @@ install-exec-hook: echo "* The special user 'oprofile' must have the default group set to 'oprofile'." ; \ echo "****************************************************************") ; - +install-exec-hook: $(INSTALL_EXEC_HOOK) diff --git a/configure.in b/configure.in index 5de0db0..82b5061 100644 --- a/configure.in +++ b/configure.in @@ -196,6 +196,12 @@ if test "$enable_optimization" = "no"; then CXXFLAGS=`echo $CXXFLAGS | sed 's/-O2//g'` fi +AC_ARG_ENABLE(account-check, + [ --disable-account-check disable account check (default is enabled)], + enable_account_check=$enableval, enable_account_check=yes) + +AM_CONDITIONAL(CHECK_ACCOUNT, test "x$enable_account_check" = "xyes") + AC_SUBST(OP_CFLAGS) AC_SUBST(OP_CXXFLAGS) @@ -272,8 +278,10 @@ if test -z "$QT_LIB"; then echo "Warning: a working Qt not found; no GUI will be built" fi -if test "`getent passwd oprofile 2>/dev/null`" == "" || \ - test "`getent group oprofile 2>/dev/null`" == ""; then +if ! test "x$enable_account_check" = "xyes"; then + : +elif test "`getent passwd oprofile 2>/dev/null`" == "" || \ + test "`getent group oprofile 2>/dev/null`" == ""; then if test `id -u` != "0"; then echo "Warning: The user account 'oprofile:oprofile' does not exist on the system." echo " To profile JITed code, this special user account must exist." -- 1.6.4 |
From: Maynard J. <may...@us...> - 2009-12-07 23:48:36
|
Robert Richter wrote: > For testing and building binary packages on a build server it is > annoying to always get a warnig the user oprofile does not exists on > this system. There are two checks, during configuration and while > running make. This patch adds an option what allows building packages > without all this checks: Patch committed. -Maynard > > ./confiure --disable-account-check ... > > Signed-off-by: Robert Richter <rob...@am...> > --- > ChangeLog | 5 +++++ > agents/Makefile.am | 10 ++++++++-- > configure.in | 12 ++++++++++-- > 3 files changed, 23 insertions(+), 4 deletions(-) > > diff --git a/ChangeLog b/ChangeLog > index 5456ac1..6fe6e5e 100644 > --- a/ChangeLog > +++ b/ChangeLog > @@ -1,5 +1,10 @@ > 2009-08-11 Robert Richter <rob...@am...> > > + * agents/Makefile.am: > + * configure.in: adding config option to disable oprofile user check > + > +2009-08-11 Robert Richter <rob...@am...> > + > * autogen.sh: create copies with libtoolize instead of symlinks > > 2009-08-11 Robert Richter <rob...@am...> > diff --git a/agents/Makefile.am b/agents/Makefile.am > index 10325a1..c049fa7 100644 > --- a/agents/Makefile.am > +++ b/agents/Makefile.am > @@ -9,7 +9,13 @@ if BUILD_JVMPI_AGENT > SUBDIRS += jvmpi > endif > > -install-exec-hook: > +INSTALL_EXEC_HOOK = > + > +if CHECK_ACCOUNT > +INSTALL_EXEC_HOOK += __install-exec-hook > +endif > + > +__install-exec-hook: > @getent passwd oprofile >/dev/null || ( \ > echo "****************************************************************" ; \ > echo "* WARNING:" ; \ > @@ -27,4 +33,4 @@ install-exec-hook: > echo "* The special user 'oprofile' must have the default group set to 'oprofile'." ; \ > echo "****************************************************************") ; > > - > +install-exec-hook: $(INSTALL_EXEC_HOOK) > diff --git a/configure.in b/configure.in > index 5de0db0..82b5061 100644 > --- a/configure.in > +++ b/configure.in > @@ -196,6 +196,12 @@ if test "$enable_optimization" = "no"; then > CXXFLAGS=`echo $CXXFLAGS | sed 's/-O2//g'` > fi > > +AC_ARG_ENABLE(account-check, > + [ --disable-account-check disable account check (default is enabled)], > + enable_account_check=$enableval, enable_account_check=yes) > + > +AM_CONDITIONAL(CHECK_ACCOUNT, test "x$enable_account_check" = "xyes") > + > AC_SUBST(OP_CFLAGS) > AC_SUBST(OP_CXXFLAGS) > > @@ -272,8 +278,10 @@ if test -z "$QT_LIB"; then > echo "Warning: a working Qt not found; no GUI will be built" > fi > > -if test "`getent passwd oprofile 2>/dev/null`" == "" || \ > - test "`getent group oprofile 2>/dev/null`" == ""; then > +if ! test "x$enable_account_check" = "xyes"; then > + : > +elif test "`getent passwd oprofile 2>/dev/null`" == "" || \ > + test "`getent group oprofile 2>/dev/null`" == ""; then > if test `id -u` != "0"; then > echo "Warning: The user account 'oprofile:oprofile' does not exist on the system." > echo " To profile JITed code, this special user account must exist." |
From: Robert R. <rob...@am...> - 2009-08-12 15:54:22
|
When running opreport for heavy loads I got some data that was producing large xml tags causing a buffer overflow. Looking at the code this behaviour was clear. The function interface is broken by design: -static void xml_do_arch_specific_event_help(struct op_event const * event, - char * buffer) +static void xml_do_arch_specific_event_help(struct op_event const *event, + char *buffer, size_t size) It requires a buffer to write to, but does not provide the size of this buffer. Thus it is not possible to avoid buffer overflows since the size of xml tags may vary and depends on the input data. This patch fixes this. Signed-off-by: Robert Richter <rob...@am...> --- ChangeLog | 7 ++ libop/op_xml_events.c | 65 ++++++++------- libop/op_xml_out.c | 197 +++++++++++++++++++++++++++------------------- libop/op_xml_out.h | 10 +- libutil++/xml_output.cpp | 18 ++--- 5 files changed, 169 insertions(+), 128 deletions(-) diff --git a/ChangeLog b/ChangeLog index 5c32677..ac4736d 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,12 @@ 2009-08-11 Robert Richter <rob...@am...> + * libop/op_xml_events.c: + * libop/op_xml_out.c: + * libop/op_xml_out.h: + * libutil++/xml_output.cpp: fix buffer overflows in xml generator + +2009-08-11 Robert Richter <rob...@am...> + * utils/opcontrol: fix, stop only if enabled 2009-08-11 Robert Richter <rob...@am...> diff --git a/libop/op_xml_events.c b/libop/op_xml_events.c index 5b9ac7d..63f21d2 100644 --- a/libop/op_xml_events.c +++ b/libop/op_xml_events.c @@ -17,37 +17,37 @@ static op_cpu cpu_type; #define MAX_BUFFER 4096 +static char buffer[MAX_BUFFER]; + void open_xml_events(char const * title, char const * doc, op_cpu the_cpu_type) { char const * schema_version = "1.0"; - char buffer[MAX_BUFFER]; buffer[0] = '\0'; cpu_type = the_cpu_type; - open_xml_element(HELP_EVENTS, 0, buffer); - open_xml_element(HELP_HEADER, 1, buffer); - init_xml_str_attr(HELP_TITLE, title, buffer); - init_xml_str_attr(SCHEMA_VERSION, schema_version, buffer); - init_xml_str_attr(HELP_DOC, doc, buffer); - close_xml_element(NONE, 0, buffer); + open_xml_element(HELP_EVENTS, 0, buffer, MAX_BUFFER); + open_xml_element(HELP_HEADER, 1, buffer, MAX_BUFFER); + init_xml_str_attr(HELP_TITLE, title, buffer, MAX_BUFFER); + init_xml_str_attr(SCHEMA_VERSION, schema_version, buffer, MAX_BUFFER); + init_xml_str_attr(HELP_DOC, doc, buffer, MAX_BUFFER); + close_xml_element(NONE, 0, buffer, MAX_BUFFER); printf("%s", buffer); } void close_xml_events(void) { - char buffer[MAX_BUFFER]; - buffer[0] = '\0'; - close_xml_element(HELP_EVENTS, 0, buffer); + close_xml_element(HELP_EVENTS, 0, buffer, MAX_BUFFER); printf("%s", buffer); } -static void xml_do_arch_specific_event_help(struct op_event const * event, - char * buffer) +static void xml_do_arch_specific_event_help(struct op_event const *event, + char *buffer, size_t size) { switch (cpu_type) { case CPU_PPC64_CELL: - init_xml_int_attr(HELP_EVENT_GROUP, event->val / 100, buffer); + init_xml_int_attr(HELP_EVENT_GROUP, event->val / 100, buffer, + size); break; default: break; @@ -60,34 +60,39 @@ void xml_help_for_event(struct op_event const * event) uint i; int nr_counters; int has_nested = strcmp(event->unit->name, "zero"); - char buffer[MAX_BUFFER]; buffer[0] = '\0'; - open_xml_element(HELP_EVENT, 1, buffer); - init_xml_str_attr(HELP_EVENT_NAME, event->name, buffer); - xml_do_arch_specific_event_help(event, buffer); - init_xml_str_attr(HELP_EVENT_DESC, event->desc, buffer); + open_xml_element(HELP_EVENT, 1, buffer, MAX_BUFFER); + init_xml_str_attr(HELP_EVENT_NAME, event->name, buffer, MAX_BUFFER); + xml_do_arch_specific_event_help(event, buffer, MAX_BUFFER); + init_xml_str_attr(HELP_EVENT_DESC, event->desc, buffer, MAX_BUFFER); nr_counters = op_get_nr_counters(cpu_type); - init_xml_int_attr(HELP_COUNTER_MASK, event->counter_mask, buffer); - init_xml_int_attr(HELP_MIN_COUNT, event->min_count, buffer); + init_xml_int_attr(HELP_COUNTER_MASK, event->counter_mask, buffer, + MAX_BUFFER); + init_xml_int_attr(HELP_MIN_COUNT, event->min_count, + buffer, MAX_BUFFER); if (has_nested) { - close_xml_element(NONE, 1, buffer); - open_xml_element(HELP_UNIT_MASKS, 1, buffer); - init_xml_int_attr(HELP_DEFAULT_MASK, event->unit->default_mask, buffer); - close_xml_element(NONE, 1, buffer); + close_xml_element(NONE, 1, buffer, MAX_BUFFER); + open_xml_element(HELP_UNIT_MASKS, 1, buffer, MAX_BUFFER); + init_xml_int_attr(HELP_DEFAULT_MASK, event->unit->default_mask, + buffer, MAX_BUFFER); + close_xml_element(NONE, 1, buffer, MAX_BUFFER); for (i = 0; i < event->unit->num; i++) { - open_xml_element(HELP_UNIT_MASK, 1, buffer); + open_xml_element(HELP_UNIT_MASK, 1, buffer, MAX_BUFFER); init_xml_int_attr(HELP_UNIT_MASK_VALUE, - event->unit->um[i].value, buffer); + event->unit->um[i].value, + buffer, MAX_BUFFER); init_xml_str_attr(HELP_UNIT_MASK_DESC, - event->unit->um[i].desc, buffer); - close_xml_element(NONE, 0, buffer); + event->unit->um[i].desc, + buffer, MAX_BUFFER); + close_xml_element(NONE, 0, buffer, MAX_BUFFER); } - close_xml_element(HELP_UNIT_MASKS, 0, buffer); + close_xml_element(HELP_UNIT_MASKS, 0, buffer, MAX_BUFFER); } - close_xml_element(has_nested ? HELP_EVENT : NONE, has_nested, buffer); + close_xml_element(has_nested ? HELP_EVENT : NONE, has_nested, + buffer, MAX_BUFFER); printf("%s", buffer); } diff --git a/libop/op_xml_out.c b/libop/op_xml_out.c index d779c45..7cf0abb 100644 --- a/libop/op_xml_out.c +++ b/libop/op_xml_out.c @@ -91,143 +91,176 @@ char const * xml_tag_name(tag_t tag) } -void open_xml_element(tag_t tag, int with_attrs, char * buffer) +void open_xml_element(tag_t tag, int with_attrs, char *buffer, size_t max) { - char const * tag_name = xml_tag_name(tag); - unsigned int const max_len = strlen(tag_name) + 3; - char tmp_buf[MAX_BUF_LEN]; + char *buf; + int size, ret; - if (max_len >= sizeof(tmp_buf)) - fprintf(stderr,"Warning: open_xml_element: buffer overflow %d\n", max_len); + buffer[max - 1] = '\0'; + size = strlen(buffer); + buf = &buffer[size]; + size = max - 1 - size; - if (snprintf(tmp_buf, sizeof(tmp_buf), "<%s%s", tag_name, - (with_attrs ? " " : ">\n")) < 0) { + ret = snprintf(buf, size, "<%s%s", xml_tag_name(tag), + (with_attrs ? " " : ">\n")); + + if (ret < 0 || ret >= size) { fprintf(stderr,"open_xml_element: snprintf failed\n"); exit(EXIT_FAILURE); } - strncat(buffer, tmp_buf, sizeof(tmp_buf)); } -void close_xml_element(tag_t tag, int has_nested, char * buffer) +void close_xml_element(tag_t tag, int has_nested, char *buffer, size_t max) { - char const * tag_name = xml_tag_name(tag); - unsigned int const max_len = strlen(tag_name) + 3; - char tmp_buf[MAX_BUF_LEN]; + char *buf; + int size, ret; - if (max_len >= sizeof(tmp_buf)) - fprintf(stderr,"Warning: close_xml_element: buffer overflow %d\n", max_len); + buffer[max - 1] = '\0'; + size = strlen(buffer); + buf = &buffer[size]; + size = max - 1 - size; - if (tag == NONE) { - if (snprintf(tmp_buf, sizeof(tmp_buf), "%s\n", (has_nested ? ">" : "/>")) < 0) { - fprintf(stderr, "close_xml_element: snprintf failed\n"); - exit(EXIT_FAILURE); - } - } else { - if (snprintf(tmp_buf, sizeof(tmp_buf), "</%s>\n", tag_name) < 0) { - fprintf(stderr, "close_xml_element: snprintf failed\n"); - exit(EXIT_FAILURE); - } + if (tag == NONE) + ret = snprintf(buf, size, "%s\n", (has_nested ? ">" : "/>")); + else + ret = snprintf(buf, size, "</%s>\n", xml_tag_name(tag)); + + if (ret < 0 || ret >= size) { + fprintf(stderr, "close_xml_element: snprintf failed\n"); + exit(EXIT_FAILURE); } - strncat(buffer, tmp_buf, sizeof(tmp_buf)); } -void init_xml_int_attr(tag_t attr, int value, char * buffer) +void init_xml_int_attr(tag_t attr, int value, char *buffer, size_t max) { - char const * attr_name = xml_tag_name(attr); - char tmp_buf[MAX_BUF_LEN]; - unsigned int const max_len = strlen(attr_name) + 50; + char *buf; + int size, ret; - if (max_len >= sizeof(tmp_buf)) { - fprintf(stderr, - "Warning: init_xml_int_attr: buffer overflow %d\n", max_len); - } + buffer[max - 1] = '\0'; + size = strlen(buffer); + buf = &buffer[size]; + size = max - 1 - size; + ret = snprintf(buf, size, " %s=\"%d\"", xml_tag_name(attr), value); - if (snprintf(tmp_buf, sizeof(tmp_buf), " %s=\"%d\"", attr_name, value) < 0) { + if (ret < 0 || ret >= size) { fprintf(stderr,"init_xml_int_attr: snprintf failed\n"); exit(EXIT_FAILURE); } - strncat(buffer, tmp_buf, sizeof(tmp_buf)); } -void init_xml_dbl_attr(tag_t attr, double value, char * buffer) +void init_xml_dbl_attr(tag_t attr, double value, char *buffer, size_t max) { - char const * attr_name = xml_tag_name(attr); - unsigned int const max_len = strlen(attr_name) + 50; - char tmp_buf[MAX_BUF_LEN]; + char *buf; + int size, ret; + + buffer[max - 1] = '\0'; + size = strlen(buffer); + buf = &buffer[size]; + size = max - 1 - size; - if (max_len >= sizeof(tmp_buf)) - fprintf(stderr, "Warning: init_xml_dbl_attr: buffer overflow %d\n", max_len); + ret = snprintf(buf, size, " %s=\"%.2f\"", xml_tag_name(attr), value); - if (snprintf(tmp_buf, sizeof(tmp_buf), " %s=\"%.2f\"", attr_name, value) < 0) { + if (ret < 0 || ret >= size) { fprintf(stderr, "init_xml_dbl_attr: snprintf failed\n"); exit(EXIT_FAILURE); } - strncat(buffer, tmp_buf, sizeof(tmp_buf)); } -static char * xml_quote(char const * str, char * quote_buf) +static void xml_quote(char const *str, char *buffer, size_t max) { - int i; - int pos = 0; - int len = strlen(str); + char *buf; + char *quote; + char *pos = (char*)str; + size_t size; + int ret; - - quote_buf[pos++] = '"'; + buffer[max - 1] = '\0'; + size = strlen(buffer); + buf = &buffer[size]; + size = max - 1 - size; - for (i = 0; i < len; i++) { - if (pos >= MAX_BUF_LEN - 10) { - fprintf(stderr,"quote_str: buffer overflow %d\n", pos); - exit(EXIT_FAILURE); - } + if (size < strlen(pos) + 2) + goto Error; - switch(str[i]) { + *buf = '"'; + buf++; + size--; + + while (*pos) { + switch(*pos) { case '&': - strncpy(quote_buf + pos, "&", 5); - pos += 5; + quote = "&"; break; case '<': - strncpy(quote_buf + pos, "<", 4); - pos += 4; + quote = "<"; break; case '>': - strncpy(quote_buf + pos, ">", 4); - pos += 4; + quote = ">"; break; case '"': - strncpy(quote_buf + pos, """, 6); - pos += 6; + quote = """; break; default: - quote_buf[pos++] = str[i]; - break; + *buf = *pos; + pos++; + buf++; + size--; + continue; } + + pos++; + ret = snprintf(buf, size, "%s", quote); + if (ret < 0 || ret >= (int)size) + goto Error; + buf += ret; + size -= ret; + if (size < strlen(pos)) + goto Error; } - quote_buf[pos++] = '"'; - quote_buf[pos++] = '\0'; - return quote_buf; + if (!size) + goto Error; + + *buf = '"'; + buf++; + *buf = '\0'; + + return; + +Error: + fprintf(stderr,"quote_str: buffer overflow\n"); + exit(EXIT_FAILURE); } -void init_xml_str_attr(tag_t attr, char const * str, char * buffer) +void init_xml_str_attr(tag_t attr, char const *str, char *buffer, size_t max) { - char tmp_buf[MAX_BUF_LEN]; - char quote_buf[MAX_BUF_LEN]; - char const * attr_name = xml_tag_name(attr); - char const * quote_str = xml_quote(str, quote_buf); - const unsigned int max_len = strlen(attr_name) + strlen(quote_str) + 10; + char *buf; + int size, ret; - if (max_len >= sizeof(tmp_buf)) - fprintf(stderr, "Warning: init_xml_str_attr: buffer overflow %d\n", max_len); + buffer[max - 1] = '\0'; + size = strlen(buffer); + buf = &buffer[size]; + size = max - 1 - size; - if (snprintf(tmp_buf, sizeof(tmp_buf), " %s=""%s""", attr_name, quote_str) < 0) { - fprintf(stderr,"init_xml_str_attr: snprintf failed\n"); - exit(EXIT_FAILURE); - } - strncat(buffer, tmp_buf, sizeof(tmp_buf)); + ret = snprintf(buf, size, " %s=", xml_tag_name(attr)); + if (ret < 0 || ret >= size) + goto Error; + + buf += ret; + size -= ret; + + if (!size) + goto Error; + + xml_quote(str, buf, size); + return; +Error: + fprintf(stderr,"init_xml_str_attr: snprintf failed\n"); + exit(EXIT_FAILURE); } diff --git a/libop/op_xml_out.h b/libop/op_xml_out.h index 52e8d8f..ae0fea5 100644 --- a/libop/op_xml_out.h +++ b/libop/op_xml_out.h @@ -59,11 +59,11 @@ typedef enum { } tag_t; char const * xml_tag_name(tag_t tag); -void open_xml_element(tag_t tag, int with_attrs, char * result); -void close_xml_element(tag_t tag, int has_nested, char * result); -void init_xml_int_attr(tag_t attr, int value, char * result); -void init_xml_dbl_attr(tag_t attr, double value, char * result); -void init_xml_str_attr(tag_t attr, char const * str, char * result); +void open_xml_element(tag_t tag, int with_attrs, char *buffer, size_t size); +void close_xml_element(tag_t tag, int has_nested, char *buffer, size_t size); +void init_xml_int_attr(tag_t attr, int value, char *buffer, size_t size); +void init_xml_dbl_attr(tag_t attr, double value, char *buffer, size_t size); +void init_xml_str_attr(tag_t attr, char const *str, char *buffer, size_t size); #ifdef __cplusplus } diff --git a/libutil++/xml_output.cpp b/libutil++/xml_output.cpp index 0f0b189..4ac9a1b 100644 --- a/libutil++/xml_output.cpp +++ b/libutil++/xml_output.cpp @@ -16,7 +16,8 @@ using namespace std; -#define MAX_XML_BUF 1024 +#define MAX_XML_BUF 4096 +static char buf[MAX_XML_BUF]; string tag_name(tag_t tag) { @@ -29,10 +30,9 @@ string tag_name(tag_t tag) string open_element(tag_t tag, bool with_attrs) { ostringstream out; - char buf[MAX_XML_BUF]; buf[0] = '\0'; - open_xml_element(tag, with_attrs, buf); + open_xml_element(tag, with_attrs, buf, MAX_XML_BUF); out << buf; return out.str(); } @@ -41,10 +41,9 @@ string open_element(tag_t tag, bool with_attrs) string close_element(tag_t tag, bool has_nested) { ostringstream out; - char buf[MAX_XML_BUF]; buf[0] = '\0'; - close_xml_element(tag, has_nested, buf); + close_xml_element(tag, has_nested, buf, MAX_XML_BUF); out << buf; return out.str(); } @@ -53,10 +52,9 @@ string close_element(tag_t tag, bool has_nested) string init_attr(tag_t attr, size_t value) { ostringstream out; - char buf[MAX_XML_BUF]; buf[0] = '\0'; - init_xml_int_attr(attr, value, buf); + init_xml_int_attr(attr, value, buf, MAX_XML_BUF); out << buf; return out.str(); } @@ -65,10 +63,9 @@ string init_attr(tag_t attr, size_t value) string init_attr(tag_t attr, double value) { ostringstream out; - char buf[MAX_XML_BUF]; buf[0] = '\0'; - init_xml_dbl_attr(attr, value, buf); + init_xml_dbl_attr(attr, value, buf, MAX_XML_BUF); out << buf; return out.str(); } @@ -77,10 +74,9 @@ string init_attr(tag_t attr, double value) string init_attr(tag_t attr, string const & str) { ostringstream out; - char buf[MAX_XML_BUF]; buf[0] = '\0'; - init_xml_str_attr(attr, str.c_str(), buf); + init_xml_str_attr(attr, str.c_str(), buf, MAX_XML_BUF); out << buf; return out.str(); } -- 1.6.4 |
From: Robert R. <rob...@am...> - 2009-08-12 15:58:19
|
Now, the functions do_stop() is only called, if the daemon is enabled. This prevents the irritating message 'Stopping profiling.' if the profiler is not running. Signed-off-by: Robert Richter <rob...@am...> --- ChangeLog | 4 ++++ utils/opcontrol | 3 ++- 2 files changed, 6 insertions(+), 1 deletions(-) diff --git a/ChangeLog b/ChangeLog index e0adc98..5c32677 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,9 @@ 2009-08-11 Robert Richter <rob...@am...> + * utils/opcontrol: fix, stop only if enabled + +2009-08-11 Robert Richter <rob...@am...> + * utils/opcontrol: fix deinit, kill daemon only if running 2009-08-11 Robert Richter <rob...@am...> diff --git a/utils/opcontrol b/utils/opcontrol index 5a4e948..6d1bfc0 100644 --- a/utils/opcontrol +++ b/utils/opcontrol @@ -943,7 +943,8 @@ do_stop() return fi - if test $KERNEL_SUPPORT = "yes"; then + if test $KERNEL_SUPPORT = "yes" \ + && test 0 != $(cat /dev/oprofile/enable); then echo "Stopping profiling." echo 0 >/dev/oprofile/enable fi -- 1.6.4 |
From: Maynard J. <may...@us...> - 2009-09-04 19:47:17
|
Robert Richter wrote: > Now, the functions do_stop() is only called, if the daemon is > enabled. This prevents the irritating message 'Stopping profiling.' if > the profiler is not running. While this patch is also a small change in user interaction (like patch #4), it's result is to get rid of the bogus message "Stopping profiling" when in fact we're doing nothing. Better to display no message at all when the stop command has no effects. Patch committed. Thanks. -Maynard > > Signed-off-by: Robert Richter <rob...@am...> > --- > ChangeLog | 4 ++++ > utils/opcontrol | 3 ++- > 2 files changed, 6 insertions(+), 1 deletions(-) > > diff --git a/ChangeLog b/ChangeLog > index e0adc98..5c32677 100644 > --- a/ChangeLog > +++ b/ChangeLog > @@ -1,5 +1,9 @@ > 2009-08-11 Robert Richter <rob...@am...> > > + * utils/opcontrol: fix, stop only if enabled > + > +2009-08-11 Robert Richter <rob...@am...> > + > * utils/opcontrol: fix deinit, kill daemon only if running > > 2009-08-11 Robert Richter <rob...@am...> > diff --git a/utils/opcontrol b/utils/opcontrol > index 5a4e948..6d1bfc0 100644 > --- a/utils/opcontrol > +++ b/utils/opcontrol > @@ -943,7 +943,8 @@ do_stop() > return > fi > > - if test $KERNEL_SUPPORT = "yes"; then > + if test $KERNEL_SUPPORT = "yes" \ > + && test 0 != $(cat /dev/oprofile/enable); then > echo "Stopping profiling." > echo 0 >/dev/oprofile/enable > fi |
From: Robert R. <rob...@am...> - 2009-08-12 16:12:12
|
On 12.08.09 12:04:09, John Levon wrote: > On Wed, Aug 12, 2009 at 05:48:35PM +0200, Robert Richter wrote: > > > This adds an xsl script allowing the creation of one event per line > > dumps with opreport. The dump is usefull to get text formated output > > of all the data in the sampling buffer. The script is installed in the > > /usr/share/oprofile/xslt directory. > > Can you give an example? You need to update the opreport man page and > the manual See below for a dump. I will update the man page and manual if the patch otherwise will be excepted. -Robert # opreport -X | xsltproc /usr/share/oprofile/xslt/opreport-denormalize.xsl - WARNING! The OProfile kernel driver reports sample buffer overflows. Such overflows can result in incorrect sample attribution, invalid sample files and other symptoms. See the oprofiled.log for details. You should adjust your sampling frequency to eliminate (or at least minimize) these overflows. warning: /no-vmlinux could not be found. <?xml version="1.0"?> schemaversion: 2.0 cputype : x86-64 processor : family10 separatedcpus: mhz : 2100.00 title : opreport -X events: 0: CPU_CLK_UNHALTED 0 50000 /bin/bash (mod) (sym) 5563 /bin/bash (mod) /bin/bash(00000000) 5563 /bin/gawk-3.1.6 (mod) (sym) 41 /bin/gawk-3.1.6 (mod) /bin/gawk-3.1.6(00000000) 41 /bin/grep (mod) (sym) 12 /bin/grep (mod) /bin/grep(00000000) 12 /sbin/init (mod) (sym) 1 /sbin/init (mod) /sbin/init(00000000) 1 /lib64/ld-2.9.so (mod) (sym) 444 /lib64/ld-2.9.so (mod) /lib64/ld-2.9.so(00000000) 444 /lib64/libc-2.9.so (mod) (sym) 7221 /lib64/libc-2.9.so (mod) /lib64/libc-2.9.so(00000000) 7221 /lib64/libdl-2.9.so (mod) (sym) 2 /lib64/libdl-2.9.so (mod) /lib64/libdl-2.9.so(00000000) 2 /lib64/libm-2.9.so (mod) (sym) 1 /lib64/libm-2.9.so (mod) /lib64/libm-2.9.so(00000000) 1 /lib64/libncurses.so.5.6 (mod) (sym) 5 /lib64/libncurses.so.5.6 (mod) /lib64/libncurses.so.5.6(00000000) 5 /usr/lib64/libpopt.so.0.0.0 (mod) (sym) 4 /usr/lib64/libpopt.so.0.0.0 (mod) /usr/lib64/libpopt.so.0.0.0(00000000) 4 /lib64/librt-2.9.so (mod) (sym) 2 /lib64/librt-2.9.so (mod) /lib64/librt-2.9.so(00000000) 2 /bin/ls (mod) (sym) 3 /bin/ls (mod) /bin/ls(00000000) 3 /no-vmlinux (mod) (sym) 6672 /no-vmlinux (mod) /no-vmlinux(00000000) 6672 /usr/bin/ophelp (mod) (sym) 84 /usr/bin/ophelp (mod) op_get_line(004052d0) 29 /usr/bin/ophelp (mod) skip_nonws(00405700) 19 /usr/bin/ophelp (mod) read_events(00403040) 13 /usr/bin/ophelp (mod) skip_ws(004056d0) 7 /usr/bin/ophelp (mod) next_token(00402f20) 4 /usr/bin/ophelp (mod) try_find_um(00402e80) 4 /usr/bin/ophelp (mod) xcalloc(004058b0) 3 /usr/bin/ophelp (mod) arch_filter_events(004038c0) 1 /usr/bin/ophelp (mod) find_um(00402ef0) 1 /usr/bin/ophelp (mod) read_unit_masks(004029b0) 1 /usr/bin/ophelp (mod) strisprefix(00405760) 1 /usr/bin/ophelp (mod) xmalloc(00405910) 1 [...] > > regards > john > -- Advanced Micro Devices, Inc. Operating System Research Center email: rob...@am... |
From: Robert R. <rob...@am...> - 2009-08-12 16:19:52
|
On 12.08.09 18:11:50, Robert Richter wrote: > On 12.08.09 12:04:09, John Levon wrote: > > On Wed, Aug 12, 2009 at 05:48:35PM +0200, Robert Richter wrote: > > > > > This adds an xsl script allowing the creation of one event per line > > > dumps with opreport. The dump is usefull to get text formated output > > > of all the data in the sampling buffer. The script is installed in the > > > /usr/share/oprofile/xslt directory. > > > > Can you give an example? You need to update the opreport man page and > > the manual Esp. I found something like this useful: # opreport -X | xsltproc /usr/share/oprofile/xslt/opreport-denormalize.xsl - | grep perl /usr/bin/perl5.8.8 (mod) (sym) 875 /usr/bin/perl5.8.8 (mod) /usr/bin/perl5.8.8(00000000) 875 -Robert -- Advanced Micro Devices, Inc. Operating System Research Center email: rob...@am... |
From: Maynard J. <may...@us...> - 2009-12-08 22:04:33
|
Robert Richter wrote: > On 12.08.09 18:11:50, Robert Richter wrote: >> On 12.08.09 12:04:09, John Levon wrote: >>> On Wed, Aug 12, 2009 at 05:48:35PM +0200, Robert Richter wrote: >>> >>>> This adds an xsl script allowing the creation of one event per line >>>> dumps with opreport. The dump is usefull to get text formated output >>>> of all the data in the sampling buffer. The script is installed in the >>>> /usr/share/oprofile/xslt directory. >>> Can you give an example? You need to update the opreport man page and >>> the manual > > Esp. I found something like this useful: > > # opreport -X | xsltproc /usr/share/oprofile/xslt/opreport-denormalize.xsl - | grep perl > /usr/bin/perl5.8.8 (mod) (sym) 875 > /usr/bin/perl5.8.8 (mod) /usr/bin/perl5.8.8(00000000) 875 Robert, While I'm sure you have found some use for this, I'm afraid what you're giving me as examples don't help me to understand the usefulness and the purpose of this script. Can you explain it in terms that I (and the general user) can grasp? Please provide a use case scenario, describing what it is you need to know, why existing tools can't provide you with the information, and how the script satisfies your requirement. Thanks. -Maynard > > -Robert > |
From: Maynard J. <may...@us...> - 2009-08-13 16:16:13
|
Robert Richter wrote: > This patch series contains fixes and updates I still have in my > oprofile userland patch queue. I posted some of them already some > months ago. There are mostly reworks of opcontrol, some changes to > make binary package creation and testing easier and some xml stuff. I > find all of them useful, the patches are necessary for my > regression. I think they are useful for others too. Robert, it seems that most (all?) of these patches are independent of one another, correct? If that's the case, in future, please submit such patches individually over time. It's easier for me to handle from a time management perspective. I won't have time to review all of these right away, but I'll chip away at them. Others in the community should also comment on these patches, since it appears many of them will result in behavioral changes to the tool. -Maynard > > -Robert > > > |
From: Robert R. <rob...@am...> - 2009-08-13 16:35:28
|
On 13.08.09 11:15:32, Maynard Johnson wrote: > Robert, it seems that most (all?) of these patches are independent of one > another, correct? If that's the case, in future, please submit such > patches individually over time. It's easier for me to handle from a time > management perspective. Yes, the patches should be independent. Individually patches would have caused conflicts in the ChangeLog file, so I decided to send a patch set. Also, this workflow is easier to me. :) But of course, I will make it easier for you... -Robert -- Advanced Micro Devices, Inc. Operating System Research Center email: rob...@am... |
From: Maynard J. <may...@us...> - 2009-08-21 20:39:37
|
Robert Richter wrote: > This patch series contains fixes and updates I still have in my > oprofile userland patch queue. I posted some of them already some > months ago. There are mostly reworks of opcontrol, some changes to > make binary package creation and testing easier and some xml stuff. I > find all of them useful, the patches are necessary for my > regression. I think they are useful for others too. Robert, I'll be out for the next week, but hope to start reviewing these patches soon after I return. Sorry for the delay. -Maynard > > -Robert > > > |
From: Maynard J. <may...@us...> - 2009-09-04 15:29:43
|
Robert Richter wrote: > There are unreachable vecho commands in the code that are never Specifically, which vecho commands are never executed? The only one I can find is in get_image_range when it is called from do_options when KERNEL_RANGE is not set. Personally, I think this invocation of get_image_range is a bug, since the value of KERNEL_RANGE depends on the order of the options you pass in. If you pass --kernel-range before --vmlinux option, then KERNEL_RANGE is set; if you pass these options in the opposite order, then KERNEL_RANGE is not set. In do_start_daemon, we call get_image_range again, which will always do the right thing since all options have been processed by then. So I think the right fix is to remove the calls to get_image_range in do_setup. Another reason why I'm not inclined to accept this patch is that if you pass --verbose during setup, you don't get *all* the verbose info that you would if you pass it with --start-daemon or --start (e.g., options to oprofiled). So users would probably want to still pass --verbose at start time, resulting in a lot of duplicate verbose information. -Maynard > executed since the options are parsed later. I added --verbose to > check_options_early(). Additional I removed the check that allows the > use of the --verbose option only for the --start or --start-daemon. > > Signed-off-by: Robert Richter <rob...@am...> > --- > ChangeLog | 4 ++++ > utils/opcontrol | 28 ++++++++++++---------------- > 2 files changed, 16 insertions(+), 16 deletions(-) > > diff --git a/ChangeLog b/ChangeLog > index 5c77afb..ce81dc7 100644 > --- a/ChangeLog > +++ b/ChangeLog > @@ -1,3 +1,7 @@ > +2009-08-11 Robert Richter <rob...@am...> > + > + * utils/opcontrol: fix vecho output > + > 2009-07-31 Maynard Johnson <may...@us...> > > * configure.in: bump version in AM_INIT_AUTOMAKE to 0.9.6cvs > diff --git a/utils/opcontrol b/utils/opcontrol > index 41c8f0e..fbea34d 100644 > --- a/utils/opcontrol > +++ b/utils/opcontrol > @@ -291,7 +291,6 @@ do_init() > NOTE_SIZE=0 > VMLINUX= > XENIMAGE="none" > - VERBOSE="" > SEPARATE_LIB=0 > SEPARATE_KERNEL=0 > SEPARATE_THREAD=0 > @@ -839,20 +838,16 @@ do_options() > DO_SETUP=yes > ;; > > - -V|--verbose) > - if test -z "$val"; then > - VERBOSE="all" > - else > - VERBOSE=$val > - fi > - ;; > - > -l|--list-events) > EXCLUSIVE_ARGC=`expr $EXCLUSIVE_ARGC + 1` > EXCLUSIVE_ARGV="$arg" > exec $OPHELP > ;; > > + # ignore early option > + -V|--verbose) > + ;; > + > *) > echo "Unknown option \"$arg\". See opcontrol --help" >&2 > exit 1 > @@ -874,13 +869,6 @@ do_options() > exit 1 > fi > > - if test -n "$VERBOSE"; then > - if test "$START" != "yes" -a "$START_DAEMON" != "yes"; then > - echo "Option --verbose may only be used with --start or --start-daemon" >&2 > - exit 1 > - fi > - fi > - > if test "$DO_SETUP" = "yes"; then > SETUP="$DO_SETUP" > fi > @@ -1756,6 +1744,7 @@ check_options_early() > { > > OPHELP="$OPDIR/ophelp" > + VERBOSE="" > > for i in $@; do > # added to handle arg=val parameters > @@ -1772,6 +1761,13 @@ check_options_early() > $OPHELP --version | cut -d' ' -f2- > exit 0 > ;; > + -V|--verbose) > + if test -z "$val"; then > + VERBOSE="all" > + else > + VERBOSE=$val > + fi > + ;; > --session-dir) > error_if_empty $arg $val > SESSION_DIR="$val" |
From: Maynard J. <may...@us...> - 2009-09-04 15:38:55
|
Robert Richter wrote: > The parameter dump was located in do_init(). Values were printed as "dump_parameters" must be something you've added in your local copy. It's not present in CVS. -Maynard > read from the setup file. Parameters that have been updated with a > setup option were wrong. This patch fixes this. > > Now, parameters are also dumped if there are no options specified for > --setup. Thus, parameters stored in the setup file can be dumped using > 'opcontrol --setup --verbose'. > > Signed-off-by: Robert Richter <rob...@am...> > --- > ChangeLog | 4 ++++ > utils/opcontrol | 10 ++++++++++ > 2 files changed, 14 insertions(+), 0 deletions(-) > > diff --git a/ChangeLog b/ChangeLog > index ce81dc7..10c54cb 100644 > --- a/ChangeLog > +++ b/ChangeLog > @@ -1,5 +1,9 @@ > 2009-08-11 Robert Richter <rob...@am...> > > + * utils/opcontrol: fix parameter dump > + > +2009-08-11 Robert Richter <rob...@am...> > + > * utils/opcontrol: fix vecho output > > 2009-07-31 Maynard Johnson <may...@us...> > diff --git a/utils/opcontrol b/utils/opcontrol > index fbea34d..fc92ac7 100644 > --- a/utils/opcontrol > +++ b/utils/opcontrol > @@ -404,6 +404,8 @@ do_save_setup() > if test "$XEN_RANGE"; then > echo "XEN_RANGE=$XEN_RANGE" >> $SETUP_FILE > fi > + > + dump_parameters; > } > > > @@ -865,6 +867,10 @@ do_options() > fi > > if test "$SETUP" = "yes" -a "$DO_SETUP" != "yes"; then > + if test -n "$VERBOSE"; then > + dump_parameters > + exit 0 > + fi > echo "No options specified for --setup." >&2 > exit 1 > fi > @@ -879,7 +885,10 @@ do_options() > exit 1 > fi > fi > +} > > +dump_parameters() > +{ > vecho "Parameters used:" > vecho "SESSION_DIR $SESSION_DIR" > vecho "LOCK_FILE $LOCK_FILE" > @@ -1424,6 +1433,7 @@ do_start_daemon() > fi > fi > > + dump_parameters > do_setup > check_valid_args > get_image_range "linux" |