lkml.org 
[lkml]   [2014]   [Jan]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 0/2] rcu_dereference_check_fdtable fix/cleanups
On 01/08, Paul E. McKenney wrote:
>
> On Wed, Jan 08, 2014 at 04:19:18PM +0100, Oleg Nesterov wrote:
> > On 01/08, Paul E. McKenney wrote:
> > >
> > > 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
>
> There are only about 20 uses of files_fdtable() in 3.12, with almost all
> of them in fs/file.c. So is changing all the users really all that
> problematic?

But only one user, close_files(), needs files_fdtable(files, true). Why
complicate the patch and the code? I think it would be better to simply
change close_files() to use rcu_dereference_raw().

And note that rcu_dereference_check_fdtable() needs the new argument too.

And we should also take care of fcheck_files(),

> > 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);
> > }

doesn't this look much simpler than adding the "bool unshared" argument
and changing the callers?

Oleg.



\
 
 \ /
  Last update: 2014-01-10 17:41    [W:0.123 / U:0.016 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site