Commit a8d4b834 authored by Oleg Nesterov's avatar Oleg Nesterov Committed by Al Viro
Browse files

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 844b9a87 "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: default avatarOleg Nesterov <oleg@redhat.com>
Signed-off-by: default avatarAl Viro <viro@zeniv.linux.org.uk>
parent 2ccdc413

fs/file.c

+2 −2
Original line number Diff line number Diff line
@@ -707,7 +707,7 @@ struct file *fget_light(unsigned int fd, int *fput_needed) 


	*fput_needed = 0;

	if (atomic_read(&files->count) == 1) {

		file = fcheck_files(files, fd);

		file = __fcheck_files(files, fd);

		if (file && (file->f_mode & FMODE_PATH))

			file = NULL;

	} else {
@@ -735,7 +735,7 @@ struct file *fget_raw_light(unsigned int fd, int *fput_needed) 


	*fput_needed = 0;

	if (atomic_read(&files->count) == 1) {

		file = fcheck_files(files, fd);

		file = __fcheck_files(files, fd);

	} else {

		rcu_read_lock();

		file = fcheck_files(files, fd);

include/linux/fdtable.h

+21 −14
Original line number Diff line number Diff line
@@ -59,29 +59,36 @@ struct files_struct { 
	struct file __rcu * fd_array[NR_OPEN_DEFAULT];

};



#define rcu_dereference_check_fdtable(files, fdtfd) \

	(rcu_dereference_check((fdtfd), \

			       lockdep_is_held(&(files)->file_lock) || \

			       atomic_read(&(files)->count) == 1 || \

			       rcu_my_thread_group_empty()))



#define files_fdtable(files) \

		(rcu_dereference_check_fdtable((files), (files)->fdt))



struct file_operations;

struct vfsmount;

struct dentry;



extern void __init files_defer_init(void);



static inline struct file * fcheck_files(struct files_struct *files, unsigned int fd)

#define rcu_dereference_check_fdtable(files, fdtfd) \

	rcu_dereference_check((fdtfd), lockdep_is_held(&(files)->file_lock))



#define files_fdtable(files) \

	rcu_dereference_check_fdtable((files), (files)->fdt)



/*

 * The caller must ensure that fd table isn't shared or hold rcu or file lock

 */

static inline struct file *__fcheck_files(struct files_struct *files, unsigned int fd)

{

	struct file * file = NULL;

	struct fdtable *fdt = files_fdtable(files);

	struct fdtable *fdt = rcu_dereference_raw(files->fdt);



	if (fd < fdt->max_fds)

		file = rcu_dereference_check_fdtable(files, fdt->fd[fd]);

	return file;

		return rcu_dereference_raw(fdt->fd[fd]);

	return NULL;

}



static inline struct file *fcheck_files(struct files_struct *files, unsigned int fd)

{

	rcu_lockdep_assert(rcu_read_lock_held() ||

			   lockdep_is_held(&files->file_lock),

			   "suspicious rcu_dereference_check() usage");

	return __fcheck_files(files, fd);

}



/*

include/linux/rcupdate.h

+0 −2
Original line number Diff line number Diff line
@@ -448,8 +448,6 @@ static inline int rcu_read_lock_sched_held(void) 


#ifdef CONFIG_PROVE_RCU



extern int rcu_my_thread_group_empty(void);



/**

 * rcu_lockdep_assert - emit lockdep splat if specified condition not met

 * @c: condition to check

kernel/rcu/update.c

+0 −11
Original line number Diff line number Diff line
@@ -195,17 +195,6 @@ void wait_rcu_gp(call_rcu_func_t crf) 
}

EXPORT_SYMBOL_GPL(wait_rcu_gp);



#ifdef CONFIG_PROVE_RCU

/*

 * wrapper function to avoid #include problems.

 */

int rcu_my_thread_group_empty(void)

{

	return thread_group_empty(current);

}

EXPORT_SYMBOL_GPL(rcu_my_thread_group_empty);

#endif /* #ifdef CONFIG_PROVE_RCU */



#ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD

static inline void debug_init_rcu_head(struct rcu_head *head)

{

CRA Git | Maintained and supported by SUSTech CRA and CCSE