From owner-svn-src-head@freebsd.org Sat Jun 10 16:33:51 2017 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 155D7BF16B6; Sat, 10 Jun 2017 16:33:51 +0000 (UTC) (envelope-from jonlooney@gmail.com) Received: from mail-wr0-x233.google.com (mail-wr0-x233.google.com [IPv6:2a00:1450:400c:c0c::233]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 9EC5A7ED10; Sat, 10 Jun 2017 16:33:50 +0000 (UTC) (envelope-from jonlooney@gmail.com) Received: by mail-wr0-x233.google.com with SMTP id g76so58671980wrd.1; Sat, 10 Jun 2017 09:33:50 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=P3Cb5dL1jvwEsYvoOTMMeh4cFvqJMShd4AvUeEAumM0=; b=Ev8zOe/ti3QGAF2JPDnIwLp77whj1SAuXozf0+k2a7P1QoFek3kApoKc+j8Bi3XBbK hqvtp/S8b/T2lZHx1t25eKoCnJPAsCndKfU0KPHroT1xtwwuH+IlsayNYvGqrhFzm5GD WnI+KfuEvIvIpHcoio7Hx1pwhFgLp/fLtWtub0yfVlPK37e5fRj3bLZugzW4YxdvVzVT qVmgK3f3ewb+R75jRb503ael7ZC0YFct57ROXgLwWHE8EOsqmONUlnR59GATh8biS2sF ctDb0pdTXW1j7IpL62cTXBHJXaStJ4o44J99O8kUNPUjg+zXJs0yssmvkTeyjjlcdSuD DgwQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=P3Cb5dL1jvwEsYvoOTMMeh4cFvqJMShd4AvUeEAumM0=; b=Sbcmdv4rD697DuEdU2YjDGUB+O4EFxbXRcNfneU4ygId3NdWmwPOuANk6xIH7Wukrl DsUJVCqCi6Ve/CHUlG8c4m5RGHyVpsokDhfDdCfpXApn8oDb0gcQfscKuEjXQmAsi1wU fWMKzhOFErXR1ScqaUz/D8AA3KEiI8EQHJB+c5DR2NfuWKPjJDWfdwgZMo4Flppxzdkr 331NUA9XS9HaSbaZ00wXxqlDhWLbs5L7hrQvEbtTIJMN2arbDTZvsozcfWozcmb3JH/x c2DnxMuTARa1GrzBmvx5biAeAAEA4qf8Crt6IHYOfiE03rYQVHjYFLFJ4hPcM2Wvn+Wl YVjw== X-Gm-Message-State: AODbwcDkN11cA29SlLgpKQMONScKVosOHUJJIChUXC531vYiTXfRoG7a FLA0PmvaEzSnqXDONO4D22WgxW094w== X-Received: by 10.28.71.91 with SMTP id u88mr3246350wma.9.1497112429151; Sat, 10 Jun 2017 09:33:49 -0700 (PDT) MIME-Version: 1.0 Received: by 10.223.151.132 with HTTP; Sat, 10 Jun 2017 09:33:48 -0700 (PDT) In-Reply-To: <20170610091203.GH2088@kib.kiev.ua> References: <201706082047.v58KlI51079003@repo.freebsd.org> <7306919.ixyIA96xWQ@ralph.baldwin.cx> <20170610091203.GH2088@kib.kiev.ua> From: Jonathan Looney Date: Sat, 10 Jun 2017 09:33:48 -0700 Message-ID: Subject: Re: svn commit: r319720 - head/sys/dev/vt To: Konstantin Belousov Cc: John Baldwin , "Jonathan T. Looney" , src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Content-Type: text/plain; charset="UTF-8" X-Content-Filtered-By: Mailman/MimeDel 2.1.23 X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 10 Jun 2017 16:33:51 -0000 Hi Konstantin, On Sat, Jun 10, 2017 at 2:12 AM, Konstantin Belousov wrote: > No, acquire is only specified for loads, and release for stores. In other > words, on some hypothetical ll/sc architecture, the atomic_add_acq() > could be implemented as follows, in asm-pseudocode > atomic_add_acq(int x): > ll x, r1 > acq x > add 1, r > sc r1, x > Your use of the atomic does not prevent stores reordering. If this is true, it sounds like the atomic(9) man page needs some revision. It says: When an atomic operation has acquire semantics, the effects of the operation must have completed before any subsequent load or store (by program order) is performed. Conversely, acquire semantics do not require that prior loads or stores have completed before the atomic operation is performed. Up until now, I have relied on this description of the way acquire barriers work. If this description is incorrect, I think it is important to fix it quickly. > I'm not claiming that this fixes all bugs in this area. (In fact, I > > specifically disclaim this.) But, it does stop the crash from occurring. > > > > If you still feel there are better mechanisms to achieve the desired > > ordering, please let me know and I'll be happy to fix and/or improve > this. > See the pseudocode I posted in my original reply, which uses acq/rel pair. The code you posted for vt_resume_flush_timer() would switch vd_timer_armed from 0 to 1 even if VDF_ASYNC is not set; however, that is not what we want. vd_timer_armed should be left untouched if VDF_ASYNC is not set. It sounds like what I want is some sort of fence in vt_upgrade() like jhb@ specified in his email. For example: vd->vd_timer_armed = 1; atomic_thread_fence_rel(); vd->vd_flags |= VDF_ASYNC; I recognize a fence would be cleaner since I really only needed the barrier and not the atomic operation. Do you agree the above would be sufficient to ensure store ordering? Jonathan