Alan Stern wrote:
>> Why exactly is this a little fragile?
>
> Because it relies on people in the future understanding the reason for
> this strange manipulation and not messing it up. Yes, it will work,
> but I'd prefer to do it differently.
>
>> Doing the reset of {transfer,setup}_dma on unmap time has benefits from doing it on map time.
>
> Maybe, but that's not what I was proposing. I didn't say transfer_dma
> should be reset at map time; I said that the submission test should be
> changed.
>
Ok, I didn't understand you.
>> In this case we will have a different transfer_dma value visible for the USB driver depending on if we are doing the first transfer of the URB after its allocation or a subsequent transfer.
>> Also it is hard (and may fail to do it right) for the USB stack to determine for the subsequent transfers if the transfer_dma value is then a real mapping or not.
>
> The test shouldn't be too hard: If URB_NO_TRANSFER_DMA_MAP is set and
> the hcd uses DMA (or the DMA address isn't ~0) and HCD_NO_COHERENT_MEM
> isn't set, then the mapping is already okay.
>
This isn't enough.
usb_buffer_alloc()
/* urb->transfer_dma == ~0 */
urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
/* 1st transfer */
usb_submit_urb(urb, GFP_KERNEL);
if URB_NO_TRANSFER_DMA_MAP is set (true) and transfer_dma != ~0 (false) and HCD_NO_COHERENT_MEM (true) do nothing
transfer gets mapped
transfer_dma becomes != ~0 during the mapping
transfer_dma is not reset during the unmapping
/* 2nd transfer */
usb_submit_urb(urb, GFP_KERNEL);
if URB_NO_TRANSFER_DMA_MAP is set (true) and transfer_dma != ~0 (true) and HCD_NO_COHERENT_MEM (true) do nothing
transfer _doesn't_ get mapped, buffer is not sync'ed, transfer silently fails
transfer_dma != ~0
> You're concerned that transfer_dma might contain a valid DMA address
> for an existing mapping. How could it? usb_buffer_alloc() won't
> return a valid address if HCD_NO_COHERENT_MEM is set. And even if it
> did, you would overwrite that valid address at unmap time anyway.
>
transfer_dma gets a valid mapping when it is mapped by dma_map_single() at map time.
In the scenario I propose, if the transfer buffer wasn't mapped then it won't get unmapped (the new flags control that), and thus transfer_dma won't be touched.
The only case it will be overwritten to ~0 at unmap time is when transfer_dma was ~0 at map time _and_ it was actually mapped (HCD_NO_COHERENT_MEM case).
> Alan Stern
>
Thanks,
Albert
--
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
