NVDIMM Device and Persistent Memory development
 help / color / mirror / Atom feed
From: Jonathan Cameron <Jonathan.Cameron@Huawei.com>
To: John Groves <John@groves.net>
Cc: Dongsheng Yang <dongsheng.yang@easystack.cn>,
	Gregory Price <gregory.price@memverge.com>,
	Dan Williams <dan.j.williams@intel.com>, <axboe@kernel.dk>,
	<linux-block@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	<linux-cxl@vger.kernel.org>, <nvdimm@lists.linux.dev>
Subject: Re: [PATCH RFC 0/7] block: Introduce CBD (CXL Block Device)
Date: Fri, 3 May 2024 10:52:45 +0100	[thread overview]
Message-ID: <20240503105245.00003676@Huawei.com> (raw)
In-Reply-To: <wold3g5ww63cwqo7rlwevqcpmlen3fl3lbtbq3qrmveoh2hale@e7carkmumnub>

On Sun, 28 Apr 2024 11:55:10 -0500
John Groves <John@groves.net> wrote:

> On 24/04/28 01:47PM, Dongsheng Yang wrote:
> > 
> > 
> > 在 2024/4/27 星期六 上午 12:14, Gregory Price 写道:  
> > > On Fri, Apr 26, 2024 at 10:53:43PM +0800, Dongsheng Yang wrote:  
> > > > 
> > > > 
> > > > 在 2024/4/26 星期五 下午 9:48, Gregory Price 写道:  
> > > > >   
> > > > 
> > > > In (5) of the cover letter, I mentioned that cbd addresses cache coherence
> > > > at the software level:
> > > > 
> > > > (5) How do blkdev and backend interact through the channel?
> > > > 	a) For reader side, before reading the data, if the data in this channel
> > > > may be modified by the other party, then I need to flush the cache before
> > > > reading to ensure that I get the latest data. For example, the blkdev needs
> > > > to flush the cache before obtaining compr_head because compr_head will be
> > > > updated by the backend handler.
> > > > 	b) For writter side, if the written information will be read by others,
> > > > then after writing, I need to flush the cache to let the other party see it
> > > > immediately. For example, after blkdev submits cbd_se, it needs to update
> > > > cmd_head to let the handler have a new cbd_se. Therefore, after updating
> > > > cmd_head, I need to flush the cache to let the backend see it.
> > > >   
> > > 
> > > Flushing the cache is insufficient.  All that cache flushing guarantees
> > > is that the memory has left the writer's CPU cache.  There are potentially
> > > many write buffers between the CPU and the actual backing media that the
> > > CPU has no visibility of and cannot pierce through to force a full
> > > guaranteed flush back to the media.
> > > 
> > > for example:
> > > 
> > > memcpy(some_cacheline, data, 64);
> > > mfence();
> > > 
> > > Will not guarantee that after mfence() completes that the remote host
> > > will have visibility of the data.  mfence() does not guarantee a full
> > > flush back down to the device, it only guarantees it has been pushed out
> > > of the CPU's cache.
> > > 
> > > similarly:
> > > 
> > > memcpy(some_cacheline, data, 64);
> > > mfence();
> > > memcpy(some_other_cacheline, data, 64);
> > > mfence()
> > > 
> > > Will not guarantee that some_cacheline reaches the backing media prior
> > > to some_other_cacheline, as there is no guarantee of write-ordering in
> > > CXL controllers (with the exception of writes to the same cacheline).
> > > 
> > > So this statement:
> > >   
> > > > I need to flush the cache to let the other party see it immediately.  
> > > 
> > > Is misleading.  They will not see is "immediately", they will see it
> > > "eventually at some completely unknowable time in the future".  
> > 
> > This is indeed one of the issues I wanted to discuss at the RFC stage. Thank
> > you for pointing it out.
> > 
> > In my opinion, using "nvdimm_flush" might be one way to address this issue,
> > but it seems to flush the entire nd_region, which might be too heavy.
> > Moreover, it only applies to non-volatile memory.
> > 
> > This should be a general problem for cxl shared memory. In theory, FAMFS
> > should also encounter this issue.
> > 
> > Gregory, John, and Dan, Any suggestion about it?
> > 
> > Thanx a lot  
> > > 
> > > ~Gregory
> > >   
> 
> Hi Dongsheng,
> 
> Gregory is right about the uncertainty around "clflush" operations, but
> let me drill in a bit further.
> 
> Say you copy a payload into a "bucket" in a queue and then update an
> index in a metadata structure; I'm thinking of the standard producer/
> consumer queuing model here, with one index mutated by the producer and
> the other mutated by the consumer. 
> 
> (I have not reviewed your queueing code, but you *must* be using this
> model - things like linked-lists won't work in shared memory without 
> shared locks/atomics.)
> 
> Normal logic says that you should clflush the payload before updating
> the index, then update and clflush the index.
> 
> But we still observe in non-cache-coherent shared memory that the payload 
> may become valid *after* the clflush of the queue index.
> 
> The famfs user space has a program called pcq.c, which implements a
> producer/consumer queue in a pair of famfs files. The only way to 
> currently guarantee a valid read of a payload is to use sequence numbers 
> and checksums on payloads.  We do observe mismatches with actual shared 
> memory, and the recovery is to clflush and re-read the payload from the 
> client side. (Aside: These file pairs theoretically might work for CBD 
> queues.)
> 
> Anoter side note: it would be super-helpful if the CPU gave us an explicit 
> invalidate rather than just clflush, which will write-back before 
> invalidating *if* the cache line is marked as dirty, even when software
> knows this should not happen.
> 
> Note that CXL 3.1 provides a way to guarantee that stuff that should not
> be written back can't be written back: read-only mappings. This one of
> the features I got into the spec; using this requires CXL 3.1 DCD, and 
> would require two DCD allocations (i.e. two tagged-capacity dax devices - 
> one writable by the server and one by the client).
> 
> Just to make things slightly gnarlier, the MESI cache coherency protocol
> allows a CPU to speculatively convert a line from exclusive to modified,
> meaning it's not clear as of now whether "occasional" clean write-backs
> can be avoided. Meaning those read-only mappings may be more important
> than one might think. (Clean write-backs basically make it
> impossible for software to manage cache coherency.)

