Date: Sat, 4 Aug 2018 19:06:47 +0300 From: Konstantin Belousov <kostikbel@gmail.com> To: Jim Harris <jim.harris@gmail.com> Cc: jhibbits@freebsd.org, John Baldwin <jhb@freebsd.org>, src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r337273 - head/sys/dev/nvme Message-ID: <20180804160647.GR6049@kib.kiev.ua> In-Reply-To: <CAJP=Hc9jeVDXau-Koce5x=rKx2Yu7xW=QxgTgSp641u2RUK-Kw@mail.gmail.com> References: <201808032004.w73K46XJ053249@repo.freebsd.org> <20180804080840.GI6049@kib.kiev.ua> <c931140b-0334-de19-35f1-5b00cd10c2d9@FreeBSD.org> <20180804130315.GK6049@kib.kiev.ua> <CAHSQbTCE%2Bbngg0NpDB38b3HBH6zugykt9U4mFXNYJTSYHD4asQ@mail.gmail.com> <20180804144733.GO6049@kib.kiev.ua> <CAJP=Hc9jeVDXau-Koce5x=rKx2Yu7xW=QxgTgSp641u2RUK-Kw@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sat, Aug 04, 2018 at 09:00:18AM -0700, Jim Harris wrote: > On Sat, Aug 4, 2018 at 7:47 AM Konstantin Belousov <kostikbel@gmail.com> > wrote: > > <snip> > > > > Yes, if something special is needed, it should happen in > > platform-specific > > > > busdma code. > > > > > > > > Also, if wmb() is needed, then it is not a supposed semantic or > > > > wmb(), but a specific side-effects of one of the instruction in the > > > > implementation of wmb(). > > > > > > > > As I noted, on x86 it is not needed and detrimental to the performance. > > > > > > > > > > According to Jim Harris it is needed for x86. I tried to remove it > > thinking > > > it was unnecessary with the sync in place now. The wmb() was added way > > > back in r240616 by him, when only x86 was supported by the driver. > > > bus_dmamap_sync() for all archs except powerpc lack a barrier, from what > > I > > > can see. See the review for more discussion that took place. If it isn't > > > needed I will gladly remove it. > > > > I am very curious why x86 would need MFENCE there. I do not see an > > explanation in the review. > > > > On x86, store buffers are not coherent. Parts of the 64-byte submission > queue entry could be in store buffers and not globally visible at this > point. The wmb() ensures that all of the data is globally visible before > the doorbell write. This ensures the NVMe device will DMA the correct data > from the submission queue. Store buffers are not coherent but are in order. If later store is visible, then all previous stores must be visible as well. And wmb() semantic only guarantees ordering, not visibility after the execution. It could be implemented as 'LOCK;ADDL' instead of MFENCE. > > (Note: the Linux nvme driver also does an explicit wmb() before writing the > submission queue doorbell.)
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20180804160647.GR6049>