[PATCH] ide.c 'clear' fix, ide=nodma documentation

[PATCH] ide.c 'clear' fix, ide=nodma documentation

[David Fries]

From: David Fries <david@fries.net>

Documentation/kernel-parameters.txt
ide=nodma is no longer valid

drivers/ide/Kconfig
the module is ide-core.ko not ide

drivers/ide/ide.c
It took me a while to figure out what the arguments %d.%d:%d to nodma
module parameter ment, so I added a comment to each.
Added a comment to each of the sscanf lines to
There is a bug, if j is 0 it would previously clear all the other bits
except the current device, changed in three different places.
mask &= (1 << i)     should be     mask &= ~(1 << i)


Signed-off-by: David Fries <david@fries.net>
Cc: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>

---
I pretty much wasted my Saturday on this.  This is an older VIA
vt82c586b (rev 41) IDE UDMA33 and I've seen disk corruption in the
past with dma enabled, I can't use hdparm because those settings don't
persist across hibernate and restore (that should be another bug
report).  ide=nodma was working on the last 2.6.24 kernel, and
kernel-parameters.txt still lists it.  I had to go through the change
logs to figure out what happened to ide=nodma, then ide.c to figure
out what replaced it and what %d.%d:%d means (so I documented it so
other would have an easier time), then figure out how to set a module
parameter on the kernel command line when Kconfig said the module name
was ide and ide.nodma=0.0 didn't work, it was ide-core.nodma=0.0.

Now instead of,
ide=nodma
I have the following.
ide-core.nodma=0.0 ide-core.nodma=0.1 ide-core.nodma=1.0 ide-core.nodma=1.1
Where else is hdc written 1.0 ?  Why doesn't it take ide-core.nodma=hdc ?

---
 Documentation/kernel-parameters.txt |    6 ++++--
 drivers/ide/Kconfig                 |    2 +-
 drivers/ide/ide.c                   |   11 ++++++++---
 3 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index d8362cf..1512f6d 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -868,8 +868,10 @@ and is between 256 and 4096 characters. It is defined in the file
 	icn=		[HW,ISDN]
 			Format: <io>[,<membase>[,<icn_id>[,<icn_id2>]]]
 
-	ide=		[HW] (E)IDE subsystem
-			Format: ide=nodma or ide=doubler
+	ide-core.nodma=	[HW] (E)IDE subsystem
+			Format: =0.0 to prevent dma on hda, =0.1 hdb =1.0 hdc
+			.vlb_clock .pci_clock .noflush .noprobe .nowerr .cdrom
+			.chs .ignore_cable are additional options
 			See Documentation/ide/ide.txt.
 
 	idebus=		[HW] (E)IDE subsystem - VLB/PCI bus speed
diff --git a/drivers/ide/Kconfig b/drivers/ide/Kconfig
index 3dad229..e072903 100644
--- a/drivers/ide/Kconfig
+++ b/drivers/ide/Kconfig
@@ -46,7 +46,7 @@ menuconfig IDE
 	  SMART parameters from disk drives.
 
 	  To compile this driver as a module, choose M here: the
-	  module will be called ide.
+	  module will be called ide-core.ko.
 
 	  For further information, please read <file:Documentation/ide/ide.txt>.
 
diff --git a/drivers/ide/ide.c b/drivers/ide/ide.c
index 258805d..7d04e22 100644
--- a/drivers/ide/ide.c
+++ b/drivers/ide/ide.c
@@ -337,6 +337,7 @@ static int ide_set_dev_param_mask(const char *s, struct kernel_param *kp)
 	int a, b, i, j = 1;
 	unsigned int *dev_param_mask = (unsigned int *)kp->arg;
 
+	/* controller . disk (0 or 1) [ : 1 (set) | 0 (clear) ] */
 	if (sscanf(s, "%d.%d:%d", &a, &b, &j) != 3 &&
 	    sscanf(s, "%d.%d", &a, &b) != 2)
 		return -EINVAL;
