Re: [PATCH 4/9] usb-core: Free bulk streams on interface release
Hi, On 10/04/2013 08:43 PM, Alan Stern wrote: On Fri, 4 Oct 2013, Hans de Goede wrote: + struct usb_host_endpoint *ep, *eps[USB_MAXENDPOINTS]; That's a big array to put on the stack: 30 entries each containing 8 bytes (on a 64-bit arch). I was wondering about this myself when I wrote this. What do you suggest as an alternative, kmalloc an array with intf-cur_altsetting-desc.bNumEndpoints pointers ? Or simply do 30 pointers every time. 240 bytes isn't all that much for kmalloc. Right, the allocating intf-cur_altsetting-desc.bNumEndpoints versus 30 pointers is just an optimization. I was wondering more if it would be ok here to do a kmalloc(..., GPF_KERNEL), or if this code path is ever called under interrupt, or some such ? Also do we allow C-99 style variable length arrays in the kernel ? Then we could just put what ever we need on the stack (which almost always will be way less then 240 bytes). Regards, Hans -- 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
Re: [PATCH 4/9] usb-core: Free bulk streams on interface release
On Sun, 6 Oct 2013, Hans de Goede wrote: I was wondering about this myself when I wrote this. What do you suggest as an alternative, kmalloc an array with intf-cur_altsetting-desc.bNumEndpoints pointers ? Or simply do 30 pointers every time. 240 bytes isn't all that much for kmalloc. Right, the allocating intf-cur_altsetting-desc.bNumEndpoints versus 30 pointers is just an optimization. I was wondering more if it would be ok here to do a kmalloc(..., GPF_KERNEL), or if this code path is ever called under interrupt, or some such ? GFP_KERNEL will be okay. This code always runs in process context with interrupts enabled. Also do we allow C-99 style variable length arrays in the kernel ? Then we could just put what ever we need on the stack (which almost always will be way less then 240 bytes). I think people try to avoid variable-length arrays. Certainly such arrays in structures are frowned upon. Alan Stern -- 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
Re: [PATCH 4/9] usb-core: Free bulk streams on interface release
On Fri, 4 Oct 2013, Hans de Goede wrote:
Documentation/usb/bulk-streams.txt says:
All stream IDs will be deallocated when the driver releases the interface, to
ensure that drivers that don't support streams will be able to use the
endpoint
This commit actually implements this.
--- a/drivers/usb/core/driver.c
+++ b/drivers/usb/core/driver.c
@@ -369,8 +369,9 @@ static int usb_unbind_interface(struct device *dev)
{
struct usb_driver *driver = to_usb_driver(dev-driver);
struct usb_interface *intf = to_usb_interface(dev);
+ struct usb_host_endpoint *ep, *eps[USB_MAXENDPOINTS];
That's a big array to put on the stack: 30 entries each containing 8
bytes (on a 64-bit arch).
Alan Stern
--
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
Re: [PATCH 4/9] usb-core: Free bulk streams on interface release
Hi,
On 10/04/2013 05:37 PM, Alan Stern wrote:
On Fri, 4 Oct 2013, Hans de Goede wrote:
Documentation/usb/bulk-streams.txt says:
All stream IDs will be deallocated when the driver releases the interface, to
ensure that drivers that don't support streams will be able to use the endpoint
This commit actually implements this.
--- a/drivers/usb/core/driver.c
+++ b/drivers/usb/core/driver.c
@@ -369,8 +369,9 @@ static int usb_unbind_interface(struct device *dev)
{
struct usb_driver *driver = to_usb_driver(dev-driver);
struct usb_interface *intf = to_usb_interface(dev);
+ struct usb_host_endpoint *ep, *eps[USB_MAXENDPOINTS];
That's a big array to put on the stack: 30 entries each containing 8
bytes (on a 64-bit arch).
I was wondering about this myself when I wrote this. What do you suggest
as an alternative, kmalloc an array with
intf-cur_altsetting-desc.bNumEndpoints
pointers ?
Regards,
Hans
--
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
Re: [PATCH 4/9] usb-core: Free bulk streams on interface release
On Fri, 4 Oct 2013, Hans de Goede wrote: + struct usb_host_endpoint *ep, *eps[USB_MAXENDPOINTS]; That's a big array to put on the stack: 30 entries each containing 8 bytes (on a 64-bit arch). I was wondering about this myself when I wrote this. What do you suggest as an alternative, kmalloc an array with intf-cur_altsetting-desc.bNumEndpoints pointers ? Or simply do 30 pointers every time. 240 bytes isn't all that much for kmalloc. Alan Stern -- 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
