ALSA: core: reduce stack usage related to snd_ctl_new()
authorTakashi Sakamoto <o-takashi@sakamocchi.jp>
Tue, 10 Mar 2015 13:13:31 +0000 (22:13 +0900)
committerTakashi Iwai <tiwai@suse.de>
Tue, 10 Mar 2015 14:29:59 +0000 (15:29 +0100)
The callers of snd_ctl_new() need to have 'struct snd_kcontrol' data,
and pass the data as template. Then, the function allocates the structure
data again and copy from the template. This is a waste of resources.
Especially, the callers use large stack for the template.

This commit removes a need of template for the function, thus, changes
the prototype of snd_ctl_new(). Furthermore, this commit changes
the code of callers, snd_ctl_new1() and snd_ctl_elem_add() for better
shape.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
sound/core/control.c

index 0b85cbc27e4d2eb9afa7531cdb15eb7851386171..e1d8e0c816f081c016415b5584730e884be9e4e3 100644 (file)
@@ -192,36 +192,43 @@ void snd_ctl_notify(struct snd_card *card, unsigned int mask,
 EXPORT_SYMBOL(snd_ctl_notify);
 
 /**
- * snd_ctl_new - create a control instance from the template
- * @control: the control template
- * @access: the default control access
+ * snd_ctl_new - create a new control instance with some elements
+ * @kctl: the pointer to store new control instance
+ * @count: the number of elements in this control
+ * @access: the default access flags for elements in this control
+ * @file: given when locking these elements
  *
- * Allocates a new struct snd_kcontrol instance and copies the given template 
- * to the new instance. It does not copy volatile data (access).
+ * Allocates a memory object for a new control instance. The instance has
+ * elements as many as the given number (@count). Each element has given
+ * access permissions (@access). Each element is locked when @file is given.
  *
- * Return: The pointer of the new instance, or %NULL on failure.
+ * Return: 0 on success, error code on failure
  */
-static struct snd_kcontrol *snd_ctl_new(struct snd_kcontrol *control,
-                                       unsigned int access)
+static int snd_ctl_new(struct snd_kcontrol **kctl, unsigned int count,
+                      unsigned int access, struct snd_ctl_file *file)
 {
-       struct snd_kcontrol *kctl;
+       unsigned int size;
        unsigned int idx;
        
-       if (snd_BUG_ON(!control || !control->count))
-               return NULL;
+       if (count == 0 || count > MAX_CONTROL_COUNT)
+               return -EINVAL;
 
-       if (control->count > MAX_CONTROL_COUNT)
-               return NULL;
+       size  = sizeof(struct snd_kcontrol);
+       size += sizeof(struct snd_kcontrol_volatile) * count;
 
-       kctl = kzalloc(sizeof(*kctl) + sizeof(struct snd_kcontrol_volatile) * control->count, GFP_KERNEL);
-       if (kctl == NULL) {
+       *kctl = kzalloc(size, GFP_KERNEL);
+       if (*kctl == NULL) {
                pr_err("ALSA: Cannot allocate control instance\n");
-               return NULL;
+               return -ENOMEM;
        }
-       *kctl = *control;
-       for (idx = 0; idx < kctl->count; idx++)
-               kctl->vd[idx].access = access;
-       return kctl;
+
+       for (idx = 0; idx < count; idx++) {
+               (*kctl)->vd[idx].access = access;
+               (*kctl)->vd[idx].owner = file;
+       }
+       (*kctl)->count = count;
+
+       return 0;
 }
 
 /**
@@ -238,37 +245,53 @@ static struct snd_kcontrol *snd_ctl_new(struct snd_kcontrol *control,
 struct snd_kcontrol *snd_ctl_new1(const struct snd_kcontrol_new *ncontrol,
                                  void *private_data)
 {
-       struct snd_kcontrol kctl;
+       struct snd_kcontrol *kctl;
+       unsigned int count;
        unsigned int access;
+       int err;
        
        if (snd_BUG_ON(!ncontrol || !ncontrol->info))
                return NULL;
-       memset(&kctl, 0, sizeof(kctl));
-       kctl.id.iface = ncontrol->iface;
-       kctl.id.device = ncontrol->device;
-       kctl.id.subdevice = ncontrol->subdevice;
+
+       count = ncontrol->count;
+       if (count == 0)
+               count = 1;
+
+       access = ncontrol->access;
+       if (access == 0)
+               access = SNDRV_CTL_ELEM_ACCESS_READWRITE;
+       access &= (SNDRV_CTL_ELEM_ACCESS_READWRITE |
+                  SNDRV_CTL_ELEM_ACCESS_VOLATILE |
+                  SNDRV_CTL_ELEM_ACCESS_INACTIVE |
+                  SNDRV_CTL_ELEM_ACCESS_TLV_READWRITE |
+                  SNDRV_CTL_ELEM_ACCESS_TLV_COMMAND |
+                  SNDRV_CTL_ELEM_ACCESS_TLV_CALLBACK);
+
+       err = snd_ctl_new(&kctl, count, access, NULL);
+       if (err < 0)
+               return NULL;
+
+       /* The 'numid' member is decided when calling snd_ctl_add(). */
+       kctl->id.iface = ncontrol->iface;
+       kctl->id.device = ncontrol->device;
+       kctl->id.subdevice = ncontrol->subdevice;
        if (ncontrol->name) {
-               strlcpy(kctl.id.name, ncontrol->name, sizeof(kctl.id.name));
-               if (strcmp(ncontrol->name, kctl.id.name) != 0)
+               strlcpy(kctl->id.name, ncontrol->name, sizeof(kctl->id.name));
+               if (strcmp(ncontrol->name, kctl->id.name) != 0)
                        pr_warn("ALSA: Control name '%s' truncated to '%s'\n",
-                               ncontrol->name, kctl.id.name);
+                               ncontrol->name, kctl->id.name);
        }
-       kctl.id.index = ncontrol->index;
-       kctl.count = ncontrol->count ? ncontrol->count : 1;
-       access = ncontrol->access == 0 ? SNDRV_CTL_ELEM_ACCESS_READWRITE :
-                (ncontrol->access & (SNDRV_CTL_ELEM_ACCESS_READWRITE|
-                                     SNDRV_CTL_ELEM_ACCESS_VOLATILE|
-                                     SNDRV_CTL_ELEM_ACCESS_INACTIVE|
-                                     SNDRV_CTL_ELEM_ACCESS_TLV_READWRITE|
-                                     SNDRV_CTL_ELEM_ACCESS_TLV_COMMAND|
-                                     SNDRV_CTL_ELEM_ACCESS_TLV_CALLBACK));
-       kctl.info = ncontrol->info;
-       kctl.get = ncontrol->get;
-       kctl.put = ncontrol->put;
-       kctl.tlv.p = ncontrol->tlv.p;
-       kctl.private_value = ncontrol->private_value;
-       kctl.private_data = private_data;
-       return snd_ctl_new(&kctl, access);
+       kctl->id.index = ncontrol->index;
+
+       kctl->info = ncontrol->info;
+       kctl->get = ncontrol->get;
+       kctl->put = ncontrol->put;
+       kctl->tlv.p = ncontrol->tlv.p;
+
+       kctl->private_value = ncontrol->private_value;
+       kctl->private_data = private_data;
+
+       return kctl;
 }
 EXPORT_SYMBOL(snd_ctl_new1);
 