@@ -349,7 +350,7 @@ static int ide_set_dev_param_mask(const char *s, struct kernel_param *kp)
 	if (j)
 		*dev_param_mask |= (1 << i);
 	else
-		*dev_param_mask &= (1 << i);
+		*dev_param_mask &= ~(1 << i);
 
 	return 0;
 }
@@ -392,6 +393,8 @@ static int ide_set_disk_chs(const char *str, struct kernel_param *kp)
 {
 	int a, b, c = 0, h = 0, s = 0, i, j = 1;
 
+	/* controller . disk (0 or 1) : Cylinders , Heads , Sectors */
+	/* controller . disk (0 or 1) : 1 (use CHS) | 0 (ignore CHS) */
 	if (sscanf(str, "%d.%d:%d,%d,%d", &a, &b, &c, &h, &s) != 5 &&
 	    sscanf(str, "%d.%d:%d", &a, &b, &j) != 3)
 		return -EINVAL;
@@ -407,7 +410,7 @@ static int ide_set_disk_chs(const char *str, struct kernel_param *kp)
 	if (j)
 		ide_disks |= (1 << i);
 	else
-		ide_disks &= (1 << i);
+		ide_disks &= ~(1 << i);
 
 	ide_disks_chs[i].cyl  = c;
 	ide_disks_chs[i].head = h;
@@ -469,6 +472,8 @@ static int ide_set_ignore_cable(const char *s, struct kernel_param *kp)
 {
 	int i, j = 1;
 
+	/* controller (ignore) */
+	/* controller : 1 (ignore) | 0 (use) */
 	if (sscanf(s, "%d:%d", &i, &j) != 2 && sscanf(s, "%d", &i) != 1)
 		return -EINVAL;
 
@@ -478,7 +483,7 @@ static int ide_set_ignore_cable(const char *s, struct kernel_param *kp)
 	if (j)
 		ide_ignore_cable |= (1 << i);
 	else
-		ide_ignore_cable &= (1 << i);
+		ide_ignore_cable &= ~(1 << i);
 
 	return 0;
 }
-- 
1.5.6.5

Re: [PATCH] ide.c 'clear' fix, ide=nodma documentation

[Bartlomiej Zolnierkiewicz]

Hi,

On Sunday 08 February 2009, David Fries wrote:
> From: David Fries <david@fries.net>
> 
> Documentation/kernel-parameters.txt
> ide=nodma is no longer valid
> 
> drivers/ide/Kconfig
> the module is ide-core.ko not ide
> 
> drivers/ide/ide.c
> It took me a while to figure out what the arguments %d.%d:%d to nodma
> module parameter ment, so I added a comment to each.

This is documented in Documentation/ide/ide.txt however having
more documentation won't hurt.

> Added a comment to each of the sscanf lines to
> There is a bug, if j is 0 it would previously clear all the other bits
> except the current device, changed in three different places.
> mask &= (1 << i)     should be     mask &= ~(1 << i)

Good catch.

> Signed-off-by: David Fries <david@fries.net>
> Cc: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>

Applied, thanks!

> ---
> I pretty much wasted my Saturday on this.  This is an older VIA
> vt82c586b (rev 41) IDE UDMA33 and I've seen disk corruption in the
> past with dma enabled, I can't use hdparm because those settings don't
> persist across hibernate and restore (that should be another bug
> report).  ide=nodma was working on the last 2.6.24 kernel, and
> kernel-parameters.txt still lists it.  I had to go through the change
> logs to figure out what happened to ide=nodma, then ide.c to figure
> out what replaced it and what %d.%d:%d means (so I documented it so
> other would have an easier time), then figure out how to set a module
> parameter on the kernel command line when Kconfig said the module name
> was ide and ide.nodma=0.0 didn't work, it was ide-core.nodma=0.0.
> 
> Now instead of,
> ide=nodma
> I have the following.
> ide-core.nodma=0.0 ide-core.nodma=0.1 ide-core.nodma=1.0 ide-core.nodma=1.1

The important question is why do you have to use debug "=nodma" option?

Is the corruption present in any recent kernels?

What disk is it?  Do we need to add it to DMA blacklist?

I don't recall any reports about such issues and via82cxxx host driver,
have you tried to verify that this is not a faulty hardware?

> Where else is hdc written 1.0 ?  Why doesn't it take ide-core.nodma=hdc ?

- simpler code, without parsing of parameters

- no dependence on legacy /dev/hd* naming

- makes people think harder about reporting/investigating issues instead of
  covering them up with using debug kernel parameters ;-)

