lkml.org 
[lkml]   [2013]   [Sep]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] SCSI: buslogic: Added check for DMA mapping errors (was Re:[BusLogic] DMA-API: device driver failed to check map error)
From
Date
Khalid Aziz wrote:
> Added check for DMA mapping errors for request sense data
> buffer. Checking for mapping error can avoid potential wild
> writes. This patch was prompted by the warning from
> dma_unmap when kernel is compiled with CONFIG_DMA_API_DEBUG.

This patch fixes CONFIG_DMA_API_DEBUG warning.
But excuse me, is this error path correct?

> @@ -309,16 +309,17 @@ static struct blogic_ccb *blogic_alloc_ccb(struct blogic_adapter *adapter)
> blogic_dealloc_ccb deallocates a CCB, returning it to the Host Adapter's
> free list. The Host Adapter's Lock should already have been acquired by the
> caller.
> */
>
> -static void blogic_dealloc_ccb(struct blogic_ccb *ccb)
> +static void blogic_dealloc_ccb(struct blogic_ccb *ccb, int dma_unmap)
> {
> struct blogic_adapter *adapter = ccb->adapter;
>
> scsi_dma_unmap(ccb->command);

blogic_dealloc_ccb() uses "ccb->command". But

> - pci_unmap_single(adapter->pci_device, ccb->sensedata,
> + if (dma_unmap)
> + pci_unmap_single(adapter->pci_device, ccb->sensedata,
> ccb->sense_datalen, PCI_DMA_FROMDEVICE);
>
> ccb->command = NULL;
> ccb->status = BLOGIC_CCB_FREE;
> ccb->next = adapter->free_ccbs;
> @@ -3177,13 +3179,21 @@ static int blogic_qcmd_lck(struct scsi_cmnd *command,
> ccb->legacy_tag = queuetag;
> }
> }
> memcpy(ccb->cdb, cdb, cdblen);
> ccb->sense_datalen = SCSI_SENSE_BUFFERSIZE;
> - ccb->sensedata = pci_map_single(adapter->pci_device,
> + sense_buf = pci_map_single(adapter->pci_device,
> command->sense_buffer, ccb->sense_datalen,
> PCI_DMA_FROMDEVICE);
> + if (dma_mapping_error(&adapter->pci_device->dev, sense_buf)) {
> + blogic_err("DMA mapping for sense data buffer failed\n",
> + adapter);
> + scsi_dma_map(command);
> + blogic_dealloc_ccb(ccb, 0);

when was "ccb->command = command;" called before calling blogic_dealloc_ccb()?

> + return SCSI_MLQUEUE_HOST_BUSY;
> + }
> + ccb->sensedata = sense_buf;
> ccb->command = command;
> command->scsi_done = comp_cb;
> if (blogic_multimaster_type(adapter)) {
> /*
> Place the CCB in an Outgoing Mailbox. The higher levels

Also, what happens if "scsi_dma_map(command);" returned -ENOMEM ?
If you are calling scsi_dma_map() because blogic_dealloc_ccb() calls
scsi_dma_unmap(), why can't we do like

{
struct blogic_adapter *adapter = ccb->adapter;
ccb->command = NULL;
ccb->status = BLOGIC_CCB_FREE;
ccb->next = adapter->free_ccbs;
adapter->free_ccbs = ccb;
}

instead of

scsi_dma_map(command);
blogic_dealloc_ccb(ccb);

?


\
 
 \ /
  Last update: 2013-09-13 18:01    [W:0.038 / U:1.308 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site