From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id CE50CC05027 for ; Mon, 6 Feb 2023 16:18:17 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 029346B0072; Mon, 6 Feb 2023 11:18:17 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id F1C046B0073; Mon, 6 Feb 2023 11:18:16 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id DE35E6B0074; Mon, 6 Feb 2023 11:18:16 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id CC0226B0072 for ; Mon, 6 Feb 2023 11:18:16 -0500 (EST) Received: from smtpin17.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id A11E31A0894 for ; Mon, 6 Feb 2023 16:18:16 +0000 (UTC) X-FDA: 80437374192.17.01961E7 Received: from frasgout.his.huawei.com (frasgout.his.huawei.com [185.176.79.56]) by imf09.hostedemail.com (Postfix) with ESMTP id D1A0914001F for ; Mon, 6 Feb 2023 16:18:13 +0000 (UTC) Authentication-Results: imf09.hostedemail.com; dkim=none; spf=pass (imf09.hostedemail.com: domain of jonathan.cameron@huawei.com designates 185.176.79.56 as permitted sender) smtp.mailfrom=jonathan.cameron@huawei.com; dmarc=pass (policy=quarantine) header.from=huawei.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1675700294; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=haDk7Cx6ITkyClkzr0DFqfbqFtbxajLCZFJbRfXqKxs=; b=CgsN2oWCLa/taGjjEXRBLvuV9H+iA2n5qn/7hDsUSzpuZ0phiLbCH+C7F1IKSd7/IPvQkN Jh4gBSWeTOfyxZKjJC/CpqSgfwiYhhcndM5Fszi4cU2IHTrjijjJEmwXF7zkIttMZrfsrE cRW4W8BRSvKOz1cxG9yxYeBmZJzGEUE= ARC-Authentication-Results: i=1; imf09.hostedemail.com; dkim=none; spf=pass (imf09.hostedemail.com: domain of jonathan.cameron@huawei.com designates 185.176.79.56 as permitted sender) smtp.mailfrom=jonathan.cameron@huawei.com; dmarc=pass (policy=quarantine) header.from=huawei.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1675700294; a=rsa-sha256; cv=none; b=yl3d7c7Q8oXCnM0MkngZggU4uzCrgg01L2u0KeEygB97R/3FSInQ8q0fpcLSdlVBGqluQT G/sIgIDm63IGkHK/gmNcsVD37i/lvkjs03mCi57D9/ikVIHheDhdtTJX0rGk7r8tTqA2W2 CpQsv7pd8leOH5pC0gPzGavc/BYa4KE= Received: from lhrpeml500005.china.huawei.com (unknown [172.18.147.200]) by frasgout.his.huawei.com (SkyGuard) with ESMTP id 4P9WXS4wm7z6J7fH; Tue, 7 Feb 2023 00:13:44 +0800 (CST) Received: from localhost (10.81.207.58) by lhrpeml500005.china.huawei.com (7.191.163.240) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.17; Mon, 6 Feb 2023 16:18:07 +0000 Date: Mon, 6 Feb 2023 16:18:04 +0000 From: Jonathan Cameron To: Dan Williams CC: , , , Subject: Re: [PATCH 05/18] cxl/region: Add volatile region creation support Message-ID: <20230206161804.00005c47@Huawei.com> In-Reply-To: <167564537678.847146.4066579806086171091.stgit@dwillia2-xfh.jf.intel.com> References: <167564534874.847146.5222419648551436750.stgit@dwillia2-xfh.jf.intel.com> <167564537678.847146.4066579806086171091.stgit@dwillia2-xfh.jf.intel.com> Organization: Huawei Technologies Research and Development (UK) Ltd. X-Mailer: Claws Mail 4.1.0 (GTK 3.24.33; x86_64-w64-mingw32) MIME-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" Content-Transfer-Encoding: 7bit X-Originating-IP: [10.81.207.58] X-ClientProxiedBy: lhrpeml500005.china.huawei.com (7.191.163.240) To lhrpeml500005.china.huawei.com (7.191.163.240) X-CFilter-Loop: Reflected X-Stat-Signature: jbajb17wuuyhtuan4o7n6kjnhjbwqhhy X-Rspam-User: X-Rspamd-Queue-Id: D1A0914001F X-Rspamd-Server: rspam06 X-HE-Tag: 1675700293-744161 X-HE-Meta: U2FsdGVkX1+f29ZZag9fFZwuK1ovhiCbHQv0zSEryJfzHYSXpGS/m3NSyyhRu1z7Kr7liUf8tgvUQfDNMkHI4Qh1Jhg7WvzBkw19VIXYfeDb7DJSIbGR/x7vfWZ8Bka1OXbF5g18TTINxbhNGO68kuxsZ4ohyXB6j71ZCYzrZh03QU1P7ZqDhcSgd07at3Dodc8ZP0Wap8PIo4qqub4BhwPkx1nbPvXYnLQurxruyJM2H3Xe5dM3r+N1fWn8u9XM2pnXm2HGUdgMuVLnmjQFMFhOqNc7iK8HBKmK4fHXVbWRwTO5CwI8myJV55Ou/UbmuonV2TTKHkOVqARnvYaaPLAqn506PA6BvtBttAQrJ4gxo562Az4t0JV9ySOm3RngrVcVoiFri1wye5JVmmF82IGPLOwyT6VgTiSkagTvTlq+Ld4IczbrK9ajc5MZ7WCtziuRIIOcBz+SI9dYdhvyRVvGJweopL1+qpTEMOzv0IzjduiUigbfeT3V6QufS1tGrC/U7TmyA2sJzGoOZGUXyh8B4hLisdQX2NuACDrWSXjAeY8NG2uzTBAUT8Tp6DbYE2Lc2KPZpbN4uH0qLHMIqdmC1wpnIqhGFkAad5vKcqU1MHb61LGxMN0Y8pxKqAyhHtrXTloUFI0N6RgJo9bWn+QwxkApNgYDS9G3hJy3SqpJhrgdE5e6E8939hofSTjWotJ8L/+e+i1DC6cfqUqRqqBS0PjUZR6/0KXrtDZzR0Dz9kTwnhWaI+U0Cm+McktbFfh3soQKYu4fNVkwEfTmE6MelucMq06OWCql6av23gvVoClaozpmKbDOa2Mb9DO06Quge8S7fgZsgMZkvy+XhWrRGrZWMSX6r8cQdRB/osE3EibKXkNPCy6CYOzHEAkASH2HMFl0/tOoqFjVKzgweuSS/MpELFgdXggvEoPMiC/FRm+n3uBo8d52ux1qFFbrMWxwcgXk5T+blhEyIQj jHXLiBxh OMRNc3vpU7D271LsgeEX0RB9a0b6P4lV6CKzwSDlcJgj9VThgiZgS/VpjADI4/a0Fo/IkYV1Uj2B9fSwrqU7odcB5JA3DjQnr0a0/eJka99ytgAkY0P3CpC1zWx1+jtZ6GxZpOyT4rZ/t5WQL8y3dDRWEVRQ2rahKTu9jCFRaFSDRIAH1+OJ1AHYgsqZfO9/+DGGnYgDDHCJ9FJmG96BZIt/1rPRPe+2/8aRQzAb/uw79BTAPr4QeAAbmVWMPQUBOdZ0GL8M/Bkd5QhDUSULdzGJrBQ5Hyyq2CdIEj9GulhDd9ZQqBNwE/EJmp1mr8FYEMip4F5URKWatcblAcvhg1WqfnT3D7K+JMvwH X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Sun, 05 Feb 2023 17:02:56 -0800 Dan Williams wrote: > Expand the region creation infrastructure to enable 'ram' > (volatile-memory) regions. The internals of create_pmem_region_store() > and create_pmem_region_show() are factored out into helpers > __create_region() and __create_region_show() for the 'ram' case to > reuse. > > Signed-off-by: Dan Williams Entirely trivial comments inline. Reviewed-by: Jonathan Cameron > --- > Documentation/ABI/testing/sysfs-bus-cxl | 22 +++++----- > drivers/cxl/core/core.h | 1 > drivers/cxl/core/port.c | 14 ++++++ > drivers/cxl/core/region.c | 71 +++++++++++++++++++++++++------ > 4 files changed, 82 insertions(+), 26 deletions(-) > > diff --git a/Documentation/ABI/testing/sysfs-bus-cxl b/Documentation/ABI/testing/sysfs-bus-cxl > index 4c4e1cbb1169..3acf2f17a73f 100644 > --- a/Documentation/ABI/testing/sysfs-bus-cxl > +++ b/Documentation/ABI/testing/sysfs-bus-cxl > @@ -285,20 +285,20 @@ Description: > interleave_granularity). > > > -What: /sys/bus/cxl/devices/decoderX.Y/create_pmem_region > -Date: May, 2022 > -KernelVersion: v6.0 > +What: /sys/bus/cxl/devices/decoderX.Y/create_{pmem,ram}_region > +Date: May, 2022, January, 2023 > +KernelVersion: v6.0 (pmem), v6.3 (ram) > Contact: linux-cxl@vger.kernel.org > Description: > (RW) Write a string in the form 'regionZ' to start the process > - of defining a new persistent memory region (interleave-set) > - within the decode range bounded by root decoder 'decoderX.Y'. > - The value written must match the current value returned from > - reading this attribute. An atomic compare exchange operation is > - done on write to assign the requested id to a region and > - allocate the region-id for the next creation attempt. EBUSY is > - returned if the region name written does not match the current > - cached value. > + of defining a new persistent, or volatile memory region > + (interleave-set) within the decode range bounded by root decoder > + 'decoderX.Y'. The value written must match the current value > + returned from reading this attribute. An atomic compare exchange > + operation is done on write to assign the requested id to a > + region and allocate the region-id for the next creation attempt. > + EBUSY is returned if the region name written does not match the > + current cached value. > > > What: /sys/bus/cxl/devices/decoderX.Y/delete_region > diff --git a/drivers/cxl/core/core.h b/drivers/cxl/core/core.h > index 8c04672dca56..5eb873da5a30 100644 > --- a/drivers/cxl/core/core.h > +++ b/drivers/cxl/core/core.h > @@ -11,6 +11,7 @@ extern struct attribute_group cxl_base_attribute_group; > > #ifdef CONFIG_CXL_REGION > extern struct device_attribute dev_attr_create_pmem_region; > +extern struct device_attribute dev_attr_create_ram_region; > extern struct device_attribute dev_attr_delete_region; > extern struct device_attribute dev_attr_region; > extern const struct device_type cxl_pmem_region_type; > diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c > index 8566451cb22f..47e450c3a5a9 100644 > --- a/drivers/cxl/core/port.c > +++ b/drivers/cxl/core/port.c > @@ -294,6 +294,7 @@ static struct attribute *cxl_decoder_root_attrs[] = { > &dev_attr_cap_type3.attr, > &dev_attr_target_list.attr, > SET_CXL_REGION_ATTR(create_pmem_region) > + SET_CXL_REGION_ATTR(create_ram_region) > SET_CXL_REGION_ATTR(delete_region) > NULL, > }; > @@ -305,6 +306,13 @@ static bool can_create_pmem(struct cxl_root_decoder *cxlrd) > return (cxlrd->cxlsd.cxld.flags & flags) == flags; > } > > +static bool can_create_ram(struct cxl_root_decoder *cxlrd) > +{ > + unsigned long flags = CXL_DECODER_F_TYPE3 | CXL_DECODER_F_RAM; > + > + return (cxlrd->cxlsd.cxld.flags & flags) == flags; > +} > + > static umode_t cxl_root_decoder_visible(struct kobject *kobj, struct attribute *a, int n) > { > struct device *dev = kobj_to_dev(kobj); > @@ -313,7 +321,11 @@ static umode_t cxl_root_decoder_visible(struct kobject *kobj, struct attribute * > if (a == CXL_REGION_ATTR(create_pmem_region) && !can_create_pmem(cxlrd)) > return 0; > > - if (a == CXL_REGION_ATTR(delete_region) && !can_create_pmem(cxlrd)) > + if (a == CXL_REGION_ATTR(create_ram_region) && !can_create_ram(cxlrd)) > + return 0; > + > + if (a == CXL_REGION_ATTR(delete_region) && > + !(can_create_pmem(cxlrd) || can_create_ram(cxlrd))) > return 0; > > return a->mode; > diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c > index 53d6dbe4de6d..8dea49c021b8 100644 > --- a/drivers/cxl/core/region.c > +++ b/drivers/cxl/core/region.c > @@ -1685,6 +1685,15 @@ static struct cxl_region *devm_cxl_add_region(struct cxl_root_decoder *cxlrd, > struct device *dev; > int rc; > > + switch (mode) { > + case CXL_DECODER_RAM: > + case CXL_DECODER_PMEM: > + break; > + default: > + dev_err(&cxlrd->cxlsd.cxld.dev, "unsupported mode %d\n", mode); > + return ERR_PTR(-EINVAL); > + } > + > cxlr = cxl_region_alloc(cxlrd, id); > if (IS_ERR(cxlr)) > return cxlr; > @@ -1713,12 +1722,38 @@ static struct cxl_region *devm_cxl_add_region(struct cxl_root_decoder *cxlrd, > return ERR_PTR(rc); > } > > +static ssize_t __create_region_show(struct cxl_root_decoder *cxlrd, char *buf) > +{ > + return sysfs_emit(buf, "region%u\n", atomic_read(&cxlrd->region_id)); > +} > + > static ssize_t create_pmem_region_show(struct device *dev, > struct device_attribute *attr, char *buf) > { > - struct cxl_root_decoder *cxlrd = to_cxl_root_decoder(dev); > + return __create_region_show(to_cxl_root_decoder(dev), buf); > +} > > - return sysfs_emit(buf, "region%u\n", atomic_read(&cxlrd->region_id)); > +static ssize_t create_ram_region_show(struct device *dev, > + struct device_attribute *attr, char *buf) > +{ > + return __create_region_show(to_cxl_root_decoder(dev), buf); > +} > + > +static struct cxl_region *__create_region(struct cxl_root_decoder *cxlrd, > + enum cxl_decoder_mode mode, int id) > +{ > + int rc; > + > + rc = memregion_alloc(GFP_KERNEL); > + if (rc < 0) > + return ERR_PTR(rc); > + > + if (atomic_cmpxchg(&cxlrd->region_id, id, rc) != id) { > + memregion_free(rc); > + return ERR_PTR(-EBUSY); > + } > + > + return devm_cxl_add_region(cxlrd, id, mode, CXL_DECODER_EXPANDER); > } > > static ssize_t create_pmem_region_store(struct device *dev, > @@ -1727,29 +1762,37 @@ static ssize_t create_pmem_region_store(struct device *dev, > { > struct cxl_root_decoder *cxlrd = to_cxl_root_decoder(dev); > struct cxl_region *cxlr; > - int id, rc; > + int rc, id; > > rc = sscanf(buf, "region%d\n", &id); > if (rc != 1) > return -EINVAL; > > - rc = memregion_alloc(GFP_KERNEL); > - if (rc < 0) > - return rc; > + cxlr = __create_region(cxlrd, CXL_DECODER_PMEM, id); > + if (IS_ERR(cxlr)) > + return PTR_ERR(cxlr); I'd have a blank line here - see below. > + return len; > +} > +DEVICE_ATTR_RW(create_pmem_region); > > - if (atomic_cmpxchg(&cxlrd->region_id, id, rc) != id) { > - memregion_free(rc); > - return -EBUSY; > - } > +static ssize_t create_ram_region_store(struct device *dev, > + struct device_attribute *attr, > + const char *buf, size_t len) > +{ > + struct cxl_root_decoder *cxlrd = to_cxl_root_decoder(dev); > + struct cxl_region *cxlr; > + int rc, id; > > - cxlr = devm_cxl_add_region(cxlrd, id, CXL_DECODER_PMEM, > - CXL_DECODER_EXPANDER); > + rc = sscanf(buf, "region%d\n", &id); > + if (rc != 1) > + return -EINVAL; > + > + cxlr = __create_region(cxlrd, CXL_DECODER_RAM, id); > if (IS_ERR(cxlr)) > return PTR_ERR(cxlr); > - Just so you know I read to the end! Spurious unrelated white space change :) > return len; > } > -DEVICE_ATTR_RW(create_pmem_region); > +DEVICE_ATTR_RW(create_ram_region); > > static ssize_t region_show(struct device *dev, struct device_attribute *attr, > char *buf) >