My understanding is that clean write backs are an implementation specific
issue that came as a surprise to some CPU arch folk I spoke to, we will
need some path for a host to say if they can ever do that.

Given this definitely effects one CPU vendor, maybe solutions that
rely on this not happening are not suitable for upstream.

Maybe this market will be important enough for that CPU vendor to stop
doing it but if they do it will take a while...

Flushing in general is as CPU architecture problem where each of the
architectures needs to be clear what they do / specify that their
licensees do.

I'm with Dan on encouraging all memory vendors to do hardware coherence!

J

> 
> Keep in mind that I don't think anybody has cxl 3 devices or CPUs yet, and 
> shared memory is not explicitly legal in cxl 2, so there are things a cpu 
> could do (or not do) in a cxl 2 environment that are not illegal because 
> they should not be observable in a no-shared-memory environment.
> 
> CBD is interesting work, though for some of the reasons above I'm somewhat
> skeptical of shared memory as an IPC mechanism.
> 
> Regards,
> John
> 
> 
> 


  reply	other threads:[~2024-05-08  9:12 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20240422071606.52637-1-dongsheng.yang@easystack.cn>
     [not found] ` <66288ac38b770_a96f294c6@dwillia2-mobl3.amr.corp.intel.com.notmuch>
     [not found]   ` <ef34808b-d25d-c953-3407-aa833ad58e61@easystack.cn>
     [not found]     ` <ZikhwAAIGFG0UU23@memverge.com>
     [not found]       ` <bbf692ec-2109-baf2-aaae-7859a8315025@easystack.cn>
     [not found]         ` <ZiuwyIVaKJq8aC6g@memverge.com>
     [not found]           ` <98ae27ff-b01a-761d-c1c6-39911a000268@easystack.cn>
     [not found]             ` <ZivS86BrfPHopkru@memverge.com>
2024-04-28  5:47               ` [PATCH RFC 0/7] block: Introduce CBD (CXL Block Device) Dongsheng Yang
2024-04-28 16:44                 ` Gregory Price
2024-04-28 16:55                 ` John Groves
2024-05-03  9:52                   ` Jonathan Cameron [this message]
2024-05-08 11:39                     ` Dongsheng Yang
2024-05-08 12:11                       ` Jonathan Cameron
2024-05-08 13:03                         ` Dongsheng Yang
2024-05-08 15:44                           ` Jonathan Cameron
2024-05-09 11:24                             ` Dongsheng Yang
2024-05-09 12:21                               ` Jonathan Cameron
2024-05-09 13:03                                 ` Dongsheng Yang
2024-05-21 18:41                                   ` Dan Williams
2024-04-30  0:34                 ` Dan Williams

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240503105245.00003676@Huawei.com \
    --to=jonathan.cameron@huawei.com \
    --cc=John@groves.net \
    --cc=axboe@kernel.dk \
    --cc=dan.j.williams@intel.com \
    --cc=dongsheng.yang@easystack.cn \
    --cc=gregory.price@memverge.com \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-cxl@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nvdimm@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).