[RFC] why do we need smp_rmb/smp_wmb pair in fd_install()/expand_fdtable()?

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



	Take a look at fs/file.c:expand_fdtable() and fs/file.c:fd_install()

In the end of the former:
        copy_fdtable(new_fdt, cur_fdt);
        rcu_assign_pointer(files->fdt, new_fdt);
        if (cur_fdt != &files->fdtab)
                call_rcu(&cur_fdt->rcu, free_fdtable_rcu);
        /* coupled with smp_rmb() in fd_install() */
        smp_wmb();
        return 1;
the only caller (expand_files()) has
        expanded = expand_fdtable(files, nr);
        files->resize_in_progress = false;

OTOH, in fd_install() there's this:
        if (unlikely(files->resize_in_progress)) {
                rcu_read_unlock_sched();
                spin_lock(&files->file_lock);
                fdt = files_fdtable(files);
                WARN_ON(fdt->fd[fd] != NULL);
                rcu_assign_pointer(fdt->fd[fd], file);
                spin_unlock(&files->file_lock);
                return;
        }
        /* coupled with smp_wmb() in expand_fdtable() */
        smp_rmb();
        fdt = rcu_dereference_sched(files->fdt);

What's the problem with droping both barriers and turning that
into
        expanded = expand_fdtable(files, nr);
        smp_store_release(&files->resize_in_progress, false);
and
        if (unlikely(smp_load_acquire(&files->resize_in_progress))) {
		....
                return;
        }
        fdt = rcu_dereference_sched(files->fdt);
resp.?  Anyone who sees ->resize_in_progress being false will
see all stores prior to setting ->fdt on the expand size...

That went into the tree in 8a81252b774b "fs/file.c: don't acquire
files->file_lock in fd_install()".  I don't remember that having been
discussed back then, but it had been 7 years ago...
<check the thread on lore>
Nobody asked, AFAICS.

Is there any problem with going that way?  Because I'm pretty sure
that it's would be cheaper to use store_release/load_acquire instead
of paying for smp_rmb() on the fd_install() side...

Am I missing something subtle here?




[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [NTFS 3]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [NTFS 3]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux