linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: [RFC/BUG?] ide_cs's removable status
  2005-09-22  0:18         ` Alan Cox
@ 2002-01-01  5:18           ` Pavel Machek
  0 siblings, 0 replies; 18+ messages in thread
From: Pavel Machek @ 2002-01-01  5:18 UTC (permalink / raw)
  To: Alan Cox
  Cc: Richard Purdie, Mark Lord, LKML, Dominik Brodowski, bzolnier, linux-ide

Hi!


> > CF slots have card detection and ide-cs will see a card removal event.
> > Have a look at ide_event() in ide-cs.c: CS_EVENT_CARD_INSERTION and
> > CS_EVENT_CARD_REMOVAL are what they say...
> 
> Not implemented on large numbers of adapters. Been there tried that,
> been 2.4 IDE maintainer.

How is CF flash card different from CF microdrive? AFAIK they are
not, so we may need to detect IBM microdrives as "is_flash"...

-- 
64 bytes from 195.113.31.123: icmp_seq=28 ttl=51 time=448769.1 ms         


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [RFC/BUG?] ide_cs's removable status
@ 2005-09-21 16:15 Richard Purdie
  2005-09-21 16:57 ` Alan Cox
  0 siblings, 1 reply; 18+ messages in thread
From: Richard Purdie @ 2005-09-21 16:15 UTC (permalink / raw)
  To: LKML; +Cc: Dominik Brodowski, bzolnier, linux-ide

I have a handheld system that takes compact flash memory cards and uses
ide_cs to interface to them. I've been trying to kill off a load of
hotplug scripts and switch to using udev. As HAL is too bloated for the
handheld, I hacked some evil auto-mount udev scripts together. When I
insert a memory card, I see the disk get repeatedly mounted and
unmounted in a loop.

I realise auto-mounting via udev is evil but I think this highlights a
kernel driver issue.

I'm seeing the following sequence of events:

1. I insert the card
2. The partitions are scanned and block devices are created
3. udev creates the device nodes
4. udev triggers my mount script which mounts the device
5. mounting the device triggers check_disk_change() in block_dev.c
6. The bd_invalidated flag is set
7. rescan_partitions is hence called
8. the block devices are destroyed
9. goto 2

As ide_cs only creates the block devices when a card is present, I think
it shouldn't be set as removable. As a point of reference, the MMC
system does not set the removable flag for exactly this reason (There is
an email from Russell King explaining this -
http://lkml.org/lkml/2005/1/8/165).

It is worth noting the MMC subsystem works with my evil udev script. If
I apply the patch below (which removes the removable flag for flash
devices), I don't see this loop.

With the patch below, the is_flash variable can actually be removed as
its only useful purpose is in ide-disk.c in a "if (drive->removable
&& !(drive->is_flash)) {" statement...

I'm wondering if one of the more experienced IDE developers could give
some insight into which drivers use bits of code at the end of
do_identify() in ide-probe.c. Some questions:

1. Can anyone provide details on what the bits in id->config really
mean?  
2. Which other drivers exploit the "if (id->config & (1<<7))
drive->removable = 1;" code? Is it just ide_cs?
3. Do any other drivers expose flash interfaces in the same way ide_cs
does but with "proper" removable media handling where the block device
is persistent?

If ide_cs is the only user of this code, there is scope for a reasonable
amount of cleanup here. Other than the ATAPI drivers, I can't see any
with special removable handling which leads me to believe none of this
code is required.

Comments welcome as I can't help feeling there's something I've missed.
I'll write a neater patch if there is some agreement on what needs to be
done.

Richard


Index: linux-2.6.13/drivers/ide/ide-probe.c
===================================================================
--- linux-2.6.13.orig/drivers/ide/ide-probe.c	2005-08-29 00:41:01.000000000 +0100
+++ linux-2.6.13/drivers/ide/ide-probe.c	2005-09-21 16:57:15.000000000 +0100
@@ -147,19 +147,19 @@
 {
 	struct hd_driveid *id = drive->id;
 
-	if (drive->removable) {
-		if (id->config == 0x848a) return 1;	/* CompactFlash */
-		if (!strncmp(id->model, "KODAK ATA_FLASH", 15)	/* Kodak */
-		 || !strncmp(id->model, "Hitachi CV", 10)	/* Hitachi */
-		 || !strncmp(id->model, "SunDisk SDCFB", 13)	/* old SanDisk */
-		 || !strncmp(id->model, "SanDisk SDCFB", 13)	/* SanDisk */
-		 || !strncmp(id->model, "HAGIWARA HPC", 12)	/* Hagiwara */
-		 || !strncmp(id->model, "LEXAR ATA_FLASH", 15)	/* Lexar */
-		 || !strncmp(id->model, "ATA_FLASH", 9))	/* Simple Tech */
-		{
-			return 1;	/* yes, it is a flash memory card */
-		}
+	if (id->config == 0x848a) return 1;	/* CompactFlash */
+
+	if (!strncmp(id->model, "KODAK ATA_FLASH", 15)	/* Kodak */
+	 || !strncmp(id->model, "Hitachi CV", 10)	/* Hitachi */
+	 || !strncmp(id->model, "SunDisk SDCFB", 13)	/* old SanDisk */
+	 || !strncmp(id->model, "SanDisk SDCFB", 13)	/* SanDisk */
+	 || !strncmp(id->model, "HAGIWARA HPC", 12)	/* Hagiwara */
+	 || !strncmp(id->model, "LEXAR ATA_FLASH", 15)	/* Lexar */
+	 || !strncmp(id->model, "ATA_FLASH", 9))	/* Simple Tech */
+	{
+		return 1;	/* yes, it is a flash memory card */
 	}
+
 	return 0;	/* no, it is not a flash memory card */
 }
 
@@ -278,11 +278,15 @@
 	/*
 	 * Not an ATAPI device: looks like a "regular" hard disk
 	 */
+
 	if (id->config & (1<<7))
 		drive->removable = 1;
 
-	if (drive_is_flashcard(drive))
+	if (drive_is_flashcard(drive)) {
 		drive->is_flash = 1;
+		drive->removable = 0;
+	}
+	
 	drive->media = ide_disk;
 	printk("%s DISK drive\n", (drive->is_flash) ? "CFA" : "ATA" );
 	QUIRK_LIST(drive);

	


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC/BUG?] ide_cs's removable status
  2005-09-21 16:15 [RFC/BUG?] ide_cs's removable status Richard Purdie
@ 2005-09-21 16:57 ` Alan Cox
  2005-09-21 17:21   ` Mark Lord
  0 siblings, 1 reply; 18+ messages in thread
From: Alan Cox @ 2005-09-21 16:57 UTC (permalink / raw)
  To: Richard Purdie; +Cc: LKML, Dominik Brodowski, bzolnier, linux-ide

On Mer, 2005-09-21 at 17:15 +0100, Richard Purdie wrote:
> As ide_cs only creates the block devices when a card is present, I think
> it shouldn't be set as removable. As a point of reference, the MMC
> system does not set the removable flag for exactly this reason (There is
> an email from Russell King explaining this -
> http://lkml.org/lkml/2005/1/8/165).

I can't comment on the MMC layer or its core requirements as I don't
know them well. IDE PCMCIA does however encompass removal devices. The
removable flag is set so that we get removable media behaviour - that is
the media can change under us and we must not cache partition data. The
current behavioiur in that sense is correct.

> 
> It is worth noting the MMC subsystem works with my evil udev script. If
> I apply the patch below (which removes the removable flag for flash
> devices), I don't see this loop.

But does MMC have a media change detect, and if not does the right thing
occur if you swap cards with partition tables ?

> 1. Can anyone provide details on what the bits in id->config really
> mean?  

ATA standards are all available for download.

> 2. Which other drivers exploit the "if (id->config & (1<<7))
> drive->removable = 1;" code? Is it just ide_cs?

It might be currently because the old IDE layer has no hotplug support
(not even for PCMCIA - it happens to work some days) but that wasn't
true in 2.4-ac or some 2.6-ac.

It sounds like something needs to be smarter about whether the media has
changed - could be kernel but it seems like a user space problem. I note
that the standard Gnome tools "just work" in this case so perhaps you
can see how they do it.

Alan


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC/BUG?] ide_cs's removable status
  2005-09-21 16:57 ` Alan Cox
