[PATCH] ALSA: control: remove limitation on the number of user-defined control element set per card

[PATCH] ALSA: control: remove limitation on the number of user-defined control element set per card

[Takashi Sakamoto]

ALSA control core allows usespace application to register control element
set by call of ioctl(2) with SNDRV_CTL_IOCTL_ELEM_ADD request. The added
control element set is called as 'user-defined'. Currently sound card has
limitation on the number of the user-defined control element set up
to 32.

The limitation is inconvenient to drivers in ALSA firewire stack since
the drivers expect userspace applications to implement function to
control device functionalities such as mixing and routing. As the
userspace application, snd-firewire-ctl-services project starts:
https://github.com/alsa-project/snd-firewire-ctl-services/

The project supports many devices supported by ALSA firewire stack. The
limitation is mostly good since routing and mixing controls can be
represented by control element set, which includes control element with
the same parameters. Nevertheless, it's actually inconvenient to device
which has many varied functionalities. For example, plugin effect such as
channel strip and reverb has many parameters. For the case, many control
elements are required to configure the parameters and control element set
cannot aggregates controls for the parameters. At present, below models
are implemented with the control elements and actually add control element
sets over 32:

 * Apogee Emsemble (snd-bebob-ctl-service)
 * TC Electronic Konnekt 24d (snd-dice-ctl-service)
 * TC Electronic Studio Konnekt 48 (snd-dice-ctl-service)
 * TC Electronic Konnekt Live (snd-dice-ctl-service)
 * TC Electronic Impact Twin (snd-dice-ctl-service)

It could be investigated to increase the number; e.g. quadruple to the
current (=128), however it's hard to find criteria about the number for
existent sound card. This commit just removes the limitation for the
reason. ALSA control core uses UINT_MAX as the maximum number of control
elements added to a sound card. It's limitation for both in-kernel driver
and userspace application.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 include/sound/core.h |  1 -
 sound/core/control.c | 13 -------------
 2 files changed, 14 deletions(-)

