2007-09-10 14:14:15

by Laurent Vivier

[permalink] [raw]
Subject: [RESEND 2][PATCH 3/4] modify account_system_time() to update guest time in cpustat and task_struct

Index: linux-2.6/include/linux/sched.h
===================================================================
--- linux-2.6.orig/include/linux/sched.h 2007-09-10 14:19:30.000000000 +0200
+++ linux-2.6/include/linux/sched.h 2007-09-10 14:22:07.000000000 +0200
@@ -1316,6 +1316,7 @@ static inline void put_task_struct(struc
#define PF_STARTING 0x00000002 /* being created */
#define PF_EXITING 0x00000004 /* getting shut down */
#define PF_EXITPIDONE 0x00000008 /* pi exit done on shut down */
+#define PF_VCPU 0x00000010 /* I'm a virtual CPU */
#define PF_FORKNOEXEC 0x00000040 /* forked but didn't exec */
#define PF_SUPERPRIV 0x00000100 /* used super-user privileges */
#define PF_DUMPCORE 0x00000200 /* dumped core */
Index: linux-2.6/kernel/sched.c
===================================================================
--- linux-2.6.orig/kernel/sched.c 2007-09-10 14:15:54.000000000 +0200
+++ linux-2.6/kernel/sched.c 2007-09-10 14:56:41.000000000 +0200
@@ -3277,6 +3277,25 @@ void account_user_time(struct task_struc
}

/*
+ * Account guest cpu time to a process.
+ * @p: the process that the cpu time gets accounted to
+ * @cputime: the cpu time spent in virtual machine since the last update
+ */
+void account_guest_time(struct task_struct *p, cputime_t cputime)
+{
+ cputime64_t tmp;
+ struct cpu_usage_stat *cpustat = &kstat_this_cpu.cpustat;
+
+ tmp = cputime_to_cputime64(cputime);
+
+ p->utime = cputime_add(p->utime, cputime);
+ p->gtime = cputime_add(p->gtime, cputime);
+
+ cpustat->user = cputime64_add(cpustat->user, tmp);
+ cpustat->guest = cputime64_add(cpustat->guest, tmp);
+}
+
+/*
* Account system cpu time to a process.
* @p: the process that the cpu time gets accounted to
* @hardirq_offset: the offset to subtract from hardirq_count()
@@ -3289,6 +3308,12 @@ void account_system_time(struct task_str
struct rq *rq = this_rq();
cputime64_t tmp;

+ if (p->flags & PF_VCPU) {
+ account_guest_time(p, cputime);
+ p->flags &= ~PF_VCPU;
+ return;
+ }
+
p->stime = cputime_add(p->stime, cputime);

/* Add system time to cpustat. */


Attachments:
account_guest (2.03 kB)

2007-09-10 15:55:45

by Randy Dunlap

[permalink] [raw]
Subject: Re: [RESEND 2][PATCH 3/4] modify account_system_time() to update guest time in cpustat and task_struct

On Mon, 10 Sep 2007 16:12:58 +0200 Laurent Vivier wrote:

> [PATCH 3/4] modify account_system_time() to add cputime to cpustat->guest if we
> are running a VCPU. We add this cputime to cpustat->user instead of
> cpustat->system because this part of KVM code is in fact user code although it
> is executed in the kernel. We duplicate VCPU time between guest and user to
> allow an unmodified "top(1)" to display correct value. A modified "top(1)" is
> able to display good cpu user time and cpu guest time by subtracting cpu guest
> time from cpu user time. Update "gtime" in task_struct accordingly.
>
> Signed-off-by: Laurent Vivier <[email protected]>


Hi,
Please use kernel-doc notation here:
(see Documentation/kernel-doc-nano-HOWTO.txt or source file examples
or ask)

easy fix:

/*
+ * Account guest cpu time to a process.

/**
* account_guest_time - Account guest cpu time to a process.

+ * @p: the process that the cpu time gets accounted to
+ * @cputime: the cpu time spent in virtual machine since the last update
+ */
+void account_guest_time(struct task_struct *p, cputime_t cputime)
+{

Thanks.
---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

2007-09-10 16:02:04

by Laurent Vivier

[permalink] [raw]
Subject: Re: [RESEND 2][PATCH 3/4] modify account_system_time() to update guest time in cpustat and task_struct

Randy Dunlap wrote:
> On Mon, 10 Sep 2007 16:12:58 +0200 Laurent Vivier wrote:
>
>> [PATCH 3/4] modify account_system_time() to add cputime to cpustat->guest if we
>> are running a VCPU. We add this cputime to cpustat->user instead of
>> cpustat->system because this part of KVM code is in fact user code although it
>> is executed in the kernel. We duplicate VCPU time between guest and user to
>> allow an unmodified "top(1)" to display correct value. A modified "top(1)" is
>> able to display good cpu user time and cpu guest time by subtracting cpu guest
>> time from cpu user time. Update "gtime" in task_struct accordingly.
>>
>> Signed-off-by: Laurent Vivier <[email protected]>
>
>
> Hi,
> Please use kernel-doc notation here:
> (see Documentation/kernel-doc-nano-HOWTO.txt or source file examples
> or ask)
>
> easy fix:
>
> /*
> + * Account guest cpu time to a process.
>
> /**
> * account_guest_time - Account guest cpu time to a process.
>
> + * @p: the process that the cpu time gets accounted to
> + * @cputime: the cpu time spent in virtual machine since the last update
> + */
> +void account_guest_time(struct task_struct *p, cputime_t cputime)
> +{

thank you for the comment.

But I just made a cut&past of the comment of the function above.

Should I be consistent with the doc or with the source ?

Laurent
--
------------- [email protected] --------------
"Software is hard" - Donald Knuth


Attachments:
signature.asc (189.00 B)
OpenPGP digital signature

2007-09-10 16:16:09

by Randy Dunlap

[permalink] [raw]
Subject: Re: [RESEND 2][PATCH 3/4] modify account_system_time() to update guest time in cpustat and task_struct

On Mon, 10 Sep 2007 18:01:34 +0200 Laurent Vivier wrote:

> Randy Dunlap wrote:
> > On Mon, 10 Sep 2007 16:12:58 +0200 Laurent Vivier wrote:
> >
> >> [PATCH 3/4] modify account_system_time() to add cputime to cpustat->guest if we
> >> are running a VCPU. We add this cputime to cpustat->user instead of
> >> cpustat->system because this part of KVM code is in fact user code although it
> >> is executed in the kernel. We duplicate VCPU time between guest and user to
> >> allow an unmodified "top(1)" to display correct value. A modified "top(1)" is
> >> able to display good cpu user time and cpu guest time by subtracting cpu guest
> >> time from cpu user time. Update "gtime" in task_struct accordingly.
> >>
> >> Signed-off-by: Laurent Vivier <[email protected]>
> >
> >
> > Hi,
> > Please use kernel-doc notation here:
> > (see Documentation/kernel-doc-nano-HOWTO.txt or source file examples
> > or ask)
> >
> > easy fix:
> >
> > /*
> > + * Account guest cpu time to a process.
> >
> > /**
> > * account_guest_time - Account guest cpu time to a process.
> >
> > + * @p: the process that the cpu time gets accounted to
> > + * @cputime: the cpu time spent in virtual machine since the last update
> > + */
> > +void account_guest_time(struct task_struct *p, cputime_t cputime)
> > +{
>
> thank you for the comment.
>
> But I just made a cut&past of the comment of the function above.
>
> Should I be consistent with the doc or with the source ?

I see. That entire file could use some kernel-doc-ization.

I suppose that lessens the need for you to make such a change.
I'll probably do it some slow night then.

Thanks,
---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***