nvdimm.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
From: Dan Williams <dan.j.williams@intel.com>
To: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: linux-cxl@vger.kernel.org, Linux NVDIMM <nvdimm@lists.linux.dev>,
	 Jonathan Cameron <Jonathan.Cameron@huawei.com>,
	Ben Widawsky <ben.widawsky@intel.com>,
	 Vishal L Verma <vishal.l.verma@intel.com>,
	"Schofield, Alison" <alison.schofield@intel.com>,
	 "Weiny, Ira" <ira.weiny@intel.com>
Subject: Re: [PATCH 10/23] libnvdimm/labels: Add uuid helpers
Date: Thu, 12 Aug 2021 15:34:59 -0700	[thread overview]
Message-ID: <CAPcyv4hY9YL7MhkeSu4GYBNo6hbeMRgqnKf8YuLuQ3khSbhn9A@mail.gmail.com> (raw)
In-Reply-To: <YRQik5OnRyYQAm4o@smile.fi.intel.com>

On Wed, Aug 11, 2021 at 12:18 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Wed, Aug 11, 2021 at 10:11:56AM -0700, Dan Williams wrote:
> > On Wed, Aug 11, 2021 at 9:59 AM Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com> wrote:
> > > On Wed, Aug 11, 2021 at 11:05:55AM +0300, Andy Shevchenko wrote:
> > > > On Mon, Aug 09, 2021 at 03:28:40PM -0700, Dan Williams wrote:
> > > > > In preparation for CXL labels that move the uuid to a different offset
> > > > > in the label, add nsl_{ref,get,validate}_uuid(). These helpers use the
> > > > > proper uuid_t type. That type definition predated the libnvdimm
> > > > > subsystem, so now is as a good a time as any to convert all the uuid
> > > > > handling in the subsystem to uuid_t to match the helpers.
> > > > >
> > > > > As for the whitespace changes, all new code is clang-format compliant.
> > > >
> > > > Thanks, looks good to me!
> > > > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > >
> > > Sorry, I'm in doubt this Rb stays. See below.

Andy, does this incremental diff restore your reviewed-by? The awkward
piece of this for me is that it introduces a handful of unnecessary
memory copies. See some of the new nsl_get_uuid() additions and the
extra copy in nsl_uuid_equal()

diff --git a/drivers/nvdimm/btt.c b/drivers/nvdimm/btt.c
index 1cdfbadb7408..52de60b7adee 100644
--- a/drivers/nvdimm/btt.c
+++ b/drivers/nvdimm/btt.c
@@ -988,8 +988,8 @@ static int btt_arena_write_layout(struct arena_info *arena)
                return -ENOMEM;

        strncpy(super->signature, BTT_SIG, BTT_SIG_LEN);
-       uuid_copy(&super->uuid, nd_btt->uuid);
-       uuid_copy(&super->parent_uuid, parent_uuid);
+       export_uuid(super->uuid, nd_btt->uuid);
+       export_uuid(super->parent_uuid, parent_uuid);
        super->flags = cpu_to_le32(arena->flags);
        super->version_major = cpu_to_le16(arena->version_major);
        super->version_minor = cpu_to_le16(arena->version_minor);
