2002-02-26 18:23:33

by Laurent

[permalink] [raw]
Subject: read_proc issue

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
[email protected]


2002-02-27 00:51:11

by Thomas Hood

[permalink] [raw]
Subject: Re: read_proc issue

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



2002-02-27 19:41:53

by Randy.Dunlap

[permalink] [raw]
Subject: Re: read_proc issue

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

2002-02-27 21:06:09

by Valerie Henson

[permalink] [raw]
Subject: Re: read_proc issue

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

2002-02-27 21:29:14

by Alan

[permalink] [raw]
Subject: Re: read_proc issue

> 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"

2002-02-27 21:48:32

by Cort Dougan

[permalink] [raw]
Subject: Re: read_proc issue

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"

2002-02-27 23:33:03

by Laurent

[permalink] [raw]
Subject: Re: read_proc issue

> 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
[email protected]

2002-02-28 00:06:57

by Erik Mouw

[permalink] [raw]
Subject: Re: read_proc issue

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: [email protected]
WWW: http://www-ict.its.tudelft.nl/~erik/

2002-03-01 02:37:29

by Daniel Phillips

[permalink] [raw]
Subject: Re: read_proc issue

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

2002-03-01 07:54:17

by Alexander Viro

[permalink] [raw]
Subject: Re: read_proc issue



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.

2002-03-01 07:18:01

by Erik Mouw

[permalink] [raw]
Subject: Re: read_proc issue

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: [email protected]
WWW: http://www-ict.its.tudelft.nl/~erik/

2002-03-01 08:26:11

by Alexander Viro

[permalink] [raw]
Subject: Re: read_proc issue



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.

2002-03-01 08:30:12

by Laurent

[permalink] [raw]
Subject: Re: read_proc issue

> 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
[email protected]

2002-03-01 19:50:22

by Cort Dougan

[permalink] [raw]
Subject: Re: read_proc issue

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 [email protected]
} More majordomo info at http://vger.kernel.org/majordomo-info.html
} Please read the FAQ at http://www.tux.org/lkml/