Re: [PATCH v9 14/27] sfc: create type2 cxl memdev
flat view
From: Dan Williams <hidden>
Date: 2025-01-18 02:41:28
Also in:
linux-cxl
alejandro.lucero-palau@ wrote:
quoted hunk ↗ jump to hunk
From: Alejandro Lucero <redacted> Use cxl API for creating a cxl memory device using the type2 cxl_dev_state struct. Signed-off-by: Alejandro Lucero <redacted> Reviewed-by: Martin Habets <redacted> Reviewed-by: Fan Ni <redacted> Acked-by: Edward Cree <ecree.xilinx@gmail.com> Reviewed-by: Jonathan Cameron <redacted> --- drivers/net/ethernet/sfc/efx_cxl.c | 9 +++++++++ 1 file changed, 9 insertions(+)diff --git a/drivers/net/ethernet/sfc/efx_cxl.c b/drivers/net/ethernet/sfc/efx_cxl.c index 911f29b91bd3..f4bf137fd878 100644 --- a/drivers/net/ethernet/sfc/efx_cxl.c +++ b/drivers/net/ethernet/sfc/efx_cxl.c@@ -96,10 +96,19 @@ int efx_cxl_init(struct efx_probe_data *probe_data) */ cxl_set_media_ready(cxl->cxlds);
It is unfortunate the media_ready is just being used as an offline/online flag for memdevs. I would be open to just switching to typical device online/offline semantics and drop the "media_ready" flag.
quoted hunk ↗ jump to hunk
+ cxl->cxlmd = devm_cxl_add_memdev(&pci_dev->dev, cxl->cxlds); + if (IS_ERR(cxl->cxlmd)) { + pci_err(pci_dev, "CXL accel memdev creation failed"); + rc = PTR_ERR(cxl->cxlmd); + goto err_memdev; + } + probe_data->cxl = cxl; return 0; +err_memdev: + cxl_release_resource(cxl->cxlds, CXL_RES_RAM); err_resource_set: kfree(cxl->cxlds);
In general a function should not mix devm and goto as that is a recipe for bugs. The bug I see here is that devm_cxl_add_memdev() runs the teardown flow *after* efx_pci_probe() returns an error code. That happens in device_unbind_cleanup(), but when it goes to cleanup endpoint decoders and anything else that might reference @cxlds it crashes because @cxlds is long gone. So if you use devm_cxl_add_memdev() then cxlds must be devm allocated as well to make sure it gets freed in the proper reverse order.