Re: [PATCH 2/2] libnvdimm, namespace: fix btt claim class crash

[Saldivar, Maurice A.]

I applied this and the kernel patch and was successfully able to
configure an NVDIMM-N into sector mode. I do have a question about
specific behavior I observed. Before I was able to configure an NVDIMM
into any mode from any mode. With these patchs if I go to sector mode
and try going to memory or dax mode it fails with:Â

# ndctl create-namespace -f -e namespace0.0 -m memory
 Error: namespace0.0: sector_size: 4096 not supported
failed to reconfigure namespace: Invalid argument
Â
but if I go sector->raw-><mode> I can get into other modes fine. Was
this the intended behavior?Â

Sincerely,Â
Maurice Â

On Tue, 2017-09-19 at 12:51 -0700, Dan Williams wrote:
> Maurice reports:
> 
> ÂÂÂÂBUG: unable to handle kernel NULL pointer dereference at
> 0000000000000028
> ÂÂÂÂIP: holder_class_store+0x253/0x2b0 [libnvdimm]
> 
> ...while trying to reconfigure an NVDIMM-N namespace into 'sector' /
> 'btt' mode. The crash points to this line:
> 
> ÂÂÂÂ(gdb) li *(holder_class_store+0x253)
> ÂÂÂÂ0x7773 is in holder_class_store
> (drivers/nvdimm/namespace_devs.c:1420).
> ÂÂÂÂ1415ÂÂÂÂÂÂÂÂÂÂÂÂfor (i = 0; i < nd_region->ndr_mappings; i++) {
> ÂÂÂÂ1416ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂstruct nd_mapping *nd_mapping =
> &nd_region->mapping[i];
> ÂÂÂÂ1417ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂstruct nvdimm_drvdata *ndd =
> to_ndd(nd_mapping);
> ÂÂÂÂ1418ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂstruct nd_namespace_index *nsindex;
> ÂÂÂÂ1419
> ÂÂÂÂ1420ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂnsindex = to_namespace_index(ndd, ndd-
> >ns_current);
> 
> ...where we are failing because ndd is NULL due to NVDIMM-N dimms not
> supporting labels.
> 
> Long story short, default to the BTTv1 format in the label-less /
> NVDIMM-N case.
> 
> Fixes: 14e494542636 ("libnvdimm, btt: BTT updates for UEFI 2.7
> format")
> Cc: <stable@xxxxxxxxxxxxxxx>
> Cc: Vishal Verma <vishal.l.verma@xxxxxxxxx>
> Reported-by: Maurice A. Saldivar <maurice.a.saldivar@xxxxxxx>
> Signed-off-by: Dan Williams <dan.j.williams@xxxxxxxxx>
> ---
> Âdrivers/nvdimm/namespace_devs.c |ÂÂÂÂ9 +++++++++
> Â1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/nvdimm/namespace_devs.c
> b/drivers/nvdimm/namespace_devs.c
> index 1427a386a033..3e4d1e7998da 100644
> --- a/drivers/nvdimm/namespace_devs.c
> +++ b/drivers/nvdimm/namespace_devs.c
> @@ -1417,6 +1417,15 @@ static int btt_claim_class(struct device *dev)
> Â		struct nvdimm_drvdata *ndd = to_ndd(nd_mapping);
> Â		struct nd_namespace_index *nsindex;
> Â
> +		/*
> +		Â* If any of the DIMMs do not support labels the
> only
> +		Â* possible BTT format is v1.
> +		Â*/
> +		if (!ndd) {
> +			loop_bitmask = 0;
> +			break;
> +		}
> +
> Â		nsindex = to_namespace_index(ndd, ndd->ns_current);
> Â		if (nsindex == NULL)
> Â			loop_bitmask |= 1;
>