Commit 86c8ea0f authored by Dan Williams's avatar Dan Williams
Browse files

cxl/core/port: Use dedicated lock for decoder target list 



Lockdep reports:

 ======================================================
 WARNING: possible circular locking dependency detected
 5.16.0-rc1+ #142 Tainted: G           OE
 ------------------------------------------------------
 cxl/1220 is trying to acquire lock:
 ffff979b85475460 (kn->active#144){++++}-{0:0}, at: __kernfs_remove+0x1ab/0x1e0

 but task is already holding lock:
 ffff979b87ab38e8 (&dev->lockdep_mutex#2/4){+.+.}-{3:3}, at: cxl_remove_ep+0x50c/0x5c0 [cxl_core]

...where cxl_remove_ep() is a helper that wants to delete ports while
holding a lock on the host device for that port. That sets up a lockdep
violation whereby target_list_show() can not rely holding the decoder's
device lock while walking the target_list. Switch to a dedicated seqlock
for this purpose.

Reported-by: default avatarBen Widawsky <ben.widawsky@intel.com>
Reviewed-by: default avatarJonathan Cameron <Jonathan.Cameron@huawei.com>
Link: https://lore.kernel.org/r/164367209095.208169.1171673319121271280.stgit@dwillia2-desk3.amr.corp.intel.com


Signed-off-by: default avatarDan Williams <dan.j.williams@intel.com>
parent 3c5b9039

drivers/cxl/core/port.c

+23 −7
Original line number Diff line number Diff line
@@ -104,14 +104,11 @@ static ssize_t target_type_show(struct device *dev, 
}

static DEVICE_ATTR_RO(target_type);



static ssize_t target_list_show(struct device *dev,

			       struct device_attribute *attr, char *buf)

static ssize_t emit_target_list(struct cxl_decoder *cxld, char *buf)

{

	struct cxl_decoder *cxld = to_cxl_decoder(dev);

	ssize_t offset = 0;

	int i, rc = 0;



	cxl_device_lock(dev);

	for (i = 0; i < cxld->interleave_ways; i++) {

		struct cxl_dport *dport = cxld->target[i];

		struct cxl_dport *next = NULL;
@@ -124,13 +121,29 @@ static ssize_t target_list_show(struct device *dev, 
		rc = sysfs_emit_at(buf, offset, "%d%s", dport->port_id,

				   next ? "," : "");

		if (rc < 0)

			break;

			return rc;

		offset += rc;

	}

	cxl_device_unlock(dev);



	return offset;

}



static ssize_t target_list_show(struct device *dev,

				struct device_attribute *attr, char *buf)

{

	struct cxl_decoder *cxld = to_cxl_decoder(dev);

	ssize_t offset;

	unsigned int seq;

	int rc;



	do {

		seq = read_seqbegin(&cxld->target_lock);

		rc = emit_target_list(cxld, buf);

	} while (read_seqretry(&cxld->target_lock, seq));



	if (rc < 0)

		return rc;

	offset = rc;



	rc = sysfs_emit_at(buf, offset, "\n");

	if (rc < 0)
@@ -494,15 +507,17 @@ static int decoder_populate_targets(struct cxl_decoder *cxld, 
		goto out_unlock;

	}



	write_seqlock(&cxld->target_lock);

	for (i = 0; i < cxld->nr_targets; i++) {

		struct cxl_dport *dport = find_dport(port, target_map[i]);



		if (!dport) {

			rc = -ENXIO;

			goto out_unlock;

			break;

		}

		cxld->target[i] = dport;

	}

	write_sequnlock(&cxld->target_lock);



out_unlock:

	cxl_device_unlock(&port->dev);
@@ -543,6 +558,7 @@ static struct cxl_decoder *cxl_decoder_alloc(struct cxl_port *port, 


	cxld->id = rc;

	cxld->nr_targets = nr_targets;

	seqlock_init(&cxld->target_lock);

	dev = &cxld->dev;

	device_initialize(dev);

	device_set_pm_not_required(dev);

drivers/cxl/cxl.h

+2 −0
Original line number Diff line number Diff line
@@ -185,6 +185,7 @@ enum cxl_decoder_type { 
 * @interleave_granularity: data stride per dport

 * @target_type: accelerator vs expander (type2 vs type3) selector

 * @flags: memory type capabilities and locking

 * @target_lock: coordinate coherent reads of the target list

 * @nr_targets: number of elements in @target

 * @target: active ordered target list in current decoder configuration

 */
@@ -199,6 +200,7 @@ struct cxl_decoder { 
	int interleave_granularity;

	enum cxl_decoder_type target_type;

	unsigned long flags;

	seqlock_t target_lock;

	int nr_targets;

	struct cxl_dport *target[];

};