taskstats: introduce verison 9 with executable file info: empty device ID

Hi,

this is a request for comments/help for a little patch I am working on. It works to give me an inode for the executable file of an existing task via taskstats, but I fail to get a device ID for that to be able to then look for the correct filesystem in userspace. I am not sure if that information is still supposed to be present at the time the stats are produced. Basically, I want an efficient way to get the path /proc/$pid/exe points to at process exit. I really would like to not bother with individual tasks but only task groups (userspace processes), but that seems to be tricky in taskstats, not being able to tell a single-threaded process from a single thread of a multi-threaded process … and also the tgid stats miss anything but the delay accounting fields right now. My application in HPC just wants global stats about wall-/cputime and memory use in a cluster, accounted for by executable path, which encodes the used scientific software package and version in our setup.

The one big hurdle I have at the moment is that I'm a n00b at kernel development and just am taking gueses from the code I see. So far I think I am accessing the correct struct member for the device ID, but perhaps at the wrong time.

Also, I am open for suggestions for some better way of identifying the executable. Instead of/in addition to the device ID, I would be interested in the fsid (as given by stat -c %i) , for example. This is stable for an NFS filesystem mounted on multiple nodes, where each node has its own major:minor made up for the mount. But then, the overlayfs for / on each netbooted node has a different fsid, while the major:minor stays the same. So both would be good, I guess. Or a filesystem UUID.

This is my commit/patch so far:

From: Thomas Orgis <[email protected]>
Date: Sat, 3 Mar 2018 08:36:43 +0100
Subject: [PATCH] taskstats: introduce verison 9 with executable file info

This adds fields to the taskstats struct that give device and inode info for the executable file associated with the process as a safer way to get that than polling /proc/$pid/exe before a process ends.

The getdelay example program is accompanied by a variant that just prints a stream of exiting tasks with the added info.

The idea is to be able to collect global statistics about use of certain programs installed on a system by associating process/task stats with an absolute executable path. The command field is unsuitable as it does not distinguish between /foo/bar/v1.2.3/bin/bar_exe and /foo/bar/v2.3.4/bin/bar_exe and is generally rather arbitrary.

I would like to just get the path communicated to userspace, but I presume the growth of the struct by PATH_MAX bytes would not be welcomed.

TODO: Device number is always zero. I am using it wrong.
Is it too late to get that info at task exit?

---
 include/uapi/linux/taskstats.h |  20 +-
 kernel/taskstats.c             |  17 ++
 tools/accounting/.gitignore    |   1 +
 tools/accounting/procacct.c    | 427 +++++++++++++++++++++++++++++++++++++++++
 4 files changed, 464 insertions(+), 1 deletion(-)
 create mode 100644 tools/accounting/procacct.c

index b7aa7bb2349f..56b4a67f035c 100644
@@ -34,7 +34,7 @@
  */
 
 
-#define TASKSTATS_VERSION	8
+#define TASKSTATS_VERSION	9
 #define TS_COMM_LEN		32	/* should be >= TASK_COMM_LEN
 					 * in linux/sched.h */
 

@@ -164,6 +164,24 @@ struct taskstats {
 	/* Delay waiting for memory reclaim */
 	__u64	freepages_count;
 	__u64	freepages_delay_total;
+
+	/*
+	 * Version 9:
+	 * Extra fields detailing executable file (/proc/$pid/exe).
+	 * Values are both zero if there is no executable file
+	 * Chose 64 bit for both to keep alignment. Only 32 bit expected
+	 * for device, while userspace will usually only use 16 anyway.
+	 * The client application is supposed to translate that to a
+	 * file path itself, with knowledge about possible failure
+	 * modes (changes in mounts, filesystem operations).
+	 * Communicating the path from the kernel's view would mean
+	 * adding PATH_MAX bytes to this struct and more work.
+	 * I guess we do not want that. It would be convenient, though.
+	 */
+	__u64	ac_exe_dev;	/* device ID */
+	__u64	ac_exe_inode;	/* inode number */
+	/* Version 9 ends here. */
+
 };
 
 

index 4e62a4a8fa91..0f589acd95cd 100644
@@ -166,6 +166,20 @@ static void send_cpu_listeners(struct sk_buff *skb,
 	up_write(&listeners->sem);
 }
 
+static void exe_add_tsk(struct taskstats *stats, struct task_struct *tsk)
+{
+	/* No idea if I'm allowed to access that here, now. */
+	struct file *exe_file = get_task_exe_file(tsk);
+	if (exe_file) {
+		stats->ac_exe_dev = new_encode_dev(exe_file->f_inode->i_rdev);
+		stats->ac_exe_inode = exe_file->f_inode->i_ino;
+		fput(exe_file);
+	} else {
+		stats->ac_exe_dev = 0;
+		stats->ac_exe_inode = 0;
+	}
+}
+
 static void fill_stats(struct user_namespace *user_ns,
 		       struct pid_namespace *pid_ns,
 		       struct task_struct *tsk, struct taskstats *stats)

