← Commit list

d16bde623

Denys Vlasenko

2025-08-05

SUCCESS Duration: 154s

Subject

top,pmap: do not use common code for reading /proc/PID/smaps

Binary

No binary (build failed)

Architecture & Compiler Flags

Architecture: x86_64-linux-gnu Compiler: gcc (Ubuntu 11.4.0-1ubuntu1~22.04.3) 11.4.0

—

Build Log

(log yok)

Commit Diff GitHub →

include/libbb.h +12 -29 libbb/procps.c +48 -82 procps/pmap.c +110 -12 procps/top.c +8 -8

include/libbb.h+12 -29

@@ -2081,33 +2081,6 @@ enum { COMM_LEN = 16 };
	# endif
	#endif

-	struct smaprec {
-	unsigned long mapped_rw;
-	unsigned long mapped_ro;
-	unsigned long shared_clean;
-	unsigned long shared_dirty;
-	unsigned long private_clean;
-	unsigned long private_dirty;
-	unsigned long stack;
-	unsigned long smap_pss, smap_swap;
-	unsigned long smap_size;
-	// For mixed 32/64 userspace, 32-bit pmap still needs
-	// 64-bit field here to correctly show 64-bit processes:
-	unsigned long long smap_start;
-	// (strictly speaking, other fields need to be wider too,
-	// but they are in kbytes, not bytes, and they hold sizes,
-	// not start addresses, sizes tend to be less than 4 terabytes)
-	char smap_mode[5];
-	char *smap_name;
-	};
-
-	#if !ENABLE_PMAP
-	#define procps_read_smaps(pid, total, cb, data) \
-	procps_read_smaps(pid, total)
-	#endif
-	int FAST_FUNC procps_read_smaps(pid_t pid, struct smaprec *total,
-	void (cb)(struct smaprec , void ), void data);
-
	typedef struct procps_status_t {
	DIR *dir;
	IF_FEATURE_SHOW_THREADS(DIR *task_dir;)
@@ -2137,7 +2110,13 @@ typedef struct procps_status_t {
	#endif
	unsigned tty_major,tty_minor;
	#if ENABLE_FEATURE_TOPMEM
-	struct smaprec smaps;
+	unsigned long mapped_rw;
+	unsigned long mapped_ro;
+	unsigned long shared_clean;
+	unsigned long shared_dirty;
+	unsigned long private_clean;
+	unsigned long private_dirty;
+	unsigned long stack;
	#endif
	char state[4];
	/* basename of executable in exec(2), read from /proc/N/stat
@@ -2186,11 +2165,15 @@ void free_procps_scan(procps_status_t* sp) FAST_FUNC;
	procps_status_t* procps_scan(procps_status_t* sp, int flags) FAST_FUNC;
	/* Format cmdline (up to col chars) into char buf[size] */
	/* Puts [comm] if cmdline is empty (-> process is a kernel thread) */
-	void read_cmdline(char buf, int size, unsigned pid, const char comm) FAST_FUNC;
+	int read_cmdline(char buf, int size, unsigned pid, const char comm) FAST_FUNC;
	pid_t find_pid_by_name(const char procName) FAST_FUNC;
	pid_t pidlist_reverse(pid_t pidList) FAST_FUNC;
	int starts_with_cpu(const char *str) FAST_FUNC;
	unsigned get_cpu_count(void) FAST_FUNC;
+	/* Some internals reused by pmap: */
+	unsigned long FAST_FUNC fast_strtoul_10(char **endptr);
+	unsigned long long FAST_FUNC fast_strtoull_16(char **endptr);
+	char* FAST_FUNC skip_fields(char *str, int count);


	/* Use strict=1 if you process input from untrusted source:

libbb/procps.c+48 -82

@@ -109,7 +109,7 @@ void FAST_FUNC free_procps_scan(procps_status_t* sp)
	}

	#if ENABLE_FEATURE_TOPMEM \|\| ENABLE_PMAP
-	static unsigned long long fast_strtoull_16(char **endptr)
+	unsigned long long FAST_FUNC fast_strtoull_16(char **endptr)
	{
	unsigned char c;
	char str = endptr;
@@ -130,7 +130,7 @@ static unsigned long long fast_strtoull_16(char **endptr)

	#if ENABLE_FEATURE_FAST_TOP \|\| ENABLE_FEATURE_TOPMEM \|\| ENABLE_PMAP
	/* We cut a lot of corners here for speed */
-	static unsigned long fast_strtoul_10(char **endptr)
+	unsigned long FAST_FUNC fast_strtoul_10(char **endptr)
	{
	unsigned char c;
	char str = endptr;
@@ -159,7 +159,7 @@ static unsigned long long fast_strtoull_10(char **endptr)
	return n;
	}
	# else
-	# define fast_strtoull_10(endptr) fast_strtoul_10(endptr)
+	# define fast_strtoull_10(endptr) fast_strtoul_10(endptr)
	# endif

	# if ENABLE_FEATURE_FAST_TOP
@@ -173,7 +173,7 @@ static long fast_strtol_10(char **endptr)
	}
	# endif

-	static char skip_fields(char str, int count)
+	char* FAST_FUNC skip_fields(char *str, int count)
	{
	do {
	while (*str++ != ' ')
@@ -184,35 +184,25 @@ static char skip_fields(char str, int count)
	}
	#endif

-	#if ENABLE_FEATURE_TOPMEM \|\| ENABLE_PMAP
-	static char* skip_whitespace_if_prefixed_with(char buf, const char prefix)
+	#if ENABLE_FEATURE_TOPMEM
+	static NOINLINE void procps_read_smaps(pid_t pid, procps_status_t *sp)
	{
-	char *tp = is_prefixed_with(buf, prefix);
-	if (tp) {
-	tp = skip_whitespace(tp);
-	}
-	return tp;
-	}
+	// There is A LOT of /proc/PID/smaps data on a big system.
+	// Optimize this for speed, makes "top -m" faster.
+	//TODO large speedup:
+	//read /proc/PID/smaps_rollup (cumulative stats of all mappings, much faster)
+	//and /proc/PID/maps to get mapped_ro and mapped_rw (IOW: VSZ,VSZRW)

-	int FAST_FUNC procps_read_smaps(pid_t pid, struct smaprec *total,
-	void (cb)(struct smaprec , void ), void data)
-	{
	FILE *file;
-	struct smaprec currec;
	char filename[sizeof("/proc/%u/smaps") + sizeof(int)*3];
	char buf[PROCPS_BUFSIZE];
-	#if !ENABLE_PMAP
-	void (cb)(struct smaprec , void *) = NULL;
-	void *data = NULL;
-	#endif

	sprintf(filename, "/proc/%u/smaps", (int)pid);

	file = fopen_for_read(filename);
	if (!file)
-	return 1;
+	return;

-	memset(&currec, 0, sizeof(currec));
	while (fgets(buf, PROCPS_BUFSIZE, file)) {
	// Each mapping datum has this form:
	// f7d29000-f7d39000 rw-s FILEOFS M:m INODE FILENAME
@@ -220,80 +210,53 @@ int FAST_FUNC procps_read_smaps(pid_t pid, struct smaprec *total,
	// Rss: nnn kB
	// .....

-	char tp, p;
+	char *tp;

+	if (buf[0] == 'S' \|\| buf[0] == 'P') {
	#define SCAN(S, X) \
-	if ((tp = skip_whitespace_if_prefixed_with(buf, S)) != NULL) { \
-	total->X += currec.X = fast_strtoul_10(&tp); \
-	continue; \
-	}
-	if (cb) {
-	SCAN("Pss:" , smap_pss );
-	SCAN("Swap:" , smap_swap );
-	}
-	SCAN("Private_Dirty:", private_dirty);
-	SCAN("Private_Clean:", private_clean);
-	SCAN("Shared_Dirty:" , shared_dirty );
-	SCAN("Shared_Clean:" , shared_clean );
+	if (memcmp(buf, S, sizeof(S)-1) == 0) { \
+	tp = skip_whitespace(buf + sizeof(S)-1); \
+	sp->X += fast_strtoul_10(&tp); \
+	continue; \
+	}
+	SCAN("Private_Dirty:", private_dirty)
+	SCAN("Private_Clean:", private_clean)
+	SCAN("Shared_Dirty:" , shared_dirty )
+	SCAN("Shared_Clean:" , shared_clean )
	#undef SCAN
+	}
	tp = strchr(buf, '-');
	if (tp) {
	// We reached next mapping - the line of this form:
	// f7d29000-f7d39000 rw-s FILEOFS M:m INODE FILENAME

-	if (cb) {
-	/* If we have a previous record, there's nothing more
-	* for it, call the callback and clear currec
-	*/
-	if (currec.smap_size)
-	cb(&currec, data);
-	free(currec.smap_name);
-	}
-	memset(&currec, 0, sizeof(currec));
+	char *rwx;
+	unsigned long sz;

	*tp = ' ';
	tp = buf;
-	currec.smap_start = fast_strtoull_16(&tp);
-	currec.smap_size = (fast_strtoull_16(&tp) - currec.smap_start) >> 10;
-
-	strncpy(currec.smap_mode, tp, sizeof(currec.smap_mode)-1);
-
+	sz = fast_strtoull_16(&tp); // start
+	sz = (fast_strtoull_16(&tp) - sz) >> 10; // end - start
+	// tp -> "rw-s" string
+	rwx = tp;
	// skipping "rw-s FILEOFS M:m INODE "
	tp = skip_whitespace(skip_fields(tp, 4));
-	// filter out /dev/something (something != zero)
-	if (!is_prefixed_with(tp, "/dev/") \|\| strcmp(tp, "/dev/zero\n") == 0) {
-	if (currec.smap_mode[1] == 'w') {
-	currec.mapped_rw = currec.smap_size;
-	total->mapped_rw += currec.smap_size;
-	} else if (currec.smap_mode[1] == '-') {
-	currec.mapped_ro = currec.smap_size;
-	total->mapped_ro += currec.smap_size;
-	}
+	// if not a device memory mapped...
+	if (memcmp(tp, "/dev/", 5) != 0 // not "/dev/something"
+	\|\| strcmp(tp + 5, "zero\n") == 0 // or is "/dev/zero" (which isn't a device)
+	) {
+	if (rwx[1] == 'w')
+	sp->mapped_rw += sz;
+	else if (rwx[0] == 'r' \|\| rwx[2] == 'x')
+	sp->mapped_ro += sz;
+	// else: seen "---p" mappings (mmap guard gaps?),
+	// do NOT account these as VSZ, they aren't really
	}
-
	if (strcmp(tp, "[stack]\n") == 0)
-	total->stack += currec.smap_size;
-	if (cb) {
-	p = skip_non_whitespace(tp);
-	if (p == tp) {
-	currec.smap_name = xstrdup(" [ anon ]");
-	} else {
-	*p = '\0';
-	currec.smap_name = xstrdup(tp);
-	}
-	}
-	total->smap_size += currec.smap_size;
+	sp->stack += sz;
	}
	}
	fclose(file);
-
-	if (cb) {
-	if (currec.smap_size)
-	cb(&currec, data);
-	free(currec.smap_name);
-	}
-
-	return 0;
	}
	#endif

@@ -502,7 +465,7 @@ procps_status_t* FAST_FUNC procps_scan(procps_status_t* sp, int flags)

	#if ENABLE_FEATURE_TOPMEM
	if (flags & PSSCAN_SMAPS)
-	procps_read_smaps(pid, &sp->smaps, NULL, NULL);
+	procps_read_smaps(pid, sp);
	#endif /* TOPMEM */
	#if ENABLE_FEATURE_PS_ADDITIONAL_COLUMNS
	if (flags & PSSCAN_RUIDGID) {
@@ -585,13 +548,15 @@ procps_status_t* FAST_FUNC procps_scan(procps_status_t* sp, int flags)
	return sp;
	}

-	void FAST_FUNC read_cmdline(char buf, int col, unsigned pid, const char comm)
+	int FAST_FUNC read_cmdline(char buf, int col, unsigned pid, const char comm)
	{
	int sz;
	char filename[sizeof("/proc/%u/cmdline") + sizeof(int)*3];

	sprintf(filename, "/proc/%u/cmdline", pid);
	sz = open_read_close(filename, buf, col - 1);
+	if (sz < 0)
+	return sz;
	if (sz > 0) {
	const char *base;
	int comm_len;
@@ -614,7 +579,7 @@ void FAST_FUNC read_cmdline(char buf, int col, unsigned pid, const char comm)
	* It allows to see thread names set by prctl(PR_SET_NAME).
	*/
	if (!comm)
-	return;
+	return 0;
	comm_len = strlen(comm);
	/* Why compare up to comm_len, not COMM_LEN-1?
	* Well, some processes rewrite argv, and use _spaces_ there
@@ -628,13 +593,14 @@ void FAST_FUNC read_cmdline(char buf, int col, unsigned pid, const char comm)
	memmove(buf + comm_len, buf, col - comm_len);
	snprintf(buf, col, "{%s}", comm);
	if (col <= comm_len)
-	return;
+	return 0;
	buf[comm_len - 1] = ' ';
	buf[col - 1] = '\0';
	}
	} else {
	snprintf(buf, col, "[%s]", comm ? comm : "?");
	}
+	return 0;
	}

	/* from kernel:

procps/pmap.c+110 -12

@@ -29,10 +29,14 @@

	#if ULLONG_MAX == 0xffffffff
	# define TABS "\t"
+	# define SIZEWIDTHx "7"
+	# define SIZEWIDTH "9"
	# define AFMTLL "8"
	# define DASHES ""
	#else
	# define TABS "\t\t"
+	# define SIZEWIDTHx "15"
+	# define SIZEWIDTH "17"
	# define AFMTLL "16"
	# define DASHES "--------"
	#endif
@@ -42,22 +46,114 @@ enum {
	OPT_q = 1 << 1,
	};

-	static void print_smaprec(struct smaprec currec, void data)
-	{
-	unsigned opt = (uintptr_t)data;
+	struct smaprec {
+	// For mixed 32/64 userspace, 32-bit pmap still needs
+	// 64-bit field here to correctly show 64-bit processes:
+	unsigned long long smap_start;
+	// Make size wider too:
+	// I've seen 1203765248 kb large "---p" mapping in a browser,
+	// this cuts close to 4 terabytes.
+	unsigned long long smap_size;
+	// (strictly speaking, other fields need to be wider too,
+	// but they are in kbytes, not bytes, and they hold sizes,
+	// not start addresses, sizes tend to be less than 4 terabytes)
+	unsigned long private_dirty;
+	unsigned long smap_pss, smap_swap;
+	char smap_mode[5];
+	char *smap_name;
+	};

+	static void print_smaprec(struct smaprec *currec)
+	{
	printf("%0" AFMTLL "llx ", currec->smap_start);

-	if (opt & OPT_x)
-	printf("%7lu %7lu %7lu %7lu ",
+	if (option_mask32 & OPT_x)
+	printf("%7llu %7lu %7lu %7lu ",
	currec->smap_size,
	currec->smap_pss,
	currec->private_dirty,
	currec->smap_swap);
	else
-	printf("%7luK", currec->smap_size);
+	printf("%7lluK", currec->smap_size);
+
+	printf(" %.4s %s\n", currec->smap_mode, currec->smap_name ? : " [ anon ]");
+	}
+
+	/* libbb's procps_read_smaps() looks somewhat similar,
+	* but the collected information is sufficiently different
+	* that merging them into one function is not a good idea
+	* (unless you feel masochistic today).
+	*/
+	static int read_smaps(pid_t pid, struct smaprec *total)
+	{
+	FILE *file;
+	struct smaprec currec;
+	char filename[sizeof("/proc/%u/smaps") + sizeof(int)*3];
+	char buf[4096]; // how long the max filenames we expect?
+
+	sprintf(filename, "/proc/%u/smaps", (int)pid);
+
+	file = fopen_for_read(filename);
+	if (!file)
+	return 1;
+
+	memset(&currec, 0, sizeof(currec));
+	while (fgets(buf, sizeof(buf), file)) {
+	// Each mapping datum has this form:
+	// f7d29000-f7d39000 rw-s FILEOFS M:m INODE FILENAME
+	// Size: nnn kB
+	// Rss: nnn kB
+	// .....
+
+	char tp, p;
+
+	if (buf[0] == 'S' \|\| buf[0] == 'P') {
+	#define SCAN(S, X) \
+	if (memcmp(buf, S, sizeof(S)-1) == 0) { \
+	tp = skip_whitespace(buf + sizeof(S)-1); \
+	total->X += currec.X = fast_strtoul_10(&tp); \
+	continue; \
+	}
+	SCAN("Pss:" , smap_pss );
+	SCAN("Swap:" , smap_swap );
+	SCAN("Private_Dirty:", private_dirty);
+	#undef SCAN
+	}
+	tp = strchr(buf, '-');
+	if (tp) {
+	// We reached next mapping - the line of this form:
+	// f7d29000-f7d39000 rw-s FILEOFS M:m INODE FILENAME
+
+	// If we have a previous record, there's nothing more
+	// for it, print and clear currec
+	if (currec.smap_size)
+	print_smaprec(&currec);
+	free(currec.smap_name);
+	memset(&currec, 0, sizeof(currec));
+
+	*tp = ' ';
+	tp = buf;
+	currec.smap_start = fast_strtoull_16(&tp);
+	currec.smap_size = (fast_strtoull_16(&tp) - currec.smap_start) >> 10;
+	strncpy(currec.smap_mode, tp, sizeof(currec.smap_mode)-1);
+
+	// skipping "rw-s FILEOFS M:m INODE "
+	tp = skip_fields(tp, 4);
+	tp = skip_whitespace(tp); // there may be many spaces, can't just "tp++"
+	p = strchrnul(tp, '\n');
+	if (p != tp) {
+	currec.smap_name = xstrndup(tp, p - tp);
+	}
+	total->smap_size += currec.smap_size;
+	}
+	} // while (got line)
+	fclose(file);
+
+	if (currec.smap_size)
+	print_smaprec(&currec);
+	free(currec.smap_name);

-	printf(" %.4s %s\n", currec->smap_mode, currec->smap_name);
+	return 0;
	}

	static int procps_get_maps(pid_t pid, unsigned opt)
@@ -66,25 +162,27 @@ static int procps_get_maps(pid_t pid, unsigned opt)
	int ret;
	char buf[256];

-	read_cmdline(buf, sizeof(buf), pid, NULL);
+	ret = read_cmdline(buf, sizeof(buf), pid, NULL);
+	if (ret < 0)
+	return ret;
+
	printf("%u: %s\n", (int)pid, buf);

	if (!(opt & OPT_q) && (opt & OPT_x))
	puts("Address" TABS " Kbytes PSS Dirty Swap Mode Mapping");

	memset(&total, 0, sizeof(total));
-
-	ret = procps_read_smaps(pid, &total, print_smaprec, (void*)(uintptr_t)opt);
+	ret = read_smaps(pid, &total);
	if (ret)
	return ret;

	if (!(opt & OPT_q)) {
	if (opt & OPT_x)
	printf("--------" DASHES " ------ ------ ------ ------\n"
-	"total" TABS " %7lu %7lu %7lu %7lu\n",
+	"total kB %"SIZEWIDTHx"llu %7lu %7lu %7lu\n",
	total.smap_size, total.smap_pss, total.private_dirty, total.smap_swap);
	else
-	printf("mapped: %luK\n", total.smap_size);
+	printf(" total %"SIZEWIDTH"lluK\n", total.smap_size);
	}

	return 0;

procps/top.c+8 -8

@@ -1302,20 +1302,20 @@ int top_main(int argc UNUSED_PARAM, char **argv)
	}
	#if ENABLE_FEATURE_TOPMEM
	else { /* TOPMEM */
-	if (!(p->smaps.mapped_ro \| p->smaps.mapped_rw))
+	if (!(p->mapped_ro \| p->mapped_rw))
	continue; /* kernel threads are ignored */
	n = ntop;
	/* No bug here - top and topmem are the same */
	top = xrealloc_vector(topmem, 6, ntop++);
	strcpy(topmem[n].comm, p->comm);
	topmem[n].pid = p->pid;
-	topmem[n].vsz = p->smaps.mapped_rw + p->smaps.mapped_ro;
-	topmem[n].vszrw = p->smaps.mapped_rw;
-	topmem[n].rss_sh = p->smaps.shared_clean + p->smaps.shared_dirty;
-	topmem[n].rss = p->smaps.private_clean + p->smaps.private_dirty + topmem[n].rss_sh;
-	topmem[n].dirty = p->smaps.private_dirty + p->smaps.shared_dirty;
-	topmem[n].dirty_sh = p->smaps.shared_dirty;
-	topmem[n].stack = p->smaps.stack;
+	topmem[n].vsz = p->mapped_rw + p->mapped_ro;
+	topmem[n].vszrw = p->mapped_rw;
+	topmem[n].rss_sh = p->shared_clean + p->shared_dirty;
+	topmem[n].rss = p->private_clean + p->private_dirty + topmem[n].rss_sh;
+	topmem[n].dirty = p->private_dirty + p->shared_dirty;
+	topmem[n].dirty_sh = p->shared_dirty;
+	topmem[n].stack = p->stack;
	}
	#endif
	} /* end of "while we read /proc" */