* [PATCH V0 0/1] asoc: msm: use hashtable to check kcontrol @ 2022-05-29 8:50 Raghu Bankapur 2022-05-29 8:50 ` [PATCH V0 1/1] " Raghu Bankapur 2022-05-30 14:08 ` Mark Brown 0 siblings, 2 replies; 18+ messages in thread From: Raghu Bankapur @ 2022-05-29 8:50 UTC (permalink / raw) To: Jaroslav Kysela, Takashi Iwai, Liam Girdwood, Mark Brown, Raghu Bankapur, alsa-devel, linux-kernel Cc: Krishna Jha use hashtabe instead of linear list to check kcontrol before adding them for improving early audio KPI. With this changes we see 600ms improvement in start of audio Raghu Bankapur (1): asoc: msm: use hashtable to check kcontrol include/sound/control.h | 4 ++ include/sound/core.h | 12 +++++- sound/core/control.c | 92 +++++++++++++++++++++++++++++++++-------- sound/core/init.c | 3 ++ sound/soc/Kconfig | 9 ++++ 5 files changed, 101 insertions(+), 19 deletions(-) -- 2.17.1 ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH V0 1/1] asoc: msm: use hashtable to check kcontrol 2022-05-29 8:50 [PATCH V0 0/1] asoc: msm: use hashtable to check kcontrol Raghu Bankapur @ 2022-05-29 8:50 ` Raghu Bankapur 2022-05-29 9:35 ` Takashi Iwai ` (3 more replies) 2022-05-30 14:08 ` Mark Brown 1 sibling, 4 replies; 18+ messages in thread From: Raghu Bankapur @ 2022-05-29 8:50 UTC (permalink / raw) To: Jaroslav Kysela, Takashi Iwai, Liam Girdwood, Mark Brown, Raghu Bankapur, alsa-devel, linux-kernel Cc: Krishna Jha use hashtabe instead of linear list to check kcontrol before adding them for improving early audio KPI. Change-Id: I7134816736e08e338c0f22a8ae283a0520aa847a Signed-off-by: Raghu Bankapur <quic_rbankapu@quicinc.com> --- include/sound/control.h | 4 ++ include/sound/core.h | 12 +++++- sound/core/control.c | 92 +++++++++++++++++++++++++++++++++-------- sound/core/init.c | 3 ++ sound/soc/Kconfig | 9 ++++ 5 files changed, 101 insertions(+), 19 deletions(-) diff --git a/include/sound/control.h b/include/sound/control.h index 985c51a8fb74..1b85d36c2066 100644 --- a/include/sound/control.h +++ b/include/sound/control.h @@ -70,6 +70,10 @@ struct snd_kcontrol_volatile { struct snd_kcontrol { struct list_head list; /* list of controls */ struct snd_ctl_elem_id id; +#ifdef CONFIG_SND_CTL_HASHTABLE + struct hlist_node hnode; + unsigned int knametoint; /* kctl name to uint, hash key value */ +#endif unsigned int count; /* count of same elements */ snd_kcontrol_info_t *info; snd_kcontrol_get_t *get; diff --git a/include/sound/core.h b/include/sound/core.h index b7e9b58d3c78..dd6714fc43ff 100644 --- a/include/sound/core.h +++ b/include/sound/core.h @@ -14,7 +14,9 @@ #include <linux/pm.h> /* pm_message_t */ #include <linux/stringify.h> #include <linux/printk.h> - +#ifdef CONFIG_SND_CTL_HASHTABLE +#include <linux/hashtable.h> +#endif /* number of supported soundcards */ #ifdef CONFIG_SND_DYNAMIC_MINORS #define SNDRV_CARDS CONFIG_SND_MAX_CARDS @@ -24,6 +26,10 @@ #define CONFIG_SND_MAJOR 116 /* standard configuration */ +#ifdef CONFIG_SND_CTL_HASHTABLE +#define SND_CTL_HASH_TABLE_BITS 14 /* buckets numbers: 1 << 14 */ +#endif + /* forward declarations */ struct pci_dev; struct module; @@ -103,7 +109,9 @@ struct snd_card { size_t user_ctl_alloc_size; // current memory allocation by user controls. struct list_head controls; /* all controls for this card */ struct list_head ctl_files; /* active control files */ - +#ifdef CONFIG_SND_CTL_HASHTABLE + DECLARE_HASHTABLE(ctl_htable, SND_CTL_HASH_TABLE_BITS); +#endif struct snd_info_entry *proc_root; /* root for soundcard specific files */ struct proc_dir_entry *proc_root_link; /* number link to real id */ diff --git a/sound/core/control.c b/sound/core/control.c index a25c0d64d104..914d05647497 100644 --- a/sound/core/control.c +++ b/sound/core/control.c @@ -368,6 +368,47 @@ enum snd_ctl_add_mode { CTL_ADD_EXCLUSIVE, CTL_REPLACE, CTL_ADD_ON_REPLACE, }; +#ifdef CONFIG_SND_CTL_HASHTABLE +char snd_ctl_string[50] = { '\0' }; + +/* Used to convert the string into int value -- BKDRHash */ +unsigned int snd_ctl_strtoint(const char *s) +{ + unsigned int res = 0; + + while (*s) + res = (res << 5) - res + (*s++); + + return (res & 0x7FFFFFFF); +} + +/** + * snd_ctl_hash_check - Check the duplicate enrty on snd hashtable + * @card: the card instance + * @nametoint: kctl name to uint + * + * Finds the control instance with the given nametoint from the card. + * + * Return: The pointer of the instance if found, or %NULL if not. + * + */ +static struct snd_kcontrol *snd_ctl_hash_check(struct snd_card *card, + unsigned int nametoint) +{ + struct snd_kcontrol *kctl = NULL; + + if (snd_BUG_ON(!card)) + return NULL; + + hash_for_each_possible(card->ctl_htable, kctl, hnode, nametoint) { + if (kctl->knametoint != nametoint) + continue; + return kctl; + } + return NULL; +} +#endif + /* add/replace a new kcontrol object; call with card->controls_rwsem locked */ static int __snd_ctl_add_replace(struct snd_card *card, struct snd_kcontrol *kcontrol, @@ -382,24 +423,38 @@ static int __snd_ctl_add_replace(struct snd_card *card, if (id.index > UINT_MAX - kcontrol->count) return -EINVAL; - old = snd_ctl_find_id(card, &id); - if (!old) { - if (mode == CTL_REPLACE) - return -EINVAL; - } else { - if (mode == CTL_ADD_EXCLUSIVE) { - dev_err(card->dev, - "control %i:%i:%i:%s:%i is already present\n", - id.iface, id.device, id.subdevice, id.name, - id.index); - return -EBUSY; - } +#ifdef CONFIG_SND_CTL_HASHTABLE + snprintf(snd_ctl_string, strlen(kcontrol->id.name) + 6, "%s%d%d%d", + kcontrol->id.name, kcontrol->id.iface, kcontrol->id.device, + kcontrol->id.subdevice); - err = snd_ctl_remove(card, old); - if (err < 0) - return err; - } + kcontrol->knametoint = snd_ctl_strtoint(snd_ctl_string); + if (kcontrol->knametoint < 0) + return -EINVAL; + + old = snd_ctl_hash_check(card, kcontrol->knametoint); + if (old) { +#endif + old = snd_ctl_find_id(card, &id); + if (!old) { + if (mode == CTL_REPLACE) + return -EINVAL; + } else { + if (mode == CTL_ADD_EXCLUSIVE) { + dev_err(card->dev, + "control %i:%i:%i:%s:%i is already present\n", + id.iface, id.device, id.subdevice, id.name, + id.index); + return -EBUSY; + } + err = snd_ctl_remove(card, old); + if (err < 0) + return err; + } +#ifdef CONFIG_SND_CTL_HASHTABLE + } +#endif if (snd_ctl_find_hole(card, kcontrol->count) < 0) return -ENOMEM; @@ -410,7 +465,10 @@ static int __snd_ctl_add_replace(struct snd_card *card, for (idx = 0; idx < kcontrol->count; idx++) snd_ctl_notify_one(card, SNDRV_CTL_EVENT_MASK_ADD, kcontrol, idx); - + +#ifdef CONFIG_SND_CTL_HASHTABLE + hash_add(card->ctl_htable, &kcontrol->hnode, kcontrol->knametoint); +#endif return 0; } diff --git a/sound/core/init.c b/sound/core/init.c index 31ba7024e3ad..fda38b2137ee 100644 --- a/sound/core/init.c +++ b/sound/core/init.c @@ -284,6 +284,9 @@ static int snd_card_init(struct snd_card *card, struct device *parent, INIT_LIST_HEAD(&card->ctl_files); spin_lock_init(&card->files_lock); INIT_LIST_HEAD(&card->files_list); +#ifdef CONFIG_SND_CTL_HASHTABLE + hash_init(card->ctl_htable); +#endif mutex_init(&card->memory_mutex); #ifdef CONFIG_PM init_waitqueue_head(&card->power_sleep); diff --git a/sound/soc/Kconfig b/sound/soc/Kconfig index 5dcf77af07af..0eb18f8ee6fd 100644 --- a/sound/soc/Kconfig +++ b/sound/soc/Kconfig @@ -58,6 +58,15 @@ config SND_SOC_TOPOLOGY_KUNIT_TEST config SND_SOC_ACPI tristate +config SND_CTL_HASHTABLE + bool "Add SND CTL hashtable" + help + This enables hash table in sound card for kcontrols. The traditional way is + traversing the linked list of controls and compare each exsiting control with + the new kcontrol to find out whether there are duplicate kcontrols, which will + consumes much time during bootup. Enable this will use hash table instead of + linked list to check new kcontrol and reduce much time for sound card registration. + # All the supported SoCs source "sound/soc/adi/Kconfig" source "sound/soc/amd/Kconfig" -- 2.17.1 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH V0 1/1] asoc: msm: use hashtable to check kcontrol 2022-05-29 8:50 ` [PATCH V0 1/1] " Raghu Bankapur @ 2022-05-29 9:35 ` Takashi Iwai 2022-05-29 10:40 ` kernel test robot ` (2 subsequent siblings) 3 siblings, 0 replies; 18+ messages in thread From: Takashi Iwai @ 2022-05-29 9:35 UTC (permalink / raw) To: Raghu Bankapur Cc: alsa-devel, linux-kernel, Liam Girdwood, Takashi Iwai, Mark Brown, Krishna Jha On Sun, 29 May 2022 10:50:09 +0200, Raghu Bankapur wrote: > > use hashtabe instead of linear list to check kcontrol before > adding them for improving early audio KPI. > > Change-Id: I7134816736e08e338c0f22a8ae283a0520aa847a > Signed-off-by: Raghu Bankapur <quic_rbankapu@quicinc.com> Aha, interesting, a faster lookup is indeed measurable and needed. One point with your patch is whether it works when a control element gets removed dynamically. It's often the case with the user-space kctls. Also, multiple ctl elements may have the same name string but with different index or device number. Comparing only the string isn't enough. (And I wonder how about the hash key collision.) FWIW, I posted an RFC patch faster lookup with Xarray some time ago: https://lore.kernel.org/all/20211028130027.18764-1-tiwai@suse.de/T/ thanks, Takashi > --- > include/sound/control.h | 4 ++ > include/sound/core.h | 12 +++++- > sound/core/control.c | 92 +++++++++++++++++++++++++++++++++-------- > sound/core/init.c | 3 ++ > sound/soc/Kconfig | 9 ++++ > 5 files changed, 101 insertions(+), 19 deletions(-) > > diff --git a/include/sound/control.h b/include/sound/control.h > index 985c51a8fb74..1b85d36c2066 100644 > --- a/include/sound/control.h > +++ b/include/sound/control.h > @@ -70,6 +70,10 @@ struct snd_kcontrol_volatile { > struct snd_kcontrol { > struct list_head list; /* list of controls */ > struct snd_ctl_elem_id id; > +#ifdef CONFIG_SND_CTL_HASHTABLE > + struct hlist_node hnode; > + unsigned int knametoint; /* kctl name to uint, hash key value */ > +#endif > unsigned int count; /* count of same elements */ > snd_kcontrol_info_t *info; > snd_kcontrol_get_t *get; > diff --git a/include/sound/core.h b/include/sound/core.h > index b7e9b58d3c78..dd6714fc43ff 100644 > --- a/include/sound/core.h > +++ b/include/sound/core.h > @@ -14,7 +14,9 @@ > #include <linux/pm.h> /* pm_message_t */ > #include <linux/stringify.h> > #include <linux/printk.h> > - > +#ifdef CONFIG_SND_CTL_HASHTABLE > +#include <linux/hashtable.h> > +#endif > /* number of supported soundcards */ > #ifdef CONFIG_SND_DYNAMIC_MINORS > #define SNDRV_CARDS CONFIG_SND_MAX_CARDS > @@ -24,6 +26,10 @@ > > #define CONFIG_SND_MAJOR 116 /* standard configuration */ > > +#ifdef CONFIG_SND_CTL_HASHTABLE > +#define SND_CTL_HASH_TABLE_BITS 14 /* buckets numbers: 1 << 14 */ > +#endif > + > /* forward declarations */ > struct pci_dev; > struct module; > @@ -103,7 +109,9 @@ struct snd_card { > size_t user_ctl_alloc_size; // current memory allocation by user controls. > struct list_head controls; /* all controls for this card */ > struct list_head ctl_files; /* active control files */ > - > +#ifdef CONFIG_SND_CTL_HASHTABLE > + DECLARE_HASHTABLE(ctl_htable, SND_CTL_HASH_TABLE_BITS); > +#endif > struct snd_info_entry *proc_root; /* root for soundcard specific files */ > struct proc_dir_entry *proc_root_link; /* number link to real id */ > > diff --git a/sound/core/control.c b/sound/core/control.c > index a25c0d64d104..914d05647497 100644 > --- a/sound/core/control.c > +++ b/sound/core/control.c > @@ -368,6 +368,47 @@ enum snd_ctl_add_mode { > CTL_ADD_EXCLUSIVE, CTL_REPLACE, CTL_ADD_ON_REPLACE, > }; > > +#ifdef CONFIG_SND_CTL_HASHTABLE > +char snd_ctl_string[50] = { '\0' }; > + > +/* Used to convert the string into int value -- BKDRHash */ > +unsigned int snd_ctl_strtoint(const char *s) > +{ > + unsigned int res = 0; > + > + while (*s) > + res = (res << 5) - res + (*s++); > + > + return (res & 0x7FFFFFFF); > +} > + > +/** > + * snd_ctl_hash_check - Check the duplicate enrty on snd hashtable > + * @card: the card instance > + * @nametoint: kctl name to uint > + * > + * Finds the control instance with the given nametoint from the card. > + * > + * Return: The pointer of the instance if found, or %NULL if not. > + * > + */ > +static struct snd_kcontrol *snd_ctl_hash_check(struct snd_card *card, > + unsigned int nametoint) > +{ > + struct snd_kcontrol *kctl = NULL; > + > + if (snd_BUG_ON(!card)) > + return NULL; > + > + hash_for_each_possible(card->ctl_htable, kctl, hnode, nametoint) { > + if (kctl->knametoint != nametoint) > + continue; > + return kctl; > + } > + return NULL; > +} > +#endif > + > /* add/replace a new kcontrol object; call with card->controls_rwsem locked */ > static int __snd_ctl_add_replace(struct snd_card *card, > struct snd_kcontrol *kcontrol, > @@ -382,24 +423,38 @@ static int __snd_ctl_add_replace(struct snd_card *card, > if (id.index > UINT_MAX - kcontrol->count) > return -EINVAL; > > - old = snd_ctl_find_id(card, &id); > - if (!old) { > - if (mode == CTL_REPLACE) > - return -EINVAL; > - } else { > - if (mode == CTL_ADD_EXCLUSIVE) { > - dev_err(card->dev, > - "control %i:%i:%i:%s:%i is already present\n", > - id.iface, id.device, id.subdevice, id.name, > - id.index); > - return -EBUSY; > - } > +#ifdef CONFIG_SND_CTL_HASHTABLE > + snprintf(snd_ctl_string, strlen(kcontrol->id.name) + 6, "%s%d%d%d", > + kcontrol->id.name, kcontrol->id.iface, kcontrol->id.device, > + kcontrol->id.subdevice); > > - err = snd_ctl_remove(card, old); > - if (err < 0) > - return err; > - } > + kcontrol->knametoint = snd_ctl_strtoint(snd_ctl_string); > + if (kcontrol->knametoint < 0) > + return -EINVAL; > + > + old = snd_ctl_hash_check(card, kcontrol->knametoint); > + if (old) { > +#endif > + old = snd_ctl_find_id(card, &id); > + if (!old) { > + if (mode == CTL_REPLACE) > + return -EINVAL; > + } else { > + if (mode == CTL_ADD_EXCLUSIVE) { > + dev_err(card->dev, > + "control %i:%i:%i:%s:%i is already present\n", > + id.iface, id.device, id.subdevice, id.name, > + id.index); > + return -EBUSY; > + } > > + err = snd_ctl_remove(card, old); > + if (err < 0) > + return err; > + } > +#ifdef CONFIG_SND_CTL_HASHTABLE > + } > +#endif > if (snd_ctl_find_hole(card, kcontrol->count) < 0) > return -ENOMEM; > > @@ -410,7 +465,10 @@ static int __snd_ctl_add_replace(struct snd_card *card, > > for (idx = 0; idx < kcontrol->count; idx++) > snd_ctl_notify_one(card, SNDRV_CTL_EVENT_MASK_ADD, kcontrol, idx); > - > + > +#ifdef CONFIG_SND_CTL_HASHTABLE > + hash_add(card->ctl_htable, &kcontrol->hnode, kcontrol->knametoint); > +#endif > return 0; > } > > diff --git a/sound/core/init.c b/sound/core/init.c > index 31ba7024e3ad..fda38b2137ee 100644 > --- a/sound/core/init.c > +++ b/sound/core/init.c > @@ -284,6 +284,9 @@ static int snd_card_init(struct snd_card *card, struct device *parent, > INIT_LIST_HEAD(&card->ctl_files); > spin_lock_init(&card->files_lock); > INIT_LIST_HEAD(&card->files_list); > +#ifdef CONFIG_SND_CTL_HASHTABLE > + hash_init(card->ctl_htable); > +#endif > mutex_init(&card->memory_mutex); > #ifdef CONFIG_PM > init_waitqueue_head(&card->power_sleep); > diff --git a/sound/soc/Kconfig b/sound/soc/Kconfig > index 5dcf77af07af..0eb18f8ee6fd 100644 > --- a/sound/soc/Kconfig > +++ b/sound/soc/Kconfig > @@ -58,6 +58,15 @@ config SND_SOC_TOPOLOGY_KUNIT_TEST > config SND_SOC_ACPI > tristate > > +config SND_CTL_HASHTABLE > + bool "Add SND CTL hashtable" > + help > + This enables hash table in sound card for kcontrols. The traditional way is > + traversing the linked list of controls and compare each exsiting control with > + the new kcontrol to find out whether there are duplicate kcontrols, which will > + consumes much time during bootup. Enable this will use hash table instead of > + linked list to check new kcontrol and reduce much time for sound card registration. > + > # All the supported SoCs > source "sound/soc/adi/Kconfig" > source "sound/soc/amd/Kconfig" > -- > 2.17.1 > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH V0 1/1] asoc: msm: use hashtable to check kcontrol @ 2022-05-29 9:35 ` Takashi Iwai 0 siblings, 0 replies; 18+ messages in thread From: Takashi Iwai @ 2022-05-29 9:35 UTC (permalink / raw) To: Raghu Bankapur Cc: Jaroslav Kysela, Takashi Iwai, Liam Girdwood, Mark Brown, alsa-devel, linux-kernel, Krishna Jha On Sun, 29 May 2022 10:50:09 +0200, Raghu Bankapur wrote: > > use hashtabe instead of linear list to check kcontrol before > adding them for improving early audio KPI. > > Change-Id: I7134816736e08e338c0f22a8ae283a0520aa847a > Signed-off-by: Raghu Bankapur <quic_rbankapu@quicinc.com> Aha, interesting, a faster lookup is indeed measurable and needed. One point with your patch is whether it works when a control element gets removed dynamically. It's often the case with the user-space kctls. Also, multiple ctl elements may have the same name string but with different index or device number. Comparing only the string isn't enough. (And I wonder how about the hash key collision.) FWIW, I posted an RFC patch faster lookup with Xarray some time ago: https://lore.kernel.org/all/20211028130027.18764-1-tiwai@suse.de/T/ thanks, Takashi > --- > include/sound/control.h | 4 ++ > include/sound/core.h | 12 +++++- > sound/core/control.c | 92 +++++++++++++++++++++++++++++++++-------- > sound/core/init.c | 3 ++ > sound/soc/Kconfig | 9 ++++ > 5 files changed, 101 insertions(+), 19 deletions(-) > > diff --git a/include/sound/control.h b/include/sound/control.h > index 985c51a8fb74..1b85d36c2066 100644 > --- a/include/sound/control.h > +++ b/include/sound/control.h > @@ -70,6 +70,10 @@ struct snd_kcontrol_volatile { > struct snd_kcontrol { > struct list_head list; /* list of controls */ > struct snd_ctl_elem_id id; > +#ifdef CONFIG_SND_CTL_HASHTABLE > + struct hlist_node hnode; > + unsigned int knametoint; /* kctl name to uint, hash key value */ > +#endif > unsigned int count; /* count of same elements */ > snd_kcontrol_info_t *info; > snd_kcontrol_get_t *get; > diff --git a/include/sound/core.h b/include/sound/core.h > index b7e9b58d3c78..dd6714fc43ff 100644 > --- a/include/sound/core.h > +++ b/include/sound/core.h > @@ -14,7 +14,9 @@ > #include <linux/pm.h> /* pm_message_t */ > #include <linux/stringify.h> > #include <linux/printk.h> > - > +#ifdef CONFIG_SND_CTL_HASHTABLE > +#include <linux/hashtable.h> > +#endif > /* number of supported soundcards */ > #ifdef CONFIG_SND_DYNAMIC_MINORS > #define SNDRV_CARDS CONFIG_SND_MAX_CARDS > @@ -24,6 +26,10 @@ > > #define CONFIG_SND_MAJOR 116 /* standard configuration */ > > +#ifdef CONFIG_SND_CTL_HASHTABLE > +#define SND_CTL_HASH_TABLE_BITS 14 /* buckets numbers: 1 << 14 */ > +#endif > + > /* forward declarations */ > struct pci_dev; > struct module; > @@ -103,7 +109,9 @@ struct snd_card { > size_t user_ctl_alloc_size; // current memory allocation by user controls. > struct list_head controls; /* all controls for this card */ > struct list_head ctl_files; /* active control files */ > - > +#ifdef CONFIG_SND_CTL_HASHTABLE > + DECLARE_HASHTABLE(ctl_htable, SND_CTL_HASH_TABLE_BITS); > +#endif > struct snd_info_entry *proc_root; /* root for soundcard specific files */ > struct proc_dir_entry *proc_root_link; /* number link to real id */ > > diff --git a/sound/core/control.c b/sound/core/control.c > index a25c0d64d104..914d05647497 100644 > --- a/sound/core/control.c > +++ b/sound/core/control.c > @@ -368,6 +368,47 @@ enum snd_ctl_add_mode { > CTL_ADD_EXCLUSIVE, CTL_REPLACE, CTL_ADD_ON_REPLACE, > }; > > +#ifdef CONFIG_SND_CTL_HASHTABLE > +char snd_ctl_string[50] = { '\0' }; > + > +/* Used to convert the string into int value -- BKDRHash */ > +unsigned int snd_ctl_strtoint(const char *s) > +{ > + unsigned int res = 0; > + > + while (*s) > + res = (res << 5) - res + (*s++); > + > + return (res & 0x7FFFFFFF); > +} > + > +/** > + * snd_ctl_hash_check - Check the duplicate enrty on snd hashtable > + * @card: the card instance > + * @nametoint: kctl name to uint > + * > + * Finds the control instance with the given nametoint from the card. > + * > + * Return: The pointer of the instance if found, or %NULL if not. > + * > + */ > +static struct snd_kcontrol *snd_ctl_hash_check(struct snd_card *card, > + unsigned int nametoint) > +{ > + struct snd_kcontrol *kctl = NULL; > + > + if (snd_BUG_ON(!card)) > + return NULL; > + > + hash_for_each_possible(card->ctl_htable, kctl, hnode, nametoint) { > + if (kctl->knametoint != nametoint) > + continue; > + return kctl; > + } > + return NULL; > +} > +#endif > + > /* add/replace a new kcontrol object; call with card->controls_rwsem locked */ > static int __snd_ctl_add_replace(struct snd_card *card, > struct snd_kcontrol *kcontrol, > @@ -382,24 +423,38 @@ static int __snd_ctl_add_replace(struct snd_card *card, > if (id.index > UINT_MAX - kcontrol->count) > return -EINVAL; > > - old = snd_ctl_find_id(card, &id); > - if (!old) { > - if (mode == CTL_REPLACE) > - return -EINVAL; > - } else { > - if (mode == CTL_ADD_EXCLUSIVE) { > - dev_err(card->dev, > - "control %i:%i:%i:%s:%i is already present\n", > - id.iface, id.device, id.subdevice, id.name, > - id.index); > - return -EBUSY; > - } > +#ifdef CONFIG_SND_CTL_HASHTABLE > + snprintf(snd_ctl_string, strlen(kcontrol->id.name) + 6, "%s%d%d%d", > + kcontrol->id.name, kcontrol->id.iface, kcontrol->id.device, > + kcontrol->id.subdevice); > > - err = snd_ctl_remove(card, old); > - if (err < 0) > - return err; > - } > + kcontrol->knametoint = snd_ctl_strtoint(snd_ctl_string); > + if (kcontrol->knametoint < 0) > + return -EINVAL; > + > + old = snd_ctl_hash_check(card, kcontrol->knametoint); > + if (old) { > +#endif > + old = snd_ctl_find_id(card, &id); > + if (!old) { > + if (mode == CTL_REPLACE) > + return -EINVAL; > + } else { > + if (mode == CTL_ADD_EXCLUSIVE) { > + dev_err(card->dev, > + "control %i:%i:%i:%s:%i is already present\n", > + id.iface, id.device, id.subdevice, id.name, > + id.index); > + return -EBUSY; > + } > > + err = snd_ctl_remove(card, old); > + if (err < 0) > + return err; > + } > +#ifdef CONFIG_SND_CTL_HASHTABLE > + } > +#endif > if (snd_ctl_find_hole(card, kcontrol->count) < 0) > return -ENOMEM; > > @@ -410,7 +465,10 @@ static int __snd_ctl_add_replace(struct snd_card *card, > > for (idx = 0; idx < kcontrol->count; idx++) > snd_ctl_notify_one(card, SNDRV_CTL_EVENT_MASK_ADD, kcontrol, idx); > - > + > +#ifdef CONFIG_SND_CTL_HASHTABLE > + hash_add(card->ctl_htable, &kcontrol->hnode, kcontrol->knametoint); > +#endif > return 0; > } > > diff --git a/sound/core/init.c b/sound/core/init.c > index 31ba7024e3ad..fda38b2137ee 100644 > --- a/sound/core/init.c > +++ b/sound/core/init.c > @@ -284,6 +284,9 @@ static int snd_card_init(struct snd_card *card, struct device *parent, > INIT_LIST_HEAD(&card->ctl_files); > spin_lock_init(&card->files_lock); > INIT_LIST_HEAD(&card->files_list); > +#ifdef CONFIG_SND_CTL_HASHTABLE > + hash_init(card->ctl_htable); > +#endif > mutex_init(&card->memory_mutex); > #ifdef CONFIG_PM > init_waitqueue_head(&card->power_sleep); > diff --git a/sound/soc/Kconfig b/sound/soc/Kconfig > index 5dcf77af07af..0eb18f8ee6fd 100644 > --- a/sound/soc/Kconfig > +++ b/sound/soc/Kconfig > @@ -58,6 +58,15 @@ config SND_SOC_TOPOLOGY_KUNIT_TEST > config SND_SOC_ACPI > tristate > > +config SND_CTL_HASHTABLE > + bool "Add SND CTL hashtable" > + help > + This enables hash table in sound card for kcontrols. The traditional way is > + traversing the linked list of controls and compare each exsiting control with > + the new kcontrol to find out whether there are duplicate kcontrols, which will > + consumes much time during bootup. Enable this will use hash table instead of > + linked list to check new kcontrol and reduce much time for sound card registration. > + > # All the supported SoCs > source "sound/soc/adi/Kconfig" > source "sound/soc/amd/Kconfig" > -- > 2.17.1 > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH V0 1/1] asoc: msm: use hashtable to check kcontrol 2022-05-29 9:35 ` Takashi Iwai (?) @ 2022-06-09 18:16 ` Raghu Ballappa Bankapur 2022-06-10 6:01 ` Takashi Iwai -1 siblings, 1 reply; 18+ messages in thread From: Raghu Ballappa Bankapur @ 2022-06-09 18:16 UTC (permalink / raw) To: Takashi Iwai Cc: alsa-devel, linux-kernel, Liam Girdwood, Takashi Iwai, Mark Brown, Krishna Jha Hi Takashi Our implementation also falls inline with your approach [PATCH RFC] ALSA: control: Use xarray for faster lookups (kernel.org) <https://lore.kernel.org/all/20211028130027.18764-1-tiwai@suse.de/T/>, but you approach looks to be clean with xarray. is it possible to upstream those changes? Regards Raghu Bankapur On 5/29/2022 3:05 PM, Takashi Iwai wrote: > On Sun, 29 May 2022 10:50:09 +0200, > Raghu Bankapur wrote: >> use hashtabe instead of linear list to check kcontrol before >> adding them for improving early audio KPI. >> >> Change-Id: I7134816736e08e338c0f22a8ae283a0520aa847a >> Signed-off-by: Raghu Bankapur<quic_rbankapu@quicinc.com> > Aha, interesting, a faster lookup is indeed measurable and needed. > > One point with your patch is whether it works when a control element > gets removed dynamically. It's often the case with the user-space > kctls. Also, multiple ctl elements may have the same name string but > with different index or device number. Comparing only the string > isn't enough. (And I wonder how about the hash key collision.) > > FWIW, I posted an RFC patch faster lookup with Xarray some time ago: > https://lore.kernel.org/all/20211028130027.18764-1-tiwai@suse.de/T/ > > > thanks, > > Takashi > > >> --- >> include/sound/control.h | 4 ++ >> include/sound/core.h | 12 +++++- >> sound/core/control.c | 92 +++++++++++++++++++++++++++++++++-------- >> sound/core/init.c | 3 ++ >> sound/soc/Kconfig | 9 ++++ >> 5 files changed, 101 insertions(+), 19 deletions(-) >> >> diff --git a/include/sound/control.h b/include/sound/control.h >> index 985c51a8fb74..1b85d36c2066 100644 >> --- a/include/sound/control.h >> +++ b/include/sound/control.h >> @@ -70,6 +70,10 @@ struct snd_kcontrol_volatile { >> struct snd_kcontrol { >> struct list_head list; /* list of controls */ >> struct snd_ctl_elem_id id; >> +#ifdef CONFIG_SND_CTL_HASHTABLE >> + struct hlist_node hnode; >> + unsigned int knametoint; /* kctl name to uint, hash key value */ >> +#endif >> unsigned int count; /* count of same elements */ >> snd_kcontrol_info_t *info; >> snd_kcontrol_get_t *get; >> diff --git a/include/sound/core.h b/include/sound/core.h >> index b7e9b58d3c78..dd6714fc43ff 100644 >> --- a/include/sound/core.h >> +++ b/include/sound/core.h >> @@ -14,7 +14,9 @@ >> #include <linux/pm.h> /* pm_message_t */ >> #include <linux/stringify.h> >> #include <linux/printk.h> >> - >> +#ifdef CONFIG_SND_CTL_HASHTABLE >> +#include <linux/hashtable.h> >> +#endif >> /* number of supported soundcards */ >> #ifdef CONFIG_SND_DYNAMIC_MINORS >> #define SNDRV_CARDS CONFIG_SND_MAX_CARDS >> @@ -24,6 +26,10 @@ >> >> #define CONFIG_SND_MAJOR 116 /* standard configuration */ >> >> +#ifdef CONFIG_SND_CTL_HASHTABLE >> +#define SND_CTL_HASH_TABLE_BITS 14 /* buckets numbers: 1 << 14 */ >> +#endif >> + >> /* forward declarations */ >> struct pci_dev; >> struct module; >> @@ -103,7 +109,9 @@ struct snd_card { >> size_t user_ctl_alloc_size; // current memory allocation by user controls. >> struct list_head controls; /* all controls for this card */ >> struct list_head ctl_files; /* active control files */ >> - >> +#ifdef CONFIG_SND_CTL_HASHTABLE >> + DECLARE_HASHTABLE(ctl_htable, SND_CTL_HASH_TABLE_BITS); >> +#endif >> struct snd_info_entry *proc_root; /* root for soundcard specific files */ >> struct proc_dir_entry *proc_root_link; /* number link to real id */ >> >> diff --git a/sound/core/control.c b/sound/core/control.c >> index a25c0d64d104..914d05647497 100644 >> --- a/sound/core/control.c >> +++ b/sound/core/control.c >> @@ -368,6 +368,47 @@ enum snd_ctl_add_mode { >> CTL_ADD_EXCLUSIVE, CTL_REPLACE, CTL_ADD_ON_REPLACE, >> }; >> >> +#ifdef CONFIG_SND_CTL_HASHTABLE >> +char snd_ctl_string[50] = { '\0' }; >> + >> +/* Used to convert the string into int value -- BKDRHash */ >> +unsigned int snd_ctl_strtoint(const char *s) >> +{ >> + unsigned int res = 0; >> + >> + while (*s) >> + res = (res << 5) - res + (*s++); >> + >> + return (res & 0x7FFFFFFF); >> +} >> + >> +/** >> + * snd_ctl_hash_check - Check the duplicate enrty on snd hashtable >> + * @card: the card instance >> + * @nametoint: kctl name to uint >> + * >> + * Finds the control instance with the given nametoint from the card. >> + * >> + * Return: The pointer of the instance if found, or %NULL if not. >> + * >> + */ >> +static struct snd_kcontrol *snd_ctl_hash_check(struct snd_card *card, >> + unsigned int nametoint) >> +{ >> + struct snd_kcontrol *kctl = NULL; >> + >> + if (snd_BUG_ON(!card)) >> + return NULL; >> + >> + hash_for_each_possible(card->ctl_htable, kctl, hnode, nametoint) { >> + if (kctl->knametoint != nametoint) >> + continue; >> + return kctl; >> + } >> + return NULL; >> +} >> +#endif >> + >> /* add/replace a new kcontrol object; call with card->controls_rwsem locked */ >> static int __snd_ctl_add_replace(struct snd_card *card, >> struct snd_kcontrol *kcontrol, >> @@ -382,24 +423,38 @@ static int __snd_ctl_add_replace(struct snd_card *card, >> if (id.index > UINT_MAX - kcontrol->count) >> return -EINVAL; >> >> - old = snd_ctl_find_id(card, &id); >> - if (!old) { >> - if (mode == CTL_REPLACE) >> - return -EINVAL; >> - } else { >> - if (mode == CTL_ADD_EXCLUSIVE) { >> - dev_err(card->dev, >> - "control %i:%i:%i:%s:%i is already present\n", >> - id.iface, id.device, id.subdevice, id.name, >> - id.index); >> - return -EBUSY; >> - } >> +#ifdef CONFIG_SND_CTL_HASHTABLE >> + snprintf(snd_ctl_string, strlen(kcontrol->id.name) + 6, "%s%d%d%d", >> + kcontrol->id.name, kcontrol->id.iface, kcontrol->id.device, >> + kcontrol->id.subdevice); >> >> - err = snd_ctl_remove(card, old); >> - if (err < 0) >> - return err; >> - } >> + kcontrol->knametoint = snd_ctl_strtoint(snd_ctl_string); >> + if (kcontrol->knametoint < 0) >> + return -EINVAL; >> + >> + old = snd_ctl_hash_check(card, kcontrol->knametoint); >> + if (old) { >> +#endif >> + old = snd_ctl_find_id(card, &id); >> + if (!old) { >> + if (mode == CTL_REPLACE) >> + return -EINVAL; >> + } else { >> + if (mode == CTL_ADD_EXCLUSIVE) { >> + dev_err(card->dev, >> + "control %i:%i:%i:%s:%i is already present\n", >> + id.iface, id.device, id.subdevice, id.name, >> + id.index); >> + return -EBUSY; >> + } >> >> + err = snd_ctl_remove(card, old); >> + if (err < 0) >> + return err; >> + } >> +#ifdef CONFIG_SND_CTL_HASHTABLE >> + } >> +#endif >> if (snd_ctl_find_hole(card, kcontrol->count) < 0) >> return -ENOMEM; >> >> @@ -410,7 +465,10 @@ static int __snd_ctl_add_replace(struct snd_card *card, >> >> for (idx = 0; idx < kcontrol->count; idx++) >> snd_ctl_notify_one(card, SNDRV_CTL_EVENT_MASK_ADD, kcontrol, idx); >> - >> + >> +#ifdef CONFIG_SND_CTL_HASHTABLE >> + hash_add(card->ctl_htable, &kcontrol->hnode, kcontrol->knametoint); >> +#endif >> return 0; >> } >> >> diff --git a/sound/core/init.c b/sound/core/init.c >> index 31ba7024e3ad..fda38b2137ee 100644 >> --- a/sound/core/init.c >> +++ b/sound/core/init.c >> @@ -284,6 +284,9 @@ static int snd_card_init(struct snd_card *card, struct device *parent, >> INIT_LIST_HEAD(&card->ctl_files); >> spin_lock_init(&card->files_lock); >> INIT_LIST_HEAD(&card->files_list); >> +#ifdef CONFIG_SND_CTL_HASHTABLE >> + hash_init(card->ctl_htable); >> +#endif >> mutex_init(&card->memory_mutex); >> #ifdef CONFIG_PM >> init_waitqueue_head(&card->power_sleep); >> diff --git a/sound/soc/Kconfig b/sound/soc/Kconfig >> index 5dcf77af07af..0eb18f8ee6fd 100644 >> --- a/sound/soc/Kconfig >> +++ b/sound/soc/Kconfig >> @@ -58,6 +58,15 @@ config SND_SOC_TOPOLOGY_KUNIT_TEST >> config SND_SOC_ACPI >> tristate >> >> +config SND_CTL_HASHTABLE >> + bool "Add SND CTL hashtable" >> + help >> + This enables hash table in sound card for kcontrols. The traditional way is >> + traversing the linked list of controls and compare each exsiting control with >> + the new kcontrol to find out whether there are duplicate kcontrols, which will >> + consumes much time during bootup. Enable this will use hash table instead of >> + linked list to check new kcontrol and reduce much time for sound card registration. >> + >> # All the supported SoCs >> source "sound/soc/adi/Kconfig" >> source "sound/soc/amd/Kconfig" >> -- >> 2.17.1 >> ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH V0 1/1] asoc: msm: use hashtable to check kcontrol 2022-06-09 18:16 ` Raghu Ballappa Bankapur @ 2022-06-10 6:01 ` Takashi Iwai 0 siblings, 0 replies; 18+ messages in thread From: Takashi Iwai @ 2022-06-10 6:01 UTC (permalink / raw) To: Raghu Ballappa Bankapur Cc: Jaroslav Kysela, Takashi Iwai, Liam Girdwood, Mark Brown, alsa-devel, linux-kernel, Krishna Jha On Thu, 09 Jun 2022 20:16:52 +0200, Raghu Ballappa Bankapur wrote: > > > Hi Takashi > > Our implementation also falls inline with your approach [PATCH RFC] ALSA: > control: Use xarray for faster lookups (kernel.org), but you approach looks to > be clean with xarray. > > is it possible to upstream those changes? I submitted a v2 patch, please check it out. thanks, Takashi ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH V0 1/1] asoc: msm: use hashtable to check kcontrol @ 2022-06-10 6:01 ` Takashi Iwai 0 siblings, 0 replies; 18+ messages in thread From: Takashi Iwai @ 2022-06-10 6:01 UTC (permalink / raw) To: Raghu Ballappa Bankapur Cc: alsa-devel, linux-kernel, Liam Girdwood, Takashi Iwai, Mark Brown, Krishna Jha On Thu, 09 Jun 2022 20:16:52 +0200, Raghu Ballappa Bankapur wrote: > > > Hi Takashi > > Our implementation also falls inline with your approach [PATCH RFC] ALSA: > control: Use xarray for faster lookups (kernel.org), but you approach looks to > be clean with xarray. > > is it possible to upstream those changes? I submitted a v2 patch, please check it out. thanks, Takashi ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH V0 1/1] asoc: msm: use hashtable to check kcontrol 2022-06-10 6:01 ` Takashi Iwai @ 2022-06-13 9:19 ` Raghu Ballappa Bankapur -1 siblings, 0 replies; 18+ messages in thread From: Raghu Ballappa Bankapur @ 2022-06-13 9:19 UTC (permalink / raw) To: Takashi Iwai Cc: Jaroslav Kysela, Takashi Iwai, Liam Girdwood, Mark Brown, alsa-devel, linux-kernel, Krishna Jha Hi Takashi v2 patch looks fine. Can we use this patch to submit to android common kernel? There is code freeze for KMI on june 17. Please share your opinion. Thanks Raghu Bankapur On 6/10/2022 11:31 AM, Takashi Iwai wrote: > On Thu, 09 Jun 2022 20:16:52 +0200, > Raghu Ballappa Bankapur wrote: >> >> Hi Takashi >> >> Our implementation also falls inline with your approach [PATCH RFC] ALSA: >> control: Use xarray for faster lookups (kernel.org), but you approach looks to >> be clean with xarray. >> >> is it possible to upstream those changes? > I submitted a v2 patch, please check it out. > > > thanks, > > Takashi ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH V0 1/1] asoc: msm: use hashtable to check kcontrol @ 2022-06-13 9:19 ` Raghu Ballappa Bankapur 0 siblings, 0 replies; 18+ messages in thread From: Raghu Ballappa Bankapur @ 2022-06-13 9:19 UTC (permalink / raw) To: Takashi Iwai Cc: alsa-devel, linux-kernel, Liam Girdwood, Takashi Iwai, Mark Brown, Krishna Jha Hi Takashi v2 patch looks fine. Can we use this patch to submit to android common kernel? There is code freeze for KMI on june 17. Please share your opinion. Thanks Raghu Bankapur On 6/10/2022 11:31 AM, Takashi Iwai wrote: > On Thu, 09 Jun 2022 20:16:52 +0200, > Raghu Ballappa Bankapur wrote: >> >> Hi Takashi >> >> Our implementation also falls inline with your approach [PATCH RFC] ALSA: >> control: Use xarray for faster lookups (kernel.org), but you approach looks to >> be clean with xarray. >> >> is it possible to upstream those changes? > I submitted a v2 patch, please check it out. > > > thanks, > > Takashi ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH V0 1/1] asoc: msm: use hashtable to check kcontrol 2022-06-13 9:19 ` Raghu Ballappa Bankapur @ 2022-06-13 10:02 ` Takashi Iwai -1 siblings, 0 replies; 18+ messages in thread From: Takashi Iwai @ 2022-06-13 10:02 UTC (permalink / raw) To: Raghu Ballappa Bankapur Cc: Jaroslav Kysela, Takashi Iwai, Liam Girdwood, Mark Brown, alsa-devel, linux-kernel, Krishna Jha On Mon, 13 Jun 2022 11:19:05 +0200, Raghu Ballappa Bankapur wrote: > > Hi Takashi > > v2 patch looks fine. Can we use this patch to submit to android common > kernel? There is code freeze for KMI on june 17. If nothing reported, I'm going to merge the v3 patch (already posted) to for-next branch for 5.19 kernel in this week. Let's see :) thanks, Takashi > > Please share your opinion. > > > Thanks > > Raghu Bankapur > > On 6/10/2022 11:31 AM, Takashi Iwai wrote: > > On Thu, 09 Jun 2022 20:16:52 +0200, > > Raghu Ballappa Bankapur wrote: > >> > >> Hi Takashi > >> > >> Our implementation also falls inline with your approach [PATCH RFC] ALSA: > >> control: Use xarray for faster lookups (kernel.org), but you approach looks to > >> be clean with xarray. > >> > >> is it possible to upstream those changes? > > I submitted a v2 patch, please check it out. > > > > > > thanks, > > > > Takashi > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH V0 1/1] asoc: msm: use hashtable to check kcontrol @ 2022-06-13 10:02 ` Takashi Iwai 0 siblings, 0 replies; 18+ messages in thread From: Takashi Iwai @ 2022-06-13 10:02 UTC (permalink / raw) To: Raghu Ballappa Bankapur Cc: alsa-devel, linux-kernel, Liam Girdwood, Takashi Iwai, Mark Brown, Krishna Jha On Mon, 13 Jun 2022 11:19:05 +0200, Raghu Ballappa Bankapur wrote: > > Hi Takashi > > v2 patch looks fine. Can we use this patch to submit to android common > kernel? There is code freeze for KMI on june 17. If nothing reported, I'm going to merge the v3 patch (already posted) to for-next branch for 5.19 kernel in this week. Let's see :) thanks, Takashi > > Please share your opinion. > > > Thanks > > Raghu Bankapur > > On 6/10/2022 11:31 AM, Takashi Iwai wrote: > > On Thu, 09 Jun 2022 20:16:52 +0200, > > Raghu Ballappa Bankapur wrote: > >> > >> Hi Takashi > >> > >> Our implementation also falls inline with your approach [PATCH RFC] ALSA: > >> control: Use xarray for faster lookups (kernel.org), but you approach looks to > >> be clean with xarray. > >> > >> is it possible to upstream those changes? > > I submitted a v2 patch, please check it out. > > > > > > thanks, > > > > Takashi > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH V0 1/1] asoc: msm: use hashtable to check kcontrol 2022-05-29 8:50 ` [PATCH V0 1/1] " Raghu Bankapur 2022-05-29 9:35 ` Takashi Iwai @ 2022-05-29 10:40 ` kernel test robot 2022-05-30 14:13 ` Mark Brown 2022-05-31 4:46 ` kernel test robot 3 siblings, 0 replies; 18+ messages in thread From: kernel test robot @ 2022-05-29 10:40 UTC (permalink / raw) To: Raghu Bankapur, Jaroslav Kysela, Takashi Iwai, Liam Girdwood, Mark Brown, alsa-devel, linux-kernel Cc: kbuild-all, Krishna Jha Hi Raghu, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on tiwai-sound/for-next] [also build test WARNING on broonie-sound/for-next v5.18 next-20220527] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/intel-lab-lkp/linux/commits/Raghu-Bankapur/asoc-msm-use-hashtable-to-check-kcontrol/20220529-165246 base: https://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git for-next config: m68k-allyesconfig (https://download.01.org/0day-ci/archive/20220529/202205291859.r117Eo4k-lkp@intel.com/config) compiler: m68k-linux-gcc (GCC) 11.3.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/intel-lab-lkp/linux/commit/244d3a3d274761ea83000880d73cc9b0a01b2a4a git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Raghu-Bankapur/asoc-msm-use-hashtable-to-check-kcontrol/20220529-165246 git checkout 244d3a3d274761ea83000880d73cc9b0a01b2a4a # save the config file mkdir build_dir && cp config build_dir/.config COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross W=1 O=build_dir ARCH=m68k SHELL=/bin/bash sound/ If you fix the issue, kindly add following tag where applicable Reported-by: kernel test robot <lkp@intel.com> All warnings (new ones prefixed by >>): >> sound/core/control.c:375:14: warning: no previous prototype for 'snd_ctl_strtoint' [-Wmissing-prototypes] 375 | unsigned int snd_ctl_strtoint(const char *s) | ^~~~~~~~~~~~~~~~ vim +/snd_ctl_strtoint +375 sound/core/control.c 373 374 /* Used to convert the string into int value -- BKDRHash */ > 375 unsigned int snd_ctl_strtoint(const char *s) 376 { 377 unsigned int res = 0; 378 379 while (*s) 380 res = (res << 5) - res + (*s++); 381 382 return (res & 0x7FFFFFFF); 383 } 384 -- 0-DAY CI Kernel Test Service https://01.org/lkp ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH V0 1/1] asoc: msm: use hashtable to check kcontrol 2022-05-29 8:50 ` [PATCH V0 1/1] " Raghu Bankapur @ 2022-05-30 14:13 ` Mark Brown 2022-05-29 10:40 ` kernel test robot ` (2 subsequent siblings) 3 siblings, 0 replies; 18+ messages in thread From: Mark Brown @ 2022-05-30 14:13 UTC (permalink / raw) To: Raghu Bankapur Cc: alsa-devel, linux-kernel, Liam Girdwood, Takashi Iwai, Krishna Jha [-- Attachment #1: Type: text/plain, Size: 731 bytes --] On Sun, May 29, 2022 at 02:20:09PM +0530, Raghu Bankapur wrote: > index 5dcf77af07af..0eb18f8ee6fd 100644 > --- a/sound/soc/Kconfig > +++ b/sound/soc/Kconfig > @@ -58,6 +58,15 @@ config SND_SOC_TOPOLOGY_KUNIT_TEST > config SND_SOC_ACPI > tristate > > +config SND_CTL_HASHTABLE > + bool "Add SND CTL hashtable" > + help > + This enables hash table in sound card for kcontrols. The traditional way is Since the rest of the change is to the ALSA core this should be in the core ALSA Kconfig, though I'm also wondering if there's any reason to make it configurable at all - is there any big downside to just using the hastable all the time? This sort of tuning control tends to be a bit unclear for users. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH V0 1/1] asoc: msm: use hashtable to check kcontrol @ 2022-05-30 14:13 ` Mark Brown 0 siblings, 0 replies; 18+ messages in thread From: Mark Brown @ 2022-05-30 14:13 UTC (permalink / raw) To: Raghu Bankapur Cc: Jaroslav Kysela, Takashi Iwai, Liam Girdwood, alsa-devel, linux-kernel, Krishna Jha [-- Attachment #1: Type: text/plain, Size: 731 bytes --] On Sun, May 29, 2022 at 02:20:09PM +0530, Raghu Bankapur wrote: > index 5dcf77af07af..0eb18f8ee6fd 100644 > --- a/sound/soc/Kconfig > +++ b/sound/soc/Kconfig > @@ -58,6 +58,15 @@ config SND_SOC_TOPOLOGY_KUNIT_TEST > config SND_SOC_ACPI > tristate > > +config SND_CTL_HASHTABLE > + bool "Add SND CTL hashtable" > + help > + This enables hash table in sound card for kcontrols. The traditional way is Since the rest of the change is to the ALSA core this should be in the core ALSA Kconfig, though I'm also wondering if there's any reason to make it configurable at all - is there any big downside to just using the hastable all the time? This sort of tuning control tends to be a bit unclear for users. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH V0 1/1] asoc: msm: use hashtable to check kcontrol 2022-05-29 8:50 ` [PATCH V0 1/1] " Raghu Bankapur ` (2 preceding siblings ...) 2022-05-30 14:13 ` Mark Brown @ 2022-05-31 4:46 ` kernel test robot 3 siblings, 0 replies; 18+ messages in thread From: kernel test robot @ 2022-05-31 4:46 UTC (permalink / raw) To: Raghu Bankapur, Jaroslav Kysela, Takashi Iwai, Liam Girdwood, Mark Brown, alsa-devel, linux-kernel Cc: llvm, kbuild-all, Krishna Jha Hi Raghu, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on tiwai-sound/for-next] [also build test WARNING on broonie-sound/for-next v5.18 next-20220527] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/intel-lab-lkp/linux/commits/Raghu-Bankapur/asoc-msm-use-hashtable-to-check-kcontrol/20220529-165246 base: https://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git for-next config: arm64-randconfig-r036-20220531 (https://download.01.org/0day-ci/archive/20220531/202205311233.abvVh5rg-lkp@intel.com/config) compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project c825abd6b0198fb088d9752f556a70705bc99dfd) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # install arm64 cross compiling tool for clang build # apt-get install binutils-aarch64-linux-gnu # https://github.com/intel-lab-lkp/linux/commit/244d3a3d274761ea83000880d73cc9b0a01b2a4a git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Raghu-Bankapur/asoc-msm-use-hashtable-to-check-kcontrol/20220529-165246 git checkout 244d3a3d274761ea83000880d73cc9b0a01b2a4a # save the config file mkdir build_dir && cp config build_dir/.config COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm64 SHELL=/bin/bash drivers/clk/mediatek/ sound/core/ If you fix the issue, kindly add following tag where applicable Reported-by: kernel test robot <lkp@intel.com> All warnings (new ones prefixed by >>): >> sound/core/control.c:375:14: warning: no previous prototype for function 'snd_ctl_strtoint' [-Wmissing-prototypes] unsigned int snd_ctl_strtoint(const char *s) ^ sound/core/control.c:375:1: note: declare 'static' if the function is not intended to be used outside of this translation unit unsigned int snd_ctl_strtoint(const char *s) ^ static 1 warning generated. vim +/snd_ctl_strtoint +375 sound/core/control.c 373 374 /* Used to convert the string into int value -- BKDRHash */ > 375 unsigned int snd_ctl_strtoint(const char *s) 376 { 377 unsigned int res = 0; 378 379 while (*s) 380 res = (res << 5) - res + (*s++); 381 382 return (res & 0x7FFFFFFF); 383 } 384 -- 0-DAY CI Kernel Test Service https://01.org/lkp ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH V0 0/1] asoc: msm: use hashtable to check kcontrol 2022-05-29 8:50 [PATCH V0 0/1] asoc: msm: use hashtable to check kcontrol Raghu Bankapur @ 2022-05-30 14:08 ` Mark Brown 2022-05-30 14:08 ` Mark Brown 1 sibling, 0 replies; 18+ messages in thread From: Mark Brown @ 2022-05-30 14:08 UTC (permalink / raw) To: Raghu Bankapur Cc: alsa-devel, linux-kernel, Liam Girdwood, Takashi Iwai, Krishna Jha [-- Attachment #1: Type: text/plain, Size: 481 bytes --] On Sun, May 29, 2022 at 02:20:08PM +0530, Raghu Bankapur wrote: > use hashtabe instead of linear list to check kcontrol before > adding them for improving early audio KPI. Please don't send cover letters for single patches, if there is anything that needs saying put it in the changelog of the patch or after the --- if it's administrative stuff. This reduces mail volume and ensures that any important information is recorded in the changelog rather than being lost. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH V0 0/1] asoc: msm: use hashtable to check kcontrol @ 2022-05-30 14:08 ` Mark Brown 0 siblings, 0 replies; 18+ messages in thread From: Mark Brown @ 2022-05-30 14:08 UTC (permalink / raw) To: Raghu Bankapur Cc: Jaroslav Kysela, Takashi Iwai, Liam Girdwood, alsa-devel, linux-kernel, Krishna Jha [-- Attachment #1: Type: text/plain, Size: 481 bytes --] On Sun, May 29, 2022 at 02:20:08PM +0530, Raghu Bankapur wrote: > use hashtabe instead of linear list to check kcontrol before > adding them for improving early audio KPI. Please don't send cover letters for single patches, if there is anything that needs saying put it in the changelog of the patch or after the --- if it's administrative stuff. This reduces mail volume and ensures that any important information is recorded in the changelog rather than being lost. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 18+ messages in thread
[parent not found: <cover.1654252861.git.quic_rbankapu@quicinc.com>]
* [PATCH V0 1/1] asoc: msm: use hashtable to check kcontrol [not found] <cover.1654252861.git.quic_rbankapu@quicinc.com> @ 2022-06-03 10:42 ` Raghu Bankapur 0 siblings, 0 replies; 18+ messages in thread From: Raghu Bankapur @ 2022-06-03 10:42 UTC (permalink / raw) To: Jaroslav Kysela, Takashi Iwai, alsa-devel, linux-kernel Cc: Krishna Jha, Raghu Bankapur use hashtable instead of linear list to check kcontrol before adding them for improving early audio KPI. With this changes we see 600ms improvement in start of audio Change-Id: I1a95d88ef41b48430b57307a0e6a8e82788b4372 Signed-off-by: Raghu Bankapur <quic_rbankapu@quicinc.com> --- include/sound/control.h | 2 + include/sound/core.h | 5 +- sound/core/control.c | 106 +++++++++++++++++++++++----------------- sound/core/init.c | 1 + 4 files changed, 68 insertions(+), 46 deletions(-) diff --git a/include/sound/control.h b/include/sound/control.h index 985c51a8fb74..e50db5c45114 100644 --- a/include/sound/control.h +++ b/include/sound/control.h @@ -70,6 +70,8 @@ struct snd_kcontrol_volatile { struct snd_kcontrol { struct list_head list; /* list of controls */ struct snd_ctl_elem_id id; + struct hlist_node hnode; + unsigned int knametoint; /* kctl name to uint, hash key value */ unsigned int count; /* count of same elements */ snd_kcontrol_info_t *info; snd_kcontrol_get_t *get; diff --git a/include/sound/core.h b/include/sound/core.h index b7e9b58d3c78..90341d6f1573 100644 --- a/include/sound/core.h +++ b/include/sound/core.h @@ -14,6 +14,7 @@ #include <linux/pm.h> /* pm_message_t */ #include <linux/stringify.h> #include <linux/printk.h> +#include <linux/hashtable.h> /* number of supported soundcards */ #ifdef CONFIG_SND_DYNAMIC_MINORS @@ -24,6 +25,8 @@ #define CONFIG_SND_MAJOR 116 /* standard configuration */ +#define SND_CTL_HASH_TABLE_BITS 14 /* buckets numbers: 1 << 14 */ + /* forward declarations */ struct pci_dev; struct module; @@ -103,7 +106,7 @@ struct snd_card { size_t user_ctl_alloc_size; // current memory allocation by user controls. struct list_head controls; /* all controls for this card */ struct list_head ctl_files; /* active control files */ - + DECLARE_HASHTABLE(ctl_htable, SND_CTL_HASH_TABLE_BITS); struct snd_info_entry *proc_root; /* root for soundcard specific files */ struct proc_dir_entry *proc_root_link; /* number link to real id */ diff --git a/sound/core/control.c b/sound/core/control.c index a25c0d64d104..e00a02015837 100644 --- a/sound/core/control.c +++ b/sound/core/control.c @@ -331,43 +331,49 @@ void snd_ctl_free_one(struct snd_kcontrol *kcontrol) } EXPORT_SYMBOL(snd_ctl_free_one); -static bool snd_ctl_remove_numid_conflict(struct snd_card *card, - unsigned int count) +enum snd_ctl_add_mode { + CTL_ADD_EXCLUSIVE, CTL_REPLACE, CTL_ADD_ON_REPLACE, +}; + +char snd_ctl_string[50] = { '\0' }; + +/* Used to convert the string into int value -- BKDRHash */ +static unsigned int snd_ctl_strtoint(const char *s) { - struct snd_kcontrol *kctl; + unsigned int res = 0; - /* Make sure that the ids assigned to the control do not wrap around */ - if (card->last_numid >= UINT_MAX - count) - card->last_numid = 0; + while (*s) + res = (res << 5) - res + (*s++); - list_for_each_entry(kctl, &card->controls, list) { - if (kctl->id.numid < card->last_numid + 1 + count && - kctl->id.numid + kctl->count > card->last_numid + 1) { - card->last_numid = kctl->id.numid + kctl->count - 1; - return true; - } - } - return false; + return (res & 0x7FFFFFFF); } -static int snd_ctl_find_hole(struct snd_card *card, unsigned int count) +/** + * snd_ctl_hash_check - Check the duplicate enrty on snd hashtable + * @card: the card instance + * @nametoint: kctl name to uint + * + * Finds the control instance with the given nametoint from the card. + * + * Return: The pointer of the instance if found, or %NULL if not. + * + */ +static struct snd_kcontrol *snd_ctl_hash_check(struct snd_card *card, + unsigned int nametoint) { - unsigned int iter = 100000; + struct snd_kcontrol *kctl = NULL; - while (snd_ctl_remove_numid_conflict(card, count)) { - if (--iter == 0) { - /* this situation is very unlikely */ - dev_err(card->dev, "unable to allocate new control numid\n"); - return -ENOMEM; - } + if (snd_BUG_ON(!card)) + return NULL; + + hash_for_each_possible(card->ctl_htable, kctl, hnode, nametoint) { + if (kctl->knametoint != nametoint) + continue; + return kctl; } - return 0; + return NULL; } -enum snd_ctl_add_mode { - CTL_ADD_EXCLUSIVE, CTL_REPLACE, CTL_ADD_ON_REPLACE, -}; - /* add/replace a new kcontrol object; call with card->controls_rwsem locked */ static int __snd_ctl_add_replace(struct snd_card *card, struct snd_kcontrol *kcontrol, @@ -382,26 +388,34 @@ static int __snd_ctl_add_replace(struct snd_card *card, if (id.index > UINT_MAX - kcontrol->count) return -EINVAL; - old = snd_ctl_find_id(card, &id); - if (!old) { - if (mode == CTL_REPLACE) - return -EINVAL; - } else { - if (mode == CTL_ADD_EXCLUSIVE) { - dev_err(card->dev, - "control %i:%i:%i:%s:%i is already present\n", - id.iface, id.device, id.subdevice, id.name, - id.index); - return -EBUSY; - } + snprintf(snd_ctl_string, strlen(kcontrol->id.name) + 6, "%s%d%d%d", + kcontrol->id.name, kcontrol->id.iface, kcontrol->id.device, + kcontrol->id.subdevice); - err = snd_ctl_remove(card, old); - if (err < 0) - return err; - } + kcontrol->knametoint = snd_ctl_strtoint(snd_ctl_string); + if (kcontrol->knametoint < 0) + return -EINVAL; - if (snd_ctl_find_hole(card, kcontrol->count) < 0) - return -ENOMEM; + old = snd_ctl_hash_check(card, kcontrol->knametoint); + if (old) { + old = snd_ctl_find_id(card, &id); + if (!old) { + if (mode == CTL_REPLACE) + return -EINVAL; + } else { + if (mode == CTL_ADD_EXCLUSIVE) { + dev_err(card->dev, + "control %i:%i:%i:%s:%i is already present\n", + id.iface, id.device, id.subdevice, id.name, + id.index); + return -EBUSY; + } + + err = snd_ctl_remove(card, old); + if (err < 0) + return err; + } + } list_add_tail(&kcontrol->list, &card->controls); card->controls_count += kcontrol->count; @@ -411,6 +425,8 @@ static int __snd_ctl_add_replace(struct snd_card *card, for (idx = 0; idx < kcontrol->count; idx++) snd_ctl_notify_one(card, SNDRV_CTL_EVENT_MASK_ADD, kcontrol, idx); + hash_add(card->ctl_htable, &kcontrol->hnode, kcontrol->knametoint); + return 0; } diff --git a/sound/core/init.c b/sound/core/init.c index 31ba7024e3ad..24138902e5f2 100644 --- a/sound/core/init.c +++ b/sound/core/init.c @@ -284,6 +284,7 @@ static int snd_card_init(struct snd_card *card, struct device *parent, INIT_LIST_HEAD(&card->ctl_files); spin_lock_init(&card->files_lock); INIT_LIST_HEAD(&card->files_list); + hash_init(card->ctl_htable); mutex_init(&card->memory_mutex); #ifdef CONFIG_PM init_waitqueue_head(&card->power_sleep); -- 2.17.1 ^ permalink raw reply related [flat|nested] 18+ messages in thread
end of thread, other threads:[~2022-06-13 10:03 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-29 8:50 [PATCH V0 0/1] asoc: msm: use hashtable to check kcontrol Raghu Bankapur
2022-05-29 8:50 ` [PATCH V0 1/1] " Raghu Bankapur
2022-05-29 9:35 ` Takashi Iwai
2022-05-29 9:35 ` Takashi Iwai
2022-06-09 18:16 ` Raghu Ballappa Bankapur
2022-06-10 6:01 ` Takashi Iwai
2022-06-10 6:01 ` Takashi Iwai
2022-06-13 9:19 ` Raghu Ballappa Bankapur
2022-06-13 9:19 ` Raghu Ballappa Bankapur
2022-06-13 10:02 ` Takashi Iwai
2022-06-13 10:02 ` Takashi Iwai
2022-05-29 10:40 ` kernel test robot
2022-05-30 14:13 ` Mark Brown
2022-05-30 14:13 ` Mark Brown
2022-05-31 4:46 ` kernel test robot
2022-05-30 14:08 ` [PATCH V0 0/1] " Mark Brown
2022-05-30 14:08 ` Mark Brown
[not found] <cover.1654252861.git.quic_rbankapu@quicinc.com>
2022-06-03 10:42 ` [PATCH V0 1/1] " Raghu Bankapur
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.