get rid of ...lookup...fdget_rcu() family

Once upon a time, predecessors of those used to do file lookup
without bumping a refcount, provided that caller held rcu_read_lock()
across the lookup and whatever it wanted to read from the struct
file found.  When struct file allocation switched to SLAB_TYPESAFE_BY_RCU,
that stopped being feasible and these primitives started to bump the
file refcount for lookup result, requiring the caller to call fput()
afterwards.

But that turned them pointless - e.g.
	rcu_read_lock();
	file = lookup_fdget_rcu(fd);
	rcu_read_unlock();
is equivalent to
	file = fget_raw(fd);
and all callers of lookup_fdget_rcu() are of that form.  Similarly,
task_lookup_fdget_rcu() calls can be replaced with calling fget_task().
task_lookup_next_fdget_rcu() doesn't have direct counterparts, but
its callers would be happier if we replaced it with an analogue that
deals with RCU internally.

Reviewed-by: Christian Brauner <brauner@kernel.org>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

diff --git a/arch/powerpc/platforms/cell/spufs/coredump.c b/arch/powerpc/platforms/cell/spufs/coredump.c
index 18daafbe2e65eeed81b84f42b6a7c916e6b41d0a..301ee7d8b7df024854f4cb13204d8fcd6199de3e 100644 (file)
--- a/arch/powerpc/platforms/cell/spufs/coredump.c
+++ b/arch/powerpc/platforms/cell/spufs/coredump.c
@@ -73,9 +73,7 @@ static struct spu_context *coredump_next_context(int *fd)
                return NULL;
        *fd = n - 1;
 
-       rcu_read_lock();
-       file = lookup_fdget_rcu(*fd);
-       rcu_read_unlock();
+       file = fget_raw(*fd);
        if (file) {
                ctx = SPUFS_I(file_inode(file))->i_ctx;
                get_spu_context(ctx);

diff --git a/fs/file.c b/fs/file.c
index eb093e73697206c99f290fd9bea1e9e5fba52beb..991860ee7848256cbbd9d521158a38fafd90258c 100644 (file)
--- a/fs/file.c
+++ b/fs/file.c
@@ -1037,29 +1037,7 @@ struct file *fget_task(struct task_struct *task, unsigned int fd)
        return file;
 }
 
