Bug 144584 - [linprocfs][patch] bogus values in linprocfs
Summary: [linprocfs][patch] bogus values in linprocfs
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: 1.0-CURRENT
Hardware: Any Any
: Normal Affects Only Me
Assignee: freebsd-emulation (Nobody)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-03-09 17:50 UTC by Petr Salinger
Modified: 2014-09-01 09:24 UTC (History)
1 user (show)

See Also:


Attachments
file.txt (4.75 KB, text/plain)
2010-03-09 17:50 UTC, Petr Salinger
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Petr Salinger 2010-03-09 17:50:01 UTC
The current linprocfs output wrong values, mainly tick related ones.
The attached is patch that we are using to improve situation.

Fix: Patch attached with submission follows:
Comment 1 Alexander Leidinger 2010-03-26 09:39:54 UTC
Hi,

mp_maxid needs to be proteced with "#ifdef SMP".

The following gives an error in a 32bit compile, either cast i to  
long, or (probably better) change the type in the printf to int.
---snip---
          sbuf_printf(sb, "%lld.%02ld %ld.%02ld\n",
              (long long)tv.tv_sec, tv.tv_usec / 10000,
-            T2S(cp_time[CP_IDLE]), T2J(cp_time[CP_IDLE]) % 100);
+            T2S((cp_time[CP_IDLE]/cnt)), i);
---snip---

Bye,
Alexander.

-- 
On SECOND thought, maybe I'll heat up some BAKED BEANS and watch REGIS
PHILBIN ...  It's GREAT to be ALIVE!!

http://www.Leidinger.net    Alexander @ Leidinger.net: PGP ID = B0063FE7
http://www.FreeBSD.org       netchild @ FreeBSD.org  : PGP ID = 72077137
Comment 2 Petr Salinger 2010-03-26 11:08:41 UTC
> mp_maxid needs to be proteced with "#ifdef SMP".

Does not seem needed in 8.0, anyway, in non-SMP the
cnt (count of active processors) is clearly 1.

> The following gives an error in a 32bit compile, either cast i to  
> long, or (probably better) change the type in the printf to int.

change the type in the printf to int as i will be in range 0..99.

Petr
Comment 3 Alexander Leidinger 2010-03-26 11:29:58 UTC
Quoting Petr Salinger <Petr.Salinger@seznam.cz> (from Fri, 26 Mar 2010  
12:08:41 +0100 (CET)):

>> mp_maxid needs to be proteced with "#ifdef SMP".
>
> Does not seem needed in 8.0, anyway, in non-SMP the
> cnt (count of active processors) is clearly 1.

I had a look at other places where it is used, and it is not protected  
in all places. So I let it like it is.

>> The following gives an error in a 32bit compile, either cast i to  
>> long, or (probably better) change the type in the printf to int.
>
> change the type in the printf to int as i will be in range 0..99.

Thanks for the confirmation.

Bye,
Alexander.

-- 
"This may be sacrilege to you, but my client can do things for you that even
Mr. Hoover might find out of his range."
		-- Tom Hagen, "Chapter 1", page 58

http://www.Leidinger.net    Alexander @ Leidinger.net: PGP ID = B0063FE7
http://www.FreeBSD.org       netchild @ FreeBSD.org  : PGP ID = 72077137
Comment 4 dfilter service freebsd_committer freebsd_triage 2010-03-26 11:43:30 UTC
Author: netchild
Date: Fri Mar 26 11:43:15 2010
New Revision: 205683
URL: http://svn.freebsd.org/changeset/base/205683

Log:
  Fix some bogus values in linprocfs.
  
  Submitted by:	Petr Salinger <Petr.Salinger@seznam.cz>
  Verified on:	GNU/kFreeBSD debian 8.0-1-686 (by submitter)
  PR:		144584

Modified:
  head/sys/compat/linprocfs/linprocfs.c

Modified: head/sys/compat/linprocfs/linprocfs.c
==============================================================================
--- head/sys/compat/linprocfs/linprocfs.c	Fri Mar 26 11:33:12 2010	(r205682)
+++ head/sys/compat/linprocfs/linprocfs.c	Fri Mar 26 11:43:15 2010	(r205683)
@@ -110,13 +110,36 @@ __FBSDID("$FreeBSD$");
 /*
  * Various conversion macros
  */
