ide-cs stack_dump

DISCUSSION Jani Monoses 3 participants 4 messages Aug 4, 2003 – Aug 5, 2003

Share & tools

Jani Monoses

author

2003-08-04 14:34 UTC #0 lore raw

Hi
as reported by someone earlier this year there's a long stack_dump starting from kobject_register failed with -17 (EEXISTS) when ide-cs detects a CF card.
The reason is as I see it that both rescan_partitions and register_disk are called, both of which in turn call add_partition for all partitions on the CF card. add_partition calls kobject_register. Also devfs_mk_bdev is called twice but it only prints an error msg 'could not append...'. I don't know if that is how things should be called or whether kobjects for IDE are broken as Alan responded to that earlier post but apart from this initial stack_dump the card works fine (not eject of course) So is kobject_register to verbose or calling code should make sure it does not attempt to register the same object multiple times?

2.6.0-test2

Russell King

2003-08-04 14:47 UTC #1 lore raw

Quoted 5 lines · Jani Monoses

Hi
as reported by someone earlier this year there's a long stack_dump starting from kobject_register failed with -17 (EEXISTS) when ide-cs detects a CF card.
The reason is as I see it that both rescan_partitions and register_disk

Quoted 8 lines

are called, both of which in turn call add_partition for all partitions on the CF card. add_partition calls kobject_register. Also devfs_mk_bdev is called twice but it only prints an error msg 'could not append...'. I don't know if that is how things should be called or whether kobjects for IDE are broken as Alan responded to that earlier post but apart from this initial stack_dump the card works fine (not eject of course) So is kobject_register to verbose or calling code should make sure it does not attempt to register the same object multiple times?

You can't kobject_register the same object multiple times.

What you're describing above sounds to me like an IDE problem. It might be worth discussing this with Bart. (cc'd)

Signature · 2 lines

Russell King ([email protected])                The developer of ARM Linux
             http://www.arm.linux.org.uk/personal/aboutme.html

Bartlomiej Zolnierkiewicz

2003-08-04 23:56 UTC #2 lore raw

In reply to Jani Monoses

Quoted 5 lines · Jani Monoses

Hi
as reported by someone earlier this year there's a long stack_dump starting from kobject_register failed with -17 (EEXISTS) when ide-cs detects a CF card.
The reason is as I see it that both rescan_partitions and register_disk

I think I know how this happens:

register_disk()->blkdev_get()->do_open(), then disk->fops->open() (idedisk_open() for ide-disk) calls check_disk_change(). check_disk_change() calls disk->fops->media_changed(). idedisk_media_changes() returns drive->removable, so instead of returning from check_disk_change() block_device is invalidated
and bdev->bd_invalidated is set to 1. Later in do_open(), bdev->bd_invalidated flag is checked and since it is 1 rescan_partitions() is triggered. Thus partitions are checked and added twice:
in do_open()->rescan_partitions() and in register_disk().

[ The same applies to ide-floppy driver and probably some other drivers. ]

Ufff... I hope it is a correct description (I don't have hardware to reproduce the problem).

Easy way is to fix is to add drive->attach flag, set it in idedisk_attach() and check+clear in idedisk_media_changed(), but I don't like this solution (patch below, Jani, can you test it?).

Any other ideas?

Quoted 8 lines

are called, both of which in turn call add_partition for all partitions on the CF card. add_partition calls kobject_register. Also devfs_mk_bdev is called twice but it only prints an error msg 'could not append...'. I don't know if that is how things should be called or whether kobjects for IDE are broken as Alan responded to that earlier post but apart from this initial stack_dump the card works fine (not eject of course) So is kobject_register to verbose or calling code should make sure it does not attempt to register the same object multiple times?

It is a bug to try to register some objects multiple times, not a feature :-).

Quoted 1 line

2.6.0-test2

Thanks,
--
Bartlomiej

 drivers/ide/ide-disk.c   |    7 +++++++
 drivers/ide/ide-floppy.c |    8 +++++++-
 include/linux/ide.h      |    1 +
 3 files changed, 15 insertions(+), 1 deletion(-)

drivers/ide/ide-disk.c renamed from drivers/ide/ide-disk.c~ide-attach-flag +7 -0