-struct file *lookup_fdget_rcu(unsigned int fd)
-{
-       return __fget_files_rcu(current->files, fd, 0);
-
-}
-EXPORT_SYMBOL_GPL(lookup_fdget_rcu);
-
-struct file *task_lookup_fdget_rcu(struct task_struct *task, unsigned int fd)
-{
-       /* Must be called with rcu_read_lock held */
-       struct files_struct *files;
-       struct file *file = NULL;
-
-       task_lock(task);
-       files = task->files;
-       if (files)
-               file = __fget_files_rcu(files, fd, 0);
-       task_unlock(task);
-
-       return file;
-}
-
-struct file *task_lookup_next_fdget_rcu(struct task_struct *task, unsigned int *ret_fd)
+struct file *fget_task_next(struct task_struct *task, unsigned int *ret_fd)
 {
        /* Must be called with rcu_read_lock held */
        struct files_struct *files;
@@ -1069,17 +1047,19 @@ struct file *task_lookup_next_fdget_rcu(struct task_struct *task, unsigned int *
        task_lock(task);
        files = task->files;
        if (files) {
+               rcu_read_lock();
                for (; fd < files_fdtable(files)->max_fds; fd++) {
                        file = __fget_files_rcu(files, fd, 0);
                        if (file)
                                break;
                }
+               rcu_read_unlock();
        }
        task_unlock(task);
        *ret_fd = fd;
        return file;
 }
-EXPORT_SYMBOL(task_lookup_next_fdget_rcu);
+EXPORT_SYMBOL(fget_task_next);
 
 /*
  * Lightweight file lookup - no refcnt increment if fd table isn't shared.

diff --git a/fs/gfs2/glock.c b/fs/gfs2/glock.c
index 269c3bc7fced7133c3e893bc8f2c471946c0ce35..4701c4aafbf4b40485bbacfd2c1132c8f8fccb25 100644 (file)
--- a/fs/gfs2/glock.c
+++ b/fs/gfs2/glock.c
@@ -34,7 +34,6 @@
 #include <linux/lockref.h>
 #include <linux/rhashtable.h>
 #include <linux/pid_namespace.h>
-#include <linux/fdtable.h>
 #include <linux/file.h>
 
 #include "gfs2.h"
@@ -2768,25 +2767,18 @@ static struct file *gfs2_glockfd_next_file(struct gfs2_glockfd_iter *i)
                i->file = NULL;
        }
 
-       rcu_read_lock();
        for(;; i->fd++) {
-               struct inode *inode;
-
-               i->file = task_lookup_next_fdget_rcu(i->task, &i->fd);
+               i->file = fget_task_next(i->task, &i->fd);
                if (!i->file) {
                        i->fd = 0;
                        break;
                }
 
-               inode = file_inode(i->file);
-               if (inode->i_sb == i->sb)
+               if (file_inode(i->file)->i_sb == i->sb)
                        break;
 
-               rcu_read_unlock();
                fput(i->file);
-               rcu_read_lock();
        }
-       rcu_read_unlock();
        return i->file;
 }
 

diff --git a/fs/notify/dnotify/dnotify.c b/fs/notify/dnotify/dnotify.c
index d5dbef7f5c95bb522c69b42982a0e728bc75b5f9..6004dfdfdf0f3c6222aad9cf2697dc1cbe0a50cc 100644 (file)
--- a/fs/notify/dnotify/dnotify.c
+++ b/fs/notify/dnotify/dnotify.c
@@ -16,7 +16,6 @@
 #include <linux/security.h>
 #include <linux/spinlock.h>
 #include <linux/slab.h>
-#include <linux/fdtable.h>
 #include <linux/fsnotify_backend.h>
 
 static int dir_notify_enable __read_mostly = 1;
@@ -347,9 +346,7 @@ int fcntl_dirnotify(int fd, struct file *filp, unsigned int arg)
                new_fsn_mark = NULL;
        }
 
-       rcu_read_lock();
-       f = lookup_fdget_rcu(fd);
-       rcu_read_unlock();
+       f = fget_raw(fd);
 
        /* if (f != filp) means that we lost a race and another task/thread
         * actually closed the fd we are still playing with before we grabbed

diff --git a/fs/proc/fd.c b/fs/proc/fd.c
index 1f54a54bfb91304914d90d1730588e4876aa4236..18d0dddc8e2fddc9a8e96a1592f55a620bbaf1b4 100644 (file)
--- a/fs/proc/fd.c
+++ b/fs/proc/fd.c
@@ -116,9 +116,7 @@ static bool tid_fd_mode(struct task_struct *task, unsigned fd, fmode_t *mode)
 {
        struct file *file;
 
-       rcu_read_lock();
-       file = task_lookup_fdget_rcu(task, fd);
-       rcu_read_unlock();
+       file = fget_task(task, fd);
        if (file) {
                *mode = file->f_mode;
                fput(file);
@@ -258,19 +256,17 @@ static int proc_readfd_common(struct file *file, struct dir_context *ctx,
        if (!dir_emit_dots(file, ctx))
                goto out;
 
-       rcu_read_lock();
        for (fd = ctx->pos - 2;; fd++) {
                struct file *f;
                struct fd_data data;
                char name[10 + 1];
                unsigned int len;
 
-               f = task_lookup_next_fdget_rcu(p, &fd);
+               f = fget_task_next(p, &fd);
                ctx->pos = fd + 2LL;
                if (!f)
                        break;
                data.mode = f->f_mode;
-               rcu_read_unlock();
                fput(f);
                data.fd = fd;
 
@@ -278,11 +274,9 @@ static int proc_readfd_common(struct file *file, struct dir_context *ctx,
                if (!proc_fill_cache(file, ctx,
                                     name, len, instantiate, p,
                                     &data))
-                       goto out;
+                       break;
                cond_resched();
-               rcu_read_lock();
        }
-       rcu_read_unlock();
 out:
        put_task_struct(p);
        return 0;

diff --git a/include/linux/fdtable.h b/include/linux/fdtable.h
index b1c5722f2b3ce4ba82e89716d5e27f85c67a8afd..e25e2cb65d30c96e6e10f53621e78967cb11494f 100644 (file)
--- a/include/linux/fdtable.h
+++ b/include/linux/fdtable.h
@@ -92,10 +92,6 @@ static inline struct file *files_lookup_fd_locked(struct files_struct *files, un
        return files_lookup_fd_raw(files, fd);
 }
 
-struct file *lookup_fdget_rcu(unsigned int fd);
-struct file *task_lookup_fdget_rcu(struct task_struct *task, unsigned int fd);
-struct file *task_lookup_next_fdget_rcu(struct task_struct *task, unsigned int *fd);
-
 static inline bool close_on_exec(unsigned int fd, const struct files_struct *files)
 {
        return test_bit(fd, files_fdtable(files)->close_on_exec);

diff --git a/include/linux/file.h b/include/linux/file.h
index f98de143245abc1361f6e7118bc415ba0206d28a..ec4ad5e6a06157bae58b1cd6a61b2c60f282c6e6 100644 (file)
--- a/include/linux/file.h
+++ b/include/linux/file.h
@@ -72,6 +72,7 @@ static inline void fdput(struct fd fd)
 extern struct file *fget(unsigned int fd);
 extern struct file *fget_raw(unsigned int fd);
 extern struct file *fget_task(struct task_struct *task, unsigned int fd);
+extern struct file *fget_task_next(struct task_struct *task, unsigned int *fd);
 extern void __f_unlock_pos(struct file *);
 
 struct fd fdget(unsigned int fd);

diff --git a/kernel/bpf/task_iter.c b/kernel/bpf/task_iter.c
index 02aa9db8d79616d3e0ebe62622a8738f41d34f28..7fe602ca74a064f7951ac9ea83f80f920b07ef2d 100644 (file)
--- a/kernel/bpf/task_iter.c
+++ b/kernel/bpf/task_iter.c
@@ -5,7 +5,6 @@
 #include <linux/namei.h>
 #include <linux/pid_namespace.h>
 #include <linux/fs.h>
-#include <linux/fdtable.h>
 #include <linux/filter.h>
 #include <linux/bpf_mem_alloc.h>
 #include <linux/btf_ids.h>
@@ -286,17 +285,14 @@ again:
                        curr_fd = 0;
        }
 
-       rcu_read_lock();
-       f = task_lookup_next_fdget_rcu(curr_task, &curr_fd);
+       f = fget_task_next(curr_task, &curr_fd);
        if (f) {
                /* set info->fd */
                info->fd = curr_fd;
-               rcu_read_unlock();
                return f;
        }
 
        /* the current task is done, go to the next task */
-       rcu_read_unlock();
        put_task_struct(curr_task);
 
        if (info->common.type == BPF_TASK_ITER_TID) {

diff --git a/kernel/kcmp.c b/kernel/kcmp.c
index b0639f21041f0720db9e0915c0e8559a785fec11..2c596851f8a90c042f3c63b320cc1ec683a5a341 100644 (file)
--- a/kernel/kcmp.c
+++ b/kernel/kcmp.c
@@ -63,9 +63,7 @@ get_file_raw_ptr(struct task_struct *task, unsigned int idx)
 {
        struct file *file;
 
-       rcu_read_lock();
-       file = task_lookup_fdget_rcu(task, idx);
-       rcu_read_unlock();
+       file = fget_task(task, idx);
        if (file)
                fput(file);