@@ -1179,44 +1202,48 @@ static int snd_ctl_elem_add(struct snd_ctl_file *file,
                [SNDRV_CTL_ELEM_TYPE_INTEGER64] = 64,
        };
        struct snd_card *card = file->card;
-       struct snd_kcontrol kctl, *_kctl;
+       struct snd_kcontrol *kctl;
+       unsigned int count;
        unsigned int access;
        long private_size;
        struct user_element *ue;
-       int idx, err;
-
-       access = info->access == 0 ? SNDRV_CTL_ELEM_ACCESS_READWRITE :
-               (info->access & (SNDRV_CTL_ELEM_ACCESS_READWRITE|
-                                SNDRV_CTL_ELEM_ACCESS_INACTIVE|
-                                SNDRV_CTL_ELEM_ACCESS_TLV_READWRITE));
-       info->id.numid = 0;
-       memset(&kctl, 0, sizeof(kctl));
+       int err;
 
+       /* Delete a control to replace them if needed. */
        if (replace) {
+               info->id.numid = 0;
                err = snd_ctl_remove_user_ctl(file, &info->id);
                if (err)
                        return err;
        }
 
-       if (card->user_ctl_count >= MAX_USER_CONTROLS)
+       /*
+        * The number of userspace controls are counted control by control,
+        * not element by element.
+        */
+       if (card->user_ctl_count + 1 > MAX_USER_CONTROLS)
                return -ENOMEM;
 