diff -puN drivers/ide/ide-disk.c~ide-attach-flag drivers/ide/ide-disk.c
@@ -1790,6 +1790,12 @@ static int idedisk_ioctl(struct inode *i
 static int idedisk_media_changed(struct gendisk *disk)
 {
 	ide_drive_t *drive = disk->private_data;
+
+	/* do not scan partitions twice if we are attaching this device */
+	if (drive->attach) {
+		drive->attach = 0;
+		return 0;
+	}
 	/* if removable, always assume it was changed */
 	return drive->removable;
 }

@@ -1848,6 +1854,7 @@ static int idedisk_attach(ide_drive_t *d
 	g->flags = drive->removable ? GENHD_FL_REMOVABLE : 0;
 	set_capacity(g, current_capacity(drive));
 	g->fops = &idedisk_ops;
+	drive->attach = 1;
 	add_disk(g);
 	return 0;
 failed:

drivers/ide/ide-floppy.c renamed from drivers/ide/ide-floppy.c~ide-attach-flag +7 -1

diff -puN drivers/ide/ide-floppy.c~ide-attach-flag drivers/ide/ide-floppy.c
@@ -2006,7 +2006,12 @@ static int idefloppy_media_changed(struc
 {
 	ide_drive_t *drive = disk->private_data;
 	idefloppy_floppy_t *floppy = drive->driver_data;
-
+
+	/* do not scan partitions twice if we are attaching this device */
+	if (drive->attach) {
+		drive->attach = 0;
+		return 0;
+	}
 	return test_and_clear_bit(IDEFLOPPY_MEDIA_CHANGED, &floppy->flags);
 }

@@ -2061,6 +2066,7 @@ static int idefloppy_attach (ide_drive_t
 	strcpy(g->devfs_name, drive->devfs_name);
 	g->flags = drive->removable ? GENHD_FL_REMOVABLE : 0;
 	g->fops = &idefloppy_ops;
+	drive->attach = 1;
 	add_disk(g);
 	return 0;
 failed:

include/linux/ide.h renamed from include/linux/ide.h~ide-attach-flag +1 -0

diff -puN include/linux/ide.h~ide-attach-flag include/linux/ide.h
@@ -711,6 +711,7 @@ typedef struct ide_drive_s {
 	unsigned id_read	: 1;	/* 1=id read from disk 0 = synthetic */
 	unsigned noprobe 	: 1;	/* from:  hdx=noprobe */
 	unsigned removable	: 1;	/* 1 if need to do check_media_change */
+	unsigned attach		: 1;	/* set to 1 in ->attach() */
 	unsigned is_flash	: 1;	/* 1 if probed as flash */
 	unsigned forced_geom	: 1;	/* 1 if hdx=c,h,s was given at boot */
 	unsigned no_unmask	: 1;	/* disallow setting unmask bit */

Jani Monoses

author

2003-08-05 06:30 UTC #3 lore raw

Quoted 15 lines · On Tue, 5 Aug 2003 01:56:01 +0200 (MET DST) Bartlomiej Zolnierkiewicz…

On Mon, 4 Aug 2003, Jani Monoses wrote:
I think I know how this happens:
register_disk()->blkdev_get()->do_open(), then disk->fops->open() (idedisk_open() for ide-disk) calls check_disk_change(). check_disk_change() calls disk->fops->media_changed(). idedisk_media_changes() returns drive->removable, so instead of returning from check_disk_change() block_device is invalidated and bdev->bd_invalidated is set to 1. Later in do_open(), bdev->bd_invalidated flag is checked and since it is 1 rescan_partitions() is triggered. Thus partitions are checked and added twice: in do_open()->rescan_partitions() and in register_disk().

Quoted 5 lines

[ The same applies to ide-floppy driver and probably some other drivers. ]
Ufff... I hope it is a correct description (I don't have hardware to reproduce the problem).

You're probably right in the description. One ugly way I solved this for 2.6.0-test1 was setting bd_invalidated to 0

Quoted 3 lines

Easy way is to fix is to add drive->attach flag, set it in idedisk_attach() and check+clear in idedisk_media_changed(), but I don't like this solution (patch below, Jani, can you test it?).

I can test is by tomorrow, it's another box that has the PCCard slot.

Jani