[PATCH] libata: Add a drivers/ide style DMA disable

[PATCH] libata: Add a drivers/ide style DMA disable

[Alan Cox]

This is useful when debugging, handling problem systems, or for
distributions just to get the system installed so it can be sorted
out later.

This is a bit smarter than the old IDE one and lets you do

libata.pata_dma=0		Disable all PATA DMA like old IDE
libata.pata_dma=1		Disk DMA only
libata.pata_dma=2		ATAPI DMA only
libata.pata_dma=4		CF DMA only

(or combinations thereof - 0,1,3 being the useful ones I suspect)

(I've split CF as it seems to be a seperate case of pain and suffering
different to the others and caused by assorted PIO wired adapters etc)

SATA is not affected - for one its not clear it makes sense to disable
DMA for SATA if even always possible, for two we've seen no failure 
evidence to justify needing to support this kind of hammer on SATA.

Signed-off-by: Alan Cox <alan@redhat.com>

diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.23rc3-mm1/include/linux/libata.h linux-2.6.23rc3-mm1/include/linux/libata.h
--- linux.vanilla-2.6.23rc3-mm1/include/linux/libata.h	2007-08-22 17:23:14.000000000 +0100
+++ linux-2.6.23rc3-mm1/include/linux/libata.h	2007-08-22 17:50:32.000000000 +0100
@@ -315,6 +315,12 @@
 	ATA_HORKAGE_NONCQ	= (1 << 2),	/* Don't use NCQ */
 	ATA_HORKAGE_MAX_SEC_128	= (1 << 3),	/* Limit max sects to 128 */
 	ATA_HORKAGE_HPA_SIZE	= (1 << 4), 	/* Reports native size off by one */
