From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from sender4-pp-o94.zoho.com (sender4-pp-o94.zoho.com [136.143.188.94]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 9061B27EFFA; Wed, 11 Mar 2026 11:12:39 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=pass smtp.client-ip=136.143.188.94 ARC-Seal:i=2; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1773227562; cv=pass; b=EXCJScFjGcI3kW8ur+5bQh3HXEC1gPfjiHpkuWms/e2j34x9pGMmmVuYpMM75SlqLDWiMDrcWrHzvmVogMUwA9c1mmPl8ZB1MPK6sJ+wFp0aKIwRqUuLmrNZlbedKq0+JVdv/J5BAFl8OhGVuH/YiQzkUH4U3XnSusVxMREkL7s= ARC-Message-Signature:i=2; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1773227562; c=relaxed/simple; bh=fsIHBqCYn+M8pi99JicXtB1LaF7y3U1LVegF/BYji6w=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=KrPzhjRZG7KWOYFFXT+qG45ktEVEsoM7ZoFsXllyPNhx1RMBzA4mcqU6jl1LOsTiNCpB5KcQNKp9pSxdmtdAoUihVkJCFvkzMu8cSorayqfjbc/dAIJInoNdxuXLeVjvr1HMOi8XvdpW1XLLg986W/XGSSNb3n0/C2ts1m/bF2s= ARC-Authentication-Results:i=2; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=zohomail.com; spf=pass smtp.mailfrom=zohomail.com; dkim=pass (1024-bit key) header.d=zohomail.com header.i=ming.li@zohomail.com header.b=OY6TzC4R; arc=pass smtp.client-ip=136.143.188.94 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=zohomail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=zohomail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=zohomail.com header.i=ming.li@zohomail.com header.b="OY6TzC4R" ARC-Seal: i=1; a=rsa-sha256; t=1773227464; cv=none; d=zohomail.com; s=zohoarc; b=B2B0Gqc3lTuHiEzV4qyaoLEvt00ZzC9PwmnUfkldxnQbEY0NCmFAx251f2Cv22rjRmX6EyXPDuFGqkHd66bVPxAODxjfkckcjL1a7wNePXpc4/0izqs8qVEJYRbnjmuhJF52OyaQoU3Y1mFeBzOf3hsAzGaQ3BPTwVJPfoVnBus= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1773227464; h=Content-Type:Content-Transfer-Encoding:Cc:Cc:Date:Date:From:From:In-Reply-To:MIME-Version:Message-ID:References:Subject:Subject:To:To:Message-Id:Reply-To; bh=h+7l01klGMtmp/b/OEqPqIJisiQUqGn0M6d8FgKOiSs=; b=OTOCKifauIYA6NuvaEptOZMBXqT+WRFLq2KVJx6fIfOuQP4bMeV0XokKcNk2Qqkn7aCb1+xS+RYpFi8thO7iiDTe8Cjq6v0uR8LXznbHF/OkWiyW6rzGfPGjAib+TEU2yLYEsOJ4X3qEI+uT0ha8rgV2liZ/N40PWQg/asB3ca4= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass header.i=zohomail.com; spf=pass smtp.mailfrom=ming.li@zohomail.com; dmarc=pass header.from= DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; t=1773227464; s=zm2022; d=zohomail.com; i=ming.li@zohomail.com; h=Message-ID:Date:Date:MIME-Version:Subject:Subject:To:To:Cc:Cc:References:From:From:In-Reply-To:Content-Type:Content-Transfer-Encoding:Feedback-ID:Message-Id:Reply-To; bh=h+7l01klGMtmp/b/OEqPqIJisiQUqGn0M6d8FgKOiSs=; b=OY6TzC4Rk/HDGj6da7vDKjspEgtiPte5/gWgJzvlUEkTl9wHQjA3jIH62gp7iTtP w/lQyfEIgFYf20PsYvP1gNOhIIYfcCbYfFFy1w3YA0yzJkn8LZbg+J0V/Kvo2q2r4w6 yoAHbfaJDmrA7I+4rWCqXsePNEo8WKHuB/Eq1cHc= Received: by mx.zohomail.com with SMTPS id 1773227462989250.9001677054888; Wed, 11 Mar 2026 04:11:02 -0700 (PDT) Message-ID: <13a62481-256b-405d-b2da-dd30b8cc6689@zohomail.com> Date: Wed, 11 Mar 2026 19:10:54 +0800 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH 3/7] cxl/region: Hold memdev lock during region poison injection/clear To: Alison Schofield Cc: Greg Kroah-Hartman , "Rafael J. Wysocki" , Danilo Krummrich , Davidlohr Bueso , Jonathan Cameron , Dave Jiang , Vishal Verma , Ira Weiny , Dan Williams , Bjorn Helgaas , Ben Cheatham , driver-core@lists.linux.dev, linux-kernel@vger.kernel.org, linux-cxl@vger.kernel.org References: <20260310-fix_access_endpoint_without_drv_check-v1-0-94fe919a0b87@zohomail.com> <20260310-fix_access_endpoint_without_drv_check-v1-3-94fe919a0b87@zohomail.com> From: Li Ming In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Feedback-ID: rr080112280e0625339ac0cf2da054f0f8000066d282e4621a1dd86e13b5913c3d70923b86b0491e0f1b8026ad:zu08011227950c86cf580db59af345d06300002553b5bd6d1b1adcd9d552c805175dd92a4a705e10a33a1cbf:rf0801122d722738fee4d9c64ea475bec40000f5fff0c337ed5655a9f670582ce1b6dc23b6dcf519154092097e5388015af7:ZohoMail X-ZohoMailClient: External 在 2026/3/11 05:57, Alison Schofield 写道: > On Tue, Mar 10, 2026 at 11:57:55PM +0800, Li Ming wrote: >> cxl_dpa_to_region() will require callers holding the given CXL memdev >> lock for endpoint validity checking in the following patch. To prepare >> it, region poison injection/clearing debugfs interfaces need to ensure >> the correct CXL memdev lock is held at the beginning. >> >> To keep lock sequence(cxlmd.dev -> cxl_rwsem.region -> cxl_rwsem.dpa) >> for avoiding deadlock. the interfaces have to find out the correct CXL >> memdev at first, holding lock in the sequence then checking if the DPA >> data has been changed before holding locks. > This seems complicated, that this path needs to do needless work to > comply with a new locking scheme that this path doesn't really need. > > One thought, if in Patch 2 we lock at the debugfs callsite for > by memdev poison, then here, we don't need to think about following > unnecessary locking protocol. I say that because I think locking > down w dpa and region rwsem is enough here. Then there is nothing > new to do in this path. Might that work? > > Along w that can cxl_dpa_to_region() go back to safety checking as > opposed to the new device_lock_assert? > > I haven't gone thru every case in this set, so might be way off ;) Actually, my understanding is same as yours. I think we don't need this patch either if we remove the device_lock_assert() in cxl_dpa_to_region().(Just keep the cxlmd->dev.driver checking) Because region is created only when endpoint is ready. Detaching an endpoint from a region always holds cxl_rwsem.region. But we always hold device lock for the device driver bingding checking in CXL subsystem. I'm OK to remove this patch, Let see any inputs from other reviewers. Ming > >> Suggested-by: Dan Williams >> Signed-off-by: Li Ming >> --- >> drivers/cxl/core/region.c | 101 +++++++++++++++++++++++++++++++++++----------- >> 1 file changed, 77 insertions(+), 24 deletions(-) >> >> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c >> index 42874948b589..806512dfab07 100644 >> --- a/drivers/cxl/core/region.c >> +++ b/drivers/cxl/core/region.c >> @@ -4074,12 +4074,60 @@ static int validate_region_offset(struct cxl_region *cxlr, u64 offset) >> return 0; >> } >> >> +static int cxl_region_poison_lookup_locked(struct cxl_region *cxlr, u64 offset, >> + struct dpa_result *res) >> +{ >> + int rc; >> + >> + *res = (struct dpa_result){ .dpa = ULLONG_MAX, .cxlmd = NULL }; >> + >> + if (validate_region_offset(cxlr, offset)) >> + return -EINVAL; >> + >> + offset -= cxlr->params.cache_size; >> + rc = region_offset_to_dpa_result(cxlr, offset, res); >> + if (rc || !res->cxlmd || res->dpa == ULLONG_MAX) { >> + dev_dbg(&cxlr->dev, >> + "Failed to resolve DPA for region offset %#llx rc %d\n", >> + offset, rc); >> + >> + return rc ? rc : -EINVAL; >> + } >> + >> + return 0; >> +} >> + >> +static int cxl_region_poison_lookup(struct cxl_region *cxlr, u64 offset, >> + struct dpa_result *res) >> +{ >> + int rc; >> + >> + ACQUIRE(rwsem_read_intr, region_rwsem)(&cxl_rwsem.region); >> + if ((rc = ACQUIRE_ERR(rwsem_read_intr, ®ion_rwsem))) >> + return rc; >> + >> + ACQUIRE(rwsem_read_intr, dpa_rwsem)(&cxl_rwsem.dpa); >> + if ((rc = ACQUIRE_ERR(rwsem_read_intr, &dpa_rwsem))) >> + return rc; >> + >> + return cxl_region_poison_lookup_locked(cxlr, offset, res); >> +} >> + >> static int cxl_region_debugfs_poison_inject(void *data, u64 offset) >> { >> - struct dpa_result result = { .dpa = ULLONG_MAX, .cxlmd = NULL }; >> struct cxl_region *cxlr = data; >> + struct dpa_result res1, res2; >> int rc; >> >> + /* To retrieve the correct memdev */ >> + rc = cxl_region_poison_lookup(cxlr, offset, &res1); >> + if (rc) >> + return rc; >> + >> + ACQUIRE(device_intr, devlock)(&res1.cxlmd->dev); >> + if ((rc = ACQUIRE_ERR(device_intr, &devlock))) >> + return rc; >> + >> ACQUIRE(rwsem_read_intr, region_rwsem)(&cxl_rwsem.region); >> if ((rc = ACQUIRE_ERR(rwsem_read_intr, ®ion_rwsem))) >> return rc; >> @@ -4088,20 +4136,18 @@ static int cxl_region_debugfs_poison_inject(void *data, u64 offset) >> if ((rc = ACQUIRE_ERR(rwsem_read_intr, &dpa_rwsem))) >> return rc; >> >> - if (validate_region_offset(cxlr, offset)) >> - return -EINVAL; >> - >> - offset -= cxlr->params.cache_size; >> - rc = region_offset_to_dpa_result(cxlr, offset, &result); >> - if (rc || !result.cxlmd || result.dpa == ULLONG_MAX) { >> + /* >> + * Retrieve memdev and DPA data again in case that the data >> + * has been changed before holding locks. >> + */ >> + rc = cxl_region_poison_lookup_locked(cxlr, offset, &res2); >> + if (rc || res2.cxlmd != res1.cxlmd || res2.dpa != res1.dpa) { >> dev_dbg(&cxlr->dev, >> - "Failed to resolve DPA for region offset %#llx rc %d\n", >> - offset, rc); >> - >> - return rc ? rc : -EINVAL; >> + "Error injection raced region reconfiguration: %d", rc); >> + return -ENXIO; >> } >> >> - return cxl_inject_poison_locked(result.cxlmd, result.dpa); >> + return cxl_inject_poison_locked(res2.cxlmd, res2.dpa); >> } >> >> DEFINE_DEBUGFS_ATTRIBUTE(cxl_poison_inject_fops, NULL, >> @@ -4109,10 +4155,19 @@ DEFINE_DEBUGFS_ATTRIBUTE(cxl_poison_inject_fops, NULL, >> >> static int cxl_region_debugfs_poison_clear(void *data, u64 offset) >> { >> - struct dpa_result result = { .dpa = ULLONG_MAX, .cxlmd = NULL }; >> struct cxl_region *cxlr = data; >> + struct dpa_result res1, res2; >> int rc; >> >> + /* To retrieve the correct memdev */ >> + rc = cxl_region_poison_lookup(cxlr, offset, &res1); >> + if (rc) >> + return rc; >> + >> + ACQUIRE(device_intr, devlock)(&res1.cxlmd->dev); >> + if ((rc = ACQUIRE_ERR(device_intr, &devlock))) >> + return rc; >> + >> ACQUIRE(rwsem_read_intr, region_rwsem)(&cxl_rwsem.region); >> if ((rc = ACQUIRE_ERR(rwsem_read_intr, ®ion_rwsem))) >> return rc; >> @@ -4121,20 +4176,18 @@ static int cxl_region_debugfs_poison_clear(void *data, u64 offset) >> if ((rc = ACQUIRE_ERR(rwsem_read_intr, &dpa_rwsem))) >> return rc; >> >> - if (validate_region_offset(cxlr, offset)) >> - return -EINVAL; >> - >> - offset -= cxlr->params.cache_size; >> - rc = region_offset_to_dpa_result(cxlr, offset, &result); >> - if (rc || !result.cxlmd || result.dpa == ULLONG_MAX) { >> + /* >> + * Retrieve memdev and DPA data again in case that the data >> + * has been changed before holding locks. >> + */ >> + rc = cxl_region_poison_lookup_locked(cxlr, offset, &res2); >> + if (rc || res2.cxlmd != res1.cxlmd || res2.dpa != res1.dpa) { >> dev_dbg(&cxlr->dev, >> - "Failed to resolve DPA for region offset %#llx rc %d\n", >> - offset, rc); >> - >> - return rc ? rc : -EINVAL; >> + "Error clearing raced region reconfiguration: %d", rc); >> + return -ENXIO; >> } >> >> - return cxl_clear_poison_locked(result.cxlmd, result.dpa); >> + return cxl_clear_poison_locked(res2.cxlmd, res2.dpa); >> } >> >> DEFINE_DEBUGFS_ATTRIBUTE(cxl_poison_clear_fops, NULL, >> >> -- >> 2.43.0 >>