Thanks for looking into this patch Dan
Dan Williams <[email protected]> writes: > On Tue, Jul 13, 2021 at 1:25 PM Vaibhav Jain <[email protected]> wrote: >> >> Presently after performing a inject-smart the nvdimm flags reported are out >> of date as shown below where no 'smart_notify' or 'flush_fail' flags were >> reported even though they are set after injecting the smart error: >> >> $ sudo inject-smart -fU nmem0 >> [ >> { >> "dev":"nmem0", >> "health":{ >> "health_state":"fatal", >> "shutdown_state":"dirty", >> "shutdown_count":0 >> } >> } >> ] >> $ sudo cat /sys/class/nd/ndctl0/device/nmem0/papr/flags >> flush_fail smart_notify >> >> This happens because nvdimm flags are only parsed once during its probe and >> not refreshed even after a inject-smart operation makes them out of >> date. To fix this the patch adds a new export from libndctl named as >> ndctl_refresh_dimm_flags() that can be called after inject-smart that >> forces a refresh of nvdimm flags. This ensures that correct nvdimm flags >> are reported after the inject-smart operation as shown below: >> >> $ sudo ndctl inject-smart -fU nmem0 >> [ >> { >> "dev":"nmem0", >> "flag_failed_flush":true, >> "flag_smart_event":true, >> "health":{ >> "health_state":"fatal", >> "shutdown_state":"dirty", >> "shutdown_count":0 >> } >> } >> ] >> >> The patch refactors populate_dimm_attributes() to move the nvdimm flags >> parsing code to the newly introduced ndctl_refresh_dimm_flags() >> export. Since reading nvdimm flags requires constructing path using >> 'bus_prefix' which is only available during add_dimm(), the patch >> introduces a new member 'struct ndctl_dimm.bus_prefix' to cache its >> value. During ndctl_refresh_dimm_flags() the cached bus_prefix is used to >> read the contents of the nvdimm flag file and pass it on to the appropriate >> flag parsing function. > > I think this can be handled without needing an explicit > ndctl_refresh_dimm_flags() api. Teach all the flag retrieval apis to > check if the cached value has been invalidated and re-parse the flags. > Then teach the inject-smart path to invalidate the cached copy of the > flags. > Thanks for the suggestion. On the same line I think refreshing nvdimm flags can also be done implicitly after a successful ndctl_cmd_submit() call as that point we have interacted with the kernel_module which may have triggered a nvdimm flags change. Such a changeset would also be smaller compared to updating all flags retrieval api's to invalidate and refresh nvdimm flags. -- Cheers ~ Vaibhav
