Linux CXL
 help / color / mirror / Atom feed
From: Alison Schofield <alison.schofield@intel.com>
To: "Verma, Vishal L" <vishal.l.verma@intel.com>
Cc: "Jiang, Dave" <dave.jiang@intel.com>,
	"linux-cxl@vger.kernel.org" <linux-cxl@vger.kernel.org>,
	"nvdimm@lists.linux.dev" <nvdimm@lists.linux.dev>
Subject: Re: [NDCTL PATCH v2] cxl: Add helper function to verify port is in memdev hierarchy
Date: Tue, 15 Jul 2025 11:15:39 -0700	[thread overview]
Message-ID: <aHaaywJPRTteMWZf@aschofie-mobl2.lan> (raw)
In-Reply-To: <4da519268938070b448f56d55535f0e3ea4585b0.camel@intel.com>

On Mon, Jul 14, 2025 at 03:27:23PM -0700, Vishal Verma wrote:
> On Fri, 2025-07-11 at 15:33 -0700, Dave Jiang wrote:
> > 'cxl enable-port -m' uses cxl_port_get_dport_by_memdev() to find the
> > memdevs that are associated with a port in order to enable those
> > associated memdevs. When the kernel switch to delayed dport
> > initialization by enumerating the dports during memdev probe, the
> > dports are no longer valid until the memdev is probed. This means
> > that cxl_port_get_dport_by_memdev() will not find any memdevs under
> > the port.
> > 
> > Add a new helper function cxl_port_is_memdev_hierarchy() that checks if a
> 
> Stale commit message - since the actual helper is called
> cxl_memdev_is_port_ancestor() ?

Sure, I can fix that up the commit log when applying.

Vishal - Can you comment on how to handle the existing library call that
now behaves differently? Here we have a comment to add to the docs. I'm
not clear that is crisp enough. Prior to this patch it would only return
NULL if dport not found. Now it returns NULL if dport not found, or memdev
not bound.

For cxl-cli - we never used the dport, so the new call suffices. If 'other'
library users, were using it similarly, then the suggestion is to switch
to the new call. But that usage is not a given - maybe they wanted the
actual dport.

So - asking for your library rules expertise on the options here.

> 
> > port is in the memdev hierarchy via the memdev->host_path where the sysfs
> > path contains all the devices in the hierarchy. This call is also backward
> > compatible with the old behavior.

I guess it's backward compatible, if user was only checking for
existense:

	if (cxl_port_get_dport_by_memdev())
		then do this

but not if they were doing this: dport = cxl_port_get_dport_by_memdev();

BTW - I'm also OK with fixing this up for cxl-cli by adding and using
the new helper, and then coming around with another patch dealing 
with the existing library call for our 'other' library users.

-- Alison


> > 
> > Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> > ---
> > v2:
> > - Remove usages of cxl_port_get_dport_by_memdev() and add documentation to explain
> >   when cxl_port_get_dport_by_memdev() should be used. (Alison)
> > ---
> >  Documentation/cxl/lib/libcxl.txt |  5 +++++
> >  cxl/filter.c                     |  2 +-
> >  cxl/lib/libcxl.c                 | 31 +++++++++++++++++++++++++++++++
> >  cxl/lib/libcxl.sym               |  5 +++++
> >  cxl/libcxl.h                     |  3 +++
> >  cxl/port.c                       |  4 ++--
> >  6 files changed, 47 insertions(+), 3 deletions(-)

  parent reply	other threads:[~2025-07-15 18:15 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-07-11 22:33 [NDCTL PATCH v2] cxl: Add helper function to verify port is in memdev hierarchy Dave Jiang
2025-07-14 22:27 ` Verma, Vishal L
2025-07-14 22:46   ` Dave Jiang
2025-07-15 18:15   ` Alison Schofield [this message]
2025-08-27  1:06 ` 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=aHaaywJPRTteMWZf@aschofie-mobl2.lan \
    --to=alison.schofield@intel.com \
    --cc=dave.jiang@intel.com \
    --cc=linux-cxl@vger.kernel.org \
    --cc=nvdimm@lists.linux.dev \
    --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