From: "Kani, Toshimitsu" <toshi.kani@hpe.com>
To: "dan.j.williams@intel.com" <dan.j.williams@intel.com>
CC: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "hch@lst.de" <hch@lst.de>,
        "viro@zeniv.linux.org.uk" <viro@zeniv.linux.org.uk>,
        "x86@kernel.org" <x86@kernel.org>,
        "mawilcox@microsoft.com" <mawilcox@microsoft.com>,
        "linux-nvdimm@lists.01.org" <linux-nvdimm@lists.01.org>,
        "Knippers, Linda" <linda.knippers@hpe.com>,
        "linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>,
        "jack@suse.cz" <jack@suse.cz>
Subject: Re: [PATCH v4 12/16] libnvdimm, nfit: enable support for volatile
 ranges
Thread-Topic: [PATCH v4 12/16] libnvdimm, nfit: enable support for volatile
 ranges
Thread-Index: AQHS8QGnKkts09hmeUykqHaORyWjLKI8N3yAgAAW54CAAAmYgIAACV8AgAAGVgCAAAR7gIAAAckAgAACPYCAAAHVAIAAAn8AgAAEW4CAACVlAIAJTuiAgAAIlICAABEOAA==
Date: Thu, 6 Jul 2017 01:17:45 +0000
Message-ID: <1499303324.2042.7.camel@hpe.com>
References: <149875877608.10031.17813337234536358002.stgit@dwillia2-desk3.amr.corp.intel.com>
         <149875884190.10031.6179599135820559644.stgit@dwillia2-desk3.amr.corp.intel.com>
         <595552F5.5040008@hpe.com>
         <CAPcyv4jr0U-a521+wAL7RDca_AoUrtvO6yeY6d0UQpY7d=CHuQ@mail.gmail.com>
         <59556E37.80808@hpe.com>
         <CAPcyv4g6ijkWncZAbP-Bmbf+J7a2hR-JFLSX-ji6v-+oG0PDJQ@mail.gmail.com>
         <d073b4a1-ff90-b44a-adf2-ad44c0d70c69@hpe.com>
         <CAPcyv4huf0ENTz=LEVC6xC7xKRPc-GAZJsZMTSYpy+UoaduTMg@mail.gmail.com>
         <595580A6.9000004@hpe.com>
         <CAPcyv4gtTYEUELPtr9mi5QkGL+cm=jcFR-Oe07OCuNM9izh6jA@mail.gmail.com>
         <d545a1ea-0199-7462-58c5-067cdc22cacc@hpe.com>
         <CAPcyv4gnkpWXJ7jOJ7r0kWLKoLfuqJzdHTjANBUPztyTWUrOjA@mail.gmail.com>
         <595589CF.5010605@hpe.com>
         <CAPcyv4iTaXc92ggQcBnqJ=P0jsMF9HhnV4nAsr1Qos4pjrRDLA@mail.gmail.com>
         <1499297819.2042.5.camel@hpe.com>
         <CAPcyv4ixQFNT_8tGFyf2ABqxpPd-jmtXKVeLBRdzBW12u3TJbg@mail.gmail.com>
In-Reply-To: <CAPcyv4ixQFNT_8tGFyf2ABqxpPd-jmtXKVeLBRdzBW12u3TJbg@mail.gmail.com>
Accept-Language: en-US
Content-Language: en-US
spamdiagnosticoutput: 1:99
spamdiagnosticmetadata: NSPM
Content-Type: text/plain; charset="utf-8"
Content-ID: <B75AD631902C9741BCA2DF2D8438F2F3@NAMPRD84.PROD.OUTLOOK.COM>
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-originalarrivaltime: 06 Jul 2017 01:17:45.2641
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 105b2061-b669-4b31-92ac-24d304d195dc
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DF4PR84MB0075
X-OriginatorOrg: hpe.com
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Transfer-Encoding: 8bit
X-MIME-Autoconverted: from base64 to 8bit by nfs id v661Jg5U023937
Content-Length: 3515
Lines: 79

On Wed, 2017-07-05 at 17:07 -0700, Dan Williams wrote:
> On Wed, Jul 5, 2017 at 4:46 PM, Kani, Toshimitsu <toshi.kani@hpe.com>
> wrote:
> > On Thu, 2017-06-29 at 18:28 -0700, Dan Williams wrote:
 :
> > 
> > Sorry for being late to respond, but I agree with Linda that this
> > naming policy is likely to confuse users.  I also care less about
> > the current users who use memmap option.  This case is pmem-
> > emulation and they know what they are doing.
> > 
> > Assuming block device interface is needed (in addition to device-
> > dax) for volatile range for use-cases like swap device, I wonder if
> > user can actually specify a right pmem device for swap from OS-
> > install GUI when both volatile and persistent block devices are
> > listed as /dev/pmemN.  Sometimes we are restricted with GUI
> > menu.  Some users use GUI all the time like Windows as well.
> > 
> > Can we differentiate the naming by adding 'v' like 'pmemNv' (if you
> > can't go with 'vmemN')?  I don't think having 's' for BTT was that
> > bad.  It's been helpful to tell users that these pmem devices are
> > not byte-addressable.  I also think that BTT for volatile range
> > makes no sense (unless emulated as persistent memory by memmap
> > option).
> 
> I'm more worried about sending the wrong signal the other way. That
> users believe that the 'p' means definitely "persistent" when we have
> no way to guarantee that.

That's a valid point.  But isn't it vendors' responsibility to
guarantee it when their devices are described as persistent in one way
or the other in FW?

> If it was only memmap= that we had to worry about that would be one
> thing, but we apparently have vendors that are shipping "e820-type-12
> memory" as their NVDIMM solution [1].

Type-12 is persistent memory in a non-standard FW interface.  So, it
makes sense to show it as pmem.

> We've also been shipping the policy that 'pmem' may front a volatile
> range ever since v4.8 (commit c2f32acdf848 "acpi, nfit: treat virtual
> ramdisk SPA as pmem region"). At least now we have the "nd_volatile"
> region type. Any change of the device name now is potentially a
> regression for environments that are already expecting /dev/pmemX.

Hmm... I thought this was for mapping ISO image for booting.  Does it
get listed as a regular pmem device and allow user to modify its
content?  I doubt this case being used today, though.  (It was
prototyped on an HPE box and I can check the status if needed.)

> As far as I know there are no OS installers that understand pmem. 

It's actually the other way around.  It was changed not to list pmem
devices since OS cannot boot from a pmem yet...

> When they do add support I think it would be straightforward to avoid
> confusion and filter "volatile" hosted pmem devices from the install
> target list. I don't see this being much different from the confusion
> when users can not differentiate their 'sd' device between USB and
> SATA. 

Right, such changes can be made.  It was just an example that typical
use-cases today do not require additional step to check persistence of
block devices.

> We have symlinks in /dev/disk/by* to make it easier to identify
> storage devices, I think it makes sense to add udev rules for
> identifying volatile pmem and not try to differentiate this in the
> default kernel device name.

I am not sure what might be a good way, but I am concerned because a
single block device naming do not represent both volatile and
persistent media today.

Thanks,
-Toshi