read_proc issue

read_proc issue

[Laurent]

Hi everyone, hope I don't disturb too much.

I'm developping a module which uses an entry in /proc (read-only)
Currently my (*read_proc) function just write a integer in my /proc entry  
and increments it. Here is the code:

static int number_read_procmem(char *buf, char **start, off_t offset, int 
count, int *eof, void *data)
{
        int len = 0;

        len = sprintf(buf, "%d\n", number_current++);
        *eof = 1;
        return len;
}

the function is registered in init_module with this:
create_proc_read_entry("number", 0, NULL, number_read_procmem, NULL);

Problem is:
when I 'cat /proc/number' multiple times, instead of getting
0
1
2
3
...

I get
0
2
4
6
...

I've searched the Net for an answer to this but in vain. I'm not sure I 
should post this to this list (and I'm very sorry if I indeed shouldn't have) 
but this list is my last hope :(

Please can you CC: me the answers to this post as I'm not on the list.
Then again, sorry if I'm intruding and thanks for any help.

Regards,
Laurent Sinitambirivoutin
laurent@augias.org

Re: read_proc issue

[Thomas Hood]

Don't count on your proc read function being
called only once to read a given piece of information.

Re: read_proc issue

[Daniel Phillips]

On February 28, 2002 01:05 am, Erik Mouw wrote:
> On Wed, Feb 27, 2002 at 09:42:04PM +0000, Alan Cox wrote:
> > > I've encountered this problem before, too.  What is the "One True Way"
> > > to do this cleanly?  In other words, if you want to do a calculation
> > > once every time someone runs "cat /proc/foo", what is the cleanest way
> > > to do that?  The solution we came up with was to check the file offset
> > > and only do the calculation if offset == 0, which seems pretty
> > > hackish.
> > 
> > Another approach is to do the calculation open and remember it in per
> > fd private data. You can recover that and free it on release. It could
> > even be a buffer holding the actual "content"
> 
> It might also be an idea to export proc_calc_metrics() from
> fs/proc/proc_misc.c because quite a lot of code actually tries to do
> exactly the same.

Look at all the parameters, they're trying to be a struct.  How about
cleaning it up before exporting?

-- 
Daniel

Re: read_proc issue

[Randy.Dunlap]

On Tue, 26 Feb 2002, Laurent wrote:

| I'm developping a module which uses an entry in /proc (read-only)
| Currently my (*read_proc) function just write a integer in my /proc entry
| and increments it. Here is the code:
|
| static int number_read_procmem(char *buf, char **start, off_t offset, int
| count, int *eof, void *data)
| {
|         int len = 0;
|
|         len = sprintf(buf, "%d\n", number_current++);
|         *eof = 1;
|         return len;
| }
|
| the function is registered in init_module with this:
| create_proc_read_entry("number", 0, NULL, number_read_procmem, NULL);
|
| Problem is:
| when I 'cat /proc/number' multiple times, instead of getting
| 0
| 1
| 2
| 3
| ...
|
| I get
| 0
| 2
| 4
| 6
| ...


I was thinking that some part(s) of the proc-fs code might call
fs/proc/generic.c::proc_file_read() multiple times (internally),
but no, it's just the application (or libc) doing it AFAICT.

app. calls read() [Key data: with offset=0], which calls sys_read(),
which calls proc_file_read(), which returns N bytes, and then
proc_file_read() and sys_read() and read() return to the app.

When the app. gets N bytes of actual data on its read(), it
doesn't know whether it was at EOF or not, so it tries another
read() [with offset=N], and sys_read() calls proc_file_read()
again, both of which return 0, so the app. knows that there
is no more data.

-- 
~Randy

Re: read_proc issue

[Val Henson]

On Wed, Feb 27, 2002 at 11:33:18AM -0800, Randy.Dunlap wrote:
> On Tue, 26 Feb 2002, Laurent wrote:
<snippety snippety>
> | Problem is:
> | when I 'cat /proc/number' multiple times, instead of getting
> | 0
> | 1
> | 2
> | 3
> | ...
> |
> | I get
> | 0
> | 2
> | 4
> | 6
> | ...
<more snippage>
> app. calls read() [Key data: with offset=0], which calls sys_read(),
> which calls proc_file_read(), which returns N bytes, and then
> proc_file_read() and sys_read() and read() return to the app.
> 
> When the app. gets N bytes of actual data on its read(), it
> doesn't know whether it was at EOF or not, so it tries another
> read() [with offset=N], and sys_read() calls proc_file_read()
> again, both of which return 0, so the app. knows that there
> is no more data.

I've encountered this problem before, too.  What is the "One True Way"
to do this cleanly?  In other words, if you want to do a calculation
once every time someone runs "cat /proc/foo", what is the cleanest way
to do that?  The solution we came up with was to check the file offset
and only do the calculation if offset == 0, which seems pretty
hackish.

-VAL

Re: read_proc issue

[Alan Cox]

> I've encountered this problem before, too.  What is the "One True Way"
> to do this cleanly?  In other words, if you want to do a calculation
> once every time someone runs "cat /proc/foo", what is the cleanest way
> to do that?  The solution we came up with was to check the file offset
> and only do the calculation if offset == 0, which seems pretty
> hackish.

Another approach is to do the calculation open and remember it in per
fd private data. You can recover that and free it on release. It could
even be a buffer holding the actual "content"

Re: read_proc issue

[Cort Dougan]

That's a good solution in the general case but doesn't work for
some of the proc entries that already exist.  cpuinfo, for example.
There seem to be a number of niche /proc methods already.  A cache-on-open
method would be very useful for files like cpuinfo and a number of other
files for PPC.

It sure would make accessing /proc files less whacky for user-code.

} Another approach is to do the calculation open and remember it in per
} fd private data. You can recover that and free it on release. It could
} even be a buffer holding the actual "content"

