* Al Viro ([email protected]) wrote: > On Fri, Jan 25, 2013 at 09:49:53AM -0500, Mathieu Desnoyers wrote: > > static > > void lttng_enumerate_task_fd(struct lttng_session *session, > > struct task_struct *p, char *tmp) > > { > > struct fdtable *fdt; > > struct file *filp; > > unsigned int i; > > const unsigned char *path; > > > > task_lock(p); > > if (!p->files) > > goto unlock_task; > > spin_lock(&p->files->file_lock); > > fdt = files_fdtable(p->files); > > for (i = 0; i < fdt->max_fds; i++) { > > filp = fcheck_files(p->files, i); > > if (!filp) > > continue; > > path = d_path(&filp->f_path, tmp, PAGE_SIZE); > > /* Make sure we give at least some info */ > > trace_lttng_statedump_file_descriptor(session, p, i, > > IS_ERR(path) ? > > filp->f_dentry->d_name.name : > > path); > > } > > spin_unlock(&p->files->file_lock); > > unlock_task: > > task_unlock(p); > > } > > *cringe* > > a) yes, it needs d_lock for that ->d_name access > b) iterate_fd() is there for purpose; use it, instead of open-coding the > damn loop. Something like > > struct ctx { > char *page; > struct lttng_session *session, > struct task_struct *p; > }; > > static int dump_one(void *p, struct file *file, unsigned fd) > { > struct ctx *ctx = p; > const char *s = d_path(&file->f_path, ctx->page, PAGE_SIZE); > struct dentry *dentry; > if (!IS_ERR(s)) { > trace_lttng_statedump_file_descriptor(ctx->session, ctx->p, fd, > s); > return 0; > } > /* Make sure we give at least some info */ > dentry = file->f_path.dentry; > spin_lock(&dentry->d_lock); > trace_lttng_statedump_file_descriptor(ctx->session, ctx->p, fd, > dentry->d_name); > spin_unlock(&dentry->d_lock); > return 0; > } > > ... > task_lock(p); > iterate_fd(p->files, 0, dump_one, &(struct ctx){tmp, session, p}); > task_unlock(p); > > assuming it wouldn't be better to pass tmp/session/p as the single pointer > to struct in the first place - I don't know enough about the callers of > that sucker to tell. And yes, iterate_fd() will DTRT if given NULL as the > first argument. The second argument is "which descriptor should I start > from?", callback is called for everything present in the table starting from > that place until it returns non-zero or the end of table is reached...
Thanks !! Modulo a couple of trivial nits, I've integrated your suggestions. I'm creating a lttng_iterate_fd() wrapper for older kernels (yeah.. we deal with kernels back to 2.6.32). Mathieu -- Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
