On Wed, May 3, 2017 at 8:38 AM, Dan Williams <[email protected]> wrote:
> On Wed, May 3, 2017 at 12:08 AM, Oliver O'Halloran <[email protected]> wrote:
>> On Wed, May 3, 2017 at 2:17 PM, Dan Williams <[email protected]>
>> wrote:
> [..]
>>>> Fair enough.
>>>
>>> All that said, there's nothing stopping us from making 'align' it's
>>> own mechanism. Where the first entry in the list is the current
>>> setting, in contrast to btt that decorates the current sector-size
>>> setting with square brackets.
>>
>> I'd be okay with this provided we force the alignment to one of the
>> supported values. Currently the only validation done by the kernel is:
>>
>> if (!is_power_of_2(val) || val < PAGE_SIZE || val > SZ_1G)
>> return -EINVAL;
>
> Yes, we'd need to validate the input against the supported values.
> There are no known binaries in the wild that I know of that depend on
> this looser definition, so we should be ok to change it.
>
>> So you can set an unsupported value by poking at sysfs directly. This
>> behaviour is useful for testing since you can use it to force an
>> alignment failure in the DAX fault handler.
>
> I'd rather move that test support to something like the nfit_test
> infrastructure.
>
>> I'm not overly concerned
>> if it goes, but it's something to keep in mind. I still think it would
>> be cleaner if we just added a separate attribute.
>
> I'm still having a hard time seeing how redundant sysfs attributes is "clean".
It turns out the NVML project is also parsing the 'align' attribute
outside of ndctl. So, now I'm with you, I think it would better to
move the 'possible alignments' to its own read-only attribute
('aligns'?) and leave 'align' as the interface to read/write the
current setting.
_______________________________________________
Linux-nvdimm mailing list
[email protected]
https://lists.01.org/mailman/listinfo/linux-nvdimm
