Re: [RFC] [PATCH] tapset to count number open file handlers and max file handlers for a process

[Srinivasa Ds <srinivasa at in dot ibm dot com>]

Frank Ch. Eigler wrote:
> Srinivasa Ds <srinivasa@in.ibm.com> writes:
> 
>>   I have developed a small tapset that caluculates number of opened file
>> handlers for a process as well as maximum file hanlders for a
>> process.
Frank
 Thanks for your encouragement.
> 
> There is at least one problem: all the pointer dereferences are
> unprotected.  Please use kread() for every link in the pointer chain,
> in anticipation that every pointer might be corrupt.

Done

> 
> A lesser problem is that the code does not even try to acquire any
> locks on the file descriptor table, so it could be modified on another
> CPU while this one is reading it.  I see some signs of RCU being used,
> so maybe this is not too bad, but a comment explaining the hazards
> would be nice.

I have gone through the code and saw 2 locks used 1)rcu_read_lock()/rcu_read_unlock()
 for reading values, another 2) spin_(un)lock(&files->file_lock) for modifying values.
I have used rcu_read_lock(), thinking it will not harm the code (Please correct me if
Iam wrong). We can even consider putting comment,if we have problem with the locks.

> 
> Finally, for a real check-in, one would need documentation in stapfuncs
> and at least a buildok item in the test suite.

Iam ready to work on this.

> 
> - FChE


Signed-off-by: Srinivasa DS<srinivasa@in.ibm.com>
=========================================


--- tapset/task.stp.orig	2007-08-30 15:04:43.000000000 +0530
+++ tapset/task.stp	2007-08-30 19:06:32.000000000 +0530
@@ -6,6 +6,10 @@
 // Public License (GPL); either version 2, or (at your option) any
 // later version.
 
+%{
+#include <linux/version.h>
+#include <linux/file.h>
+%}
 
 // Return the task_struct representing the current process
 function task_current:long () %{ /* pure */
@@ -83,6 +87,27 @@ function task_uid:long (task:long) %{ /*
     CATCH_DEREF_FAULT();
 %}
 
+function task_open_file_handler:long (task:long) %{ 
+    struct task_struct *t = (struct task_struct *)(long)THIS->task;
+    struct fdtable *fdt = kread(&(t->files->fdt));
+    unsigned int count=0, fd;
+    rcu_read_lock();	
+    for (fd = 0; fd < kread(&(fdt->max_fds)); fd++) {
+	        if ( kread(&(fdt->fd[fd])) != NULL) 
+        	        count ++;
+	}
+    rcu_read_unlock();
+    THIS->__retvalue = count;
+    CATCH_DEREF_FAULT();
+%} 
+
+function task_max_file_handler:long (task:long) %{
+    struct task_struct *t = (struct task_struct *)(long)THIS->task;
+    rcu_read_lock();
+    THIS->__retvalue = kread(&(t->files->fdt->max_fds));
+    rcu_read_unlock();
+    CATCH_DEREF_FAULT();
+%}
 
 // Return the effective user id of the given task
 function task_euid:long (task:long) %{ /* pure */