All of lore.kernel.org
 help / color / mirror / Atom feed
From: Coly Li <colyli@suse.de>
To: Christoph Hellwig <hch@lst.de>
Cc: "Huang, Ying" <ying.huang@intel.com>,
	Dan Williams <dan.j.williams@intel.com>, Jan Kara <jack@suse.com>,
	Hannes Reinecke <hare@suse.com>,
	linux-bcache@vger.kernel.org, linux-block@vger.kernel.org,
	Jianpeng Ma <jianpeng.ma@intel.com>,
	Qiaowei Ren <qiaowei.ren@intel.com>,
	axboe@kernel.dk
Subject: Re: Ask help for code review (was Re: [PATCH 03/14] bcache: add initial data structures for nvm pages)
Date: Wed, 23 Jun 2021 18:05:51 +0800	[thread overview]
Message-ID: <466c1678-8cdc-7240-1422-b435a599cad3@suse.de> (raw)
In-Reply-To: <20210623072140.GA837@lst.de>

On 6/23/21 3:21 PM, Christoph Hellwig wrote:
> On Wed, Jun 23, 2021 at 03:19:11PM +0800, Coly Li wrote:
>> Bcache does not support endian clean indeed,
> Then we need to fix that eventually rather than making it worse.  Which
> means any _new_ data structure should start that way.

The cache device (typically SSD) of bcache is designed to dedicate to a
single local machine. Any
storage migration between machines with different endians should firstly
flush the dirty data to
backing hard drive. For bcache meta-data on cache device (typically
SSD), it is designed to NOT
move dirty cache between machines with different endians, and in
practice there is no such use
case indeed and not supported by any Linux Distribution.

Not supporting different endian is as design, why we should fix it for
no real use case ?

BTW, the discussion is only for cache device because the bcache meta
data stored on it. For
backing hard drive, its endian is transparent to bcache and decided by
upper layer code like
file system or user space application, it is fully endian clean.


>> and libnvdimm only works with
>> 64bit physical address width.
> Maybe it does right now.  But ther is nothing fundamental in that, so
> please don't design stupid on-disk formats to encode that are going to
> come back to bite us sooner or later.  Be that by adding 32-bit support
> for any Linux DAX device, or by new 96 or 128bit CPUs.

This is unfair restriction :-)
The nvdimm support for bcache heavily depends on libnvdimm, that is, for
all conditions that libnvdimm
supports we should follow up. But requiring us to support the condition
that even libnvdimm does not
support yet, it is too early at this stage.

And, if libnvdimm (not DAX) supports 32-bit or new 96 or 128bit CPUs,
considering the data sturectures
are arrays and single lists,  it won't be too complicated to follow up.

>> The only restriction here by using pointer is
>> the CPU register word should be 64bits, because we use the NVDIMM as memory.
>>
>> Is it one of the way how NVDIMM (especially Intel AEP) designed to use ?
>> As a non-volatiled memory.
> Not for on-disk data structures.

This is not on-disk data structure. We use the NVDIMM as memory, and
access the internal data
structures as current existing code does onto DRAM.

Please encourage us to have a series try with this might-be-different idea.

>> Does the already mapped DAX base address change in runtime during memory
>> hot plugable ?
>> If not, it won't be a problem here for this specific use case.
> It could change between one use and another.

Hmm, I don't understand the implicit meaning of the above line.
Could you please offer a detail example ?


Thank you for looking at this and provide value comment. All the above
response is not argument or
stubbornness, I do want to have a clear understand by the discussion
with you, that we won't regret
in future for current design.

Coly Li

  reply	other threads:[~2021-06-23 10:06 UTC|newest]

