Re: [PATCH v4 12/16] libnvdimm, nfit: enable support for volatile ranges

From: Kani, Toshimitsu
Date: Wed Jul 05 2017 - 21:19:41 EST


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@xxxxxxx>
> 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