lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Tue, 28 Apr 2015 09:20:55 -0700 From: Eric Dumazet <eric.dumazet@...il.com> To: Mateusz Guzik <mguzik@...hat.com> Cc: Al Viro <viro@...IV.linux.org.uk>, Andrew Morton <akpm@...ux-foundation.org>, "Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>, Yann Droneaud <ydroneaud@...eya.com>, Konstantin Khlebnikov <khlebnikov@...dex-team.ru>, linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org Subject: Re: [PATCH] fs/file.c: don't acquire files->file_lock in fd_install() On Mon, 2015-04-27 at 21:05 +0200, Mateusz Guzik wrote: > On Tue, Apr 21, 2015 at 09:59:28PM -0700, Eric Dumazet wrote: > > From: Eric Dumazet <edumazet@...gle.com> > > > > Mateusz Guzik reported : > > > > Currently obtaining a new file descriptor results in locking fdtable > > twice - once in order to reserve a slot and second time to fill it. > > > > Holding the spinlock in __fd_install() is needed in case a resize is > > done, or to prevent a resize. > > > > Mateusz provided an RFC patch and a micro benchmark : > > http://people.redhat.com/~mguzik/pipebench.c > > > > A resize is an unlikely operation in a process lifetime, > > as table size is at least doubled at every resize. > > > > We can use RCU instead of the spinlock : > > > > __fd_install() must wait if a resize is in progress. > > > > The resize must block new __fd_install() callers from starting, > > and wait that ongoing install are finished (synchronize_sched()) > > > > resize should be attempted by a single thread to not waste resources. > > > > rcu_sched variant is used, as __fd_install() and expand_fdtable() run > > from process context. > > > > It gives us a ~30% speedup using pipebench with 16 threads, and a ~10% > > speedup with another program using TCP sockets. > > > > Signed-off-by: Eric Dumazet <edumazet@...gle.com> > > Reported-by: Mateusz Guzik <mguzik@...hat.com> > > Reviewed-by: Mateusz Guzik <mguzik@...hat.com> > Tested-by: Mateusz Guzik <mguzik@...hat.com> > Thanks Mateusz I'll send a v2, replacing the READ_ONCE() by more appropriate rcu_dereference_sched() : > > new_fdt->max_fds = NR_OPEN_DEFAULT; > > @@ -553,11 +572,20 @@ void __fd_install(struct files_struct *files, unsigned int fd, > > struct file *file) > > { > > struct fdtable *fdt; > > - spin_lock(&files->file_lock); > > - fdt = files_fdtable(files); > > + > > + rcu_read_lock_sched(); > > + > > + while (unlikely(files->resize_in_progress)) { > > + rcu_read_unlock_sched(); > > + wait_event(files->resize_wait, !files->resize_in_progress); > > + rcu_read_lock_sched(); > > + } > > + /* coupled with smp_wmb() in expand_fdtable() */ > > + smp_rmb(); > > + fdt = READ_ONCE(files->fdt); This should be : fdt = rcu_dereference_sched(files->fdt); > > BUG_ON(fdt->fd[fd] != NULL); > > rcu_assign_pointer(fdt->fd[fd], file); > > - spin_unlock(&files->file_lock); > > + rcu_read_unlock_sched(); > > } > > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists