diff options
author | Dominik Brodowski <linux@dominikbrodowski.net> | 2010-04-20 08:49:01 -0400 |
---|---|---|
committer | Dominik Brodowski <linux@dominikbrodowski.net> | 2010-04-21 02:09:17 -0400 |
commit | 04de0816173c86948b75da93a6344a0a02bbec4d (patch) | |
tree | ed4b274c1ce5da03a9c37f5858a331529e5b150f /drivers | |
parent | 05ce7bfe547c9fa967d9cab6c37867a9cb6fb3fa (diff) |
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>
Diffstat (limited to 'drivers')
-rw-r--r-- | drivers/pcmcia/ds.c | 47 |
1 files changed, 14 insertions, 33 deletions
diff --git a/drivers/pcmcia/ds.c b/drivers/pcmcia/ds.c index 4014cf8e4a26..92a5af8aa0b4 100644 --- a/drivers/pcmcia/ds.c +++ b/drivers/pcmcia/ds.c | |||
@@ -335,7 +335,6 @@ static void pcmcia_card_remove(struct pcmcia_socket *s, struct pcmcia_device *le | |||
335 | 335 | ||
336 | mutex_lock(&s->ops_mutex); | 336 | mutex_lock(&s->ops_mutex); |
337 | list_del(&p_dev->socket_device_list); | 337 | list_del(&p_dev->socket_device_list); |
338 | p_dev->_removed = 1; | ||
339 | mutex_unlock(&s->ops_mutex); | 338 | mutex_unlock(&s->ops_mutex); |
340 | 339 | ||
341 | dev_dbg(&p_dev->dev, "unregistering device\n"); | 340 | dev_dbg(&p_dev->dev, "unregistering device\n"); |
@@ -654,14 +653,7 @@ static int pcmcia_requery_callback(struct device *dev, void * _data) | |||
654 | 653 | ||
655 | static void pcmcia_requery(struct pcmcia_socket *s) | 654 | static void pcmcia_requery(struct pcmcia_socket *s) |
656 | { | 655 | { |
657 | int present, has_pfc; | 656 | int 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; | ||
665 | 657 | ||
666 | if (s->functions == 0) { | 658 | if (s->functions == 0) { |
667 | pcmcia_card_add(s); | 659 | pcmcia_card_add(s); |
@@ -1260,9 +1252,7 @@ static int ds_event(struct pcmcia_socket *skt, event_t event, int priority) | |||
1260 | 1252 | ||
1261 | switch (event) { | 1253 | switch (event) { |
1262 | case CS_EVENT_CARD_REMOVAL: | 1254 | case CS_EVENT_CARD_REMOVAL: |
1263 | mutex_lock(&s->ops_mutex); | 1255 | atomic_set(&skt->present, 0); |
1264 | s->pcmcia_state.present = 0; | ||
1265 | mutex_unlock(&s->ops_mutex); | ||
1266 | pcmcia_card_remove(skt, NULL); | 1256 | pcmcia_card_remove(skt, NULL); |
1267 | handle_event(skt, event); | 1257 | handle_event(skt, event); |
1268 | mutex_lock(&s->ops_mutex); | 1258 | mutex_lock(&s->ops_mutex); |
@@ -1271,9 +1261,9 @@ static int ds_event(struct pcmcia_socket *skt, event_t event, int priority) | |||
1271 | break; | 1261 | break; |
1272 | 1262 | ||
1273 | case CS_EVENT_CARD_INSERTION: | 1263 | case CS_EVENT_CARD_INSERTION: |
1264 | atomic_set(&skt->present, 1); | ||
1274 | mutex_lock(&s->ops_mutex); | 1265 | mutex_lock(&s->ops_mutex); |
1275 | s->pcmcia_state.has_pfc = 0; | 1266 | s->pcmcia_state.has_pfc = 0; |
1276 | s->pcmcia_state.present = 1; | ||
1277 | destroy_cis_cache(s); /* to be on the safe side... */ | 1267 | destroy_cis_cache(s); /* to be on the safe side... */ |
1278 | mutex_unlock(&s->ops_mutex); | 1268 | mutex_unlock(&s->ops_mutex); |
1279 | pcmcia_card_add(skt); | 1269 | pcmcia_card_add(skt); |
@@ -1313,7 +1303,13 @@ static int ds_event(struct pcmcia_socket *skt, event_t event, int priority) | |||
1313 | return 0; | 1303 | return 0; |
1314 | } /* ds_event */ | 1304 | } /* ds_event */ |
1315 | 1305 | ||
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 | */ | ||
1317 | struct pcmcia_device *pcmcia_dev_present(struct pcmcia_device *_p_dev) | 1313 | struct pcmcia_device *pcmcia_dev_present(struct pcmcia_device *_p_dev) |
1318 | { | 1314 | { |
1319 | struct pcmcia_device *p_dev; | 1315 | struct pcmcia_device *p_dev; |
@@ -1323,22 +1319,9 @@ struct pcmcia_device *pcmcia_dev_present(struct pcmcia_device *_p_dev) | |||
1323 | if (!p_dev) | 1319 | if (!p_dev) |
1324 | return NULL; | 1320 | return NULL; |
1325 | 1321 | ||
1326 | mutex_lock(&p_dev->socket->ops_mutex); | 1322 | if (atomic_read(&p_dev->socket->present) != 0) |
1327 | if (!p_dev->socket->pcmcia_state.present) | 1323 | ret = p_dev; |
1328 | goto out; | ||
1329 | 1324 | ||
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); | ||
1342 | pcmcia_put_dev(p_dev); | 1325 | pcmcia_put_dev(p_dev); |
1343 | return ret; | 1326 | return ret; |
1344 | } | 1327 | } |
@@ -1388,6 +1371,8 @@ static int __devinit pcmcia_bus_add_socket(struct device *dev, | |||
1388 | return ret; | 1371 | return ret; |
1389 | } | 1372 | } |
1390 | 1373 | ||
1374 | atomic_set(&socket->present, 0); | ||
1375 | |||
1391 | return 0; | 1376 | return 0; |
1392 | } | 1377 | } |
1393 | 1378 | ||
@@ -1399,10 +1384,6 @@ static void pcmcia_bus_remove_socket(struct device *dev, | |||
1399 | if (!socket) | 1384 | if (!socket) |
1400 | return; | 1385 | return; |
1401 | 1386 | ||
1402 | mutex_lock(&socket->ops_mutex); | ||
1403 | socket->pcmcia_state.dead = 1; | ||
1404 | mutex_unlock(&socket->ops_mutex); | ||
1405 | |||
1406 | pccard_register_pcmcia(socket, NULL); | 1387 | pccard_register_pcmcia(socket, NULL); |
1407 | 1388 | ||
1408 | /* unregister any unbound devices */ | 1389 | /* unregister any unbound devices */ |