On 10/14/24 23:24, Dan Williams wrote:
Alejandro Lucero Palau wrote:
On 10/12/24 22:57, Dan Williams wrote:
Alejandro Lucero Palau wrote: [..]
I am skeptical that PROBE_FORCE_SYNCRONOUS is a fix for any device-readiness bug. Some other assumption is violated if that is required.
But that problem is not about device readiness but just how the device model works. In this case the memdev creation is adding devices, no real ones but those abstractions we use from the device model, and that device creation is done asynchronously.
Device creation is not done asynchronously, the PCI driver is attaching asynchrounously. When the PCI driver attaches it creates memdevs and those are attached to cxl_mem synchronously.
memdev, a Type2 driver in my case, is going to work with such a device abstraction just after the memdev creation, it is not there yet.
Oh, is the concern that you always want to have the memdev attached to cxl_mem immediately after it is registered?
I think that is another case where "MODULE_SOFTDEP("pre: cxl_mem")" is needed. However, to fix this situation once and for all I think I would rather just drop all this modularity and move both cxl_port and cxl_mem to be drivers internal to cxl_core.ko similar to the cxl_region driver.
Oh, so the problem is the code is not ready because the functionality is in a module not loaded yet.
Right.
Then it makes sense that change. I'll do it if not already taken. I'll send v4 without the PROBE_FORCE_SYNCHRONOUS flag and without the previous loop with delays implemented in v3.
So I think EPROBE_DEFER can stay out of the CXL core because it is up to the accelerator driver to decide whether CXL availability is fatal to init or not.
It needs support from the cxl core though. If the cxl root is not there yet, the driver needs to know, and that is what you did in your original patch and I'm keeping as well.
Additionally, I am less and less convinced that Type-2 drivers should be forced to depend on the cxl_mem driver to attach vs just arranging for those Type-2 drivers to call devm_cxl_enumerate_ports() and devm_cxl_add_endpoint() directly. In other words I am starting to worry that the generic cxl_mem driver design pattern is a midlayer mistake.
You know better than me but in my view, a Type2 should follow what a Type3 does with some small changes for dealing with the differences, mainly the way it is going to be used and those optional capabilities for Type2. This makes more sense to me for Type1.
So, if it makes it easier for sfc, I would be open to exploring a direct scheme for endpoint attachment, and not requiring the generic cxl_mem driver as an intermediary.
V4 is ready (just having problems when testing with 6.12-rcX) so I would like to keep it and explore this once we have something working and accepted. Type2 and Type1 with CXL cache will bring new challenges and I bet we will need refactoring in the code and potentially new design for generic code (Type3 and Type2, Type2 and Type1).