public inbox for linux-cxl@vger.kernel.org
 help / color / mirror / Atom feed
From: Dan Williams <djbw@kernel.org>
To: Dave Jiang <dave.jiang@intel.com>
Cc: driver-core@lists.linux.dev,  linux-kernel@vger.kernel.org,
	 linux-cxl@vger.kernel.org,
	 Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	 "Rafael J. Wysocki" <rafael@kernel.org>,
	 Ira Weiny <ira.weiny@intel.com>,
	 Bjorn Helgaas <bhelgaas@google.com>,
	 Danilo Krummrich <dakr@kernel.org>,
	 Davidlohr Bueso <dave@stgolabs.net>,
	 Jonathan Cameron <jonathan.cameron@huawei.com>,
	 Vishal Verma <vishal.l.verma@intel.com>,
	 Ben Cheatham <benjamin.cheatham@amd.com>
Subject: Re: [PATCH v2 2/4] cxl/memdev: Hold memdev lock during memdev poison injection/clear
Date: Sat, 11 Apr 2026 12:23:18 -0700	[thread overview]
Message-ID: <69da9fa696d0f_6c31a100e8@djbw-dev.notmuch> (raw)
In-Reply-To: <dfc0979a-b01a-4c90-8bc8-7a60fb795dc7@intel.com>

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@intel.com>
> >>> Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> >>> Reviewed-by: Alison Schofield <alison.schofield@intel.com>
> >>> Signed-off-by: Li Ming <ming.li@zohomail.com>
> >> 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@kernel.org>

  parent reply	other threads:[~2026-04-11 19:23 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-14  7:06 [PATCH v2 0/4] cxl: Consolidate cxlmd->endpoint accessing Li Ming
2026-03-14  7:06 ` [PATCH v2 1/4] driver core: Add conditional guard support for device_lock() Li Ming
2026-03-14  7:06 ` [PATCH v2 2/4] cxl/memdev: Hold memdev lock during memdev poison injection/clear Li Ming
2026-03-17 15:00   ` Dave Jiang
2026-03-18 11:59     ` Li Ming
2026-03-18 15:36       ` Dave Jiang
2026-03-19 11:10         ` Li Ming
2026-04-11 19:23         ` Dan Williams [this message]
2026-03-14  7:06 ` [PATCH v2 3/4] cxl/pci: Hold memdev lock in cxl_event_trace_record() Li Ming
2026-03-14  7:06 ` [PATCH v2 4/4] cxl/pci: Check memdev driver binding status in cxl_reset_done() Li Ming
2026-03-16 17:57 ` [PATCH v2 0/4] cxl: Consolidate cxlmd->endpoint accessing Dave Jiang

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=69da9fa696d0f_6c31a100e8@djbw-dev.notmuch \
    --to=djbw@kernel.org \
    --cc=benjamin.cheatham@amd.com \
    --cc=bhelgaas@google.com \
    --cc=dakr@kernel.org \
    --cc=dave.jiang@intel.com \
    --cc=dave@stgolabs.net \
    --cc=driver-core@lists.linux.dev \
    --cc=gregkh@linuxfoundation.org \
    --cc=ira.weiny@intel.com \
    --cc=jonathan.cameron@huawei.com \
    --cc=linux-cxl@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rafael@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