From: Dan Williams <dan.j.williams@intel.com>
To: Dave Jiang <dave.jiang@intel.com>, <linux-cxl@vger.kernel.org>
Cc: <dan.j.williams@intel.com>, <ira.weiny@intel.com>,
<vishal.l.verma@intel.com>, <alison.schofield@intel.com>,
<Jonathan.Cameron@huawei.com>, <dave@stgolabs.net>
Subject: RE: [PATCH] cxl: Calculate region bandwidth of targets with shared upstream link
Date: Tue, 9 Apr 2024 15:03:18 -0700 [thread overview]
Message-ID: <6615bb26a0cfb_2583ad2943a@dwillia2-xfh.jf.intel.com.notmuch> (raw)
In-Reply-To: <20240409185023.151885-1-dave.jiang@intel.com>
Dave Jiang wrote:
> For a topology where multiple targets sharing the same upstream link, the
> bandwidth must be divided amongst all the sharing targets.
> cxl_rr->num_targets keeps track of the numbers of targets sharing the same
> upstream port. The bandwidth should be divided amongst all those targets.
> Take the min of that bandwidth and the whole path bandwidth as the actual
> bandwidth for each of the target.
>
> Suggested-by: Jonathan Cameron <Jonathan.Cameron@Huawei.com>
> Link: https://lore.kernel.org/linux-cxl/20240405143242.0000363a@Huawei.com/
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
> drivers/cxl/core/cdat.c | 24 ++++++++++++++++++++++--
> drivers/cxl/core/core.h | 3 +++
> drivers/cxl/core/pci.c | 17 +++++++++++++++++
> drivers/cxl/core/region.c | 10 ++++++++++
> 4 files changed, 52 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/cxl/core/cdat.c b/drivers/cxl/core/cdat.c
> index 4b717d2f5a9d..af5c02ab49e3 100644
> --- a/drivers/cxl/core/cdat.c
> +++ b/drivers/cxl/core/cdat.c
> @@ -551,7 +551,10 @@ void cxl_region_perf_data_calculate(struct cxl_region *cxlr,
> .start = cxled->dpa_res->start,
> .end = cxled->dpa_res->end,
> };
> + struct cxl_port *port = cxlmd->endpoint;
> + struct pci_dev *pdev = to_pci_dev(port->uport_dev);
> struct cxl_dpa_perf *perf;
> + int usp_bw, targets;
>
> switch (cxlr->mode) {
> case CXL_DECODER_RAM:
> @@ -569,6 +572,19 @@ void cxl_region_perf_data_calculate(struct cxl_region *cxlr,
> if (!range_contains(&perf->dpa_range, &dpa))
> return;
>
> + usp_bw = cxl_pci_get_bandwidth(pdev);
> + if (usp_bw < 0)
> + return;
> +
> + /*
> + * Get the number of targets that share the upstream link. If there are more
> + * than 1 shared targets, the upstream port bandwidth is divided equally
> + * amongst all the targets.
> + */
> + targets = cxl_region_targets(port, cxlr);
cxl_region_targets() is too generic of a name, or otherwise feels
misnamed for a function that is answering a port relative question with
region data, not a region-relative question with port data.
Also, how does this handle all the shared links in the topology? For
example cxl_test has 2 endpoints per switch and 2 switches per
host-bridge. As far as I can see @port is the endpoint port that is
never a shared port. So I would have expected this calcuation where the
topology is walked.
> + if (!targets)
> + return;
This can only return the shared targets per port *after* all the
endpoints have been added. Looks like cxl_region_perf_data_calculate()
is called during region attach before all endpoints are known.
Maybe step back and do a different algorithm?
Note that when an endpoint is added you can see if it shares links with
a previously added and skip adding its bandwidth to the region, unless
the shared links have leftover bandwidth.
In other words, rather than try to divide the shared bandwidth just to
add it all back up again, skip adding it for shared / saturated links.
Not sure if that is acutally easier in the end, but might save you from
needing to wait until the end of region assembly to figure out the
number of shared targets per port.
prev parent reply other threads:[~2024-04-09 22:03 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-09 18:50 [PATCH] cxl: Calculate region bandwidth of targets with shared upstream link Dave Jiang
2024-04-09 22:03 ` Dan Williams [this message]
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=6615bb26a0cfb_2583ad2943a@dwillia2-xfh.jf.intel.com.notmuch \
--to=dan.j.williams@intel.com \
--cc=Jonathan.Cameron@huawei.com \
--cc=alison.schofield@intel.com \
--cc=dave.jiang@intel.com \
--cc=dave@stgolabs.net \
--cc=ira.weiny@intel.com \
--cc=linux-cxl@vger.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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).