RE: [PATCH] ALSA: core: Replace mutex_lock with mutex_trylock

[강신형 <s47.kang@samsung.com>]

> Thanks for the patch.  But this change may break the current working
> behavior; e.g. when two proc reads are running concurrently, one would
> be aborted unexpectedly.
> 
> IIUC, the problem is the call of proc_remove(), and this call itself
> can be outside the global mutex.
> 
> Could you check whether the patch below works instead?  (Note that
> it's only compile-tested.)  It makes the proc_remove() called at
> first, then clearing the internal entries.  The function was renamed
> accordingly for avoiding confusion, too.
> 
> 
> Takashi

You are right. My patch is just for avoiding the deadlock. 
It may lead to other problem instead the deadlock(e.g. USB sound card
registration failure)
Your patch works well without any problems.
But I can't confirm that the problem is solved or not.
because the issue has occurred only once until now.
(Test method: USB insertion / removal during a call)


> 
> --- a/sound/core/info.c
> +++ b/sound/core/info.c
> @@ -56,7 +56,7 @@ struct snd_info_private_data {
>  };
>  
>  static int snd_info_version_init(void);
> -static void snd_info_disconnect(struct snd_info_entry *entry);
> +static void snd_info_clear_entries(struct snd_info_entry *entry);
>  
> /*
>  
> @@ -569,11 +569,16 @@ void snd_info_card_disconnect(struct snd_card *card)
>  {
>  	if (!card)
>  		return;
> -	mutex_lock(&info_mutex);
> +
>  	proc_remove(card->proc_root_link);
> -	card->proc_root_link = NULL;
>  	if (card->proc_root)
> -		snd_info_disconnect(card->proc_root);
> +		proc_remove(card->proc_root->p);
> +
> +	mutex_lock(&info_mutex);
> +	if (card->proc_root)
> +		snd_info_clear_entries(card->proc_root);
> +	card->proc_root_link = NULL;
> +	card->proc_root = NULL;
>  	mutex_unlock(&info_mutex);
> }
>  
> @@ -745,15 +750,14 @@ struct snd_info_entry
*snd_info_create_card_entry(struct snd_card *card,
>  }
>  EXPORT_SYMBOL(snd_info_create_card_entry);
>  
> -static void snd_info_disconnect(struct snd_info_entry *entry)
> +static void snd_info_clear_entries(struct snd_info_entry *entry)
>  {
>  	struct snd_info_entry *p;
>  
>  	if (!entry->p)
>  		return;
>  	list_for_each_entry(p, &entry->children, list)
> -		snd_info_disconnect(p);
> -	proc_remove(entry->p);
> +		snd_info_clear_entries(p);
>  	entry->p = NULL;
>  }
>  
> @@ -770,8 +774,9 @@ void snd_info_free_entry(struct snd_info_entry *
entry)
>  	if (!entry)
>  		return;
>  	if (entry->p) {
> +		proc_remove(entry->p);
>  		mutex_lock(&info_mutex);
> -		snd_info_disconnect(entry);
> +		snd_info_clear_entries(entry);
>  		mutex_unlock(&info_mutex);
>  	}