+
+/* The LINUX_USER_HZ is assumed 100 for now */
+
+#if defined(__i386__) && defined(__GNUCLIKE_ASM)
+/* we need intermediate result as 64 bit, otherwise it overflows too early */
+#define DO64_MULDIV(v,m,d)       \
+({                              \
+   unsigned long rv0;           \
+   unsigned long rv1;           \
+   __asm__ __volatile__(        \
+                "mull %1\n\t"   \
+                "divl %2\n\t"   \
+                :"=a" (rv0), "=d" (rv1) \
+                :"r" (d), "0" (v), "1" (m) \
+                :"cc" ); \
+  rv0; \
+})
+
+#define T2J(x) DO64_MULDIV((x), 100UL, (stathz ? stathz : hz)) /* ticks to jiffies */
+#else
 #define T2J(x) (((x) * 100UL) / (stathz ? stathz : hz))	/* ticks to jiffies */
+#endif
 #define T2S(x) ((x) / (stathz ? stathz : hz))		/* ticks to seconds */
 #define B2K(x) ((x) >> 10)				/* bytes to kbytes */
 #define B2P(x) ((x) >> PAGE_SHIFT)			/* bytes to pages */
 #define P2B(x) ((x) << PAGE_SHIFT)			/* pages to bytes */
 #define P2K(x) ((x) << (PAGE_SHIFT - 10))		/* pages to kbytes */
 
+#define TV2J(x)	(((x)->tv_sec) * 100UL + ((x)->tv_usec) / 10000)
+
 /**
  * @brief Mapping of ki_stat in struct kinfo_proc to the linux state
  *
@@ -502,12 +525,24 @@ linprocfs_douptime(PFS_FILL_ARGS)
 {
 	long cp_time[CPUSTATES];
 	struct timeval tv;
+	int cnt, i;
 
 	getmicrouptime(&tv);
 	read_cpu_time(cp_time);
-	sbuf_printf(sb, "%lld.%02ld %ld.%02ld\n",
+
+	for (cnt = 0, i = 0; i <= mp_maxid; ++i)
+		if (!(CPU_ABSENT(i)))
+		    cnt++;
+
+	if (!cnt)
+	    cnt = 1;
+
+	i = ((cp_time[CP_IDLE])/cnt) % (stathz ? stathz : hz);
+	i = (i * 100) / (stathz ? stathz : hz);
+
+	sbuf_printf(sb, "%lld.%02ld %ld.%02d\n",
 	    (long long)tv.tv_sec, tv.tv_usec / 10000,
-	    T2S(cp_time[CP_IDLE]), T2J(cp_time[CP_IDLE]) % 100);
+	    T2S((cp_time[CP_IDLE]/cnt)), i);
 	return (0);
 }
 
@@ -613,9 +648,17 @@ linprocfs_doprocstat(PFS_FILL_ARGS)
 	struct kinfo_proc kp;
 	char state;
 	static int ratelimit = 0;
+	unsigned long startcode, startdata;
 
 	PROC_LOCK(p);
 	fill_kinfo_proc(p, &kp);
+	if (p->p_vmspace) {
+	   startcode = (unsigned long) p->p_vmspace->vm_taddr;
+	   startdata = (unsigned long) p->p_vmspace->vm_daddr;
+	} else {
+	   startcode = 0;
+	   startdata = 0;
+	};
 	sbuf_printf(sb, "%d", p->p_pid);
 #define PS_ADD(name, fmt, arg) sbuf_printf(sb, " " fmt, arg)
 	PS_ADD("comm",		"(%s)",	p->p_comm);
@@ -634,30 +677,27 @@ linprocfs_doprocstat(PFS_FILL_ARGS)
 	PS_ADD("pgrp",		"%d",	p->p_pgid);
 	PS_ADD("session",	"%d",	p->p_session->s_sid);
 	PROC_UNLOCK(p);
-	PS_ADD("tty",		"%d",	0); /* XXX */
+	PS_ADD("tty",		"%d",	kp.ki_tdev);
 	PS_ADD("tpgid",		"%d",	kp.ki_tpgid);
 	PS_ADD("flags",		"%u",	0); /* XXX */
 	PS_ADD("minflt",	"%lu",	kp.ki_rusage.ru_minflt);
 	PS_ADD("cminflt",	"%lu",	kp.ki_rusage_ch.ru_minflt);
 	PS_ADD("majflt",	"%lu",	kp.ki_rusage.ru_majflt);
 	PS_ADD("cmajflt",	"%lu",	kp.ki_rusage_ch.ru_majflt);
