On 20/09/25 12:44PM, Ira Weiny wrote:
Dave Jiang wrote:
[snip]
> @@ -998,9 +998,8 @@ static int init_labels(struct nd_mapping *nd_mapping, int
num_labels)
> label_ent = kzalloc(sizeof(*label_ent), GFP_KERNEL);
> if (!label_ent)
> return -ENOMEM;
> - mutex_lock(&nd_mapping->lock);
> + guard(mutex)(&nd_mapping->lock);
> list_add_tail(&label_ent->list, &nd_mapping->labels);
> - mutex_unlock(&nd_mapping->lock);
I would not mix and match old and new locking flow in a function. If you are
going to convert, then do the whole function. I think earlier in this function
you may need a scoped_guard() call.
FWIW I would limit the changes to __pmem_label_update() because that is
the function which benefits from these changes.
> }
>
> if (ndd->ns_current == -1 || ndd->ns_next == -1)
> @@ -1039,7 +1038,7 @@ static int del_labels(struct nd_mapping *nd_mapping,
uuid_t *uuid)
> if (!preamble_next(ndd, &nsindex, &free, &nslot))
> return 0;
>
> - mutex_lock(&nd_mapping->lock);
> + guard(mutex)(&nd_mapping->lock);
So this change now includes nd_label_write_index() in the lock context as well
compare to the old code. So either you should use a scoped_guard() or create a
helper function and move the block of code being locked to the helper function
with guard() to avoid changing the original code flow.
Sure you could do this but again I don't think these updates are worth
this amount of work right now.
Yes Ira,
Adding change as per Dave's suggestion would require some extra code change
which may not be required here.
I will fix the locking in __pmem_label_update() only.
Regards,
Neeraj
