2003-06-01 22:45:18

by Adam J. Richter

[permalink] [raw]
Subject: 2.5.70-bk[56] breaks disk partitioning with multiple IDE disks

Disk partition under linux-2.5.70-bk[56] systems with two
IDE disks is broken. Under these kernels, the first disk gets
the partitioning of the second disk. Reverting drivers/ide/ide.c
to the 2.5.70-bk4 version makes the problem go away.

However, there were other changes made to ide.c that are
probably not related to this problem, and I haven't yet analyzed the
problem enough to suggest a potentially correct patch.

Note that I use user-level disk partition table parsing via
partx from util-linux and run ide as a loadable module. It is
possible that other configuations might not experience this problem.

Adam J. Richter __ ______________ 575 Oroville Road
[email protected] \ / Miplitas, California 95035
+1 408 309-6081 | g g d r a s i l United States of America
"Free Software For The Rest Of Us."


2003-06-01 23:40:12

by Al Viro

[permalink] [raw]
Subject: [FIX] Re: 2.5.70-bk[56] breaks disk partitioning with multiple IDE disks

On Sun, Jun 01, 2003 at 03:59:02PM -0700, Adam J. Richter wrote:
> Disk partition under linux-2.5.70-bk[56] systems with two
> IDE disks is broken. Under these kernels, the first disk gets
> the partitioning of the second disk. Reverting drivers/ide/ide.c
> to the 2.5.70-bk4 version makes the problem go away.

vi drivers/ide/ide.c -c'/drive->list.*driver->drives/s/list_add/&_tail/|x'

Now that we use idedefault_driver.drives instead of ata_unused, the order
of drives on the driver->drives becomes significant; note that when we added
to ata_unused, we had done that to the end of list.

2003-06-02 00:31:24

by Adam J. Richter

[permalink] [raw]
Subject: Re: 2.5.70-bk[56] breaks disk partitioning with multiple IDE disks

To add to my own posting, here is a patch to 2.5.70-bk6
that reverts only the changes that replaced ata_unused with
idedeault_driver.drives. This change makes bk6 work for me with
multiple IDE disk drives present.

I have tried a number of smaller changes to stock
2.5.70-bk6/drivers/ide/ide.c, but, I have not yet found a change
simpler than this that works.

Adam J. Richter __ ______________ 575 Oroville Road
[email protected] \ / Miplitas, California 95035
+1 408 309-6081 | g g d r a s i l United States of America
"Free Software For The Rest Of Us."


--- linux-2.5.70-bk6/drivers/ide/ide.c 2003-06-01 12:01:28.000000000 -0700
+++ linux/drivers/ide/ide.c 2003-06-01 16:52:28.000000000 -0700
@@ -462,6 +462,7 @@
return -ENXIO;
}

+static LIST_HEAD(ata_unused);
static spinlock_t drives_lock = SPIN_LOCK_UNLOCKED;
static spinlock_t drivers_lock = SPIN_LOCK_UNLOCKED;
static LIST_HEAD(drivers);
@@ -1436,6 +1437,9 @@
spin_unlock(&drivers_lock);
if(idedefault_driver.attach(drive) != 0)
panic("ide: default attach failed");
+ spin_lock(&drives_lock);
+ list_add_tail(&drive->list, &ata_unused);
+ spin_unlock(&drives_lock);
return 1;
}

@@ -2379,8 +2383,8 @@
spin_unlock_irqrestore(&ide_lock, flags);
spin_lock(&drives_lock);
list_del_init(&drive->list);
+ list_add(&drive->list, &drive->driver->drives);
spin_unlock(&drives_lock);
- /* drive will be added to &idedefault_driver->drives in ata_attach() */
return 0;
}

@@ -2403,9 +2407,9 @@
list_add(&driver->drivers, &drivers);
spin_unlock(&drivers_lock);

- INIT_LIST_HEAD(&list);
spin_lock(&drives_lock);
- list_splice_init(&idedefault_driver.drives, &list);
+ INIT_LIST_HEAD(&list);
+ list_splice_init(&ata_unused, &list);
spin_unlock(&drives_lock);

list_for_each_safe(list_loop, tmp_storage, &list) {

2003-06-02 01:36:46

by Linus Torvalds

[permalink] [raw]
Subject: Re: [FIX] Re: 2.5.70-bk[56] breaks disk partitioning with multiple IDE disks


On Mon, 2 Jun 2003 [email protected] wrote:
>
> vi drivers/ide/ide.c -c'/drive->list.*driver->drives/s/list_add/&_tail/|x'

Mind sending me a patch? There's only so much I like doing with vi
scripts, and this went over my threshold.

Linus

2003-06-02 01:55:40

by Al Viro

[permalink] [raw]
Subject: Re: [FIX] Re: 2.5.70-bk[56] breaks disk partitioning with multiple IDE disks

On Sun, Jun 01, 2003 at 06:49:41PM -0700, Linus Torvalds wrote:
>
> On Mon, 2 Jun 2003 [email protected] wrote:
> >
> > vi drivers/ide/ide.c -c'/drive->list.*driver->drives/s/list_add/&_tail/|x'
>
> Mind sending me a patch? There's only so much I like doing with vi
> scripts, and this went over my threshold.

No problem...

--- drivers/ide/ide.c Sat May 31 15:31:08 2003
+++ /tmp/ide.c Sun Jun 1 19:50:38 2003
@@ -2335,7 +2335,7 @@
setup_driver_defaults(drive);
spin_unlock_irqrestore(&ide_lock, flags);
spin_lock(&drives_lock);
- list_add(&drive->list, &driver->drives);
+ list_add_tail(&drive->list, &driver->drives);
spin_unlock(&drives_lock);
// printk(KERN_INFO "%s: attached %s driver.\n", drive->name, driver->name);
if ((drive->autotune == IDE_TUNE_DEFAULT) ||

2003-06-02 05:53:26

by William Lee Irwin III

[permalink] [raw]
Subject: Re: 2.5.70-bk[56] breaks disk partitioning with multiple IDE disks

On Sun, Jun 01, 2003 at 05:45:09PM -0700, Adam J. Richter wrote:
> @@ -1436,6 +1437,9 @@
> spin_unlock(&drivers_lock);
> if(idedefault_driver.attach(drive) != 0)
> panic("ide: default attach failed");
> + spin_lock(&drives_lock);
> + list_add_tail(&drive->list, &ata_unused);
> + spin_unlock(&drives_lock);
> return 1;
> }

This looks dubious; check idedefault_driver.attach(). It was just put on
another list. I don't know what you're trying to do but list_move_tail()
would at least not be as oopsable.


-- wli