-	PS_ADD("utime",		"%ld",	T2J(tvtohz(&kp.ki_rusage.ru_utime)));
-	PS_ADD("stime",		"%ld",	T2J(tvtohz(&kp.ki_rusage.ru_stime)));
-	PS_ADD("cutime",	"%ld",	T2J(tvtohz(&kp.ki_rusage_ch.ru_utime)));
-	PS_ADD("cstime",	"%ld",	T2J(tvtohz(&kp.ki_rusage_ch.ru_stime)));
+	PS_ADD("utime",		"%ld",	TV2J((&kp.ki_rusage.ru_utime)));
+	PS_ADD("stime",		"%ld",	TV2J((&kp.ki_rusage.ru_stime)));
+	PS_ADD("cutime",	"%ld",	TV2J((&kp.ki_rusage_ch.ru_utime)));
+	PS_ADD("cstime",	"%ld",	TV2J((&kp.ki_rusage_ch.ru_stime)));
 	PS_ADD("priority",	"%d",	kp.ki_pri.pri_user);
 	PS_ADD("nice",		"%d",	kp.ki_nice); /* 19 (nicest) to -19 */
 	PS_ADD("0",		"%d",	0); /* removed field */
 	PS_ADD("itrealvalue",	"%d",	0); /* XXX */
-	/* XXX: starttime is not right, it is the _same_ for _every_ process.
-	   It should be the number of jiffies between system boot and process
-	   start. */
-	PS_ADD("starttime",	"%lu",	T2J(tvtohz(&kp.ki_start)));
+	PS_ADD("starttime",	"%lu",	TV2J((&kp.ki_start)) - TV2J((&boottime)));
 	PS_ADD("vsize",		"%ju",	P2K((uintmax_t)kp.ki_size));
 	PS_ADD("rss",		"%ju",	(uintmax_t)kp.ki_rssize);
 	PS_ADD("rlim",		"%lu",	kp.ki_rusage.ru_maxrss);
-	PS_ADD("startcode",	"%u",	(unsigned)0);
-	PS_ADD("endcode",	"%u",	0); /* XXX */
+	PS_ADD("startcode",	"%lu",	startcode);
+	PS_ADD("endcode",	"%lu",	startdata);
 	PS_ADD("startstack",	"%u",	0); /* XXX */
 	PS_ADD("kstkesp",	"%u",	0); /* XXX */
 	PS_ADD("kstkeip",	"%u",	0); /* XXX */
@@ -800,7 +840,7 @@ linprocfs_doprocstatus(PFS_FILL_ARGS)
 	 */
 	sbuf_printf(sb, "VmSize:\t%8ju kB\n",	B2K((uintmax_t)kp.ki_size));
 	sbuf_printf(sb, "VmLck:\t%8u kB\n",	P2K(0)); /* XXX */
-	sbuf_printf(sb, "VmRss:\t%8ju kB\n",	P2K((uintmax_t)kp.ki_rssize));
+	sbuf_printf(sb, "VmRSS:\t%8ju kB\n",	P2K((uintmax_t)kp.ki_rssize));
 	sbuf_printf(sb, "VmData:\t%8ju kB\n",	P2K((uintmax_t)kp.ki_dsize));
 	sbuf_printf(sb, "VmStk:\t%8ju kB\n",	P2K((uintmax_t)kp.ki_ssize));
 	sbuf_printf(sb, "VmExe:\t%8ju kB\n",	P2K((uintmax_t)kp.ki_tsize));
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
Comment 5 Alexander Leidinger freebsd_committer freebsd_triage 2010-03-26 11:43:57 UTC
State Changed
From-To: open->patched

Committed to 9-current.
Comment 6 dfilter service freebsd_committer freebsd_triage 2010-04-02 07:50:42 UTC
Author: netchild
Date: Fri Apr  2 06:50:28 2010
New Revision: 206081
URL: http://svn.freebsd.org/changeset/base/206081

Log:
  Re-apply r205683 with some modifications:
    Fix some bogus values in linprocfs.
  
    Submitted by:	Petr Salinger <Petr.Salinger@seznam.cz>
    Verified on:	GNU/kFreeBSD debian 8.0-1-686 (by submitter)
    PR:		144584
  
  Reviewed by / discussed with:	kib, des, jhb, submitter

Modified:
  head/sys/compat/linprocfs/linprocfs.c

