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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to