On Tue, 6 Feb 2024 15:28:39 -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
> 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>
A few trivial things inline.
> static __init void hmat_add_locality(struct acpi_hmat_locality *hmat_loc)
> {
> struct memory_locality *loc;
> @@ -699,6 +729,12 @@ static void hmat_update_target_attrs(struct memory_target *target,
> u32 best = 0;
> int i;
>
> + /*
> + * Don't update if an external agent has changed the data.
Single line comment fine here I think.
> + */
> + if (target->ext_updated)
> + return;
> +
> +
> +static void remove_coord_notifier(void *data)
> +{
> + struct cxl_region *cxlr = data;
> +
> + unregister_memory_notifier(&cxlr->memory_notifier);
Simpler to pass in the notifier not the region.
More obvious that a remove notifier would take a notifier
and less code.
> +}
> +
> static int cxl_region_probe(struct device *dev)
> {
> struct cxl_region *cxlr = to_cxl_region(dev);
> @@ -3091,6 +3161,11 @@ 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);
> +
> /*
> * From this point on any path that changes the region's state away from
> * CXL_CONFIG_COMMIT is also responsible for releasing the driver.
