Re: [PATCH v2 2/4] cxl/memdev: Hold memdev lock during memdev poison injection/clear
From: Dave Jiang
Date: Wed Apr 22 2026 - 10:54:19 EST
On 4/22/26 5:09 AM, Li Ming wrote:
>
> 在 2026/4/21 23:29, Dave Jiang 写道:
>>
>> On 4/21/26 3:48 AM, Li Ming wrote:
>>> 在 2026/4/12 03:23, Dan Williams 写道:
>>>> Dave Jiang wrote:
>>>>> On 3/18/26 4:59 AM, Li Ming wrote:
>>>>>> 在 2026/3/17 23:00, Dave Jiang 写道:
>>>>>>> On 3/14/26 12:06 AM, Li Ming wrote:
>>>>>>>> CXL memdev poison injection/clearing debugfs interfaces are visible
>>>>>>>> before the CXL memdev endpoint initialization, If user accesses the
>>>>>>>> interfaces before cxlmd->endpoint updated, it is possible to access an
>>>>>>>> invalid endpoint in cxl_dpa_to_region().
>>>>>>>>
>>>>>>>> Hold CXL memdev lock at the beginning of the interfaces, this blocks the
>>>>>>>> interfaces until CXL memdev probing completed.
>>>>>>>>
>>>>>>>> The following patch will check the given endpoint validity in
>>>>>>>> cxl_dpa_to_region().
>>>>>>>>
>>>>>>>> Suggested-by: Dan Williams <dan.j.williams@xxxxxxxxx>
>>>>>>>> Reviewed-by: Dave Jiang <dave.jiang@xxxxxxxxx>
>>>>>>>> Reviewed-by: Alison Schofield <alison.schofield@xxxxxxxxx>
>>>>>>>> Signed-off-by: Li Ming <ming.li@xxxxxxxxxxxx>
>>>>>>> Hi Ming, I dropped this patch with Dan's comments [1] and updated cxl/next. Please check and make sure everything looks ok to you. Thanks!
>>>>>>>
>>>>>>> [1]: https://lore.kernel.org/linux-cxl/69b8b81621e16_452b100e@dwillia2-mobl4.notmuch/
>>>>>> Hi Dave,
>>>>>>
>>>>>> I think there is no change needed in this patch, just need a independent patch for holding memdev lock in cxl_region_debugfs_poison_inject/clear.
>>>>>>
>>>>>> Or you want to review this patch again?
>>>>> No. Maybe I misunderstood the comment from Dan. So we should keep this patch?
>>>> tl;dr: yes, this patch is *now* valid.
>>>>
>>>> As originally structured having patch2 before patch3 added no value.
>>>> Going forward I would say do not structure a series such that a new lock
>>>> taken in patch N starts being useful in patch N+1.
>>>>
>>>> I assume that is what:
>>>>
>>>> "The following patch will check the given endpoint validity in
>>>> cxl_dpa_to_region()."
>>>>
>>>> ...is trying to say, but when commits are applied to the tree it is
>>>> difficult to see that this commit was from a series.
>>>>
>>>> So do pick this one up, but the changelog needs adjusting. Something
>>>> like:
>>>>
>>>> "cxl_dpa_to_region() assumes that it is running a context where it is not
>>>> racing changes to "cxlmd->dev.driver". Acquire the memdev device lock in
>>>> the debugfs entry points to preclude debugfs usage racing cxl_mem driver
>>>> detach."
>>>>
>>>> With that you can add:
>>>>
>>>> Reviewed-by: Dan Williams <djbw@xxxxxxxxxx>
>>> Hi Dave,
>>>
>>> Will you pick this patch with changes from Dan's comment? Or I need to send out a new version?
>> I believe it's merged into linus/master already. Can you check and see if everything looks correct?
>
> I don't see this patch in linus/master or cxl/next. I remember you dropped it after applying the patchset to cxl/next.
Oh then can you please just resubmit it with updates? That way I don't mess it up. Thanks!
>
>
> Ming
>