From: Dan Williams <dan.j.williams@intel.com>
To: "Verma, Vishal L" <vishal.l.verma@intel.com>
Cc: "Widawsky, Ben" <ben.widawsky@intel.com>,
"linux-cxl@vger.kernel.org" <linux-cxl@vger.kernel.org>,
"nvdimm@lists.linux.dev" <nvdimm@lists.linux.dev>,
"patches@lists.linux.dev" <patches@lists.linux.dev>,
"Schofield, Alison" <alison.schofield@intel.com>,
"Jonathan.Cameron@huawei.com" <Jonathan.Cameron@huawei.com>,
"Weiny, Ira" <ira.weiny@intel.com>
Subject: Re: [RFC PATCH 12/15] cxl/region: Add region creation ABI
Date: Wed, 4 May 2022 22:17:49 -0700 [thread overview]
Message-ID: <CAPcyv4jM_N0Sz_MpFC9+tr01ysJ16EwkSsbxOXxCM5aC7FSe3w@mail.gmail.com> (raw)
In-Reply-To: <f9fa0a306b167db2a91021aff70bcdbc8c154391.camel@intel.com>
On Wed, May 4, 2022 at 3:57 PM Verma, Vishal L <vishal.l.verma@intel.com> wrote:
>
> On Wed, 2022-04-13 at 11:37 -0700, Ben Widawsky wrote:
> > Regions are created as a child of the decoder that encompasses an
> > address space with constraints. Regions have a number of attributes that
> > must be configured before the region can be activated.
> >
> > Multiple processes which are trying not to race with each other
> > shouldn't need special userspace synchronization to do so.
> >
> > // Allocate a new region name
> > region=$(cat /sys/bus/cxl/devices/decoder0.0/create_pmem_region)
> >
> > // Create a new region by name
> > while
> > region=$(cat /sys/bus/cxl/devices/decoder0.0/create_pmem_region)
> > ! echo $region > /sys/bus/cxl/devices/decoder0.0/create_pmem_region
> > do true; done
> >
> > // Region now exists in sysfs
> > stat -t /sys/bus/cxl/devices/decoder0.0/$region
> >
> > // Delete the region, and name
> > echo $region > /sys/bus/cxl/devices/decoder0.0/delete_region
>
> I noticed a slight ABI inconsistency here while working on the libcxl
> side of this - see below.
>
> > +
> > +static ssize_t create_pmem_region_show(struct device *dev,
> > + struct device_attribute *attr, char *buf)
> > +{
> > + struct cxl_decoder *cxld = to_cxl_decoder(dev);
> > + struct cxl_root_decoder *cxlrd = to_cxl_root_decoder(cxld);
> > + size_t rc;
> > +
> > + /*
> > + * There's no point in returning known bad answers when the lock is held
> > + * on the store side, even though the answer given here may be
> > + * immediately invalidated as soon as the lock is dropped it's still
> > + * useful to throttle readers in the presence of writers.
> > + */
> > + rc = mutex_lock_interruptible(&cxlrd->id_lock);
> > + if (rc)
> > + return rc;
> > + rc = sysfs_emit(buf, "%d\n", cxlrd->next_region_id);
>
> This emits a numeric region ID, e.g. "0", where as
>
> > + mutex_unlock(&cxlrd->id_lock);
> > +
> > + return rc;
> > +}
> > +
>
> <snip>
>
> > +static ssize_t delete_region_store(struct device *dev,
> > + struct device_attribute *attr,
> > + const char *buf, size_t len)
> > +{
> > + struct cxl_port *port = to_cxl_port(dev->parent);
> > + struct cxl_decoder *cxld = to_cxl_decoder(dev);
> > + struct cxl_region *cxlr;
> > +
> > + cxlr = cxl_find_region_by_name(cxld, buf);
>
> This expects a full region name string e.g. "region0"
>
> Was this intentional? I don't think it's a huge deal, the library can
> certainly deal with it if needed - but would it be better to have the
> ABI symmetrical between create and delete?
Yes, makes sense to me.
next prev parent reply other threads:[~2022-05-05 5:18 UTC|newest]
Thread overview: 53+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-04-13 18:37 [RFC PATCH 00/15] Region driver Ben Widawsky
2022-04-13 18:37 ` [RFC PATCH 01/15] cxl/core: Use is_endpoint_decoder Ben Widawsky
2022-04-13 21:22 ` Dan Williams
[not found] ` <CGME20220415205052uscas1p209e03abf95b9c80b2ba1f287c82dfd80@uscas1p2.samsung.com>
2022-04-15 20:50 ` Adam Manzanares
2022-04-13 18:37 ` [RFC PATCH 02/15] cxl/core/hdm: Bail on endpoint init fail Ben Widawsky
2022-04-13 21:31 ` Dan Williams
[not found] ` <CGME20220418163713uscas1p17b3b1b45c7d27e54e3ecb62eb8af2469@uscas1p1.samsung.com>
2022-04-18 16:37 ` Adam Manzanares
2022-05-12 15:50 ` Ben Widawsky
2022-05-12 17:27 ` Luis Chamberlain
2022-05-13 12:09 ` Jonathan Cameron
2022-05-13 15:03 ` Dan Williams
2022-05-13 15:12 ` Luis Chamberlain
2022-05-13 19:14 ` Dan Williams
2022-05-13 19:31 ` Luis Chamberlain
2022-05-19 5:09 ` Dan Williams
2022-04-13 18:37 ` [RFC PATCH 03/15] Revert "cxl/core: Convert decoder range to resource" Ben Widawsky
2022-04-13 21:43 ` Dan Williams
2022-05-12 16:09 ` Ben Widawsky
2022-04-13 18:37 ` [RFC PATCH 04/15] cxl/core: Create distinct decoder structs Ben Widawsky
2022-04-15 1:45 ` Dan Williams
2022-04-18 20:43 ` Dan Williams
2022-04-13 18:37 ` [RFC PATCH 05/15] cxl/acpi: Reserve CXL resources from request_free_mem_region Ben Widawsky
2022-04-18 16:42 ` Dan Williams
2022-04-19 16:43 ` Jason Gunthorpe
2022-04-19 21:50 ` Dan Williams
2022-04-19 21:59 ` Dan Williams
2022-04-19 23:04 ` Jason Gunthorpe
2022-04-20 0:47 ` Dan Williams
2022-04-20 14:34 ` Jason Gunthorpe
2022-04-20 15:32 ` Dan Williams
2022-04-13 18:37 ` [RFC PATCH 06/15] cxl/acpi: Manage root decoder's address space Ben Widawsky
2022-04-18 22:15 ` Dan Williams
2022-05-12 19:18 ` Ben Widawsky
2022-04-13 18:37 ` [RFC PATCH 07/15] cxl/port: Surface ram and pmem resources Ben Widawsky
2022-04-13 18:37 ` [RFC PATCH 08/15] cxl/core/hdm: Allocate resources from the media Ben Widawsky
2022-04-13 18:37 ` [RFC PATCH 09/15] cxl/core/port: Add attrs for size and volatility Ben Widawsky
2022-04-13 18:37 ` [RFC PATCH 10/15] cxl/core: Extract IW/IG decoding Ben Widawsky
2022-04-13 18:37 ` [RFC PATCH 11/15] cxl/acpi: Use common " Ben Widawsky
2022-04-13 18:37 ` [RFC PATCH 12/15] cxl/region: Add region creation ABI Ben Widawsky
2022-05-04 22:56 ` Verma, Vishal L
2022-05-05 5:17 ` Dan Williams [this message]
2022-05-12 15:54 ` Ben Widawsky
2022-04-13 18:37 ` [RFC PATCH 13/15] cxl/core/port: Add attrs for root ways & granularity Ben Widawsky
2022-04-13 18:37 ` [RFC PATCH 14/15] cxl/region: Introduce configuration Ben Widawsky
2022-04-13 18:37 ` [RFC PATCH 15/15] cxl/region: Introduce a cxl_region driver Ben Widawsky
2022-05-20 16:23 ` [RFC PATCH 00/15] Region driver Jonathan Cameron
2022-05-20 16:41 ` Dan Williams
2022-05-31 12:21 ` Jonathan Cameron
2022-06-23 5:40 ` Dan Williams
2022-06-23 15:08 ` Jonathan Cameron
2022-06-23 17:33 ` Dan Williams
2022-06-23 23:44 ` Dan Williams
2022-06-24 9:08 ` Jonathan Cameron
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=CAPcyv4jM_N0Sz_MpFC9+tr01ysJ16EwkSsbxOXxCM5aC7FSe3w@mail.gmail.com \
--to=dan.j.williams@intel.com \
--cc=Jonathan.Cameron@huawei.com \
--cc=alison.schofield@intel.com \
--cc=ben.widawsky@intel.com \
--cc=ira.weiny@intel.com \
--cc=linux-cxl@vger.kernel.org \
--cc=nvdimm@lists.linux.dev \
--cc=patches@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;
as well as URLs for NNTP newsgroup(s).