@ 2005-09-21 17:21   ` Mark Lord
  2005-09-21 18:27     ` Alan Cox
  0 siblings, 1 reply; 18+ messages in thread
From: Mark Lord @ 2005-09-21 17:21 UTC (permalink / raw)
  To: Alan Cox; +Cc: Richard Purdie, LKML, Dominik Brodowski, bzolnier, linux-ide

Alan Cox wrote:
> 
> I can't comment on the MMC layer or its core requirements as I don't
> know them well. IDE PCMCIA does however encompass removal devices. The
> removable flag is set so that we get removable media behaviour - that is
> the media can change under us and we must not cache partition data. The
> current behavioiur in that sense is correct.

Mmm.. I'm not so sure about that.

In the case of CF cards in ide-cs, removing the card is equivalent
to removing the entire IDE controller, not just the media.

So "media change" is not what happens here.

But yes, it still should be managed as a removable device,
but we currently seem to be using this bit to mean two things,
as explained by Russell in the given link.

 > http://lkml.org/lkml/2005/1/8/165

Cheers

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC/BUG?] ide_cs's removable status
  2005-09-21 17:21   ` Mark Lord
@ 2005-09-21 18:27     ` Alan Cox
  2005-09-21 18:46       ` Richard Purdie
  2005-09-21 19:29       ` Russell King
  0 siblings, 2 replies; 18+ messages in thread
From: Alan Cox @ 2005-09-21 18:27 UTC (permalink / raw)
  To: Mark Lord; +Cc: Richard Purdie, LKML, Dominik Brodowski, bzolnier, linux-ide

