On Wed, May 3, 2017 at 8:38 AM, Dan Williams <dan.j.williams(a)intel.com> wrote:
On Wed, May 3, 2017 at 12:08 AM, Oliver O'Halloran
> On Wed, May 3, 2017 at 2:17 PM, Dan Williams <dan.j.williams(a)intel.com> wrote:
>>> Fair enough.
>> All that said, there's nothing stopping us from making 'align'
>> 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
> 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
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