Re: [PATCH v2 2/4] cxl/memdev: Hold memdev lock during memdev poison injection/clear

[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>