On Fri, 29 Jun 2012, Hans de Goede wrote:
> > One possible enhancement would be to try allocating storage in units
> > larger than 16 KB, with fallback to smaller sizes when the larger
> > allocations fail.
>
> This puts us above order 2 (x86_64) / order 3 (i386) allocations, and high
> order allocations are quite expensive (if we ask the kernel will do a lot of
> work to try and satisfy this before it fails) this is one of the reasons why
> the kernel stack was reduced to be 1 page.
>
> So I think the costs may be higher then the gains while making the code more
> complicated.
All right, that makes sense.
> > However your code is just as good as the existing
> > mechanism.
> >
> >> +static void snoop_urb_data(struct urb *urb, unsigned len)
> >> +{
> >> + int i, size;
> >> +
> >> + if (!usbfs_snoop)
> >> + return;
> >> +
> >> + if (urb->num_sgs == 0) {
> >> + print_hex_dump(KERN_DEBUG, "data: ", DUMP_PREFIX_NONE, 32, 1,
> >> + urb->transfer_buffer, len, 1);
> >> + return;
> >> + }
> >> +
> >> + for (i = 0; i < urb->num_sgs && len; i++) {
> >> + size = (len > USB_SG_SIZE) ? USB_SG_SIZE : len;
> >> + print_hex_dump(KERN_DEBUG, "data: ", DUMP_PREFIX_NONE, 32, 1,
> >> + sg_virt(&urb->sg[i]), size, 1);
> >> + len -= size;
> >> + }
> >> +}
> >
> > Why didn't you just merge this directly into snoop_urb?
>
> Because snoop_urb is used in the synchronous ctrl / bulk ioctls too, and there
> there is no struct urb. I could have added a struct urb parameter to snoop_urb
> but that seemed more complicated / convoluted.
Okay.
Acked-by: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
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
