On 03/09/2018 04:23 PM, Dan Williams wrote: > On Fri, Mar 9, 2018 at 11:58 AM, Jeff Moyer <[email protected]> wrote: >> Dan Williams <[email protected]> writes: >> >>> On Fri, Mar 9, 2018 at 11:39 AM, Jeff Moyer <[email protected]> wrote: >>>> Dan Williams <[email protected]> writes: >>>> >>>>> Commit a7320456f1bc "ndctl: add sector_size to 'ndctl list' output" >>>>> mishandles the case where a namespace does not specify a sector_size, >>>>> but otherwise supports a sector_size selection: >>>>> >>>>> # ndctl list --namespace=namespace1.0 >>>>> { >>>>> "dev":"namespace1.0", >>>>> "mode":"memory", >>>>> "size":32763805696, >>>>> "uuid":"7c985ba5-6d33-48bd-8fde-6c25a520abe0", >>>>> "sector_size":-1, >>>>> "blockdev":"pmem1", >>>>> "numa_node":0 >>>>> } >>>>> >>>>> Fix this and clean up the output to only provide a sector_size in the >>>>> non-default (i.e. != 512 bytes) case. >>>> >>>> That sounds confusing. Why not just always print out the sector size? >>> >>> I'm assuming the users that will go through the hassle of specifying a >>> non-default sector_size are few and far between. This helps with the >>> useful information density on a single screen for humans looking at >>> json output. >> >> I think that, to the novice, seeing one namespace print out sector size >> and another not do the same, that would be confusing. Also, you'd have >> to know what the default is.
As a former ndctl user, I'm with Jeff. > Yes, but personally I think it is only an expert that would care about > non-512 byte sector sizes. There are no device physics or other > benefits that require different sector sizes. The only potential > benefit is amortizing the cost of BTT over larger sectors, but for > Linux ext4 and xfs should be fine without a BTT. The UEFI spec > unfortunately requires a default of 4K sector size, but that's only > for inter-OS compatibility. I'm just finding it hard to justify > treating sector_size as a first class attribute that the typical user > would care about. Since the value can be set, even to the default value, and the kernel default doesn't match the UEFI default, it seems worth including in all cases. -- ljk > _______________________________________________ > Linux-nvdimm mailing list > [email protected] > https://lists.01.org/mailman/listinfo/linux-nvdimm > _______________________________________________ Linux-nvdimm mailing list [email protected] https://lists.01.org/mailman/listinfo/linux-nvdimm
