Re: [PATCH 1/3] 2.6.0 aacraid driver update

Matthew Wilcox
2003-10-08T13:55:56Z

On Wed, Oct 08, 2003 at 09:11:24AM -0400, Salyzyn, Mark wrote:
> The scsicmd->SCp.ptr is `stealing' an unused member of the command to hold
> on to the physical address of an additional allocation that is part of the
> request so that when the command completes the memory can be `freed'.
>
> My recommendation is to allocate an additional structure to hold on to all
> classes of `theft' like this in the driver (my guess is there is more) and
> use the SCp.ptr to point to this allocation rather than try to `steal' more
> to make up the full length.
>
> Any wisdom of the list with regards to this?

[Someone with more in-depth knowledge of Linux scsi please correct me
if I'm wrong]

include/scsi/scsi_cmnd.h defines struct scsi_cmnd which embeds the
scsi_pointer SCp. The comment beside it says, "Scratchpad used by some
host adapters". So I believe we're free to use any element of SCp we
like. Am I correct?

If so, it already contains dma_addr_t dma_handle, so we can simplify the
aacraid code to use this wherever we currently use ptr. The following
patch does this:

Index: drivers/scsi/aacraid/aachba.c
===================================================================
RCS file: /var/cvs/linux-2.6/drivers/scsi/aacraid/aachba.c,v
retrieving revision 1.2
diff -u -p -r1.2 aachba.c
--- drivers/scsi/aacraid/aachba.c 29 Jul 2003 17:25:47 -0000 1.2
+++ drivers/scsi/aacraid/aachba.c 8 Oct 2003 13:53:25 -0000
@@ -559,7 +559,7 @@ static void read_callback(void *context,
scsicmd->use_sg,
scsi_to_pci_dma_dir(scsicmd->sc_data_direction));
else if(scsicmd->request_bufflen)
- pci_unmap_single(dev->pdev, (dma_addr_t)(ulong)scsicmd->SCp.ptr,
+ pci_unmap_single(dev->pdev, scsicmd->SCp.dma_handle,
scsicmd->request_bufflen,
scsi_to_pci_dma_dir(scsicmd->sc_data_direction));
readreply = (struct aac_read_reply *)fib_data(fibptr);
@@ -603,7 +603,7 @@ static void write_callback(void *context
scsicmd->use_sg,
scsi_to_pci_dma_dir(scsicmd->sc_data_direction));
else if(scsicmd->request_bufflen)
- pci_unmap_single(dev->pdev, (dma_addr_t)(ulong)scsicmd->SCp.ptr,
+ pci_unmap_single(dev->pdev, scsicmd->SCp.dma_handle,
scsicmd->request_bufflen,
scsi_to_pci_dma_dir(scsicmd->sc_data_direction));

@@ -1235,7 +1235,8 @@ static void aac_srb_callback(void *conte
scsicmd->use_sg,
scsi_to_pci_dma_dir(scsicmd->sc_data_direction));
else if(scsicmd->request_bufflen)
- pci_unmap_single(dev->pdev, (ulong)scsicmd->SCp.ptr, scsicmd->request_bufflen,
+ pci_unmap_single(dev->pdev, scsicmd->SCp.dma_handle,
+ scsicmd->request_bufflen,
scsi_to_pci_dma_dir(scsicmd->sc_data_direction));

/*
@@ -1547,15 +1548,13 @@ static unsigned long aac_build_sg(Scsi_C
}
}
else if(scsicmd->request_bufflen) {
- dma_addr_t addr;
- addr = pci_map_single(dev->pdev,
+ scsicmd->SCp.dma_handle = pci_map_single(dev->pdev,
scsicmd->request_buffer,
scsicmd->request_bufflen,
scsi_to_pci_dma_dir(scsicmd->sc_data_direction));
psg->count = cpu_to_le32(1);
- psg->sg[0].addr = cpu_to_le32(addr);
+ psg->sg[0].addr = cpu_to_le32(scsicmd->SCp.dma_handle);
psg->sg[0].count = cpu_to_le32(scsicmd->request_bufflen);
- scsicmd->SCp.ptr = (char *)(ulong)addr;
byte_count = scsicmd->request_bufflen;
}
return byte_count;
@@ -1606,17 +1605,15 @@ static unsigned long aac_build_sg64(Scsi
}
}
else if(scsicmd->request_bufflen) {
- dma_addr_t addr;
- addr = pci_map_single(dev->pdev,
+ scsicmd->SCp.dma_handle = pci_map_single(dev->pdev,
scsicmd->request_buffer,
scsicmd->request_bufflen,
scsi_to_pci_dma_dir(scsicmd->sc_data_direction));
psg->count = cpu_to_le32(1);
- le_addr = cpu_to_le64(addr);
+ le_addr = cpu_to_le64(scsicmd->SCp.dma_handle);
psg->sg[0].addr[1] = (u32)(le_addr>>32);
psg->sg[0].addr[0] = (u32)(le_addr & 0xffffffff);
psg->sg[0].count = cpu_to_le32(scsicmd->request_bufflen);
- scsicmd->SCp.ptr = (char *)(ulong)addr;
byte_count = scsicmd->request_bufflen;
}
return byte_count;

I hope you all now feel like you might be masking a bug every time you
use a cast :-P

--
"It's not Hollywood. War is real, war is primarily not about defeat or
victory, it is about death. I've seen thousands and thousands of dead bodies.
Do you think I want to have an academic debate on this subject?" -- Robert Fisk