On Mer, 2005-09-21 at 13:21 -0400, Mark Lord wrote:
> In the case of CF cards in ide-cs, removing the card is equivalent
> to removing the entire IDE controller, not just the media.

It isn't the same as removing the entire PCMCIA controller layer. As far
as PCMCIA is concerned there has been no change. Thus we have no media
change event and we need ->removable = 1

If the PCMCIA card disappeared each time it would be different


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC/BUG?] ide_cs's removable status
  2005-09-21 18:27     ` Alan Cox
@ 2005-09-21 18:46       ` Richard Purdie
  2005-09-22  0:18         ` Alan Cox
  2005-09-21 19:29       ` Russell King
  1 sibling, 1 reply; 18+ messages in thread
From: Richard Purdie @ 2005-09-21 18:46 UTC (permalink / raw)
  To: Alan Cox; +Cc: Mark Lord, LKML, Dominik Brodowski, bzolnier, linux-ide

On Wed, 2005-09-21 at 19:27 +0100, Alan Cox wrote:
> On Mer, 2005-09-21 at 13:21 -0400, Mark Lord wrote:
> > In the case of CF cards in ide-cs, removing the card is equivalent
> > to removing the entire IDE controller, not just the media.
> 
> It isn't the same as removing the entire PCMCIA controller layer. As far
> as PCMCIA is concerned there has been no change. Thus we have no media
> change event and we need ->removable = 1

CF slots have card detection and ide-cs will see a card removal event.
Have a look at ide_event() in ide-cs.c: CS_EVENT_CARD_INSERTION and
CS_EVENT_CARD_REMOVAL are what they say...

> If the PCMCIA card disappeared each time it would be different

Just the card disappears.

This along with your comments about the IDE layer having no hotplug
support suggest the code in question can be removed pending a better
replacement when hotplug is implemented.

Richard


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC/BUG?] ide_cs's removable status
  2005-09-21 18:27     ` Alan Cox
  2005-09-21 18:46       ` Richard Purdie
@ 2005-09-21 19:29       ` Russell King
  2005-09-21 21:52         ` Richard Purdie
  2005-09-22  0:10         ` Alan Cox
  1 sibling, 2 replies; 18+ messages in thread
From: Russell King @ 2005-09-21 19:29 UTC (permalink / raw)
  To: Alan Cox
  Cc: Mark Lord, Richard Purdie, LKML, Dominik Brodowski, bzolnier, linux-ide

On Wed, Sep 21, 2005 at 07:27:23PM +0100, Alan Cox wrote:
> On Mer, 2005-09-21 at 13:21 -0400, Mark Lord wrote:
> > In the case of CF cards in ide-cs, removing the card is equivalent
> > to removing the entire IDE controller, not just the media.
> 
> It isn't the same as removing the entire PCMCIA controller layer. As far
> as PCMCIA is concerned there has been no change. Thus we have no media
> change event and we need ->removable = 1
> 
> If the PCMCIA card disappeared each time it would be different

Last time I checked, with CF cards the media was an inherent part of
the CF card and is not changable without removing the card, opening
it, getting out the soldering iron... or alternatively plugging in
a different CF card.

Of course, PCMCIA will detect removal of the CF card provided the
PCMCIA hardware is working.  PCMCIA will also detect a CF card which
has been changed while the system has been suspended _provided_ the
CIS does not match the previous cards CIS.  It'll even do this if
you use cardctl suspend/cardctl resume.

However, if you suspend your system, remove your CF card, put it in
a different machine, use it (note: by doing this it could _already_
be in an inconsistent state), and put it back in the original machine
before resuming it, the cache on the original machine will disagree
with what is on the card.  But then you have done something silly
already by taking media in an inconsistent state to another machine
- and modified that inconsistent filesystem state.

It sounds like you know of a case where this isn't true - maybe a bug
report.  Can you expand on it?

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 Serial core

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC/BUG?] ide_cs's removable status
  2005-09-21 19:29       ` Russell King
