On Mon, Dec 23, 2019 at 10:26:46PM +0100, Christian Brauner wrote: > On Mon, Dec 23, 2019 at 09:08:55PM +0000, Sargun Dhillon wrote: > > This introduces a function which can be used to fetch a file, given an > > arbitrary task. As long as the user holds a reference (refcnt) to the > > task_struct it is safe to call, and will either return NULL on failure, > > or a pointer to the file, with a refcnt. > > > > Signed-off-by: Sargun Dhillon <sargun@xxxxxxxxx> Could you please add: Suggested-by: Oleg Nesterov <oleg@xxxxxxxxxx> and add a line like: "This is based on a patch sent by Oleg (cf. [1]) a while ago." [1]: Link: https://lore.kernel.org/r/20180915160423.GA31461@xxxxxxxxxx apart from a few nits below: Acked-by: Christian Brauner <christian.brauner@xxxxxxxxxx> > > --- > > fs/file.c | 22 ++++++++++++++++++++-- > > include/linux/file.h | 2 ++ > > 2 files changed, 22 insertions(+), 2 deletions(-) > > > > diff --git a/fs/file.c b/fs/file.c > > index 2f4fcf985079..0ceeb046f4f3 100644 > > --- a/fs/file.c > > +++ b/fs/file.c > > @@ -706,9 +706,9 @@ void do_close_on_exec(struct files_struct *files) > > spin_unlock(&files->file_lock); > > } > > > > -static struct file *__fget(unsigned int fd, fmode_t mask, unsigned int refs) > > +static struct file *__fget_files(struct files_struct *files, unsigned int fd, > > + fmode_t mask, unsigned int refs) > > { > > - struct files_struct *files = current->files; > > struct file *file; > > > > rcu_read_lock(); > > @@ -729,6 +729,11 @@ static struct file *__fget(unsigned int fd, fmode_t mask, unsigned int refs) > > return file; > > } > > > > +static struct file *__fget(unsigned int fd, fmode_t mask, unsigned int refs) This can be: static inline struct file *__fget(...) > > +{ > > + return __fget_files(current->files, fd, mask, refs); > > +} > > + > > struct file *fget_many(unsigned int fd, unsigned int refs) > > { > > return __fget(fd, FMODE_PATH, refs); > > @@ -746,6 +751,19 @@ struct file *fget_raw(unsigned int fd) > > } > > EXPORT_SYMBOL(fget_raw); > > > > +struct file *fget_task(struct task_struct *task, unsigned int fd) > > +{ > > + struct file *file = NULL; > > + > > + task_lock(task); > > + if (task->files) > > + file = __fget_files(task->files, fd, 0, 1); > > + > > + task_unlock(task); > > Nit: remove the \n: > > task_lock(task); > if (task->files) > file = __fget_files(task->files, fd, 0, 1); > task_unlock(task); > > Christian
