* [PATCH 1/1] proc: test lseek on /proc/net/dev
2025-12-26 19:55 [PATCH 1/2] proc: add tgid_iter.pid_ns member Alexey Dobriyan
@ 2025-12-26 19:55 ` Alexey Dobriyan
2025-12-26 19:55 ` [PATCH 2/2] proc: rewrite next_tgid() Alexey Dobriyan
1 sibling, 0 replies; 3+ messages in thread
From: Alexey Dobriyan @ 2025-12-26 19:55 UTC (permalink / raw)
To: akpm; +Cc: linux-fsdevel, linux-kernel, adobriyan
---
tools/testing/selftests/proc/.gitignore | 1 +
tools/testing/selftests/proc/Makefile | 1 +
.../selftests/proc/proc-net-dev-lseek.c | 68 +++++++++++++++++++
3 files changed, 70 insertions(+)
create mode 100644 tools/testing/selftests/proc/proc-net-dev-lseek.c
diff --git a/tools/testing/selftests/proc/.gitignore b/tools/testing/selftests/proc/.gitignore
index 19bb333e2485..243f4537a670 100644
--- a/tools/testing/selftests/proc/.gitignore
+++ b/tools/testing/selftests/proc/.gitignore
@@ -7,6 +7,7 @@
/proc-loadavg-001
/proc-maps-race
/proc-multiple-procfs
+/proc-net-dev-lseek
/proc-empty-vm
/proc-pid-vm
/proc-self-map-files-001
diff --git a/tools/testing/selftests/proc/Makefile b/tools/testing/selftests/proc/Makefile
index 50aba102201a..2a9547630115 100644
--- a/tools/testing/selftests/proc/Makefile
+++ b/tools/testing/selftests/proc/Makefile
@@ -10,6 +10,7 @@ TEST_GEN_PROGS += fd-003-kthread
TEST_GEN_PROGS += proc-2-is-kthread
TEST_GEN_PROGS += proc-loadavg-001
TEST_GEN_PROGS += proc-maps-race
+TEST_GEN_PROGS += proc-net-dev-lseek
TEST_GEN_PROGS += proc-empty-vm
TEST_GEN_PROGS += proc-pid-vm
TEST_GEN_PROGS += proc-self-map-files-001
diff --git a/tools/testing/selftests/proc/proc-net-dev-lseek.c b/tools/testing/selftests/proc/proc-net-dev-lseek.c
new file mode 100644
index 000000000000..742a3e804451
--- /dev/null
+++ b/tools/testing/selftests/proc/proc-net-dev-lseek.c
@@ -0,0 +1,68 @@
+/*
+ * Copyright (c) 2025 Alexey Dobriyan <adobriyan@gmail.com>
+ *
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+#undef _GNU_SOURCE
+#define _GNU_SOURCE
+#undef NDEBUG
+#include <assert.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <string.h>
+#include <unistd.h>
+#include <sched.h>
+/*
+ * Test that lseek("/proc/net/dev/", 0, SEEK_SET)
+ * a) works,
+ * b) does what you think it does.
+ */
+int main(void)
+{
+ /* /proc/net/dev output is deterministic in fresh netns only. */
+ if (unshare(CLONE_NEWNET) == -1) {
+ if (errno == ENOSYS || errno == EPERM) {
+ return 4;
+ }
+ return 1;
+ }
+
+ const int fd = open("/proc/net/dev", O_RDONLY);
+ assert(fd >= 0);
+
+ char buf1[4096];
+ const ssize_t rv1 = read(fd, buf1, sizeof(buf1));
+ /*
+ * Not "<=", this file can't be empty:
+ * there is header, "lo" interface with some zeroes.
+ */
+ assert(0 < rv1);
+ assert(rv1 <= sizeof(buf1));
+
+ /* Believe it or not, this line broke one day. */
+ assert(lseek(fd, 0, SEEK_SET) == 0);
+
+ char buf2[4096];
+ const ssize_t rv2 = read(fd, buf2, sizeof(buf2));
+ /* Not "<=", see above. */
+ assert(0 < rv2);
+ assert(rv2 <= sizeof(buf2));
+
+ /* Test that lseek rewinds to the beginning of the file. */
+ assert(rv1 == rv2);
+ assert(memcmp(buf1, buf2, rv1) == 0);
+
+ /* Contents of the file is not validated: this test is about lseek(). */
+
+ return 0;
+}
--
2.49.1
^ permalink raw reply [flat|nested] 3+ messages in thread* [PATCH 2/2] proc: rewrite next_tgid()
2025-12-26 19:55 [PATCH 1/2] proc: add tgid_iter.pid_ns member Alexey Dobriyan
2025-12-26 19:55 ` [PATCH 1/1] proc: test lseek on /proc/net/dev Alexey Dobriyan
@ 2025-12-26 19:55 ` Alexey Dobriyan
1 sibling, 0 replies; 3+ messages in thread
From: Alexey Dobriyan @ 2025-12-26 19:55 UTC (permalink / raw)
To: akpm; +Cc: linux-fsdevel, linux-kernel, adobriyan
* deduplicate "iter.tgid += 1" line,
it is done once inside next_tgid() itself and second time inside
"for" loop
* deduplicate next_tgid() call itself with different loop style:
auto it = make_xxx_iter();
while (next_xxx(&it)) {
}
gcc seems to inline it twice in the original code:
$ ./scripts/bloat-o-meter ../vmlinux-000 ../obj/vmlinux
add/remove: 0/1 grow/shrink: 1/0 up/down: 100/-245 (-145)
Function old new delta
proc_pid_readdir 531 631 +100
next_tgid 245 - -245
But if there is only one call, it doesn't matter if it is inlined or not!
* make tgid_iter.pid_ns const
it never changes during readdir, returning instance + C99 initializer
make it possible,
* rename "iter" to "it", this is what another language seems to be doing.
* limit declaration scope to prevent problems (in general).
---
fs/proc/base.c | 69 ++++++++++++++++++++++++++++----------------------
1 file changed, 39 insertions(+), 30 deletions(-)
diff --git a/fs/proc/base.c b/fs/proc/base.c
index 7c1089226a47..ddf5e16c795b 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -3535,35 +3535,48 @@ struct dentry *proc_pid_lookup(struct dentry *dentry, unsigned int flags)
/*
* Find the first task with tgid >= tgid
- *
*/
struct tgid_iter {
unsigned int tgid;
struct task_struct *task;
- struct pid_namespace *pid_ns;
+ struct pid_namespace *const pid_ns;
};
-static struct tgid_iter next_tgid(struct tgid_iter iter)
+static
+struct tgid_iter
+make_tgid_iter(unsigned int init_tgid, struct pid_namespace *pid_ns)
{
- struct pid *pid;
+ return (struct tgid_iter){
+ /* See preincrement below. */
+ .tgid = init_tgid - 1,
+ .pid_ns = pid_ns,
+ };
+}
+
+static bool next_tgid(struct tgid_iter *it)
+{
+ if (it->task) {
+ put_task_struct(it->task);
+ it->task = NULL;
+ }
- if (iter.task)
- put_task_struct(iter.task);
rcu_read_lock();
-retry:
- iter.task = NULL;
- pid = find_ge_pid(iter.tgid, iter.pid_ns);
- if (pid) {
- iter.tgid = pid_nr_ns(pid, iter.pid_ns);
- iter.task = pid_task(pid, PIDTYPE_TGID);
- if (!iter.task) {
- iter.tgid += 1;
- goto retry;
+ while (1) {
+ it->tgid += 1;
+ struct pid *pid = find_ge_pid(it->tgid, it->pid_ns);
+ if (pid) {
+ it->tgid = pid_nr_ns(pid, it->pid_ns);
+ it->task = pid_task(pid, PIDTYPE_TGID);
+ if (it->task) {
+ get_task_struct(it->task);
+ rcu_read_unlock();
+ return true;
+ }
+ } else {
+ rcu_read_unlock();
+ return false;
}
- get_task_struct(iter.task);
}
- rcu_read_unlock();
- return iter;
}
#define TGID_OFFSET (FIRST_PROCESS_ENTRY + 2)
@@ -3571,7 +3584,6 @@ static struct tgid_iter next_tgid(struct tgid_iter iter)
/* for the /proc/ directory itself, after non-process stuff has been done */
int proc_pid_readdir(struct file *file, struct dir_context *ctx)
{
- struct tgid_iter iter;
struct proc_fs_info *fs_info = proc_sb_info(file_inode(file)->i_sb);
struct pid_namespace *pid_ns = proc_pid_ns(file_inode(file)->i_sb);
loff_t pos = ctx->pos;
@@ -3589,24 +3601,21 @@ int proc_pid_readdir(struct file *file, struct dir_context *ctx)
return 0;
ctx->pos = pos = pos + 1;
}
- iter.tgid = pos - TGID_OFFSET;
- iter.task = NULL;
- iter.pid_ns = pid_ns;
- for (iter = next_tgid(iter);
- iter.task;
- iter.tgid += 1, iter = next_tgid(iter)) {
+
+ auto it = make_tgid_iter(pos - TGID_OFFSET, pid_ns);
+ while (next_tgid(&it)) {
char name[10 + 1];
unsigned int len;
cond_resched();
- if (!has_pid_permissions(fs_info, iter.task, HIDEPID_INVISIBLE))
+ if (!has_pid_permissions(fs_info, it.task, HIDEPID_INVISIBLE))
continue;
- len = snprintf(name, sizeof(name), "%u", iter.tgid);
- ctx->pos = iter.tgid + TGID_OFFSET;
+ len = snprintf(name, sizeof(name), "%u", it.tgid);
+ ctx->pos = it.tgid + TGID_OFFSET;
if (!proc_fill_cache(file, ctx, name, len,
- proc_pid_instantiate, iter.task, NULL)) {
- put_task_struct(iter.task);
+ proc_pid_instantiate, it.task, NULL)) {
+ put_task_struct(it.task);
return 0;
}
}
--
2.51.2
^ permalink raw reply [flat|nested] 3+ messages in thread