From owner-freebsd-arch@FreeBSD.ORG Thu Aug 14 16:07:13 2014 Return-Path: Delivered-To: freebsd-arch@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 0EA6124B; Thu, 14 Aug 2014 16:07:13 +0000 (UTC) Received: from bigwig.baldwin.cx (bigwig.baldwin.cx [IPv6:2001:470:1f11:75::1]) (using TLSv1 with cipher DHE-RSA-CAMELLIA256-SHA (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id DA7BB29A3; Thu, 14 Aug 2014 16:07:12 +0000 (UTC) Received: from jhbbsd.localnet (unknown [209.249.190.124]) by bigwig.baldwin.cx (Postfix) with ESMTPSA id 1F601B917; Thu, 14 Aug 2014 12:07:11 -0400 (EDT) From: John Baldwin To: freebsd-arch@freebsd.org Subject: Re: Getting rid of atomic_load_acq_int(&fdp->fd_nfiles)) from fget_unlocked Date: Thu, 14 Aug 2014 08:41:47 -0400 User-Agent: KMail/1.13.5 (FreeBSD/8.4-CBSD-20140415; KDE/4.5.5; amd64; ; ) References: <20140713035500.GC16884@dft-labs.eu> In-Reply-To: <20140713035500.GC16884@dft-labs.eu> MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Message-Id: <201408140841.47510.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.2.7 (bigwig.baldwin.cx); Thu, 14 Aug 2014 12:07:11 -0400 (EDT) Cc: Mateusz Guzik , kib@freebsd.org X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 14 Aug 2014 16:07:13 -0000 On Saturday, July 12, 2014 11:55:00 pm Mateusz Guzik wrote: > Currently: > /* > * Avoid reads reordering and then a first access to the > * fdp->fd_ofiles table which could result in OOB operation. > */ > if (fd < 0 || fd >= atomic_load_acq_int(&fdp->fd_nfiles)) > return (EBADF); > > However, if we put fd_nfiles and fd_otable into one atomically replaced > structure the only need to: > 1. make sure the pointer is read once > 2. issue a data dependency barrier - this is a noop on all supported > architectures and we don't even have approprate macro, so doing nothing > seems fine > > The motivation is to boost performance to amortize for seqlock cost, in > case it hits the tree. > > This has no impact on races with capability lookup. > > In a microbenchmark of 16 threads reading from the same pipe fd > immediately returning EAGAIN the numbers are: > x vanilla-readpipe-run-sum > + noacq-readpipe-run-sum > [..] > N Min Max Median Avg Stddev > x 20 13133671 14900364 13893331 13827075 471500.82 > + 20 59479718 59527286 59496714 59499504 13752.968 > Difference at 95.0% confidence > 4.56724e+07 +/- 213483 > 330.312% +/- 1.54395% > > There are 3 steps: > 1. tidy up capsicum to accept fde: > http://people.freebsd.org/~mjg/patches/single-fdtable-read-capsicum.patch The KASSERT() on the 'fd' being valid was lost from cap_fcntl_check(). Should probably put that back. However, cap_fcntl_check() is now no longer used. Instead, you should just rename cap_fcntl_check_fd() to cap_fcntl_check() and remove the old one. > 2. add __READ_ONCE: > http://people.freebsd.org/~mjg/patches/read-once.patch > 3. put stuff into one structure: > http://people.freebsd.org/~mjg/patches/filedescenttable.patch Anotehr name besides the one suggested by Konstantin could just be 'struct filetable'. Even shorter while not having abbreviations. Also, I think you can leave the comment in 'struct filedesc' as just 'open files' in that case. -- John Baldwin