@ 2005-09-21 21:52         ` Richard Purdie
  2005-09-22  0:06           ` Alan Cox
  2005-09-22  0:10         ` Alan Cox
  1 sibling, 1 reply; 18+ messages in thread
From: Richard Purdie @ 2005-09-21 21:52 UTC (permalink / raw)
  To: LKML
  Cc: linux-ide, bzolnier, Dominik Brodowski, Mark Lord, Alan Cox,
	Russell King

A proposed solution/patch:
[One step further is to totally remove the drive->removable = 1]

This patch stops CompactFlash devices being marked as removable. They
are not removable as the media and device are inseparable. When a card
is removed, the device is removed from the system.

Further, once this change is made, there is no need for ide to
differentiate between flash and other devices so the is_flash variable
can be removed from ide_drive_t.

Signed-Off-By: Richard Purdie <rpurdie@rpsys.net> 

Index: linux-2.6.13/drivers/ide/ide-probe.c
===================================================================
--- linux-2.6.13.orig/drivers/ide/ide-probe.c	2005-08-29 00:41:01.000000000 +0100
+++ linux-2.6.13/drivers/ide/ide-probe.c	2005-09-21 20:57:34.000000000 +0100
@@ -125,45 +125,6 @@
 }
 
 /**
- *	drive_is_flashcard	-	check for compact flash
- *	@drive: drive to check
- *
- *	CompactFlash cards and their brethern pretend to be removable
- *	hard disks, except:
- * 		(1) they never have a slave unit, and
- *		(2) they don't have doorlock mechanisms.
- *	This test catches them, and is invoked elsewhere when setting
- *	appropriate config bits.
- *
- *	FIXME: This treatment is probably applicable for *all* PCMCIA (PC CARD)
- *	devices, so in linux 2.3.x we should change this to just treat all
- *	PCMCIA  drives this way, and get rid of the model-name tests below
- *	(too big of an interface change for 2.4.x).
- *	At that time, we might also consider parameterizing the timeouts and
- *	retries, since these are MUCH faster than mechanical drives. -M.Lord
- */
- 
-static inline int drive_is_flashcard (ide_drive_t *drive)
-{
-	struct hd_driveid *id = drive->id;
-
-	if (drive->removable) {
-		if (id->config == 0x848a) return 1;	/* CompactFlash */
-		if (!strncmp(id->model, "KODAK ATA_FLASH", 15)	/* Kodak */
-		 || !strncmp(id->model, "Hitachi CV", 10)	/* Hitachi */
-		 || !strncmp(id->model, "SunDisk SDCFB", 13)	/* old SanDisk */
-		 || !strncmp(id->model, "SanDisk SDCFB", 13)	/* SanDisk */
-		 || !strncmp(id->model, "HAGIWARA HPC", 12)	/* Hagiwara */
-		 || !strncmp(id->model, "LEXAR ATA_FLASH", 15)	/* Lexar */
-		 || !strncmp(id->model, "ATA_FLASH", 9))	/* Simple Tech */
-		{
-			return 1;	/* yes, it is a flash memory card */
-		}
-	}
-	return 0;	/* no, it is not a flash memory card */
-}
-
-/**
  *	do_identify	-	identify a drive
  *	@drive: drive to identify 
  *	@cmd: command used
@@ -278,13 +239,17 @@
 	/*
 	 * Not an ATAPI device: looks like a "regular" hard disk
 	 */
-	if (id->config & (1<<7))
-		drive->removable = 1;
 
-	if (drive_is_flashcard(drive))
-		drive->is_flash = 1;
+	/* 
+	 * 0x848a = CompactFlash device 
+	 * These are *not* removable in Linux definition of the term
+	 */
+
+	if ((id->config != 0x848a) && (id->config & (1<<7)))
+		drive->removable = 1;
+
 	drive->media = ide_disk;
-	printk("%s DISK drive\n", (drive->is_flash) ? "CFA" : "ATA" );
+	printk("%s DISK drive\n", (id->config == 0x848a) ? "CFA" : "ATA" );
 	QUIRK_LIST(drive);
 	return;
 
Index: linux-2.6.13/drivers/ide/ide.c
===================================================================
--- linux-2.6.13.orig/drivers/ide/ide.c	2005-09-19 10:53:59.000000000 +0100
+++ linux-2.6.13/drivers/ide/ide.c	2005-09-21 20:52:53.000000000 +0100
@@ -242,7 +242,6 @@
 		drive->name[2]			= 'a' + (index * MAX_DRIVES) + unit;
 		drive->max_failures		= IDE_DEFAULT_MAX_FAILURES;
 		drive->using_dma		= 0;
-		drive->is_flash			= 0;
 		drive->vdma			= 0;
 		INIT_LIST_HEAD(&drive->list);
 		sema_init(&drive->gendev_rel_sem, 0);
Index: linux-2.6.13/drivers/ide/ide-disk.c
===================================================================
--- linux-2.6.13.orig/drivers/ide/ide-disk.c	2005-09-19 10:53:59.000000000 +0100
+++ linux-2.6.13/drivers/ide/ide-disk.c	2005-09-21 20:51:31.000000000 +0100
@@ -895,11 +895,7 @@
 	if (drive->id_read == 0)
 		return;
 
-	/*
-	 * CompactFlash cards and their brethern look just like hard drives
-	 * to us, but they are removable and don't have a doorlock mechanism.
-	 */
-	if (drive->removable && !(drive->is_flash)) {
+	if (drive->removable) {
 		/*
 		 * Removable disks (eg. SYQUEST); ignore 'WD' drives 
 		 */
Index: linux-2.6.13/include/linux/ide.h
===================================================================
--- linux-2.6.13.orig/include/linux/ide.h	2005-08-29 00:41:01.000000000 +0100
+++ linux-2.6.13/include/linux/ide.h	2005-09-21 20:56:29.000000000 +0100
@@ -697,7 +697,6 @@
 	unsigned noprobe 	: 1;	/* from:  hdx=noprobe */
 	unsigned removable	: 1;	/* 1 if need to do check_media_change */
 	unsigned attach		: 1;	/* needed for removable devices */
-	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 */
 	unsigned no_io_32bit	: 1;	/* disallow enabling 32bit I/O */


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC/BUG?] ide_cs's removable status
  2005-09-21 21:52         ` Richard Purdie
