ALSA: rawmidi: Use guard() for locking
authorTakashi Iwai <tiwai@suse.de>
Tue, 27 Feb 2024 08:52:51 +0000 (09:52 +0100)
committerTakashi Iwai <tiwai@suse.de>
Wed, 28 Feb 2024 14:01:21 +0000 (15:01 +0100)
We can simplify the code gracefully with new guard() macro and co for
automatic cleanup of locks.

There are a few remaining explicit mutex and spinlock calls, and those
are the places where the temporary unlock/relocking happens -- which
guard() doens't cover well yet.

Only the code refactoring, and no functional changes.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
Link: https://lore.kernel.org/r/20240227085306.9764-10-tiwai@suse.de
sound/core/rawmidi.c

index 1431cb997808d071d44efadba2b8a1d49117ee25..7accf9a1ddf4c625255fd47794d6ff1d5cbf068c 100644 (file)
@@ -105,13 +105,8 @@ static inline bool __snd_rawmidi_ready(struct snd_rawmidi_runtime *runtime)
 
 static bool snd_rawmidi_ready(struct snd_rawmidi_substream *substream)
 {
-       unsigned long flags;
-       bool ready;
-
-       spin_lock_irqsave(&substream->lock, flags);
-       ready = __snd_rawmidi_ready(substream->runtime);
-       spin_unlock_irqrestore(&substream->lock, flags);
-       return ready;
+       guard(spinlock_irqsave)(&substream->lock);
+       return __snd_rawmidi_ready(substream->runtime);
 }
 
 static inline int snd_rawmidi_ready_append(struct snd_rawmidi_substream *substream,
@@ -238,12 +233,9 @@ static void __reset_runtime_ptrs(struct snd_rawmidi_runtime *runtime,
 static void reset_runtime_ptrs(struct snd_rawmidi_substream *substream,
                               bool is_input)
 {
-       unsigned long flags;
-
-       spin_lock_irqsave(&substream->lock, flags);
+       guard(spinlock_irqsave)(&substream->lock);
        if (substream->opened && substream->runtime)
                __reset_runtime_ptrs(substream->runtime, is_input);
-       spin_unlock_irqrestore(&substream->lock, flags);
 }
 
 int snd_rawmidi_drop_output(struct snd_rawmidi_substream *substream)
@@ -260,33 +252,29 @@ int snd_rawmidi_drain_output(struct snd_rawmidi_substream *substream)
        long timeout;
        struct snd_rawmidi_runtime *runtime;
 
-       spin_lock_irq(&substream->lock);
-       runtime = substream->runtime;
-       if (!substream->opened || !runtime || !runtime->buffer) {
-               err = -EINVAL;
-       } else {
+       scoped_guard(spinlock_irq, &substream->lock) {
+               runtime = substream->runtime;
+               if (!substream->opened || !runtime || !runtime->buffer)
+                       return -EINVAL;
                snd_rawmidi_buffer_ref(runtime);
                runtime->drain = 1;
        }
-       spin_unlock_irq(&substream->lock);
-       if (err < 0)
-               return err;
 
        timeout = wait_event_interruptible_timeout(runtime->sleep,
                                (runtime->avail >= runtime->buffer_size),
                                10*HZ);
 
-       spin_lock_irq(&substream->lock);
-       if (signal_pending(current))
-               err = -ERESTARTSYS;
-       if (runtime->avail < runtime->buffer_size && !timeout) {
-               rmidi_warn(substream->rmidi,
-                          "rawmidi drain error (avail = %li, buffer_size = %li)\n",
-                          (long)runtime->avail, (long)runtime->buffer_size);
-               err = -EIO;
+       scoped_guard(spinlock_irq, &substream->lock) {
+               if (signal_pending(current))
+                       err = -ERESTARTSYS;
+               if (runtime->avail < runtime->buffer_size && !timeout) {
+                       rmidi_warn(substream->rmidi,
+                                  "rawmidi drain error (avail = %li, buffer_size = %li)\n",
+                                  (long)runtime->avail, (long)runtime->buffer_size);
+                       err = -EIO;
+               }
+               runtime->drain = 0;
        }
-       runtime->drain = 0;
-       spin_unlock_irq(&substream->lock);
 
        if (err != -ERESTARTSYS) {
                /* we need wait a while to make sure that Tx FIFOs are empty */
@@ -297,9 +285,8 @@ int snd_rawmidi_drain_output(struct snd_rawmidi_substream *substream)
                snd_rawmidi_drop_output(substream);
        }
 
-       spin_lock_irq(&substream->lock);
-       snd_rawmidi_buffer_unref(runtime);
-       spin_unlock_irq(&substream->lock);
+       scoped_guard(spinlock_irq, &substream->lock)
+               snd_rawmidi_buffer_unref(runtime);
 
        return err;
 }
@@ -363,14 +350,13 @@ static int open_substream(struct snd_rawmidi *rmidi,
                        snd_rawmidi_runtime_free(substream);
                        return err;
                }
-               spin_lock_irq(&substream->lock);
+               guard(spinlock_irq)(&substream->lock);
                substream->opened = 1;
                substream->active_sensing = 0;
                if (mode & SNDRV_RAWMIDI_LFLG_APPEND)
                        substream->append = 1;
                substream->pid = get_pid(task_pid(current));
                rmidi->streams[substream->stream].substream_opened++;
-               spin_unlock_irq(&substream->lock);
        }
        substream->use_count++;
        return 0;
@@ -433,9 +419,8 @@ int snd_rawmidi_kernel_open(struct snd_rawmidi *rmidi, int subdevice,
        if (!try_module_get(rmidi->card->module))
                return -ENXIO;
 
-       mutex_lock(&rmidi->open_mutex);
+       guard(mutex)(&rmidi->open_mutex);
        err = rawmidi_open_priv(rmidi, subdevice, mode, rfile);
-       mutex_unlock(&rmidi->open_mutex);
        if (err < 0)
                module_put(rmidi->card->module);
        return err;
@@ -568,10 +553,10 @@ static void close_substream(struct snd_rawmidi *rmidi,
                }
                snd_rawmidi_buffer_ref_sync(substream);
        }
-       spin_lock_irq(&substream->lock);
-       substream->opened = 0;
-       substream->append = 0;
-       spin_unlock_irq(&substream->lock);
+       scoped_guard(spinlock_irq, &substream->lock) {
+               substream->opened = 0;
+               substream->append = 0;
+       }
        substream->ops->close(substream);
        if (substream->runtime->private_free)
                substream->runtime->private_free(substream);
@@ -586,7 +571,7 @@ static void rawmidi_release_priv(struct snd_rawmidi_file *rfile)
        struct snd_rawmidi *rmidi;
 
        rmidi = rfile->rmidi;
-       mutex_lock(&rmidi->open_mutex);
+       guard(mutex)(&rmidi->open_mutex);
        if (rfile->input) {
                close_substream(rmidi, rfile->input, 1);
                rfile->input = NULL;
@@ -596,7 +581,6 @@ static void rawmidi_release_priv(struct snd_rawmidi_file *rfile)
                rfile->output = NULL;
        }
        rfile->rmidi = NULL;
-       mutex_unlock(&rmidi->open_mutex);
        wake_up(&rmidi->open_wait);
 }
 
@@ -695,12 +679,8 @@ static int __snd_rawmidi_info_select(struct snd_card *card,
 
 int snd_rawmidi_info_select(struct snd_card *card, struct snd_rawmidi_info *info)
 {
-       int ret;
-
-       mutex_lock(&register_mutex);
-       ret = __snd_rawmidi_info_select(card, info);
-       mutex_unlock(&register_mutex);
-       return ret;
+       guard(mutex)(&register_mutex);
+       return __snd_rawmidi_info_select(card, info);
 }
 EXPORT_SYMBOL(snd_rawmidi_info_select);
 
@@ -744,9 +724,8 @@ static int resize_runtime_buffer(struct snd_rawmidi_substream *substream,
                newbuf = kvzalloc(params->buffer_size, GFP_KERNEL);
                if (!newbuf)
                        return -ENOMEM;
-               spin_lock_irq(&substream->lock);
+               guard(spinlock_irq)(&substream->lock);
                if (runtime->buffer_ref) {
-                       spin_unlock_irq(&substream->lock);
                        kvfree(newbuf);
                        return -EBUSY;
                }
@@ -754,7 +733,6 @@ static int resize_runtime_buffer(struct snd_rawmidi_substream *substream,
                runtime->buffer = newbuf;
                runtime->buffer_size = params->buffer_size;
                __reset_runtime_ptrs(runtime, is_input);
-               spin_unlock_irq(&substream->lock);
                kvfree(oldbuf);
        }
        runtime->avail_min = params->avail_min;
@@ -767,15 +745,12 @@ int snd_rawmidi_output_params(struct snd_rawmidi_substream *substream,
        int err;
 
        snd_rawmidi_drain_output(substream);
-       mutex_lock(&substream->rmidi->open_mutex);
+       guard(mutex)(&substream->rmidi->open_mutex);
        if (substream->append && substream->use_count > 1)
-               err = -EBUSY;
-       else
-               err = resize_runtime_buffer(substream, params, false);
-
+               return -EBUSY;
+       err = resize_runtime_buffer(substream, params, false);
        if (!err)
                substream->active_sensing = !params->no_active_sensing;
-       mutex_unlock(&substream->rmidi->open_mutex);
        return err;
 }
 EXPORT_SYMBOL(snd_rawmidi_output_params);
@@ -788,7 +763,7 @@ int snd_rawmidi_input_params(struct snd_rawmidi_substream *substream,
        int err;
 
        snd_rawmidi_drain_input(substream);
-       mutex_lock(&substream->rmidi->open_mutex);
+       guard(mutex)(&substream->rmidi->open_mutex);
        if (framing == SNDRV_RAWMIDI_MODE_FRAMING_NONE && clock_type != SNDRV_RAWMIDI_MODE_CLOCK_NONE)
                err = -EINVAL;
        else if (clock_type > SNDRV_RAWMIDI_MODE_CLOCK_MONOTONIC_RAW)
@@ -802,7 +777,6 @@ int snd_rawmidi_input_params(struct snd_rawmidi_substream *substream,
                substream->framing = framing;
                substream->clock_type = clock_type;
        }
-       mutex_unlock(&substream->rmidi->open_mutex);
        return 0;
 }
 EXPORT_SYMBOL(snd_rawmidi_input_params);
@@ -814,9 +788,8 @@ static int snd_rawmidi_output_status(struct snd_rawmidi_substream *substream,
 
        memset(status, 0, sizeof(*status));
        status->stream = SNDRV_RAWMIDI_STREAM_OUTPUT;
-       spin_lock_irq(&substream->lock);
+       guard(spinlock_irq)(&substream->lock);
        status->avail = runtime->avail;
-       spin_unlock_irq(&substream->lock);
        return 0;
 }
 
@@ -827,11 +800,10 @@ static int snd_rawmidi_input_status(struct snd_rawmidi_substream *substream,
 
        memset(status, 0, sizeof(*status));
        status->stream = SNDRV_RAWMIDI_STREAM_INPUT;
-       spin_lock_irq(&substream->lock);
+       guard(spinlock_irq)(&substream->lock);
        status->avail = runtime->avail;
        status->xruns = runtime->xruns;
        runtime->xruns = 0;
-       spin_unlock_irq(&substream->lock);
        return 0;
 }
 
@@ -1025,19 +997,19 @@ static int snd_rawmidi_next_device(struct snd_card *card, int __user *argp,
                return -EFAULT;
        if (device >= SNDRV_RAWMIDI_DEVICES) /* next device is -1 */
                device = SNDRV_RAWMIDI_DEVICES - 1;
-       mutex_lock(&register_mutex);
-       device = device < 0 ? 0 : device + 1;
-       for (; device < SNDRV_RAWMIDI_DEVICES; device++) {
-               rmidi = snd_rawmidi_search(card, device);
-               if (!rmidi)
-                       continue;
-               is_ump = rawmidi_is_ump(rmidi);
-               if (find_ump == is_ump)
-                       break;
+       scoped_guard(mutex, &register_mutex) {
+               device = device < 0 ? 0 : device + 1;
+               for (; device < SNDRV_RAWMIDI_DEVICES; device++) {
+                       rmidi = snd_rawmidi_search(card, device);
+                       if (!rmidi)
+                               continue;
+                       is_ump = rawmidi_is_ump(rmidi);
+                       if (find_ump == is_ump)
+                               break;
+               }
+               if (device == SNDRV_RAWMIDI_DEVICES)
+                       device = -1;
        }
-       if (device == SNDRV_RAWMIDI_DEVICES)
-               device = -1;
-       mutex_unlock(&register_mutex);
        if (put_user(device, argp))
                return -EFAULT;
        return 0;
@@ -1050,18 +1022,16 @@ static int snd_rawmidi_call_ump_ioctl(struct snd_card *card, int cmd,
 {
        struct snd_ump_endpoint_info __user *info = argp;
        struct snd_rawmidi *rmidi;
-       int device, ret;
+       int device;
 
        if (get_user(device, &info->device))
                return -EFAULT;
-       mutex_lock(&register_mutex);
+       guard(mutex)(&register_mutex);
        rmidi = snd_rawmidi_search(card, device);
        if (rmidi && rmidi->ops && rmidi->ops->ioctl)
-               ret = rmidi->ops->ioctl(rmidi, cmd, argp);
+               return rmidi->ops->ioctl(rmidi, cmd, argp);
        else
-               ret = -ENXIO;
-       mutex_unlock(&register_mutex);
-       return ret;
+               return -ENXIO;
 }
 #endif
 
@@ -1168,27 +1138,23 @@ static struct timespec64 get_framing_tstamp(struct snd_rawmidi_substream *substr
 int snd_rawmidi_receive(struct snd_rawmidi_substream *substream,
                        const unsigned char *buffer, int count)
 {
-       unsigned long flags;
        struct timespec64 ts64 = get_framing_tstamp(substream);
        int result = 0, count1;
        struct snd_rawmidi_runtime *runtime;
 
-       spin_lock_irqsave(&substream->lock, flags);
-       if (!substream->opened) {
-               result = -EBADFD;
-               goto unlock;
-       }
+       guard(spinlock_irqsave)(&substream->lock);
+       if (!substream->opened)
+               return -EBADFD;
        runtime = substream->runtime;
        if (!runtime || !runtime->buffer) {
                rmidi_dbg(substream->rmidi,
                          "snd_rawmidi_receive: input is not active!!!\n");
-               result = -EINVAL;
-               goto unlock;
+               return -EINVAL;
        }
 
        count = get_aligned_size(runtime, count);
        if (!count)
-               goto unlock;
+               return result;
 
        if (substream->framing == SNDRV_RAWMIDI_MODE_FRAMING_TSTAMP) {
                result = receive_with_tstamp_framing(substream, buffer, count, &ts64);
@@ -1211,7 +1177,7 @@ int snd_rawmidi_receive(struct snd_rawmidi_substream *substream,
                        count1 = runtime->buffer_size - runtime->avail;
                count1 = get_aligned_size(runtime, count1);
                if (!count1)
-                       goto unlock;
+                       return result;
                memcpy(runtime->buffer + runtime->hw_ptr, buffer, count1);
                runtime->hw_ptr += count1;
                runtime->hw_ptr %= runtime->buffer_size;
@@ -1239,8 +1205,6 @@ int snd_rawmidi_receive(struct snd_rawmidi_substream *substream,
                else if (__snd_rawmidi_ready(runtime))
                        wake_up(&runtime->sleep);
        }
- unlock:
-       spin_unlock_irqrestore(&substream->lock, flags);
        return result;
 }
 EXPORT_SYMBOL(snd_rawmidi_receive);
@@ -1362,20 +1326,15 @@ static ssize_t snd_rawmidi_read(struct file *file, char __user *buf, size_t coun
 int snd_rawmidi_transmit_empty(struct snd_rawmidi_substream *substream)
 {
        struct snd_rawmidi_runtime *runtime;
-       int result;
-       unsigned long flags;
 
-       spin_lock_irqsave(&substream->lock, flags);
+       guard(spinlock_irqsave)(&substream->lock);
        runtime = substream->runtime;
        if (!substream->opened || !runtime || !runtime->buffer) {
                rmidi_dbg(substream->rmidi,
                          "snd_rawmidi_transmit_empty: output is not active!!!\n");
-               result = 1;
-       } else {
-               result = runtime->avail >= runtime->buffer_size;
+               return 1;
        }
-       spin_unlock_irqrestore(&substream->lock, flags);
-       return result;
+       return (runtime->avail >= runtime->buffer_size);
 }
 EXPORT_SYMBOL(snd_rawmidi_transmit_empty);
 
@@ -1449,16 +1408,10 @@ static int __snd_rawmidi_transmit_peek(struct snd_rawmidi_substream *substream,
 int snd_rawmidi_transmit_peek(struct snd_rawmidi_substream *substream,
                              unsigned char *buffer, int count)
 {
-       int result;
-       unsigned long flags;
-
-       spin_lock_irqsave(&substream->lock, flags);
+       guard(spinlock_irqsave)(&substream->lock);
        if (!substream->opened || !substream->runtime)
-               result = -EBADFD;
-       else
-               result = __snd_rawmidi_transmit_peek(substream, buffer, count);
-       spin_unlock_irqrestore(&substream->lock, flags);
-       return result;
+               return -EBADFD;
+       return __snd_rawmidi_transmit_peek(substream, buffer, count);
 }
 EXPORT_SYMBOL(snd_rawmidi_transmit_peek);
 
@@ -1505,16 +1458,10 @@ static int __snd_rawmidi_transmit_ack(struct snd_rawmidi_substream *substream,
  */
 int snd_rawmidi_transmit_ack(struct snd_rawmidi_substream *substream, int count)
 {
-       int result;
-       unsigned long flags;
-
-       spin_lock_irqsave(&substream->lock, flags);
+       guard(spinlock_irqsave)(&substream->lock);
        if (!substream->opened || !substream->runtime)
-               result = -EBADFD;
-       else
-               result = __snd_rawmidi_transmit_ack(substream, count);
-       spin_unlock_irqrestore(&substream->lock, flags);
-       return result;
+               return -EBADFD;
+       return __snd_rawmidi_transmit_ack(substream, count);
 }
 EXPORT_SYMBOL(snd_rawmidi_transmit_ack);
 
@@ -1531,21 +1478,13 @@ EXPORT_SYMBOL(snd_rawmidi_transmit_ack);
 int snd_rawmidi_transmit(struct snd_rawmidi_substream *substream,
                         unsigned char *buffer, int count)
 {
-       int result;
-       unsigned long flags;
-
-       spin_lock_irqsave(&substream->lock, flags);
+       guard(spinlock_irqsave)(&substream->lock);
        if (!substream->opened)
-               result = -EBADFD;
-       else {
-               count = __snd_rawmidi_transmit_peek(substream, buffer, count);
-               if (count <= 0)
-                       result = count;
-               else
-                       result = __snd_rawmidi_transmit_ack(substream, count);
-       }
-       spin_unlock_irqrestore(&substream->lock, flags);
-       return result;
+               return -EBADFD;
+       count = __snd_rawmidi_transmit_peek(substream, buffer, count);
+       if (count <= 0)
+               return count;
+       return __snd_rawmidi_transmit_ack(substream, count);
 }
 EXPORT_SYMBOL(snd_rawmidi_transmit);
 
@@ -1558,17 +1497,15 @@ EXPORT_SYMBOL(snd_rawmidi_transmit);
 int snd_rawmidi_proceed(struct snd_rawmidi_substream *substream)
 {
        struct snd_rawmidi_runtime *runtime;
-       unsigned long flags;
        int count = 0;
 
-       spin_lock_irqsave(&substream->lock, flags);
+       guard(spinlock_irqsave)(&substream->lock);
        runtime = substream->runtime;
        if (substream->opened && runtime &&
            runtime->avail < runtime->buffer_size) {
                count = runtime->buffer_size - runtime->avail;
                __snd_rawmidi_transmit_ack(substream, count);
        }
-       spin_unlock_irqrestore(&substream->lock, flags);
        return count;
 }
 EXPORT_SYMBOL(snd_rawmidi_proceed);
@@ -1772,7 +1709,7 @@ static void snd_rawmidi_proc_info_read(struct snd_info_entry *entry,
                            rawmidi_is_ump(rmidi) ? "UMP" : "Legacy");
        if (rmidi->ops && rmidi->ops->proc_read)
                rmidi->ops->proc_read(entry, buffer);
-       mutex_lock(&rmidi->open_mutex);
+       guard(mutex)(&rmidi->open_mutex);
        if (rmidi->info_flags & SNDRV_RAWMIDI_INFO_OUTPUT) {
                list_for_each_entry(substream,
                                    &rmidi->streams[SNDRV_RAWMIDI_STREAM_OUTPUT].substreams,
@@ -1787,10 +1724,10 @@ static void snd_rawmidi_proc_info_read(struct snd_info_entry *entry,
                                    "  Owner PID    : %d\n",
                                    pid_vnr(substream->pid));
                                runtime = substream->runtime;
-                               spin_lock_irq(&substream->lock);
-                               buffer_size = runtime->buffer_size;
-                               avail = runtime->avail;
-                               spin_unlock_irq(&substream->lock);
+                               scoped_guard(spinlock_irq, &substream->lock) {
+                                       buffer_size = runtime->buffer_size;
+                                       avail = runtime->avail;
+                               }
                                snd_iprintf(buffer,
                                    "  Mode         : %s\n"
                                    "  Buffer size  : %lu\n"
@@ -1814,11 +1751,11 @@ static void snd_rawmidi_proc_info_read(struct snd_info_entry *entry,
                                            "  Owner PID    : %d\n",
                                            pid_vnr(substream->pid));
                                runtime = substream->runtime;
-                               spin_lock_irq(&substream->lock);
-                               buffer_size = runtime->buffer_size;
-                               avail = runtime->avail;
-                               xruns = runtime->xruns;
-                               spin_unlock_irq(&substream->lock);
+                               scoped_guard(spinlock_irq, &substream->lock) {
+                                       buffer_size = runtime->buffer_size;
+                                       avail = runtime->avail;
+                                       xruns = runtime->xruns;
+                               }
                                snd_iprintf(buffer,
                                            "  Buffer size  : %lu\n"
                                            "  Avail        : %lu\n"
@@ -1835,7 +1772,6 @@ static void snd_rawmidi_proc_info_read(struct snd_info_entry *entry,
                        }
                }
        }
-       mutex_unlock(&rmidi->open_mutex);
 }
 
 /*
@@ -2024,12 +1960,12 @@ static int snd_rawmidi_dev_register(struct snd_device *device)
        if (rmidi->device >= SNDRV_RAWMIDI_DEVICES)
                return -ENOMEM;
        err = 0;
-       mutex_lock(&register_mutex);
-       if (snd_rawmidi_search(rmidi->card, rmidi->device))
-               err = -EBUSY;
-       else
-               list_add_tail(&rmidi->list, &snd_rawmidi_devices);
-       mutex_unlock(&register_mutex);
+       scoped_guard(mutex, &register_mutex) {
+               if (snd_rawmidi_search(rmidi->card, rmidi->device))
+                       err = -EBUSY;
+               else
+                       list_add_tail(&rmidi->list, &snd_rawmidi_devices);
+       }
        if (err < 0)
                return err;
 
@@ -2102,9 +2038,8 @@ static int snd_rawmidi_dev_register(struct snd_device *device)
  error_unregister:
        snd_unregister_device(rmidi->dev);
  error:
-       mutex_lock(&register_mutex);
-       list_del(&rmidi->list);
-       mutex_unlock(&register_mutex);
+       scoped_guard(mutex, &register_mutex)
+               list_del(&rmidi->list);
        return err;
 }
 
@@ -2113,8 +2048,8 @@ static int snd_rawmidi_dev_disconnect(struct snd_device *device)
        struct snd_rawmidi *rmidi = device->device_data;
        int dir;
 
-       mutex_lock(&register_mutex);
-       mutex_lock(&rmidi->open_mutex);
+       guard(mutex)(&register_mutex);
+       guard(mutex)(&rmidi->open_mutex);
        wake_up(&rmidi->open_wait);
        list_del_init(&rmidi->list);
        for (dir = 0; dir < 2; dir++) {
@@ -2140,8 +2075,6 @@ static int snd_rawmidi_dev_disconnect(struct snd_device *device)
        }
 #endif /* CONFIG_SND_OSSEMUL */
        snd_unregister_device(rmidi->dev);
-       mutex_unlock(&rmidi->open_mutex);
-       mutex_unlock(&register_mutex);
        return 0;
 }