diff --git a/drivers/nvdimm/btt.h b/drivers/nvdimm/btt.h
index fc3512d92ae5..0c76c0333f6e 100644
--- a/drivers/nvdimm/btt.h
+++ b/drivers/nvdimm/btt.h
@@ -94,8 +94,8 @@ struct log_group {

 struct btt_sb {
        u8 signature[BTT_SIG_LEN];
-       uuid_t uuid;
-       uuid_t parent_uuid;
+       u8 uuid[16];
+       u8 parent_uuid[16];
        __le32 flags;
        __le16 version_major;
        __le16 version_minor;
diff --git a/drivers/nvdimm/btt_devs.c b/drivers/nvdimm/btt_devs.c
index 5ad45e9e48c9..8b52e5144f08 100644
--- a/drivers/nvdimm/btt_devs.c
+++ b/drivers/nvdimm/btt_devs.c
@@ -244,14 +244,16 @@ struct device *nd_btt_create(struct nd_region *nd_region)
  */
 bool nd_btt_arena_is_valid(struct nd_btt *nd_btt, struct btt_sb *super)
 {
-       const uuid_t *parent_uuid = nd_dev_to_uuid(&nd_btt->ndns->dev);
+       const uuid_t *ns_uuid = nd_dev_to_uuid(&nd_btt->ndns->dev);
+       uuid_t parent_uuid;
        u64 checksum;

        if (memcmp(super->signature, BTT_SIG, BTT_SIG_LEN) != 0)
                return false;

-       if (!uuid_is_null(&super->parent_uuid))
-               if (!uuid_equal(&super->parent_uuid, parent_uuid))
+       import_uuid(&parent_uuid, super->parent_uuid);
+       if (!uuid_is_null(&parent_uuid))
+               if (!uuid_equal(&parent_uuid, ns_uuid))
                        return false;

        checksum = le64_to_cpu(super->checksum);
diff --git a/drivers/nvdimm/label.c b/drivers/nvdimm/label.c
index 99608e6aeaae..a799ccbc8c05 100644
--- a/drivers/nvdimm/label.c
+++ b/drivers/nvdimm/label.c
@@ -925,7 +925,7 @@ static int __pmem_label_update(struct nd_region *nd_region,
                if (!label_ent->label)
                        continue;
                if (test_and_clear_bit(ND_LABEL_REAP, &label_ent->flags) ||
-                   uuid_equal(nspm->uuid, nsl_ref_uuid(ndd, label_ent->label)))
+                   nsl_uuid_equal(ndd, label_ent->label, nspm->uuid))
                        reap_victim(nd_mapping, label_ent);
        }

@@ -1087,7 +1087,7 @@ static int __blk_label_update(struct nd_region *nd_region,
                /* mark unused labels for garbage collection */
                for_each_clear_bit_le(slot, free, nslot) {
                        nd_label = to_label(ndd, slot);
-                       if (!nsl_validate_uuid(ndd, nd_label, nsblk->uuid))
+                       if (!nsl_uuid_equal(ndd, nd_label, nsblk->uuid))
                                continue;
                        res = to_resource(ndd, nd_label);
                        if (res && is_old_resource(res, old_res_list,
@@ -1204,7 +1204,7 @@ static int __blk_label_update(struct nd_region *nd_region,
                if (!nd_label)
                        continue;
                nlabel++;
-               if (!nsl_validate_uuid(ndd, nd_label, nsblk->uuid))
+               if (!nsl_uuid_equal(ndd, nd_label, nsblk->uuid))
                        continue;
                nlabel--;
                list_move(&label_ent->list, &list);
@@ -1234,7 +1234,7 @@ static int __blk_label_update(struct nd_region *nd_region,
        }
        for_each_clear_bit_le(slot, free, nslot) {
                nd_label = to_label(ndd, slot);
-               if (!nsl_validate_uuid(ndd, nd_label, nsblk->uuid))
+               if (!nsl_uuid_equal(ndd, nd_label, nsblk->uuid))
                        continue;
                res = to_resource(ndd, nd_label);
                res->flags &= ~DPA_RESOURCE_ADJUSTED;
@@ -1338,7 +1338,7 @@ static int del_labels(struct nd_mapping
*nd_mapping, uuid_t *uuid)
                if (!nd_label)
                        continue;
                active++;
-               if (!nsl_validate_uuid(ndd, nd_label, uuid))
+               if (!nsl_uuid_equal(ndd, nd_label, uuid))
                        continue;
                active--;
                slot = to_slot(ndd, nd_label);
diff --git a/drivers/nvdimm/label.h b/drivers/nvdimm/label.h
index e6e77691dbec..6e07771aa8f1 100644
--- a/drivers/nvdimm/label.h
+++ b/drivers/nvdimm/label.h
@@ -79,7 +79,7 @@ struct nd_namespace_index {
  * @unused: must be zero
  */
 struct nd_namespace_label {
-       uuid_t uuid;
+       u8 uuid[16];
        u8 name[NSLABEL_NAME_LEN];
        __le32 flags;
        __le16 nlabel;
diff --git a/drivers/nvdimm/namespace_devs.c b/drivers/nvdimm/namespace_devs.c
index 20ea3ccd1f29..d4959981c7d4 100644
--- a/drivers/nvdimm/namespace_devs.c
+++ b/drivers/nvdimm/namespace_devs.c
@@ -1231,10 +1231,12 @@ static int namespace_update_uuid(struct
nd_region *nd_region,
                list_for_each_entry(label_ent, &nd_mapping->labels, list) {
                        struct nd_namespace_label *nd_label = label_ent->label;
                        struct nd_label_id label_id;
+                       uuid_t uuid;

                        if (!nd_label)
                                continue;
-                       nd_label_gen_id(&label_id, nsl_ref_uuid(ndd, nd_label),
+                       nsl_get_uuid(ndd, nd_label, &uuid);
+                       nd_label_gen_id(&label_id, &uuid,
                                        nsl_get_flags(ndd, nd_label));
                        if (strcmp(old_label_id.id, label_id.id) == 0)
                                set_bit(ND_LABEL_REAP, &label_ent->flags);
@@ -1856,7 +1858,7 @@ static bool has_uuid_at_pos(struct nd_region
*nd_region, const uuid_t *uuid,
                        if (!nsl_validate_isetcookie(ndd, nd_label, cookie))
                                continue;

-                       if (!nsl_validate_uuid(ndd, nd_label, uuid))
+                       if (!nsl_uuid_equal(ndd, nd_label, uuid))
                                continue;

                        if (!nsl_validate_type_guid(ndd, nd_label,
@@ -1900,7 +1902,7 @@ static int select_pmem_id(struct nd_region
*nd_region, const uuid_t *pmem_id)
                        nd_label = label_ent->label;
                        if (!nd_label)
                                continue;
-                       if (nsl_validate_uuid(ndd, nd_label, pmem_id))
+                       if (nsl_uuid_equal(ndd, nd_label, pmem_id))
                                break;
                        nd_label = NULL;
                }
@@ -1924,7 +1926,7 @@ static int select_pmem_id(struct nd_region
*nd_region, const uuid_t *pmem_id)
                else {
                        dev_dbg(&nd_region->dev, "%s invalid label for %pUb\n",
                                dev_name(ndd->dev),
-                               nsl_ref_uuid(ndd, nd_label));
+                               nsl_uuid_raw(ndd, nd_label));
                        return -EINVAL;
                }

@@ -1954,6 +1956,7 @@ static struct device
*create_namespace_pmem(struct nd_region *nd_region,
        resource_size_t size = 0;
        struct resource *res;
        struct device *dev;
+       uuid_t uuid;
        int rc = 0;
        u16 i;

@@ -1964,12 +1967,12 @@ static struct device
*create_namespace_pmem(struct nd_region *nd_region,

        if (!nsl_validate_isetcookie(ndd, nd_label, cookie)) {
                dev_dbg(&nd_region->dev, "invalid cookie in label: %pUb\n",
-                       nsl_ref_uuid(ndd, nd_label));
+                       nsl_uuid_raw(ndd, nd_label));
                if (!nsl_validate_isetcookie(ndd, nd_label, altcookie))
                        return ERR_PTR(-EAGAIN);

                dev_dbg(&nd_region->dev, "valid altcookie in label: %pUb\n",
-                       nsl_ref_uuid(ndd, nd_label));
+                       nsl_uuid_raw(ndd, nd_label));
        }

        nspm = kzalloc(sizeof(*nspm), GFP_KERNEL);
@@ -1985,11 +1988,12 @@ static struct device
*create_namespace_pmem(struct nd_region *nd_region,
        res->flags = IORESOURCE_MEM;

        for (i = 0; i < nd_region->ndr_mappings; i++) {
-               if (has_uuid_at_pos(nd_region, nsl_ref_uuid(ndd, nd_label),
-                                   cookie, i))
+               uuid_t uuid;
+
+               nsl_get_uuid(ndd, nd_label, &uuid);
+               if (has_uuid_at_pos(nd_region, &uuid, cookie, i))
                        continue;
-               if (has_uuid_at_pos(nd_region, nsl_ref_uuid(ndd, nd_label),
-                                   altcookie, i))
+               if (has_uuid_at_pos(nd_region, &uuid, altcookie, i))
                        continue;
                break;
        }
@@ -2003,7 +2007,7 @@ static struct device
*create_namespace_pmem(struct nd_region *nd_region,
                 * find a dimm with two instances of the same uuid.
                 */
                dev_err(&nd_region->dev, "%s missing label for %pUb\n",
-                       nvdimm_name(nvdimm), nsl_ref_uuid(ndd, nd_label));
+                       nvdimm_name(nvdimm), nsl_uuid_raw(ndd, nd_label));
                rc = -EINVAL;
                goto err;
        }
@@ -2016,7 +2020,8 @@ static struct device
*create_namespace_pmem(struct nd_region *nd_region,
         * the dimm being enabled (i.e. nd_label_reserve_dpa()
         * succeeded).
         */
-       rc = select_pmem_id(nd_region, nsl_ref_uuid(ndd, nd_label));
+       nsl_get_uuid(ndd, nd_label, &uuid);
+       rc = select_pmem_id(nd_region, &uuid);
        if (rc)
                goto err;

@@ -2042,8 +2047,8 @@ static struct device
*create_namespace_pmem(struct nd_region *nd_region,
                WARN_ON(nspm->alt_name || nspm->uuid);
                nspm->alt_name = kmemdup(nsl_ref_name(ndd, label0),
                                         NSLABEL_NAME_LEN, GFP_KERNEL);
-               nspm->uuid = kmemdup(nsl_ref_uuid(ndd, label0), sizeof(uuid_t),
-                                    GFP_KERNEL);
+               nsl_get_uuid(ndd, label0, &uuid);
+               nspm->uuid = kmemdup(&uuid, sizeof(uuid_t), GFP_KERNEL);
                nspm->lbasize = nsl_get_lbasize(ndd, label0);
                nspm->nsio.common.claim_class =
                        nsl_get_claim_class(ndd, label0);
@@ -2228,7 +2233,7 @@ static int add_namespace_resource(struct
nd_region *nd_region,
                        continue;
                }

-               if (!nsl_validate_uuid(ndd, nd_label, uuid))
+               if (!nsl_uuid_equal(ndd, nd_label, uuid))
                        continue;
                if (is_namespace_blk(devs[i])) {
                        res = nsblk_add_resource(nd_region, ndd,
@@ -2260,6 +2265,7 @@ static struct device
*create_namespace_blk(struct nd_region *nd_region,
        char name[NSLABEL_NAME_LEN];
        struct device *dev = NULL;
        struct resource *res;
+       uuid_t uuid;

        if (!nsl_validate_type_guid(ndd, nd_label, &nd_set->type_guid))
                return ERR_PTR(-EAGAIN);
@@ -2274,7 +2280,8 @@ static struct device
*create_namespace_blk(struct nd_region *nd_region,
        dev->parent = &nd_region->dev;
        nsblk->id = -1;
        nsblk->lbasize = nsl_get_lbasize(ndd, nd_label);
-       nsblk->uuid = kmemdup(nsl_ref_uuid(ndd, nd_label),
sizeof(uuid_t), GFP_KERNEL);
+       nsl_get_uuid(ndd, nd_label, &uuid);
+       nsblk->uuid = kmemdup(&uuid, sizeof(uuid_t), GFP_KERNEL);
        nsblk->common.claim_class = nsl_get_claim_class(ndd, nd_label);
        if (!nsblk->uuid)
                goto blk_err;
diff --git a/drivers/nvdimm/nd.h b/drivers/nvdimm/nd.h
index 132a8021e3ad..b781bf674f0a 100644
--- a/drivers/nvdimm/nd.h
+++ b/drivers/nvdimm/nd.h
@@ -180,7 +180,7 @@ static inline const uuid_t *nsl_get_uuid(struct
nvdimm_drvdata *ndd,
                                         struct nd_namespace_label *nd_label,
                                         uuid_t *uuid)
 {
-       uuid_copy(uuid, &nd_label->uuid);
+       import_uuid(uuid, nd_label->uuid);
        return uuid;
 }

@@ -188,21 +188,24 @@ static inline const uuid_t *nsl_set_uuid(struct
nvdimm_drvdata *ndd,
                                         struct nd_namespace_label *nd_label,
                                         const uuid_t *uuid)
 {
-       uuid_copy(&nd_label->uuid, uuid);
-       return &nd_label->uuid;
+       export_uuid(nd_label->uuid, uuid);
+       return uuid;
 }

-static inline bool nsl_validate_uuid(struct nvdimm_drvdata *ndd,
-                                    struct nd_namespace_label *nd_label,
-                                    const uuid_t *uuid)
+static inline bool nsl_uuid_equal(struct nvdimm_drvdata *ndd,
+                                 struct nd_namespace_label *nd_label,
+                                 const uuid_t *uuid)
 {
-       return uuid_equal(&nd_label->uuid, uuid);
+       uuid_t tmp;
+
+       import_uuid(&tmp, nd_label->uuid);
+       return uuid_equal(&tmp, uuid);
 }

-static inline const uuid_t *nsl_ref_uuid(struct nvdimm_drvdata *ndd,
-                                        struct nd_namespace_label *nd_label)
+static inline const u8 *nsl_uuid_raw(struct nvdimm_drvdata *ndd,
+                                    struct nd_namespace_label *nd_label)
 {
-       return &nd_label->uuid;
+       return nd_label->uuid;
 }

 bool nsl_validate_blk_isetcookie(struct nvdimm_drvdata *ndd,

  parent reply	other threads:[~2021-08-12 22:33 UTC|newest]

Thread overview: 61+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-09 22:27 [PATCH 00/23] cxl_test: Enable CXL Topology and UAPI regression tests Dan Williams
2021-08-09 22:27 ` [PATCH 01/23] libnvdimm/labels: Introduce getters for namespace label fields Dan Williams
2021-08-10 20:48   ` Ben Widawsky
2021-08-10 21:58     ` Dan Williams
2021-08-11 18:44   ` Jonathan Cameron
2021-08-09 22:27 ` [PATCH 02/23] libnvdimm/labels: Add isetcookie validation helper Dan Williams
2021-08-11 18:44   ` Jonathan Cameron
2021-08-09 22:28 ` [PATCH 03/23] libnvdimm/labels: Introduce label setter helpers Dan Williams
2021-08-11 17:27   ` Jonathan Cameron
2021-08-11 17:42     ` Dan Williams
2021-08-09 22:28 ` [PATCH 04/23] libnvdimm/labels: Add a checksum calculation helper Dan Williams
2021-08-11 18:44   ` Jonathan Cameron
2021-08-09 22:28 ` [PATCH 05/23] libnvdimm/labels: Add blk isetcookie set / validation helpers Dan Williams
2021-08-11 18:45   ` Jonathan Cameron
2021-08-09 22:28 ` [PATCH 06/23] libnvdimm/labels: Add blk special cases for nlabel and position helpers Dan Williams
2021-08-11 18:45   ` Jonathan Cameron
2021-08-09 22:28 ` [PATCH 07/23] libnvdimm/labels: Add type-guid helpers Dan Williams
2021-08-11 18:46   ` Jonathan Cameron
2021-08-09 22:28 ` [PATCH 08/23] libnvdimm/labels: Add claim class helpers Dan Williams
2021-08-11 18:46   ` Jonathan Cameron
2021-08-09 22:28 ` [PATCH 09/23] libnvdimm/labels: Add address-abstraction uuid definitions Dan Williams
2021-08-11 18:49   ` Jonathan Cameron
2021-08-11 22:47     ` Dan Williams
2021-08-09 22:28 ` [PATCH 10/23] libnvdimm/labels: Add uuid helpers Dan Williams
2021-08-11  8:05   ` Andy Shevchenko
2021-08-11 16:59     ` Andy Shevchenko
2021-08-11 17:11       ` Dan Williams
2021-08-11 19:18         ` Andy Shevchenko
2021-08-11 19:26           ` Dan Williams
2021-08-12 22:34           ` Dan Williams [this message]
2021-08-13 10:14             ` Andy Shevchenko
2021-08-14  7:35               ` Christoph Hellwig
2021-08-11 18:13   ` Jonathan Cameron
2021-08-12 21:17     ` Dan Williams
2021-08-09 22:28 ` [PATCH 11/23] libnvdimm/labels: Introduce CXL labels Dan Williams
2021-08-11 18:41   ` Jonathan Cameron
2021-08-11 23:01     ` Dan Williams
2021-08-09 22:28 ` [PATCH 12/23] cxl/pci: Make 'struct cxl_mem' device type generic Dan Williams
2021-08-09 22:28 ` [PATCH 13/23] cxl/mbox: Introduce the mbox_send operation Dan Williams
2021-08-09 22:29 ` [PATCH 14/23] cxl/mbox: Move mailbox and other non-PCI specific infrastructure to the core Dan Williams
2021-08-11  6:11   ` [PATCH v2 " Dan Williams
2021-08-09 22:29 ` [PATCH 15/23] cxl/pci: Use module_pci_driver Dan Williams
2021-08-09 22:29 ` [PATCH 16/23] cxl/mbox: Convert 'enabled_cmds' to DECLARE_BITMAP Dan Williams
2021-08-09 22:29 ` [PATCH 17/23] cxl/mbox: Add exclusive kernel command support Dan Williams
2021-08-10 21:34   ` Ben Widawsky
2021-08-10 21:52     ` Dan Williams
2021-08-10 22:06       ` Ben Widawsky
2021-08-11  1:22         ` Dan Williams
2021-08-11  2:14           ` Dan Williams
2021-08-09 22:29 ` [PATCH 18/23] cxl/pmem: Translate NVDIMM label commands to CXL label commands Dan Williams
2021-08-09 22:29 ` [PATCH 19/23] cxl/pmem: Add support for multiple nvdimm-bridge objects Dan Williams
2021-08-09 22:29 ` [PATCH 20/23] tools/testing/cxl: Introduce a mocked-up CXL port hierarchy Dan Williams
2021-08-10 21:57   ` Ben Widawsky
2021-08-10 22:40     ` Dan Williams
2021-08-11 15:18       ` Ben Widawsky
     [not found]       ` <xp0k4.l2r85dw1p7do@intel.com>
2021-08-11 21:03         ` Dan Williams
2021-08-09 22:29 ` [PATCH 21/23] cxl/bus: Populate the target list at decoder create Dan Williams
2021-08-09 22:29 ` [PATCH 22/23] cxl/mbox: Move command definitions to common location Dan Williams
2021-08-09 22:29 ` [PATCH 23/23] tools/testing/cxl: Introduce a mock memory device + driver Dan Williams
2021-08-10 22:10 ` [PATCH 00/23] cxl_test: Enable CXL Topology and UAPI regression tests Ben Widawsky
2021-08-10 22:58   ` Dan Williams

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=CAPcyv4hY9YL7MhkeSu4GYBNo6hbeMRgqnKf8YuLuQ3khSbhn9A@mail.gmail.com \
    --to=dan.j.williams@intel.com \
    --cc=Jonathan.Cameron@huawei.com \
    --cc=alison.schofield@intel.com \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=ben.widawsky@intel.com \
    --cc=ira.weiny@intel.com \
    --cc=linux-cxl@vger.kernel.org \
    --cc=nvdimm@lists.linux.dev \
    --cc=vishal.l.verma@intel.com \
    --subject='Re: [PATCH 10/23] libnvdimm/labels: Add uuid helpers' \
    /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

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).