Hi Jason, On 5/17/2021 9:47 AM, Jason Gunthorpe wrote:
IB has a complex sysfs with a deep nesting of attributes. Nathan and Kees recently noticed this was not even slightly sane with how it was handling attributes and a deeper inspection shows the whole thing is a pretty "ick" coding style. Further review shows the ick extends outward from the ib_port sysfs and basically everything is pretty crazy. Simplify all of it: - Organize the ib_port and gid_attr's kobj's to have clear setup/destroy function pairings that work only on their own kobjs. - All memory allocated in service of a kobject's attributes is freed as part of the kobj release function. Thus all the error handling defers the memory frees to a put. - Build up lists of groups for every kobject and add the entire group list as a one-shot operation as the last thing in setup function. - Remove essentially all the error cleanup. The final kobject_put() will always free any memory allocated or do an internal kobject_del() if required. The new ordering eliminates all the other cleanup cases. - Make all attributes use proper typing for the kobj they are attached to. Split device and port hw_stats handling. - Create a ib_port_attribute type and change hfi1, qib and the CM code to work with attribute lists of ib_port_attribute type instead of building their own kobject madness This is sort of RFCy in that I qib and hfi1 stuff is complex enough it needs Dennis to look at it, and the core stuff has only passed basic testing at this moment. Nathan confirmed an earlier version solves the CFI warning.
This series still passes my basic testing of LTP's read_all test case on /sys with CFI in enforcing mode. If there is any more in-depth testing, I can put it through, let me know. I'll continue testing the series and when it is in a mergeable state, I can provide you with a Tested-by tag.
Cheers, Nathan