Skip to content

Commit 04de081

Browse files
author
Dominik Brodowski
committed
pcmcia: pcmcia_dev_present bugfix
pcmcia_dev_present is in and by itself buggy. Add a note specifying why it is broken, and replace the broken locking -- taking a mutex is a bad idea in IRQ context, from which this function is rarely called -- by an atomic_t. Signed-off-by: Dominik Brodowski <linux@dominikbrodowski.net>
1 parent 05ce7bf commit 04de081

File tree

3 files changed

+20
-42
lines changed

3 files changed

+20
-42
lines changed

‎drivers/pcmcia/ds.c

Lines changed: 14 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -335,7 +335,6 @@ static void pcmcia_card_remove(struct pcmcia_socket *s, struct pcmcia_device *le
335335

336336
mutex_lock(&s->ops_mutex);
337337
list_del(&p_dev->socket_device_list);
338-
p_dev->_removed = 1;
339338
mutex_unlock(&s->ops_mutex);
340339

341340
dev_dbg(&p_dev->dev, "unregistering device\n");
@@ -654,14 +653,7 @@ static int pcmcia_requery_callback(struct device *dev, void * _data)
654653

655654
static void pcmcia_requery(struct pcmcia_socket *s)
656655
{
657-
int present, has_pfc;
658-
659-
mutex_lock(&s->ops_mutex);
660-
present = s->pcmcia_state.present;
661-
mutex_unlock(&s->ops_mutex);
662-
663-
if (!present)
664-
return;
656+
int has_pfc;
665657

666658
if (s->functions == 0) {
667659
pcmcia_card_add(s);
@@ -1260,9 +1252,7 @@ static int ds_event(struct pcmcia_socket *skt, event_t event, int priority)
12601252

12611253
switch (event) {
12621254
case CS_EVENT_CARD_REMOVAL:
1263-
mutex_lock(&s->ops_mutex);
1264-
s->pcmcia_state.present = 0;
1265-
mutex_unlock(&s->ops_mutex);
1255+
atomic_set(&skt->present, 0);
12661256
pcmcia_card_remove(skt, NULL);
12671257
handle_event(skt, event);
12681258
mutex_lock(&s->ops_mutex);
@@ -1271,9 +1261,9 @@ static int ds_event(struct pcmcia_socket *skt, event_t event, int priority)
12711261
break;
12721262

12731263
case CS_EVENT_CARD_INSERTION:
1264+
atomic_set(&skt->present, 1);
12741265
mutex_lock(&s->ops_mutex);
12751266
s->pcmcia_state.has_pfc = 0;
1276-
s->pcmcia_state.present = 1;
12771267
destroy_cis_cache(s); /* to be on the safe side... */
12781268
mutex_unlock(&s->ops_mutex);
12791269
pcmcia_card_add(skt);
@@ -1313,7 +1303,13 @@ static int ds_event(struct pcmcia_socket *skt, event_t event, int priority)
13131303
return 0;
13141304
} /* ds_event */
13151305

1316-
1306+
/*
1307+
* NOTE: This is racy. There's no guarantee the card will still be
1308+
* physically present, even if the call to this function returns
1309+
* non-NULL. Furthermore, the device driver most likely is unbound
1310+
* almost immediately, so the timeframe where pcmcia_dev_present
1311+
* returns NULL is probably really really small.
1312+
*/
13171313
struct pcmcia_device *pcmcia_dev_present(struct pcmcia_device *_p_dev)
13181314
{
13191315
struct pcmcia_device *p_dev;
@@ -1323,22 +1319,9 @@ struct pcmcia_device *pcmcia_dev_present(struct pcmcia_device *_p_dev)
13231319
if (!p_dev)
13241320
return NULL;
13251321

1326-
mutex_lock(&p_dev->socket->ops_mutex);
1327-
if (!p_dev->socket->pcmcia_state.present)
1328-
goto out;
1322+
if (atomic_read(&p_dev->socket->present) != 0)
1323+
ret = p_dev;
13291324

1330-
if (p_dev->socket->pcmcia_state.dead)
1331-
goto out;
1332-
1333-
if (p_dev->_removed)
1334-
goto out;
1335-
1336-
if (p_dev->suspended)
1337-
goto out;
1338-
1339-
ret = p_dev;
1340-
out:
1341-
mutex_unlock(&p_dev->socket->ops_mutex);
13421325
pcmcia_put_dev(p_dev);
13431326
return ret;
13441327
}
@@ -1388,6 +1371,8 @@ static int __devinit pcmcia_bus_add_socket(struct device *dev,
13881371
return ret;
13891372
}
13901373

1374+
atomic_set(&socket->present, 0);
1375+
13911376
return 0;
13921377
}
13931378

@@ -1399,10 +1384,6 @@ static void pcmcia_bus_remove_socket(struct device *dev,
13991384
if (!socket)
14001385
return;
14011386

1402-
mutex_lock(&socket->ops_mutex);
1403-
socket->pcmcia_state.dead = 1;
1404-
mutex_unlock(&socket->ops_mutex);
1405-
14061387
pccard_register_pcmcia(socket, NULL);
14071388

14081389
/* unregister any unbound devices */

‎include/pcmcia/ds.h

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
#ifdef __KERNEL__
2727
#include <linux/device.h>
2828
#include <pcmcia/ss.h>
29+
#include <asm/atomic.h>
2930

3031
/*
3132
* PCMCIA device drivers (16-bit cards only; 32-bit cards require CardBus
@@ -94,10 +95,8 @@ struct pcmcia_device {
9495
config_req_t conf;
9596
window_handle_t win;
9697

97-
/* Is the device suspended, or in the process of
98-
* being removed? */
98+
/* Is the device suspended? */
9999
u16 suspended:1;
100-
u16 _removed:1;
101100

102101
/* Flags whether io, irq, win configurations were
103102
* requested, and whether the configuration is "locked" */
@@ -115,7 +114,7 @@ struct pcmcia_device {
115114
u16 has_card_id:1;
116115
u16 has_func_id:1;
117116

118-
u16 reserved:3;
117+
u16 reserved:4;
119118

120119
u8 func_id;
121120
u16 manf_id;

‎include/pcmcia/ss.h

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -224,18 +224,16 @@ struct pcmcia_socket {
224224

225225
/* 16-bit state: */
226226
struct {
227-
/* PCMCIA card is present in socket */
228-
u8 present:1;
229227
/* "master" ioctl is used */
230228
u8 busy:1;
231-
/* pcmcia module is being unloaded */
232-
u8 dead:1;
233229
/* the PCMCIA card consists of two pseudo devices */
234230
u8 has_pfc:1;
235231

236-
u8 reserved:4;
232+
u8 reserved:6;
237233
} pcmcia_state;
238234

235+
/* non-zero if PCMCIA card is present */
236+
atomic_t present;
239237

240238
#ifdef CONFIG_PCMCIA_IOCTL
241239
struct user_info_t *user;

0 commit comments

Comments
 (0)