From owner-svn-src-head@freebsd.org Wed Dec 2 19:21:39 2020 Return-Path: Delivered-To: svn-src-head@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id B53B14A455A; Wed, 2 Dec 2020 19:21:39 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from kib.kiev.ua (kib.kiev.ua [IPv6:2001:470:d5e7:1::1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 4CmTNg3ZpBz3j5G; Wed, 2 Dec 2020 19:21:39 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from tom.home (kib@localhost [127.0.0.1]) by kib.kiev.ua (8.16.1/8.16.1) with ESMTPS id 0B2JLTvq023258 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NO); Wed, 2 Dec 2020 21:21:33 +0200 (EET) (envelope-from kostikbel@gmail.com) DKIM-Filter: OpenDKIM Filter v2.10.3 kib.kiev.ua 0B2JLTvq023258 Received: (from kostik@localhost) by tom.home (8.16.1/8.16.1/Submit) id 0B2JLTFN023257; Wed, 2 Dec 2020 21:21:29 +0200 (EET) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: tom.home: kostik set sender to kostikbel@gmail.com using -f Date: Wed, 2 Dec 2020 21:21:29 +0200 From: Konstantin Belousov To: Warner Losh Cc: Michal Meloun , src-committers , svn-src-all , svn-src-head Subject: Re: svn commit: r368279 - head/sys/dev/nvme Message-ID: References: <202012021654.0B2GsOP8000763@repo.freebsd.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Spam-Status: No, score=-1.0 required=5.0 tests=ALL_TRUSTED,BAYES_00, DKIM_ADSP_CUSTOM_MED,FORGED_GMAIL_RCVD,FREEMAIL_FROM, NML_ADSP_CUSTOM_MED autolearn=no autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on tom.home X-Rspamd-Queue-Id: 4CmTNg3ZpBz3j5G X-Spamd-Bar: ---- Authentication-Results: mx1.freebsd.org; none X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[] X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.34 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: Wed, 02 Dec 2020 19:21:39 -0000 On Wed, Dec 02, 2020 at 11:17:01AM -0700, Warner Losh wrote: > On Wed, Dec 2, 2020 at 10:48 AM Konstantin Belousov > wrote: > > > On Wed, Dec 02, 2020 at 04:54:24PM +0000, Michal Meloun wrote: > > > Author: mmel > > > Date: Wed Dec 2 16:54:24 2020 > > > New Revision: 368279 > > > URL: https://svnweb.freebsd.org/changeset/base/368279 > > > > > > Log: > > > NVME: Multiple busdma related fixes. > > ... > > > > > - in nvme_qpair_submit_tracker(), don't do explicit wmb() also for arm > > > and arm64. Bus_dmamap_sync() on these architectures is sufficient to > > ensure > > > that all CPU stores are visible to external (including DMA) > > observers. > > > @@ -982,7 +982,7 @@ nvme_qpair_submit_tracker(struct nvme_qpair *qpair, > > st > > > > > > bus_dmamap_sync(qpair->dma_tag, qpair->queuemem_map, > > > BUS_DMASYNC_PREREAD | BUS_DMASYNC_PREWRITE); > > > -#ifndef __powerpc__ > > > +#if !defined( __powerpc__) && !defined( __aarch64__) && !defined( > > __arm__) > > > /* > > > * powerpc's bus_dmamap_sync() already includes a heavyweight > > sync, but > > > * no other archs do. > > Does anybody have any evidence that the wmb() below is useful ? > > For instance, on x86, does nvme driver use any write-combining mappings ? > > > > It translates to a sfence on x86. It is done just before the write > that moves the tail pointer. sfence ensures that all writes are done before > that write is done. I believe that the nvme spec requires that the entire > submission queue entry be fully filled out before the write to the tailq > pointer moving it.[*] Right, and SFENCE is mostly a slow NOP, except situations where we deal with write-combining memory, or some specific instructions (CLFLUSHOPT etc). More, IN/OUT and uncached memory accesses, like BAR io, provide strongest serialization. > > I'm not enough of an expert on the exact details here to know if it's > absolutely required or not, but given the ordering requirements in the > spec, the intent appears to be related to enforcing that ordering. I don't > know enough to know if it accomplishes this goal or not. My point is that, unless there are some additional reasons, and not just the need to order writes, SFENCE is not needed and probably somewhat slows down the driver. It might be even measurable for NVMe that takes several millions of iops/sec. > > Warner > > [*] I know I said in the review it may be due to getting lower latency to > the device, but I now believe that to be mistaken after studying it further.