On Wed, 10 Jun 2009, Pete Zaitcev wrote:
> Hi, Alan:
>
> Please look at the attached patch. It allows the "new" usbmon to show
> the storage data in kernels that could possibly be running on systems
> with GART-based IOMMUs, but aren't actually. These #ifdefs always
> bothered me.
The basic idea is sound and the code is cleaner.
> Greetings,
> -- Pete
>
> diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
> index b626283..e2ecf36 100644
> --- a/drivers/usb/core/message.c
> +++ b/drivers/usb/core/message.c
> @@ -421,30 +421,21 @@ int usb_sg_init(struct usb_sg_request *io, struct usb_device *dev,
> /*
> * Some systems need to revert to PIO when DMA is temporarily
> * unavailable. For their sakes, both transfer_buffer and
> - * transfer_dma are set when possible. However this can only
> - * work on systems without:
> + * transfer_dma are set when possible.
> *
> - * - HIGHMEM, since DMA buffers located in high memory are
> - * not directly addressable by the CPU for PIO;
> - *
> - * - IOMMU, since dma_map_sg() is allowed to use an IOMMU to
> - * make virtually discontiguous buffers be "dma-contiguous"
> - * so that PIO and DMA need diferent numbers of URBs.
> - *
> - * So when HIGHMEM or IOMMU are in use, transfer_buffer is NULL
> - * to prevent stale pointers and to help spot bugs.
> + * Note that if the IOMMU coalescing occured, we cannot
s/if the/if/
s/occured/occurred/
> + * trust sg_page anymore, so check if S/G list shrunk.
> */
> + if (io->nents == io->entries && !PageHighMem(sg_page(sg))) {
> + io->urbs[i]->transfer_buffer = sg_virt(sg);
> + } else {
> + io->urbs[i]->transfer_buffer = NULL;
> + }
> if (dma) {
> io->urbs[i]->transfer_dma = sg_dma_address(sg);
> len = sg_dma_len(sg);
> -#if defined(CONFIG_HIGHMEM) || defined(CONFIG_GART_IOMMU)
> - io->urbs[i]->transfer_buffer = NULL;
> -#else
> - io->urbs[i]->transfer_buffer = sg_virt(sg);
> -#endif
> } else {
> /* hc may use _only_ transfer_buffer */
> - io->urbs[i]->transfer_buffer = sg_virt(sg);
> len = sg->length;
There needs to be a test here for transfer_buffer != NULL. It would
not be good for an HCD using PIO to crash the system because it tried
to access invalid memory.
> }
Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