Re: read_proc issue

[Laurent]

> I've encountered this problem before, too.  What is the "One True Way"
> to do this cleanly?  In other words, if you want to do a calculation
> once every time someone runs "cat /proc/foo", what is the cleanest way
> to do that?  The solution we came up with was to check the file offset
> and only do the calculation if offset == 0, which seems pretty
> hackish.

I've tried it and... it works ! :)
Many many thanks :)

In the meantime, I've also followed Tommy Reynolds' advice to not modify 
global state variables within read_procmem. I've intercepted a syscall which 
does the calculation (I've used the open syscall since it allowed me to 
increase the counter by just running vi on any file ;) ) and put it into a 
buffer which is dumped when the /proc entry is read. Works great that way too.
By the way, as the final module will intercept syscalls like open, creat, 
close, link, unlink, mkdir, etc. , I'm wondering if there'll be a dramatic 
negative impact on file operations performance. Is there any efficient method 
to measure this ?
In any case, thanks for all the help you gave me :)

Regards, 
Laurent Sinitambirivoutin
laurent@augias.org

Re: read_proc issue

[Erik Mouw]

On Wed, Feb 27, 2002 at 09:42:04PM +0000, Alan Cox wrote:
> > I've encountered this problem before, too.  What is the "One True Way"
> > to do this cleanly?  In other words, if you want to do a calculation
> > once every time someone runs "cat /proc/foo", what is the cleanest way
> > to do that?  The solution we came up with was to check the file offset
> > and only do the calculation if offset == 0, which seems pretty
> > hackish.
> 
> Another approach is to do the calculation open and remember it in per
> fd private data. You can recover that and free it on release. It could
> even be a buffer holding the actual "content"

It might also be an idea to export proc_calc_metrics() from
fs/proc/proc_misc.c because quite a lot of code actually tries to do
exactly the same.


Erik

-- 
J.A.K. (Erik) Mouw, Information and Communication Theory Group, Faculty
of Information Technology and Systems, Delft University of Technology,
PO BOX 5031, 2600 GA Delft, The Netherlands  Phone: +31-15-2783635
Fax: +31-15-2781843  Email: J.A.K.Mouw@its.tudelft.nl
WWW: http://www-ict.its.tudelft.nl/~erik/

Re: read_proc issue

[Erik Mouw]

On Wed, Feb 27, 2002 at 04:19:35AM +0100, Daniel Phillips wrote:
> On February 28, 2002 01:05 am, Erik Mouw wrote:
> > It might also be an idea to export proc_calc_metrics() from
> > fs/proc/proc_misc.c because quite a lot of code actually tries to do
> > exactly the same.
> 
> Look at all the parameters, they're trying to be a struct.  How about
> cleaning it up before exporting?

Look at all the parameters of a procfs read() function and compare them
with the parameters of proc_calc_metrics(). See why cleaning up
would make things only more complicated?


Erik

-- 
J.A.K. (Erik) Mouw, Information and Communication Theory Group, Faculty
of Information Technology and Systems, Delft University of Technology,
PO BOX 5031, 2600 GA Delft, The Netherlands  Phone: +31-15-2783635
Fax: +31-15-2781843  Email: J.A.K.Mouw@its.tudelft.nl
WWW: http://www-ict.its.tudelft.nl/~erik/

Re: read_proc issue

[Alexander Viro]

On Fri, 1 Mar 2002, Erik Mouw wrote:

> On Wed, Feb 27, 2002 at 04:19:35AM +0100, Daniel Phillips wrote:
> > On February 28, 2002 01:05 am, Erik Mouw wrote:
> > > It might also be an idea to export proc_calc_metrics() from
> > > fs/proc/proc_misc.c because quite a lot of code actually tries to do
> > > exactly the same.
> > 
> > Look at all the parameters, they're trying to be a struct.  How about
> > cleaning it up before exporting?
> 
> Look at all the parameters of a procfs read() function and compare them
> with the parameters of proc_calc_metrics(). See why cleaning up
> would make things only more complicated?

Oh, for fsck sake...

We already have better mechanism.  Let ->proc_read() die, it's an ugly
kludge, breeding overcomplicated code and buffer overflows.

Re: read_proc issue

[Alexander Viro]

On Fri, 1 Mar 2002, Laurent wrote:

> > We already have better mechanism.  Let ->proc_read() die, it's an ugly
> > kludge, breeding overcomplicated code and buffer overflows.
> 
> What better mechanism ??

seq_file.c and seq_file.h.  Grep around for stuff using it.

Re: read_proc issue

[Laurent]

> We already have better mechanism.  Let ->proc_read() die, it's an ugly
> kludge, breeding overcomplicated code and buffer overflows.

What better mechanism ??

-- 
Laurent Sinitambirivoutin
laurent@augias.org

Re: read_proc issue

[Cort Dougan]

Yes, please kill it!  seq files are much better and don't give people as
much of a chance to do things the wrong way.  Fewer interfaces to /proc
would be vast improvement.

} Oh, for fsck sake...
} 
} We already have better mechanism.  Let ->proc_read() die, it's an ugly
} kludge, breeding overcomplicated code and buffer overflows.
} 
} -
} To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
} the body of a message to majordomo@vger.kernel.org
} More majordomo info at  http://vger.kernel.org/majordomo-info.html
} Please read the FAQ at  http://www.tux.org/lkml/