[PATCH] Make SG_SET_FORCE_LOW_DMA behave as advertised

[PATCH] Make SG_SET_FORCE_LOW_DMA behave as advertised

[Olaf Kirch]

Make SG_SET_FORCE_LOW_DMA behave as advertised

I came across this by accident. I have serious doubts whether ISA DMA
is really relevant these days :-) but what the heck. Feel free to disregard
if this code is headed for the recycler anyway.

The SCSI-HOWTO says this about SG_SET_FORCE_LOW_DMA:

	If the "reserved" buffer allocated on open() is not in use then
	it will be de-allocated and re-allocated under the 16MB level
	(and the latter operation could fail yielding ENOMEM).

I came across this by accident - the current code will never reallocate
the buffer during the ioctl, because it first sets sfp->low_dma to 1,
and then checks the very same variable for equality with 0.

The patch below changes the order of commands, and also moves
the buffer reallocation around so that it also happens when
the device has unchecked_isa_dma set.

Signed-off-by: olaf.kirch@oracle.com

---
 drivers/scsi/sg.c |   13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

Index: linux-2.6.20/drivers/scsi/sg.c
===================================================================
--- linux-2.6.20.orig/drivers/scsi/sg.c
+++ linux-2.6.20/drivers/scsi/sg.c
@@ -842,17 +842,22 @@ sg_ioctl(struct inode *inode, struct fil
 		result = get_user(val, ip);
 		if (result)
 			return result;
+		if (val == 0) {
+			if (sdp->detached)
+				return -ENODEV;
+			val = sdp->device->host->unchecked_isa_dma;
+		}
 		if (val) {
+			int was_low_dma = sfp->low_dma;
+
 			sfp->low_dma = 1;
-			if ((0 == sfp->low_dma) && (0 == sg_res_in_use(sfp))) {
+			if ((0 == was_low_dma) && (0 == sg_res_in_use(sfp))) {
 				val = (int) sfp->reserve.bufflen;
 				sg_remove_scat(&sfp->reserve);
 				sg_build_reserve(sfp, val);
 			}
 		} else {
-			if (sdp->detached)
-				return -ENODEV;
-			sfp->low_dma = sdp->device->host->unchecked_isa_dma;
+			sfp->low_dma = 0;
 		}
 		return 0;
 	case SG_GET_LOW_DMA:

-- 
Olaf Kirch  |  --- o --- Nous sommes du soleil we love when we play
okir@lst.de |    / | \   sol.dhoop.naytheet.ah kin.ir.samse.qurax

Re: [PATCH] Make SG_SET_FORCE_LOW_DMA behave as advertised

[Mike Christie]

Olaf Kirch wrote:
> Make SG_SET_FORCE_LOW_DMA behave as advertised
> 
> I came across this by accident. I have serious doubts whether ISA DMA
> is really relevant these days :-) but what the heck. Feel free to disregard
> if this code is headed for the recycler anyway.
> 
> The SCSI-HOWTO says this about SG_SET_FORCE_LOW_DMA:
> 
> 	If the "reserved" buffer allocated on open() is not in use then
> 	it will be de-allocated and re-allocated under the 16MB level
> 	(and the latter operation could fail yielding ENOMEM).
> 
> I came across this by accident - the current code will never reallocate
> the buffer during the ioctl, because it first sets sfp->low_dma to 1,
> and then checks the very same variable for equality with 0.
> 
> The patch below changes the order of commands, and also moves
> the buffer reallocation around so that it also happens when
> the device has unchecked_isa_dma set.

It might be needed. The removal was my fault. If you are just worried
about using the right memory and the host had unchecked_isa_dma set, the
buffer is now bounced for sg by the block layer helpers so sg does not
need to worry about that.

The problem with this is that the sg code would allocate large chunks of
memory so it would try to make large segments, and if you were trying to
do a really large IO then the isa_page_pool may not work like the old sg
code.