Modified: head/sys/compat/linprocfs/linprocfs.c
==============================================================================
--- head/sys/compat/linprocfs/linprocfs.c	Fri Apr  2 06:41:45 2010	(r206080)
+++ head/sys/compat/linprocfs/linprocfs.c	Fri Apr  2 06:50:28 2010	(r206081)
@@ -110,12 +110,14 @@ __FBSDID("$FreeBSD$");
 /*
  * Various conversion macros
  */
-#define T2J(x) (((x) * 100UL) / (stathz ? stathz : hz))	/* ticks to jiffies */
+#define T2J(x) ((long)(((x) * 100ULL) / (stathz ? stathz : hz)))	/* ticks to jiffies */
+#define T2CS(x) ((unsigned long)(((x) * 100ULL) / (stathz ? stathz : hz)))	/* ticks to centiseconds */
 #define T2S(x) ((x) / (stathz ? stathz : hz))		/* ticks to seconds */
 #define B2K(x) ((x) >> 10)				/* bytes to kbytes */
 #define B2P(x) ((x) >> PAGE_SHIFT)			/* bytes to pages */
 #define P2B(x) ((x) << PAGE_SHIFT)			/* pages to bytes */
 #define P2K(x) ((x) << (PAGE_SHIFT - 10))		/* pages to kbytes */
+#define TV2J(x)	((x)->tv_sec * 100UL + (x)->tv_usec / 10000)
 
 /**
  * @brief Mapping of ki_stat in struct kinfo_proc to the linux state
@@ -505,9 +507,10 @@ linprocfs_douptime(PFS_FILL_ARGS)
 
 	getmicrouptime(&tv);
 	read_cpu_time(cp_time);
-	sbuf_printf(sb, "%lld.%02ld %ld.%02ld\n",
+	sbuf_printf(sb, "%lld.%02ld %ld.%02lu\n",
 	    (long long)tv.tv_sec, tv.tv_usec / 10000,
-	    T2S(cp_time[CP_IDLE]), T2J(cp_time[CP_IDLE]) % 100);
+	    T2S(cp_time[CP_IDLE] / mp_ncpus),
+	    T2CS(cp_time[CP_IDLE] / mp_ncpus) % 100);
 	return (0);
 }
 
@@ -613,9 +616,17 @@ linprocfs_doprocstat(PFS_FILL_ARGS)
 	struct kinfo_proc kp;
 	char state;
 	static int ratelimit = 0;
+	vm_offset_t startcode, startdata;
 
 	PROC_LOCK(p);
 	fill_kinfo_proc(p, &kp);
+	if (p->p_vmspace) {
+	   startcode = (vm_offset_t)p->p_vmspace->vm_taddr;
+	   startdata = (vm_offset_t)p->p_vmspace->vm_daddr;
+	} else {
+	   startcode = 0;
+	   startdata = 0;
+	};
 	sbuf_printf(sb, "%d", p->p_pid);
 #define PS_ADD(name, fmt, arg) sbuf_printf(sb, " " fmt, arg)
 	PS_ADD("comm",		"(%s)",	p->p_comm);
@@ -634,30 +645,27 @@ linprocfs_doprocstat(PFS_FILL_ARGS)
 	PS_ADD("pgrp",		"%d",	p->p_pgid);
 	PS_ADD("session",	"%d",	p->p_session->s_sid);
 	PROC_UNLOCK(p);
-	PS_ADD("tty",		"%d",	0); /* XXX */
+	PS_ADD("tty",		"%d",	kp.ki_tdev);
 	PS_ADD("tpgid",		"%d",	kp.ki_tpgid);
 	PS_ADD("flags",		"%u",	0); /* XXX */
 	PS_ADD("minflt",	"%lu",	kp.ki_rusage.ru_minflt);
 	PS_ADD("cminflt",	"%lu",	kp.ki_rusage_ch.ru_minflt);
 	PS_ADD("majflt",	"%lu",	kp.ki_rusage.ru_majflt);
 	PS_ADD("cmajflt",	"%lu",	kp.ki_rusage_ch.ru_majflt);
