Re: [PATCH 3/7] cxl/region: Hold memdev lock during region poison injection/clear

[Dan Williams]

Li Ming wrote:
> 
> 在 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.

The question is can cxlmd->dev.driver be invalidated while userspace is
pending inside of a debugfs callback?

Yes, it can. Debugfs, unlike sysfs which uses kernfs_drain(), does not
flush active threads inside debugfs handlers. At least not as far as I
can see.

The next question is, are we protected by the fact that a region needs
to disconnect a decoder and it only does that under the rwsem held for
write?

I think the answer is "no".

If the protocol was to have a region in hand and then validate its
locked association to an endpoint decoder then, yes. In this case though
we have no idea if the cxlmd is associated with any region at all.  So I
think you can theoretically race injecting an error to an idle cxlmd
that is in the process of destroying its ->endpoint.

Now, maybe some other detail saves us, but I do not think it is worth
the mental gymnastics of just requiring that no driver detach
shenanigans are running concurrent with poison injection shenanigans.