diff options
author | Oleg Nesterov <oleg@redhat.com> | 2014-01-11 13:19:32 -0500 |
---|---|---|
committer | Al Viro <viro@zeniv.linux.org.uk> | 2014-01-25 03:14:36 -0500 |
commit | a8d4b8345e0ee48b732126d980efaf0dc373e2b0 (patch) | |
tree | 4bdfc52ef4ad176f54e4ce7b81091c0fbef0fd04 /fs/file.c | |
parent | 2ccdc413196b43a02bb68b46be5b68850904e9ea (diff) |
introduce __fcheck_files() to fix rcu_dereference_check_fdtable(), kill rcu_my_thread_group_empty()
rcu_dereference_check_fdtable() looks very wrong,
1. rcu_my_thread_group_empty() was added by 844b9a8707f1 "vfs: fix
RCU-lockdep false positive due to /proc" but it doesn't really
fix the problem. A CLONE_THREAD (without CLONE_FILES) task can
hit the same race with get_files_struct().
And otoh rcu_my_thread_group_empty() can suppress the correct
warning if the caller is the CLONE_FILES (without CLONE_THREAD)
task.
2. files->count == 1 check is not really right too. Even if this
files_struct is not shared it is not safe to access it lockless
unless the caller is the owner.
Otoh, this check is sub-optimal. files->count == 0 always means
it is safe to use it lockless even if files != current->files,
but put_files_struct() has to take rcu_read_lock(). See the next
patch.
This patch removes the buggy checks and turns fcheck_files() into
__fcheck_files() which uses rcu_dereference_raw(), the "unshared"
callers, fget_light() and fget_raw_light(), can use it to avoid
the warning from RCU-lockdep.
fcheck_files() is trivially reimplemented as rcu_lockdep_assert()
plus __fcheck_files().
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Diffstat (limited to 'fs/file.c')
-rw-r--r-- | fs/file.c | 4 |
1 files changed, 2 insertions, 2 deletions
@@ -707,7 +707,7 @@ struct file *fget_light(unsigned int fd, int *fput_needed) | |||
707 | 707 | ||
708 | *fput_needed = 0; | 708 | *fput_needed = 0; |
709 | if (atomic_read(&files->count) == 1) { | 709 | if (atomic_read(&files->count) == 1) { |
710 | file = fcheck_files(files, fd); | 710 | file = __fcheck_files(files, fd); |
711 | if (file && (file->f_mode & FMODE_PATH)) | 711 | if (file && (file->f_mode & FMODE_PATH)) |
712 | file = NULL; | 712 | file = NULL; |
713 | } else { | 713 | } else { |
@@ -735,7 +735,7 @@ struct file *fget_raw_light(unsigned int fd, int *fput_needed) | |||
735 | 735 | ||
736 | *fput_needed = 0; | 736 | *fput_needed = 0; |
737 | if (atomic_read(&files->count) == 1) { | 737 | if (atomic_read(&files->count) == 1) { |
738 | file = fcheck_files(files, fd); | 738 | file = __fcheck_files(files, fd); |
739 | } else { | 739 | } else { |
740 | rcu_read_lock(); | 740 | rcu_read_lock(); |
741 | file = fcheck_files(files, fd); | 741 | file = fcheck_files(files, fd); |