+ 
+	/* DMA mask for user DMA control: User visible values do not 
+	   renumber */
+	ATA_DMA_MASK_ATA	= (1 << 0),	/* DMA on ATA Disk */
+	ATA_DMA_MASK_ATAPI	= (1 << 1),	/* DMA on ATAPI */
+	ATA_DMA_MASK_CFA	= (1 << 2),	/* DMA on CF Card */
 };
 
 enum hsm_task_states {
diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.23rc3-mm1/drivers/ata/libata-core.c linux-2.6.23rc3-mm1/drivers/ata/libata-core.c
--- linux.vanilla-2.6.23rc3-mm1/drivers/ata/libata-core.c	2007-08-22 17:23:00.000000000 +0100
+++ linux-2.6.23rc3-mm1/drivers/ata/libata-core.c	2007-08-22 18:17:31.321738376 +0100
@@ -99,6 +99,10 @@
 module_param_named(ignore_hpa, ata_ignore_hpa, int, 0644);
 MODULE_PARM_DESC(ignore_hpa, "Ignore HPA limit (0=keep BIOS limits, 1=ignore limits, using full disk)");
 
+static int ata_pata_dma = ATA_DMA_MASK_ATA|ATA_DMA_MASK_ATAPI|ATA_DMA_MASK_CFA;
+module_param_named(pata_dma, ata_pata_dma, int, 0644);
+MODULE_PARM_DESC(pata_dma, "Use DMA on PATA devices");
+
 static int ata_probe_timeout = ATA_TMOUT_INTERNAL / HZ;
 module_param(ata_probe_timeout, int, 0444);
 MODULE_PARM_DESC(ata_probe_timeout, "Set ATA probing timeout (seconds)");
@@ -2839,14 +2854,27 @@
 	/* step 1: calculate xfer_mask */
 	ata_link_for_each_dev(dev, link) {
 		unsigned int pio_mask, dma_mask;
+		unsigned int mode_mask;
 
 		if (!ata_dev_enabled(dev))
 			continue;
 
+		mode_mask = ATA_DMA_MASK_ATA;
+		if (dev->class == ATA_DEV_ATAPI)
+			mode_mask = ATA_DMA_MASK_ATAPI;
+		else if (ata_id_is_cfa(dev->id))
+			mode_mask = ATA_DMA_MASK_CFA;
+
 		ata_dev_xfermask(dev);
 
 		pio_mask = ata_pack_xfermask(dev->pio_mask, 0, 0);
 		dma_mask = ata_pack_xfermask(0, dev->mwdma_mask, dev->udma_mask);
+
+		if ((ata_pata_dma & mode_mask) || ap->cbl == ATA_CBL_SATA)
+			dma_mask = ata_pack_xfermask(0, dev->mwdma_mask, dev->udma_mask);
+		else
+			dma_mask = 0;
+
 		dev->pio_mode = ata_xfer_mask2mode(pio_mask);
 		dev->dma_mode = ata_xfer_mask2mode(dma_mask);
 

Re: [PATCH] libata: Add a drivers/ide style DMA disable

[Tejun Heo]

Alan Cox wrote:
> This is useful when debugging, handling problem systems, or for
> distributions just to get the system installed so it can be sorted
> out later.
> 
> This is a bit smarter than the old IDE one and lets you do
> 
> libata.pata_dma=0		Disable all PATA DMA like old IDE
> libata.pata_dma=1		Disk DMA only
> libata.pata_dma=2		ATAPI DMA only
> libata.pata_dma=4		CF DMA only
> 
> (or combinations thereof - 0,1,3 being the useful ones I suspect)
> 
> (I've split CF as it seems to be a seperate case of pain and suffering
> different to the others and caused by assorted PIO wired adapters etc)
> 
> SATA is not affected - for one its not clear it makes sense to disable
> DMA for SATA if even always possible, for two we've seen no failure 
> evidence to justify needing to support this kind of hammer on SATA.
> 
> Signed-off-by: Alan Cox <alan@redhat.com>

Acked-by: Tejun Heo <htejun@gmail.com>

-- 
tejun

Re: [PATCH] libata: Add a drivers/ide style DMA disable

[Jeff Garzik]

Tejun Heo wrote:
> Alan Cox wrote:
>> This is useful when debugging, handling problem systems, or for
>> distributions just to get the system installed so it can be sorted
>> out later.
>>
>> This is a bit smarter than the old IDE one and lets you do
>>
>> libata.pata_dma=0		Disable all PATA DMA like old IDE
>> libata.pata_dma=1		Disk DMA only
>> libata.pata_dma=2		ATAPI DMA only
>> libata.pata_dma=4		CF DMA only
>>
>> (or combinations thereof - 0,1,3 being the useful ones I suspect)
>>
>> (I've split CF as it seems to be a seperate case of pain and suffering
>> different to the others and caused by assorted PIO wired adapters etc)
>>
>> SATA is not affected - for one its not clear it makes sense to disable
>> DMA for SATA if even always possible, for two we've seen no failure 
>> evidence to justify needing to support this kind of hammer on SATA.
>>
>> Signed-off-by: Alan Cox <alan@redhat.com>
> 
> Acked-by: Tejun Heo <htejun@gmail.com>

FWIW -- as I noted to Alan personally at KS, I would rather drop the 
"pata_" and have it apply to all, PATA or SATA.

It is far less useful on SATA, but I can still see a possibility that 
somebody might want to avoid DMA for whatever reason, chiefly, possibly 
keeping failing hardware alive by slowing things down via PIO.  And 
being able to avoid the DMA protocols can be useful for debugging or 
other things.

As an aside, it would be nice to get the EH in shape where it can pause 
all ports, so we can finally allow userspace to submit SET FEATURES - 
XFER MODE and have it handled cleanly and properly.

	Jeff

Re: [PATCH] libata: Add a drivers/ide style DMA disable

[Tejun Heo]

Jeff Garzik wrote:
> FWIW -- as I noted to Alan personally at KS, I would rather drop the
> "pata_" and have it apply to all, PATA or SATA.

Having the distinction between sata and pata is also nice tho
considering many modern machines mix the two and use the P part for
ATAPI, CF kind of stuff.  It's all just masks anyway, we can easily do both.

> As an aside, it would be nice to get the EH in shape where it can pause
> all ports, so we can finally allow userspace to submit SET FEATURES -
> XFER MODE and have it handled cleanly and properly.

Aye aye sir, bumping up in the to-do list.  BTW, I'm currently on
airplane and won't be back home in about ten days.  PMP patchset is
regenerated but it has some problem integrating with the AN support and
I need my gears to iron it out, so PMP patches will have to wait a bit
more.  FWIW, openSUSE 10.3 defaults to libata drivers and contains the
PMP patches.  It already received quite a bit of testing going through
two betas, so I don't think the PMP patches would introduce too many
serious regressions for non-PMP users.

Thanks.

-- 
tejun

Re: [PATCH] libata: Add a drivers/ide style DMA disable

[Alan Cox]

> FWIW -- as I noted to Alan personally at KS, I would rather drop the 
> "pata_" and have it apply to all, PATA or SATA.

Agreed - this is a sensible path.

You can drop a patch to do that over my changes easily enough I think,
its just a rename and removing a single check.

Alan

Re: [PATCH] libata: Add a drivers/ide style DMA disable

[Chuck Ebbert]

On 09/07/2007 06:47 PM, Jeff Garzik wrote:
>>> This is useful when debugging, handling problem systems, or for
>>> distributions just to get the system installed so it can be sorted
>>> out later.
>>>
>>> This is a bit smarter than the old IDE one and lets you do
>>>
>>> libata.pata_dma=0        Disable all PATA DMA like old IDE
>>> libata.pata_dma=1        Disk DMA only
>>> libata.pata_dma=2        ATAPI DMA only
>>> libata.pata_dma=4        CF DMA only
>>>
> 
> FWIW -- as I noted to Alan personally at KS, I would rather drop the
> "pata_" and have it apply to all, PATA or SATA.
> 

Not sure that's a good idea for people who need to use it as a workaround.
E.g. they might have DMA working on some SATA devices but not on PATA, and
only want to disable the PATA DMA. Either adding a separate "sata_dma" or
additional bitfields for SATA would be useful.

Re: [PATCH] libata: Add a drivers/ide style DMA disable

[Bartlomiej Zolnierkiewicz]

On Friday 14 September 2007, Chuck Ebbert wrote:
> On 09/07/2007 06:47 PM, Jeff Garzik wrote:
> >>> This is useful when debugging, handling problem systems, or for
> >>> distributions just to get the system installed so it can be sorted
> >>> out later.
> >>>
> >>> This is a bit smarter than the old IDE one and lets you do
> >>>
> >>> libata.pata_dma=0        Disable all PATA DMA like old IDE

2.6.24 will have "hdx=nodma"

> >>> libata.pata_dma=1        Disk DMA only
> >>> libata.pata_dma=2        ATAPI DMA only
> >>> libata.pata_dma=4        CF DMA only
> >>>
> > 
> > FWIW -- as I noted to Alan personally at KS, I would rather drop the
> > "pata_" and have it apply to all, PATA or SATA.
> > 
> 
> Not sure that's a good idea for people who need to use it as a workaround.
> E.g. they might have DMA working on some SATA devices but not on PATA, and
> only want to disable the PATA DMA. Either adding a separate "sata_dma" or
> additional bitfields for SATA would be useful.

Re: [PATCH] libata: Add a drivers/ide style DMA disable

[Chuck Ebbert]

On 08/22/2007 06:37 PM, Alan Cox wrote:
> This is useful when debugging, handling problem systems, or for
> distributions just to get the system installed so it can be sorted
> out later.
> 
> This is a bit smarter than the old IDE one and lets you do
> 
> libata.pata_dma=0		Disable all PATA DMA like old IDE
> libata.pata_dma=1		Disk DMA only
> libata.pata_dma=2		ATAPI DMA only
> libata.pata_dma=4		CF DMA only
> 
> (or combinations thereof - 0,1,3 being the useful ones I suspect)
> 

This doesn't help distro installers with modular libata, as there's
no good way to get the parameter to the driver. (libata.pata_dma=0
just causes "unrecognized parameter" message on boot.)

Re: [PATCH] libata: Add a drivers/ide style DMA disable

[Alan Cox]

> This doesn't help distro installers with modular libata, as there's
> no good way to get the parameter to the driver. (libata.pata_dma=0
> just causes "unrecognized parameter" message on boot.)

And for how long have people been saying that Fedora needs to sort out
passing module parameters from the boot command line via anaconda ?

So I'm sorry but "Mummy my installer sucks" doesn't get sympathy.
Distribution problem not kernel.

Alan

Re: [PATCH] libata: Add a drivers/ide style DMA disable

[Tejun Heo]

Chuck Ebbert wrote:
> On 08/22/2007 06:37 PM, Alan Cox wrote:
>> This is useful when debugging, handling problem systems, or for
>> distributions just to get the system installed so it can be sorted
>> out later.
>>
>> This is a bit smarter than the old IDE one and lets you do
>>
>> libata.pata_dma=0		Disable all PATA DMA like old IDE
>> libata.pata_dma=1		Disk DMA only
>> libata.pata_dma=2		ATAPI DMA only
>> libata.pata_dma=4		CF DMA only
>>
>> (or combinations thereof - 0,1,3 being the useful ones I suspect)
>>
> 
> This doesn't help distro installers with modular libata, as there's
> no good way to get the parameter to the driver. (libata.pata_dma=0
> just causes "unrecognized parameter" message on boot.)

Dunno how all those initrd black magics work but SUSE initrd manages to
deliver "module.param" parameters on kernel command line to respective
modules in initrd.  I don't think it gets to modules which are loaded
after root is mounted tho.

-- 
tejun

Re: [PATCH] libata: Add a drivers/ide style DMA disable

[Jeff Garzik]

Alan Cox wrote:
> This is useful when debugging, handling problem systems, or for
> distributions just to get the system installed so it can be sorted
> out later.
> 
> This is a bit smarter than the old IDE one and lets you do
> 
> libata.pata_dma=0		Disable all PATA DMA like old IDE
> libata.pata_dma=1		Disk DMA only
> libata.pata_dma=2		ATAPI DMA only
> libata.pata_dma=4		CF DMA only
> 
> (or combinations thereof - 0,1,3 being the useful ones I suspect)
> 
> (I've split CF as it seems to be a seperate case of pain and suffering
> different to the others and caused by assorted PIO wired adapters etc)
> 
> SATA is not affected - for one its not clear it makes sense to disable
> DMA for SATA if even always possible, for two we've seen no failure 
> evidence to justify needing to support this kind of hammer on SATA.
> 
> Signed-off-by: Alan Cox <alan@redhat.com>

applied, after making it work on SATA too