PATCH: straighten out the IDE layer locking and add hotplug

From: Alan Cox (alan_at_redhat.com)
Date: 08/15/04

  • Next message: Alan Cox: "PATCH: convert pci ide support to new locking"
    Date:	Sun, 15 Aug 2004 11:13:46 -0400
    To: linux-ide@vger.kernel.org, linux-kernel@vger.kernel.org, torvalds@osdl.org
    
    

    There really isnt any sane way to break this patch down because all the
    changes are interlinked so closely.

    We now
            - enforce and document an actual lock order
            - use a seperate semaphore to get sleep-read/spin-write locking for
              the lists that need it
            - Fix the driver list locking
            - Fix the hwif list locking
            - Remove the impossible to fix and unused stuff for passing
              a device between drivers
            - Change ide_unregister into ide_unregister_hwif as it now takes
              a hwif pointer
            - Fix various cases we wrongly dropped locks
            - Split configured/present so that we can do unregister right
            - Call ->remove() callbacks

    This patch leaves pci and ide-cs broken, patches to move them to the new
    locking follow

    diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.8-rc3/drivers/ide/ide.c linux-2.6.8-rc3/drivers/ide/ide.c
    --- linux.vanilla-2.6.8-rc3/drivers/ide/ide.c 2004-08-09 15:51:00.000000000 +0100
    +++ linux-2.6.8-rc3/drivers/ide/ide.c 2004-08-12 17:55:05.000000000 +0100
    @@ -448,31 +515,46 @@
             return -ENXIO;
     }
     
    +/*
    + * drives_lock protects the list of drives, drivers lock the
    + * list of drivers. Currently nobody takes both at once.
    + * drivers_sem guards the drivers_list for readers that may
    + * sleep. It must be taken before drivers_lock. Take drivers_sem
    + * before ide_setting_sem and idecfg_sem before either of the
    + * others.
    + */
    +
     static spinlock_t drives_lock = SPIN_LOCK_UNLOCKED;
    +static DECLARE_MUTEX(drivers_sem);
     static spinlock_t drivers_lock = SPIN_LOCK_UNLOCKED;
    +
     static LIST_HEAD(drivers);
     
    -/* Iterator */
    +/* Iterator for the driver list. */
    +
     static void *m_start(struct seq_file *m, loff_t *pos)
     {
             struct list_head *p;
             loff_t l = *pos;
    - spin_lock(&drivers_lock);
    + down(&drivers_sem);
             list_for_each(p, &drivers)
                     if (!l--)
                             return list_entry(p, ide_driver_t, drivers);
             return NULL;
     }
    +
     static void *m_next(struct seq_file *m, void *v, loff_t *pos)
     {
             struct list_head *p = ((ide_driver_t *)v)->drivers.next;
             (*pos)++;
             return p==&drivers ? NULL : list_entry(p, ide_driver_t, drivers);
     }
    +
     static void m_stop(struct seq_file *m, void *v)
     {
    - spin_unlock(&drivers_lock);
    + up(&drivers_sem);
     }
    +
     static int show_driver(struct seq_file *m, void *v)
     {
             ide_driver_t *driver = v;
    @@ -515,6 +597,7 @@
      * MMIO leaves it to the controller driver,
      * PIO will migrate this way over time.
      */
    +
     int ide_hwif_request_regions(ide_hwif_t *hwif)
     {
             unsigned long addr;
    @@ -564,6 +647,7 @@
      * importantly our caller should be doing this so we need to
      * restructure this as a helper function for drivers.
      */
    +
     void ide_hwif_release_regions(ide_hwif_t *hwif)
     {
             u32 i = 0;
    @@ -581,7 +665,15 @@
                             release_region(hwif->io_ports[i], 1);
     }
     
    -/* restore hwif to a sane state */
    +/**
    + * ide_hwif_restore - restore hwif to template
    + * @hwif: hwif to update
    + * @tmp_hwif: template
    + *
    + * Restore hwif to a default state by copying most settngs
    + * from the template.
    + */
    +
     static void ide_hwif_restore(ide_hwif_t *hwif, ide_hwif_t *tmp_hwif)
     {
             hwif->hwgroup = tmp_hwif->hwgroup;
    @@ -678,15 +771,16 @@
     }
     
     /**
    - * ide_unregister - free an ide interface
    - * @index: index of interface (will change soon to a pointer)
    + * __ide_unregister_hwif - free an ide interface
    + * @hwif: interface to unregister
      *
      * Perform the final unregister of an IDE interface. At the moment
      * we don't refcount interfaces so this will also get split up.
      *
      * Locking:
    - * The caller must not hold the IDE locks
    - * The drive present/vanishing is not yet properly locked
    + * The caller must not hold the IDE locks except for ide_cfg_sem
    + * which must be held.
    + *
      * Take care with the callbacks. These have been split to avoid
      * deadlocking the IDE layer. The shutdown callback is called
      * before we take the lock and free resources. It is up to the
    @@ -695,31 +789,43 @@
      * isnt yet done btw). After we commit to the final kill we
      * call the cleanup callback with the ide locks held.
      *
    + * An interface can be in four states we care about
    + * - It can be busy (drive or driver thinks its active). No unload
    + * - It can be unconfigured - which means its already gone
    + * - It can be configured and present - a full interface
    + * - It can be configured and not present - pci configured but no drives
    + * so logically absent.
    + *
      * Unregister restores the hwif structures to the default state.
    - * This is raving bonkers.
      */
     
    -void ide_unregister(unsigned int index)
    +int __ide_unregister_hwif(ide_hwif_t *hwif)
     {
             ide_drive_t *drive;
    - ide_hwif_t *hwif, *g, *tmp_hwif;
    + ide_hwif_t *g;
             ide_hwgroup_t *hwgroup;
             int irq_count = 0, unit, i;
    + int index;
    + static ide_hwif_t tmp_hwif; /* Serialized by locking */
    + int ret = 0;
    + int was_present;
     
    - BUG_ON(index >= MAX_HWIFS);
    -
    - tmp_hwif = kmalloc(sizeof(*tmp_hwif), GFP_KERNEL|__GFP_NOFAIL);
    - if (!tmp_hwif) {
    - printk(KERN_ERR "%s: unable to allocate memory\n", __FUNCTION__);
    - return;
    - }
    -
    + index = hwif->index;
    +
             BUG_ON(in_interrupt());
             BUG_ON(irqs_disabled());
    - down(&ide_cfg_sem);
    +
    + /* Make sure nobody sneaks in via the proc interface */
    + down(&ide_setting_sem);
    +
    + /* Now ensure nobody gets in for I/O while we clean up and
    + do the busy check. If busy is set then the device is still
    + open and we must stop */
             spin_lock_irq(&ide_lock);
    - hwif = &ide_hwifs[index];
    - if (!hwif->present)
    +
    + printk("Unregister %d\n", index);
    +
    + if (!hwif->configured)
                     goto abort;
             for (unit = 0; unit < MAX_DRIVES; ++unit) {
                     drive = &hwif->drives[unit];
    @@ -729,10 +835,18 @@
                             goto abort;
                     drive->dead = 1;
             }
    + /*
    + * Protect against new users. From this point the hwif
    + * is not present so cannot be opened by a new I/O source.
    + * This also invalidates key driven access from procfs
    + */
    +
    + was_present = hwif->present;
             hwif->present = 0;
     
             spin_unlock_irq(&ide_lock);
    -
    + up(&ide_setting_sem);
    +
             for (unit = 0; unit < MAX_DRIVES; ++unit) {
                     drive = &hwif->drives[unit];
                     if (!drive->present)
    @@ -743,27 +857,36 @@
     #ifdef CONFIG_PROC_FS
             destroy_proc_ide_drives(hwif);
     #endif
    -
    + spin_lock_irq(&ide_lock);
    +
             hwgroup = hwif->hwgroup;
    - /*
    - * free the irq if we were the only hwif using it
    - */
    - g = hwgroup->hwif;
    - do {
    - if (g->irq == hwif->irq)
    - ++irq_count;
    - g = g->next;
    - } while (g != hwgroup->hwif);
    - if (irq_count == 1)
    + if(hwgroup)
    + {
    + /*
    + * free the irq if we were the only hwif using it
    + */
    + g = hwgroup->hwif;
    + do {
    + if (g->irq == hwif->irq)
    + ++irq_count;
    + g = g->next;
    + } while (g != hwgroup->hwif);
    + }
    + spin_unlock_irq(&ide_lock);
    +
    + if (irq_count == 1 && hwgroup)
                     free_irq(hwif->irq, hwgroup);
     
    - spin_lock_irq(&ide_lock);
             /*
              * Note that we only release the standard ports,
              * and do not even try to handle any extra ports
              * allocated for weird IDE interface chipsets.
    + *
    + * FIXME: should defer this I think
              */
    - ide_hwif_release_regions(hwif);
    +
    + if(was_present)
    + ide_hwif_release_regions(hwif);
     
             /*
              * Remove us from the hwgroup, and free
    @@ -777,6 +900,14 @@
                     }
                     if (!drive->present)
                             continue;
    +
    + /*
    + * The hwgroup chain is IRQ touched. We must protect
    + * walking this from an IDE event for another device
    + * in the chain
    + */
    +
    + spin_lock_irq(&ide_lock);
                     if (drive == drive->next) {
                             /* special case: last drive from hwgroup. */
                             BUG_ON(hwgroup->drive != drive);
    @@ -793,51 +924,73 @@
                                     hwgroup->hwif = HWIF(hwgroup->drive);
                             }
                     }
    + spin_unlock_irq(&ide_lock);
    +
    + /*
    + * The rest of the cleanup is private
    + */
    +
                     BUG_ON(hwgroup->drive == drive);
                     if (drive->id != NULL) {
                             kfree(drive->id);
                             drive->id = NULL;
                     }
                     drive->present = 0;
    - /* Messed up locking ... */
    - spin_unlock_irq(&ide_lock);
                     blk_cleanup_queue(drive->queue);
                     device_unregister(&drive->gendev);
                     down(&drive->gendev_rel_sem);
    - spin_lock_irq(&ide_lock);
                     drive->queue = NULL;
             }
    - if (hwif->next == hwif) {
    - BUG_ON(hwgroup->hwif != hwif);
    - kfree(hwgroup);
    - } else {
    - /* There is another interface in hwgroup.
    - * Unlink us, and set hwgroup->drive and ->hwif to
    - * something sane.
    - */
    - g = hwgroup->hwif;
    - while (g->next != hwif)
    - g = g->next;
    - g->next = hwif->next;
    - if (hwgroup->hwif == hwif) {
    - /* Chose a random hwif for hwgroup->hwif.
    - * It's guaranteed that there are no drives
    - * left in the hwgroup.
    +
    + /*
    + * Lock against hwgroup walkers including interrupts off other
    + * IDE devices wile we unhook ourselves.
    + */
    +
    + spin_lock_irq(&ide_lock);
    +
    + if (hwgroup)
    + {
    + if (hwif->next == hwif) {
    + BUG_ON(hwgroup->hwif != hwif);
    + kfree(hwgroup);
    + } else {
    + /* There is another interface in hwgroup.
    + * Unlink us, and set hwgroup->drive and ->hwif to
    + * something sane.
                              */
    - BUG_ON(hwgroup->drive != NULL);
    - hwgroup->hwif = g;
    + g = hwgroup->hwif;
    + while (g->next != hwif)
    + g = g->next;
    + g->next = hwif->next;
    + if (hwgroup->hwif == hwif) {
    + /* Chose a random hwif for hwgroup->hwif.
    + * It's guaranteed that there are no drives
    + * left in the hwgroup.
    + */
    + BUG_ON(hwgroup->drive != NULL);
    + hwgroup->hwif = g;
    + }
    + BUG_ON(hwgroup->hwif == hwif);
                     }
    - BUG_ON(hwgroup->hwif == hwif);
             }
    -
    - /* More messed up locking ... */
             spin_unlock_irq(&ide_lock);
    - device_unregister(&hwif->gendev);
    - down(&hwif->gendev_rel_sem);
    +
    + /*
    + * PCI interfaces with no devices don't exist in the device
    + * tree so don't unregister them.
    + */
    +
    + if(was_present)
    + {
    + device_unregister(&hwif->gendev);
    + down(&hwif->gendev_rel_sem);
    + }
     
             /*
              * Remove us from the kernel's knowledge
              */
    +
             blk_unregister_region(MKDEV(hwif->major, 0), MAX_DRIVES<<PARTN_BITS);
             for (i = 0; i < MAX_DRIVES; i++) {
                     struct gendisk *disk = hwif->drives[i].disk;
    @@ -845,8 +998,16 @@
                     put_disk(disk);
             }
             unregister_blkdev(hwif->major, hwif->name);
    +
             spin_lock_irq(&ide_lock);
     
    + /*
    + * Let the driver free up private objects
    + */
    +
    + if(hwif->remove)
    + hwif->remove(hwif);
    +
             if (hwif->dma_base) {
                     (void) ide_release_dma(hwif);
     
    @@ -858,24 +1019,70 @@
                     hwif->dma_vendor3 = 0;
                     hwif->dma_prdtable = 0;
             }
    +
    + hwif->chipset = ide_unknown;
     
             /* copy original settings */
    - *tmp_hwif = *hwif;
    + tmp_hwif = *hwif;
     
             /* restore hwif data to pristine status */
             init_hwif_data(hwif, index);
             init_hwif_default(hwif, index);
    +
    + hwif->key++;
    + hwif->configured = 0;
     
    - ide_hwif_restore(hwif, tmp_hwif);
    + ide_hwif_restore(hwif, &tmp_hwif);
    +
    + spin_unlock_irq(&ide_lock);
    + return 0;
     
     abort:
    + if(hwif->configured)
    + {
    + printk("Unregister %d fail %d %d\n", index, drive->usage, DRIVER(drive)->busy);
    + ret = -EBUSY;
    + }
    + else
    + {
    + printk("No such hwif!\n");
    + ret = -ENOENT;
    + }
             spin_unlock_irq(&ide_lock);
    - up(&ide_cfg_sem);
    + up(&ide_setting_sem);
    + return ret;
    +}
    +
    +EXPORT_SYMBOL_GPL(__ide_unregister_hwif);
     
    - kfree(tmp_hwif);
    +
    +/**
    + * ide_unregister_hwif - free an ide interface
    + * @hwif: interface to unregister
    + *
    + * Perform the final unregister of an IDE interface. At the moment
    + * we don't refcount interfaces so this will also get split up.
    + * Unregister restores the hwif structures to the default state.
    + *
    + * No locks should be held on entry. When an unregister must
    + * be done atomically with a register see __ide_unregister_hwif
    + * and hold the ide_cfg_sem yourself.
    + */
    +
    +int ide_unregister_hwif(ide_hwif_t *hwif)
    +{
    + int ret;
    +
    + /* This protects two things. Firstly it serializes the
    + shutdown sequence, secondly it protects us from
    + races while we are killing off a device */
    + down(&ide_cfg_sem);
    + ret = __ide_unregister_hwif(hwif);
    + up(&ide_cfg_sem);
    + return ret;
     }
     
    -EXPORT_SYMBOL(ide_unregister);
    +EXPORT_SYMBOL_GPL(ide_unregister_hwif);
     
     
     /**
    @@ -931,15 +1138,25 @@
      */
     }
     
    -/*
    - * Register an IDE interface, specifying exactly the registers etc
    - * Set init=1 iff calling before probes have taken place.
    +/**
    + * ide_register_hw - register IDE interface
    + * @hw: hardware registers
    + * @hwifp: pointer to returned hwif
    + *
    + * Register an IDE interface, specifying exactly the registers etc
    + * Set init=1 iff calling before probes have taken place. The
    + * ide_cfg_sem protects this against races.
    + *
    + * Returns -1 on error.
      */
    +
     int ide_register_hw (hw_regs_t *hw, ide_hwif_t **hwifp)
     {
             int index, retry = 1;
             ide_hwif_t *hwif;
     
    + down(&ide_cfg_sem);
    +
             do {
                     for (index = 0; index < MAX_HWIFS; ++index) {
                             hwif = &ide_hwifs[index];
    @@ -950,28 +1167,37 @@
                             hwif = &ide_hwifs[index];
                             if (hwif->hold)
                                     continue;
    - if ((!hwif->present && !hwif->mate && !initializing) ||
    + if ((!hwif->configured && !hwif->mate && !initializing) ||
                                 (!hwif->hw.io_ports[IDE_DATA_OFFSET] && initializing))
                                     goto found;
                     }
    + /* FIXME- this check should die as should the retry loop */
                     for (index = 0; index < MAX_HWIFS; index++)
    - ide_unregister(index);
    + {
    + hwif = &ide_hwifs[index];
    + __ide_unregister_hwif(hwif);
    + }
             } while (retry--);
    +
    + up(&ide_cfg_sem);
             return -1;
     found:
    - if (hwif->present)
    - ide_unregister(index);
    + /* FIXME: do we really need this case */
    + if (hwif->configured)
    + __ide_unregister_hwif(hwif);
             else if (!hwif->hold) {
                     init_hwif_data(hwif, index);
                     init_hwif_default(hwif, index);
             }
    - if (hwif->present)
    + if (hwif->configured)
                     return -1;
    + hwif->configured = 1;
             memcpy(&hwif->hw, hw, sizeof(*hw));
             memcpy(hwif->io_ports, hwif->hw.io_ports, sizeof(hwif->hw.io_ports));
             hwif->irq = hw->irq;
             hwif->noprobe = 0;
             hwif->chipset = hw->chipset;
    + up(&ide_cfg_sem);
     
             if (!initializing) {
                     probe_hwif_init(hwif);
    @@ -1008,21 +1234,21 @@
      * @set: setting
      *
      * Removes the setting named from the device if it is present.
    - * The function takes the settings_lock to protect against
    - * parallel changes. This function must not be called from IRQ
    - * context. Returns 0 on success or -1 on failure.
    + * This function must not be called from IRQ context. Returns 0
    + * on success or -1 on failure.
      *
      * BUGS: This code is seriously over-engineered. There is also
      * magic about how the driver specific features are setup. If
      * a driver is attached we assume the driver settings are auto
      * remove.
    + *
    + * The caller must hold settings_lock
      */
      
     int ide_add_setting (ide_drive_t *drive, const char *name, int rw, int read_ioctl, int write_ioctl, int data_type, int min, int max, int mul_factor, int div_factor, void *data, ide_procset_t *set)
     {
             ide_settings_t **p = (ide_settings_t **) &drive->settings, *setting = NULL;
     
    - down(&ide_setting_sem);
             while ((*p) && strcmp((*p)->name, name) < 0)
                     p = &((*p)->next);
             if ((setting = kmalloc(sizeof(*setting), GFP_KERNEL)) == NULL)
    @@ -1046,10 +1272,8 @@
             if (drive->driver != &idedefault_driver)
                     setting->auto_remove = 1;
             *p = setting;
    - up(&ide_setting_sem);
             return 0;
     abort:
    - up(&ide_setting_sem);
             if (setting)
                     kfree(setting);
             return -1;
    @@ -1058,7 +1282,7 @@
     EXPORT_SYMBOL(ide_add_setting);
     
     /**
    - * __ide_remove_setting - remove an ide setting option
    + * ide_remove_setting - remove an ide setting option
      * @drive: drive to use
      * @name: setting name
      *
    @@ -1066,7 +1290,7 @@
      * The caller must hold the setting semaphore.
      */
      
    -static void __ide_remove_setting (ide_drive_t *drive, char *name)
    +static void ide_remove_setting (ide_drive_t *drive, char *name)
     {
             ide_settings_t **p, *setting;
     
    @@ -1144,7 +1368,7 @@
             setting = drive->settings;
             while (setting) {
                     if (setting->auto_remove) {
    - __ide_remove_setting(drive, setting->name);
    + ide_remove_setting(drive, setting->name);
                             goto repeat;
                     }
                     setting = setting->next;
    @@ -1322,25 +1556,19 @@
             return err;
     }
     
    -int ide_atapi_to_scsi (ide_drive_t *drive, int arg)
    -{
    - if (drive->media == ide_disk) {
    - drive->scsi = 0;
    - return 0;
    - }
    -
    - if (DRIVER(drive)->cleanup(drive)) {
    - drive->scsi = 0;
    - return 0;
    - }
    -
    - drive->scsi = (u8) arg;
    - ata_attach(drive);
    - return 0;
    -}
     
    +/**
    + * ide_add_generic_settings - generic /proc settings
    + * @drive: drive being configured
    + *
    + * Add the generic parts of the system settings to the /proc files
    + * for this IDE device. The caller must not be holding the settings_sem
    + * .lock
    + */
    +
     void ide_add_generic_settings (ide_drive_t *drive)
     {
    + down(&ide_setting_sem);
     /*
      * drive setting name read/write access read ioctl write ioctl data type min max mul_factor div_factor data pointer set function
      */
    @@ -1353,10 +1581,17 @@
             ide_add_setting(drive, "init_speed", SETTING_RW, -1, -1, TYPE_BYTE, 0, 70, 1, 1, &drive->init_speed, NULL);
             ide_add_setting(drive, "current_speed", SETTING_RW, -1, -1, TYPE_BYTE, 0, 70, 1, 1, &drive->current_speed, set_xfer_rate);
             ide_add_setting(drive, "number", SETTING_RW, -1, -1, TYPE_BYTE, 0, 3, 1, 1, &drive->dn, NULL);
    - if (drive->media != ide_disk)
    - ide_add_setting(drive, "ide-scsi", SETTING_RW, -1, HDIO_SET_IDE_SCSI, TYPE_BYTE, 0, 1, 1, 1, &drive->scsi, ide_atapi_to_scsi);
    +
    + up(&ide_setting_sem);
     }
     
    +/**
    + * system_bus_clock - clock guess
    + *
    + * External version of the bus clock guess used by old old IDE drivers
    + * for things like VLB timings. Should not be used.
    + */
    +
     int system_bus_clock (void)
     {
             return((int) ((!system_bus_speed) ? ide_system_bus_speed() : system_bus_speed ));
    @@ -1364,52 +1599,42 @@
     
     EXPORT_SYMBOL(system_bus_clock);
     
    -/*
    - * Locking is badly broken here - since way back. That sucker is
    - * root-only, but that's not an excuse... The real question is what
    - * exclusion rules do we want here.
    +/**
    + * ata_attach - attach an ATA/ATAPI device
    + * @drive: drive to attach
    + *
    + * Takes a drive that is as yet not assigned to any midlayer IDE
    + * module and figures out which driver would like to own it. If
    + * nobody claims the driver then it is automatically attached
    + * to the default driver used for unclaimed objects.
    + *
    + * A return of zero indicates attachment to a driver, of one
    + * attachment to the default driver
    + *
    + * Takes the driver list lock and the ide_settings semaphore.
      */
    -int ide_replace_subdriver (ide_drive_t *drive, const char *driver)
    -{
    - if (!drive->present || drive->usage || drive->dead)
    - goto abort;
    - if (DRIVER(drive)->cleanup(drive))
    - goto abort;
    - strlcpy(drive->driver_req, driver, sizeof(drive->driver_req));
    - if (ata_attach(drive)) {
    - spin_lock(&drives_lock);
    - list_del_init(&drive->list);
    - spin_unlock(&drives_lock);
    - drive->driver_req[0] = 0;
    - ata_attach(drive);
    - } else {
    - drive->driver_req[0] = 0;
    - }
    - if (DRIVER(drive)!= &idedefault_driver && !strcmp(DRIVER(drive)->name, driver))
    - return 0;
    -abort:
    - return 1;
    -}
    -
    +
     int ata_attach(ide_drive_t *drive)
     {
             struct list_head *p;
    - spin_lock(&drivers_lock);
    + down(&drivers_sem);
    + down(&ide_setting_sem);
             list_for_each(p, &drivers) {
                     ide_driver_t *driver = list_entry(p, ide_driver_t, drivers);
                     if (!try_module_get(driver->owner))
                             continue;
    - spin_unlock(&drivers_lock);
                     if (driver->attach(drive) == 0) {
                             module_put(driver->owner);
                             drive->gendev.driver = &driver->gen_driver;
    + up(&ide_setting_sem);
    + up(&drivers_sem);
                             return 0;
                     }
    - spin_lock(&drivers_lock);
                     module_put(driver->owner);
             }
             drive->gendev.driver = &idedefault_driver.gen_driver;
    - spin_unlock(&drivers_lock);
    + up(&ide_setting_sem);
    + up(&drivers_sem);
             if(idedefault_driver.attach(drive) != 0)
                     panic("ide: default attach failed");
             return 1;
    @@ -1549,9 +1774,11 @@
                     }
                     case HDIO_UNREGISTER_HWIF:
                             if (!capable(CAP_SYS_RAWIO)) return -EACCES;
    - /* (arg > MAX_HWIFS) checked in function */
    - ide_unregister(arg);
    - return 0;
    + if(arg > MAX_HWIFS || arg < 0)
    + return -EINVAL;
    + if(ide_hwifs[arg].pci_dev)
    + return -EINVAL;
    + return ide_unregister_hwif(&ide_hwifs[arg]);
                     case HDIO_SET_NICE:
                             if (!capable(CAP_SYS_ADMIN)) return -EACCES;
                             if (arg != (arg & ((1 << IDE_NICE_DSC_OVERLAP) | (1 << IDE_NICE_1))))
    @@ -2199,9 +2426,23 @@
     
     EXPORT_SYMBOL(ide_register_subdriver);
     
    +/**
    + * ide_unregister_subdriver - disconnect drive from driver
    + * @drive: drive to unplug
    + *
    + * Disconnect a drive from the driver it was attached to and then
    + * clean up the various proc files and other objects attached to it.
    + * Takes ide_sem, ide_lock, and drive_lock. Caller must hold none of
    + * the locks.
    + *
    + * No locking versus subdriver unload because we are moving to the
    + * default driver anyway. Wants double checking.
    + */
    +
     int ide_unregister_subdriver (ide_drive_t *drive)
     {
             unsigned long flags;
    + ide_proc_entry_t *dir;
             
             down(&ide_setting_sem);
             spin_lock_irqsave(&ide_lock, flags);
    @@ -2210,13 +2451,14 @@
                     up(&ide_setting_sem);
                     return 1;
             }
    + dir = DRIVER(drive)->proc;
    + drive->driver = &idedefault_driver;
    + spin_unlock_irqrestore(&ide_lock, flags);
     #ifdef CONFIG_PROC_FS
    - ide_remove_proc_entries(drive->proc, DRIVER(drive)->proc);
    + ide_remove_proc_entries(drive->proc, dir);
             ide_remove_proc_entries(drive->proc, generic_subdriver_entries);
     #endif
             auto_remove_settings(drive);
    - drive->driver = &idedefault_driver;
    - spin_unlock_irqrestore(&ide_lock, flags);
             up(&ide_setting_sem);
             spin_lock(&drives_lock);
             list_del_init(&drive->list);
    @@ -2234,6 +2476,18 @@
             return 0;
     }
     
    +/**
    + * ide_register_driver - new driver loaded
    + * @driver: the IDE driver module
    + *
    + * Register a new driver module and then scan the devices
    + * on the IDE bus in case any should be attached to the
    + * driver we have just attached. If so attach them
    + *
    + * Takes the drivers and drives lock. Should take the
    + * ide_sem but doesn't - FIXME
    + */
    +
     int ide_register_driver(ide_driver_t *driver)
     {
             struct list_head list;
    @@ -2242,9 +2496,11 @@
     
             setup_driver_defaults(driver);
     
    + down(&drivers_sem);
             spin_lock(&drivers_lock);
             list_add(&driver->drivers, &drivers);
             spin_unlock(&drivers_lock);
    + up(&drivers_sem);
     
             INIT_LIST_HEAD(&list);
             spin_lock(&drives_lock);
    @@ -2265,13 +2521,25 @@
     
     EXPORT_SYMBOL(ide_register_driver);
     
    +/**
    + * ide_unregister_driver - IDE module unload
    + * @driver: IDE driver module
    + *
    + * Unload a driver module and reattach any devices to whatever
    + * driver claims them next (typically the default driver). Takes
    + * the drivers lock, and called functions will take the
    + * IDE setting semaphore.
    + */
    +
     void ide_unregister_driver(ide_driver_t *driver)
     {
             ide_drive_t *drive;
     
    + down(&drivers_sem);
             spin_lock(&drivers_lock);
             list_del(&driver->drivers);
             spin_unlock(&drivers_lock);
    + up(&drivers_sem);
     
             driver_unregister(&driver->gen_driver);
     
    @@ -2385,7 +2653,8 @@
             int index;
     
             for (index = 0; index < MAX_HWIFS; ++index) {
    - ide_unregister(index);
    + if(ide_unregister_hwif(&ide_hwifs[index]))
    + printk(KERN_ERR "ide: unload yet busy!\n");
                     if (ide_hwifs[index].dma_base)
                             (void) ide_release_dma(&ide_hwifs[index]);
             }
    diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.8-rc3/include/linux/ide.h linux-2.6.8-rc3/include/linux/ide.h
    --- linux.vanilla-2.6.8-rc3/include/linux/ide.h 2004-08-09 15:50:59.000000000 +0100
    +++ linux-2.6.8-rc3/include/linux/ide.h 2004-08-12 16:45:17.000000000 +0100
    @@ -1503,9 +1509,11 @@
     extern void ide_pci_unregister_driver(struct pci_driver *driver);
     extern void ide_pci_setup_ports(struct pci_dev *dev, struct ide_pci_device_s *d, int autodma, int pciirq, ata_index_t *index);
     extern void ide_setup_pci_noise (struct pci_dev *dev, struct ide_pci_device_s *d);
    +extern void ide_pci_remove_hwifs(struct pci_dev *dev);
     
     extern void default_hwif_iops(ide_hwif_t *);
     extern void default_hwif_mmiops(ide_hwif_t *);
    +extern void removed_hwif_iops(ide_hwif_t *);
     extern void default_hwif_transport(ide_hwif_t *);
     
     int ide_register_driver(ide_driver_t *driver);
    @@ -1603,8 +1611,9 @@
     #endif
     
     extern int ide_hwif_request_regions(ide_hwif_t *hwif);
    -extern void ide_hwif_release_regions(ide_hwif_t* hwif);
    -extern void ide_unregister (unsigned int index);
    +extern void ide_hwif_release_regions(ide_hwif_t *hwif);
    +extern int ide_unregister_hwif(ide_hwif_t *hwif);
    +extern int __ide_unregister_hwif(ide_hwif_t *hwif);
     
     extern int probe_hwif_init(ide_hwif_t *);

    Signed-off-by: Alan Cox <alan@redhat.com>
     
    -
    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/


  • Next message: Alan Cox: "PATCH: convert pci ide support to new locking"

    Relevant Pages