On Fri, Dec 16, 2022 at 09:55:09AM +0100, Ricardo Ribalda wrote: > Hi Alan > > On Thu, 15 Dec 2022 at 16:34, Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote: > > > > On Thu, Dec 15, 2022 at 11:57:17AM +0100, Ricardo Ribalda wrote: > > > There is no need to make a kzalloc just for 16 bytes. Let's embed the data > > > into the main data structure. > > > > > > Now that we are at it, lets remove all the castings and open coding of > > > offsets for it. > > > > > > [Christoph, do you think dma wise we are violating any non written rules? :) thanks] > > > > There _is_ a rule, and it is not exactly unwritten. The kerneldoc for > > the transfer_buffer member of struct urb says: > > > > This buffer must be suitable for DMA; allocate it with > > kmalloc() or equivalent. > > > > Which in general means that the buffer must not be part of a larger > > structure -- not unless the driver can guarantee that the structure will > > _never_ be accessed while a USB transfer to/from the buffer is taking > > place. > > > > Thanks a lot for the clarification. I was mainly looking at the kerneldoc from: > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/usb.h#n1687 > > and I could not see any reference to the DMA requirements. > > Mind if I send a patch to add a reference there? Not at all. But if you change the documentation for usb_fill_int_urb() then you should also change it for usb_fill_control_urb() and usb_fill_bulk_urb(). Alan Stern > > There are examples all over the USB subsystem where buffers as small as > > one or two bytes get kmalloc'ed in order to obey this rule. 16 bytes is > > certainly big enough that you shouldn't worry about it being allocated > > separately. > > > Yep, we should keep it malloced. Thanks a lot for looking into this :) > > > > Alan Stern > > > > -- > Ricardo Ribalda
