Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 13 Jul 2014 05:55:00 +0200
From:      Mateusz Guzik <mjguzik@gmail.com>
To:        freebsd-arch@freebsd.org
Cc:        kib@freebsd.org
Subject:   Getting rid of atomic_load_acq_int(&fdp->fd_nfiles)) from fget_unlocked
Message-ID:  <20140713035500.GC16884@dft-labs.eu>

next in thread | raw e-mail | index | archive | help
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
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

Comments?
-- 
Mateusz Guzik <mjguzik gmail.com>



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20140713035500.GC16884>