From: "Du, Changbin" <changbin.du@xxxxxxxxx> Add debugobject support to track the life time of struct urb. This feature help us detect violation of urb operations by generating a warning message from debugobject core. And we fix the possible issues at runtime to avoid oops if we can. I have done some tests with some class drivers, no violation found in them which is good. Expect this feature can be used for debugging future problems. Signed-off-by: Du, Changbin <changbin.du@xxxxxxxxx> --- drivers/usb/core/hcd.c | 1 + drivers/usb/core/urb.c | 117 +++++++++++++++++++++++++++++++++++++++++++++++-- include/linux/usb.h | 8 ++++ lib/Kconfig.debug | 8 ++++ 4 files changed, 130 insertions(+), 4 deletions(-) diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c index 34b837a..a8ea128 100644 --- a/drivers/usb/core/hcd.c +++ b/drivers/usb/core/hcd.c @@ -1750,6 +1750,7 @@ static void __usb_hcd_giveback_urb(struct urb *urb) /* pass ownership to the completion handler */ urb->status = status; + debug_urb_deactivate(urb); /* * We disable local IRQs here avoid possible deadlock because * drivers may call spin_lock() to hold lock which might be diff --git a/drivers/usb/core/urb.c b/drivers/usb/core/urb.c index c601e25..0d1eccb 100644 --- a/drivers/usb/core/urb.c +++ b/drivers/usb/core/urb.c @@ -10,6 +10,100 @@ #define to_urb(d) container_of(d, struct urb, kref) +#ifdef CONFIG_DEBUG_OBJECTS_URB +static struct debug_obj_descr urb_debug_descr; + +static void *urb_debug_hint(void *addr) +{ + return ((struct urb *) addr)->complete; +} + +/* + * fixup_init is called when: + * - an active object is initialized + */ +static bool urb_fixup_init(void *addr, enum debug_obj_state state) +{ + struct urb *urb = addr; + + switch (state) { + case ODEBUG_STATE_ACTIVE: + usb_kill_urb(urb); + debug_object_init(urb, &urb_debug_descr); + return true; + default: + return false; + } +} + +/* + * fixup_activate is called when: + * - an active object is activated + * - an unknown non-static object is activated + */ +static bool urb_fixup_activate(void *addr, enum debug_obj_state state) +{ + struct urb *urb = urb; + + switch (state) { + case ODEBUG_STATE_ACTIVE: + usb_kill_urb(urb); + debug_object_activate(urb, &urb_debug_descr); + return true; + default: + return false; + } +} + +/* + * fixup_free is called when: + * - an active object is freed + */ +static bool urb_fixup_free(void *addr, enum debug_obj_state state) +{ + struct urb *urb = addr; + + switch (state) { + case ODEBUG_STATE_ACTIVE: + usb_kill_urb(urb); + debug_object_free(urb, &urb_debug_descr); + return true; + default: + return false; + } +} + +static struct debug_obj_descr urb_debug_descr = { + .name = "urb", + .debug_hint = urb_debug_hint, + .fixup_init = urb_fixup_init, + .fixup_activate = urb_fixup_activate, + .fixup_free = urb_fixup_free, +}; + +static void debug_urb_init(struct urb *urb) +{ + /** + * The struct urb structure must never be + * created statically, so no init object + * on stack case. + */ + debug_object_init(urb, &urb_debug_descr); +} + +int debug_urb_activate(struct urb *urb) +{ + return debug_object_activate(urb, &urb_debug_descr); +} + +void debug_urb_deactivate(struct urb *urb) +{ + debug_object_deactivate(urb, &urb_debug_descr); +} + +#else +static inline void debug_urb_init(struct urb *urb) { } +#endif static void urb_destroy(struct kref *kref) { @@ -41,6 +135,7 @@ void usb_init_urb(struct urb *urb) memset(urb, 0, sizeof(*urb)); kref_init(&urb->kref); INIT_LIST_HEAD(&urb->anchor_list); + debug_urb_init(urb); } } EXPORT_SYMBOL_GPL(usb_init_urb); @@ -331,6 +426,7 @@ int usb_submit_urb(struct urb *urb, gfp_t mem_flags) struct usb_host_endpoint *ep; int is_out; unsigned int allowed; + int ret; if (!urb || !urb->complete) return -EINVAL; @@ -539,10 +635,23 @@ int usb_submit_urb(struct urb *urb, gfp_t mem_flags) } } - return usb_hcd_submit_urb(urb, mem_flags); + ret = debug_urb_activate(urb); + if (ret) + return ret; + ret = usb_hcd_submit_urb(urb, mem_flags); + if (ret) + debug_urb_deactivate(urb); + + return ret; } EXPORT_SYMBOL_GPL(usb_submit_urb); +static inline int __usb_unlink_urb(struct urb *urb, int status) +{ + debug_urb_deactivate(urb); + return usb_hcd_unlink_urb(urb, status); +} + /*-------------------------------------------------------------------*/ /** @@ -626,7 +735,7 @@ int usb_unlink_urb(struct urb *urb) return -ENODEV; if (!urb->ep) return -EIDRM; - return usb_hcd_unlink_urb(urb, -ECONNRESET); + return __usb_unlink_urb(urb, -ECONNRESET); } EXPORT_SYMBOL_GPL(usb_unlink_urb); @@ -664,7 +773,7 @@ void usb_kill_urb(struct urb *urb) return; atomic_inc(&urb->reject); - usb_hcd_unlink_urb(urb, -ENOENT); + __usb_unlink_urb(urb, -ENOENT); wait_event(usb_kill_urb_queue, atomic_read(&urb->use_count) == 0); atomic_dec(&urb->reject); @@ -708,7 +817,7 @@ void usb_poison_urb(struct urb *urb) if (!urb->dev || !urb->ep) return; - usb_hcd_unlink_urb(urb, -ENOENT); + __usb_unlink_urb(urb, -ENOENT); wait_event(usb_kill_urb_queue, atomic_read(&urb->use_count) == 0); } EXPORT_SYMBOL_GPL(usb_poison_urb); diff --git a/include/linux/usb.h b/include/linux/usb.h index eba1f10..16f2dee 100644 --- a/include/linux/usb.h +++ b/include/linux/usb.h @@ -1606,6 +1606,14 @@ static inline void usb_fill_int_urb(struct urb *urb, urb->start_frame = -1; } +#ifdef CONFIG_DEBUG_OBJECTS_URB +extern int debug_urb_activate(struct urb *urb); +extern void debug_urb_deactivate(struct urb *urb); +#else +static inline int debug_urb_activate(struct urb *urb) { return 0; } +static inline void debug_urb_deactivate(struct urb *urb) { } +#endif + extern void usb_init_urb(struct urb *urb); extern struct urb *usb_alloc_urb(int iso_packets, gfp_t mem_flags); extern void usb_free_urb(struct urb *urb); diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug index e707ab3..4f5bd59 100644 --- a/lib/Kconfig.debug +++ b/lib/Kconfig.debug @@ -458,6 +458,14 @@ config DEBUG_OBJECTS_PERCPU_COUNTER percpu counter routines to track the life time of percpu counter objects and validate the percpu counter operations. +config DEBUG_OBJECTS_URB + bool "Debug usb urb objects" + depends on DEBUG_OBJECTS + help + If you say Y here, additional code will be inserted into the + usb core routines to track the life time of urb objects and + validate the urb operations. + config DEBUG_OBJECTS_ENABLE_DEFAULT int "debug_objects bootup default value (0-1)" range 0 1 -- 2.7.4 -- 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