[...]

> --- a/drivers/ide/ide.c
> +++ b/drivers/ide/ide.c
> @@ -337,6 +337,7 @@ static int ide_set_dev_param_mask(const char *s, struct kernel_param *kp)
>  	int a, b, i, j = 1;
>  	unsigned int *dev_param_mask = (unsigned int *)kp->arg;
>  
> +	/* controller . disk (0 or 1) [ : 1 (set) | 0 (clear) ] */

I did s/disk/device/ here and in other places while merging the patch.

Thanks,
Bart

Re: [PATCH] ide.c 'clear' fix, ide=nodma documentation

[David Fries]

On Mon, Feb 09, 2009 at 08:57:54PM +0100, Bartlomiej Zolnierkiewicz wrote:
> > I pretty much wasted my Saturday on this.  This is an older VIA
> > vt82c586b (rev 41) IDE UDMA33 and I've seen disk corruption in the
> > past with dma enabled, I can't use hdparm because those settings don't
> > persist across hibernate and restore (that should be another bug
> > report).  ide=nodma was working on the last 2.6.24 kernel, and
> > kernel-parameters.txt still lists it.  I had to go through the change
> > logs to figure out what happened to ide=nodma, then ide.c to figure
> > out what replaced it and what %d.%d:%d means (so I documented it so
> > other would have an easier time), then figure out how to set a module
> > parameter on the kernel command line when Kconfig said the module name
> > was ide and ide.nodma=0.0 didn't work, it was ide-core.nodma=0.0.
> > 
> > Now instead of,
> > ide=nodma
> > I have the following.
> > ide-core.nodma=0.0 ide-core.nodma=0.1 ide-core.nodma=1.0 ide-core.nodma=1.1
> 
> The important question is why do you have to use debug "=nodma" option?

> What disk is it?  Do we need to add it to DMA blacklist?

Model=Maxtor 6Y120P0, FwRev=YAR41BW0
Model=PLEXTOR CD-R PX-W4012A, FwRev=1.06

But I've had the same model in another computer without any issue and
each are on their own ribben cable.

> I don't recall any reports about such issues and via82cxxx host driver,
> have you tried to verify that this is not a faulty hardware?

I was expecting it to be faulty hardware, this isn't the only issue
with this FIC VA-503 motherboard.  I'm running one dimm because when I
had two memcheck passed everything but block moves and failed (alone
either dimm would pass).  I've been told the PCI controller doesn't
honor the don't do write combining which has taken driver work
arounds, I've had problems running a 3Com network card but had a hack
that would get it to work, another network card would randomly trash
memory and crash the computer, but infrequently enough that it took me
a while to finger it and switching to yet another card to solve it.

> Is the corruption present in any recent kernels?

I haven't tried.  At the time I had a formula for 'do this', and
sometimes it comes out corrupted.  The test case was reliable enough
that dma on corruption, off no corruption.  I looked and I couldn't
find it.  My time would be better spent replacing the system than
risking my filesystem and data in order to potentially get some
performance.  If I turn on dma and see a corruption I could positively
say it was still there, if I don't, well, I might have just not hit
the case.

-- 
David Fries <david@fries.net>
http://fries.net/~david/ (PGP encryption key available)