| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20100119185213.GB5154@csn.ul.ie>
Date: Tue, 19 Jan 2010 18:52:13 +0000
From: Mel Gorman <mel@....ul.ie>
To: Balbir Singh <balbir@...ux.vnet.ibm.com>
Cc: Alexey Dobriyan <adobriyan@...il.com>,
LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] delay-accounting: Re-implement -c for getdelays.c to
report information on a target command
On Tue, Jan 19, 2010 at 11:43:43PM +0530, Balbir Singh wrote:
> On Tuesday 19 January 2010 11:30 PM, Mel Gorman wrote:
> > Task delay-accounting was identified as one means of determining how long
> > a process spends in congestion_wait() without adding new statistics. For
> > example, if the workload should not be doing IO, delay-accounting could
> > reveal how long it was spending in unexpected IO or delays. Unfortunately,
> > on closer examination it was clear that getdelays does not act as documented.
> >
> > Commit a3baf649 added Documentation/accounting/getdelays.c with a -c switch
> > that was documented to fork/exec a child and report statistics on it but for
> > reasons that are unclear to me, commit 9e06d3f9 deleted support for this
>
> That is an oversight and not intentional. I've not gotten around to
> reimplementing the -c option due to lack for people asking for it.
>
Consider this a request then. Man, I'd really like if someone
implemented that -c switch thing :P
> > switch but did not update the documentation. It might be an oversight or
> > it might be because the control flow of the program meant that accounting
> > information would be printed once early in the lifetime of the program
> > making it of limited use.
> >
> > This patch reimplements -c for getdelays.c to act as documented. Unlike the
> > original version, it waits until the command completes before printing any
> > information on it. An example of it being used looks like
> >
>
> Looks good, could you please keep the original sign-offs as well?
>
Well, the reimplementation is significantly different to what was there
so I'm not sure that's appropriate. Look at the differences yourself.
> > $ ./getdelays -d -c find /home/mel -name mel
> > print delayacct stats ON
> > /home/mel
> > /home/mel/.notes-wine/drive_c/windows/profiles/mel
> > /home/mel/.wine/drive_c/windows/profiles/mel
> > /home/mel/git-configs/dot.kde/share/apps/konqueror/home/mel
> > PID 5923
> >
> > CPU count real total virtual total delay total
> > 42779 5051232096 5164722692 564207988
> > IO count delay total
> > 41727 97804147758
> > SWAP count delay total
> > 0 0
> > RECLAIM count delay total
> > 0 0
> >
> > It's not clear how or if this subsystem is being maintained. If the
> > authors agree on it but do not pick it up for merging, I'll go bug
> > Andrew with it.
> >
>
> I am maintaining it, please do let me know if you have an issue with the
> subsystem or utilities.
>
If you're happy with the patch then, can you pick it up, add your
signed-off-by and go with whatever submission path you use for this
subsystem?
Thanks
> > Signed-off-by: Mel Gorman <mel@....ul.ie>
> > ---
> > Documentation/accounting/getdelays.c | 37 +++++++++++++++++++++++++++++++++--
> > 1 file changed, 35 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/accounting/getdelays.c b/Documentation/accounting/getdelays.c
> > index 6e25c26..c6648bc 100644
> > --- a/Documentation/accounting/getdelays.c
> > +++ b/Documentation/accounting/getdelays.c
> > @@ -21,6 +21,7 @@
> > #include <sys/types.h>
> > #include <sys/stat.h>
> > #include <sys/socket.h>
> > +#include <sys/wait.h>
> > #include <signal.h>
> >
> > #include <linux/genetlink.h>
> > @@ -266,11 +267,13 @@ int main(int argc, char *argv[])
> > int containerset = 0;
> > char containerpath[1024];
> > int cfd = 0;
> > + int forking = 0;
> > + sigset_t sigset;
> >
> > struct msgtemplate msg;
> >
> > - while (1) {
> > - c = getopt(argc, argv, "qdiw:r:m:t:p:vlC:");
> > + while (!forking) {
> > + c = getopt(argc, argv, "qdiw:r:m:t:p:vlC:c:");
> > if (c < 0)
> > break;
> >
> > @@ -319,6 +322,27 @@ int main(int argc, char *argv[])
> > err(1, "Invalid pid\n");
> > cmd_type = TASKSTATS_CMD_ATTR_PID;
> > break;
> > + case 'c':
> > +
> > + /* Block SIGCHLD for sigwait() later */
> > + if (sigemptyset(&sigset) == -1)
> > + err(1, "Failed to empty sigset");
> > + if (sigaddset(&sigset, SIGCHLD))
> > + err(1, "Failed to set sigchld in sigset");
> > + sigprocmask(SIG_BLOCK, &sigset, NULL);
> > +
> > + /* fork/exec a child */
> > + tid = fork();
> > + if (tid < 0)
> > + err(1, "Fork failed\n");
> > + if (tid == 0)
> > + if (execvp(argv[optind - 1], &argv[optind - 1]) < 0)
> > + exit(-1);
> > +
> > + /* Set the command type and avoid further processing */
> > + cmd_type = TASKSTATS_CMD_ATTR_PID;
> > + forking = 1;
> > + break;
> > case 'v':
> > printf("debug on\n");
> > dbg = 1;
> > @@ -370,6 +394,15 @@ int main(int argc, char *argv[])
> > goto err;
> > }
> >
> > + /*
> > + * If we forked a child, wait for it to exit. Cannot use waitpid()
> > + * as all the delicious data would be reaped as part of the wait
> > + */
> > + if (tid && forking) {
> > + int sig_received;
> > + sigwait(&sigset, &sig_received);
> > + }
> > +
> > if (tid) {
> > rc = send_cmd(nl_sd, id, mypid, TASKSTATS_CMD_GET,
> > cmd_type, &tid, sizeof(__u32));
>
--
Mel Gorman
Part-time Phd Student Linux Technology Center
University of Limerick IBM Dublin Software Lab
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists