On Wed, Aug 11, 2021 at 11:14 AM Jonathan Cameron <[email protected]> wrote: > > On Mon, 9 Aug 2021 15:28:40 -0700 > Dan Williams <[email protected]> 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. > > > > Reported-by: Andy Shevchenko <[email protected]> > > Signed-off-by: Dan Williams <[email protected]> > > There are a few interesting corners where you have cleaned out a pointless > copy before validating uuids. Perhaps call that out as a change in here > as it isn't as simple as just replacing like with like? > Perhaps I'm missing some reason that was needed in the code before this > patch. > > All LGTM. > > Reviewed-by: Jonathan Cameron <[email protected]> > > > --- > > drivers/nvdimm/btt.c | 11 +++-- > > drivers/nvdimm/btt.h | 4 +- > > drivers/nvdimm/btt_devs.c | 12 +++--- > > drivers/nvdimm/core.c | 40 ++----------------- > > drivers/nvdimm/label.c | 34 +++++++--------- > > drivers/nvdimm/label.h | 3 - > > drivers/nvdimm/namespace_devs.c | 83 > > ++++++++++++++++++++------------------- > > drivers/nvdimm/nd-core.h | 5 +- > > drivers/nvdimm/nd.h | 37 ++++++++++++++++- > > drivers/nvdimm/pfn_devs.c | 2 - > > include/linux/nd.h | 4 +- > > 11 files changed, 115 insertions(+), 120 deletions(-) > > > > diff --git a/drivers/nvdimm/btt.c b/drivers/nvdimm/btt.c > > index 92dec4952297..1cdfbadb7408 100644 > > > @@ -1050,7 +1050,6 @@ static int __blk_label_update(struct nd_region > > *nd_region, > > unsigned long *free, *victim_map = NULL; > > struct resource *res, **old_res_list; > > struct nd_label_id label_id; > > - u8 uuid[NSLABEL_UUID_LEN]; > > int min_dpa_idx = 0; > > LIST_HEAD(list); > > u32 nslot, slot; > > @@ -1088,8 +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); > > - memcpy(uuid, nd_label->uuid, NSLABEL_UUID_LEN); > > - if (memcmp(uuid, nsblk->uuid, NSLABEL_UUID_LEN) != 0) > > + if (!nsl_validate_uuid(ndd, nd_label, nsblk->uuid)) > > continue; > > The original code here was 'unusual'. I'm not sure why it couldn't always be > validated in place. >
Correct, I noticed that too and cleaned it up, but you're right I should have at least noted that in the changelog.
