Page MenuHomeFreeBSD

nvdimm(4): Only expose namespaces for accessible data SPAs
ClosedPublic

Authored by scottph on Oct 11 2019, 10:27 PM.

Details

Summary

Apply the same user accessible filter to namespaces as is applied
to full-SPA devices. Also, explicitly filter out control region
SPAs which don't expose the nvdimm data area.

MFC after: 1 week
Sponsored by: Intel Corporation

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

scottph created this revision.Oct 11 2019, 10:27 PM
imp added inline comments.Oct 11 2019, 10:49 PM
sys/dev/nvdimm/nvdimm_spa.c
164 ↗(On Diff #63170)

Why the loop? Why not just a single line that's return? I'm guessing there is a field we need to check

scottph added inline comments.Oct 11 2019, 11:04 PM
sys/dev/nvdimm/nvdimm_spa.c
164 ↗(On Diff #63170)

SPA_TYPE_UNKNOWN = 127,
is outside the bounds of the array. How about:

return (spa_type >= 0 && spa_type < nitems(nvdimm_SPA_uuid_list) ? nvdimm_SPA_uuid_list[spa_type].u_usr_acc : false);

cem added a comment.Oct 12 2019, 12:10 AM

Do Control Region SPAs have namespaces? I'm still fairly new to NFIT.

sys/dev/nvdimm/nvdimm_spa.c
164 ↗(On Diff #63170)

I like the suggestion and the proposal. I'm not sure it needs to be a one-liner but I'm on-board with the bounds check approach. I'd probably spell it more like:

if ((int)spa_type < 0 || spa_type >= nitems(nvdimm_SPA_uuid_list))
    return (false);
return (nvdimm_SPA_uuid_list[spa_type].u_usr_acc);

But it doesn't really matter to me how it's formatted.

In D21987#480367, @cem wrote:

Do Control Region SPAs have namespaces? I'm still fairly new to NFIT.

No, namespaces should only apply to areas where there's a mapping to the dimm physical addresses, which I believe is never the case with a control region. Which leads me to think that this check is necessary (don't show namespaces for regions that we also hide) but not sufficient on its own.

scottph updated this revision to Diff 64215.Tue, Nov 12, 1:03 AM
scottph retitled this revision from nvdimm(4): Don't expose namespaces for inaccessible SPAs to nvdimm(4): Only expose namespaces for accessible data SPAs.
scottph edited the summary of this revision. (Show Details)
scottph marked 2 inline comments as done.
scottl accepted this revision.Tue, Nov 12, 1:05 AM
This revision is now accepted and ready to land.Tue, Nov 12, 1:05 AM
cem accepted this revision.Tue, Nov 12, 1:30 AM
This revision was automatically updated to reflect the committed changes.