Re: [PATCHv2 3/3] nvme: Expose cntrltype and dctype through sysfs

Sagi Grimberg
2022-02-06T15:56:13Z

>> The idea was to add a separate udev announcement that would tell
>> userspace that these attributes are stable (it actually serves
>> a different purpose, but it does come after that point).
>
> like KOBJECT_CHANGE is there for?

Yes, that was the point, to add a change event (patch 1 in
the series).

>>> So there's a reason you are being forced to add a driver-core change
>>> like this, because it's not something you should do :)
>>
>> I do agree that its a bit backwards to do this update. But the device
>> even today is not fully usable when it is created, so we know no
>> userspace program relies on the creation announcement.
>>
>>>> So now I'm in a bit of a pickle. Sagi suggested this current patch set, but from what you're saying this is not a good approach. If you could suggest a solution that would satisfy all parties, I would greatly appreciate it.
>>>
>>> Wait to create the device when you know the device type.
>>
>> The initial proposal was to defer the device creation to after the
>> device is fully initialized. That though is a heavier lift because the
>> device initialization is something that happens on every reset or
>> reconnection after connectivity loss,
>
> That's fine, what's wrong with tearing it all down and bringing it back
> up then? That's what the hardware just did, don't fake things out by
> not conveying the same stuff to userspace.
>
>> so if we move the device creation there we have to now check if this
>> is the first time we are doing this or not, and also error recovery
>> becomes more opinionated...
>
> I still think you should not create anything until you know what it is.

We probably should take a step back and understand if we can delay the
controller device creation. There are some corner cases that I'm
concerned we might introduce if we do that, but it may be the right
thing to do...

Or, we can expose this specific attribute via a different interface like
ioctl, or embedding it in the uevent that we send anyways...