@ 2005-09-22  0:06           ` Alan Cox
  0 siblings, 0 replies; 18+ messages in thread
From: Alan Cox @ 2005-09-22  0:06 UTC (permalink / raw)
  To: Richard Purdie
  Cc: LKML, linux-ide, bzolnier, Dominik Brodowski, Mark Lord, Russell King

On Mer, 2005-09-21 at 22:52 +0100, Richard Purdie wrote:
> A proposed solution/patch:
> [One step further is to totally remove the drive->removable = 1]
> 
> This patch stops CompactFlash devices being marked as removable. They
> are not removable as the media and device are inseparable. When a card
> is removed, the device is removed from the system.


No it is not. The PCMCIA adapter does not generate a hot plug event when
the card is changed. This proposal is still wrong. Please either fix
your user space so it works (like the GNOME one does out of the box) or
propose a sensible kernel change to suppress the hotplug events such as
serial number checking. The latter sounds sensible to me because it
would cleanup somewhat.

Simply posting new variants of the wrong patch repeating false claims is
not a way to fix bugs.

Alan


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC/BUG?] ide_cs's removable status
  2005-09-21 19:29       ` Russell King
  2005-09-21 21:52         ` Richard Purdie
@ 2005-09-22  0:10         ` Alan Cox
  2005-09-22 10:22           ` Russell King
  1 sibling, 1 reply; 18+ messages in thread
From: Alan Cox @ 2005-09-22  0:10 UTC (permalink / raw)
  To: Russell King
  Cc: Mark Lord, Richard Purdie, LKML, Dominik Brodowski, bzolnier, linux-ide

On Mer, 2005-09-21 at 20:29 +0100, Russell King wrote:
> Last time I checked, with CF cards the media was an inherent part of
> the CF card and is not changable without removing the card, opening
> it, getting out the soldering iron... or alternatively plugging in
> a different CF card.

Last time I checked the spinning platter on my hard disk wasn't
removable without an axe either. What is your point ?

> Of course, PCMCIA will detect removal of the CF card provided the
> PCMCIA hardware is working.  PCMCIA will also detect a CF card which
> has been changed while the system has been suspended _provided_ the
> CIS does not match the previous cards CIS.  It'll even do this if
> you use cardctl suspend/cardctl resume.

Most adapters do not do this. It works solely because we set
drive->removable so we force a new partition scan.

> It sounds like you know of a case where this isn't true - maybe a bug
> report.  Can you expand on it?

With drive->removable = 0 if I insert a card I get partition tables, it
will then not rescan that in future even if the card changed, because
there is no "media change detect" line, unlike on a floppy.

If I pull the CF adapter out it is fine because you get pcmcia level
hotplug but that is not neccessary for card changing on better designed
adapters or when the CF adapter is on the board itself with a CF slot
exposed to the user.

SCSI also treats CF cards as removable for the same reason.


Alan


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC/BUG?] ide_cs's removable status
  2005-09-21 18:46       ` Richard Purdie
@ 2005-09-22  0:18         ` Alan Cox
  2002-01-01  5:18           ` Pavel Machek
  0 siblings, 1 reply; 18+ messages in thread
From: Alan Cox @ 2005-09-22  0:18 UTC (permalink / raw)
  To: Richard Purdie; +Cc: Mark Lord, LKML, Dominik Brodowski, bzolnier, linux-ide

On Mer, 2005-09-21 at 19:46 +0100, Richard Purdie wrote:
> CF slots have card detection and ide-cs will see a card removal event.
> Have a look at ide_event() in ide-cs.c: CS_EVENT_CARD_INSERTION and
> CS_EVENT_CARD_REMOVAL are what they say...

Not implemented on large numbers of adapters. Been there tried that,
been 2.4 IDE maintainer.

> This along with your comments about the IDE layer having no hotplug
> support suggest the code in question can be removed pending a better
> replacement when hotplug is implemented.

That attitude is why there is not hotplug code. Its the "delete anything
that gets in my way" approach. Its why 2.4 is the last kernel thats
useful on a thinkpad with removable drive bay.

The kernel isn't perfect so lets delete it and use DOS. Same argument,
same logical fallacy.

Also every other hotplug environment gets it right today, so GNOME and
KDE are doing things differently and it works. You can therefore do the
same.

The right approach is to ask

1.	Do you need to fix it, everyone elses userspace works maybe your user
space is inadequate and you need to use GNOME or KDE userspace
implementations (or steal the relevant algorithms)

