From 84bb065b316e8367e14a8824a8f4d21056b10c53 Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Tue, 27 Feb 2024 09:52:51 +0100 Subject: [PATCH] ALSA: rawmidi: Use guard() for locking 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 Link: https://lore.kernel.org/r/20240227085306.9764-10-tiwai@suse.de --- sound/core/rawmidi.c | 253 ++++++++++++++++--------------------------- 1 file changed, 93 insertions(+), 160 deletions(-) diff --git a/sound/core/rawmidi.c b/sound/core/rawmidi.c index 1431cb997808..7accf9a1ddf4 100644 --- a/sound/core/rawmidi.c +++ b/sound/core/rawmidi.c @@ -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(®ister_mutex); - ret = __snd_rawmidi_info_select(card, info); - mutex_unlock(®ister_mutex); - return ret; + guard(mutex)(®ister_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(®ister_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, ®ister_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(®ister_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(®ister_mutex); + guard(mutex)(®ister_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(®ister_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(®ister_mutex); - if (snd_rawmidi_search(rmidi->card, rmidi->device)) - err = -EBUSY; - else - list_add_tail(&rmidi->list, &snd_rawmidi_devices); - mutex_unlock(®ister_mutex); + scoped_guard(mutex, ®ister_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(®ister_mutex); - list_del(&rmidi->list); - mutex_unlock(®ister_mutex); + scoped_guard(mutex, ®ister_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(®ister_mutex); - mutex_lock(&rmidi->open_mutex); + guard(mutex)(®ister_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(®ister_mutex); return 0; }