Commit 84bb065b authored by Takashi Iwai's avatar Takashi Iwai
Browse files

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: default avatarTakashi Iwai <tiwai@suse.de>
Link: https://lore.kernel.org/r/20240227085306.9764-10-tiwai@suse.de
parent 471be437

sound/core/rawmidi.c

+93 −160
Original line number Diff line number Diff line
@@ -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,23 +252,19 @@ int snd_rawmidi_drain_output(struct snd_rawmidi_substream *substream) 
	long timeout;

	struct snd_rawmidi_runtime *runtime;



	spin_lock_irq(&substream->lock);

	scoped_guard(spinlock_irq, &substream->lock) {

		runtime = substream->runtime;

	if (!substream->opened || !runtime || !runtime->buffer) {

		err = -EINVAL;

	} else {

		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);

	scoped_guard(spinlock_irq, &substream->lock) {

		if (signal_pending(current))

			err = -ERESTARTSYS;

		if (runtime->avail < runtime->buffer_size && !timeout) {
@@ -286,7 +274,7 @@ int snd_rawmidi_drain_output(struct snd_rawmidi_substream *substream) 
			err = -EIO;

		}

		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);

	scoped_guard(spinlock_irq, &substream->lock)

		snd_rawmidi_buffer_unref(runtime);

	spin_unlock_irq(&substream->lock);



	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);

	scoped_guard(spinlock_irq, &substream->lock) {

		substream->opened = 0;

		substream->append = 0;

	spin_unlock_irq(&substream->lock);

	}

	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

		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,7 +997,7 @@ 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);

	scoped_guard(mutex, &register_mutex) {

		device = device < 0 ? 0 : device + 1;

		for (; device < SNDRV_RAWMIDI_DEVICES; device++) {

			rmidi = snd_rawmidi_search(card, device);
@@ -1037,7 +1009,7 @@ static int snd_rawmidi_next_device(struct snd_card *card, int __user *argp, 
		}

		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 {

		return -EBADFD;

	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 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);

				scoped_guard(spinlock_irq, &substream->lock) {

					buffer_size = runtime->buffer_size;

					avail = runtime->avail;

				spin_unlock_irq(&substream->lock);

				}

				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);

				scoped_guard(spinlock_irq, &substream->lock) {

					buffer_size = runtime->buffer_size;

					avail = runtime->avail;

					xruns = runtime->xruns;

				spin_unlock_irq(&substream->lock);

				}

				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);

	scoped_guard(mutex, &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);

	}

	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);

	scoped_guard(mutex, &register_mutex)

		list_del(&rmidi->list);

	mutex_unlock(&register_mutex);

	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;

}