All of lore.kernel.org
 help / color / mirror / Atom feed
From: changbin.du@intel.com
To: gregkh@linuxfoundation.org
Cc: mathias.nyman@linux.intel.com, stefan.koch10@gmail.com,
	hkallweit1@gmail.com, sergei.shtylyov@cogentembedded.com,
	jonas.hesselmann@hotmail.de, mingo@kernel.org,
	paulmck@linux.vnet.ibm.com, akpm@linux-foundation.org,
	tglx@linutronix.de, keescook@chromium.org,
	dan.j.williams@intel.com, aryabinin@virtuozzo.com, tj@kernel.org,
	dave@stgolabs.net, nikolay@cumulusnetworks.com,
	dvyukov@google.com, adrien+dev@schischi.me,
	linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, "Du,
	Changbin" <changbin.du@intel.com>
Subject: [PATCH] usb: core: add debugobjects support for urb object
Date: Tue, 24 May 2016 15:53:53 +0800	[thread overview]
Message-ID: <1464076433-3284-1-git-send-email-changbin.du@intel.com> (raw)

From: "Du, Changbin" <changbin.du@intel.com>

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@intel.com>
---
 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

             reply	other threads:[~2016-05-24  8:06 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-24  7:53 changbin.du [this message]
2016-05-24 14:23 ` [PATCH] usb: core: add debugobjects support for urb object Alan Stern
2016-05-24 14:44 ` Greg KH
2016-05-26  2:15   ` Du, Changbin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1464076433-3284-1-git-send-email-changbin.du@intel.com \
    --to=changbin.du@intel.com \
    --cc=adrien+dev@schischi.me \
    --cc=akpm@linux-foundation.org \
    --cc=aryabinin@virtuozzo.com \
    --cc=dan.j.williams@intel.com \
    --cc=dave@stgolabs.net \
    --cc=dvyukov@google.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=hkallweit1@gmail.com \
    --cc=jonas.hesselmann@hotmail.de \
    --cc=keescook@chromium.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=mathias.nyman@linux.intel.com \
    --cc=mingo@kernel.org \
    --cc=nikolay@cumulusnetworks.com \
    --cc=paulmck@linux.vnet.ibm.com \
    --cc=sergei.shtylyov@cogentembedded.com \
    --cc=stefan.koch10@gmail.com \
    --cc=tglx@linutronix.de \
    --cc=tj@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.