From owner-freebsd-arch@FreeBSD.ORG Fri Jun 20 01:04:31 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 B784E4AB; Fri, 20 Jun 2014 01:04:31 +0000 (UTC) Received: from mail-wg0-x22c.google.com (mail-wg0-x22c.google.com [IPv6:2a00:1450:400c:c00::22c]) (using TLSv1 with cipher ECDHE-RSA-RC4-SHA (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id D5E422D8E; Fri, 20 Jun 2014 01:04:30 +0000 (UTC) Received: by mail-wg0-f44.google.com with SMTP id x13so2939889wgg.15 for ; Thu, 19 Jun 2014 18:04:28 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=date:from:to:cc:subject:message-id:references:mime-version :content-type:content-disposition:in-reply-to:user-agent; bh=RHqvaeRtQO5LWdaVMSfSeSgyXS3ly0fAFw6FLS4SZP0=; b=dWjSigQcarf6CHZ5b90PNYgGKYLjeeX2yydDZ/RaWoIZIVsyMB/A5X5r517vjTljF+ 23dfpy9vXyu+Csn3qfGsSdGDUraRr2h3I8MffBZBsEH8ucZsofWaJEzaz3KhZwSuT/2y iJUdOLWm5ySuQL2jDaeC8MiZYPg8VHeHhx9B8i/veP3ud91MqYB4Y4duEf/rB+kjuEJ3 IcsM5kEmc8WWBTKB2c8PBp4DvD3SkA0WFSPA+BErkMsrtXDSbwnvUqI3nH1x9CJ9Ki2X cy2yOZ7uqlDwjWMACJUDhNzZJPN6Yy4oc5leGySFz6EIE+gnJ1rUXrvyGipFoKJ3aXi3 XEfw== X-Received: by 10.194.8.102 with SMTP id q6mr120751wja.74.1403226268188; Thu, 19 Jun 2014 18:04:28 -0700 (PDT) Received: from dft-labs.eu (n1x0n-1-pt.tunnel.tserv5.lon1.ipv6.he.net. [2001:470:1f08:1f7::2]) by mx.google.com with ESMTPSA id fn1sm16189115wib.18.2014.06.19.18.04.26 for (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Thu, 19 Jun 2014 18:04:27 -0700 (PDT) Date: Fri, 20 Jun 2014 03:04:24 +0200 From: Mateusz Guzik To: Adrian Chadd Subject: Re: capability races (was: Re: Seeing ENOTCAPABLE from write FDs in kqueue?) Message-ID: <20140620010424.GA5830@dft-labs.eu> References: <20140608220000.GA5030@dft-labs.eu> <20140608230059.GB5030@dft-labs.eu> <20140618203314.GB7157@dft-labs.eu> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20140618203314.GB7157@dft-labs.eu> User-Agent: Mutt/1.5.21 (2010-09-15) Cc: Robert Watson , "freebsd-arch@freebsd.org" X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 20 Jun 2014 01:04:31 -0000 Good news. While previous approached were giving ~16% worse performance in my microbenchmarks I found a way to make up for this (needs verification for correctness). fget_unlocked does: /* * 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); Turns out replacing this read with mere "fdp->fd_nfiles" gives ~10% performance increase over regular kernel and almost brings kernel with sequence counters to the level of regular kernel. So how to get rid of the need for atomic_load_acq_int? Currently struct filedesc contains file table and its size. File table pointer update is one operation, size update is another. Thus the need for memory barriers. Now let's consider the following structure: struct fdtable { int f_size; struct filedesc f_files[0]; }; Pointer to this strucure in struct filedesc can be replaced atomically, and from what I'm told the only synchronization needed is data dependency barrier (which in case of amd64 means there is no need to do anything). In short, we can get ~10% better performance and then sacrifice the gain (and some more) to have capability races fixed and be slightly slower than now. Thoughts? The patch is a total hack so I'm not attaching it, but as far as performance goes you can just move fd_nfiles to be next to fd_ofiles and remove atomic_load_acq_int in fget_unlocked. fdgrowtable will need to have a write barrier before the pointer is atomically replaced. -- Mateusz Guzik