[PATCHv2] nvme/hwmon: rework to avoid devm allocation

[Guenter Roeck]

On 1/19/21 11:07 AM, Hannes Reinecke wrote:
> On 1/19/21 3:56 PM, Guenter Roeck wrote:
>> On Tue, Jan 19, 2021 at 07:43:18AM +0100, Hannes Reinecke wrote:
>>> The original design to use device-managed resource allocation
>>> doesn't really work as the NVMe controller has a vastly different
>>> lifetime than the hwmon sysfs attributes, causing warning about
>>> duplicate sysfs entries upon reconnection.
>>> This patch reworks the hwmon allocation to avoid device-managed
>>> resource allocation, and uses the NVMe controller as parent for
>>> the sysfs attributes.
>>
>> I don't really know enough about the nvme infrastructure to understand
>> this part of the change. Couple of questions: Why is the parent change
>> needed, and does the "sensors" command still work after this change ?
>>
> Problem is that it's not really a 'new' infrastructure, it's NVMe-over-fabrics, which behaves slightly different from the PCI-based NVMe devices.
> In particular we can and will have connectivity failures, requiring the controller to be reset or recreated.
> And as the hwmon sysfs attributes were never removed correctly in the first place we're seeing these errors.
> It isn't easily recreated on PCI devices as the sysfs attributes will only ever be removed on shutdown (or module unload).
> So I fear the issue was with us since day-1.
> 
I understand; that is why I suggested to add a Fixes: tag.

> As for the parent change: for fabrics the '->dev' device points to a static nvmf_device, which is identical for every fabrics controller.
> So you really need to use the actual device in '->device' to have a different parent device for different controllers.
> So for PCI that means that the 'hwmon' device moved one element down (from the PCI device to the nvme controller).
> But shouldn't make much of a difference; I'll check on if 'sensors' continues to work.
> 

It needs lm-sensors version 3.5.0 or later. Older versions
don't support a device hierarchy.

Guenter