2.	If it does need fixing (which I think you have a case for) then how
do I fix it properly

IDE CF cards have serial numbers. I believe the CF standard requires
they all do but I need to go re-read that to check. If so then you don't
want to remove the correct drive->removable handling (and break cache
flush etc too) but make the hotplug handler smarter. 

What does ->removable mean, "can go away without warning and detection".
That is true in this case. The problem you have is that the correctly
performed partition rescan generates events and you respond to them in a
way that gets you looping. That means either kernel or user space should
be asking "is this event for the same disk" and either not generating it
(because its not a change) or ignoring it in the user case.

But first of all you need to explain why GNOME and KDE work and your
code doesn't.

Alan


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC/BUG?] ide_cs's removable status
  2005-09-22  0:10         ` Alan Cox
@ 2005-09-22 10:22           ` Russell King
  2005-09-22 13:39             ` Alan Cox
  0 siblings, 1 reply; 18+ messages in thread
From: Russell King @ 2005-09-22 10:22 UTC (permalink / raw)
  To: Alan Cox
  Cc: Mark Lord, Richard Purdie, LKML, Dominik Brodowski, bzolnier, linux-ide

On Thu, Sep 22, 2005 at 01:10:44AM +0100, Alan Cox wrote:
> With drive->removable = 0 if I insert a card I get partition tables, it
> will then not rescan that in future even if the card changed, because
> there is no "media change detect" line, unlike on a floppy.
> 
> If I pull the CF adapter out it is fine because you get pcmcia level
> hotplug but that is not neccessary for card changing on better designed
> adapters or when the CF adapter is on the board itself with a CF slot
> exposed to the user.

Interesting - all my CF adapters (and I have several, some cheapo
noname things to some branded ones) are dumb pieces of hardware -
they merely convert the PCMCIA connector to a CF connector, just as
dumb as those 240V mains adapters.

Also, "CF" is just a different form factor of PCMCIA - don't get
mislead by the term "Compact Flash" - you can get "CF" network
cards, serial cards, bluetooth cards, etc as well.  They're exactly
the same as PCMCIA network, serial, bluetooth cards, just in a
smaller package.

If you have a CF adapter which behaves as you describe above, could
you please check what happens as far as PCMCIA goes when you unplug
the CF card - particularly what happens to cardctl status / cardctl
ident ?

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 Serial core

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC/BUG?] ide_cs's removable status
  2005-09-22 13:39             ` Alan Cox
@ 2005-09-22 13:29               ` Russell King
  2005-09-22 14:41                 ` Alan Cox
  2005-09-22 14:21               ` Richard Purdie
  1 sibling, 1 reply; 18+ messages in thread
From: Russell King @ 2005-09-22 13:29 UTC (permalink / raw)
  To: Alan Cox
  Cc: Mark Lord, Richard Purdie, LKML, Dominik Brodowski, bzolnier, linux-ide

On Thu, Sep 22, 2005 at 02:39:42PM +0100, Alan Cox wrote:
> On Iau, 2005-09-22 at 11:22 +0100, Russell King wrote:
> > If you have a CF adapter which behaves as you describe above, could
> > you please check what happens as far as PCMCIA goes when you unplug
> > the CF card - particularly what happens to cardctl status / cardctl
> > ident ?
> 
> If I remove the CF card I get garbage reported. I don't however get a
> card plug/unplug event.

That's interesting, because PCMCIA caches the CIS data and only
invalidates the CIS cache when an unplug event is seen.  I assume
the whole lot isn't garbage, just some parts of it?

> On the other card I have I get a card plug/unplug event.

Using the same CF card with these two adapters, do you get differing
CIS data?  Can you post the entire CIS data from both using dump_cis
please?

I'm wondering if your first adapter is somehow "inteligent" and isn't
operating the CF card in "PCMCIA" mode.  If this is the case, PCMCIA
should be more inteligent about it than it currently is - telling IDE
that the media is replaceable when the interface is registered.


There is another concern I have in all this, one which seems to have
been completely missed.  Yes we do this partition rescanning each time
a "removable" IDE device is opened, but do we re-read the identity?
I'm asking this because what happens if you replace a 64MB CF card
with a 256MB CF card in your first adapter?  Do we assume that it's
still a 64MB CF card with weird partitions?

> The pcmcia ide floppy (40MB clik! drive if anyone
> wants to play) I have always shows up as present. It triggers the same
> hotplug behaviour being complained about as far as I can see and
> correctly so.

Not having a clik drive, but "ide floppy" sounds like standard floppy
behaviour - the "other" class of hotplug where the media is replaceable
independent of the controller itself, so this is a slightly different
problem, and one which we handle correctly.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 Serial core

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC/BUG?] ide_cs's removable status
  2005-09-22 10:22           ` Russell King
