Re: questions about give back urb in tasklet
On Wed, Mar 19, 2014 at 11:05 PM, vichy vichy@gmail.com wrote: hi Ming: One approach I like to use is trace event. Would you please let us how to to use trace event for calculate max/average usb irq time? You can enable irq_handler_entry and irq_handler_exit event, then write a script easily to figure out the time consumed in ehci irq handler, see Documentation/trace/events.txt of linux kernel for detailed steps. I find it ^^ But actually the local interrupt is still disabled during completion because we need to convert spin_lock in all current usb drivers to spin_lock_irqsave() first. Would you please let me know the function or source you mentioned? The local interrupt is disabled during completion handler in __usb_hcd_giveback_urb(). BTW, why I cannot see xhci add HCD_BH? There might be some reasons: - in previous following up discussion, Alan thought it should be easier to just split the hcd interrupt handler into two parts - as I said, spin_lock() isn't converted to spin_lock_irqsave() in drivers' completion handler yet Do you mean , take uvc/uac for example, if we put completion what they register previously in BH, the spin_lock() in their completions should be changed to spin_lock_irqsave()? Is it due to original completion is called in irq context? Some drivers may assume that the completion handler is called from hardirq handler, so call spin_lock(), and they should have called spin_lock_irqsave() if there are shared resources between its completion handler and other irq handler. So the simplest approach for the conversion is to replace all spin_lock() inside urb completion path into spin_lock_irqsave(). - maybe no one really care the time in which local interrupt is disabled by USB completion handler.(That might be true because USB devices are commonly used in personal computer, not in big server product) is there some reason xhci cannot put completion in BH? IMO, the completion handler should be put into tasklet, and can be done. If you like, you can take time to do that. Below is so far kernel implementation. Isn't it put completion handler into tasklet? Yes, it is run in tasklet context with local irq disabled. As you know we need to enable irq during URB completion. ... .. spin_lock(bh-lock); list_add_tail(urb-urb_list, bh-head); running = bh-running; spin_unlock(bh-lock); if (running) ; else if (high_prio_bh) tasklet_hi_schedule(bh-bh); else tasklet_schedule(bh-bh); Thanks for your help, -- 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: questions about give back urb in tasklet
hi Ming.lei: 2014-03-17 22:01 GMT+08:00 Alan Stern st...@rowland.harvard.edu: On Sun, 16 Mar 2014, vichy wrote: hi all: recently we bump to system performance issue when usb irq take quite long. I found below link for discussing how to short in http://permalink.gmane.org/gmane.linux.usb.general/89363 I have some questions about this patch. 1. is there patch or kernel config I can use to measure man/avage usb irq time consuming like the above link show 2. I see this patch is roll back in commit c04ee4b1136e462722567cf6e76bb35a181574a7 and intend to be ready in 3.13-rc1 Is there special reason why we need to roll back? Appreciate your help in advance, -- 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: questions about give back urb in tasklet
hi all: 2014-03-19 17:57 GMT+08:00 vichy vichy@gmail.com: hi Ming.lei: 2014-03-17 22:01 GMT+08:00 Alan Stern st...@rowland.harvard.edu: On Sun, 16 Mar 2014, vichy wrote: hi all: recently we bump to system performance issue when usb irq take quite long. I found below link for discussing how to short in http://permalink.gmane.org/gmane.linux.usb.general/89363 I have some questions about this patch. 1. is there patch or kernel config I can use to measure man/avage usb irq time consuming like the above link show 2. I see this patch is roll back in commit c04ee4b1136e462722567cf6e76bb35a181574a7 and intend to be ready in 3.13-rc1 Is there special reason why we need to roll back? why this patch didn't implement xhci? in xhci-pci.c there is no HCD_HB at xhci_pci_hc_driver .flags =HCD_MEMORY | HCD_USB3 | HCD_SHARED, does that mean xhci don't need to pug complete in the BH? thanks for your help, -- 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: questions about give back urb in tasklet
On Wed, Mar 19, 2014 at 5:57 PM, vichy vichy@gmail.com wrote: hi Ming.lei: 2014-03-17 22:01 GMT+08:00 Alan Stern st...@rowland.harvard.edu: On Sun, 16 Mar 2014, vichy wrote: hi all: recently we bump to system performance issue when usb irq take quite long. I found below link for discussing how to short in http://permalink.gmane.org/gmane.linux.usb.general/89363 I have some questions about this patch. 1. is there patch or kernel config I can use to measure man/avage usb irq time consuming like the above link show One approach I like to use is trace event. 2. I see this patch is roll back in commit c04ee4b1136e462722567cf6e76bb35a181574a7 and intend to be ready in 3.13-rc1 Is there special reason why we need to roll back? Yes, the revert revert can enable to run completion handler in BH, isn't that what you need? But actually the local interrupt is still disabled during completion because we need to convert spin_lock in all current usb drivers to spin_lock_irqsave() first. I posted lot of patches to do the conversion, but unfortunately most of them aren't merged. Thanks, -- Ming Lei -- 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: questions about give back urb in tasklet
hi Ming in http://permalink.gmane.org/gmane.linux.usb.general/89363 I have some questions about this patch. 1. is there patch or kernel config I can use to measure man/avage usb irq time consuming like the above link show One approach I like to use is trace event. Would you please let us how to to use trace event for calculate max/average usb irq time? 2. I see this patch is roll back in commit c04ee4b1136e462722567cf6e76bb35a181574a7 and intend to be ready in 3.13-rc1 Is there special reason why we need to roll back? Yes, the revert revert can enable to run completion handler in BH, isn't that what you need? YES, what I need is try to see whether my problem will be solved if running completion handlers in BH. At beginning, the revert let me think there are something wrong after we put completion in BH. And we need to revert it back to put completion in irq so the revert mean put completion in BH, right? But actually the local interrupt is still disabled during completion because we need to convert spin_lock in all current usb drivers to spin_lock_irqsave() first. Would you please let me know the function or source you mentioned? BTW, why I cannot see xhci add HCD_BH? is there some reason xhci cannot put completion in BH? thanks for your kind help, -- 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: questions about give back urb in tasklet
On Wed, Mar 19, 2014 at 9:34 PM, vichy vichy@gmail.com wrote: hi Ming in http://permalink.gmane.org/gmane.linux.usb.general/89363 I have some questions about this patch. 1. is there patch or kernel config I can use to measure man/avage usb irq time consuming like the above link show One approach I like to use is trace event. Would you please let us how to to use trace event for calculate max/average usb irq time? You can enable irq_handler_entry and irq_handler_exit event, then write a script easily to figure out the time consumed in ehci irq handler, see Documentation/trace/events.txt of linux kernel for detailed steps. 2. I see this patch is roll back in commit c04ee4b1136e462722567cf6e76bb35a181574a7 and intend to be ready in 3.13-rc1 Is there special reason why we need to roll back? Yes, the revert revert can enable to run completion handler in BH, isn't that what you need? YES, what I need is try to see whether my problem will be solved if running completion handlers in BH. At beginning, the revert let me think there are something wrong after we put completion in BH. And we need to revert it back to put completion in irq so the revert mean put completion in BH, right? Yes, it is a revert revert. But actually the local interrupt is still disabled during completion because we need to convert spin_lock in all current usb drivers to spin_lock_irqsave() first. Would you please let me know the function or source you mentioned? The local interrupt is disabled during completion handler in __usb_hcd_giveback_urb(). BTW, why I cannot see xhci add HCD_BH? There might be some reasons: - in previous following up discussion, Alan thought it should be easier to just split the hcd interrupt handler into two parts - as I said, spin_lock() isn't converted to spin_lock_irqsave() in drivers' completion handler yet - maybe no one really care the time in which local interrupt is disabled by USB completion handler.(That might be true because USB devices are commonly used in personal computer, not in big server product) is there some reason xhci cannot put completion in BH? IMO, the completion handler should be put into tasklet, and can be done. If you like, you can take time to do that. Thanks, -- Ming Lei -- 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: questions about give back urb in tasklet
On Wed, 19 Mar 2014, Ming Lei wrote: But actually the local interrupt is still disabled during completion because we need to convert spin_lock in all current usb drivers to spin_lock_irqsave() first. I posted lot of patches to do the conversion, but unfortunately most of them aren't merged. You should resubmit those patches after the end of the upcoming merge window. They probably will be accepted. 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: questions about give back urb in tasklet
hi Ming: One approach I like to use is trace event. Would you please let us how to to use trace event for calculate max/average usb irq time? You can enable irq_handler_entry and irq_handler_exit event, then write a script easily to figure out the time consumed in ehci irq handler, see Documentation/trace/events.txt of linux kernel for detailed steps. I find it ^^ But actually the local interrupt is still disabled during completion because we need to convert spin_lock in all current usb drivers to spin_lock_irqsave() first. Would you please let me know the function or source you mentioned? The local interrupt is disabled during completion handler in __usb_hcd_giveback_urb(). BTW, why I cannot see xhci add HCD_BH? There might be some reasons: - in previous following up discussion, Alan thought it should be easier to just split the hcd interrupt handler into two parts - as I said, spin_lock() isn't converted to spin_lock_irqsave() in drivers' completion handler yet Do you mean , take uvc/uac for example, if we put completion what they register previously in BH, the spin_lock() in their completions should be changed to spin_lock_irqsave()? Is it due to original completion is called in irq context? - maybe no one really care the time in which local interrupt is disabled by USB completion handler.(That might be true because USB devices are commonly used in personal computer, not in big server product) is there some reason xhci cannot put completion in BH? IMO, the completion handler should be put into tasklet, and can be done. If you like, you can take time to do that. Below is so far kernel implementation. Isn't it put completion handler into tasklet? ... .. spin_lock(bh-lock); list_add_tail(urb-urb_list, bh-head); running = bh-running; spin_unlock(bh-lock); if (running) ; else if (high_prio_bh) tasklet_hi_schedule(bh-bh); else tasklet_schedule(bh-bh); Thanks for your help, -- 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: questions about give back urb in tasklet
On Sun, 16 Mar 2014, vichy wrote: hi all: recently we bump to system performance issue when usb irq take quite long. I found below link for discussing how to short http://permalink.gmane.org/gmane.linux.usb.general/89363 I have some questions about this patch. Instead of just posting your questions to the mailing list, why don't you ask the person who wrote that patch? 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
questions about give back urb in tasklet
hi all: recently we bump to system performance issue when usb irq take quite long. I found below link for discussing how to short http://permalink.gmane.org/gmane.linux.usb.general/89363 I have some questions about this patch. 1. is there patch or kernel config I can use to measure man/avage usb irq time consuming like the above link show 2. I see this patch is roll back in commit c04ee4b1136e462722567cf6e76bb35a181574a7 and intend to be ready in 3.13-rc1 Is there special reason why we need to roll back? appreciate your help in advance, -- 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
