[PATCH 0/2][RFC] Refactor snd primitives refcounters
From: Curtis Malainey cujomalainey@chromium.org
As previously identified in [1] there are some issues with how kobjs are handled in sound/core. The solution provided in [2] is a workaround for the issues to fix the failures.
This series is a first attempt at the larger refactor needed to properly handle the objects.
[1] https://mailman.alsa-project.org/hyperkitty/list/alsa-devel@alsa-project.org... [2] https://mailman.alsa-project.org/hyperkitty/list/alsa-devel@alsa-project.org...
Curtis Malainey (2): ALSA: core: add snd_device_init ALSA: core: split control primitives out of snd_card
include/sound/control.h | 1 + include/sound/core.h | 34 +-- sound/core/control.c | 330 +++++++++++++---------- sound/core/control_compat.c | 8 +- sound/core/control_led.c | 18 +- sound/core/init.c | 33 +-- sound/pci/hda/hda_codec.c | 3 +- sound/soc/intel/atom/sst-atom-controls.c | 8 +- sound/soc/soc-card.c | 2 +- sound/usb/media.c | 2 +- 10 files changed, 249 insertions(+), 190 deletions(-)
From: Curtis Malainey cujomalainey@chromium.org
Begin allowing refactored modules to allocate their own device but use a common initialization procedure for their devices.
Signed-off-by: Curtis Malainey cujomalainey@chromium.org --- include/sound/core.h | 1 + sound/core/init.c | 19 ++++++++++++++++--- 2 files changed, 17 insertions(+), 3 deletions(-)
diff --git a/include/sound/core.h b/include/sound/core.h index dfef0c9d4b9f7..a4744e142c7e3 100644 --- a/include/sound/core.h +++ b/include/sound/core.h @@ -240,6 +240,7 @@ extern struct dentry *sound_debugfs_root; void snd_request_card(int card);
int snd_device_alloc(struct device **dev_p, struct snd_card *card); +void snd_device_init(struct device *dev, struct snd_card *card);
int snd_register_device(int type, struct snd_card *card, int dev, const struct file_operations *f_ops, diff --git a/sound/core/init.c b/sound/core/init.c index d61bde1225f23..37a8e4791f781 100644 --- a/sound/core/init.c +++ b/sound/core/init.c @@ -132,15 +132,28 @@ int snd_device_alloc(struct device **dev_p, struct snd_card *card) dev = kzalloc(sizeof(*dev), GFP_KERNEL); if (!dev) return -ENOMEM; + snd_device_init(dev, card); + *dev_p = dev; + return 0; +} +EXPORT_SYMBOL_GPL(snd_device_alloc); + +/** + * snd_device_init - Initialize struct device for sound devices + * @dev_p: pointer to store the allocated device + * @card: card to assign, optional + * + * For releasing the allocated device, call put_device(). + */ +void snd_device_init(struct device *dev, struct snd_card *card) +{ device_initialize(dev); if (card) dev->parent = &card->card_dev; dev->class = &sound_class; dev->release = default_release_alloc; - *dev_p = dev; - return 0; } -EXPORT_SYMBOL_GPL(snd_device_alloc); +EXPORT_SYMBOL_GPL(snd_device_init);
static int snd_card_init(struct snd_card *card, struct device *parent, int idx, const char *xid, struct module *module,
Hi,
kernel test robot noticed the following build warnings:
[auto build test WARNING on tiwai-sound/for-next] [also build test WARNING on next-20230824] [cannot apply to tiwai-sound/for-linus broonie-sound/for-next linus/master v6.5-rc7] [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#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/cujomalainey-chromium-org/ALS... base: https://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git for-next patch link: https://lore.kernel.org/r/20230824210339.1126993-2-cujomalainey%40chromium.o... patch subject: [PATCH 1/2] ALSA: core: add snd_device_init config: m68k-allyesconfig (https://download.01.org/0day-ci/archive/20230825/202308250609.8A2yaCZs-lkp@i...) compiler: m68k-linux-gcc (GCC) 13.2.0 reproduce: (https://download.01.org/0day-ci/archive/20230825/202308250609.8A2yaCZs-lkp@i...)
If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot lkp@intel.com | Closes: https://lore.kernel.org/oe-kbuild-all/202308250609.8A2yaCZs-lkp@intel.com/
All warnings (new ones prefixed by >>):
sound/core/init.c:149: warning: Function parameter or member 'dev' not described in 'snd_device_init' sound/core/init.c:149: warning: Excess function parameter 'dev_p' description in 'snd_device_init'
vim +149 sound/core/init.c
140 141 /** 142 * snd_device_init - Initialize struct device for sound devices 143 * @dev_p: pointer to store the allocated device 144 * @card: card to assign, optional 145 * 146 * For releasing the allocated device, call put_device(). 147 */ 148 void snd_device_init(struct device *dev, struct snd_card *card)
149 {
150 device_initialize(dev); 151 if (card) 152 dev->parent = &card->card_dev; 153 dev->class = &sound_class; 154 dev->release = default_release_alloc; 155 } 156 EXPORT_SYMBOL_GPL(snd_device_init); 157
On Thu, 24 Aug 2023 23:02:52 +0200, cujomalainey@chromium.org wrote:
From: Curtis Malainey cujomalainey@chromium.org
Begin allowing refactored modules to allocate their own device but use a common initialization procedure for their devices.
Signed-off-by: Curtis Malainey cujomalainey@chromium.org
include/sound/core.h | 1 + sound/core/init.c | 19 ++++++++++++++++--- 2 files changed, 17 insertions(+), 3 deletions(-)
diff --git a/include/sound/core.h b/include/sound/core.h index dfef0c9d4b9f7..a4744e142c7e3 100644 --- a/include/sound/core.h +++ b/include/sound/core.h @@ -240,6 +240,7 @@ extern struct dentry *sound_debugfs_root; void snd_request_card(int card);
int snd_device_alloc(struct device **dev_p, struct snd_card *card); +void snd_device_init(struct device *dev, struct snd_card *card);
int snd_register_device(int type, struct snd_card *card, int dev, const struct file_operations *f_ops, diff --git a/sound/core/init.c b/sound/core/init.c index d61bde1225f23..37a8e4791f781 100644 --- a/sound/core/init.c +++ b/sound/core/init.c @@ -132,15 +132,28 @@ int snd_device_alloc(struct device **dev_p, struct snd_card *card) dev = kzalloc(sizeof(*dev), GFP_KERNEL); if (!dev) return -ENOMEM;
- snd_device_init(dev, card);
- *dev_p = dev;
- return 0;
+} +EXPORT_SYMBOL_GPL(snd_device_alloc);
+/**
- snd_device_init - Initialize struct device for sound devices
- @dev_p: pointer to store the allocated device
- @card: card to assign, optional
- For releasing the allocated device, call put_device().
- */
+void snd_device_init(struct device *dev, struct snd_card *card) +{ device_initialize(dev); if (card) dev->parent = &card->card_dev; dev->class = &sound_class; dev->release = default_release_alloc;
- *dev_p = dev;
- return 0;
} -EXPORT_SYMBOL_GPL(snd_device_alloc); +EXPORT_SYMBOL_GPL(snd_device_init);
This will call kfree() at the default release. It should be avoided for this case, no?
Also, it's worth that this practically a kind of revive of the old API that was dropped in the commit 01ed7f3535a2.
thanks,
Takashi
From: Curtis Malainey cujomalainey@chromium.org
Having two kobj in the same struct is broken at its core. This splits card_dev from ctl_dev so they can properly refcount and release on their own schedules without the workaround of having them being just a pointer.
Signed-off-by: Curtis Malainey cujomalainey@chromium.org --- include/sound/control.h | 1 + include/sound/core.h | 33 +-- sound/core/control.c | 330 +++++++++++++---------- sound/core/control_compat.c | 8 +- sound/core/control_led.c | 18 +- sound/core/init.c | 14 +- sound/pci/hda/hda_codec.c | 3 +- sound/soc/intel/atom/sst-atom-controls.c | 8 +- sound/soc/soc-card.c | 2 +- sound/usb/media.c | 2 +- 10 files changed, 232 insertions(+), 187 deletions(-)
diff --git a/include/sound/control.h b/include/sound/control.h index 9a4f4f7138da8..32920f33eb47e 100644 --- a/include/sound/control.h +++ b/include/sound/control.h @@ -128,6 +128,7 @@ typedef int (*snd_kctl_ioctl_func_t) (struct snd_card * card, struct snd_ctl_file * control, unsigned int cmd, unsigned long arg);
+int snd_control_new(struct snd_card * card); void snd_ctl_notify(struct snd_card * card, unsigned int mask, struct snd_ctl_elem_id * id); void snd_ctl_notify_one(struct snd_card * card, unsigned int mask, struct snd_kcontrol * kctl, unsigned int ioff);
diff --git a/include/sound/core.h b/include/sound/core.h index a4744e142c7e3..93048a7a800f4 100644 --- a/include/sound/core.h +++ b/include/sound/core.h @@ -95,21 +95,7 @@ struct snd_card { void (*private_free) (struct snd_card *card); /* callback for freeing of private data */ struct list_head devices; /* devices */ - - struct device *ctl_dev; /* control device */ - unsigned int last_numid; /* last used numeric ID */ - struct rw_semaphore controls_rwsem; /* controls lock (list and values) */ - rwlock_t ctl_files_rwlock; /* ctl_files list lock */ - int controls_count; /* count of all controls */ - 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_FAST_LOOKUP - struct xarray ctl_numids; /* hash table for numids */ - struct xarray ctl_hash; /* hash table for ctl id matching */ - bool ctl_hash_collision; /* ctl_hash collision seen? */ -#endif - + struct snd_control *ctl; /* control devices */ struct snd_info_entry *proc_root; /* root for soundcard specific files */ struct proc_dir_entry *proc_root_link; /* number link to real id */
@@ -147,6 +133,23 @@ struct snd_card { #endif };
+struct snd_control +{ + struct device dev; /* control device */ + struct rw_semaphore controls_rwsem; /* controls lock (list and values) */ + rwlock_t files_rwlock; /* ctl_files list lock */ + int controls_count; /* count of all controls */ + size_t user_ctl_alloc_size; // current memory allocation by user controls. + struct list_head controls; /* all controls for this card */ + struct list_head files; /* active control files */ + unsigned int last_numid; /* last used numeric ID */ +#ifdef CONFIG_SND_CTL_FAST_LOOKUP + struct xarray numids; /* hash table for numids */ + struct xarray hash; /* hash table for ctl id matching */ + bool hash_collision; /* ctl_hash collision seen? */ +#endif +}; + #define dev_to_snd_card(p) container_of(p, struct snd_card, card_dev)
#ifdef CONFIG_PM diff --git a/sound/core/control.c b/sound/core/control.c index 59c8658966d4c..09066d05a8800 100644 --- a/sound/core/control.c +++ b/sound/core/control.c @@ -41,12 +41,49 @@ static struct snd_ctl_layer_ops *snd_ctl_layer;
static int snd_ctl_remove_locked(struct snd_card *card, struct snd_kcontrol *kcontrol); +/** + * snd_control_new - Allocate and initialize snd_control + * @card: the card to be initialized + * + * This function creates and initializes snd_control + */ +int snd_control_new(struct snd_card *card) +{ + struct snd_control *ctl = kzalloc(sizeof(*ctl), GFP_KERNEL); + int err; + + if (snd_BUG_ON(!card)) + return -EINVAL; + if (!ctl) + return -ENOMEM; + + init_rwsem(&ctl->controls_rwsem); + rwlock_init(&ctl->files_rwlock); + INIT_LIST_HEAD(&ctl->controls); + INIT_LIST_HEAD(&ctl->files); +#ifdef CONFIG_SND_CTL_FAST_LOOKUP + xa_init(&ctl->numids); + xa_init(&ctl->hash); +#endif + card->ctl = ctl; + /* the control interface cannot be accessed from the user space until */ + /* snd_cards_bitmask and snd_cards are set with snd_card_register */ + err = snd_ctl_create(card); + if (err < 0) { + dev_err(card->dev, "unable to register control minors\n"); + kfree(ctl); + return err; + } + return 0; +} +EXPORT_SYMBOL(snd_control_new);
static int snd_ctl_open(struct inode *inode, struct file *file) { unsigned long flags; struct snd_card *card; - struct snd_ctl_file *ctl; + struct snd_control *ctl; + struct snd_ctl_file *ctlf; int i, err;
err = stream_open(inode, file); @@ -58,6 +95,7 @@ static int snd_ctl_open(struct inode *inode, struct file *file) err = -ENODEV; goto __error1; } + ctl = card->ctl; err = snd_card_file_add(card, file); if (err < 0) { err = -ENODEV; @@ -67,22 +105,22 @@ static int snd_ctl_open(struct inode *inode, struct file *file) err = -EFAULT; goto __error2; } - ctl = kzalloc(sizeof(*ctl), GFP_KERNEL); - if (ctl == NULL) { + ctlf = kzalloc(sizeof(*ctlf), GFP_KERNEL); + if (ctlf == NULL) { err = -ENOMEM; goto __error; } - INIT_LIST_HEAD(&ctl->events); - init_waitqueue_head(&ctl->change_sleep); - spin_lock_init(&ctl->read_lock); - ctl->card = card; + INIT_LIST_HEAD(&ctlf->events); + init_waitqueue_head(&ctlf->change_sleep); + spin_lock_init(&ctlf->read_lock); + ctlf->card = card; for (i = 0; i < SND_CTL_SUBDEV_ITEMS; i++) - ctl->preferred_subdevice[i] = -1; - ctl->pid = get_pid(task_pid(current)); - file->private_data = ctl; - write_lock_irqsave(&card->ctl_files_rwlock, flags); - list_add_tail(&ctl->list, &card->ctl_files); - write_unlock_irqrestore(&card->ctl_files_rwlock, flags); + ctlf->preferred_subdevice[i] = -1; + ctlf->pid = get_pid(task_pid(current)); + file->private_data = ctlf; + write_lock_irqsave(&ctl->files_rwlock, flags); + list_add_tail(&ctlf->list, &ctl->files); + write_unlock_irqrestore(&ctl->files_rwlock, flags); snd_card_unref(card); return 0;
@@ -113,27 +151,26 @@ static void snd_ctl_empty_read_queue(struct snd_ctl_file * ctl) static int snd_ctl_release(struct inode *inode, struct file *file) { unsigned long flags; - struct snd_card *card; - struct snd_ctl_file *ctl; + struct snd_ctl_file *ctlf = file->private_data; + struct snd_card *card = ctlf->card; + struct snd_control *ctl = card->ctl; struct snd_kcontrol *control; unsigned int idx;
- ctl = file->private_data; file->private_data = NULL; - card = ctl->card; - write_lock_irqsave(&card->ctl_files_rwlock, flags); - list_del(&ctl->list); - write_unlock_irqrestore(&card->ctl_files_rwlock, flags); - down_write(&card->controls_rwsem); - list_for_each_entry(control, &card->controls, list) + write_lock_irqsave(&ctl->files_rwlock, flags); + list_del(&ctlf->list); + write_unlock_irqrestore(&ctl->files_rwlock, flags); + down_write(&ctl->controls_rwsem); + list_for_each_entry(control, &ctl->controls, list) for (idx = 0; idx < control->count; idx++) - if (control->vd[idx].owner == ctl) + if (control->vd[idx].owner == ctlf) control->vd[idx].owner = NULL; - up_write(&card->controls_rwsem); - snd_fasync_free(ctl->fasync); - snd_ctl_empty_read_queue(ctl); - put_pid(ctl->pid); - kfree(ctl); + up_write(&ctl->controls_rwsem); + snd_fasync_free(ctlf->fasync); + snd_ctl_empty_read_queue(ctlf); + put_pid(ctlf->pid); + kfree(ctlf); module_put(card->module); snd_card_file_remove(card, file); return 0; @@ -160,11 +197,11 @@ void snd_ctl_notify(struct snd_card *card, unsigned int mask, return; if (card->shutdown) return; - read_lock_irqsave(&card->ctl_files_rwlock, flags); + read_lock_irqsave(&card->ctl->files_rwlock, flags); #if IS_ENABLED(CONFIG_SND_MIXER_OSS) card->mixer_oss_change_count++; #endif - list_for_each_entry(ctl, &card->ctl_files, list) { + list_for_each_entry(ctl, &card->ctl->files, list) { if (!ctl->subscribed) continue; spin_lock(&ctl->read_lock); @@ -187,7 +224,7 @@ void snd_ctl_notify(struct snd_card *card, unsigned int mask, spin_unlock(&ctl->read_lock); snd_kill_fasync(ctl->fasync, SIGIO, POLL_IN); } - read_unlock_irqrestore(&card->ctl_files_rwlock, flags); + read_unlock_irqrestore(&card->ctl->files_rwlock, flags); } EXPORT_SYMBOL(snd_ctl_notify);
@@ -341,13 +378,13 @@ static bool snd_ctl_remove_numid_conflict(struct snd_card *card, struct snd_kcontrol *kctl;
/* Make sure that the ids assigned to the control do not wrap around */ - if (card->last_numid >= UINT_MAX - count) - card->last_numid = 0; + if (card->ctl->last_numid >= UINT_MAX - count) + card->ctl->last_numid = 0;
- 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; + list_for_each_entry(kctl, &card->ctl->controls, list) { + if (kctl->id.numid < card->ctl->last_numid + 1 + count && + kctl->id.numid + kctl->count > card->ctl->last_numid + 1) { + card->ctl->last_numid = kctl->id.numid + kctl->count - 1; return true; } } @@ -406,18 +443,19 @@ static void add_hash_entries(struct snd_card *card, struct snd_kcontrol *kcontrol) { struct snd_ctl_elem_id id = kcontrol->id; + struct snd_control *ctl = card->ctl; int i;
- xa_store_range(&card->ctl_numids, kcontrol->id.numid, + xa_store_range(&ctl->numids, kcontrol->id.numid, kcontrol->id.numid + kcontrol->count - 1, kcontrol, GFP_KERNEL);
for (i = 0; i < kcontrol->count; i++) { id.index = kcontrol->id.index + i; - if (xa_insert(&card->ctl_hash, get_ctl_id_hash(&id), + if (xa_insert(&ctl->hash, get_ctl_id_hash(&id), kcontrol, GFP_KERNEL)) { /* skip hash for this entry, noting we had collision */ - card->ctl_hash_collision = true; + ctl->hash_collision = true; dev_dbg(card->dev, "ctl_hash collision %d:%s:%d\n", id.iface, id.name, id.index); } @@ -429,17 +467,18 @@ static void remove_hash_entries(struct snd_card *card, struct snd_kcontrol *kcontrol) { struct snd_ctl_elem_id id = kcontrol->id; + struct snd_control *ctl = card->ctl; struct snd_kcontrol *matched; unsigned long h; int i;
for (i = 0; i < kcontrol->count; i++) { - xa_erase(&card->ctl_numids, id.numid); + xa_erase(&ctl->numids, id.numid); h = get_ctl_id_hash(&id); - matched = xa_load(&card->ctl_hash, h); + matched = xa_load(&ctl->hash, h); if (matched && (matched == kcontrol || elem_id_matches(matched, &id))) - xa_erase(&card->ctl_hash, h); + xa_erase(&ctl->hash, h); id.index++; id.numid++; } @@ -469,7 +508,7 @@ static int __snd_ctl_add_replace(struct snd_card *card, struct snd_kcontrol *old; int err;
- lockdep_assert_held_write(&card->controls_rwsem); + lockdep_assert_held_write(&card->ctl->controls_rwsem);
id = kcontrol->id; if (id.index > UINT_MAX - kcontrol->count) @@ -496,10 +535,10 @@ static int __snd_ctl_add_replace(struct snd_card *card, if (snd_ctl_find_hole(card, kcontrol->count) < 0) return -ENOMEM;
- list_add_tail(&kcontrol->list, &card->controls); - card->controls_count += kcontrol->count; - kcontrol->id.numid = card->last_numid + 1; - card->last_numid += kcontrol->count; + list_add_tail(&kcontrol->list, &card->ctl->controls); + card->ctl->controls_count += kcontrol->count; + kcontrol->id.numid = card->ctl->last_numid + 1; + card->ctl->last_numid += kcontrol->count;
add_hash_entries(card, kcontrol);
@@ -520,9 +559,9 @@ static int snd_ctl_add_replace(struct snd_card *card, if (snd_BUG_ON(!card || !kcontrol->info)) goto error;
- down_write(&card->controls_rwsem); + down_write(&card->ctl->controls_rwsem); err = __snd_ctl_add_replace(card, kcontrol, mode); - up_write(&card->controls_rwsem); + up_write(&card->ctl->controls_rwsem); if (err < 0) goto error; return 0; @@ -580,7 +619,7 @@ static int __snd_ctl_remove(struct snd_card *card, { unsigned int idx;
- lockdep_assert_held_write(&card->controls_rwsem); + lockdep_assert_held_write(&card->ctl->controls_rwsem);
if (snd_BUG_ON(!card || !kcontrol)) return -EINVAL; @@ -589,7 +628,7 @@ static int __snd_ctl_remove(struct snd_card *card, if (remove_hash) remove_hash_entries(card, kcontrol);
- card->controls_count -= kcontrol->count; + card->ctl->controls_count -= kcontrol->count; for (idx = 0; idx < kcontrol->count; idx++) snd_ctl_notify_one(card, SNDRV_CTL_EVENT_MASK_REMOVE, kcontrol, idx); snd_ctl_free_one(kcontrol); @@ -618,9 +657,9 @@ int snd_ctl_remove(struct snd_card *card, struct snd_kcontrol *kcontrol) { int ret;
- down_write(&card->controls_rwsem); + down_write(&card->ctl->controls_rwsem); ret = snd_ctl_remove_locked(card, kcontrol); - up_write(&card->controls_rwsem); + up_write(&card->ctl->controls_rwsem); return ret; } EXPORT_SYMBOL(snd_ctl_remove); @@ -640,14 +679,14 @@ int snd_ctl_remove_id(struct snd_card *card, struct snd_ctl_elem_id *id) struct snd_kcontrol *kctl; int ret;
- down_write(&card->controls_rwsem); + down_write(&card->ctl->controls_rwsem); kctl = snd_ctl_find_id_locked(card, id); if (kctl == NULL) { - up_write(&card->controls_rwsem); + up_write(&card->ctl->controls_rwsem); return -ENOENT; } ret = snd_ctl_remove_locked(card, kctl); - up_write(&card->controls_rwsem); + up_write(&card->ctl->controls_rwsem); return ret; } EXPORT_SYMBOL(snd_ctl_remove_id); @@ -669,7 +708,7 @@ static int snd_ctl_remove_user_ctl(struct snd_ctl_file * file, struct snd_kcontrol *kctl; int idx, ret;
- down_write(&card->controls_rwsem); + down_write(&card->ctl->controls_rwsem); kctl = snd_ctl_find_id_locked(card, id); if (kctl == NULL) { ret = -ENOENT; @@ -686,7 +725,7 @@ static int snd_ctl_remove_user_ctl(struct snd_ctl_file * file, } ret = snd_ctl_remove_locked(card, kctl); error: - up_write(&card->controls_rwsem); + up_write(&card->ctl->controls_rwsem); return ret; }
@@ -710,7 +749,7 @@ int snd_ctl_activate_id(struct snd_card *card, struct snd_ctl_elem_id *id, unsigned int index_offset; int ret;
- down_write(&card->controls_rwsem); + down_write(&card->ctl->controls_rwsem); kctl = snd_ctl_find_id_locked(card, id); if (kctl == NULL) { ret = -ENOENT; @@ -729,13 +768,13 @@ int snd_ctl_activate_id(struct snd_card *card, struct snd_ctl_elem_id *id, vd->access |= SNDRV_CTL_ELEM_ACCESS_INACTIVE; } snd_ctl_build_ioff(id, kctl, index_offset); - downgrade_write(&card->controls_rwsem); + downgrade_write(&card->ctl->controls_rwsem); snd_ctl_notify_one(card, SNDRV_CTL_EVENT_MASK_INFO, kctl, index_offset); - up_read(&card->controls_rwsem); + up_read(&card->ctl->controls_rwsem); return 1;
unlock: - up_write(&card->controls_rwsem); + up_write(&card->ctl->controls_rwsem); return ret; } EXPORT_SYMBOL_GPL(snd_ctl_activate_id); @@ -764,10 +803,10 @@ int snd_ctl_rename_id(struct snd_card *card, struct snd_ctl_elem_id *src_id, struct snd_kcontrol *kctl; int saved_numid;
- down_write(&card->controls_rwsem); + down_write(&card->ctl->controls_rwsem); kctl = snd_ctl_find_id_locked(card, src_id); if (kctl == NULL) { - up_write(&card->controls_rwsem); + up_write(&card->ctl->controls_rwsem); return -ENOENT; } saved_numid = kctl->id.numid; @@ -775,7 +814,7 @@ int snd_ctl_rename_id(struct snd_card *card, struct snd_ctl_elem_id *src_id, kctl->id = *dst_id; kctl->id.numid = saved_numid; add_hash_entries(card, kctl); - up_write(&card->controls_rwsem); + up_write(&card->ctl->controls_rwsem); return 0; } EXPORT_SYMBOL(snd_ctl_rename_id); @@ -793,7 +832,7 @@ EXPORT_SYMBOL(snd_ctl_rename_id); void snd_ctl_rename(struct snd_card *card, struct snd_kcontrol *kctl, const char *name) { - down_write(&card->controls_rwsem); + down_write(&card->ctl->controls_rwsem); remove_hash_entries(card, kctl);
if (strscpy(kctl->id.name, name, sizeof(kctl->id.name)) < 0) @@ -801,7 +840,7 @@ void snd_ctl_rename(struct snd_card *card, struct snd_kcontrol *kctl, name, kctl->id.name);
add_hash_entries(card, kctl); - up_write(&card->controls_rwsem); + up_write(&card->ctl->controls_rwsem); } EXPORT_SYMBOL(snd_ctl_rename);
@@ -836,9 +875,9 @@ snd_ctl_find_numid_locked(struct snd_card *card, unsigned int numid) { if (snd_BUG_ON(!card || !numid)) return NULL; - lockdep_assert_held(&card->controls_rwsem); + lockdep_assert_held(&card->ctl->controls_rwsem); #ifdef CONFIG_SND_CTL_FAST_LOOKUP - return xa_load(&card->ctl_numids, numid); + return xa_load(&card->ctl->numids, numid); #else return snd_ctl_find_numid_slow(card, numid); #endif @@ -861,9 +900,9 @@ struct snd_kcontrol *snd_ctl_find_numid(struct snd_card *card, { struct snd_kcontrol *kctl;
- down_read(&card->controls_rwsem); + down_read(&card->ctl->controls_rwsem); kctl = snd_ctl_find_numid_locked(card, numid); - up_read(&card->controls_rwsem); + up_read(&card->ctl->controls_rwsem); return kctl; } EXPORT_SYMBOL(snd_ctl_find_numid); @@ -887,18 +926,18 @@ struct snd_kcontrol *snd_ctl_find_id_locked(struct snd_card *card,
if (snd_BUG_ON(!card || !id)) return NULL; - lockdep_assert_held(&card->controls_rwsem); + lockdep_assert_held(&card->ctl->controls_rwsem); if (id->numid != 0) return snd_ctl_find_numid_locked(card, id->numid); #ifdef CONFIG_SND_CTL_FAST_LOOKUP - kctl = xa_load(&card->ctl_hash, get_ctl_id_hash(id)); + kctl = xa_load(&card->ctl->hash, get_ctl_id_hash(id)); if (kctl && elem_id_matches(kctl, id)) return kctl; - if (!card->ctl_hash_collision) + if (!card->ctl->hash_collision) return NULL; /* we can rely on only hash table */ #endif /* no matching in hash table - try all as the last resort */ - list_for_each_entry(kctl, &card->controls, list) + list_for_each_entry(kctl, &card->ctl->controls, list) if (elem_id_matches(kctl, id)) return kctl;
@@ -922,9 +961,9 @@ struct snd_kcontrol *snd_ctl_find_id(struct snd_card *card, { struct snd_kcontrol *kctl;
- down_read(&card->controls_rwsem); + down_read(&card->ctl->controls_rwsem); kctl = snd_ctl_find_id_locked(card, id); - up_read(&card->controls_rwsem); + up_read(&card->ctl->controls_rwsem); return kctl; } EXPORT_SYMBOL(snd_ctl_find_id); @@ -965,11 +1004,11 @@ static int snd_ctl_elem_list(struct snd_card *card, offset = list->offset; space = list->space;
- down_read(&card->controls_rwsem); - list->count = card->controls_count; + down_read(&card->ctl->controls_rwsem); + list->count = card->ctl->controls_count; list->used = 0; if (space > 0) { - list_for_each_entry(kctl, &card->controls, list) { + list_for_each_entry(kctl, &card->ctl->controls, list) { if (offset >= kctl->count) { offset -= kctl->count; continue; @@ -989,7 +1028,7 @@ static int snd_ctl_elem_list(struct snd_card *card, } } out: - up_read(&card->controls_rwsem); + up_read(&card->ctl->controls_rwsem); return err; }
@@ -1240,13 +1279,13 @@ static int snd_ctl_elem_info(struct snd_ctl_file *ctl, struct snd_kcontrol *kctl; int result;
- down_read(&card->controls_rwsem); + down_read(&card->ctl->controls_rwsem); kctl = snd_ctl_find_id_locked(card, &info->id); if (kctl == NULL) result = -ENOENT; else result = __snd_ctl_elem_info(card, kctl, info, ctl); - up_read(&card->controls_rwsem); + up_read(&card->ctl->controls_rwsem); return result; }
@@ -1279,7 +1318,7 @@ static int snd_ctl_elem_read(struct snd_card *card, const u32 pattern = 0xdeadbeef; int ret;
- down_read(&card->controls_rwsem); + down_read(&card->ctl->controls_rwsem); kctl = snd_ctl_find_id_locked(card, &control->id); if (kctl == NULL) { ret = -ENOENT; @@ -1323,7 +1362,7 @@ static int snd_ctl_elem_read(struct snd_card *card, goto unlock; } unlock: - up_read(&card->controls_rwsem); + up_read(&card->ctl->controls_rwsem); return ret; }
@@ -1356,10 +1395,10 @@ static int snd_ctl_elem_write(struct snd_card *card, struct snd_ctl_file *file, unsigned int index_offset; int result;
- down_write(&card->controls_rwsem); + down_write(&card->ctl->controls_rwsem); kctl = snd_ctl_find_id_locked(card, &control->id); if (kctl == NULL) { - up_write(&card->controls_rwsem); + up_write(&card->ctl->controls_rwsem); return -ENOENT; }
@@ -1367,7 +1406,7 @@ static int snd_ctl_elem_write(struct snd_card *card, struct snd_ctl_file *file, vd = &kctl->vd[index_offset]; if (!(vd->access & SNDRV_CTL_ELEM_ACCESS_WRITE) || kctl->put == NULL || (file && vd->owner && vd->owner != file)) { - up_write(&card->controls_rwsem); + up_write(&card->ctl->controls_rwsem); return -EPERM; }
@@ -1388,16 +1427,16 @@ static int snd_ctl_elem_write(struct snd_card *card, struct snd_ctl_file *file, result = kctl->put(kctl, control); snd_power_unref(card); if (result < 0) { - up_write(&card->controls_rwsem); + up_write(&card->ctl->controls_rwsem); return result; }
if (result > 0) { - downgrade_write(&card->controls_rwsem); + downgrade_write(&card->ctl->controls_rwsem); snd_ctl_notify_one(card, SNDRV_CTL_EVENT_MASK_VALUE, kctl, index_offset); - up_read(&card->controls_rwsem); + up_read(&card->ctl->controls_rwsem); } else { - up_write(&card->controls_rwsem); + up_write(&card->ctl->controls_rwsem); }
return 0; @@ -1437,7 +1476,7 @@ static int snd_ctl_elem_lock(struct snd_ctl_file *file,
if (copy_from_user(&id, _id, sizeof(id))) return -EFAULT; - down_write(&card->controls_rwsem); + down_write(&card->ctl->controls_rwsem); kctl = snd_ctl_find_id_locked(card, &id); if (kctl == NULL) { result = -ENOENT; @@ -1450,7 +1489,7 @@ static int snd_ctl_elem_lock(struct snd_ctl_file *file, result = 0; } } - up_write(&card->controls_rwsem); + up_write(&card->ctl->controls_rwsem); return result; }
@@ -1465,7 +1504,7 @@ static int snd_ctl_elem_unlock(struct snd_ctl_file *file,
if (copy_from_user(&id, _id, sizeof(id))) return -EFAULT; - down_write(&card->controls_rwsem); + down_write(&card->ctl->controls_rwsem); kctl = snd_ctl_find_id_locked(card, &id); if (kctl == NULL) { result = -ENOENT; @@ -1480,7 +1519,7 @@ static int snd_ctl_elem_unlock(struct snd_ctl_file *file, result = 0; } } - up_write(&card->controls_rwsem); + up_write(&card->ctl->controls_rwsem); return result; }
@@ -1497,7 +1536,7 @@ struct user_element { // check whether the addition (in bytes) of user ctl element may overflow the limit. static bool check_user_elem_overflow(struct snd_card *card, ssize_t add) { - return (ssize_t)card->user_ctl_alloc_size + add > max_user_ctl_alloc_size; + return (ssize_t)card->ctl->user_ctl_alloc_size + add > max_user_ctl_alloc_size; }
static int snd_ctl_elem_user_info(struct snd_kcontrol *kcontrol, @@ -1575,7 +1614,7 @@ static int replace_user_tlv(struct snd_kcontrol *kctl, unsigned int __user *buf, int i; int change;
- lockdep_assert_held_write(&ue->card->controls_rwsem); + lockdep_assert_held_write(&ue->card->ctl->controls_rwsem);
if (size > 1024 * 128) /* sane value */ return -EINVAL; @@ -1602,7 +1641,7 @@ static int replace_user_tlv(struct snd_kcontrol *kctl, unsigned int __user *buf, kctl->vd[i].access |= SNDRV_CTL_ELEM_ACCESS_TLV_READ; mask = SNDRV_CTL_EVENT_MASK_INFO; } else { - ue->card->user_ctl_alloc_size -= ue->tlv_data_size; + ue->card->ctl->user_ctl_alloc_size -= ue->tlv_data_size; ue->tlv_data_size = 0; kvfree(ue->tlv_data); } @@ -1610,7 +1649,7 @@ static int replace_user_tlv(struct snd_kcontrol *kctl, unsigned int __user *buf, ue->tlv_data = container; ue->tlv_data_size = size; // decremented at private_free. - ue->card->user_ctl_alloc_size += size; + ue->card->ctl->user_ctl_alloc_size += size;
mask |= SNDRV_CTL_EVENT_MASK_TLV; for (i = 0; i < kctl->count; ++i) @@ -1653,7 +1692,7 @@ static int snd_ctl_elem_init_enum_names(struct user_element *ue) unsigned int i; const uintptr_t user_ptrval = ue->info.value.enumerated.names_ptr;
- lockdep_assert_held_write(&ue->card->controls_rwsem); + lockdep_assert_held_write(&ue->card->ctl->controls_rwsem);
buf_len = ue->info.value.enumerated.names_length; if (buf_len > 64 * 1024) @@ -1680,7 +1719,7 @@ static int snd_ctl_elem_init_enum_names(struct user_element *ue) ue->priv_data = names; ue->info.value.enumerated.names_ptr = 0; // increment the allocation size; decremented again at private_free. - ue->card->user_ctl_alloc_size += ue->info.value.enumerated.names_length; + ue->card->ctl->user_ctl_alloc_size += ue->info.value.enumerated.names_length;
return 0; } @@ -1695,10 +1734,10 @@ static void snd_ctl_elem_user_free(struct snd_kcontrol *kcontrol) struct user_element *ue = kcontrol->private_data;
// decrement the allocation size. - ue->card->user_ctl_alloc_size -= compute_user_elem_size(ue->elem_data_size, kcontrol->count); - ue->card->user_ctl_alloc_size -= ue->tlv_data_size; + ue->card->ctl->user_ctl_alloc_size -= compute_user_elem_size(ue->elem_data_size, kcontrol->count); + ue->card->ctl->user_ctl_alloc_size -= ue->tlv_data_size; if (ue->priv_data) - ue->card->user_ctl_alloc_size -= ue->info.value.enumerated.names_length; + ue->card->ctl->user_ctl_alloc_size -= ue->info.value.enumerated.names_length;
kvfree(ue->tlv_data); kvfree(ue->priv_data); @@ -1763,7 +1802,7 @@ static int snd_ctl_elem_add(struct snd_ctl_file *file, private_size = value_sizes[info->type] * info->count; alloc_size = compute_user_elem_size(private_size, count);
- down_write(&card->controls_rwsem); + down_write(&card->ctl->controls_rwsem); if (check_user_elem_overflow(card, alloc_size)) { err = -ENOMEM; goto unlock; @@ -1789,7 +1828,7 @@ static int snd_ctl_elem_add(struct snd_ctl_file *file, kctl->private_free = snd_ctl_elem_user_free;
// increment the allocated size; decremented again at private_free. - card->user_ctl_alloc_size += alloc_size; + card->ctl->user_ctl_alloc_size += alloc_size;
/* Set private data for this userspace control. */ ue->card = card; @@ -1833,7 +1872,7 @@ static int snd_ctl_elem_add(struct snd_ctl_file *file, * which locks the element. */ unlock: - up_write(&card->controls_rwsem); + up_write(&card->ctl->controls_rwsem); return err; }
@@ -1959,7 +1998,7 @@ static int snd_ctl_tlv_ioctl(struct snd_ctl_file *file, struct snd_ctl_elem_id id; struct snd_kcontrol_volatile *vd;
- lockdep_assert_held(&file->card->controls_rwsem); + lockdep_assert_held(&file->card->ctl->controls_rwsem);
if (copy_from_user(&header, buf, sizeof(header))) return -EFAULT; @@ -2036,19 +2075,19 @@ static long snd_ctl_ioctl(struct file *file, unsigned int cmd, unsigned long arg case SNDRV_CTL_IOCTL_SUBSCRIBE_EVENTS: return snd_ctl_subscribe_events(ctl, ip); case SNDRV_CTL_IOCTL_TLV_READ: - down_read(&ctl->card->controls_rwsem); + down_read(&ctl->card->ctl->controls_rwsem); err = snd_ctl_tlv_ioctl(ctl, argp, SNDRV_CTL_TLV_OP_READ); - up_read(&ctl->card->controls_rwsem); + up_read(&ctl->card->ctl->controls_rwsem); return err; case SNDRV_CTL_IOCTL_TLV_WRITE: - down_write(&ctl->card->controls_rwsem); + down_write(&ctl->card->ctl->controls_rwsem); err = snd_ctl_tlv_ioctl(ctl, argp, SNDRV_CTL_TLV_OP_WRITE); - up_write(&ctl->card->controls_rwsem); + up_write(&ctl->card->ctl->controls_rwsem); return err; case SNDRV_CTL_IOCTL_TLV_COMMAND: - down_write(&ctl->card->controls_rwsem); + down_write(&ctl->card->ctl->controls_rwsem); err = snd_ctl_tlv_ioctl(ctl, argp, SNDRV_CTL_TLV_OP_CMD); - up_write(&ctl->card->controls_rwsem); + up_write(&ctl->card->ctl->controls_rwsem); return err; case SNDRV_CTL_IOCTL_POWER: return -ENOPROTOOPT; @@ -2258,15 +2297,15 @@ int snd_ctl_get_preferred_subdevice(struct snd_card *card, int type) int subdevice = -1; unsigned long flags;
- read_lock_irqsave(&card->ctl_files_rwlock, flags); - list_for_each_entry(kctl, &card->ctl_files, list) { + read_lock_irqsave(&card->ctl->files_rwlock, flags); + list_for_each_entry(kctl, &card->ctl->files, list) { if (kctl->pid == task_pid(current)) { subdevice = kctl->preferred_subdevice[type]; if (subdevice != -1) break; } } - read_unlock_irqrestore(&card->ctl_files_rwlock, flags); + read_unlock_irqrestore(&card->ctl->files_rwlock, flags); return subdevice; } EXPORT_SYMBOL_GPL(snd_ctl_get_preferred_subdevice); @@ -2326,9 +2365,9 @@ void snd_ctl_register_layer(struct snd_ctl_layer_ops *lops) for (card_number = 0; card_number < SNDRV_CARDS; card_number++) { card = snd_card_ref(card_number); if (card) { - down_read(&card->controls_rwsem); + down_read(&card->ctl->controls_rwsem); lops->lregister(card); - up_read(&card->controls_rwsem); + up_read(&card->ctl->controls_rwsem); snd_card_unref(card); } } @@ -2389,15 +2428,15 @@ static int snd_ctl_dev_register(struct snd_device *device) int err;
err = snd_register_device(SNDRV_DEVICE_TYPE_CONTROL, card, -1, - &snd_ctl_f_ops, card, card->ctl_dev); + &snd_ctl_f_ops, card, &card->ctl->dev); if (err < 0) return err; - down_read(&card->controls_rwsem); + down_read(&card->ctl->controls_rwsem); down_read(&snd_ctl_layer_rwsem); for (lops = snd_ctl_layer; lops; lops = lops->next) lops->lregister(card); up_read(&snd_ctl_layer_rwsem); - up_read(&card->controls_rwsem); + up_read(&card->ctl->controls_rwsem); return 0; }
@@ -2411,21 +2450,21 @@ static int snd_ctl_dev_disconnect(struct snd_device *device) struct snd_ctl_layer_ops *lops; unsigned long flags;
- read_lock_irqsave(&card->ctl_files_rwlock, flags); - list_for_each_entry(ctl, &card->ctl_files, list) { + read_lock_irqsave(&card->ctl->files_rwlock, flags); + list_for_each_entry(ctl, &card->ctl->files, list) { wake_up(&ctl->change_sleep); snd_kill_fasync(ctl->fasync, SIGIO, POLL_ERR); } - read_unlock_irqrestore(&card->ctl_files_rwlock, flags); + read_unlock_irqrestore(&card->ctl->files_rwlock, flags);
- down_read(&card->controls_rwsem); + down_read(&card->ctl->controls_rwsem); down_read(&snd_ctl_layer_rwsem); for (lops = snd_ctl_layer; lops; lops = lops->next) lops->ldisconnect(card); up_read(&snd_ctl_layer_rwsem); - up_read(&card->controls_rwsem); + up_read(&card->ctl->controls_rwsem);
- return snd_unregister_device(card->ctl_dev); + return snd_unregister_device(&card->ctl->dev); }
/* @@ -2436,21 +2475,28 @@ static int snd_ctl_dev_free(struct snd_device *device) struct snd_card *card = device->device_data; struct snd_kcontrol *control;
- down_write(&card->controls_rwsem); - while (!list_empty(&card->controls)) { - control = snd_kcontrol(card->controls.next); + down_write(&card->ctl->controls_rwsem); + while (!list_empty(&card->ctl->controls)) { + control = snd_kcontrol(card->ctl->controls.next); __snd_ctl_remove(card, control, false); }
#ifdef CONFIG_SND_CTL_FAST_LOOKUP - xa_destroy(&card->ctl_numids); - xa_destroy(&card->ctl_hash); + xa_destroy(&card->ctl->numids); + xa_destroy(&card->ctl->hash); #endif - up_write(&card->controls_rwsem); - put_device(card->ctl_dev); + up_write(&card->ctl->controls_rwsem); + put_device(&card->ctl->dev); return 0; }
+static void release_control_device(struct device *dev) +{ + struct snd_control *ctl = container_of(dev, struct snd_control, dev); + + kfree(ctl); +} + /* * create control core: * called from init.c @@ -2462,6 +2508,7 @@ int snd_ctl_create(struct snd_card *card) .dev_register = snd_ctl_dev_register, .dev_disconnect = snd_ctl_dev_disconnect, }; + struct snd_control *ctl = card->ctl; int err;
if (snd_BUG_ON(!card)) @@ -2469,14 +2516,13 @@ int snd_ctl_create(struct snd_card *card) if (snd_BUG_ON(card->number < 0 || card->number >= SNDRV_CARDS)) return -ENXIO;
- err = snd_device_alloc(&card->ctl_dev, card); - if (err < 0) - return err; - dev_set_name(card->ctl_dev, "controlC%d", card->number); + snd_device_init(&ctl->dev, card); + dev_set_name(&ctl->dev, "controlC%d", card->number); + ctl->dev.release = release_control_device;
err = snd_device_new(card, SNDRV_DEV_CONTROL, card, &ops); if (err < 0) - put_device(card->ctl_dev); + put_device(&ctl->dev); return err; }
diff --git a/sound/core/control_compat.c b/sound/core/control_compat.c index 0e8b1bfb040e0..0e0b555f71537 100644 --- a/sound/core/control_compat.c +++ b/sound/core/control_compat.c @@ -172,15 +172,15 @@ static int get_ctl_type(struct snd_card *card, struct snd_ctl_elem_id *id, struct snd_ctl_elem_info *info; int err;
- down_read(&card->controls_rwsem); + down_read(&card->ctl->controls_rwsem); kctl = snd_ctl_find_id_locked(card, id); if (! kctl) { - up_read(&card->controls_rwsem); + up_read(&card->ctl->controls_rwsem); return -ENOENT; } info = kzalloc(sizeof(*info), GFP_KERNEL); if (info == NULL) { - up_read(&card->controls_rwsem); + up_read(&card->ctl->controls_rwsem); return -ENOMEM; } info->id = *id; @@ -188,7 +188,7 @@ static int get_ctl_type(struct snd_card *card, struct snd_ctl_elem_id *id, if (!err) err = kctl->info(kctl, info); snd_power_unref(card); - up_read(&card->controls_rwsem); + up_read(&card->ctl->controls_rwsem); if (err >= 0) { err = info->type; *countp = info->count; diff --git a/sound/core/control_led.c b/sound/core/control_led.c index a78eb48927c7b..6b53de7660438 100644 --- a/sound/core/control_led.c +++ b/sound/core/control_led.c @@ -243,6 +243,7 @@ static int snd_ctl_led_set_id(int card_number, struct snd_ctl_elem_id *id, unsigned int group, bool set) { struct snd_card *card; + struct snd_control *ctl; struct snd_kcontrol *kctl; struct snd_kcontrol_volatile *vd; unsigned int ioff, access, new_access; @@ -250,7 +251,8 @@ static int snd_ctl_led_set_id(int card_number, struct snd_ctl_elem_id *id,
card = snd_card_ref(card_number); if (card) { - down_write(&card->controls_rwsem); + ctl = card->ctl; + down_write(&ctl->controls_rwsem); kctl = snd_ctl_find_id_locked(card, id); if (kctl) { ioff = snd_ctl_get_ioff(kctl, id); @@ -271,7 +273,7 @@ static int snd_ctl_led_set_id(int card_number, struct snd_ctl_elem_id *id, err = -ENOENT; } unlock: - up_write(&card->controls_rwsem); + up_write(&ctl->controls_rwsem); snd_card_unref(card); } else { err = -ENXIO; @@ -357,7 +359,7 @@ static void snd_ctl_led_register(struct snd_card *card) snd_ctl_led_card_valid[card->number] = true; mutex_unlock(&snd_ctl_led_mutex); /* the register callback is already called with held card->controls_rwsem */ - list_for_each_entry(kctl, &card->controls, list) + list_for_each_entry(kctl, &card->ctl->controls, list) for (ioff = 0; ioff < kctl->count; ioff++) snd_ctl_led_notify(card, SNDRV_CTL_EVENT_MASK_VALUE, kctl, ioff); snd_ctl_led_refresh(); @@ -617,12 +619,14 @@ static ssize_t list_show(struct device *dev, struct snd_ctl_led_card *led_card = container_of(dev, struct snd_ctl_led_card, dev); struct snd_card *card; struct snd_ctl_led_ctl *lctl; + struct snd_control *ctl; size_t l = 0;
card = snd_card_ref(led_card->number); if (!card) return -ENXIO; - down_read(&card->controls_rwsem); + ctl = card->ctl; + down_read(&ctl->controls_rwsem); mutex_lock(&snd_ctl_led_mutex); if (snd_ctl_led_card_valid[led_card->number]) { list_for_each_entry(lctl, &led_card->led->controls, list) { @@ -635,7 +639,7 @@ static ssize_t list_show(struct device *dev, } } mutex_unlock(&snd_ctl_led_mutex); - up_read(&card->controls_rwsem); + up_read(&ctl->controls_rwsem); snd_card_unref(card); return l; } @@ -688,7 +692,7 @@ static void snd_ctl_led_sysfs_add(struct snd_card *card) goto cerr; led->cards[card->number] = led_card; snprintf(link_name, sizeof(link_name), "led-%s", led->name); - WARN(sysfs_create_link(&card->ctl_dev->kobj, &led_card->dev.kobj, link_name), + WARN(sysfs_create_link(&card->ctl->dev.kobj, &led_card->dev.kobj, link_name), "can't create symlink to controlC%i device\n", card->number); WARN(sysfs_create_link(&led_card->dev.kobj, &card->card_dev.kobj, "card"), "can't create symlink to card%i\n", card->number); @@ -714,7 +718,7 @@ static void snd_ctl_led_sysfs_remove(struct snd_card *card) if (!led_card) continue; snprintf(link_name, sizeof(link_name), "led-%s", led->name); - sysfs_remove_link(&card->ctl_dev->kobj, link_name); + sysfs_remove_link(&card->ctl->dev.kobj, link_name); sysfs_remove_link(&led_card->dev.kobj, "card"); device_unregister(&led_card->dev); led->cards[card->number] = NULL; diff --git a/sound/core/init.c b/sound/core/init.c index 37a8e4791f781..695b080603e93 100644 --- a/sound/core/init.c +++ b/sound/core/init.c @@ -331,14 +331,6 @@ static int snd_card_init(struct snd_card *card, struct device *parent, card->module = module; #endif INIT_LIST_HEAD(&card->devices); - init_rwsem(&card->controls_rwsem); - rwlock_init(&card->ctl_files_rwlock); - INIT_LIST_HEAD(&card->controls); - INIT_LIST_HEAD(&card->ctl_files); -#ifdef CONFIG_SND_CTL_FAST_LOOKUP - xa_init(&card->ctl_numids); - xa_init(&card->ctl_hash); -#endif spin_lock_init(&card->files_lock); INIT_LIST_HEAD(&card->files_list); mutex_init(&card->memory_mutex); @@ -363,11 +355,9 @@ static int snd_card_init(struct snd_card *card, struct device *parent, snprintf(card->irq_descr, sizeof(card->irq_descr), "%s:%s", dev_driver_string(card->dev), dev_name(&card->card_dev));
- /* the control interface cannot be accessed from the user space until */ - /* snd_cards_bitmask and snd_cards are set with snd_card_register */ - err = snd_ctl_create(card); + err = snd_control_new(card); if (err < 0) { - dev_err(parent, "unable to register control minors\n"); + dev_err(parent, "unable to create controls and register\n"); goto __error; } err = snd_info_card_create(card); diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c index 33af707a65ab1..7afc9a29ea9ec 100644 --- a/sound/pci/hda/hda_codec.c +++ b/sound/pci/hda/hda_codec.c @@ -1784,13 +1784,14 @@ void snd_hda_ctls_clear(struct hda_codec *codec) int snd_hda_lock_devices(struct hda_bus *bus) { struct snd_card *card = bus->card; + struct snd_control *ctl = card->ctl; struct hda_codec *codec;
spin_lock(&card->files_lock); if (card->shutdown) goto err_unlock; card->shutdown = 1; - if (!list_empty(&card->ctl_files)) + if (!list_empty(&ctl->files)) goto err_clear;
list_for_each_codec(codec, bus) { diff --git a/sound/soc/intel/atom/sst-atom-controls.c b/sound/soc/intel/atom/sst-atom-controls.c index 38116c7587174..8cd5681de9fbd 100644 --- a/sound/soc/intel/atom/sst-atom-controls.c +++ b/sound/soc/intel/atom/sst-atom-controls.c @@ -1434,9 +1434,9 @@ static int sst_fill_widget_module_info(struct snd_soc_dapm_widget *w, struct snd_card *card = component->card->snd_card; char *idx;
- down_read(&card->controls_rwsem); + down_read(&card->ctl->controls_rwsem);
- list_for_each_entry(kctl, &card->controls, list) { + list_for_each_entry(kctl, &card->ctl->controls, list) { idx = strchr(kctl->id.name, ' '); if (idx == NULL) continue; @@ -1469,12 +1469,12 @@ static int sst_fill_widget_module_info(struct snd_soc_dapm_widget *w, }
if (ret < 0) { - up_read(&card->controls_rwsem); + up_read(&card->ctl->controls_rwsem); return ret; } }
- up_read(&card->controls_rwsem); + up_read(&card->ctl->controls_rwsem); return 0; }
diff --git a/sound/soc/soc-card.c b/sound/soc/soc-card.c index 285ab4c9c7168..1129eb33f78c9 100644 --- a/sound/soc/soc-card.c +++ b/sound/soc/soc-card.c @@ -35,7 +35,7 @@ struct snd_kcontrol *snd_soc_card_get_kcontrol(struct snd_soc_card *soc_card, if (unlikely(!name)) return NULL;
- list_for_each_entry(kctl, &card->controls, list) + list_for_each_entry(kctl, &card->ctl->controls, list) if (!strncmp(kctl->id.name, name, sizeof(kctl->id.name))) return kctl; return NULL; diff --git a/sound/usb/media.c b/sound/usb/media.c index d48db6f3ae659..5298c2f49a164 100644 --- a/sound/usb/media.c +++ b/sound/usb/media.c @@ -163,7 +163,7 @@ void snd_media_stop_pipeline(struct snd_usb_substream *subs)
static int snd_media_mixer_init(struct snd_usb_audio *chip) { - struct device *ctl_dev = chip->card->ctl_dev; + struct device *ctl_dev = &chip->card->ctl->dev; struct media_intf_devnode *ctl_intf; struct usb_mixer_interface *mixer; struct media_device *mdev = chip->media_dev;
On Thu, Aug 24, 2023 at 02:02:53PM -0700, cujomalainey@chromium.org wrote:
From: Curtis Malainey cujomalainey@chromium.org
Having two kobj in the same struct is broken at its core. This splits card_dev from ctl_dev so they can properly refcount and release on their own schedules without the workaround of having them being just a pointer.
Acked-by: Mark Brown broonie@kernel.org
On Thu, 24 Aug 2023 23:02:53 +0200, cujomalainey@chromium.org wrote:
+struct snd_control +{
The open brace should be at the line above.
- struct device dev; /* control device */
- struct rw_semaphore controls_rwsem; /* controls lock (list and values) */
- rwlock_t files_rwlock; /* ctl_files list lock */
- int controls_count; /* count of all controls */
- size_t user_ctl_alloc_size; // current memory allocation by user controls.
Better to have the same comment style if we move the whole stuff.
+int snd_control_new(struct snd_card *card)
....
+{
- struct snd_control *ctl = kzalloc(sizeof(*ctl), GFP_KERNEL);
- int err;
- if (snd_BUG_ON(!card))
return -EINVAL;
This may leak the memory allocated in the above.
- /* the control interface cannot be accessed from the user space until */
- /* snd_cards_bitmask and snd_cards are set with snd_card_register */
- err = snd_ctl_create(card);
- if (err < 0) {
dev_err(card->dev, "unable to register control minors\n");kfree(ctl);return err;
This needs a more care. snd_ctl_create() calls put_device() when snd_device_new() fails, and this already does kfree(). OTOH, the error path before that point doesn't release the object.
- }
- return 0;
+} +EXPORT_SYMBOL(snd_control_new);
This is never called from the driver but only from snd_card_new() & co, so no need to export.
thanks,
Takashi
Hi,
kernel test robot noticed the following build errors:
[auto build test ERROR on tiwai-sound/for-next] [also build test ERROR on tiwai-sound/for-linus linus/master next-20230831] [cannot apply to broonie-sound/for-next v6.5] [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#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/cujomalainey-chromium-org/ALS... base: https://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git for-next patch link: https://lore.kernel.org/r/20230824210339.1126993-3-cujomalainey%40chromium.o... patch subject: [PATCH 2/2] ALSA: core: split control primitives out of snd_card config: x86_64-allyesconfig (https://download.01.org/0day-ci/archive/20230904/202309040658.LwLeCNwY-lkp@i...) compiler: gcc-12 (Debian 12.2.0-14) 12.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230904/202309040658.LwLeCNwY-lkp@i...)
If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot lkp@intel.com | Closes: https://lore.kernel.org/oe-kbuild-all/202309040658.LwLeCNwY-lkp@intel.com/
All errors (new ones prefixed by >>):
sound/core/oss/mixer_oss.c: In function 'snd_mixer_oss_get_volume1_vol':
sound/core/oss/mixer_oss.c:542:24: error: 'struct snd_card' has no member named 'controls_rwsem'
542 | down_read(&card->controls_rwsem); | ^~ sound/core/oss/mixer_oss.c:545:30: error: 'struct snd_card' has no member named 'controls_rwsem' 545 | up_read(&card->controls_rwsem); | ^~ sound/core/oss/mixer_oss.c:563:22: error: 'struct snd_card' has no member named 'controls_rwsem' 563 | up_read(&card->controls_rwsem); | ^~ sound/core/oss/mixer_oss.c: In function 'snd_mixer_oss_get_volume1_sw': sound/core/oss/mixer_oss.c:581:24: error: 'struct snd_card' has no member named 'controls_rwsem' 581 | down_read(&card->controls_rwsem); | ^~ sound/core/oss/mixer_oss.c:584:30: error: 'struct snd_card' has no member named 'controls_rwsem' 584 | up_read(&card->controls_rwsem); | ^~ sound/core/oss/mixer_oss.c:603:22: error: 'struct snd_card' has no member named 'controls_rwsem' 603 | up_read(&card->controls_rwsem); | ^~ sound/core/oss/mixer_oss.c: In function 'snd_mixer_oss_put_volume1_vol': sound/core/oss/mixer_oss.c:647:24: error: 'struct snd_card' has no member named 'controls_rwsem' 647 | down_read(&card->controls_rwsem); | ^~ sound/core/oss/mixer_oss.c:650:30: error: 'struct snd_card' has no member named 'controls_rwsem' 650 | up_read(&card->controls_rwsem); | ^~ sound/core/oss/mixer_oss.c:671:22: error: 'struct snd_card' has no member named 'controls_rwsem' 671 | up_read(&card->controls_rwsem); | ^~ sound/core/oss/mixer_oss.c: In function 'snd_mixer_oss_put_volume1_sw': sound/core/oss/mixer_oss.c:690:24: error: 'struct snd_card' has no member named 'controls_rwsem' 690 | down_read(&card->controls_rwsem); | ^~ sound/core/oss/mixer_oss.c:693:30: error: 'struct snd_card' has no member named 'controls_rwsem' 693 | up_read(&card->controls_rwsem); | ^~ sound/core/oss/mixer_oss.c:718:22: error: 'struct snd_card' has no member named 'controls_rwsem' 718 | up_read(&card->controls_rwsem); | ^~ sound/core/oss/mixer_oss.c: In function 'snd_mixer_oss_get_recsrc2': sound/core/oss/mixer_oss.c:835:24: error: 'struct snd_card' has no member named 'controls_rwsem' 835 | down_read(&card->controls_rwsem); | ^~ sound/core/oss/mixer_oss.c:863:22: error: 'struct snd_card' has no member named 'controls_rwsem' 863 | up_read(&card->controls_rwsem); | ^~ sound/core/oss/mixer_oss.c: In function 'snd_mixer_oss_put_recsrc2': sound/core/oss/mixer_oss.c:888:24: error: 'struct snd_card' has no member named 'controls_rwsem' 888 | down_read(&card->controls_rwsem); | ^~ sound/core/oss/mixer_oss.c:919:22: error: 'struct snd_card' has no member named 'controls_rwsem' 919 | up_read(&card->controls_rwsem); | ^~ sound/core/oss/mixer_oss.c: In function 'snd_mixer_oss_build_test': sound/core/oss/mixer_oss.c:939:24: error: 'struct snd_card' has no member named 'controls_rwsem' 939 | down_read(&card->controls_rwsem); | ^~ sound/core/oss/mixer_oss.c:942:30: error: 'struct snd_card' has no member named 'controls_rwsem' 942 | up_read(&card->controls_rwsem); | ^~ sound/core/oss/mixer_oss.c:947:30: error: 'struct snd_card' has no member named 'controls_rwsem' 947 | up_read(&card->controls_rwsem); | ^~ sound/core/oss/mixer_oss.c:952:30: error: 'struct snd_card' has no member named 'controls_rwsem' 952 | up_read(&card->controls_rwsem); | ^~ sound/core/oss/mixer_oss.c:957:22: error: 'struct snd_card' has no member named 'controls_rwsem' 957 | up_read(&card->controls_rwsem); | ^~ sound/core/oss/mixer_oss.c: In function 'snd_mixer_oss_build_input': sound/core/oss/mixer_oss.c:1071:31: error: 'struct snd_card' has no member named 'controls_rwsem' 1071 | down_read(&mixer->card->controls_rwsem); | ^~ sound/core/oss/mixer_oss.c:1080:45: error: 'struct snd_card' has no member named 'controls_rwsem' 1080 | up_read(&mixer->card->controls_rwsem); | ^~ sound/core/oss/mixer_oss.c:1085:45: error: 'struct snd_card' has no member named 'controls_rwsem' 1085 | up_read(&mixer->card->controls_rwsem); | ^~ sound/core/oss/mixer_oss.c:1101:61: error: 'struct snd_card' has no member named 'controls_rwsem' 1101 | up_read(&mixer->card->controls_rwsem); | ^~ sound/core/oss/mixer_oss.c:1113:29: error: 'struct snd_card' has no member named 'controls_rwsem' 1113 | up_read(&mixer->card->controls_rwsem); | ^~ -- In file included from include/linux/kernel.h:21, from arch/x86/include/asm/percpu.h:27, from arch/x86/include/asm/current.h:10, from include/linux/sched.h:12, from include/linux/ratelimit.h:6, from include/linux/dev_printk.h:16, from include/linux/device.h:15, from include/linux/pm_runtime.h:11, from sound/soc/intel/catpt/pcm.c:8: sound/soc/intel/catpt/pcm.c: In function 'catpt_dai_apply_usettings':
sound/soc/intel/catpt/pcm.c:357:60: error: 'struct snd_card' has no member named 'controls'
357 | list_for_each_entry(pos, &component->card->snd_card->controls, list) { | ^~ include/linux/container_of.h:19:33: note: in definition of macro 'container_of' 19 | void *__mptr = (void *)(ptr); \ | ^~~ include/linux/list.h:531:9: note: in expansion of macro 'list_entry' 531 | list_entry((ptr)->next, type, member) | ^~~~~~~~~~ include/linux/list.h:689:20: note: in expansion of macro 'list_first_entry' 689 | for (pos = list_first_entry(head, typeof(*pos), member); \ | ^~~~~~~~~~~~~~~~ sound/soc/intel/catpt/pcm.c:357:9: note: in expansion of macro 'list_for_each_entry' 357 | list_for_each_entry(pos, &component->card->snd_card->controls, list) { | ^~~~~~~~~~~~~~~~~~~ In file included from include/linux/bits.h:21, from include/linux/ratelimit_types.h:5, from include/linux/ratelimit.h:5:
sound/soc/intel/catpt/pcm.c:357:60: error: 'struct snd_card' has no member named 'controls'
357 | list_for_each_entry(pos, &component->card->snd_card->controls, list) { | ^~ include/linux/build_bug.h:78:56: note: in definition of macro '__static_assert' 78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg) | ^~~~ include/linux/container_of.h:20:9: note: in expansion of macro 'static_assert' 20 | static_assert(__same_type(*(ptr), ((type *)0)->member) || \ | ^~~~~~~~~~~~~ include/linux/container_of.h:20:23: note: in expansion of macro '__same_type' 20 | static_assert(__same_type(*(ptr), ((type *)0)->member) || \ | ^~~~~~~~~~~ include/linux/list.h:520:9: note: in expansion of macro 'container_of' 520 | container_of(ptr, type, member) | ^~~~~~~~~~~~ include/linux/list.h:531:9: note: in expansion of macro 'list_entry' 531 | list_entry((ptr)->next, type, member) | ^~~~~~~~~~ include/linux/list.h:689:20: note: in expansion of macro 'list_first_entry' 689 | for (pos = list_first_entry(head, typeof(*pos), member); \ | ^~~~~~~~~~~~~~~~ sound/soc/intel/catpt/pcm.c:357:9: note: in expansion of macro 'list_for_each_entry' 357 | list_for_each_entry(pos, &component->card->snd_card->controls, list) { | ^~~~~~~~~~~~~~~~~~~
sound/soc/intel/catpt/pcm.c:357:60: error: 'struct snd_card' has no member named 'controls'
357 | list_for_each_entry(pos, &component->card->snd_card->controls, list) { | ^~ include/linux/build_bug.h:78:56: note: in definition of macro '__static_assert' 78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg) | ^~~~ include/linux/container_of.h:20:9: note: in expansion of macro 'static_assert' 20 | static_assert(__same_type(*(ptr), ((type *)0)->member) || \ | ^~~~~~~~~~~~~ include/linux/container_of.h:21:23: note: in expansion of macro '__same_type' 21 | __same_type(*(ptr), void), \ | ^~~~~~~~~~~ include/linux/list.h:520:9: note: in expansion of macro 'container_of' 520 | container_of(ptr, type, member) | ^~~~~~~~~~~~ include/linux/list.h:531:9: note: in expansion of macro 'list_entry' 531 | list_entry((ptr)->next, type, member) | ^~~~~~~~~~ include/linux/list.h:689:20: note: in expansion of macro 'list_first_entry' 689 | for (pos = list_first_entry(head, typeof(*pos), member); \ | ^~~~~~~~~~~~~~~~ sound/soc/intel/catpt/pcm.c:357:9: note: in expansion of macro 'list_for_each_entry' 357 | list_for_each_entry(pos, &component->card->snd_card->controls, list) { | ^~~~~~~~~~~~~~~~~~~ include/linux/compiler_types.h:338:27: error: expression in static assertion is not an integer 338 | #define __same_type(a, b) __builtin_types_compatible_p(typeof(a), typeof(b)) | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~ include/linux/build_bug.h:78:56: note: in definition of macro '__static_assert' 78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg) | ^~~~ include/linux/container_of.h:20:9: note: in expansion of macro 'static_assert' 20 | static_assert(__same_type(*(ptr), ((type *)0)->member) || \ | ^~~~~~~~~~~~~ include/linux/container_of.h:20:23: note: in expansion of macro '__same_type' 20 | static_assert(__same_type(*(ptr), ((type *)0)->member) || \ | ^~~~~~~~~~~ include/linux/list.h:520:9: note: in expansion of macro 'container_of' 520 | container_of(ptr, type, member) | ^~~~~~~~~~~~ include/linux/list.h:531:9: note: in expansion of macro 'list_entry' 531 | list_entry((ptr)->next, type, member) | ^~~~~~~~~~ include/linux/list.h:689:20: note: in expansion of macro 'list_first_entry' 689 | for (pos = list_first_entry(head, typeof(*pos), member); \ | ^~~~~~~~~~~~~~~~ sound/soc/intel/catpt/pcm.c:357:9: note: in expansion of macro 'list_for_each_entry' 357 | list_for_each_entry(pos, &component->card->snd_card->controls, list) { | ^~~~~~~~~~~~~~~~~~~ In file included from include/linux/rculist.h:10, from include/linux/pid.h:5, from include/linux/sched.h:14:
sound/soc/intel/catpt/pcm.c:357:60: error: 'struct snd_card' has no member named 'controls'
357 | list_for_each_entry(pos, &component->card->snd_card->controls, list) { | ^~ include/linux/list.h:680:27: note: in definition of macro 'list_entry_is_head' 680 | (&pos->member == (head)) | ^~~~ sound/soc/intel/catpt/pcm.c:357:9: note: in expansion of macro 'list_for_each_entry' 357 | list_for_each_entry(pos, &component->card->snd_card->controls, list) { | ^~~~~~~~~~~~~~~~~~~ sound/soc/intel/catpt/pcm.c:362:63: error: 'struct snd_card' has no member named 'controls' 362 | if (list_entry_is_head(pos, &component->card->snd_card->controls, list)) | ^~ include/linux/list.h:680:27: note: in definition of macro 'list_entry_is_head' 680 | (&pos->member == (head)) | ^~~~ -- In file included from include/linux/kernel.h:21, from arch/x86/include/asm/percpu.h:27, from arch/x86/include/asm/current.h:10, from include/linux/sched.h:12, from include/linux/ratelimit.h:6, from include/linux/dev_printk.h:16, from include/linux/device.h:15, from include/linux/pm_runtime.h:11, from sound/soc/sh/rcar/core.c:93: sound/soc/sh/rcar/core.c: In function 'rsnd_kctrl_new':
sound/soc/sh/rcar/core.c:1813:41: error: 'struct snd_card' has no member named 'controls'
1813 | list_for_each_entry(kctrl, &card->controls, list) { | ^~ include/linux/container_of.h:19:33: note: in definition of macro 'container_of' 19 | void *__mptr = (void *)(ptr); \ | ^~~ include/linux/list.h:531:9: note: in expansion of macro 'list_entry' 531 | list_entry((ptr)->next, type, member) | ^~~~~~~~~~ include/linux/list.h:689:20: note: in expansion of macro 'list_first_entry' 689 | for (pos = list_first_entry(head, typeof(*pos), member); \ | ^~~~~~~~~~~~~~~~ sound/soc/sh/rcar/core.c:1813:9: note: in expansion of macro 'list_for_each_entry' 1813 | list_for_each_entry(kctrl, &card->controls, list) { | ^~~~~~~~~~~~~~~~~~~ In file included from include/linux/bits.h:21, from include/linux/ratelimit_types.h:5, from include/linux/ratelimit.h:5:
sound/soc/sh/rcar/core.c:1813:41: error: 'struct snd_card' has no member named 'controls'
1813 | list_for_each_entry(kctrl, &card->controls, list) { | ^~ include/linux/build_bug.h:78:56: note: in definition of macro '__static_assert' 78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg) | ^~~~ include/linux/container_of.h:20:9: note: in expansion of macro 'static_assert' 20 | static_assert(__same_type(*(ptr), ((type *)0)->member) || \ | ^~~~~~~~~~~~~ include/linux/container_of.h:20:23: note: in expansion of macro '__same_type' 20 | static_assert(__same_type(*(ptr), ((type *)0)->member) || \ | ^~~~~~~~~~~ include/linux/list.h:520:9: note: in expansion of macro 'container_of' 520 | container_of(ptr, type, member) | ^~~~~~~~~~~~ include/linux/list.h:531:9: note: in expansion of macro 'list_entry' 531 | list_entry((ptr)->next, type, member) | ^~~~~~~~~~ include/linux/list.h:689:20: note: in expansion of macro 'list_first_entry' 689 | for (pos = list_first_entry(head, typeof(*pos), member); \ | ^~~~~~~~~~~~~~~~ sound/soc/sh/rcar/core.c:1813:9: note: in expansion of macro 'list_for_each_entry' 1813 | list_for_each_entry(kctrl, &card->controls, list) { | ^~~~~~~~~~~~~~~~~~~
sound/soc/sh/rcar/core.c:1813:41: error: 'struct snd_card' has no member named 'controls'
1813 | list_for_each_entry(kctrl, &card->controls, list) { | ^~ include/linux/build_bug.h:78:56: note: in definition of macro '__static_assert' 78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg) | ^~~~ include/linux/container_of.h:20:9: note: in expansion of macro 'static_assert' 20 | static_assert(__same_type(*(ptr), ((type *)0)->member) || \ | ^~~~~~~~~~~~~ include/linux/container_of.h:21:23: note: in expansion of macro '__same_type' 21 | __same_type(*(ptr), void), \ | ^~~~~~~~~~~ include/linux/list.h:520:9: note: in expansion of macro 'container_of' 520 | container_of(ptr, type, member) | ^~~~~~~~~~~~ include/linux/list.h:531:9: note: in expansion of macro 'list_entry' 531 | list_entry((ptr)->next, type, member) | ^~~~~~~~~~ include/linux/list.h:689:20: note: in expansion of macro 'list_first_entry' 689 | for (pos = list_first_entry(head, typeof(*pos), member); \ | ^~~~~~~~~~~~~~~~ sound/soc/sh/rcar/core.c:1813:9: note: in expansion of macro 'list_for_each_entry' 1813 | list_for_each_entry(kctrl, &card->controls, list) { | ^~~~~~~~~~~~~~~~~~~ include/linux/compiler_types.h:338:27: error: expression in static assertion is not an integer 338 | #define __same_type(a, b) __builtin_types_compatible_p(typeof(a), typeof(b)) | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~ include/linux/build_bug.h:78:56: note: in definition of macro '__static_assert' 78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg) | ^~~~ include/linux/container_of.h:20:9: note: in expansion of macro 'static_assert' 20 | static_assert(__same_type(*(ptr), ((type *)0)->member) || \ | ^~~~~~~~~~~~~ include/linux/container_of.h:20:23: note: in expansion of macro '__same_type' 20 | static_assert(__same_type(*(ptr), ((type *)0)->member) || \ | ^~~~~~~~~~~ include/linux/list.h:520:9: note: in expansion of macro 'container_of' 520 | container_of(ptr, type, member) | ^~~~~~~~~~~~ include/linux/list.h:531:9: note: in expansion of macro 'list_entry' 531 | list_entry((ptr)->next, type, member) | ^~~~~~~~~~ include/linux/list.h:689:20: note: in expansion of macro 'list_first_entry' 689 | for (pos = list_first_entry(head, typeof(*pos), member); \ | ^~~~~~~~~~~~~~~~ sound/soc/sh/rcar/core.c:1813:9: note: in expansion of macro 'list_for_each_entry' 1813 | list_for_each_entry(kctrl, &card->controls, list) { | ^~~~~~~~~~~~~~~~~~~ In file included from include/linux/rculist.h:10, from include/linux/pid.h:5, from include/linux/sched.h:14:
sound/soc/sh/rcar/core.c:1813:41: error: 'struct snd_card' has no member named 'controls'
1813 | list_for_each_entry(kctrl, &card->controls, list) { | ^~ include/linux/list.h:680:27: note: in definition of macro 'list_entry_is_head' 680 | (&pos->member == (head)) | ^~~~ sound/soc/sh/rcar/core.c:1813:9: note: in expansion of macro 'list_for_each_entry' 1813 | list_for_each_entry(kctrl, &card->controls, list) { | ^~~~~~~~~~~~~~~~~~~
vim +542 sound/core/oss/mixer_oss.c
^1da177e4c3f41 Linus Torvalds 2005-04-16 529 f956b4a3ae790e Takashi Iwai 2005-11-17 530 static void snd_mixer_oss_get_volume1_vol(struct snd_mixer_oss_file *fmixer, f956b4a3ae790e Takashi Iwai 2005-11-17 531 struct snd_mixer_oss_slot *pslot, ^1da177e4c3f41 Linus Torvalds 2005-04-16 532 unsigned int numid, ^1da177e4c3f41 Linus Torvalds 2005-04-16 533 int *left, int *right) ^1da177e4c3f41 Linus Torvalds 2005-04-16 534 { f956b4a3ae790e Takashi Iwai 2005-11-17 535 struct snd_ctl_elem_info *uinfo; f956b4a3ae790e Takashi Iwai 2005-11-17 536 struct snd_ctl_elem_value *uctl; f956b4a3ae790e Takashi Iwai 2005-11-17 537 struct snd_kcontrol *kctl; f956b4a3ae790e Takashi Iwai 2005-11-17 538 struct snd_card *card = fmixer->card; ^1da177e4c3f41 Linus Torvalds 2005-04-16 539 ^1da177e4c3f41 Linus Torvalds 2005-04-16 540 if (numid == ID_UNKNOWN) ^1da177e4c3f41 Linus Torvalds 2005-04-16 541 return; ^1da177e4c3f41 Linus Torvalds 2005-04-16 @542 down_read(&card->controls_rwsem); b1e055f67611da Takashi Iwai 2023-07-18 543 kctl = snd_ctl_find_numid_locked(card, numid); 51c816fdd17c63 Takashi Iwai 2021-06-08 544 if (!kctl) { ^1da177e4c3f41 Linus Torvalds 2005-04-16 545 up_read(&card->controls_rwsem); ^1da177e4c3f41 Linus Torvalds 2005-04-16 546 return; ^1da177e4c3f41 Linus Torvalds 2005-04-16 547 } ca2c0966562cfb Takashi Iwai 2005-09-09 548 uinfo = kzalloc(sizeof(*uinfo), GFP_KERNEL); ca2c0966562cfb Takashi Iwai 2005-09-09 549 uctl = kzalloc(sizeof(*uctl), GFP_KERNEL); ^1da177e4c3f41 Linus Torvalds 2005-04-16 550 if (uinfo == NULL || uctl == NULL) ^1da177e4c3f41 Linus Torvalds 2005-04-16 551 goto __unalloc; 7c22f1aaa23370 Takashi Iwai 2005-10-10 552 if (kctl->info(kctl, uinfo)) 7c22f1aaa23370 Takashi Iwai 2005-10-10 553 goto __unalloc; 7c22f1aaa23370 Takashi Iwai 2005-10-10 554 if (kctl->get(kctl, uctl)) 7c22f1aaa23370 Takashi Iwai 2005-10-10 555 goto __unalloc; 7c22f1aaa23370 Takashi Iwai 2005-10-10 556 if (uinfo->type == SNDRV_CTL_ELEM_TYPE_BOOLEAN && 7c22f1aaa23370 Takashi Iwai 2005-10-10 557 uinfo->value.integer.min == 0 && uinfo->value.integer.max == 1) 7c22f1aaa23370 Takashi Iwai 2005-10-10 558 goto __unalloc; ^1da177e4c3f41 Linus Torvalds 2005-04-16 559 *left = snd_mixer_oss_conv1(uctl->value.integer.value[0], uinfo->value.integer.min, uinfo->value.integer.max, &pslot->volume[0]); ^1da177e4c3f41 Linus Torvalds 2005-04-16 560 if (uinfo->count > 1) ^1da177e4c3f41 Linus Torvalds 2005-04-16 561 *right = snd_mixer_oss_conv1(uctl->value.integer.value[1], uinfo->value.integer.min, uinfo->value.integer.max, &pslot->volume[1]); ^1da177e4c3f41 Linus Torvalds 2005-04-16 562 __unalloc: ^1da177e4c3f41 Linus Torvalds 2005-04-16 563 up_read(&card->controls_rwsem); ^1da177e4c3f41 Linus Torvalds 2005-04-16 564 kfree(uctl); ^1da177e4c3f41 Linus Torvalds 2005-04-16 565 kfree(uinfo); ^1da177e4c3f41 Linus Torvalds 2005-04-16 566 } ^1da177e4c3f41 Linus Torvalds 2005-04-16 567
On Thu, 24 Aug 2023 23:02:51 +0200, cujomalainey@chromium.org wrote:
From: Curtis Malainey cujomalainey@chromium.org
As previously identified in [1] there are some issues with how kobjs are handled in sound/core. The solution provided in [2] is a workaround for the issues to fix the failures.
This series is a first attempt at the larger refactor needed to properly handle the objects.
[1] https://mailman.alsa-project.org/hyperkitty/list/alsa-devel@alsa-project.org... [2] https://mailman.alsa-project.org/hyperkitty/list/alsa-devel@alsa-project.org...
Curtis Malainey (2): ALSA: core: add snd_device_init ALSA: core: split control primitives out of snd_card
Thanks for the patches. But, as the 6.6 merge window open is pretty close, I'd postpone those unless it's urgently needed.
Also, before moving the resource tied with the device object, we'll need a refcount to the ctl dev from pcm dev, as PCM does release chmap at its free path (calling free_chmap()). Otherwise it'll lead to another UAF, if both objects releases are done asynchronously without dependency.
BTW, the cover letter and the subject prefix of the patches don't match, and also the cover letter didn't include Cc. Please try to make both cover letter and patches sent properly.
thanks,
Takashi
On Fri, Aug 25, 2023 at 4:16 AM Takashi Iwai tiwai@suse.de wrote:
Thanks for the patches. But, as the 6.6 merge window open is pretty close, I'd postpone those unless it's urgently needed.
I have no timeline since we have the workaround in, hence the RFC
Also, before moving the resource tied with the device object, we'll need a refcount to the ctl dev from pcm dev, as PCM does release chmap at its free path (calling free_chmap()). Otherwise it'll lead to another UAF, if both objects releases are done asynchronously without dependency.
Got it, thanks.
BTW, the cover letter and the subject prefix of the patches don't match, and also the cover letter didn't include Cc. Please try to make both cover letter and patches sent properly.
Thanks for catching that, I was struggling getting cc to copy across with git from get_maintainers. I will give it another shot with v2.
Curtis
thanks,
Takashi
participants (5)
-
cujomalainey＠chromium.org -
Curtis Malainey -
kernel test robot -
Mark Brown -
Takashi Iwai