@ 2005-09-22 13:39             ` Alan Cox
  2005-09-22 13:29               ` Russell King
  2005-09-22 14:21               ` Richard Purdie
  0 siblings, 2 replies; 18+ messages in thread
From: Alan Cox @ 2005-09-22 13:39 UTC (permalink / raw)
  To: Russell King
  Cc: Mark Lord, Richard Purdie, LKML, Dominik Brodowski, bzolnier, linux-ide

On Iau, 2005-09-22 at 11:22 +0100, Russell King wrote:
> If you have a CF adapter which behaves as you describe above, could
> you please check what happens as far as PCMCIA goes when you unplug
> the CF card - particularly what happens to cardctl status / cardctl
> ident ?

If I remove the CF card I get garbage reported. I don't however get a
card plug/unplug event. On the other card I have I get a card
plug/unplug event. The pcmcia ide floppy (40MB clik! drive if anyone
wants to play) I have always shows up as present. It triggers the same
hotplug behaviour being complained about as far as I can see and
correctly so.

I dug out some old emails - GNOME does indeed handle this correctly
using HAL and the recursive probing problem is true for all removable
media types. HAL handles this correctly although David Zeuthen wasn't
exactly happy with the kernel behaviour.

So I'm definitely against removing drive->removable when it should be
set, although its less serious than 2.4 because we do more flushing
anyway. There do seem to be a lot of people who would rather it didnt
always generate hotplug events when rescanning partitions. Perhaps we
need a ->same_media() check if the partitions match the old ones ?

Alan


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC/BUG?] ide_cs's removable status
  2005-09-22 13:39             ` Alan Cox
  2005-09-22 13:29               ` Russell King
@ 2005-09-22 14:21               ` Richard Purdie
  2005-09-22 14:36                 ` Russell King
  2005-09-22 15:08                 ` Alan Cox
  1 sibling, 2 replies; 18+ messages in thread
From: Richard Purdie @ 2005-09-22 14:21 UTC (permalink / raw)
  To: Alan Cox
  Cc: Russell King, Mark Lord, LKML, Dominik Brodowski, bzolnier, linux-ide

Lets try a different way of illustrating the problem. We'll hopefully
all agree that my ide CDROM drive is a removable device.

When my system boots, it creates a /dev/hdd entry for the cdrom. When I
insert a disk and mount it, udev doesn't touch the device node as its
already there and ready to go. If the media changes, this is detected
and the same device node is used to present the new data.

/dev/hdd *always* exists and may or may not have media present.

When I boot a system with an empty PCMCIA slot, no ide device exists.
ide-cs knows nothing about the PCMCIA slot and therefore no device nodes
exist. When you insert the CF card, PCMCIA realises its an ide device,
tells ide-cs which creates the appropriate devices through the ide
layer. When the card is removed, the devices and controller are removed.

