| Message ID | 20241220140819.9887-1-00107082@163.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] seq_file: copy as much as possible to user buffer in seq_read() | expand |
> 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
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
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;
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(+)