From: Dave Jiang <dave.jiang@intel.com>
To: Alison Schofield <alison.schofield@intel.com>
Cc: <linux-cxl@vger.kernel.org>, <dave@stgolabs.net>,
<jonathan.cameron@huawei.com>, <vishal.l.verma@intel.com>,
<ira.weiny@intel.com>, <dan.j.williams@intel.com>
Subject: Re: [PATCH] cxl: Convert pioson ops rwsem usages to scope based resource management
Date: Wed, 15 Nov 2023 16:55:11 -0700 [thread overview]
Message-ID: <c35ce419-4881-4cb9-999e-d3034ecad8ab@intel.com> (raw)
In-Reply-To: <ZVVVH8ITotdk1nyN@aschofie-mobl2>
On 11/15/23 16:32, Alison Schofield wrote:
> On Tue, Nov 14, 2023 at 11:25:20AM -0700, Dave Jiang wrote:
>> Cleanup the rwsem usages in the poison ops to reduce complexity and reduce
>> code lines.
>>
>> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
>> ---
>>
>> Hi Alison, follow on patch to yours. Can't be backported, but nice clean
>> up going forward.
>
> Tell me more about your backport worry. Are we expected to avoid using
> the new guard any place where there is a backport possibility?
Given that there's none of the cleanup.h support in stable kernels, I don't see how we can backport the guard() code automatically. Thus your original fix with a fixes tag plus a new cleanup patch follow on w/o backport issues seems necessary. Otherwise a separate backport patch would be needed no?
>
> I'm thinking I should just rev the patch with your updates.
>
>>
>> drivers/cxl/core/memdev.c | 32 ++++++++++++--------------------
>> 1 file changed, 12 insertions(+), 20 deletions(-)
>>
>> diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
>> index 961da365b097..9ab748fadb38 100644
>> --- a/drivers/cxl/core/memdev.c
>> +++ b/drivers/cxl/core/memdev.c
>> @@ -227,8 +227,8 @@ int cxl_trigger_poison_list(struct cxl_memdev *cxlmd)
>> if (!port || !is_cxl_endpoint(port))
>> return -EINVAL;
>>
>> - down_read(&cxl_region_rwsem);
>> - down_read(&cxl_dpa_rwsem);
>> + guard(rwsem_read)(&cxl_region_rwsem);
>> + guard(rwsem_read)(&cxl_dpa_rwsem);
>>
>> if (cxl_num_decoders_committed(port) == 0) {
>> /* No regions mapped to this memdev */
>> @@ -237,8 +237,6 @@ int cxl_trigger_poison_list(struct cxl_memdev *cxlmd)
>> /* Regions mapped, collect poison by endpoint */
>> rc = cxl_get_poison_by_endpoint(port);
>> }
>> - up_read(&cxl_dpa_rwsem);
>> - up_read(&cxl_region_rwsem);
>>
>> return rc;
>> }
>> @@ -324,12 +322,12 @@ int cxl_inject_poison(struct cxl_memdev *cxlmd, u64 dpa)
>> if (!IS_ENABLED(CONFIG_DEBUG_FS))
>> return 0;
>>
>> - down_read(&cxl_region_rwsem);
>> - down_read(&cxl_dpa_rwsem);
>> + guard(rwsem_read)(&cxl_region_rwsem);
>> + guard(rwsem_read)(&cxl_dpa_rwsem);
>>
>> rc = cxl_validate_poison_dpa(cxlmd, dpa);
>> if (rc)
>> - goto out;
>> + return rc;
>>
>> inject.address = cpu_to_le64(dpa);
>> mbox_cmd = (struct cxl_mbox_cmd) {
>> @@ -339,7 +337,7 @@ int cxl_inject_poison(struct cxl_memdev *cxlmd, u64 dpa)
>> };
>> rc = cxl_internal_send_cmd(mds, &mbox_cmd);
>> if (rc)
>> - goto out;
>> + return rc;
>>
>> cxlr = cxl_dpa_to_region(cxlmd, dpa);
>> if (cxlr)
>> @@ -352,11 +350,8 @@ int cxl_inject_poison(struct cxl_memdev *cxlmd, u64 dpa)
>> .length = cpu_to_le32(1),
>> };
>> trace_cxl_poison(cxlmd, cxlr, &record, 0, 0, CXL_POISON_TRACE_INJECT);
>> -out:
>> - up_read(&cxl_dpa_rwsem);
>> - up_read(&cxl_region_rwsem);
>>
>> - return rc;
>> + return 0;
>> }
>> EXPORT_SYMBOL_NS_GPL(cxl_inject_poison, CXL);
>>
>> @@ -372,12 +367,12 @@ int cxl_clear_poison(struct cxl_memdev *cxlmd, u64 dpa)
>> if (!IS_ENABLED(CONFIG_DEBUG_FS))
>> return 0;
>>
>> - down_read(&cxl_region_rwsem);
>> - down_read(&cxl_dpa_rwsem);
>> + guard(rwsem_read)(&cxl_region_rwsem);
>> + guard(rwsem_read)(&cxl_dpa_rwsem);
>>
>> rc = cxl_validate_poison_dpa(cxlmd, dpa);
>> if (rc)
>> - goto out;
>> + return rc;
>>
>> /*
>> * In CXL 3.0 Spec 8.2.9.8.4.3, the Clear Poison mailbox command
>> @@ -396,7 +391,7 @@ int cxl_clear_poison(struct cxl_memdev *cxlmd, u64 dpa)
>>
>> rc = cxl_internal_send_cmd(mds, &mbox_cmd);
>> if (rc)
>> - goto out;
>> + return rc;
>>
>> cxlr = cxl_dpa_to_region(cxlmd, dpa);
>> if (cxlr)
>> @@ -409,11 +404,8 @@ int cxl_clear_poison(struct cxl_memdev *cxlmd, u64 dpa)
>> .length = cpu_to_le32(1),
>> };
>> trace_cxl_poison(cxlmd, cxlr, &record, 0, 0, CXL_POISON_TRACE_CLEAR);
>> -out:
>> - up_read(&cxl_dpa_rwsem);
>> - up_read(&cxl_region_rwsem);
>>
>> - return rc;
>> + return 0;
>> }
>> EXPORT_SYMBOL_NS_GPL(cxl_clear_poison, CXL);
>>
>>
>>
next prev parent reply other threads:[~2023-11-15 23:55 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-11-14 2:53 [PATCH] cxl/core: Hold the region rwsem during poison ops alison.schofield
2023-11-14 18:20 ` Dave Jiang
2023-11-14 18:25 ` [PATCH] cxl: Convert pioson ops rwsem usages to scope based resource management Dave Jiang
2023-11-15 23:32 ` Alison Schofield
2023-11-15 23:55 ` Dave Jiang [this message]
2023-11-16 2:17 ` Alison Schofield
2023-11-16 16:27 ` Dave Jiang
2023-11-23 1:12 ` Dan Williams
2023-11-23 1:33 ` Dan Williams
2023-11-16 22:14 ` [PATCH] cxl/core: Hold the region rwsem during poison ops Davidlohr Bueso
2023-11-20 19:07 ` Alison Schofield
2023-11-23 1:48 ` Dan Williams
2023-11-27 0:03 ` Alison Schofield
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=c35ce419-4881-4cb9-999e-d3034ecad8ab@intel.com \
--to=dave.jiang@intel.com \
--cc=alison.schofield@intel.com \
--cc=dan.j.williams@intel.com \
--cc=dave@stgolabs.net \
--cc=ira.weiny@intel.com \
--cc=jonathan.cameron@huawei.com \
--cc=linux-cxl@vger.kernel.org \
--cc=vishal.l.verma@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox