Re: [PATCH 0/2] rcu_dereference_check_fdtable fix/cleanups
From: Oleg Nesterov
Date: Wed Jan 08 2014 - 10:25:13 EST
On 01/08, Paul E. McKenney wrote:
>
> I am not all that excited about invoking rcu_lock_acquire() outside
> of RCU...
Yes, me too. That is why I thought about the helper with a good name,
see below.
> Another approach would be to add an argument to files_fdtable()
> that is zero normally and one for "we know we don't need RCU
> protection." Then rcu_dereference_check() could be something
> like the following:
>
> #define files_fdtable(files, c) \
> (rcu_dereference_check_fdtable((files), (files)->fdt) || c)
>
> Would that work?
Yes, I considered this optiion, but this needs much more uglifications^W
changes.
Either we need to change all users of files_fdtable(), or we need something
like
#define __rcu_dereference_check_fdtable(files, unshared, fdtfd) \
rcu_dereference_check((fdtfd), unshared || lockdep_is_held(&(files)->file_lock))
#define rcu_dereference_check_fdtable(files, fdtfd)
__rcu_dereference_check_fdtable(files, false, fdtfd)
#define __files_fdtable(files)
__rcu_dereference_check_fdtable((files), true, (files)->fdt)
#define files_fdtable(files)
__rcu_dereference_check_fdtable((files), false, (files)->fdt)
Plus we need
static inline struct file *__fcheck_files(struct files_struct *files,
bool unshared, unsigned int fd)
{
struct file *file = NULL;
struct fdtable *fdt = __rcu_dereference_check_fdtable(files, unshared, files->fdt);
if (fd < fdt->max_fds)
file = __rcu_dereference_check_fdtable(files, unshared, fdt->fd[fd]);
return file;
}
doesn't look very nice...
As for 2/2, probably close_files() can simply do
/*
* It is safe to dereference the fd table without RCU or
* ->file_lock because this is the last reference to the
* files structure.
*/
fdt = rcu_dereference_raw(files->fdt);
Or we can add
#define __files_fdtable(files) \
rcu_dereference_raw((files)->fdt)
but it is not clear to me what 1/1 should do. Perhaps
static inline struct file *__fcheck_files(struct files_struct *files, unsigned int fd)
{
struct fdtable *fdt = rcu_dereference_raw(files->fdt);
struct file *file = NULL;
if (fd < fdt->max_fds)
file = rcu_dereference_raw(fdt->fd[fd]);
return file;
}
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),
"message");
return __fcheck_files(files, fd);
}
?
Oleg.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/