2003-07-10 08:07:42

by Vladimir Kondratiev

[permalink] [raw]
Subject: PATCH: seq_file interface to provide large data chunks

seq_file interface, as it exist in last official kernel, never provides
more then one page for each 'read' call. Old read_proc_t did loop to
fill more than one page.

Following patch against 2.4.21 fixes seq_file to provide more than one
page if user requests it.
Many programs do read(large_buffer) once, instead of looping while
read()>0. They work wrong with seq_file. Also, one may expect read() to
provide whole information atomically (OK, relatively to other process
context stuff).
This patch loops over while some space remains in user provided buffer.

I am not subscribed to lkml, thus please cc: me (Vladimir Kondratiev
<[email protected]>) explicitly.

--- linux-2.4.21/fs/seq_file.c 2003-06-13 17:51:37.000000000 +0300
+++ linux/fs/seq_file.c 2003-07-10 10:47:53.000000000 +0300
@@ -55,6 +55,7 @@
return -EPIPE;

down(&m->sem);
+Again:
/* grab buffer if we didn't have one */
if (!m->buf) {
m->buf = kmalloc(m->size = PAGE_SIZE, GFP_KERNEL);
@@ -123,11 +124,14 @@
goto Efault;
copied += n;
m->count -= n;
+ size -= n;
+ buf += n;
if (m->count)
m->from = n;
else
pos++;
m->index = pos;
+ goto Again;
Done:
if (!copied)
copied = err;



2003-07-10 10:58:16

by Alan

[permalink] [raw]
Subject: Re: PATCH: seq_file interface to provide large data chunks

On Iau, 2003-07-10 at 09:19, Vladimir Kondratiev wrote:
> seq_file interface, as it exist in last official kernel, never provides
> more then one page for each 'read' call. Old read_proc_t did loop to
> fill more than one page.

There is a merge of Al's additional seq_file stuff to 2.4 floating
around (its in -ac for one) that may be a better thing to merge instead
if we want it

Al ?

2003-07-10 11:59:43

by Vladimir Kondratiev

[permalink] [raw]
Subject: Re: PATCH: seq_file interface to provide large data chunks

Alan,
I analyzed 2 latest patches from http://www.kernel.org:
patch-2.4.22-pre4 and patch-2.4.22-pre3-ac1.

-pre4 do not touch seq_file;

-pre3-ac1 corrects spelling and adds single_xxx functions. There is no
conflict between this patch and my one. I verified, they do apply in any
order (with some offset).

Vladimir.

Alan Cox wrote:

>On Iau, 2003-07-10 at 09:19, Vladimir Kondratiev wrote:
>
>
>>seq_file interface, as it exist in last official kernel, never provides
>>more then one page for each 'read' call. Old read_proc_t did loop to
>>fill more than one page.
>>
>>
>
>There is a merge of Al's additional seq_file stuff to 2.4 floating
>around (its in -ac for one) that may be a better thing to merge instead
>if we want it
>
>Al ?
>
>
>


2003-07-11 14:59:57

by Matt Mackall

[permalink] [raw]
Subject: Re: PATCH: seq_file interface to provide large data chunks

On Thu, Jul 10, 2003 at 11:19:07AM +0300, Vladimir Kondratiev wrote:
> seq_file interface, as it exist in last official kernel, never provides
> more then one page for each 'read' call. Old read_proc_t did loop to
> fill more than one page.
>
> Following patch against 2.4.21 fixes seq_file to provide more than one
> page if user requests it.
> Many programs do read(large_buffer) once, instead of looping while
> read()>0. They work wrong with seq_file. Also, one may expect read() to
> provide whole information atomically (OK, relatively to other process
> context stuff).
> This patch loops over while some space remains in user provided buffer.
>
> I am not subscribed to lkml, thus please cc: me (Vladimir Kondratiev
> <[email protected]>) explicitly.
>
> +++ linux/fs/seq_file.c 2003-07-10 10:47:53.000000000 +0300
> @@ -55,6 +55,7 @@
> return -EPIPE;
>
> down(&m->sem);
> +Again:
> /* grab buffer if we didn't have one */
> if (!m->buf) {
> m->buf = kmalloc(m->size = PAGE_SIZE, GFP_KERNEL);
> @@ -123,11 +124,14 @@
> goto Efault;
> copied += n;
> m->count -= n;
> + size -= n;
> + buf += n;
> if (m->count)
> m->from = n;
> else
> pos++;
> m->index = pos;
> + goto Again;
> Done:
> if (!copied)
> copied = err;

This patch looks rather tab damaged.

I think it's problematic in that it can hold the semaphore for an
unbounded amount of time.

I'd suggest dropping and acquiring the semaphore each time through the
loop, but then you get into a situation where a second process can
cause the returned results to no longer be coherent. Since we'd no
longer have a way to get single results, this'd be bad.

--
Matt Mackall : http://www.selenic.com : of or relating to the moon

2003-07-15 11:44:39

by Vladimir Kondratiev

[permalink] [raw]
Subject: Re: PATCH: seq_file interface to provide large data chunks

Couple of weeks ago I submitted (for 2.4 series) the patch for seq_file
that fixes read() behavior to be able return more than one page of data.
There was quick mail exchange with Alan, he pointed out that there is
some more seq_file stuff (for "single", by the way). Since that - no any
information, whether this seq_file stuff is going to be included or, if
not, at least any reason. 2.4.22-pre6 released with no seq_file changes.

Regarding to this, I have more generic question: is it some way to track
status of patches? Accepted/rejected, scheduled for xxx version? For 2.5
kernel, Bugzilla provides this feature. What for 2.4?

Vladimir.