From patchwork Wed Feb 11 10:40:13 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Takashi Sakamoto X-Patchwork-Id: 5812061 Return-Path: X-Original-To: patchwork-alsa-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id A3D35BF440 for ; Wed, 11 Feb 2015 10:48:15 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 7882220109 for ; Wed, 11 Feb 2015 10:48:14 +0000 (UTC) Received: from alsa0.perex.cz (alsa0.perex.cz [77.48.224.243]) by mail.kernel.org (Postfix) with ESMTP id F35A920225 for ; Wed, 11 Feb 2015 10:48:12 +0000 (UTC) Received: by alsa0.perex.cz (Postfix, from userid 1000) id E36C42650AF; Wed, 11 Feb 2015 11:48:11 +0100 (CET) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Spam-Level: X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 Received: from alsa0.perex.cz (localhost [IPv6:::1]) by alsa0.perex.cz (Postfix) with ESMTP id C5BCA261B03; Wed, 11 Feb 2015 11:43:56 +0100 (CET) X-Original-To: alsa-devel@alsa-project.org Delivered-To: alsa-devel@alsa-project.org Received: by alsa0.perex.cz (Postfix, from userid 1000) id F0FD3261AB7; Wed, 11 Feb 2015 11:43:52 +0100 (CET) Received: from smtp311.phy.lolipop.jp (smtp311.phy.lolipop.jp [210.157.22.79]) by alsa0.perex.cz (Postfix) with ESMTP id 15B2E264FF0 for ; Wed, 11 Feb 2015 11:40:23 +0100 (CET) Received: from smtp311.phy.lolipop.lan (HELO smtp311.phy.lolipop.jp) (172.17.1.11) (smtp-auth username m12129643-o-takashi, mechanism plain) by smtp311.phy.lolipop.jp (qpsmtpd/0.82) with ESMTPA; Wed, 11 Feb 2015 19:40:19 +0900 Received: from 127.0.0.1 (127.0.0.1) by smtp311.phy.lolipop.jp (LOLIPOP-Fsecure); Wed, 11 Feb 2015 19:40:13 +0900 (JST) X-Virus-Status: clean(LOLIPOP-Fsecure) From: Takashi Sakamoto To: clemens@ladisch.de, tiwai@suse.de Date: Wed, 11 Feb 2015 19:40:13 +0900 Message-Id: <1423651213-19829-6-git-send-email-o-takashi@sakamocchi.jp> X-Mailer: git-send-email 2.1.0 In-Reply-To: <1423651213-19829-1-git-send-email-o-takashi@sakamocchi.jp> References: <1423651213-19829-1-git-send-email-o-takashi@sakamocchi.jp> Cc: alsa-devel@alsa-project.org Subject: [alsa-devel] [PATCH 5/5] ALSA: control: save stack usage at creating new instance X-BeenThere: alsa-devel@alsa-project.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "Alsa-devel mailing list for ALSA developers - http://www.alsa-project.org" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: alsa-devel-bounces@alsa-project.org Sender: alsa-devel-bounces@alsa-project.org X-Virus-Scanned: ClamAV using ClamSMTP When creating a new instance of control, both of userspace or kernel driver uses stack for a instance of kernel control structure with parameters, then keep memory object and copy the instance. This way is a waste of stack. This commit change to keep memory object at first, then copy the parameters to the object. These items are applied: * change prototype of ctl_new() with new parameters * call ctl_new() at first and get memory objects * then set identical info and callback by callers's side Signed-off-by: Takashi Sakamoto --- sound/core/control.c | 198 ++++++++++++++++++++++++++++----------------------- 1 file changed, 107 insertions(+), 91 deletions(-) diff --git a/sound/core/control.c b/sound/core/control.c index 9944d75..e7fabbb 100644 --- a/sound/core/control.c +++ b/sound/core/control.c @@ -193,30 +193,35 @@ void snd_ctl_notify(struct snd_card *card, unsigned int mask, } EXPORT_SYMBOL(snd_ctl_notify); -static struct snd_kcontrol *ctl_new(struct snd_kcontrol *control, - unsigned int access) +static int ctl_new(struct snd_kcontrol **kctl, unsigned int count, + unsigned int access, unsigned int private_size) { - struct snd_kcontrol *kctl; unsigned int size; unsigned int i; - if (snd_BUG_ON(!control || !control->count)) - return NULL; + if (count == 0 || count > MAX_VALUES_COUNT) + return -EINVAL; - if (control->count > MAX_VALUES_COUNT) - return NULL; + size = sizeof(struct snd_kcontrol); + size += sizeof(struct snd_kcontrol_volatile) * count; - size = sizeof(*kctl); - size += sizeof(struct snd_kcontrol_volatile) * control->count; - kctl = kzalloc(size, GFP_KERNEL); - if (kctl == NULL) { - pr_err("ALSA: Cannot allocate control instance\n"); - return NULL; + *kctl = kzalloc(size, GFP_KERNEL); + if (*kctl == NULL) + return -ENOMEM; + + if (private_size > 0) { + (*kctl)->private_data = kzalloc(private_size, GFP_KERNEL); + if ((*kctl)->private_data == NULL) { + kfree(*kctl); + return -ENOMEM; + } } - *kctl = *control; - for (i = 0; i < kctl->count; i++) - kctl->vd[i].access = access; - return kctl; + + for (i = 0; i < count; i++) + (*kctl)->vd[i].access = access; + (*kctl)->count = count; + + return 0; } /** @@ -233,37 +238,52 @@ static struct snd_kcontrol *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; 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; + + if (ctl_new(&kctl, count, access, 0) < 0) { + pr_err("ALSA: Cannot allocate control instance\n"); + return NULL; + } + + kctl->id.iface = ncontrol->iface; + kctl->id.device = ncontrol->device; + kctl->id.subdevice = ncontrol->subdevice; + kctl->id.index = ncontrol->index; 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 ctl_new(&kctl, access); + + 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); @@ -1175,14 +1195,29 @@ 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; unsigned int elem_data_size; struct user_element *ue; int i, err; - if (info->count < 1) + if (info->type < SNDRV_CTL_ELEM_TYPE_BOOLEAN || + info->type > SNDRV_CTL_ELEM_TYPE_INTEGER64) + return -EINVAL; + if (info->count < 1 || info->count > max_value_counts[info->type]) return -EINVAL; + if (info->type == SNDRV_CTL_ELEM_TYPE_ENUMERATED && + info->value.enumerated.items == 0) + return -EINVAL; + elem_data_size = value_sizes[info->type] * info->count; + + /* + * The number of controls with the same feature, distinguished by index. + */ + count = info->owner; + if (count == 0) + count = 1; access = info->access; if (access == 0) @@ -1194,48 +1229,36 @@ static int snd_ctl_elem_add(struct snd_ctl_file *file, access |= SNDRV_CTL_ELEM_ACCESS_TLV_CALLBACK; access |= SNDRV_CTL_ELEM_ACCESS_USER; - memset(&kctl, 0, sizeof(kctl)); - kctl.id = info->id; - kctl.id.numid = 0; + err = ctl_new(&kctl, count, access, + sizeof(struct user_element) + elem_data_size); + if (err < 0) + return err; + kctl->private_free = snd_ctl_elem_user_free; + kctl->id = info->id; if (replace) { - err = snd_ctl_remove_user_ctl(file, &kctl.id); - if (err) - return err; + err = snd_ctl_remove_user_ctl(file, &kctl->id); + if (err < 0) + goto error_allocated; } - /* - * The number of controls with the same feature, distinguished by index. - */ - kctl.count = info->owner; - if (kctl.count == 0) - kctl.count = 1; - if (card->user_ctl_count + kctl.count > MAX_USER_CONTROLS) - return -ENOSPC; + if (card->user_ctl_count + kctl->count > MAX_USER_CONTROLS) { + err = -ENOSPC; + goto error_allocated; + } if (info->type == SNDRV_CTL_ELEM_TYPE_ENUMERATED) - kctl.info = snd_ctl_elem_user_enum_info; + kctl->info = snd_ctl_elem_user_enum_info; else - kctl.info = snd_ctl_elem_user_info; + kctl->info = snd_ctl_elem_user_info; if (access & SNDRV_CTL_ELEM_ACCESS_READ) - kctl.get = snd_ctl_elem_user_get; + kctl->get = snd_ctl_elem_user_get; if (access & SNDRV_CTL_ELEM_ACCESS_WRITE) - kctl.put = snd_ctl_elem_user_put; + kctl->put = snd_ctl_elem_user_put; if (access & SNDRV_CTL_ELEM_ACCESS_TLV_READWRITE) - kctl.tlv.c = snd_ctl_elem_user_tlv; + kctl->tlv.c = snd_ctl_elem_user_tlv; - if (info->type < SNDRV_CTL_ELEM_TYPE_BOOLEAN || - info->type > SNDRV_CTL_ELEM_TYPE_INTEGER64) - return -EINVAL; - if (info->count > max_value_counts[info->type]) - return -EINVAL; - if (info->type == SNDRV_CTL_ELEM_TYPE_ENUMERATED && - info->value.enumerated.items == 0) - return -EINVAL; - elem_data_size = value_sizes[info->type] * info->count; - ue = kzalloc(sizeof(struct user_element) + elem_data_size, GFP_KERNEL); - if (ue == NULL) - return -ENOMEM; + ue = (struct user_element *)kctl->private_data; ue->card = card; ue->info = *info; ue->info.access = 0; @@ -1243,33 +1266,26 @@ static int snd_ctl_elem_add(struct snd_ctl_file *file, ue->elem_data_size = elem_data_size; if (ue->info.type == SNDRV_CTL_ELEM_TYPE_ENUMERATED) { err = snd_ctl_elem_init_enum_names(ue); - if (err < 0) { - kfree(ue); - return err; - } - } - kctl.private_free = snd_ctl_elem_user_free; - _kctl = ctl_new(&kctl, access); - if (_kctl == NULL) { - kfree(ue->priv_data); - kfree(ue); - return -ENOMEM; + if (err < 0) + goto error_allocated; } - _kctl->private_data = ue; /* Lock values in this user controls. */ - for (i = 0; i < _kctl->count; i++) - _kctl->vd[i].owner = file; + for (i = 0; i < kctl->count; i++) + kctl->vd[i].owner = file; - err = snd_ctl_add(card, _kctl); + err = snd_ctl_add(card, kctl); if (err < 0) - return err; + return err; /* already freed. */ down_write(&card->controls_rwsem); - card->user_ctl_count += _kctl->count; + card->user_ctl_count += kctl->count; up_write(&card->controls_rwsem); return 0; +error_allocated: + snd_ctl_free_one(kctl); + return err; } static int snd_ctl_elem_add_user(struct snd_ctl_file *file,