Re: [PATCH V1 1/2] libnvdimm/nsio: differentiate between probe mapping and runtime mapping

["Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com>]

Dan Williams <dan.j.williams@intel.com> writes:

> On Wed, Oct 16, 2019 at 8:19 PM Aneesh Kumar K.V
> <aneesh.kumar@linux.ibm.com> wrote:
>>
>> On 10/17/19 8:34 AM, Dan Williams wrote:
>> > On Wed, Oct 16, 2019 at 7:43 PM Aneesh Kumar K.V
>>
>>
>> ....
>>
>> >>>
>> >> The error is printed by generic code and the failures are due to fixed
>> >> size. We can't workaround that using vmalloc=<size> option.
>> >
>> > Darn.
>> >
>> > Ok, explain to me again how this patch helps. This just seems to delay
>> > the inevitable failure a bit, but the end result is that the user
>> > needs to pick and choose which namespaces to enable after the kernel
>> > has tried to auto-probe namespaces.
>> >
>>
>> Right now we map the entire namespace using I/O remap range while
>> probing and if we find the namespace mode to be fsdax or devdax we unmap
>> the namespace and map them back using direct-map range. This temporary
>> mapping cause transient failures if two large namespaces were probed at
>> the same time. We try to map the second names space in the I/O remap
>> range while the first one is already mapped there. ie,
>>
>> If we have two 6TB namespaces with an I/O remap range limit of 8TB. We
>> can individually create these namespaces and enable/disable them because
>> they are within the 8TB limit. But if we try to probe them together,
>> and if namespace1 is already mapped in the I/O remap range we have only
>> 2TB space left in the I/O remap range and trying to probe the second
>> namespace at the same time fails due to lack of space to map the
>> namespace during probing.
>>
>> This is not an issue with raw/btt, because they keep the namespace
>> always mapped in the I/O remap range and if we are able to create a
>> namespace we can enable them together. (we do have the issue of creating
>> large raw/btt namespace while other raw/btt namespaces are disabled and
>> trying to enable them all together).
>>
>> The fix proposed in this patch is to not map the full namespace range
>> while probing. We just need to map the reserve block to find the
>> namespace mode. Based on the detected namespace mode, we either use
>> direct-map range or I/O range to map the full namespace.
>>
>
> Ah, ok, you're not trying to address the case where raw and btt
> namespaces don't fit you're trying to fix the case where raw and btt
> namespaces *do* fit in the vmap, but pfn / fsdax namespaces exceed the
> vmap.
>
> I would highlight that in the changelog, namely that an otherwise
> working configuration with two namespaces (one btt and one pfn), where
> the btt namespace consumes almost all the vmap space, will fail to
> enable due to needing too much temporary vmap space to initialize the
> pfn mode namespace.
>
> Let's also make this a bit more self contained to devm_nsio_enable()
> and add a length parameter. Then when btt is ready to start accessing
> more than the initial 8K it can be just be called again with the full
> namespace size. Then devm_nsio_enable can notice when the mapping
> needs to be expanded vs established.
>
> Although btt should not be making assumptions about the underlying
> namepsace type so it would need to call a new devm_ndns_enable()
> wrapper that calls devm_nsio_enable() for nsio namespaces and is a nop
> for ndblk namespaces.

How about the below?

modified   drivers/nvdimm/pmem.c
@@ -491,17 +491,26 @@ static int pmem_attach_disk(struct device *dev,
 static int nd_pmem_probe(struct device *dev)
 {
 	int ret;
+	struct nd_namespace_io *nsio;
 	struct nd_namespace_common *ndns;
 
 	ndns = nvdimm_namespace_common_probe(dev);
 	if (IS_ERR(ndns))
 		return PTR_ERR(ndns);
 
-	if (devm_nsio_enable(dev, to_nd_namespace_io(&ndns->dev)))
-		return -ENXIO;
+	nsio = to_nd_namespace_io(&ndns->dev);
 
-	if (is_nd_btt(dev))
+	if (is_nd_btt(dev)) {
+		/*
+		 * Map with resource size
+		 */
+		if (devm_nsio_enable(dev, nsio, resource_size(&nsio->res)))
+			return -ENXIO;
 		return nvdimm_namespace_attach_btt(ndns);
+	}
+
+	if (devm_nsio_enable(dev, nsio, info_block_reserve()))
+		return -ENXIO;
 
 	if (is_nd_pfn(dev))
 		return pmem_attach_disk(dev, ndns);

-aneesh
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org