[PATCH] pcmcia/pccard deadlock fix

[PATCH] pcmcia/pccard deadlock fix

[Markus Rechberger]

[-- Attachment #1: Type: text/plain, Size: 345 bytes --]

following patch prevents a mutex/semaphore deadlock within the pcmcia
framework when ejecting devices multiple times using pccardctl eject.

For some more details see:
http://lkml.org/lkml/2007/2/19/58

Signed-off-by: Markus Rechberger <markus.rechberger@amd.com>

-- 
Markus Rechberger
Operating System Research Center
AMD Saxony LLC & Co. KG


[-- Attachment #2: pcmcia-pccard-deadlock-fix.diff --]
[-- Type: text/plain, Size: 756 bytes --]

index ac00424..c02bf0d 100644
--- a/drivers/pcmcia/cs.c
+++ b/drivers/pcmcia/cs.c
@@ -856,7 +856,8 @@ int pcmcia_eject_card(struct pcmcia_socket *skt)
     
 	cs_dbg(skt, 1, "user eject request\n");
 
-	mutex_lock(&skt->skt_mutex);
+	if (!mutex_trylock(&skt->skt_mutex)) 
+		return -EAGAIN;
 	do {
 		if (!(skt->state & SOCKET_PRESENT)) {
 			ret = -ENODEV;
index 18e111e..b9d3440 100644
--- a/drivers/pcmcia/ds.c
+++ b/drivers/pcmcia/ds.c
@@ -1100,7 +1100,9 @@ static ssize_t pcmcia_store_allow_func_id_match(struct device *dev,
 	if (!count)
 		return -EINVAL;
 
-	mutex_lock(&p_dev->socket->skt_mutex);
+	if (!mutex_trylock(&p_dev->socket->skt_mutex)) 
+		return -EAGAIN;
+
 	p_dev->allow_func_id_match = 1;
 	mutex_unlock(&p_dev->socket->skt_mutex);
 

Re: [PATCH] pcmcia/pccard deadlock fix

[Andrew Morton]

> On Tue, 20 Feb 2007 16:08:11 +0100 "Markus Rechberger" <markus.rechberger@amd.com> wrote:
> following patch prevents a mutex/semaphore deadlock within the pcmcia
> framework when ejecting devices multiple times using pccardctl eject.
> 
> For some more details see:
> http://lkml.org/lkml/2007/2/19/58
> 
> --- a/drivers/pcmcia/cs.c
> +++ b/drivers/pcmcia/cs.c
> @@ -856,7 +856,8 @@ int pcmcia_eject_card(struct pcmcia_socket *skt)
>      
>  	cs_dbg(skt, 1, "user eject request\n");
>  
> -	mutex_lock(&skt->skt_mutex);
> +	if (!mutex_trylock(&skt->skt_mutex)) 
> +		return -EAGAIN;
>  	do {
>  		if (!(skt->state & SOCKET_PRESENT)) {
>  			ret = -ENODEV;
> index 18e111e..b9d3440 100644
> --- a/drivers/pcmcia/ds.c
> +++ b/drivers/pcmcia/ds.c
> @@ -1100,7 +1100,9 @@ static ssize_t pcmcia_store_allow_func_id_match(struct device *dev,
>  	if (!count)
>  		return -EINVAL;
>  
> -	mutex_lock(&p_dev->socket->skt_mutex);
> +	if (!mutex_trylock(&p_dev->socket->skt_mutex)) 
> +		return -EAGAIN;
> +
>  	p_dev->allow_func_id_match = 1;
>  	mutex_unlock(&p_dev->socket->skt_mutex);

That looks rather hacky.  I'll queue it up to bug Dominik with, but
hopeully we'll come up with some fix which avoids the trylocks.

Re: [PATCH] pcmcia/pccard deadlock fix

[Andrew Morton]

> On Tue, 20 Feb 2007 16:08:11 +0100

20 Feb was a long time ago, sorry.  I was hoping to feed the pcmcia patches
through Dominik but I think he's busy with exams or such.  So I get to
pretend to be pcmcia maintainer.

"Markus Rechberger" <markus.rechberger@amd.com> wrote:

> following patch prevents a mutex/semaphore deadlock within the pcmcia
> framework when ejecting devices multiple times using pccardctl eject.
> 
> For some more details see:
> http://lkml.org/lkml/2007/2/19/58
> 
> Signed-off-by: Markus Rechberger <markus.rechberger@amd.com>
> 
> -- 
> Markus Rechberger
> Operating System Research Center
> AMD Saxony LLC & Co. KG
> 
> 
> 
> [pcmcia-pccard-deadlock-fix.diff  text/plain (757B)]
> index ac00424..c02bf0d 100644
> --- a/drivers/pcmcia/cs.c
> +++ b/drivers/pcmcia/cs.c
> @@ -856,7 +856,8 @@ int pcmcia_eject_card(struct pcmcia_socket *skt)
>      
>  	cs_dbg(skt, 1, "user eject request\n");
>  
> -	mutex_lock(&skt->skt_mutex);
> +	if (!mutex_trylock(&skt->skt_mutex)) 
> +		return -EAGAIN;
>  	do {
>  		if (!(skt->state & SOCKET_PRESENT)) {
>  			ret = -ENODEV;
> index 18e111e..b9d3440 100644
> --- a/drivers/pcmcia/ds.c
> +++ b/drivers/pcmcia/ds.c
> @@ -1100,7 +1100,9 @@ static ssize_t pcmcia_store_allow_func_id_match(struct device *dev,
>  	if (!count)
>  		return -EINVAL;
>  
> -	mutex_lock(&p_dev->socket->skt_mutex);
> +	if (!mutex_trylock(&p_dev->socket->skt_mutex)) 
> +		return -EAGAIN;
> +
>  	p_dev->allow_func_id_match = 1;
>  	mutex_unlock(&p_dev->socket->skt_mutex);
>  

This is a pretty sad-looking solution.  Does it not mean that sometimes
user-initiated actions will mysteriously fail?

Are you able to provide a more detailed description of why/how the deadlock
actually occurs so that perhaps a more robust fix can be implemented?

Thanks.

Re: [PATCH] pcmcia/pccard deadlock fix

[Markus Rechberger]

Andrew Morton wrote:
>> On Tue, 20 Feb 2007 16:08:11 +0100
>>     
>
> 20 Feb was a long time ago, sorry.  I was hoping to feed the pcmcia patches
> through Dominik but I think he's busy with exams or such.  So I get to
> pretend to be pcmcia maintainer.
>
> "Markus Rechberger" <markus.rechberger@amd.com> wrote:
>
>   
>> following patch prevents a mutex/semaphore deadlock within the pcmcia
>> framework when ejecting devices multiple times using pccardctl eject.
>>
>> For some more details see:
>> http://lkml.org/lkml/2007/2/19/58
>>
>> Signed-off-by: Markus Rechberger <markus.rechberger@amd.com>
>>
>> -- 
>> Markus Rechberger
>> Operating System Research Center
>> AMD Saxony LLC & Co. KG
>>
>>
>>
>> [pcmcia-pccard-deadlock-fix.diff  text/plain (757B)]
>> index ac00424..c02bf0d 100644
>> --- a/drivers/pcmcia/cs.c
>> +++ b/drivers/pcmcia/cs.c
>> @@ -856,7 +856,8 @@ int pcmcia_eject_card(struct pcmcia_socket *skt)
>>      
>>  	cs_dbg(skt, 1, "user eject request\n");
>>  
>> -	mutex_lock(&skt->skt_mutex);
>> +	if (!mutex_trylock(&skt->skt_mutex)) 
>> +		return -EAGAIN;
>>  	do {
>>  		if (!(skt->state & SOCKET_PRESENT)) {
>>  			ret = -ENODEV;
>> index 18e111e..b9d3440 100644
>> --- a/drivers/pcmcia/ds.c
>> +++ b/drivers/pcmcia/ds.c
>> @@ -1100,7 +1100,9 @@ static ssize_t pcmcia_store_allow_func_id_match(struct device *dev,
>>  	if (!count)
>>  		return -EINVAL;
>>  
>> -	mutex_lock(&p_dev->socket->skt_mutex);
>> +	if (!mutex_trylock(&p_dev->socket->skt_mutex)) 
>> +		return -EAGAIN;
>> +
>>  	p_dev->allow_func_id_match = 1;
>>  	mutex_unlock(&p_dev->socket->skt_mutex);
>>  
>>     
>
> This is a pretty sad-looking solution.  Does it not mean that sometimes
> user-initiated actions will mysteriously fail?
>   
The userspace application should return the appropriate error then. It 
can really happen any time when someone tries to eject the pcmcia device 
(any time as within 1 minute - never in a normal scenario)

> Are you able to provide a more detailed description of why/how the deadlock
> actually occurs so that perhaps a more robust fix can be implemented?
>
>   
There's a description about it in following Email:
http://lkml.org/lkml/2007/2/19/58

Markus