@@ -188,6 +202,9 @@ static void fill_stats(struct user_namespace *user_ns,
 
 	/* fill in extended acct fields */
 	xacct_add_tsk(stats, tsk);
+
+	/* add executable info */
+	exe_add_tsk(stats, tsk);
 }
 
 static int fill_stats_for_pid(pid_t pid, struct taskstats *stats)

index 86485203c4ae..6a420bc8a63d 100644
@@ -1 +1,2 @@
 getdelays
+procacct

2.15.1




-- 
Dr. Thomas Orgis
HPC @ Universität Hamburg

This adds fields to the taskstats struct that give device and inode info for the executable file associated with the process as a safer way to get that than polling /proc/$pid/exe before a process ends.

The getdelay example program is accompanied by a variant that just prints a stream of exiting tasks with the added info.

The idea is to be able to collect global statistics about use of certain programs installed on a system by associating process/task stats with an absolute executable path. The command field is unsuitable as it does not distinguish between /foo/bar/v1.2.3/bin/bar_exe and /foo/bar/v2.3.4/bin/bar_exe and is generally rather arbitrary.

I would like to just get the path communicated to userspace, but I presume the growth of the struct by PATH_MAX bytes would not be welcomed.

This version of the patch returns correct device IDs by copying what fs/stat.c does, despite the claim that the s_dev member of struct super_block is not supposed to be a search index and not a device ID (include/linux/fs.h).

---
 include/uapi/linux/taskstats.h |  21 +-
 kernel/taskstats.c             |  19 ++
 tools/accounting/.gitignore    |   1 +
 tools/accounting/Makefile      |   2 +-
 tools/accounting/procacct.c    | 425 +++++++++++++++++++++++++++++++++++++++++
 5 files changed, 466 insertions(+), 2 deletions(-)
 create mode 100644 tools/accounting/procacct.c

index b7aa7bb2349f..17cbfeaf09a7 100644
@@ -34,7 +34,7 @@
  */
 
 
-#define TASKSTATS_VERSION	8
+#define TASKSTATS_VERSION	9
 #define TS_COMM_LEN		32	/* should be >= TASK_COMM_LEN
 					 * in linux/sched.h */
 

@@ -164,6 +164,25 @@ struct taskstats {
 	/* Delay waiting for memory reclaim */
 	__u64	freepages_count;
 	__u64	freepages_delay_total;
+
+	/*
+	 * Version 9:
+	 * Extra fields detailing executable file (/proc/$pid/exe).
+	 * Values are both zero if there is no executable file
+	 * Chose 64 bit for both to keep alignment, possibly really
+	 * used for device, while userspace will usually only use
+	 * 16 bits anyway.
+	 * The client application is supposed to translate that to a
+	 * file path itself, with knowledge about possible failure
+	 * modes (changes in mounts, filesystem operations).
+	 * Communicating the path from the kernel's view would mean
+	 * adding PATH_MAX bytes to this struct and more work.
+	 * I guess we do not want that. It would be convenient, though.
+	 */
+	__u64	ac_exe_dev;	/* device ID */
+	__u64	ac_exe_inode;	/* inode number */
+	/* Version 9 ends here. */
+
 };
 
 

index 4e62a4a8fa91..520ff6eb3a2f 100644
@@ -166,6 +166,22 @@ static void send_cpu_listeners(struct sk_buff *skb,
 	up_write(&listeners->sem);
 }
 
+static void exe_add_tsk(struct taskstats *stats, struct task_struct *tsk)
+{
+	/* No idea if I'm allowed to access that here, now. */
+	struct file *exe_file = get_task_exe_file(tsk);
+	if (exe_file) {
+		/* Following cp_new_stat64() in stat.c . */
+		stats->ac_exe_dev =
+			huge_encode_dev(exe_file->f_inode->i_sb->s_dev);
+		stats->ac_exe_inode = exe_file->f_inode->i_ino;
+		fput(exe_file);
+	} else {
+		stats->ac_exe_dev = 0;
+		stats->ac_exe_inode = 0;
+	}
+}
+
 static void fill_stats(struct user_namespace *user_ns,
 		       struct pid_namespace *pid_ns,
 		       struct task_struct *tsk, struct taskstats *stats)

@@ -188,6 +204,9 @@ static void fill_stats(struct user_namespace *user_ns,
 
 	/* fill in extended acct fields */
 	xacct_add_tsk(stats, tsk);
+
+	/* add executable info */
+	exe_add_tsk(stats, tsk);
 }
 
 static int fill_stats_for_pid(pid_t pid, struct taskstats *stats)

index 86485203c4ae..6a420bc8a63d 100644
@@ -1 +1,2 @@
 getdelays
+procacct

index 03687f19cbb1..11def1ad046c 100644
@@ -2,7 +2,7 @@
 CC := $(CROSS_COMPILE)gcc
 CFLAGS := -I../../usr/include
 
-PROGS := getdelays
+PROGS := getdelays procacct
 
 all: $(PROGS)
 

2.15.0