Commit b3a88225 authored by Dan Williams's avatar Dan Williams Committed by Dave Jiang
Browse files

cxl/region: Consolidate cxl_decoder_kill_region() and cxl_region_detach() 



Both detach_target() and cxld_unregister() want to tear down a cxl_region
when an endpoint decoder is either detached or destroyed.

When a region is to be destroyed cxl_region_detach() releases
cxl_region_rwsem unbinds the cxl_region driver and re-acquires the rwsem.

This "reverse" locking pattern is difficult to reason about, not amenable
to scope-based cleanup, and the minor differences in the calling context of
detach_target() and cxld_unregister() currently results in the
cxl_decoder_kill_region() wrapper.

Introduce cxl_decoder_detach() to wrap a core __cxl_decoder_detach() that
serves both cases. I.e. either detaching a known position in a region
(interruptible), or detaching an endpoint decoder if it is found to be a
member of a region (uninterruptible).

Cc: Davidlohr Bueso <dave@stgolabs.net>
Cc: Jonathan Cameron <jonathan.cameron@huawei.com>
Cc: Dave Jiang <dave.jiang@intel.com>
Cc: Alison Schofield <alison.schofield@intel.com>
Cc: Vishal Verma <vishal.l.verma@intel.com>
Cc: Ira Weiny <ira.weiny@intel.com>
Acked-by: default avatarPeter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: default avatarDan Williams <dan.j.williams@intel.com>
Reviewed-by: default avatarFabio M. De Francesco <fabio.m.de.francesco@linux.intel.com>
Reviewed-by: default avatarDave Jiang <dave.jiang@intel.com>
Reviewed-by: default avatarJonathan Cameron <jonathan.cameron@huawei.com>
Reviewed-by: default avatarAlison Schofield <alison.schofield@intel.com>
Reviewed-by: default avatarDavidlohr Bueso <dave@stgolabs.net>
Link: https://patch.msgid.link/20250711234932.671292-8-dan.j.williams@intel.com


Signed-off-by: default avatarDave Jiang <dave.jiang@intel.com>
parent 695d9455

drivers/cxl/core/core.h

+13 −2
Original line number Diff line number Diff line
@@ -12,6 +12,11 @@ extern const struct device_type cxl_pmu_type; 


extern struct attribute_group cxl_base_attribute_group;



enum cxl_detach_mode {

	DETACH_ONLY,

	DETACH_INVALIDATE,

};



#ifdef CONFIG_CXL_REGION

extern struct device_attribute dev_attr_create_pmem_region;

extern struct device_attribute dev_attr_create_ram_region;
@@ -20,7 +25,11 @@ extern struct device_attribute dev_attr_region; 
extern const struct device_type cxl_pmem_region_type;

extern const struct device_type cxl_dax_region_type;

extern const struct device_type cxl_region_type;

void cxl_decoder_kill_region(struct cxl_endpoint_decoder *cxled);



int cxl_decoder_detach(struct cxl_region *cxlr,

		       struct cxl_endpoint_decoder *cxled, int pos,

		       enum cxl_detach_mode mode);



#define CXL_REGION_ATTR(x) (&dev_attr_##x.attr)

#define CXL_REGION_TYPE(x) (&cxl_region_type)

#define SET_CXL_REGION_ATTR(x) (&dev_attr_##x.attr),
@@ -48,7 +57,9 @@ static inline int cxl_get_poison_by_endpoint(struct cxl_port *port) 
{

	return 0;

}

static inline void cxl_decoder_kill_region(struct cxl_endpoint_decoder *cxled)

static inline int cxl_decoder_detach(struct cxl_region *cxlr,

				     struct cxl_endpoint_decoder *cxled,

				     int pos, enum cxl_detach_mode mode)

{

}

static inline int cxl_region_init(void)

drivers/cxl/core/port.c

+3 −6
Original line number Diff line number Diff line
@@ -2001,12 +2001,9 @@ EXPORT_SYMBOL_NS_GPL(cxl_decoder_add, "CXL"); 


static void cxld_unregister(void *dev)

{

	struct cxl_endpoint_decoder *cxled;



	if (is_endpoint_decoder(dev)) {

		cxled = to_cxl_endpoint_decoder(dev);

		cxl_decoder_kill_region(cxled);

	}

	if (is_endpoint_decoder(dev))

		cxl_decoder_detach(NULL, to_cxl_endpoint_decoder(dev), -1,

				   DETACH_INVALIDATE);



	device_unregister(dev);

}

drivers/cxl/core/region.c

+59 −44
Original line number Diff line number Diff line
@@ -2135,27 +2135,43 @@ static int cxl_region_attach(struct cxl_region *cxlr, 
	return 0;

}



