* [PATCH] seq_file: copy as much as possible to user buffer in seq_read()
@ 2024-12-20 4:16 David Wang
2024-12-20 12:13 ` Markus Elfring
2024-12-20 14:08 ` [PATCH v2] " David Wang
0 siblings, 2 replies; 6+ messages in thread
From: David Wang @ 2024-12-20 4:16 UTC (permalink / raw)
To: viro, brauner; +Cc: linux-fsdevel, linux-kernel, surenb, David Wang
seq_read() yields only seq_file->size bytes to userspace, even when user
buffer is prepare to hold more data. This causes lots of extra *read*
syscalls to fetch data from /proc/*.
For example, on an 8-core system, cat /proc/interrupts needs three
*read*:
$ strace -T -e read cat /proc/interrupts > /dev/null
...
43 read(3, " CPU0 CPU1 "..., 131072) = 4082 <0.000068>
44 read(3, " 75: 13490876 0 "..., 131072) = 2936 <0.000148>
45 read(3, "", 131072) = 0 <0.000010>
On a system with hundreds of cpus, it would need tens of more read call.
A more convincing example is /proc/allocinfo, which is available when
CONFIG_MEM_ALLOC_PROFILING=y. When cat /proc/allocinfo, 4k+ lines need ~100
read calls.
This patch try to fill up user buffer as much as possible, extra read
calls can be avoided, and 2%~10% performance improvement would be
observed:
$ strace -T -e read cat /proc/interrupts > /dev/null
...
56 read(3, " CPU0 CPU1 "..., 131072) = 7018 <0.000208>
57 read(3, "", 131072) = 0 <0.000010>
Signed-off-by: David Wang <00107082@163.com>
---
fs/seq_file.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/fs/seq_file.c b/fs/seq_file.c
index 8bbb1ad46335..2cda43aec4a2 100644
--- a/fs/seq_file.c
+++ b/fs/seq_file.c
@@ -220,6 +220,7 @@ ssize_t seq_read_iter(struct kiocb *iocb, struct iov_iter *iter)
if (m->count) // hadn't managed to copy everything
goto Done;
}
+Restart:
// get a non-empty record in the buffer
m->from = 0;
p = m->op->start(m, &m->index);
@@ -282,6 +283,11 @@ ssize_t seq_read_iter(struct kiocb *iocb, struct iov_iter *iter)
copied += n;
m->count -= n;
m->from = n;
+ /*
+ * Keep reading in case more data could be copied into user buffer.
+ */
+ if (m->count == 0)
+ goto Restart;
Done:
if (unlikely(!copied)) {
copied = m->count ? -EFAULT : err;
--
2.39.2
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] seq_file: copy as much as possible to user buffer in seq_read()
2024-12-20 4:16 [PATCH] seq_file: copy as much as possible to user buffer in seq_read() David Wang
@ 2024-12-20 12:13 ` Markus Elfring
2024-12-20 13:54 ` David Wang
2024-12-20 14:08 ` [PATCH v2] " David Wang
1 sibling, 1 reply; 6+ messages in thread
From: Markus Elfring @ 2024-12-20 12:13 UTC (permalink / raw)
To: David Wang, linux-fsdevel, Alexander Viro, Christian Brauner,
Suren Baghdasaryan
Cc: LKML
…
> This patch try to fill up user buffer as much as possible, …
See also:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.13-rc3#n94
Regards,
Markus
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] seq_file: copy as much as possible to user buffer in seq_read()
2024-12-20 4:16 [PATCH] seq_file: copy as much as possible to user buffer in seq_read() David Wang
2024-12-20 12:13 ` Markus Elfring
@ 2024-12-20 14:08 ` David Wang
2024-12-20 14:34 ` Markus Elfring
1 sibling, 1 reply; 6+ messages in thread
From: David Wang @ 2024-12-20 14:08 UTC (permalink / raw)
To: brauner, viro
Cc: linux-fsdevel, linux-kernel, surenb, Markus.Elfring, David Wang
seq_read() yields at most seq_file->size bytes to userspace, even when
user buffer is prepared to hold more data. This causes lots of extra
*read* syscalls to fetch data from /proc/*.
For example, on an 8-core system, cat /proc/interrupts needs three
*read*:
$ strace -T -e read cat /proc/interrupts > /dev/null
...
43 read(3, " CPU0 CPU1 "..., 131072) = 4082 <0.000068>
44 read(3, " 75: 13490876 0 "..., 131072) = 2936 <0.000148>
45 read(3, "", 131072) = 0 <0.000010>
On a system with hundreds of cpus, it would need tens of more read calls.
A more convincing example is /proc/allocinfo, which is available when
CONFIG_MEM_ALLOC_PROFILING=y. When cat /proc/allocinfo, 4k+ lines need ~100
read calls.
Fill up user buffer as much as possible in seq_read(), extra read
calls can be avoided with a larger user buffer, and 2%~10% performance
improvement would be observed:
$ strace -T -e read cat /proc/interrupts > /dev/null
...
56 read(3, " CPU0 CPU1 "..., 131072) = 7018 <0.000208>
57 read(3, "", 131072) = 0 <0.000010>
Signed-off-by: David Wang <00107082@163.com>
---
fs/seq_file.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/fs/seq_file.c b/fs/seq_file.c
index 8bbb1ad46335..2cda43aec4a2 100644
--- a/fs/seq_file.c
+++ b/fs/seq_file.c
@@ -220,6 +220,7 @@ ssize_t seq_read_iter(struct kiocb *iocb, struct iov_iter *iter)
if (m->count) // hadn't managed to copy everything
goto Done;
}
+Restart:
// get a non-empty record in the buffer
m->from = 0;
p = m->op->start(m, &m->index);
@@ -282,6 +283,11 @@ ssize_t seq_read_iter(struct kiocb *iocb, struct iov_iter *iter)
copied += n;
m->count -= n;
m->from = n;
+ /*
+ * Keep reading in case more data could be copied into user buffer.
+ */
+ if (m->count == 0)
+ goto Restart;
Done:
if (unlikely(!copied)) {
copied = m->count ? -EFAULT : err;
--
2.39.2
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2] seq_file: copy as much as possible to user buffer in seq_read()
2024-12-20 14:08 ` [PATCH v2] " David Wang
@ 2024-12-20 14:34 ` Markus Elfring
2024-12-20 16:48 ` David Wang
0 siblings, 1 reply; 6+ messages in thread
From: Markus Elfring @ 2024-12-20 14:34 UTC (permalink / raw)
To: David Wang, linux-fsdevel, Alexander Viro, Christian Brauner
Cc: LKML, Suren Baghdasaryan
> seq_read() yields at most seq_file->size bytes to userspace, …
user space?
…
> $ strace -T -e read cat /proc/interrupts > /dev/null
…
> 45 read(3, "", 131072) = 0 <0.000010>
> On a system with hundreds of cpus, it would need …
CPUs?
Is it a bit nicer to separate test output and subsequent comments by blank lines?
…
> Fill up user buffer as much as possible in seq_read(), extra read
> calls can be avoided with a larger user buffer, and 2%~10% performance
> improvement would be observed:
Will it help to split such a paragraph into three sentences
(on separate lines)?
Regards,
Markus
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] seq_file: copy as much as possible to user buffer in seq_read()
2024-12-20 14:34 ` Markus Elfring
@ 2024-12-20 16:48 ` David Wang
0 siblings, 0 replies; 6+ messages in thread
From: David Wang @ 2024-12-20 16:48 UTC (permalink / raw)
To: Markus Elfring
Cc: linux-fsdevel, Alexander Viro, Christian Brauner, LKML,
Suren Baghdasaryan
At 2024-12-20 22:34:12, "Markus Elfring" <Markus.Elfring@web.de> wrote:
>> seq_read() yields at most seq_file->size bytes to userspace, …
>
> user space?
>
>
>…
>> $ strace -T -e read cat /proc/interrupts > /dev/null
>…
>> 45 read(3, "", 131072) = 0 <0.000010>
>> On a system with hundreds of cpus, it would need …
>
> CPUs?
>
>
>Is it a bit nicer to separate test output and subsequent comments by blank lines?
>
>
>…
>> Fill up user buffer as much as possible in seq_read(), extra read
>> calls can be avoided with a larger user buffer, and 2%~10% performance
>> improvement would be observed:
>Will it help to split such a paragraph into three sentences
>(on separate lines)?
>
>Regards,
>Markus
Thanks for the comments, I will address it later.
Any concern about the code?
David
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-12-20 16:49 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-12-20 4:16 [PATCH] seq_file: copy as much as possible to user buffer in seq_read() David Wang
2024-12-20 12:13 ` Markus Elfring
2024-12-20 13:54 ` David Wang
2024-12-20 14:08 ` [PATCH v2] " David Wang
2024-12-20 14:34 ` Markus Elfring
2024-12-20 16:48 ` David Wang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox