* 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