On Friday 28 June 2013 15:39:27 Jussi Kivilinna wrote: > On 16.06.2013 13:35, Jussi Kivilinna wrote: > > On 16.06.2013 11:21, Oliver Neukum wrote: > >> On Saturday 15 June 2013 16:22:30 Jussi Kivilinna wrote: > >> > >>> Hm.. rethink this a bit. > >>> > >>> Transfer buffer might be dma aligned but shorter than cacheline and end > >>> of cacheline > >>> used as something else. Manual alignment by host driver does not catch > >>> that > >>> or fix that. > >>> So, yes.. dma mapping should work with unaligned buffers, but maybe the > >>> actual > >>> problem is multiple buffers from same cacheline. > >> > >> The buffers kmalloc() returns are OK in that regard. A driver that uses > >> a buffer for anything but buffering is buggy. > > > > Ok, I'll look at that direction. Thanks. > > > > So if I understood correctly, drivers that allocate these as part of larger > structures (struct *_device etc) are doing wrong thing and are potentially > buggy. And this is because cachelines of buffers can be DMA mapped after > usb_submit_urb() and editing same cacheline while URB is in-flight can > therefore be hazardous. > > I checked setup_packet and transfer_buffer usage of some drivers in 3.9.8 and > made some observations. Should these be fixed? > > URB setup_packet and transfer_buffer part of same structure (might share same > cacheline for same URB): > * iforce: > - > http://lxr.linux.no/linux+v3.9.8/drivers/input/joystick/iforce/iforce-usb.c#L173 > - > http://lxr.linux.no/linux+v3.9.8/drivers/input/joystick/iforce/iforce.h#L101
Buggy > * usbvision: > - > http://lxr.linux.no/linux+v3.9.8/drivers/media/usb/usbvision/usbvision-core.c#L1445 > - > http://lxr.linux.no/linux+v3.9.8/drivers/media/usb/usbvision/usbvision.h#L366 Buggy > * catc: > - http://lxr.linux.no/linux+v3.9.8/drivers/net/usb/catc.c#L499 > - http://lxr.linux.no/linux+v3.9.8/drivers/net/usb/catc.c#L500 > - ctrl_buf, ctrl_dr: > http://lxr.linux.no/linux+v3.9.8/drivers/net/usb/catc.c#L162 > * rtl8150: > - http://lxr.linux.no/linux+v3.9.8/drivers/net/usb/rtl8150.c#L200 > - http://lxr.linux.no/linux+v3.9.8/drivers/net/usb/rtl8150.c#L128 > * rt2x000usb: > - > http://lxr.linux.no/linux+v3.9.8/drivers/net/wireless/rt2x00/rt2x00usb.c#L212 > - > http://lxr.linux.no/linux+v3.9.8/drivers/net/wireless/rt2x00/rt2x00usb.c#L169 > * rtl8187: > - > http://lxr.linux.no/linux+v3.9.8/drivers/net/wireless/rtl818x/rtl8187/dev.c#L156 > - > http://lxr.linux.no/linux+v3.9.8/drivers/net/wireless/rtl818x/rtl8187/dev.c#L130 > * uss720: > - http://lxr.linux.no/linux+v3.9.8/drivers/usb/misc/uss720.c#L176 > - http://lxr.linux.no/linux+v3.9.8/drivers/usb/misc/uss720.c#L72 Well, I didn't look through them all, but we must assume that they are buggy. > URB transfer_buffer array (transfer buffers preloaded as array, element size > less than cacheline): > * rtlwifi: > - http://lxr.linux.no/linux+v3.9.8/drivers/net/wireless/rtlwifi/usb.c#L152 > - http://lxr.linux.no/linux+v3.9.8/drivers/net/wireless/rtlwifi/wifi.h#L1859 > - http://lxr.linux.no/linux+v3.9.8/drivers/net/wireless/rtlwifi/usb.c#L980 Good catch. This is a very large number. I suggest you split it up. Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
