Re: [patch] add file position info to proc

2007-03-27 Thread Miklos Szeredi
> > > > > For each fd the information is provided in the following format:
> > > > > 
> > > > > pos:  1234
> > > > > flags:012
> > > > 
> > > > Octal? Maybe we should use more traditional hex here?
> > 
> > It's octal in , so it's easier for a human to read.
> > 
> > > Good point.  The O_foo flags are per-arch, so this field has the potential
> > > to be subtly different on different architectures, which is unpleasing.
> > > 
> > > > Or even list flags by name?
> > > 
> > > urg.  Simple enough to do (lookup table, please).  But is it worth it? 
> > > Perhaps just remove that field?
> > 
> > I wouldn't mind.  But leaving it to an application or for a human to
> > sort out is OK I guess.  There are lots of non-portable numbers in
> > proc.
> 
> Hmm, I do not think we want non-portable numbers in proc. What are
> other examples?

Signal masks in PID/status for example.  I'm sure there are others.

Miklos
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch] add file position info to proc

2007-03-27 Thread Pavel Machek
Hi!

> > > > For each fd the information is provided in the following format:
> > > > 
> > > > pos:1234
> > > > flags:  012
> > > 
> > > Octal? Maybe we should use more traditional hex here?
> 
> It's octal in , so it's easier for a human to read.
> 
> > Good point.  The O_foo flags are per-arch, so this field has the potential
> > to be subtly different on different architectures, which is unpleasing.
> > 
> > > Or even list flags by name?
> > 
> > urg.  Simple enough to do (lookup table, please).  But is it worth it? 
> > Perhaps just remove that field?
> 
> I wouldn't mind.  But leaving it to an application or for a human to
> sort out is OK I guess.  There are lots of non-portable numbers in
> proc.

Hmm, I do not think we want non-portable numbers in proc. What are
other examples?
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch] add file position info to proc

2007-03-27 Thread Dave Hansen
On Sun, 2007-03-25 at 15:45 -0800, Andrew Morton wrote:
> On Sat, 24 Mar 2007 23:04:09 +0100 Miklos Szeredi <[EMAIL PROTECTED]> wrote:
> 
> > 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've seen the idea mentioned once or twice, but not with any great
> enthusiasm.  Why does Linux want this feature?

We can also use it for checkpoint/restart.  When restarting, we need to
put back the files in the same state they were in when we checkpointed.
This would at least allow us to do normal file checkpointing in
userspace.

-- Dave

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch] add file position info to proc

2007-03-27 Thread Miklos Szeredi
> > Hi!
> > 
> > > From: Miklos Szeredi <[EMAIL PROTECTED]>
> > > 
> > > 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
> > 
> > Octal? Maybe we should use more traditional hex here?

It's octal in , so it's easier for a human to read.

> Good point.  The O_foo flags are per-arch, so this field has the potential
> to be subtly different on different architectures, which is unpleasing.
> 
> > Or even list flags by name?
> 
> urg.  Simple enough to do (lookup table, please).  But is it worth it? 
> Perhaps just remove that field?

I wouldn't mind.  But leaving it to an application or for a human to
sort out is OK I guess.  There are lots of non-portable numbers in
proc.

Miklos
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch] add file position info to proc

2007-03-27 Thread Andrew Morton
On Tue, 27 Mar 2007 21:24:20 +
Pavel Machek <[EMAIL PROTECTED]> wrote:

> Hi!
> 
> > From: Miklos Szeredi <[EMAIL PROTECTED]>
> > 
> > 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
> 
> Octal? Maybe we should use more traditional hex here?

Good point.  The O_foo flags are per-arch, so this field has the potential
to be subtly different on different architectures, which is unpleasing.

> Or even list flags by name?

urg.  Simple enough to do (lookup table, please).  But is it worth it? 
Perhaps just remove that field?
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch] add file position info to proc

2007-03-27 Thread Jörn Engel
On Tue, 27 March 2007 21:24:20 +, Pavel Machek wrote:
> > From: Miklos Szeredi <[EMAIL PROTECTED]>
> > 
> > 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
> 
> Octal? Maybe we should use more traditional hex here? Or even list
> flags by name?

The flags are defined in octal.  Whether that choice makes sense or
should be rethought is a different question.  I would definitely prefer
hex.

Jörn

-- 
You ain't got no problem, Jules. I'm on the motherfucker. Go back in
there, chill them niggers out and wait for the Wolf, who should be
coming directly.
-- Marsellus Wallace
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch] add file position info to proc

2007-03-27 Thread Pavel Machek
Hi!

