On 9/22/25 3:50 PM, dan.j.willi...@intel.com wrote:
> Dave Jiang wrote:
>> Converting nvdimm_bus_lock/unlock to guard() to clean up usage
>> of gotos for error handling and avoid future mistakes of missed
>> unlock on error paths.
>>
>> Link: https://lore.kernel.org/linux-cxl/20250917163623.00004...@huawei.com/
>> Suggested-by: Jonathan Cameron <jonathan.came...@huawei.com>
>> Signed-off-by: Dave Jiang <dave.ji...@intel.com>
>> ---
>> Ira, up to you if you want to take this cleanup.
>> ---
> [..]
>> diff --git a/drivers/nvdimm/bus.c b/drivers/nvdimm/bus.c
>> index 0ccf4a9e523a..d2c2d71e7fe0 100644
>> --- a/drivers/nvdimm/bus.c
>> +++ b/drivers/nvdimm/bus.c
>> @@ -1177,15 +1175,15 @@ static int __nd_ioctl(struct nvdimm_bus *nvdimm_bus,
>> struct nvdimm *nvdimm,
>> goto out;
>> }
>>
>> - device_lock(dev);
>> - nvdimm_bus_lock(dev);
>> + guard(device)(dev);
>> + guard(nvdimm_bus)(dev);
>> rc = nd_cmd_clear_to_send(nvdimm_bus, nvdimm, func, buf);
>> if (rc)
>> - goto out_unlock;
>> + goto out;
>>
>> rc = nd_desc->ndctl(nd_desc, nvdimm, cmd, buf, buf_len, &cmd_rc);
>> if (rc < 0)
>> - goto out_unlock;
>> + goto out;
>>
>> if (!nvdimm && cmd == ND_CMD_CLEAR_ERROR && cmd_rc >= 0) {
>> struct nd_cmd_clear_error *clear_err = buf;
> [..]
>> diff --git a/drivers/nvdimm/dimm.c b/drivers/nvdimm/dimm.c
>> index 91d9163ee303..2018458a3dba 100644
>> --- a/drivers/nvdimm/dimm.c
>> +++ b/drivers/nvdimm/dimm.c
>> @@ -95,13 +95,13 @@ static int nvdimm_probe(struct device *dev)
>>
>> dev_dbg(dev, "config data size: %d\n", ndd->nsarea.config_size);
>>
>> - nvdimm_bus_lock(dev);
>> - if (ndd->ns_current >= 0) {
>> - rc = nd_label_reserve_dpa(ndd);
>> - if (rc == 0)
>> - nvdimm_set_labeling(dev);
>> + scoped_guard(nvdimm_bus, dev) {
>> + if (ndd->ns_current >= 0) {
>> + rc = nd_label_reserve_dpa(ndd);
>> + if (rc == 0)
>> + nvdimm_set_labeling(dev);
>> + }
>> }
>> - nvdimm_bus_unlock(dev);
>>
>> if (rc)
>> goto err;
> [.. trim the hunks that successfully eliminated goto ..]
>
> My litmus test for scoped-based-cleanup conversions is whether *all* of
> the gotos are removed in a converted function. So either fully convert
> all error unwind in a function to scope-based, or none of it. Do not
> leave people to reason about potentially jumping through scopes with
> goto.
This needs a bit more changes. I was undecided if I should include those
changes in this patch or if they push out of scope. But it sounds like I should
include them.
