On Tue, 20 Feb 2024 16:12:40 -0700
Dave Jiang <dave.jiang@xxxxxxxxx> wrote:
> When the CXL region is formed, the driver would computed the performance
> data for the region. However this data is not available at the node data
> collection that has been populated by the HMAT during kernel
> initialization. Add a memory hotplug notifier to update the access
> coordinates to the 'struct memory_target' context kept by the
> HMAT_REPORTING code.
>
> Add CXL_CALLBACK_PRI for a memory hotplug callback priority. Set the
> priority number to be called before HMAT_CALLBACK_PRI. The CXL update must
> happen before hmat_callback().
>
> A new HMAT_REPORING helper hmat_update_target_coordinates() is added in
REPORTING
> order to allow CXL to update the memory_target access coordinates.
>
> A new ext_updated member is added to the memory_target to indicate that
> the access coordinates within the memory_target has been updated by an
> external agent such as CXL. This prevents data being overwritten by the
> hmat_update_target_attrs() triggered by hmat_callback().
>
> Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
> Cc: Rafael J. Wysocki <rafael@xxxxxxxxxx>
> Reviewed-by: "Huang, Ying" <ying.huang@xxxxxxxxx>
> Signed-off-by: Dave Jiang <dave.jiang@xxxxxxxxx>
One missing error check and another trivial comment.
With error check handled.
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>
> +static void remove_coord_notifier(void *data)
> +{
> + struct notifier_block *memory_notifier = data;
> +
> + unregister_memory_notifier(memory_notifier);
Trivial but no real loss of info if you do
unregister_memory_notifier(data);
> +}
> +
> static int cxl_region_probe(struct device *dev)
> {
> struct cxl_region *cxlr = to_cxl_region(dev);
> @@ -3081,6 +3151,12 @@ static int cxl_region_probe(struct device *dev)
> goto out;
> }
>
> + cxlr->memory_notifier.notifier_call = cxl_region_perf_attrs_callback;
> + cxlr->memory_notifier.priority = CXL_CALLBACK_PRI;
> + register_memory_notifier(&cxlr->memory_notifier);
> + rc = devm_add_action_or_reset(&cxlr->dev, remove_coord_notifier,
> + &cxlr->memory_notifier);
> +
Check rc? Very unlikely to fail, but you never know..
> /*
> * From this point on any path that changes the region's state away from
> * CXL_CONFIG_COMMIT is also responsible for releasing the driver.