-       memcpy(&kctl.id, &info->id, sizeof(info->id));
-       kctl.count = info->owner ? info->owner : 1;
-       access |= SNDRV_CTL_ELEM_ACCESS_USER;
-       if (info->type == SNDRV_CTL_ELEM_TYPE_ENUMERATED)
-               kctl.info = snd_ctl_elem_user_enum_info;
-       else
-               kctl.info = snd_ctl_elem_user_info;
-       if (access & SNDRV_CTL_ELEM_ACCESS_READ)
-               kctl.get = snd_ctl_elem_user_get;
-       if (access & SNDRV_CTL_ELEM_ACCESS_WRITE)
-               kctl.put = snd_ctl_elem_user_put;
-       if (access & SNDRV_CTL_ELEM_ACCESS_TLV_READWRITE) {
-               kctl.tlv.c = snd_ctl_elem_user_tlv;
+       /* Check the number of elements for this userspace control. */
+       count = info->owner;
+       if (count == 0)
+               count = 1;
+
+       /* Arrange access permissions if needed. */
+       access = info->access;
+       if (access == 0)
+               access = SNDRV_CTL_ELEM_ACCESS_READWRITE;
+       access &= (SNDRV_CTL_ELEM_ACCESS_READWRITE |
+                  SNDRV_CTL_ELEM_ACCESS_INACTIVE |
+                  SNDRV_CTL_ELEM_ACCESS_TLV_READWRITE);
+       if (access & SNDRV_CTL_ELEM_ACCESS_TLV_READWRITE)
                access |= SNDRV_CTL_ELEM_ACCESS_TLV_CALLBACK;
-       }
+       access |= SNDRV_CTL_ELEM_ACCESS_USER;
 
+       /*
+        * Check information and calculate the size of data specific to
+        * this userspace control.
+        */
        if (info->type < SNDRV_CTL_ELEM_TYPE_BOOLEAN ||
            info->type > SNDRV_CTL_ELEM_TYPE_INTEGER64)
                return -EINVAL;
@@ -1226,11 +1253,27 @@ static int snd_ctl_elem_add(struct snd_ctl_file *file,
        if (info->count < 1 ||
            info->count > max_value_counts[info->type])
                return -EINVAL;
-
        private_size = value_sizes[info->type] * info->count;
-       ue = kzalloc(sizeof(struct user_element) + private_size, GFP_KERNEL);
-       if (ue == NULL)
+
+       /*
+        * Keep memory object for this userspace control. After passing this
+        * code block, the instance should be freed by snd_ctl_free_one().
+        *
+        * Note that these elements in this control are locked.
+        */
+       err = snd_ctl_new(&kctl, count, access, file);
+       if (err < 0)
+               return err;
+       kctl->private_data = kzalloc(sizeof(struct user_element) + private_size,
+                                    GFP_KERNEL);
+       if (kctl->private_data == NULL) {
+               kfree(kctl);
                return -ENOMEM;
+       }
+       kctl->private_free = snd_ctl_elem_user_free;
+
+       /* Set private data for this userspace control. */
+       ue = (struct user_element *)kctl->private_data;
        ue->card = card;
        ue->info = *info;
        ue->info.access = 0;
@@ -1239,21 +1282,25 @@ static int snd_ctl_elem_add(struct snd_ctl_file *file,
        if (ue->info.type == SNDRV_CTL_ELEM_TYPE_ENUMERATED) {
                err = snd_ctl_elem_init_enum_names(ue);
                if (err < 0) {
-                       kfree(ue);
+                       snd_ctl_free_one(kctl);
                        return err;
                }
        }
-       kctl.private_free = snd_ctl_elem_user_free;
-       _kctl = snd_ctl_new(&kctl, access);
-       if (_kctl == NULL) {
-               kfree(ue->priv_data);
-               kfree(ue);
-               return -ENOMEM;
-       }
-       _kctl->private_data = ue;
-       for (idx = 0; idx < _kctl->count; idx++)
-               _kctl->vd[idx].owner = file;
-       err = snd_ctl_add(card, _kctl);
+
+       /* Set callback functions. */
+       if (info->type == SNDRV_CTL_ELEM_TYPE_ENUMERATED)
+               kctl->info = snd_ctl_elem_user_enum_info;
+       else
+               kctl->info = snd_ctl_elem_user_info;
+       if (access & SNDRV_CTL_ELEM_ACCESS_READ)
+               kctl->get = snd_ctl_elem_user_get;
+       if (access & SNDRV_CTL_ELEM_ACCESS_WRITE)
+               kctl->put = snd_ctl_elem_user_put;
+       if (access & SNDRV_CTL_ELEM_ACCESS_TLV_READWRITE)
+               kctl->tlv.c = snd_ctl_elem_user_tlv;
+
+       /* This function manage to free the instance on failure. */
+       err = snd_ctl_add(card, kctl);
        if (err < 0)
                return err;