]> www.infradead.org Git - users/hch/misc.git/commitdiff
ALSA: seq: Clean up queue locking with auto cleanup
authorTakashi Iwai <tiwai@suse.de>
Wed, 27 Aug 2025 08:05:12 +0000 (10:05 +0200)
committerTakashi Iwai <tiwai@suse.de>
Fri, 29 Aug 2025 09:52:35 +0000 (11:52 +0200)
Yet more cleanup with the auto-cleanup macro: now we replace the
queuefree() calls with the magic pointer attribute
__free(snd_seq_queue).

Only code refactoring, and no behavior change.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
Link: https://patch.msgid.link/20250827080520.7544-7-tiwai@suse.de
sound/core/seq/seq_clientmgr.c
sound/core/seq/seq_queue.c
sound/core/seq/seq_queue.h
sound/core/seq/seq_timer.c

index 18424f56251a16c7050f10d58e92879ca90adc2e..38ad5bbd270695f70002d64820a95a810ab1c483 100644 (file)
@@ -566,7 +566,7 @@ static int bounce_error_event(struct snd_seq_client *client,
 static int update_timestamp_of_queue(struct snd_seq_event *event,
                                     int queue, int real_time)
 {
-       struct snd_seq_queue *q;
+       struct snd_seq_queue *q __free(snd_seq_queue) = NULL;
 
        q = queueptr(queue);
        if (! q)
@@ -580,7 +580,6 @@ static int update_timestamp_of_queue(struct snd_seq_event *event,
                event->time.tick = snd_seq_timer_get_cur_tick(q->timer);
                event->flags |= SNDRV_SEQ_TIME_STAMP_TICK;
        }
-       queuefree(q);
        return 1;
 }
 
@@ -1520,7 +1519,7 @@ static int snd_seq_ioctl_unsubscribe_port(struct snd_seq_client *client,
 static int snd_seq_ioctl_create_queue(struct snd_seq_client *client, void *arg)
 {
        struct snd_seq_queue_info *info = arg;
-       struct snd_seq_queue *q;
+       struct snd_seq_queue *q __free(snd_seq_queue) = NULL;
 
        q = snd_seq_queue_alloc(client->number, info->locked, info->flags);
        if (IS_ERR(q))
@@ -1534,7 +1533,6 @@ static int snd_seq_ioctl_create_queue(struct snd_seq_client *client, void *arg)
        if (!info->name[0])
                snprintf(info->name, sizeof(info->name), "Queue-%d", q->queue);
        strscpy(q->name, info->name, sizeof(q->name));
-       snd_use_lock_free(&q->use_lock);
 
        return 0;
 }
@@ -1552,7 +1550,7 @@ static int snd_seq_ioctl_get_queue_info(struct snd_seq_client *client,
                                        void *arg)
 {
        struct snd_seq_queue_info *info = arg;
-       struct snd_seq_queue *q;
+       struct snd_seq_queue *q __free(snd_seq_queue) = NULL;
 
        q = queueptr(info->queue);
        if (q == NULL)
@@ -1563,7 +1561,6 @@ static int snd_seq_ioctl_get_queue_info(struct snd_seq_client *client,
        info->owner = q->owner;
        info->locked = q->locked;
        strscpy(info->name, q->name, sizeof(info->name));
-       queuefree(q);
 
        return 0;
 }
@@ -1573,7 +1570,7 @@ static int snd_seq_ioctl_set_queue_info(struct snd_seq_client *client,
                                        void *arg)
 {
        struct snd_seq_queue_info *info = arg;
-       struct snd_seq_queue *q;
+       struct snd_seq_queue *q __free(snd_seq_queue) = NULL;
 
        if (info->owner != client->number)
                return -EINVAL;
@@ -1591,12 +1588,9 @@ static int snd_seq_ioctl_set_queue_info(struct snd_seq_client *client,
        q = queueptr(info->queue);
        if (! q)
                return -EINVAL;
-       if (q->owner != client->number) {
-               queuefree(q);
+       if (q->owner != client->number)
                return -EPERM;
-       }
        strscpy(q->name, info->name, sizeof(q->name));
-       queuefree(q);
 
        return 0;
 }
@@ -1606,7 +1600,7 @@ static int snd_seq_ioctl_get_named_queue(struct snd_seq_client *client,
                                         void *arg)
 {
        struct snd_seq_queue_info *info = arg;
-       struct snd_seq_queue *q;
+       struct snd_seq_queue *q __free(snd_seq_queue) = NULL;
 
        q = snd_seq_queue_find_name(info->name);
        if (q == NULL)
@@ -1614,7 +1608,6 @@ static int snd_seq_ioctl_get_named_queue(struct snd_seq_client *client,
        info->queue = q->queue;
        info->owner = q->owner;
        info->locked = q->locked;
-       queuefree(q);
 
        return 0;
 }
@@ -1624,7 +1617,7 @@ static int snd_seq_ioctl_get_queue_status(struct snd_seq_client *client,
                                          void *arg)
 {
        struct snd_seq_queue_status *status = arg;
-       struct snd_seq_queue *queue;
+       struct snd_seq_queue *queue __free(snd_seq_queue) = NULL;
        struct snd_seq_timer *tmr;
 
        queue = queueptr(status->queue);
@@ -1642,7 +1635,6 @@ static int snd_seq_ioctl_get_queue_status(struct snd_seq_client *client,
        status->running = tmr->running;
 
        status->flags = queue->flags;
-       queuefree(queue);
 
        return 0;
 }
@@ -1653,7 +1645,7 @@ static int snd_seq_ioctl_get_queue_tempo(struct snd_seq_client *client,
                                         void *arg)
 {
        struct snd_seq_queue_tempo *tempo = arg;
-       struct snd_seq_queue *queue;
+       struct snd_seq_queue *queue __free(snd_seq_queue) = NULL;
        struct snd_seq_timer *tmr;
 
        queue = queueptr(tempo->queue);
@@ -1670,7 +1662,6 @@ static int snd_seq_ioctl_get_queue_tempo(struct snd_seq_client *client,
        tempo->skew_base = tmr->skew_base;
        if (client->user_pversion >= SNDRV_PROTOCOL_VERSION(1, 0, 4))
                tempo->tempo_base = tmr->tempo_base;
-       queuefree(queue);
 
        return 0;
 }
@@ -1703,14 +1694,14 @@ static int snd_seq_ioctl_get_queue_timer(struct snd_seq_client *client,
                                         void *arg)
 {
        struct snd_seq_queue_timer *timer = arg;
-       struct snd_seq_queue *queue;
+       struct snd_seq_queue *queue __free(snd_seq_queue) = NULL;
        struct snd_seq_timer *tmr;
 
        queue = queueptr(timer->queue);
        if (queue == NULL)
                return -EINVAL;
 
-       mutex_lock(&queue->timer_mutex);
+       guard(mutex)(&queue->timer_mutex);
        tmr = queue->timer;
        memset(timer, 0, sizeof(*timer));
        timer->queue = queue->queue;
@@ -1720,8 +1711,6 @@ static int snd_seq_ioctl_get_queue_timer(struct snd_seq_client *client,
                timer->u.alsa.id = tmr->alsa_id;
                timer->u.alsa.resolution = tmr->preferred_resolution;
        }
-       mutex_unlock(&queue->timer_mutex);
-       queuefree(queue);
        
        return 0;
 }
@@ -1738,13 +1727,13 @@ static int snd_seq_ioctl_set_queue_timer(struct snd_seq_client *client,
                return -EINVAL;
 
        if (snd_seq_queue_check_access(timer->queue, client->number)) {
-               struct snd_seq_queue *q;
+               struct snd_seq_queue *q __free(snd_seq_queue) = NULL;
                struct snd_seq_timer *tmr;
 
                q = queueptr(timer->queue);
                if (q == NULL)
                        return -ENXIO;
-               mutex_lock(&q->timer_mutex);
+               guard(mutex)(&q->timer_mutex);
                tmr = q->timer;
                snd_seq_queue_timer_close(timer->queue);
                tmr->type = timer->type;
@@ -1753,8 +1742,6 @@ static int snd_seq_ioctl_set_queue_timer(struct snd_seq_client *client,
                        tmr->preferred_resolution = timer->u.alsa.resolution;
                }
                result = snd_seq_queue_timer_open(timer->queue);
-               mutex_unlock(&q->timer_mutex);
-               queuefree(q);
        } else {
                return -EPERM;
        }       
index 10add922323dab0ec055857d608c74b53756b5b6..f5c0e401c8ae56e0d8e1d64a1a4a2d1ab5387d27 100644 (file)
@@ -209,14 +209,13 @@ struct snd_seq_queue *queueptr(int queueid)
 struct snd_seq_queue *snd_seq_queue_find_name(char *name)
 {
        int i;
-       struct snd_seq_queue *q;
 
        for (i = 0; i < SNDRV_SEQ_MAX_QUEUES; i++) {
+               struct snd_seq_queue *q __free(snd_seq_queue) = NULL;
                q = queueptr(i);
                if (q) {
                        if (strncmp(q->name, name, sizeof(q->name)) == 0)
-                               return q;
-                       queuefree(q);
+                               return no_free_ptr(q);
                }
        }
        return NULL;
@@ -286,7 +285,7 @@ void snd_seq_check_queue(struct snd_seq_queue *q, int atomic, int hop)
 int snd_seq_enqueue_event(struct snd_seq_event_cell *cell, int atomic, int hop)
 {
        int dest, err;
-       struct snd_seq_queue *q;
+       struct snd_seq_queue *q __free(snd_seq_queue) = NULL;
 
        if (snd_BUG_ON(!cell))
                return -EINVAL;
@@ -321,16 +320,12 @@ int snd_seq_enqueue_event(struct snd_seq_event_cell *cell, int atomic, int hop)
                break;
        }
 
-       if (err < 0) {
-               queuefree(q); /* unlock */
+       if (err < 0)
                return err;
-       }
 
        /* trigger dispatching */
        snd_seq_check_queue(q, atomic, hop);
 
-       queuefree(q); /* unlock */
-
        return 0;
 }
 
@@ -366,15 +361,12 @@ static inline void queue_access_unlock(struct snd_seq_queue *q)
 /* exported - only checking permission */
 int snd_seq_queue_check_access(int queueid, int client)
 {
-       struct snd_seq_queue *q = queueptr(queueid);
-       int access_ok;
+       struct snd_seq_queue *q __free(snd_seq_queue) = queueptr(queueid);
 
        if (! q)
                return 0;
-       scoped_guard(spinlock_irqsave, &q->owner_lock)
-               access_ok = check_access(q, client);
-       queuefree(q);
-       return access_ok;
+       guard(spinlock_irqsave)(&q->owner_lock);
+       return check_access(q, client);
 }
 
 /*----------------------------------------------------------------*/
@@ -384,22 +376,19 @@ int snd_seq_queue_check_access(int queueid, int client)
  */
 int snd_seq_queue_set_owner(int queueid, int client, int locked)
 {
-       struct snd_seq_queue *q = queueptr(queueid);
+       struct snd_seq_queue *q __free(snd_seq_queue) = queueptr(queueid);
 
        if (q == NULL)
                return -EINVAL;
 
-       if (! queue_access_lock(q, client)) {
-               queuefree(q);
+       if (!queue_access_lock(q, client))
                return -EPERM;
-       }
 
        scoped_guard(spinlock_irqsave, &q->owner_lock) {
                q->locked = locked ? 1 : 0;
                q->owner = client;
        }
        queue_access_unlock(q);
-       queuefree(q);
 
        return 0;
 }
@@ -414,7 +403,7 @@ int snd_seq_queue_set_owner(int queueid, int client, int locked)
 int snd_seq_queue_timer_open(int queueid)
 {
        int result = 0;
-       struct snd_seq_queue *queue;
+       struct snd_seq_queue *queue __free(snd_seq_queue) = NULL;
        struct snd_seq_timer *tmr;
 
        queue = queueptr(queueid);
@@ -426,7 +415,6 @@ int snd_seq_queue_timer_open(int queueid)
                snd_seq_timer_defaults(tmr);
                result = snd_seq_timer_open(queue);
        }
-       queuefree(queue);
        return result;
 }
 
@@ -435,14 +423,13 @@ int snd_seq_queue_timer_open(int queueid)
  */
 int snd_seq_queue_timer_close(int queueid)
 {
-       struct snd_seq_queue *queue;
+       struct snd_seq_queue *queue __free(snd_seq_queue) = NULL;
        int result = 0;
 
        queue = queueptr(queueid);
        if (queue == NULL)
                return -EINVAL;
        snd_seq_timer_close(queue);
-       queuefree(queue);
        return result;
 }
 
@@ -450,15 +437,13 @@ int snd_seq_queue_timer_close(int queueid)
 int snd_seq_queue_timer_set_tempo(int queueid, int client,
                                  struct snd_seq_queue_tempo *info)
 {
-       struct snd_seq_queue *q = queueptr(queueid);
+       struct snd_seq_queue *q __free(snd_seq_queue) = queueptr(queueid);
        int result;
 
        if (q == NULL)
                return -EINVAL;
-       if (! queue_access_lock(q, client)) {
-               queuefree(q);
+       if (!queue_access_lock(q, client))
                return -EPERM;
-       }
 
        result = snd_seq_timer_set_tempo_ppq(q->timer, info->tempo, info->ppq,
                                             info->tempo_base);
@@ -466,7 +451,6 @@ int snd_seq_queue_timer_set_tempo(int queueid, int client,
                result = snd_seq_timer_set_skew(q->timer, info->skew_value,
                                                info->skew_base);
        queue_access_unlock(q);
-       queuefree(q);
        return result;
 }
 
@@ -495,15 +479,13 @@ static void queue_use(struct snd_seq_queue *queue, int client, int use)
  */
 int snd_seq_queue_use(int queueid, int client, int use)
 {
-       struct snd_seq_queue *queue;
+       struct snd_seq_queue *queue __free(snd_seq_queue) = NULL;
 
        queue = queueptr(queueid);
        if (queue == NULL)
                return -EINVAL;
-       mutex_lock(&queue->timer_mutex);
+       guard(mutex)(&queue->timer_mutex);
        queue_use(queue, client, use);
-       mutex_unlock(&queue->timer_mutex);
-       queuefree(queue);
        return 0;
 }
 
@@ -514,15 +496,12 @@ int snd_seq_queue_use(int queueid, int client, int use)
  */
 int snd_seq_queue_is_used(int queueid, int client)
 {
-       struct snd_seq_queue *q;
-       int result;
+       struct snd_seq_queue *q __free(snd_seq_queue) = NULL;
 
        q = queueptr(queueid);
        if (q == NULL)
                return -EINVAL; /* invalid queue */
-       result = test_bit(client, q->clients_bitmap) ? 1 : 0;
-       queuefree(q);
-       return result;
+       return test_bit(client, q->clients_bitmap) ? 1 : 0;
 }
 
 
@@ -535,11 +514,10 @@ int snd_seq_queue_is_used(int queueid, int client)
 void snd_seq_queue_client_leave(int client)
 {
        int i;
-       struct snd_seq_queue *q;
 
        /* delete own queues from queue list */
        for (i = 0; i < SNDRV_SEQ_MAX_QUEUES; i++) {
-               q = queue_list_remove(i, client);
+               struct snd_seq_queue *q = queue_list_remove(i, client);
                if (q)
                        queue_delete(q);
        }
@@ -548,7 +526,7 @@ void snd_seq_queue_client_leave(int client)
         * they are not owned by this client
         */
        for (i = 0; i < SNDRV_SEQ_MAX_QUEUES; i++) {
-               q = queueptr(i);
+               struct snd_seq_queue *q __free(snd_seq_queue) = queueptr(i);
                if (!q)
                        continue;
                if (test_bit(client, q->clients_bitmap)) {
@@ -556,7 +534,6 @@ void snd_seq_queue_client_leave(int client)
                        snd_seq_prioq_leave(q->timeq, client, 0);
                        snd_seq_queue_use(q->queue, client, 0);
                }
-               queuefree(q);
        }
 }
 
@@ -568,10 +545,9 @@ void snd_seq_queue_client_leave(int client)
 void snd_seq_queue_remove_cells(int client, struct snd_seq_remove_events *info)
 {
        int i;
-       struct snd_seq_queue *q;
 
        for (i = 0; i < SNDRV_SEQ_MAX_QUEUES; i++) {
-               q = queueptr(i);
+               struct snd_seq_queue *q __free(snd_seq_queue) = queueptr(i);
                if (!q)
                        continue;
                if (test_bit(client, q->clients_bitmap) &&
@@ -580,7 +556,6 @@ void snd_seq_queue_remove_cells(int client, struct snd_seq_remove_events *info)
                        snd_seq_prioq_remove_events(q->tickq, client, info);
                        snd_seq_prioq_remove_events(q->timeq, client, info);
                }
-               queuefree(q);
        }
 }
 
@@ -667,7 +642,7 @@ static void snd_seq_queue_process_event(struct snd_seq_queue *q,
  */
 int snd_seq_control_queue(struct snd_seq_event *ev, int atomic, int hop)
 {
-       struct snd_seq_queue *q;
+       struct snd_seq_queue *q __free(snd_seq_queue) = NULL;
 
        if (snd_BUG_ON(!ev))
                return -EINVAL;
@@ -676,15 +651,12 @@ int snd_seq_control_queue(struct snd_seq_event *ev, int atomic, int hop)
        if (q == NULL)
                return -EINVAL;
 
-       if (! queue_access_lock(q, ev->source.client)) {
-               queuefree(q);
+       if (!queue_access_lock(q, ev->source.client))
                return -EPERM;
-       }
 
        snd_seq_queue_process_event(q, ev, atomic, hop);
 
        queue_access_unlock(q);
-       queuefree(q);
        return 0;
 }
 
@@ -697,13 +669,12 @@ void snd_seq_info_queues_read(struct snd_info_entry *entry,
                              struct snd_info_buffer *buffer)
 {
        int i, bpm;
-       struct snd_seq_queue *q;
        struct snd_seq_timer *tmr;
        bool locked;
        int owner;
 
        for (i = 0; i < SNDRV_SEQ_MAX_QUEUES; i++) {
-               q = queueptr(i);
+               struct snd_seq_queue *q __free(snd_seq_queue) = queueptr(i);
                if (!q)
                        continue;
 
@@ -731,7 +702,6 @@ void snd_seq_info_queues_read(struct snd_info_entry *entry,
                snd_iprintf(buffer, "current time       : %d.%09d s\n", tmr->cur_time.tv_sec, tmr->cur_time.tv_nsec);
                snd_iprintf(buffer, "current tick       : %d\n", tmr->tick.cur_tick);
                snd_iprintf(buffer, "\n");
-               queuefree(q);
        }
 }
 #endif /* CONFIG_SND_PROC_FS */
index b81379c9af43e6ac9c099944c0d29a29881effe2..afcd3c5484a6316fe73ab927e7ac4b2c92484403 100644 (file)
@@ -73,6 +73,8 @@ struct snd_seq_queue *queueptr(int queueid);
 /* unlock */
 #define queuefree(q) snd_use_lock_free(&(q)->use_lock)
 
+DEFINE_FREE(snd_seq_queue, struct snd_seq_queue *, if (!IS_ERR_OR_NULL(_T)) queuefree(_T))
+
 /* return the (first) queue matching with the specified name */
 struct snd_seq_queue *snd_seq_queue_find_name(char *name);
 
index c9f0392ac7f156cc6bfeb1f70721baa3f9193bd7..29b018a212fc3102374e934e37242bae7f87cdce 100644 (file)
@@ -440,13 +440,13 @@ void snd_seq_info_timer_read(struct snd_info_entry *entry,
                             struct snd_info_buffer *buffer)
 {
        int idx;
-       struct snd_seq_queue *q;
        struct snd_seq_timer *tmr;
        struct snd_timer_instance *ti;
        unsigned long resolution;
        
        for (idx = 0; idx < SNDRV_SEQ_MAX_QUEUES; idx++) {
-               q = queueptr(idx);
+               struct snd_seq_queue *q __free(snd_seq_queue) = queueptr(idx);
+
                if (q == NULL)
                        continue;
                scoped_guard(mutex, &q->timer_mutex) {
@@ -461,7 +461,6 @@ void snd_seq_info_timer_read(struct snd_info_entry *entry,
                        snd_iprintf(buffer, "  Period time : %lu.%09lu\n", resolution / 1000000000, resolution % 1000000000);
                        snd_iprintf(buffer, "  Skew : %u / %u\n", tmr->skew, tmr->skew_base);
                }
-               queuefree(q);
        }
 }
 #endif /* CONFIG_SND_PROC_FS */