Thread overview: 60+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-15  5:49 [PATCH 00/14] bcache patches for Linux v5.14 Coly Li
2021-06-15  5:49 ` [PATCH 01/14] bcache: fix error info in register_bcache() Coly Li
2021-06-22  9:47   ` Hannes Reinecke
2021-06-15  5:49 ` [PATCH 02/14] md: bcache: Fix spelling of 'acquire' Coly Li
2021-06-22 10:03   ` Hannes Reinecke
2021-06-15  5:49 ` [PATCH 03/14] bcache: add initial data structures for nvm pages Coly Li
2021-06-21 16:17   ` Ask help for code review (was Re: [PATCH 03/14] bcache: add initial data structures for nvm pages) Coly Li
2021-06-22  8:41     ` Huang, Ying
2021-06-23  4:32       ` Coly Li
2021-06-23  6:53         ` Huang, Ying
2021-06-23  7:04           ` Christoph Hellwig
2021-06-23  7:19             ` Coly Li
2021-06-23  7:21               ` Christoph Hellwig
2021-06-23 10:05                 ` Coly Li [this message]
2021-06-23 11:16                   ` Coly Li
2021-06-23 11:49                   ` Christoph Hellwig
2021-06-23 12:09                     ` Coly Li
2021-06-22 10:19   ` [PATCH 03/14] bcache: add initial data structures for nvm pages Hannes Reinecke
2021-06-23  7:09     ` Coly Li
2021-06-15  5:49 ` [PATCH 04/14] bcache: initialize the nvm pages allocator Coly Li
2021-06-22 10:39   ` Hannes Reinecke
2021-06-23  5:26     ` Coly Li
2021-06-23  9:16       ` Hannes Reinecke
2021-06-23  9:34         ` Coly Li
2021-06-15  5:49 ` [PATCH 05/14] bcache: initialization of the buddy Coly Li
2021-06-22 10:45   ` Hannes Reinecke
2021-06-23  5:35     ` Coly Li
2021-06-23  5:46       ` Re[2]: " Pavel Goran
2021-06-23  6:03         ` Coly Li
2021-06-15  5:49 ` [PATCH 06/14] bcache: bch_nvm_alloc_pages() " Coly Li
2021-06-22 10:51   ` Hannes Reinecke
2021-06-23  6:02     ` Coly Li
2021-06-15  5:49 ` [PATCH 07/14] bcache: bch_nvm_free_pages() " Coly Li
2021-06-22 10:53   ` Hannes Reinecke
2021-06-23  6:06     ` Coly Li
2021-06-15  5:49 ` [PATCH 08/14] bcache: get allocated pages from specific owner Coly Li
2021-06-22 10:54   ` Hannes Reinecke
2021-06-23  6:08     ` Coly Li
2021-06-15  5:49 ` [PATCH 09/14] bcache: use bucket index to set GC_MARK_METADATA for journal buckets in bch_btree_gc_finish() Coly Li
2021-06-22 10:55   ` Hannes Reinecke
2021-06-23  6:09     ` Coly Li
2021-06-15  5:49 ` [PATCH 10/14] bcache: add BCH_FEATURE_INCOMPAT_NVDIMM_META into incompat feature set Coly Li
2021-06-22 10:59   ` Hannes Reinecke
2021-06-23  6:09     ` Coly Li
2021-06-15  5:49 ` [PATCH 11/14] bcache: initialize bcache journal for NVDIMM meta device Coly Li
2021-06-22 11:01   ` Hannes Reinecke
2021-06-23  6:17     ` Coly Li
2021-06-23  9:20       ` Hannes Reinecke
2021-06-23 10:14         ` Coly Li
2021-06-15  5:49 ` [PATCH 12/14] bcache: support storing bcache journal into " Coly Li
2021-06-22 11:03   ` Hannes Reinecke
2021-06-23  6:19     ` Coly Li
2021-06-15  5:49 ` [PATCH 13/14] bcache: read jset from NVDIMM pages for journal replay Coly Li
2021-06-22 11:04   ` Hannes Reinecke
2021-06-23  6:21     ` Coly Li
2021-06-15  5:49 ` [PATCH 14/14] bcache: add sysfs interface register_nvdimm_meta to register NVDIMM meta device Coly Li
2021-06-22 11:04   ` Hannes Reinecke
2021-06-21 15:14 ` [PATCH 00/14] bcache patches for Linux v5.14 Jens Axboe
2021-06-21 15:25   ` Coly Li
2021-06-21 15:27     ` Jens Axboe

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=466c1678-8cdc-7240-1422-b435a599cad3@suse.de \
    --to=colyli@suse.de \
    --cc=axboe@kernel.dk \
    --cc=dan.j.williams@intel.com \
    --cc=hare@suse.com \
    --cc=hch@lst.de \
    --cc=jack@suse.com \
    --cc=jianpeng.ma@intel.com \
    --cc=linux-bcache@vger.kernel.org \
    --cc=linux-block@vger.kernel.org \
    --cc=qiaowei.ren@intel.com \
    --cc=ying.huang@intel.com \
    --subject='Re: Ask help for code review (was Re: [PATCH 03/14] bcache: add initial data structures for nvm pages)' \
    /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

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.