hello list,
when doing "cat /proc/uptime" or similar files, the
routine which prepares the buffer gets called twice.
this is because "cat" reads two times from the file,
as a system-call-trace shows:
# strace -f cat /proc/uptime
...
read(3, "5241.09 5082.74\n", 1024) = 16
...
read(3, "", 1024) = 0
...
this leads to uptime_read_proc() being called two times,
the second time with parameter off=16, but since this
is ignored, the work is done twice: clock_...gettime(),
cputime_to_timespec(), sprintf(), proc_calc_metrics()
and so on.
insert a printk-statement if you don't beleive this.
btw, there's no wrong information produced because of this,
because *page points to the same location both calls.
a simple way to get rid of this:
static int uptime_read_proc(char *page, char **start, off_t off,
int count, int *eof, void *data)
{
struct timespec uptime;
struct timespec idle;
int len;
cputime_t idletime;
+ if (off)
+ return 0;
cputime_add(init_task.utime, init_task.stime);
do_posix_clock_monotonic_gettime(&uptime);
cputime_to_timespec(idletime, &idle);
len = sprintf(page,"%lu.%02lu %lu.%02lu\n",
...
and so on.
this affects possibly all /proc files which ignore the offset parameter
and are evaluated(?) with "cat".
regards,
h.rosmanith
Herbert Rosmanith wrote:
> a simple way to get rid of this:
>
> static int uptime_read_proc(char *page, char **start, off_t off,
> int count, int *eof, void *data)
> {
> struct timespec uptime;
> struct timespec idle;
> int len;
> cputime_t idletime;
>
> + if (off)
> + return 0;
Except that this is wrong - if you try to advance the offset a bit from
the start of the file and read something, you'll get nothing. This is
inconsistent with normal file behavior.
--
Robert Hancock Saskatoon, SK, Canada
To email, remove "nospam" from [email protected]
Home Page: http://www.roberthancock.com/
> > static int uptime_read_proc(char *page, char **start, off_t off,
> > int count, int *eof, void *data)
> > {
> > struct timespec uptime;
> > struct timespec idle;
> > int len;
> > cputime_t idletime;
> >
> > + if (off)
> > + return 0;
>
> Except that this is wrong - if you try to advance the offset
> a bit from
> the start of the file and read something, you'll get nothing. This is
> inconsistent with normal file behavior.
Right... What's weird is : what do we get if a process decides to read
this using a 1 byte buffer, asking for 1 char at a time ?
And what we'll be the result if you read 1 char every 1 second ?
#include <stdio.h>
int main(int argc, char * argv[])
{
FILE * f;
char lChar;
f = fopen("/proc/uptime", "r");
if (f == NULL) {
exit(0);
} /* endif */
while (!feof(f)) {
fread(&lChar, 1, 1, f);
fprintf(stdout, "%c", lChar); fflush(stdout);
sleep(1);
} /* endwhile */
close(f);
exit(0);
}
is funny enough...
2.2.x :
58 [15:30] rol@www-dev:/tmp> cat /proc/uptime ; ./test
13849305.25 13555633.92
13849312.38 13555635.64
2.4.31 :
bash-2.05# cat /proc/uptime ; ./test
100711.77 100366.30
100711.77 100366.30
Paul
Robert Hancock explained what's wrong with your proposal of:
+ if (off)
+ return 0;
For an alternative that seems to work better, see the processing
behind the /proc/<pid>/cpuset file, by grep'ing for seq_file in
kernel/cpuset.c.
Essentially, it composes the result string on the open, and then
lets user code read and seek over that string at will.
--
I won't rest till it's the best ...
Programmer, Linux Scalability
Paul Jackson <[email protected]> 1.925.600.0401
Paul Rolland wrote:
> is funny enough...
You used the stdio routine fread - which buffers in user space. It
does a single read, and then feeds you the characters as you ask for
them, out of its stdio buffer.
Try the following program, which doesn't buffer:
main()
{
char c;
int fd = open("/proc/uptime", 0);
while (read(fd, &c, 1) == 1) {
write(1, &c, 1);
sleep(1);
}
}
--
I won't rest till it's the best ...
Programmer, Linux Scalability
Paul Jackson <[email protected]> 1.925.600.0401
Hello,
> main()
> {
> char c;
> int fd = open("/proc/uptime", 0);
> while (read(fd, &c, 1) == 1) {
> write(1, &c, 1);
> sleep(1);
> }
> }
Yes, much better, same result for 2.2.x and 2.4.31 :
bash-2.05# cat /proc/uptime; ./test2
112049.81 111665.11
112054.89 111670.29
The result is more "correct", but I guess this makes it worse for
Herbert, as uptime_read_proc() is then called for each character...
Paul
> > static int uptime_read_proc(char *page, char **start, off_t off,
> > int count, int *eof, void *data)
> > {
> > struct timespec uptime;
> > struct timespec idle;
> > int len;
> > cputime_t idletime;
> >
> > + if (off)
> > + return 0;
>
> Except that this is wrong - if you try to advance the offset a bit from
> the start of the file and read something, you'll get nothing. This is
> inconsistent with normal file behavior.
so what - "uptime_read_proc" ignores the offset parameter anyway.
you get wrong results right now, too, even without the two lines
I've added.
if you write "Except that this is wrong" you should have in mind that
"this is wrong" currently, too.
just "try to advance the offset a bit from the start of the file and
read something", like "dd if=/proc/uptime bs=1 count=1 seek=1".
do you expect to get right results?
regards,
h.rosmanith
> > > static int uptime_read_proc(char *page, char **start, off_t off,
> > > int count, int *eof, void *data)
> > > {
> > > struct timespec uptime;
> > > struct timespec idle;
> > > int len;
> > > cputime_t idletime;
> > >
> > > + if (off)
> > > + return 0;
> >
> > Except that this is wrong - if you try to advance the offset a bit from
> > the start of the file and read something, you'll get nothing. This is
> > inconsistent with normal file behavior.
>
> so what - "uptime_read_proc" ignores the offset parameter anyway.
> you get wrong results right now, too, even without the two lines
> I've added.
>
> if you write "Except that this is wrong" you should have in mind that
> "this is wrong" currently, too.
>
> just "try to advance the offset a bit from the start of the file and
> read something", like "dd if=/proc/uptime bs=1 count=1 seek=1".
> do you expect to get right results?
argh, my mistake, sorry. I see. I tried reading a character at a time
and with the two lines, /proc/uptime will return only a single character.
so even though currently the update-routine is called more than
needed, my patch is even more wrong.
but this means that e.g.:
int main() {
int fd;
char ch;
fd=open("/proc/uptime",0);
for(;;) {
if (read(fd,&ch,1)!=1)
break;
write(1,&ch,1);
}
}
will call the buffer-formatting routines 16 times on the whole
buffer, just to return 1 single character, and each call will
return a different result. eeks...
by the way: why does "dd if=/proc/uptime bs=1 seek=1" hang?
e.g.:
root@afp ~ # strace -f dd if=/proc/uptime bs=1 count=1 seek=1
open("/proc/uptime", O_RDONLY|O_LARGEFILE) = 0
...
_llseek(1, 1, 0xbfaa8464, SEEK_CUR) = -1 ESPIPE (Illegal seek)
read(1, <unfinished ...>
processing stops here.
regards,
h.rosmanith
regards,
h.rosmanith