Re: [PATCH] add file position info to proc
> >On Sat, Mar 10, 2007 at 03:02:22PM +0100, Jan Engelhardt wrote: > > > >> http://www.mail-archive.com/linux-fsdevel@vger.kernel.org/msg03964.html > > > >to make 'lsof -o' work/happy? > > To make the user happy. lsof is another thing. Yeah, I occasionally come across requests for this feature, and needed it myself a few times as well. It's definitely not essential, but it could be useful. Miklos - To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] add file position info to proc
>On Sat, Mar 10, 2007 at 03:02:22PM +0100, Jan Engelhardt wrote: > >> http://www.mail-archive.com/linux-fsdevel@vger.kernel.org/msg03964.html > >to make 'lsof -o' work/happy? To make the user happy. lsof is another thing. Jan -- - To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] add file position info to proc
On Sat, Mar 10, 2007 at 03:02:22PM +0100, Jan Engelhardt wrote: > http://www.mail-archive.com/linux-fsdevel@vger.kernel.org/msg03964.html to make 'lsof -o' work/happy? is that really worth it? we've managed without it for years... i'n not entirely against the concept, i just work that we too easily add things to /proc and hence it's the mess it is now - To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] add file position info to proc
On Mar 9 2007 23:05, Chris Wedgwood wrote: >On Fri, Mar 09, 2007 at 02:54:38PM +0100, Miklos Szeredi wrote: > >> This patch adds support for finding out the current file position, >> open flags and possibly other info in the future. > >who needs this? /proc/ is an ugly beast at the best of times, is this >really needed? > http://www.mail-archive.com/linux-fsdevel@vger.kernel.org/msg03964.html Jan -- - To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] add file position info to proc
On Fri, Mar 09, 2007 at 02:54:38PM +0100, Miklos Szeredi wrote: > This patch adds support for finding out the current file position, > open flags and possibly other info in the future. who needs this? /proc/ is an ugly beast at the best of times, is this really needed? - To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] add file position info to proc
On Fri, 2007-03-09 at 22:03 +0100, Miklos Szeredi wrote: > > I think this information would be a little easier to access if there > > would be a single file per pid or thread containing something like: > > > > handle flags pos path > > 0 012 1234/dev/pts/1 > > 1 014 5678/tmp/output > > etc. > > >>> > > >>> That would not be a good idea, as not all users have the same > > >>> permissions > > >>> for viewing this information. > > > > Since /proc/$$/fd is S_IRWXU, only the user owning the process (plus > > usually root) can view it. So making the single file you propose S_IRWXU > > too should solve any security issue, should not it? > > > > >I think the problem is not with permissions, but with scaling to large > > >numbers of file descriptors. > > > > Would not that be dealt accordingly with seq_file? > > Yes, most likely it wouldn't be so bad either. But it _would_ be: > > a) more complex, this implementation reuses most of the the /proc/fd code > b) less efficient for querying a single fd > > So I prefer the current solution, but not very strongly. I don't have a real strong opinion, but I wanted to throw that out as a suggestion. You're the one writing the code. Thanks, Shaggy -- David Kleikamp IBM Linux Technology Center - To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] add file position info to proc
> I think this information would be a little easier to access if there > would be a single file per pid or thread containing something like: > > handle flags pos path > 0012 1234/dev/pts/1 > 1014 5678/tmp/output > etc. > >>> > >>> That would not be a good idea, as not all users have the same permissions > >>> for viewing this information. > > Since /proc/$$/fd is S_IRWXU, only the user owning the process (plus > usually root) can view it. So making the single file you propose S_IRWXU > too should solve any security issue, should not it? > > >I think the problem is not with permissions, but with scaling to large > >numbers of file descriptors. > > Would not that be dealt accordingly with seq_file? Yes, most likely it wouldn't be so bad either. But it _would_ be: a) more complex, this implementation reuses most of the the /proc/fd code b) less efficient for querying a single fd So I prefer the current solution, but not very strongly. Miklos - To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] add file position info to proc
Hello, first of all, big thanks to Miklos for rebasing the patch to a newer kernel. :-) On Mar 9 2007 17:00, Miklos Szeredi wrote: I think this information would be a little easier to access if there would be a single file per pid or thread containing something like: handle flags pos path 0 012 1234/dev/pts/1 1 014 5678/tmp/output etc. >>> >>> That would not be a good idea, as not all users have the same permissions >>> for viewing this information. Since /proc/$$/fd is S_IRWXU, only the user owning the process (plus usually root) can view it. So making the single file you propose S_IRWXU too should solve any security issue, should not it? >I think the problem is not with permissions, but with scaling to large >numbers of file descriptors. Would not that be dealt accordingly with seq_file? (You do not seem to use a seq_file in your patch.) >The user is usually interested in a single file descriptor, so it >would be a large waste of resources to put together all this info at >the kernel end, just to let the user parse it all and select the >single line in which s/he is interested in. And in case s/he is interested in all fds, one could just run grep ^offset: /proc/$$/fdpos/* and get a neat view. Jan -- - To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] add file position info to proc
> On Fri, 2007-03-09 at 10:23 -0500, Benjamin LaHaise wrote: > > On Fri, Mar 09, 2007 at 09:15:06AM -0600, Dave Kleikamp wrote: > > > I think this information would be a little easier to access if there > > > would be a single file per pid or thread containing something like: > > > > > > handleflags pos path > > > 0 012 1234/dev/pts/1 > > > 1 014 5678/tmp/output > > > etc. > > > > That would not be a good idea, as not all users have the same permissions > > for viewing this information. > > How will this have different permission issues than Miklos' patch? > > > It's also quite against the design philosophy > > used elsewhere in /proc. > > What's so different between this and /proc//maps > or /proc//mounts? I think the problem is not with permissions, but with scaling to large numbers of file descriptors. The user is usually interested in a single file descriptor, so it would be a large waste of resources to put together all this info at the kernel end, just to let the user parse it all and select the single line in which s/he is interested in. Miklos - To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] add file position info to proc
> > > > pos:1234 > > flags: 012 > > Why are the flags in octal? File mode bits are represented in octal in both and , so this is the least "human unreadable" format. Miklos - To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] add file position info to proc
> > > > > > pos: 1234 > > > flags:012 > > > > Why are the flags in octal? > > File mode bits are represented in octal in both s/mode/flag/ - To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] add file position info to proc
On Fri, 2007-03-09 at 10:23 -0500, Benjamin LaHaise wrote: > On Fri, Mar 09, 2007 at 09:15:06AM -0600, Dave Kleikamp wrote: > > I think this information would be a little easier to access if there > > would be a single file per pid or thread containing something like: > > > > handle flags pos path > > 0 012 1234/dev/pts/1 > > 1 014 5678/tmp/output > > etc. > > That would not be a good idea, as not all users have the same permissions > for viewing this information. How will this have different permission issues than Miklos' patch? > It's also quite against the design philosophy > used elsewhere in /proc. What's so different between this and /proc//maps or /proc//mounts? -- David Kleikamp IBM Linux Technology Center - To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] add file position info to proc
On Fri, Mar 09, 2007 at 09:15:06AM -0600, Dave Kleikamp wrote: > I think this information would be a little easier to access if there > would be a single file per pid or thread containing something like: > > handleflags pos path > 0 012 1234/dev/pts/1 > 1 014 5678/tmp/output > etc. That would not be a good idea, as not all users have the same permissions for viewing this information. It's also quite against the design philosophy used elsewhere in /proc. -ben -- "Time is of no importance, Mr. President, only life is important." Don't Email: <[EMAIL PROTECTED]>. - To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] add file position info to proc
On Fri, 9 March 2007 14:54:38 +0100, Miklos Szeredi wrote: > > pos: 1234 > flags:012 Why are the flags in octal? Jörn -- Happiness isn't having what you want, it's wanting what you have. -- unknown - To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] add file position info to proc
On Fri, 2007-03-09 at 14:54 +0100, Miklos Szeredi wrote: > Comments are welcome. > > > This patch adds support for finding out the current file position, > open flags and possibly other info in the future. > > These new entries are added: > > /proc/PID/fdinfo/FD > /proc/PID/task/TID/fdinfo/FD > > For each fd the information is provided in the following format: > > pos: 1234 > flags:012 I think this information would be a little easier to access if there would be a single file per pid or thread containing something like: handle flags pos path 0 012 1234/dev/pts/1 1 014 5678/tmp/output etc. -- David Kleikamp IBM Linux Technology Center - To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] add file position info to proc
Comments are welcome.
This patch adds support for finding out the current file position,
open flags and possibly other info in the future.
These new entries are added:
/proc/PID/fdinfo/FD
/proc/PID/task/TID/fdinfo/FD
For each fd the information is provided in the following format:
pos:1234
flags: 012
Signed-off-by: Miklos Szeredi <[EMAIL PROTECTED]>
---
Index: linux/fs/proc/base.c
===
--- linux.orig/fs/proc/base.c 2007-03-08 22:15:47.0 +0100
+++ linux/fs/proc/base.c2007-03-09 14:48:06.0 +0100
@@ -1199,7 +1199,10 @@ out:
return ~0U;
}
-static int proc_fd_link(struct inode *inode, struct dentry **dentry, struct
vfsmount **mnt)
+#define PROC_FDINFO_MAX 64
+
+static int proc_fd_info(struct inode *inode, struct dentry **dentry,
+ struct vfsmount **mnt, char *info)
{
struct task_struct *task = get_proc_task(inode);
struct files_struct *files = NULL;
@@ -1218,8 +1221,16 @@ static int proc_fd_link(struct inode *in
spin_lock(&files->file_lock);
file = fcheck_files(files, fd);
if (file) {
- *mnt = mntget(file->f_path.mnt);
- *dentry = dget(file->f_path.dentry);
+ if (mnt)
+ *mnt = mntget(file->f_path.mnt);
+ if (dentry)
+ *dentry = dget(file->f_path.dentry);
+ if (info)
+ snprintf(info, PROC_FDINFO_MAX,
+"pos:\t%lli\n"
+"flags:\t0%o\n",
+(long long) file->f_pos,
+file->f_flags);
spin_unlock(&files->file_lock);
put_files_struct(files);
return 0;
@@ -1230,6 +1241,12 @@ static int proc_fd_link(struct inode *in
return -ENOENT;
}
+static int proc_fd_link(struct inode *inode, struct dentry **dentry,
+ struct vfsmount **mnt)
+{
+ return proc_fd_info(inode, dentry, mnt, NULL);
+}
+
static int tid_fd_revalidate(struct dentry *dentry, struct nameidata *nd)
{
struct inode *inode = dentry->d_inode;
@@ -1325,7 +1342,9 @@ out_iput:
goto out;
}
-static struct dentry *proc_lookupfd(struct inode * dir, struct dentry *
dentry, struct nameidata *nd)
+static struct dentry *proc_lookupfd_common(struct inode *dir,
+ struct dentry *dentry,
+ instantiate_t instantiate)
{
struct task_struct *task = get_proc_task(dir);
unsigned fd = name_to_int(dentry);
@@ -1336,23 +1355,15 @@ static struct dentry *proc_lookupfd(stru
if (fd == ~0U)
goto out;
- result = proc_fd_instantiate(dir, dentry, task, &fd);
+ result = instantiate(dir, dentry, task, &fd);
out:
put_task_struct(task);
out_no_task:
return result;
}
-static int proc_fd_fill_cache(struct file *filp, void *dirent, filldir_t
filldir,
- struct task_struct *task, int fd)
-{
- char name[PROC_NUMBUF];
- int len = snprintf(name, sizeof(name), "%d", fd);
- return proc_fill_cache(filp, dirent, filldir, name, len,
- proc_fd_instantiate, task, &fd);
-}
-
-static int proc_readfd(struct file * filp, void * dirent, filldir_t filldir)
+static int proc_readfd_common(struct file * filp, void * dirent,
+ filldir_t filldir, instantiate_t instantiate)
{
struct dentry *dentry = filp->f_path.dentry;
struct inode *inode = dentry->d_inode;
@@ -1388,12 +1399,17 @@ static int proc_readfd(struct file * fil
for (fd = filp->f_pos-2;
fd < fdt->max_fds;
fd++, filp->f_pos++) {
+ char name[PROC_NUMBUF];
+ int len;
if (!fcheck_files(files, fd))
continue;
rcu_read_unlock();
- if (proc_fd_fill_cache(filp, dirent, filldir,
p, fd) < 0) {
+ len = snprintf(name, sizeof(name), "%d", fd);
+ if (proc_fill_cache(filp, dirent, filldir,
+ name, len, instantiate,
+ p, &fd) < 0) {
rcu_read_lock();
break;
}
@@ -1408,6 +1424,32 @@ out_no_task:
return retval;
}
+static struct dentry *proc_lookupfd(struct inode *dir, struct dentry *dentry,
+
