Re: btt ndctl question

On Tue, Jul 26, 2016 at 2:41 PM, Vishal Verma <vishal.l.verma(a)intel.com&gt; wrote: > On 07/26, Dan Williams wrote: >> On Tue, Jul 26, 2016 at 2:24 PM, Verma, Vishal L >> <vishal.l.verma(a)intel.com&gt; wrote: >>> On Tue, 2016-07-26 at 14:58 -0600, Vishal Verma wrote: >>>> On 07/26, Linda Knippers wrote: >>>>> >>>>> My system has 4 8G NVDIMMs and I have them configured in different >>>>> ways, as you can >>>>> see from the ndctl output: >>>>> >>>>> $ ndctl list >>>>> [ >>>>> { >>>>> "dev":"namespace3.0", >>>>> "mode":"raw", >>>>> "size":8589934592, >>>>> "blockdev":"pmem3" >>>>> }, >>>>> { >>>>> "dev":"namespace2.0", >>>>> "mode":"memory", >>>>> "size":8587837440, >>>>> "uuid":"2567d762-68ae-486b-a6eb-2d3ab1b9dca9", >>>>> "blockdev":"pmem2" >>>>> }, >>>>> { >>>>> "dev":"namespace1.0", >>>>> "mode":"sector", >>>>> "uuid":"44fb474e-7db8-4438-ad95-05ecb9f2075e", >>>>> "sector_size":4096, >>>>> "blockdev":"pmem1s" >>>>> }, >>>>> { >>>>> "dev":"namespace0.0", >>>>> "mode":"memory", >>>>> "size":8453619712, >>>>> "uuid":"933ed54b-5b64-47f1-8409-c88f7c846522", >>>>> "blockdev":"pmem0" >>>>> } >>>>> ] >>>>> >>>>> The two memory namespaces have different sizes because one is -- >>>>> map=dev and the other is --map=mem. >>>>> It would be nice if the map option was displayed but my question is >>>>> about the size value for the >>>>> btt device, or lack of one. >>>>> >>>>> All the namespaces show a size except for the btt. The btt only >>>>> shows a sector size. There >>>>> is no size value exposed by the btt sysfs information, which is >>>>> probably why it's not in ndctl. >>>>> >>>>> I know the size can be gotten from the block device but it looks >>>>> like an omission here. >>>>> Is this a bug or a feature? >>>> >>>> Probably an omission :) >>>> This patch should expose a size attribute in sysfs: >>>> >>>> $ cat /sys/bus/nd/devices/btt7.0/size >>>> 32440320 >>>> >>>> I can look at the 'ndctl list' enabling too if this looks good. >>>> >>>> 8<------ >>>> >>>> From fb119bf4380d1d65d82754e581bbd41161c2100f Mon Sep 17 00:00:00 2001 >>>> From: Vishal Verma <vishal.l.verma(a)intel.com&gt; >>>> Date: Tue, 26 Jul 2016 14:54:39 -0600 >>>> Subject: [PATCH] nvdimm, btt: add a size attribute for BTTs >>>> >>>> To be consistent with other namespaces, expose a 'size' attribute for >>>> BTT devices also. >>>> >>>> Cc: Dan Williams <dan.j.williams(a)intel.com&gt; >>>> Reported-by: Linda Knippers <linda.knippers(a)hpe.com&gt; >>>> Signed-off-by: Vishal Verma <vishal.l.verma(a)intel.com&gt; >>>> --- >>>> drivers/nvdimm/btt.c | 1 + >>>> drivers/nvdimm/btt_devs.c | 15 +++++++++++++++ >>>> drivers/nvdimm/nd.h | 1 + >>>> 3 files changed, 17 insertions(+) >>>> >>>> diff --git a/drivers/nvdimm/btt.c b/drivers/nvdimm/btt.c >>>> index 68a7c3c..71ce0dc 100644 >>>> --- a/drivers/nvdimm/btt.c >>>> +++ b/drivers/nvdimm/btt.c >>>> @@ -1270,6 +1270,7 @@ static int btt_blk_init(struct btt *btt) >>>> } >>>> } >>>> set_capacity(btt->btt_disk, btt->nlba * btt->sector_size >> >>>> 9); >>>> + btt->nd_btt->size = btt->nlba * btt->sector_size; >>> >>> Blargh, I think I was a bit hasty; I think this should be: >>> >>> + btt->nd_btt->size = btt->nlba * (u64)btt->sector_size; >>> >>> Right? (I always get bit by integer promotion rules...) >> >> ...but at this point we're identical to what the block layer is >> reporting. The other 'size' attributes are communicating the raw >> byte-aligned capacity of the namespace minus local driver overhead. > > Wouldn't we want to be identical to what the block layer is reporting? > The only difference would come from removing any sub-sector capacity in > our case - is that information useful/desirable? I'd think being > consistent with the block layer reporting makes most sense.. > It does for btt, but since it's impossible for btt/size to a return a different answer than block/size, userspace should just look at block/size. We have the 'size' attribute independent of sector alignment for the other configurations because those use cases might be ignoring the block interface... well only in the device-dax case now that we de-featured raw block-device dax. I guess we could just go ahead and add just to be symmetrical, but the motivation for the other 'size' attributes is that they could identify capacity lost to sector alignment.