-	PS_ADD("utime",		"%ld",	T2J(tvtohz(&kp.ki_rusage.ru_utime)));
-	PS_ADD("stime",		"%ld",	T2J(tvtohz(&kp.ki_rusage.ru_stime)));
-	PS_ADD("cutime",	"%ld",	T2J(tvtohz(&kp.ki_rusage_ch.ru_utime)));
-	PS_ADD("cstime",	"%ld",	T2J(tvtohz(&kp.ki_rusage_ch.ru_stime)));
+	PS_ADD("utime",		"%ld",	TV2J(&kp.ki_rusage.ru_utime));
+	PS_ADD("stime",		"%ld",	TV2J(&kp.ki_rusage.ru_stime));
+	PS_ADD("cutime",	"%ld",	TV2J(&kp.ki_rusage_ch.ru_utime));
+	PS_ADD("cstime",	"%ld",	TV2J(&kp.ki_rusage_ch.ru_stime));
 	PS_ADD("priority",	"%d",	kp.ki_pri.pri_user);
 	PS_ADD("nice",		"%d",	kp.ki_nice); /* 19 (nicest) to -19 */
 	PS_ADD("0",		"%d",	0); /* removed field */
 	PS_ADD("itrealvalue",	"%d",	0); /* XXX */
-	/* XXX: starttime is not right, it is the _same_ for _every_ process.
-	   It should be the number of jiffies between system boot and process
-	   start. */
-	PS_ADD("starttime",	"%lu",	T2J(tvtohz(&kp.ki_start)));
+	PS_ADD("starttime",	"%lu",	TV2J(&kp.ki_start) - TV2J(&boottime));
 	PS_ADD("vsize",		"%ju",	P2K((uintmax_t)kp.ki_size));
 	PS_ADD("rss",		"%ju",	(uintmax_t)kp.ki_rssize);
 	PS_ADD("rlim",		"%lu",	kp.ki_rusage.ru_maxrss);
-	PS_ADD("startcode",	"%u",	(unsigned)0);
-	PS_ADD("endcode",	"%u",	0); /* XXX */
+	PS_ADD("startcode",	"%ju",	(uintmax_t)startcode);
+	PS_ADD("endcode",	"%ju",	(uintmax_t)startdata);
 	PS_ADD("startstack",	"%u",	0); /* XXX */
 	PS_ADD("kstkesp",	"%u",	0); /* XXX */
 	PS_ADD("kstkeip",	"%u",	0); /* XXX */
@@ -800,7 +808,7 @@ linprocfs_doprocstatus(PFS_FILL_ARGS)
 	 */
 	sbuf_printf(sb, "VmSize:\t%8ju kB\n",	B2K((uintmax_t)kp.ki_size));
 	sbuf_printf(sb, "VmLck:\t%8u kB\n",	P2K(0)); /* XXX */
-	sbuf_printf(sb, "VmRss:\t%8ju kB\n",	P2K((uintmax_t)kp.ki_rssize));
+	sbuf_printf(sb, "VmRSS:\t%8ju kB\n",	P2K((uintmax_t)kp.ki_rssize));
 	sbuf_printf(sb, "VmData:\t%8ju kB\n",	P2K((uintmax_t)kp.ki_dsize));
 	sbuf_printf(sb, "VmStk:\t%8ju kB\n",	P2K((uintmax_t)kp.ki_ssize));
 	sbuf_printf(sb, "VmExe:\t%8ju kB\n",	P2K((uintmax_t)kp.ki_tsize));
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
Comment 7 Alexander Best freebsd_committer freebsd_triage 2011-01-28 20:59:53 UTC
Fixed in HEAD and 8-stable (r215394).
-- 
a13x
Comment 8 Alexander Best freebsd_committer freebsd_triage 2011-01-28 21:00:42 UTC
Responsible Changed
From-To: freebsd-bugs->netchild

Over to committer as MFC reminder.
Comment 9 Alexander Best freebsd_committer freebsd_triage 2011-02-08 13:40:56 UTC
Responsible Changed
From-To: netchild->freebsd-bugs

Back into the pool. MFC'ing is non-trivial and requires quite a lot of extra 
work. The initial committer isn't running any older branch and thus cannot MFC 
these changes himself. 
To any developer wishing to work on MFC'ing these changes to stable/{8,7}: 
Please pick up this PR and contact netchild@.
Comment 10 Alexander Best freebsd_committer freebsd_triage 2011-02-15 00:33:50 UTC
Responsible Changed
From-To: freebsd-bugs->freebsd-emulation

Better assign this one to the emulation folks.
Comment 11 Tijl Coosemans freebsd_committer freebsd_triage 2014-09-01 09:24:17 UTC
Not going to be MFCed to stable/8.