diff --git a/include/sound/core.h b/include/sound/core.h
index 0462c577d7a3..6b443ce0acf8 100644
--- a/include/sound/core.h
+++ b/include/sound/core.h
@@ -100,7 +100,6 @@ struct snd_card {
 	struct rw_semaphore controls_rwsem;	/* controls list lock */
 	rwlock_t ctl_files_rwlock;	/* ctl_files list lock */
 	int controls_count;		/* count of all controls */
-	int user_ctl_count;		/* count of all user controls */
 	struct list_head controls;	/* all controls for this card */
 	struct list_head ctl_files;	/* active control files */
 
diff --git a/sound/core/control.c b/sound/core/control.c
index 5165741a8400..1bcef22713f4 100644
--- a/sound/core/control.c
+++ b/sound/core/control.c
@@ -18,8 +18,6 @@
 #include <sound/info.h>
 #include <sound/control.h>
 
-/* max number of user-defined controls */
-#define MAX_USER_CONTROLS	32
 #define MAX_CONTROL_COUNT	1028
 
 struct snd_kctl_ioctl {
@@ -539,7 +537,6 @@ static int snd_ctl_remove_user_ctl(struct snd_ctl_file * file,
 	ret = snd_ctl_remove(card, kctl);
 	if (ret < 0)
 		goto error;
-	card->user_ctl_count--;
 error:
 	up_write(&card->controls_rwsem);
 	return ret;
@@ -1435,13 +1432,6 @@ static int snd_ctl_elem_add(struct snd_ctl_file *file,
 			return err;
 	}
 
-	/*
-	 * 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;
-
 	/* Check the number of elements for this userspace control. */
 	count = info->owner;
 	if (count == 0)
@@ -1534,9 +1524,6 @@ static int snd_ctl_elem_add(struct snd_ctl_file *file,
 	 * applications because the field originally means PID of a process
 	 * which locks the element.
 	 */
-
-	card->user_ctl_count++;
-
  unlock:
 	up_write(&card->controls_rwsem);
 	return err;
-- 
2.27.0

Re: [PATCH] ALSA: control: remove limitation on the number of user-defined control element set per card

[Takashi Iwai]

On Wed, 20 Jan 2021 09:55:41 +0100,
Takashi Sakamoto wrote:
> 
> ALSA control core allows usespace application to register control element
> set by call of ioctl(2) with SNDRV_CTL_IOCTL_ELEM_ADD request. The added
> control element set is called as 'user-defined'. Currently sound card has
> limitation on the number of the user-defined control element set up
> to 32.
> 
> The limitation is inconvenient to drivers in ALSA firewire stack since
> the drivers expect userspace applications to implement function to
> control device functionalities such as mixing and routing. As the
> userspace application, snd-firewire-ctl-services project starts:
> https://github.com/alsa-project/snd-firewire-ctl-services/
> 
> The project supports many devices supported by ALSA firewire stack. The
> limitation is mostly good since routing and mixing controls can be
> represented by control element set, which includes control element with
> the same parameters. Nevertheless, it's actually inconvenient to device
> which has many varied functionalities. For example, plugin effect such as
> channel strip and reverb has many parameters. For the case, many control
> elements are required to configure the parameters and control element set
> cannot aggregates controls for the parameters. At present, below models
> are implemented with the control elements and actually add control element
> sets over 32:
> 
>  * Apogee Emsemble (snd-bebob-ctl-service)
>  * TC Electronic Konnekt 24d (snd-dice-ctl-service)
>  * TC Electronic Studio Konnekt 48 (snd-dice-ctl-service)
>  * TC Electronic Konnekt Live (snd-dice-ctl-service)
>  * TC Electronic Impact Twin (snd-dice-ctl-service)
> 
> It could be investigated to increase the number; e.g. quadruple to the
> current (=128), however it's hard to find criteria about the number for
> existent sound card. This commit just removes the limitation for the
> reason. ALSA control core uses UINT_MAX as the maximum number of control
> elements added to a sound card. It's limitation for both in-kernel driver
> and userspace application.
> 
> Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>

I'm still worried by the unlimited number of possible additions.
Did you check what would happen if you run a test program to add
user-space ctls (with the max count) in a loop repeatedly?  If that
doesn't blow up too much, it might be OK.  Otherwise we have to add
some practical limits.

So, let's prove that it's absolutely safe to release the limit at
first.


thanks,

Takashi

Re: [PATCH] ALSA: control: remove limitation on the number of user-defined control element set per card

[Jaroslav Kysela]

Dne 20. 01. 21 v 10:27 Takashi Iwai napsal(a):
> On Wed, 20 Jan 2021 09:55:41 +0100,
> Takashi Sakamoto wrote:
>>
>> ALSA control core allows usespace application to register control element
>> set by call of ioctl(2) with SNDRV_CTL_IOCTL_ELEM_ADD request. The added
>> control element set is called as 'user-defined'. Currently sound card has
>> limitation on the number of the user-defined control element set up
>> to 32.
>>
>> The limitation is inconvenient to drivers in ALSA firewire stack since
>> the drivers expect userspace applications to implement function to
>> control device functionalities such as mixing and routing. As the
>> userspace application, snd-firewire-ctl-services project starts:
>> https://github.com/alsa-project/snd-firewire-ctl-services/
>>
>> The project supports many devices supported by ALSA firewire stack. The
>> limitation is mostly good since routing and mixing controls can be
>> represented by control element set, which includes control element with
>> the same parameters. Nevertheless, it's actually inconvenient to device
>> which has many varied functionalities. For example, plugin effect such as
>> channel strip and reverb has many parameters. For the case, many control
>> elements are required to configure the parameters and control element set
>> cannot aggregates controls for the parameters. At present, below models
>> are implemented with the control elements and actually add control element
>> sets over 32:
>>
>>  * Apogee Emsemble (snd-bebob-ctl-service)
>>  * TC Electronic Konnekt 24d (snd-dice-ctl-service)
>>  * TC Electronic Studio Konnekt 48 (snd-dice-ctl-service)
>>  * TC Electronic Konnekt Live (snd-dice-ctl-service)
>>  * TC Electronic Impact Twin (snd-dice-ctl-service)
>>
>> It could be investigated to increase the number; e.g. quadruple to the
>> current (=128), however it's hard to find criteria about the number for
>> existent sound card. This commit just removes the limitation for the
>> reason. ALSA control core uses UINT_MAX as the maximum number of control
>> elements added to a sound card. It's limitation for both in-kernel driver
>> and userspace application.
>>
>> Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
> 
> I'm still worried by the unlimited number of possible additions.
> Did you check what would happen if you run a test program to add
> user-space ctls (with the max count) in a loop repeatedly?  If that
> doesn't blow up too much, it might be OK.  Otherwise we have to add
> some practical limits.
> 
> So, let's prove that it's absolutely safe to release the limit at
> first.

I agree. The UINT_MAX limit is really big in my eyes. The condition was added
to check for the insane allocations. I basically agree to increase this limit
(512, 1024?), but it should not be UINT_MAX.

						Jaroslav

-- 
Jaroslav Kysela <perex@perex.cz>
Linux Sound Maintainer; ALSA Project; Red Hat, Inc.

Re: [PATCH] ALSA: control: remove limitation on the number of user-defined control element set per card

[Takashi Sakamoto]

Hi,

On Wed, Jan 20, 2021 at 10:38:36AM +0100, Jaroslav Kysela wrote:
> Dne 20. 01. 21 v 10:27 Takashi Iwai napsal(a):
> > On Wed, 20 Jan 2021 09:55:41 +0100,
> > Takashi Sakamoto wrote:
> >>
> >> ALSA control core allows usespace application to register control element
> >> set by call of ioctl(2) with SNDRV_CTL_IOCTL_ELEM_ADD request. The added
> >> control element set is called as 'user-defined'. Currently sound card has
> >> limitation on the number of the user-defined control element set up
> >> to 32.
> >>
> >> The limitation is inconvenient to drivers in ALSA firewire stack since
> >> the drivers expect userspace applications to implement function to
> >> control device functionalities such as mixing and routing. As the
> >> userspace application, snd-firewire-ctl-services project starts:
> >> https://github.com/alsa-project/snd-firewire-ctl-services/
> >>
> >> The project supports many devices supported by ALSA firewire stack. The
> >> limitation is mostly good since routing and mixing controls can be
> >> represented by control element set, which includes control element with
> >> the same parameters. Nevertheless, it's actually inconvenient to device
> >> which has many varied functionalities. For example, plugin effect such as
> >> channel strip and reverb has many parameters. For the case, many control
> >> elements are required to configure the parameters and control element set
> >> cannot aggregates controls for the parameters. At present, below models
> >> are implemented with the control elements and actually add control element
> >> sets over 32:
> >>
> >>  * Apogee Emsemble (snd-bebob-ctl-service)
> >>  * TC Electronic Konnekt 24d (snd-dice-ctl-service)
> >>  * TC Electronic Studio Konnekt 48 (snd-dice-ctl-service)
> >>  * TC Electronic Konnekt Live (snd-dice-ctl-service)
> >>  * TC Electronic Impact Twin (snd-dice-ctl-service)
> >>
> >> It could be investigated to increase the number; e.g. quadruple to the
> >> current (=128), however it's hard to find criteria about the number for
> >> existent sound card. This commit just removes the limitation for the
> >> reason. ALSA control core uses UINT_MAX as the maximum number of control
> >> elements added to a sound card. It's limitation for both in-kernel driver
> >> and userspace application.
> >>
> >> Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
> > 
> > I'm still worried by the unlimited number of possible additions.
> > Did you check what would happen if you run a test program to add
> > user-space ctls (with the max count) in a loop repeatedly?  If that
> > doesn't blow up too much, it might be OK.  Otherwise we have to add
> > some practical limits.
> > 
> > So, let's prove that it's absolutely safe to release the limit at
> > first.
> 
> I agree. The UINT_MAX limit is really big in my eyes. The condition was added
> to check for the insane allocations. I basically agree to increase this limit
> (512, 1024?), but it should not be UINT_MAX.

Thanks for your comments. I have no objection to relax current limitation
instead of removal.

Either 512 or 1024 (of course 32), it just comes from multiples of 2 and
seems not to be reasonable for the practical limits. However, as I
note in the commit message, I cannot find no criteria for the limitation.
In this time, 40-50 control element sets are enough for the service program,
thus I'll post v2 patch with 128 as limitation enough large to the
requirement.


Thanks

Takashi Sakamoto