The /dev/* entries only exist when the ide controller is present. There
is no state where the controller is present with no media.

Under Linux, removable is defined to mean the device can exist with no
media. That is clearly not the case for CF and therefore it shouldn't be
marked as such. 

If it were removable, ide-cs should be informed of every card slot in
the system and should create ide controllers for each of them whether
they have cards in or not. We could then correctly check for media
changes. Yes, this would be ridiculous but it illustrates that CF cards
should not have the removable flag set.


There are other bugs which this issue is getting confused with. In
summary, I see the following separate issues:

1. Are ide-cs devices removable or not. See above.
2. The following loop exists for removable devices:

Device node is created (by udev).
An attempt to mount the device results in a media change check.
The media change check defaults to media changed.
The device node is removed and recreated (by udev).

Technically, the device node you mounted is now invalid and has changed
so you should remount it. You then have a loop.

This can be solved by having the media change check look at serial
numbers as mentioned by Alan in a previous email.

3. ide-cs sometimes can't/doesn't detect the removal of an ide
controller.

Using media_change events to see if the controller has changed/been
unplugged is incorrect. 


Each of the above are separate bugs. I'm trying to fix one of them (and
might try and work on the others if I can).

Richard



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC/BUG?] ide_cs's removable status
  2005-09-22 14:21               ` Richard Purdie
@ 2005-09-22 14:36                 ` Russell King
  2005-09-22 15:08                 ` Alan Cox
  1 sibling, 0 replies; 18+ messages in thread
From: Russell King @ 2005-09-22 14:36 UTC (permalink / raw)
  To: Richard Purdie
  Cc: Alan Cox, Mark Lord, LKML, Dominik Brodowski, bzolnier, linux-ide

On Thu, Sep 22, 2005 at 03:21:16PM +0100, Richard Purdie wrote:
> 3. ide-cs sometimes can't/doesn't detect the removal of an ide
> controller.

This is the one I'm particularly interested in, and I think it's
responsible for a lot of problems in this area.

"can't" is correct - from what I now understand from Alan, it seems
that there are PCMCIA to CF adapters out there which tie the PCMCIA
card detect lines to ground, rather than passing them through to the
CF socket.

This means that if you leave the PCMCIA to CF adapter in the slot,
pull out the CF card, replace it with another CF card, PCMCIA will
not notice the change.

You could even plug this adapter into the slot with a CF IDE card in,
unplug the CF IDE card and replace it with a CF network card.  Then
watch the fun and games when IDE tries to access the CF network card!

Therefore, it's completely unsafe to assume anything about what's
plugged in with such a broken adapter... the only way you could be
sure is to regularly check the CIS matches the PCMCIA cached version,
provided you have enough of the CIS cached.

I think this basically comes down to a buggy CF adapter which needs
bining.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 Serial core

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC/BUG?] ide_cs's removable status
  2005-09-22 13:29               ` Russell King
@ 2005-09-22 14:41                 ` Alan Cox
  0 siblings, 0 replies; 18+ messages in thread
From: Alan Cox @ 2005-09-22 14:41 UTC (permalink / raw)
  To: Russell King
  Cc: Mark Lord, Richard Purdie, LKML, Dominik Brodowski, bzolnier, linux-ide

On Iau, 2005-09-22 at 14:29 +0100, Russell King wrote:
> the whole lot isn't garbage, just some parts of it?

Bits.

> I'm wondering if your first adapter is somehow "inteligent" and isn't
> operating the CF card in "PCMCIA" mode.  If this is the case, PCMCIA
> should be more inteligent about it than it currently is - telling IDE
> that the media is replaceable when the interface is registered.

It isn't being intelligent, its being dumb. I've not seen a smart
PCMCIA<->CF convertor. USB ones yes but not PCMCIA.

> There is another concern I have in all this, one which seems to have
> been completely missed.  Yes we do this partition rescanning each time
> a "removable" IDE device is opened, but do we re-read the identity?

Don't think so - and just tested - we don't. Its totally [select an
expletive]ed if I do that. the /proc/ide identify data and the dumped
ident data directly from ide are different.

> > The pcmcia ide floppy (40MB clik! drive if anyone
> > wants to play) I have always shows up as present. It triggers the same
> > hotplug behaviour being complained about as far as I can see and
> > correctly so.
> 
> Not having a clik drive, but "ide floppy" sounds like standard floppy
> behaviour - the "other" class of hotplug where the media is replaceable
> independent of the controller itself, so this is a slightly different
> problem, and one which we handle correctly.

We generate hot plug events when scanning the partition tables each
time. So you'd get the same loops - or am I misunderstanding something
here about what you mean by "correctly" ?

So in fact its not a case of ->removable but that IDE wants to seperate
->removable and ->mightgoaway. That lets us fix the broken cache flush
behaviour with modern media monitoring tools.

ie change idedisk_release to do

    if(drive->mightgoaway||drive->usage==1) ide_cacheflush_p(drive)

then
    if(drive->doorlocking && drive->usage == 1)

Syquests (real removable IDE disk) will have drive->doorlocking = 1 set
PCMCIA will have drive->mightgoaway set so will flush

(and M/O disks are ide-scsi or ide-cd so not affected)

The first change is needed because currently several control apps may
hold the disk open for media change detection, SMART and the like and if
they do so the user perceived "close" - end of a raw I/O access or
umount return will not media flush.

Alan


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC/BUG?] ide_cs's removable status
  2005-09-22 14:21               ` Richard Purdie
  2005-09-22 14:36                 ` Russell King
@ 2005-09-22 15:08                 ` Alan Cox
  1 sibling, 0 replies; 18+ messages in thread
From: Alan Cox @ 2005-09-22 15:08 UTC (permalink / raw)
  To: Richard Purdie
  Cc: Russell King, Mark Lord, LKML, Dominik Brodowski, bzolnier, linux-ide

On Iau, 2005-09-22 at 15:21 +0100, Richard Purdie wrote:
> 1. Are ide-cs devices removable or not. See above.

Having done testing on the cards I have based on RMK's suggestion I
agree they are not removable except for specific cases (IDE PCMCIA cable
adapter plugged into a Syquest). That case is already handled in the
core code.

The fact cache flushing is all odd now is I guess bug 4. on the list but
easy to fix while fixing 1

Alan


^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2005-09-24  9:57 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2005-09-21 16:15 [RFC/BUG?] ide_cs's removable status Richard Purdie
2005-09-21 16:57 ` Alan Cox
2005-09-21 17:21   ` Mark Lord
2005-09-21 18:27     ` Alan Cox
2005-09-21 18:46       ` Richard Purdie
2005-09-22  0:18         ` Alan Cox
2002-01-01  5:18           ` Pavel Machek
2005-09-21 19:29       ` Russell King
2005-09-21 21:52         ` Richard Purdie
2005-09-22  0:06           ` Alan Cox
2005-09-22  0:10         ` Alan Cox
2005-09-22 10:22           ` Russell King
2005-09-22 13:39             ` Alan Cox
2005-09-22 13:29               ` Russell King
2005-09-22 14:41                 ` Alan Cox
2005-09-22 14:21               ` Richard Purdie
2005-09-22 14:36                 ` Russell King
2005-09-22 15:08                 ` Alan Cox

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).