| 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
| ||
|
Message-ID: <1380604354.19002.9.camel@edumazet-glaptop.roam.corp.google.com>
Date: Mon, 30 Sep 2013 22:12:34 -0700
From: Eric Dumazet <eric.dumazet@...il.com>
To: Al Viro <viro@...IV.linux.org.uk>
Cc: Linus Torvalds <torvalds@...ux-foundation.org>,
Ingo Molnar <mingo@...nel.org>,
Peter Zijlstra <peterz@...radead.org>,
Waiman Long <Waiman.Long@...com>,
Benjamin Herrenschmidt <benh@...nel.crashing.org>,
"Chandramouleeswaran, Aswin" <aswin@...com>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
linux-fsdevel <linux-fsdevel@...r.kernel.org>
Subject: Re: spinlock contention of files->file_lock
On Mon, 2013-09-30 at 20:36 -0700, Eric Dumazet wrote:
> I have a patch mostly working here, and pretty short.
Here is the RFC patch. Unfortunately I cant really give performance
numbers, as a patch like this would need weeks before being tested here.
fs/file.c | 66 ++++++++++++++++++++------------------
include/linux/fdtable.h | 1
2 files changed, 37 insertions(+), 30 deletions(-)
diff --git a/fs/file.c b/fs/file.c
index 4a78f98..b511f14 100644
--- a/fs/file.c
+++ b/fs/file.c
@@ -62,15 +62,23 @@ static void free_fdtable_rcu(struct rcu_head *rcu)
* Expand the fdset in the files_struct. Called with the files spinlock
* held for write.
*/
-static void copy_fdtable(struct fdtable *nfdt, struct fdtable *ofdt)
+static void copy_fdtable(struct fdtable *nfdt, struct fdtable *ofdt,
+ bool shared)
{
- unsigned int cpy, set;
+ unsigned int cpy, set, i;
BUG_ON(nfdt->max_fds < ofdt->max_fds);
cpy = ofdt->max_fds * sizeof(struct file *);
set = (nfdt->max_fds - ofdt->max_fds) * sizeof(struct file *);
- memcpy(nfdt->fd, ofdt->fd, cpy);
+
+ if (shared) {
+ /* see fd_install() why we need xchg() */
+ for (i = 0; i < ofdt->max_fds; i++)
+ nfdt->fd[i] = xchg(&ofdt->fd[i], NULL);
+ } else {
+ memcpy(nfdt->fd, ofdt->fd, cpy);
+ }
memset((char *)(nfdt->fd) + cpy, 0, set);
cpy = ofdt->max_fds / BITS_PER_BYTE;
@@ -167,8 +175,12 @@ static int expand_fdtable(struct files_struct *files, int nr)
cur_fdt = files_fdtable(files);
if (nr >= cur_fdt->max_fds) {
/* Continue as planned */
- copy_fdtable(new_fdt, cur_fdt);
+
+ write_seqlock(&files->resize);
+ copy_fdtable(new_fdt, cur_fdt, atomic_read(&files->count) > 1);
rcu_assign_pointer(files->fdt, new_fdt);
+ write_sequnlock(&files->resize);
+
if (cur_fdt != &files->fdtab)
call_rcu(&cur_fdt->rcu, free_fdtable_rcu);
} else {
@@ -258,6 +270,7 @@ struct files_struct *dup_fd(struct files_struct *oldf, int *errorp)
atomic_set(&newf->count, 1);
spin_lock_init(&newf->file_lock);
+ seqlock_init(&newf->resize);
newf->next_fd = 0;
new_fdt = &newf->fdtab;
new_fdt->max_fds = NR_OPEN_DEFAULT;
@@ -453,6 +466,7 @@ struct files_struct init_files = {
.open_fds = init_files.open_fds_init,
},
.file_lock = __SPIN_LOCK_UNLOCKED(init_files.file_lock),
+ .resize = __SEQLOCK_UNLOCKED(init_task.resize),
};
/*
@@ -569,11 +583,24 @@ void __fd_install(struct files_struct *files, unsigned int fd,
struct file *file)
{
struct fdtable *fdt;
- spin_lock(&files->file_lock);
+ struct file *old;
+ unsigned int seq;
+
+ rcu_read_lock();
+retry:
+ seq = read_seqbegin(&files->resize);
fdt = files_fdtable(files);
- BUG_ON(fdt->fd[fd] != NULL);
- rcu_assign_pointer(fdt->fd[fd], file);
- spin_unlock(&files->file_lock);
+ old = xchg(&fdt->fd[fd], file);
+ if (unlikely(old)) {
+ /* dup2() race */
+ fput(old);
+ }
+ if (unlikely(read_seqretry(&files->resize, seq))) {
+ if (cmpxchg(&fdt->fd[fd], file, NULL) == file)
+ goto retry;
+ /* 'file' was consumed by resizer or dup2(), we are done */
+ }
+ rcu_read_unlock();
}
void fd_install(unsigned int fd, struct file *file)
@@ -783,26 +810,9 @@ static int do_dup2(struct files_struct *files,
struct file *tofree;
struct fdtable *fdt;
- /*
- * We need to detect attempts to do dup2() over allocated but still
- * not finished descriptor. NB: OpenBSD avoids that at the price of
- * extra work in their equivalent of fget() - they insert struct
- * file immediately after grabbing descriptor, mark it larval if
- * more work (e.g. actual opening) is needed and make sure that
- * fget() treats larval files as absent. Potentially interesting,
- * but while extra work in fget() is trivial, locking implications
- * and amount of surgery on open()-related paths in VFS are not.
- * FreeBSD fails with -EBADF in the same situation, NetBSD "solution"
- * deadlocks in rather amusing ways, AFAICS. All of that is out of
- * scope of POSIX or SUS, since neither considers shared descriptor
- * tables and this condition does not arise without those.
- */
fdt = files_fdtable(files);
- tofree = fdt->fd[fd];
- if (!tofree && fd_is_open(fd, fdt))
- goto Ebusy;
get_file(file);
- rcu_assign_pointer(fdt->fd[fd], file);
+ tofree = xchg(&fdt->fd[fd], file);
__set_open_fd(fd, fdt);
if (flags & O_CLOEXEC)
__set_close_on_exec(fd, fdt);
@@ -814,10 +824,6 @@ static int do_dup2(struct files_struct *files,
filp_close(tofree, files);
return fd;
-
-Ebusy:
- spin_unlock(&files->file_lock);
- return -EBUSY;
}
int replace_fd(unsigned fd, struct file *file, unsigned flags)
diff --git a/include/linux/fdtable.h b/include/linux/fdtable.h
index 085197b..893c60f 100644
--- a/include/linux/fdtable.h
+++ b/include/linux/fdtable.h
@@ -49,6 +49,7 @@ struct files_struct {
atomic_t count;
struct fdtable __rcu *fdt;
struct fdtable fdtab;
+ seqlock_t resize;
/*
* written part on a separate cache line in SMP
*/
--
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