Skip to content

Commit 8fd3395

Browse files
author
Al Viro
committed
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>
1 parent 8cf0b93 commit 8fd3395

9 files changed

Lines changed: 14 additions & 62 deletions

File tree

arch/powerpc/platforms/cell/spufs/coredump.c

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -73,9 +73,7 @@ static struct spu_context *coredump_next_context(int *fd)
7373
return NULL;
7474
*fd = n - 1;
7575

76-
rcu_read_lock();
77-
file = lookup_fdget_rcu(*fd);
78-
rcu_read_unlock();
76+
file = fget_raw(*fd);
7977
if (file) {
8078
ctx = SPUFS_I(file_inode(file))->i_ctx;
8179
get_spu_context(ctx);

fs/file.c

Lines changed: 4 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1037,29 +1037,7 @@ struct file *fget_task(struct task_struct *task, unsigned int fd)
10371037
return file;
10381038
}
10391039

1040-
struct file *lookup_fdget_rcu(unsigned int fd)
1041-
{
1042-
return __fget_files_rcu(current->files, fd, 0);
1043-
1044-
}
1045-
EXPORT_SYMBOL_GPL(lookup_fdget_rcu);
1046-
1047-
struct file *task_lookup_fdget_rcu(struct task_struct *task, unsigned int fd)
1048-
{
1049-
/* Must be called with rcu_read_lock held */
1050-
struct files_struct *files;
1051-
struct file *file = NULL;
1052-
1053-
task_lock(task);
1054-
files = task->files;
1055-
if (files)
1056-
file = __fget_files_rcu(files, fd, 0);
1057-
task_unlock(task);
1058-
1059-
return file;
1060-
}
1061-
1062-
struct file *task_lookup_next_fdget_rcu(struct task_struct *task, unsigned int *ret_fd)
1040+
struct file *fget_task_next(struct task_struct *task, unsigned int *ret_fd)
10631041
{
10641042
/* Must be called with rcu_read_lock held */
10651043
struct files_struct *files;
@@ -1069,17 +1047,19 @@ struct file *task_lookup_next_fdget_rcu(struct task_struct *task, unsigned int *
10691047
task_lock(task);
10701048
files = task->files;
10711049
if (files) {
1050+
rcu_read_lock();
10721051
for (; fd < files_fdtable(files)->max_fds; fd++) {
10731052
file = __fget_files_rcu(files, fd, 0);
10741053
if (file)
10751054
break;
10761055
}
1056+
rcu_read_unlock();
10771057
}
10781058
task_unlock(task);
10791059
*ret_fd = fd;
10801060
return file;
10811061
}
1082-
EXPORT_SYMBOL(task_lookup_next_fdget_rcu);
1062+
EXPORT_SYMBOL(fget_task_next);
10831063

10841064
/*
10851065
* Lightweight file lookup - no refcnt increment if fd table isn't shared.

fs/gfs2/glock.c

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@
3434
#include <linux/lockref.h>
3535
#include <linux/rhashtable.h>
3636
#include <linux/pid_namespace.h>
37-
#include <linux/fdtable.h>
3837
#include <linux/file.h>
3938

4039
#include "gfs2.h"
@@ -2768,25 +2767,18 @@ static struct file *gfs2_glockfd_next_file(struct gfs2_glockfd_iter *i)
27682767
i->file = NULL;
27692768
}
27702769

2771-
rcu_read_lock();
27722770
for(;; i->fd++) {
2773-
struct inode *inode;
2774-
2775-
i->file = task_lookup_next_fdget_rcu(i->task, &i->fd);
2771+
i->file = fget_task_next(i->task, &i->fd);
27762772
if (!i->file) {
27772773
i->fd = 0;
27782774
break;
27792775
}
27802776

2781-
inode = file_inode(i->file);
2782-
if (inode->i_sb == i->sb)
2777+
if (file_inode(i->file)->i_sb == i->sb)
27832778
break;
27842779

2785-
rcu_read_unlock();
27862780
fput(i->file);
2787-
rcu_read_lock();
27882781
}
2789-
rcu_read_unlock();
27902782
return i->file;
27912783
}
27922784

fs/notify/dnotify/dnotify.c

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616
#include <linux/security.h>
1717
#include <linux/spinlock.h>
1818
#include <linux/slab.h>
19-
#include <linux/fdtable.h>
2019
#include <linux/fsnotify_backend.h>
2120

2221
static int dir_notify_enable __read_mostly = 1;
@@ -347,9 +346,7 @@ int fcntl_dirnotify(int fd, struct file *filp, unsigned int arg)
347346
new_fsn_mark = NULL;
348347
}
349348

350-
rcu_read_lock();
351-
f = lookup_fdget_rcu(fd);
352-
rcu_read_unlock();
349+
f = fget_raw(fd);
353350

354351
/* if (f != filp) means that we lost a race and another task/thread
355352
* actually closed the fd we are still playing with before we grabbed

fs/proc/fd.c

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -116,9 +116,7 @@ static bool tid_fd_mode(struct task_struct *task, unsigned fd, fmode_t *mode)
116116
{
117117
struct file *file;
118118

119-
rcu_read_lock();
120-
file = task_lookup_fdget_rcu(task, fd);
121-
rcu_read_unlock();
119+
file = fget_task(task, fd);
122120
if (file) {
123121
*mode = file->f_mode;
124122
fput(file);
@@ -258,31 +256,27 @@ static int proc_readfd_common(struct file *file, struct dir_context *ctx,
258256
if (!dir_emit_dots(file, ctx))
259257
goto out;
260258

261-
rcu_read_lock();
262259
for (fd = ctx->pos - 2;; fd++) {
263260
struct file *f;
264261
struct fd_data data;
265262
char name[10 + 1];
266263
unsigned int len;
267264

268-
f = task_lookup_next_fdget_rcu(p, &fd);
265+
f = fget_task_next(p, &fd);
269266
ctx->pos = fd + 2LL;
270267
if (!f)
271268
break;
272269
data.mode = f->f_mode;
273-
rcu_read_unlock();
274270
fput(f);
275271
data.fd = fd;
276272

277273
len = snprintf(name, sizeof(name), "%u", fd);
278274
if (!proc_fill_cache(file, ctx,
279275
name, len, instantiate, p,
280276
&data))
281-
goto out;
277+
break;
282278
cond_resched();
283-
rcu_read_lock();
284279
}
285-
rcu_read_unlock();
286280
out:
287281
put_task_struct(p);
288282
return 0;

include/linux/fdtable.h

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -92,10 +92,6 @@ static inline struct file *files_lookup_fd_locked(struct files_struct *files, un
9292
return files_lookup_fd_raw(files, fd);
9393
}
9494

95-
struct file *lookup_fdget_rcu(unsigned int fd);
96-
struct file *task_lookup_fdget_rcu(struct task_struct *task, unsigned int fd);
97-
struct file *task_lookup_next_fdget_rcu(struct task_struct *task, unsigned int *fd);
98-
9995
static inline bool close_on_exec(unsigned int fd, const struct files_struct *files)
10096
{
10197
return test_bit(fd, files_fdtable(files)->close_on_exec);

include/linux/file.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ static inline void fdput(struct fd fd)
7272
extern struct file *fget(unsigned int fd);
7373
extern struct file *fget_raw(unsigned int fd);
7474
extern struct file *fget_task(struct task_struct *task, unsigned int fd);
75+
extern struct file *fget_task_next(struct task_struct *task, unsigned int *fd);
7576
extern void __f_unlock_pos(struct file *);
7677

7778
struct fd fdget(unsigned int fd);

kernel/bpf/task_iter.c

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
#include <linux/namei.h>
66
#include <linux/pid_namespace.h>
77
#include <linux/fs.h>
8-
#include <linux/fdtable.h>
98
#include <linux/filter.h>
109
#include <linux/bpf_mem_alloc.h>
1110
#include <linux/btf_ids.h>
@@ -286,17 +285,14 @@ task_file_seq_get_next(struct bpf_iter_seq_task_file_info *info)
286285
curr_fd = 0;
287286
}
288287

289-
rcu_read_lock();
290-
f = task_lookup_next_fdget_rcu(curr_task, &curr_fd);
288+
f = fget_task_next(curr_task, &curr_fd);
291289
if (f) {
292290
/* set info->fd */
293291
info->fd = curr_fd;
294-
rcu_read_unlock();
295292
return f;
296293
}
297294

298295
/* the current task is done, go to the next task */
299-
rcu_read_unlock();
300296
put_task_struct(curr_task);
301297

302298
if (info->common.type == BPF_TASK_ITER_TID) {

kernel/kcmp.c

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -63,9 +63,7 @@ get_file_raw_ptr(struct task_struct *task, unsigned int idx)
6363
{
6464
struct file *file;
6565

66-
rcu_read_lock();
67-
file = task_lookup_fdget_rcu(task, idx);
68-
rcu_read_unlock();
66+
file = fget_task(task, idx);
6967
if (file)
7068
fput(file);
7169

0 commit comments

Comments
 (0)