Thu, Feb 29, 2024 at 04:10:26PM +0100, Nuno Sa kirjoitti:
> Use the new cleanup magic for handling mutexes in IIO. This allows us to
> greatly simplify some code paths.
...
> /* Add to list of available triggers held by the IIO core */
> - mutex_lock(&iio_trigger_list_lock);
> - if (__iio_trigger_find_by_name(trig_info->name)) {
> - pr_err("Duplicate trigger name '%s'\n", trig_info->name);
> - ret = -EEXIST;
> - goto error_device_del;
> + scoped_guard(mutex, &iio_trigger_list_lock) {
> + if (__iio_trigger_find_by_name(trig_info->name)) {
> + pr_err("Duplicate trigger name '%s'\n", trig_info->name);
> + ret = -EEXIST;
> + goto error_device_del;
With scoped_guard() it can't be achived, but in the original code the unlocking
can be potentially done before printing the message.
Here are pros and cons of printing messages under the lock:
+ the timestamp and appearance in the log probably more accurate
- the lock maybe taken for much longer time (if there is a slow
console is involved)
That said, always consider where to put message printing when it's a critical
section.
> + }
> + list_add_tail(&trig_info->list, &iio_trigger_list);
> }
...
> list_for_each_entry(iter, &iio_trigger_list, list)
> - if (sysfs_streq(iter->name, name)) {
> - trig = iter;
> - iio_trigger_get(trig);
> - break;
> - }
> - mutex_unlock(&iio_trigger_list_lock);
> + if (sysfs_streq(iter->name, name))
> + return iio_trigger_get(iter);
In this case the outer {} better to have.
...
> - ret = bitmap_find_free_region(trig->pool,
> - CONFIG_IIO_CONSUMERS_PER_TRIGGER,
> - ilog2(1));
> + ret = bitmap_find_free_region(trig->pool,
> + CONFIG_IIO_CONSUMERS_PER_TRIGGER,
> + ilog2(1));
Despite being in the original code, this is funny magic constant...
--
With Best Regards,
Andy Shevchenko
