Re: any chance of 2.6.0-test*?

From: Paul Mackerras (paulus@samba.org)
Date: Sun Jan 12 2003 - 22:33:30 EST


Alan Cox writes:

> The pcmcia stuff crashes erratically with the old IDE the bugs have not
> changed but the problem now shows up far more often. With a 2Gb type II
> PCMCIA IDE disk I get a crash about 1 in 3 ejects now.

I'm using the patch below, which makes sure that ide_release (in
ide-cs.c) runs in process context not interrupt context. The specific
problem I saw was that ide_unregister calls various devfs routines
that don't like being called in interrupt context, but there may well
be other thing ide_unregister does that aren't safe in interrupt
context.

I am also hitting another problem on my powerbook that I would like
your advice on. The thing is that my powerbook (a 500MHz G4 titanium
powerbook) has 3 IDE interfaces, but only the first two have anything
attached. The pmac IDE driver goes through and initializes all three,
including setting hwif->selectproc for each one. Then, when I plug in
a CF card in the pcmcia/cardbus slot, ide_register_hw goes and looks
for a free ide_hwifs[] entry. Slot 2 appears to be available because
there are no devices attached, and so it uses that. In the process of
setting up ide_hwifs[2], it sets hwif->io_ports but it doesn't reset
selectproc (or hwif->OUTB etc., AFAICS). The result is that it calls
the pmac IDE selectproc, which crashes (since we munged hwif->io_ports
without telling it).

I have "fixed" the problem for now by adding a call to
init_hwif_data(index) in ide_register_hw just before the first
memcpy. I'd be interested to know what the right fix is. :)

Regards,
Paul.

diff -urN linux-2.4/drivers/ide/legacy/ide-cs.c pmac/drivers/ide/legacy/ide-cs.c
--- linux-2.4/drivers/ide/legacy/ide-cs.c 2002-12-19 11:50:35.000000000 +1100
+++ pmac/drivers/ide/legacy/ide-cs.c 2003-01-13 14:02:31.000000000 +1100
@@ -92,10 +92,11 @@
     int ndev;
     dev_node_t node;
     int hd;
+ struct tq_struct rel_task;
 } ide_info_t;
 
 static void ide_config(dev_link_t *link);
-static void ide_release(u_long arg);
+static void ide_release(void *arg);
 static int ide_event(event_t event, int priority,
                      event_callback_args_t *args);
 
@@ -136,9 +137,8 @@
     if (!info) return NULL;
     memset(info, 0, sizeof(*info));
     link = &info->link; link->priv = info;
+ INIT_TQUEUE(&info->rel_task, ide_release, link);
 
- link->release.function = &ide_release;
- link->release.data = (u_long)link;
     link->io.Attributes1 = IO_DATA_PATH_WIDTH_AUTO;
     link->io.Attributes2 = IO_DATA_PATH_WIDTH_8;
     link->io.IOAddrLines = 3;
@@ -187,6 +187,7 @@
 static void ide_detach(dev_link_t *link)
 {
     dev_link_t **linkp;
+ ide_info_t *info = link->priv;
     int ret;
 
     DEBUG(0, "ide_detach(0x%p)\n", link);
@@ -197,9 +198,10 @@
     if (*linkp == NULL)
         return;
 
- del_timer(&link->release);
- if (link->state & DEV_CONFIG)
- ide_release((u_long)link);
+ if (link->state & DEV_CONFIG) {
+ schedule_task(&info->rel_task);
+ flush_scheduled_tasks();
+ }
     
     if (link->handle) {
         ret = CardServices(DeregisterClient, link->handle);
@@ -209,7 +211,7 @@
     
     /* Unlink, free device structure */
     *linkp = link->next;
- kfree(link->priv);
+ kfree(info);
     
 } /* ide_detach */
 
@@ -381,7 +383,7 @@
 cs_failed:
     cs_error(link->handle, last_fn, last_ret);
 failed:
- ide_release((u_long)link);
+ ide_release(link);
 
 } /* ide_config */
 
@@ -393,12 +395,15 @@
     
 ======================================================================*/
 
-void ide_release(u_long arg)
+static void ide_release(void *arg)
 {
- dev_link_t *link = (dev_link_t *)arg;
+ dev_link_t *link = arg;
     ide_info_t *info = link->priv;
     
- DEBUG(0, "ide_release(0x%p)\n", link);
+ if (!(link->state & DEV_CONFIG))
+ return;
+
+ DEBUG(0, "ide_do_release(0x%p)\n", link);
 
     if (info->ndev) {
         /* FIXME: if this fails we need to queue the cleanup somehow
@@ -435,6 +440,7 @@
               event_callback_args_t *args)
 {
     dev_link_t *link = args->client_data;
+ ide_info_t *info = link->priv;
 
     DEBUG(1, "ide_event(0x%06x)\n", event);
     
@@ -442,7 +448,7 @@
     case CS_EVENT_CARD_REMOVAL:
         link->state &= ~DEV_PRESENT;
         if (link->state & DEV_CONFIG)
- mod_timer(&link->release, jiffies + HZ/20);
+ schedule_task(&info->rel_task);
         break;
     case CS_EVENT_CARD_INSERTION:
         link->state |= DEV_PRESENT | DEV_CONFIG_PENDING;
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/



This archive was generated by hypermail 2b29 : Wed Jan 15 2003 - 22:00:43 EST