static int cxl_region_detach(struct cxl_endpoint_decoder *cxled)

static struct cxl_region *

__cxl_decoder_detach(struct cxl_region *cxlr,

		     struct cxl_endpoint_decoder *cxled, int pos,

		     enum cxl_detach_mode mode)

{

	struct cxl_port *iter, *ep_port = cxled_to_port(cxled);

	struct cxl_region *cxlr = cxled->cxld.region;

	struct cxl_region_params *p;

	int rc = 0;



	lockdep_assert_held_write(&cxl_region_rwsem);



	if (!cxlr)

		return 0;

	if (!cxled) {

		p = &cxlr->params;



		if (pos >= p->interleave_ways) {

			dev_dbg(&cxlr->dev, "position %d out of range %d\n",

				pos, p->interleave_ways);

			return ERR_PTR(-ENXIO);

		}



		if (!p->targets[pos])

			return NULL;

		cxled = p->targets[pos];

	} else {

		cxlr = cxled->cxld.region;

		if (!cxlr)

			return NULL;

		p = &cxlr->params;

	get_device(&cxlr->dev);

	}



	if (mode == DETACH_INVALIDATE)

		cxled->part = -1;



	if (p->state > CXL_CONFIG_ACTIVE) {

		cxl_region_decode_reset(cxlr, p->interleave_ways);

		p->state = CXL_CONFIG_ACTIVE;

	}



	for (iter = ep_port; !is_cxl_root(iter);

	for (struct cxl_port *iter = cxled_to_port(cxled); !is_cxl_root(iter);

	     iter = to_cxl_port(iter->dev.parent))

		cxl_port_detach_region(iter, cxlr, cxled);
@@ -2166,7 +2182,7 @@ static int cxl_region_detach(struct cxl_endpoint_decoder *cxled) 
		dev_WARN_ONCE(&cxlr->dev, 1, "expected %s:%s at position %d\n",

			      dev_name(&cxlmd->dev), dev_name(&cxled->cxld.dev),

			      cxled->pos);

		goto out;

		return NULL;

	}



	if (p->state == CXL_CONFIG_ACTIVE) {
@@ -2180,21 +2196,42 @@ static int cxl_region_detach(struct cxl_endpoint_decoder *cxled) 
		.end = -1,

	};



	/* notify the region driver that one of its targets has departed */

	up_write(&cxl_region_rwsem);

	device_release_driver(&cxlr->dev);

	down_write(&cxl_region_rwsem);

out:

	put_device(&cxlr->dev);

	return rc;

	get_device(&cxlr->dev);

	return cxlr;

}



void cxl_decoder_kill_region(struct cxl_endpoint_decoder *cxled)

/*

 * Cleanup a decoder's interest in a region. There are 2 cases to

 * handle, removing an unknown @cxled from a known position in a region

 * (detach_target()) or removing a known @cxled from an unknown @cxlr

 * (cxld_unregister())

 *

 * When the detachment finds a region release the region driver.

 */

int cxl_decoder_detach(struct cxl_region *cxlr,

		       struct cxl_endpoint_decoder *cxled, int pos,

		       enum cxl_detach_mode mode)

{

	struct cxl_region *detach;



	/* when the decoder is being destroyed lock unconditionally */

	if (mode == DETACH_INVALIDATE)

		down_write(&cxl_region_rwsem);

	cxled->part = -1;

	cxl_region_detach(cxled);

	else {

		int rc = down_write_killable(&cxl_region_rwsem);



		if (rc)

			return rc;

	}



	detach = __cxl_decoder_detach(cxlr, cxled, pos, mode);

	up_write(&cxl_region_rwsem);



	if (detach) {

		device_release_driver(&detach->dev);

		put_device(&detach->dev);

	}

	return 0;

}



static int attach_target(struct cxl_region *cxlr,
@@ -2225,29 +2262,7 @@ static int attach_target(struct cxl_region *cxlr, 


static int detach_target(struct cxl_region *cxlr, int pos)

{

	struct cxl_region_params *p = &cxlr->params;

	int rc;



	rc = down_write_killable(&cxl_region_rwsem);

	if (rc)

		return rc;



	if (pos >= p->interleave_ways) {

		dev_dbg(&cxlr->dev, "position %d out of range %d\n", pos,

			p->interleave_ways);

		rc = -ENXIO;

		goto out;

	}



	if (!p->targets[pos]) {

		rc = 0;

		goto out;

	}



	rc = cxl_region_detach(p->targets[pos]);

out:

	up_write(&cxl_region_rwsem);

	return rc;

	return cxl_decoder_detach(cxlr, NULL, pos, DETACH_ONLY);

}



static size_t store_targetN(struct cxl_region *cxlr, const char *buf, int pos,