From owner-svn-src-head@freebsd.org Wed Dec 2 18:17:13 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 EF8CA4A2F88 for ; Wed, 2 Dec 2020 18:17:13 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Received: from mail-qt1-x82f.google.com (mail-qt1-x82f.google.com [IPv6:2607:f8b0:4864:20::82f]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "GTS CA 1O1" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4CmRyK65LCz3Nyl for ; Wed, 2 Dec 2020 18:17:13 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Received: by mail-qt1-x82f.google.com with SMTP id d5so1753167qtn.0 for ; Wed, 02 Dec 2020 10:17:13 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bsdimp-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=84bjTcQi703HDGyahN5yQo/rZX9soJ6RN7KfYUc/KEs=; b=l1a0Oqw+tkP5D9IBr6Ec0Kle35AL8ZryGv+pZaxnZtnfoUaDt5X3FBN5hCQACXOL+H NR07wE0zbMHNg4xnJod8dcvizNWZBCppd2DMf1T6iYFs6EmXBUhjBNCsmllRJfsUdFTP 2g5zi4192bv3Hoct4ZrOVeqViGInZQGV4LapYu3u8sdG3H1cKchLKvyCBba5BAY5hEqj GILxQreZW/lOPicM/j1m1rumd98gUUyevdJ2ftsBzs5cPhTGu2QwHwPI4V+MSDbznwtL kg9ddkASDrFVRaMOMDjsRJdt1Vn16pinvtycl7EEZGEEzCHGpUfIeZgURrFSw0vhSxMR 5ocA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=84bjTcQi703HDGyahN5yQo/rZX9soJ6RN7KfYUc/KEs=; b=XGU9WBdTkt5Z/h5WpArm66+khWqPih+00Ho3cAD1CxmigiwOXpxGxlNcArcyMImjt1 VPQypUqt5M2v1K+Q+Mswt8n0+zToqy2rXzu357/MFee6uzUILKWObq9NUpO9BUNLFvAC n7pxRUuHxUvqaDT7Z3EMgywxXnH9V/yBPXIBmGFy1oQupGnlzc6H4EdQVQvAosUYBXb+ pmbCHJi1iECFRZqWUbNEVdHRMUtdzU4CqxVtg9tLCP5qpgbf1wRY23IQb+5EtMzCc58S NPhZJTW7DK8uWNa1Cyo2Q8I7Lu7HAKyqYcLYiwQ3Db5WhsLma3efYe6RS6Pe8aUMftO9 BNjA== X-Gm-Message-State: AOAM5334XHo9TNipk50bWhZEzcsj4nPZ/7oC7C8lxxOPBv+6se8ov7gE FQ7oMKgSonXxfHFdeeE4u6IiV9npIoezhhSmZfXdCQ== X-Google-Smtp-Source: ABdhPJy/z+MwVcCUALIkj55mGoUz8FpfvgaR3A0hopdcfI9HrjzUVIYFlfmshgDY1oJIeSJY3hFOtrM9aD3xE0RJUpw= X-Received: by 2002:ac8:4802:: with SMTP id g2mr3769071qtq.235.1606933032871; Wed, 02 Dec 2020 10:17:12 -0800 (PST) MIME-Version: 1.0 References: <202012021654.0B2GsOP8000763@repo.freebsd.org> In-Reply-To: From: Warner Losh Date: Wed, 2 Dec 2020 11:17:01 -0700 Message-ID: Subject: Re: svn commit: r368279 - head/sys/dev/nvme To: Konstantin Belousov Cc: Michal Meloun , src-committers , svn-src-all , svn-src-head X-Rspamd-Queue-Id: 4CmRyK65LCz3Nyl X-Spamd-Bar: ---- Authentication-Results: mx1.freebsd.org; none X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[] Content-Type: text/plain; charset="UTF-8" X-Content-Filtered-By: Mailman/MimeDel 2.1.34 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 18:17:14 -0000 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.[*] 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. 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.