> From: Miklos Szeredi <[EMAIL PROTECTED]>
> 
> 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

Octal? Maybe we should use more traditional hex here? Or even list
flags by name?
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch] add file position info to proc

2007-03-26 Thread Andrew Morton
On Tue, 27 Mar 2007 09:08:35 +0200 Miklos Szeredi <[EMAIL PROTECTED]> wrote:

> > > This patch adds support for finding out the current file position,
> > > open flags and possibly other info in the future.
> > 
> > fs/proc/base.c: In function 'proc_lookupfdinfo':
> > fs/proc/base.c:1584: warning: passing argument 3 of 'proc_lookupfd_common' 
> > from incompatible pointer type
> > fs/proc/base.c: In function 'proc_readfdinfo':
> > fs/proc/base.c:1590: warning: passing argument 4 of 'proc_readfd_common' 
> > from incompatible pointer type
> > 
> > If taken, that callback into proc_lookupfd_common() will crash the kernel.
> 
> Ugh.  Some constification in -mm that I hadn't noticed.  Resending.
> 

OK, I misread the code.  I fixed it up.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch] add file position info to proc

2007-03-26 Thread Miklos Szeredi
> > This patch adds support for finding out the current file position,
> > open flags and possibly other info in the future.
> 
> fs/proc/base.c: In function 'proc_lookupfdinfo':
> fs/proc/base.c:1584: warning: passing argument 3 of 'proc_lookupfd_common' 
> from incompatible pointer type
> fs/proc/base.c: In function 'proc_readfdinfo':
> fs/proc/base.c:1590: warning: passing argument 4 of 'proc_readfd_common' from 
> incompatible pointer type
> 
> If taken, that callback into proc_lookupfd_common() will crash the kernel.

Ugh.  Some constification in -mm that I hadn't noticed.  Resending.

Thanks,
Miklos
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch] add file position info to proc

2007-03-26 Thread Andrew Morton
On Sat, 24 Mar 2007 23:04:09 +0100
Miklos Szeredi <[EMAIL PROTECTED]> wrote:

> This patch adds support for finding out the current file position,
> open flags and possibly other info in the future.

fs/proc/base.c: In function 'proc_lookupfdinfo':
fs/proc/base.c:1584: warning: passing argument 3 of 'proc_lookupfd_common' from 
incompatible pointer type
fs/proc/base.c: In function 'proc_readfdinfo':
fs/proc/base.c:1590: warning: passing argument 4 of 'proc_readfd_common' from 
incompatible pointer type

If taken, that callback into proc_lookupfd_common() will crash the kernel.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch] add file position info to proc

2007-03-26 Thread Folkert van Heusden
> > 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've seen the idea mentioned once or twice, but not with any great
> enthusiasm.  Why does Linux want this feature?

Well, it happened quiet a few times that I started some perlscript
parsing a few GB of data and forgot to add some progress counter.
When after an hour of processing the script still has not stopped, I'd
like to see how much it processed so that I can estimate how long I
still have to wait and if it is worthwhile to stop the script for
optimalisations and such.


Folkert van Heusden

-- 
MultiTail è uno flexible tool per seguire di logfiles e effettuazione
di commissioni. Feltrare, provedere da colore, merge, 'diff-view',
etc. http://www.vanheusden.com/multitail/
--
Phone: +31-6-41278122, PGP-key: 1F28D8AE, www.vanheusden.com
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch] add file position info to proc

2007-03-25 Thread Neil Brown
On Sunday March 25, [EMAIL PROTECTED] wrote:
> On Sat, 24 Mar 2007 23:04:09 +0100 Miklos Szeredi <[EMAIL PROTECTED]> wrote:
> 
> > 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've seen the idea mentioned once or twice, but not with any great
> enthusiasm.  Why does Linux want this feature?

Same reason we want ltrace and lsof.  To be able to look inside a
process and see what it is doing.
e.g. how far has that program got through reading that enormous file?
is it really making progress, or is it going much more slowly than I
would expect and so should I look more deeply.

I'm in favour.

NeilBrown
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch] add file position info to proc

2007-03-25 Thread Andrew Morton
On Sat, 24 Mar 2007 23:04:09 +0100 Miklos Szeredi <[EMAIL PROTECTED]> wrote:

> 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've seen the idea mentioned once or twice, but not with any great
enthusiasm.  Why does Linux want this feature?
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[patch] add file position info to proc

2007-03-24 Thread Miklos Szeredi
From: Miklos Szeredi <[EMAIL PROTECTED]>

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-24 19:00:48.0 +0100
+++ linux/fs/proc/base.c2007-03-24 22:28:14.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 dentr