[PATCH] USB: move usb debugfs directory creation to the usb common core

[PATCH] USB: move usb debugfs directory creation to the usb common core

[Greg Kroah-Hartman]

The USB gadget subsystem wants to use the USB debugfs root directory, so
move it to the common "core" USB code so that it is properly initialized
and removed as needed.

Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---

This should be the "correct" version of this, Chunfeng, can you test
this to verify it works for you?


diff --git a/drivers/usb/common/common.c b/drivers/usb/common/common.c
index 18f5dcf58b0d..3b5e4263ffef 100644
--- a/drivers/usb/common/common.c
+++ b/drivers/usb/common/common.c
@@ -15,6 +15,7 @@
 #include <linux/usb/of.h>
 #include <linux/usb/otg.h>
 #include <linux/of_platform.h>
+#include <linux/debugfs.h>
 
 static const char *const ep_type_names[] = {
 	[USB_ENDPOINT_XFER_CONTROL] = "ctrl",
@@ -291,4 +292,21 @@ struct device *usb_of_get_companion_dev(struct device *dev)
 EXPORT_SYMBOL_GPL(usb_of_get_companion_dev);
 #endif
 
+struct dentry *usb_debug_root;
+EXPORT_SYMBOL_GPL(usb_debug_root);
+
+static int usb_common_init(void)
+{
+	usb_debug_root = debugfs_create_dir("usb", NULL);
+	return 0;
+}
+
+static void usb_common_exit(void)
+{
+	debugfs_remove_recursive(usb_debug_root);
+}
+
+module_init(usb_common_init);
+module_exit(usb_common_exit);
+
 MODULE_LICENSE("GPL");
diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
index 7fcb9f782931..f3d6b1ab80cb 100644
--- a/drivers/usb/core/usb.c
+++ b/drivers/usb/core/usb.c
@@ -1185,19 +1185,17 @@ static struct notifier_block usb_bus_nb = {
 	.notifier_call = usb_bus_notify,
 };
 
-struct dentry *usb_debug_root;
-EXPORT_SYMBOL_GPL(usb_debug_root);
+static struct dentry *usb_devices_root;
 
 static void usb_debugfs_init(void)
 {
-	usb_debug_root = debugfs_create_dir("usb", NULL);
-	debugfs_create_file("devices", 0444, usb_debug_root, NULL,
-			    &usbfs_devices_fops);
+	usb_devices_root = debugfs_create_file("devices", 0444, usb_debug_root,
+					       NULL, &usbfs_devices_fops);
 }
 
 static void usb_debugfs_cleanup(void)
 {
-	debugfs_remove_recursive(usb_debug_root);
+	debugfs_remove_recursive(usb_devices_root);
 }
 
 /*

[PATCH] USB: move usb debugfs directory creation to the usb common core

[Greg Kroah-Hartman]

The USB gadget subsystem wants to use the USB debugfs root directory, so
move it to the common "core" USB code so that it is properly initialized
and removed as needed.

Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---

This should be the "correct" version of this, Chunfeng, can you test
this to verify it works for you?


diff --git a/drivers/usb/common/common.c b/drivers/usb/common/common.c
index 18f5dcf58b0d..3b5e4263ffef 100644
--- a/drivers/usb/common/common.c
+++ b/drivers/usb/common/common.c
@@ -15,6 +15,7 @@
 #include <linux/usb/of.h>
 #include <linux/usb/otg.h>
 #include <linux/of_platform.h>
+#include <linux/debugfs.h>
 
 static const char *const ep_type_names[] = {
 	[USB_ENDPOINT_XFER_CONTROL] = "ctrl",
@@ -291,4 +292,21 @@ struct device *usb_of_get_companion_dev(struct device *dev)
 EXPORT_SYMBOL_GPL(usb_of_get_companion_dev);
 #endif
 
+struct dentry *usb_debug_root;
+EXPORT_SYMBOL_GPL(usb_debug_root);
+
+static int usb_common_init(void)
+{
+	usb_debug_root = debugfs_create_dir("usb", NULL);
+	return 0;
+}
+
+static void usb_common_exit(void)
+{
+	debugfs_remove_recursive(usb_debug_root);
+}
+
+module_init(usb_common_init);
+module_exit(usb_common_exit);
+
 MODULE_LICENSE("GPL");
diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
index 7fcb9f782931..f3d6b1ab80cb 100644
--- a/drivers/usb/core/usb.c
+++ b/drivers/usb/core/usb.c
@@ -1185,19 +1185,17 @@ static struct notifier_block usb_bus_nb = {
 	.notifier_call = usb_bus_notify,
 };
 
-struct dentry *usb_debug_root;
-EXPORT_SYMBOL_GPL(usb_debug_root);
+static struct dentry *usb_devices_root;
 
 static void usb_debugfs_init(void)
 {
-	usb_debug_root = debugfs_create_dir("usb", NULL);
-	debugfs_create_file("devices", 0444, usb_debug_root, NULL,
-			    &usbfs_devices_fops);
+	usb_devices_root = debugfs_create_file("devices", 0444, usb_debug_root,
+					       NULL, &usbfs_devices_fops);
 }
 
 static void usb_debugfs_cleanup(void)
 {
-	debugfs_remove_recursive(usb_debug_root);
+	debugfs_remove_recursive(usb_devices_root);
 }
 
 /*

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] USB: move usb debugfs directory creation to the usb common core

[Greg Kroah-Hartman]

On Tue, Jun 04, 2019 at 11:32:58AM +0200, Greg Kroah-Hartman wrote:
> The USB gadget subsystem wants to use the USB debugfs root directory, so
> move it to the common "core" USB code so that it is properly initialized
> and removed as needed.
> 
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> 
> ---
> 
> This should be the "correct" version of this, Chunfeng, can you test
> this to verify it works for you?
> 
> 
> diff --git a/drivers/usb/common/common.c b/drivers/usb/common/common.c
> index 18f5dcf58b0d..3b5e4263ffef 100644
> --- a/drivers/usb/common/common.c
> +++ b/drivers/usb/common/common.c
> @@ -15,6 +15,7 @@
>  #include <linux/usb/of.h>
>  #include <linux/usb/otg.h>
>  #include <linux/of_platform.h>
> +#include <linux/debugfs.h>
>  
>  static const char *const ep_type_names[] = {
>  	[USB_ENDPOINT_XFER_CONTROL] = "ctrl",
> @@ -291,4 +292,21 @@ struct device *usb_of_get_companion_dev(struct device *dev)
>  EXPORT_SYMBOL_GPL(usb_of_get_companion_dev);
>  #endif
>  
> +struct dentry *usb_debug_root;
> +EXPORT_SYMBOL_GPL(usb_debug_root);
> +
> +static int usb_common_init(void)
> +{
> +	usb_debug_root = debugfs_create_dir("usb", NULL);
> +	return 0;
> +}
> +
> +static void usb_common_exit(void)
> +{
> +	debugfs_remove_recursive(usb_debug_root);
> +}
> +
> +module_init(usb_common_init);
> +module_exit(usb_common_exit);
> +
>  MODULE_LICENSE("GPL");
> diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
> index 7fcb9f782931..f3d6b1ab80cb 100644
> --- a/drivers/usb/core/usb.c
> +++ b/drivers/usb/core/usb.c
> @@ -1185,19 +1185,17 @@ static struct notifier_block usb_bus_nb = {
>  	.notifier_call = usb_bus_notify,
>  };
>  
> -struct dentry *usb_debug_root;
> -EXPORT_SYMBOL_GPL(usb_debug_root);
> +static struct dentry *usb_devices_root;
>  
>  static void usb_debugfs_init(void)
>  {
> -	usb_debug_root = debugfs_create_dir("usb", NULL);
> -	debugfs_create_file("devices", 0444, usb_debug_root, NULL,
> -			    &usbfs_devices_fops);
> +	usb_devices_root = debugfs_create_file("devices", 0444, usb_debug_root,
> +					       NULL, &usbfs_devices_fops);
>  }
>  
>  static void usb_debugfs_cleanup(void)
>  {
> -	debugfs_remove_recursive(usb_debug_root);
> +	debugfs_remove_recursive(usb_devices_root);

That should just be debugfs_remove();

I'll fix it up after someone tests this :)

thanks,

greg k-h

Re: [PATCH] USB: move usb debugfs directory creation to the usb common core

[Greg Kroah-Hartman]

On Tue, Jun 04, 2019 at 11:32:58AM +0200, Greg Kroah-Hartman wrote:
> The USB gadget subsystem wants to use the USB debugfs root directory, so
> move it to the common "core" USB code so that it is properly initialized
> and removed as needed.
> 
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> 
> ---
> 
> This should be the "correct" version of this, Chunfeng, can you test
> this to verify it works for you?
> 
> 
> diff --git a/drivers/usb/common/common.c b/drivers/usb/common/common.c
> index 18f5dcf58b0d..3b5e4263ffef 100644
> --- a/drivers/usb/common/common.c
> +++ b/drivers/usb/common/common.c
> @@ -15,6 +15,7 @@
>  #include <linux/usb/of.h>
>  #include <linux/usb/otg.h>
>  #include <linux/of_platform.h>
> +#include <linux/debugfs.h>
>  
>  static const char *const ep_type_names[] = {
>  	[USB_ENDPOINT_XFER_CONTROL] = "ctrl",
> @@ -291,4 +292,21 @@ struct device *usb_of_get_companion_dev(struct device *dev)
>  EXPORT_SYMBOL_GPL(usb_of_get_companion_dev);
>  #endif
>  
> +struct dentry *usb_debug_root;
> +EXPORT_SYMBOL_GPL(usb_debug_root);
> +
> +static int usb_common_init(void)
> +{
> +	usb_debug_root = debugfs_create_dir("usb", NULL);
> +	return 0;
> +}
> +
> +static void usb_common_exit(void)
> +{
> +	debugfs_remove_recursive(usb_debug_root);
> +}
> +
> +module_init(usb_common_init);
> +module_exit(usb_common_exit);
> +
>  MODULE_LICENSE("GPL");
> diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
> index 7fcb9f782931..f3d6b1ab80cb 100644
> --- a/drivers/usb/core/usb.c
> +++ b/drivers/usb/core/usb.c
> @@ -1185,19 +1185,17 @@ static struct notifier_block usb_bus_nb = {
>  	.notifier_call = usb_bus_notify,
>  };
>  
> -struct dentry *usb_debug_root;
> -EXPORT_SYMBOL_GPL(usb_debug_root);
> +static struct dentry *usb_devices_root;
>  
>  static void usb_debugfs_init(void)
>  {
> -	usb_debug_root = debugfs_create_dir("usb", NULL);
> -	debugfs_create_file("devices", 0444, usb_debug_root, NULL,
> -			    &usbfs_devices_fops);
> +	usb_devices_root = debugfs_create_file("devices", 0444, usb_debug_root,
> +					       NULL, &usbfs_devices_fops);
>  }
>  
>  static void usb_debugfs_cleanup(void)
>  {
> -	debugfs_remove_recursive(usb_debug_root);
> +	debugfs_remove_recursive(usb_devices_root);

That should just be debugfs_remove();

I'll fix it up after someone tests this :)

thanks,

greg k-h

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] USB: move usb debugfs directory creation to the usb common core

[Felipe Balbi]

Hi,

Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
> diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
> index 7fcb9f782931..f3d6b1ab80cb 100644
> --- a/drivers/usb/core/usb.c
> +++ b/drivers/usb/core/usb.c
> @@ -1185,19 +1185,17 @@ static struct notifier_block usb_bus_nb = {
>  	.notifier_call = usb_bus_notify,
>  };
>  
> -struct dentry *usb_debug_root;
> -EXPORT_SYMBOL_GPL(usb_debug_root);
> +static struct dentry *usb_devices_root;
>  
>  static void usb_debugfs_init(void)
>  {
> -	usb_debug_root = debugfs_create_dir("usb", NULL);
> -	debugfs_create_file("devices", 0444, usb_debug_root, NULL,
> -			    &usbfs_devices_fops);
> +	usb_devices_root = debugfs_create_file("devices", 0444, usb_debug_root,

don't we have a race now? Can usbcore ever probe before usb common?

-- 
balbi

Re: [PATCH] USB: move usb debugfs directory creation to the usb common core

[Felipe Balbi]

Hi,

Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
> diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
> index 7fcb9f782931..f3d6b1ab80cb 100644
> --- a/drivers/usb/core/usb.c
> +++ b/drivers/usb/core/usb.c
> @@ -1185,19 +1185,17 @@ static struct notifier_block usb_bus_nb = {
>  	.notifier_call = usb_bus_notify,
>  };
>  
> -struct dentry *usb_debug_root;
> -EXPORT_SYMBOL_GPL(usb_debug_root);
> +static struct dentry *usb_devices_root;
>  
>  static void usb_debugfs_init(void)
>  {
> -	usb_debug_root = debugfs_create_dir("usb", NULL);
> -	debugfs_create_file("devices", 0444, usb_debug_root, NULL,
> -			    &usbfs_devices_fops);
> +	usb_devices_root = debugfs_create_file("devices", 0444, usb_debug_root,

don't we have a race now? Can usbcore ever probe before usb common?

-- 
balbi

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] USB: move usb debugfs directory creation to the usb common core

[Greg Kroah-Hartman]

On Tue, Jun 04, 2019 at 03:25:14PM +0300, Felipe Balbi wrote:
> 
> Hi,
> 
> Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
> > diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
> > index 7fcb9f782931..f3d6b1ab80cb 100644
> > --- a/drivers/usb/core/usb.c
> > +++ b/drivers/usb/core/usb.c
> > @@ -1185,19 +1185,17 @@ static struct notifier_block usb_bus_nb = {
> >  	.notifier_call = usb_bus_notify,
> >  };
> >  
> > -struct dentry *usb_debug_root;
> > -EXPORT_SYMBOL_GPL(usb_debug_root);
> > +static struct dentry *usb_devices_root;
> >  
> >  static void usb_debugfs_init(void)
> >  {
> > -	usb_debug_root = debugfs_create_dir("usb", NULL);
> > -	debugfs_create_file("devices", 0444, usb_debug_root, NULL,
> > -			    &usbfs_devices_fops);
> > +	usb_devices_root = debugfs_create_file("devices", 0444, usb_debug_root,
> 
> don't we have a race now? Can usbcore ever probe before usb common?

How can that happen if usb_debug_root is in usb common?  The module
loader will not let that happen.  Or it shouldn't :)

thanks,

greg k-h

Re: [PATCH] USB: move usb debugfs directory creation to the usb common core

[Greg Kroah-Hartman]

On Tue, Jun 04, 2019 at 03:25:14PM +0300, Felipe Balbi wrote:
> 
> Hi,
> 
> Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
> > diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
> > index 7fcb9f782931..f3d6b1ab80cb 100644
> > --- a/drivers/usb/core/usb.c
> > +++ b/drivers/usb/core/usb.c
> > @@ -1185,19 +1185,17 @@ static struct notifier_block usb_bus_nb = {
> >  	.notifier_call = usb_bus_notify,
> >  };
> >  
> > -struct dentry *usb_debug_root;
> > -EXPORT_SYMBOL_GPL(usb_debug_root);
> > +static struct dentry *usb_devices_root;
> >  
> >  static void usb_debugfs_init(void)
> >  {
> > -	usb_debug_root = debugfs_create_dir("usb", NULL);
> > -	debugfs_create_file("devices", 0444, usb_debug_root, NULL,
> > -			    &usbfs_devices_fops);
> > +	usb_devices_root = debugfs_create_file("devices", 0444, usb_debug_root,
> 
> don't we have a race now? Can usbcore ever probe before usb common?

How can that happen if usb_debug_root is in usb common?  The module
loader will not let that happen.  Or it shouldn't :)

thanks,

greg k-h

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] USB: move usb debugfs directory creation to the usb common core

[Chunfeng Yun]

On Tue, 2019-06-04 at 13:59 +0200, Greg Kroah-Hartman wrote:
> On Tue, Jun 04, 2019 at 11:32:58AM +0200, Greg Kroah-Hartman wrote:
> > The USB gadget subsystem wants to use the USB debugfs root directory, so
> > move it to the common "core" USB code so that it is properly initialized
> > and removed as needed.
> > 
> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > 
> > ---
> > 
> > This should be the "correct" version of this, Chunfeng, can you test
> > this to verify it works for you?
I'll test it ASAP, thanks a lot

> > 
> > 
> > diff --git a/drivers/usb/common/common.c b/drivers/usb/common/common.c
> > index 18f5dcf58b0d..3b5e4263ffef 100644
> > --- a/drivers/usb/common/common.c
> > +++ b/drivers/usb/common/common.c
> > @@ -15,6 +15,7 @@
> >  #include <linux/usb/of.h>
> >  #include <linux/usb/otg.h>
> >  #include <linux/of_platform.h>
> > +#include <linux/debugfs.h>
> >  
> >  static const char *const ep_type_names[] = {
> >  	[USB_ENDPOINT_XFER_CONTROL] = "ctrl",
> > @@ -291,4 +292,21 @@ struct device *usb_of_get_companion_dev(struct device *dev)
> >  EXPORT_SYMBOL_GPL(usb_of_get_companion_dev);
> >  #endif
> >  
> > +struct dentry *usb_debug_root;
> > +EXPORT_SYMBOL_GPL(usb_debug_root);
> > +
> > +static int usb_common_init(void)
> > +{
> > +	usb_debug_root = debugfs_create_dir("usb", NULL);
> > +	return 0;
> > +}
> > +
> > +static void usb_common_exit(void)
> > +{
> > +	debugfs_remove_recursive(usb_debug_root);
> > +}
> > +
> > +module_init(usb_common_init);
> > +module_exit(usb_common_exit);
> > +
> >  MODULE_LICENSE("GPL");
> > diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
> > index 7fcb9f782931..f3d6b1ab80cb 100644
> > --- a/drivers/usb/core/usb.c
> > +++ b/drivers/usb/core/usb.c
> > @@ -1185,19 +1185,17 @@ static struct notifier_block usb_bus_nb = {
> >  	.notifier_call = usb_bus_notify,
> >  };
> >  
> > -struct dentry *usb_debug_root;
> > -EXPORT_SYMBOL_GPL(usb_debug_root);
> > +static struct dentry *usb_devices_root;
> >  
> >  static void usb_debugfs_init(void)
> >  {
> > -	usb_debug_root = debugfs_create_dir("usb", NULL);
> > -	debugfs_create_file("devices", 0444, usb_debug_root, NULL,
> > -			    &usbfs_devices_fops);
> > +	usb_devices_root = debugfs_create_file("devices", 0444, usb_debug_root,
> > +					       NULL, &usbfs_devices_fops);
> >  }
> >  
> >  static void usb_debugfs_cleanup(void)
> >  {
> > -	debugfs_remove_recursive(usb_debug_root);
> > +	debugfs_remove_recursive(usb_devices_root);
> 
> That should just be debugfs_remove();
> 
> I'll fix it up after someone tests this :)
> 
> thanks,
> 
> greg k-h

Re: [PATCH] USB: move usb debugfs directory creation to the usb common core

[Chunfeng Yun]

On Tue, 2019-06-04 at 13:59 +0200, Greg Kroah-Hartman wrote:
> On Tue, Jun 04, 2019 at 11:32:58AM +0200, Greg Kroah-Hartman wrote:
> > The USB gadget subsystem wants to use the USB debugfs root directory, so
> > move it to the common "core" USB code so that it is properly initialized
> > and removed as needed.
> > 
> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > 
> > ---
> > 
> > This should be the "correct" version of this, Chunfeng, can you test
> > this to verify it works for you?
I'll test it ASAP, thanks a lot

> > 
> > 
> > diff --git a/drivers/usb/common/common.c b/drivers/usb/common/common.c
> > index 18f5dcf58b0d..3b5e4263ffef 100644
> > --- a/drivers/usb/common/common.c
> > +++ b/drivers/usb/common/common.c
> > @@ -15,6 +15,7 @@
> >  #include <linux/usb/of.h>
> >  #include <linux/usb/otg.h>
> >  #include <linux/of_platform.h>
> > +#include <linux/debugfs.h>
> >  
> >  static const char *const ep_type_names[] = {
> >  	[USB_ENDPOINT_XFER_CONTROL] = "ctrl",
> > @@ -291,4 +292,21 @@ struct device *usb_of_get_companion_dev(struct device *dev)
> >  EXPORT_SYMBOL_GPL(usb_of_get_companion_dev);
> >  #endif
> >  
> > +struct dentry *usb_debug_root;
> > +EXPORT_SYMBOL_GPL(usb_debug_root);
> > +
> > +static int usb_common_init(void)
> > +{
> > +	usb_debug_root = debugfs_create_dir("usb", NULL);
> > +	return 0;
> > +}
> > +
> > +static void usb_common_exit(void)
> > +{
> > +	debugfs_remove_recursive(usb_debug_root);
> > +}
> > +
> > +module_init(usb_common_init);
> > +module_exit(usb_common_exit);
> > +
> >  MODULE_LICENSE("GPL");
> > diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
> > index 7fcb9f782931..f3d6b1ab80cb 100644
> > --- a/drivers/usb/core/usb.c
> > +++ b/drivers/usb/core/usb.c
> > @@ -1185,19 +1185,17 @@ static struct notifier_block usb_bus_nb = {
> >  	.notifier_call = usb_bus_notify,
> >  };
> >  
> > -struct dentry *usb_debug_root;
> > -EXPORT_SYMBOL_GPL(usb_debug_root);
> > +static struct dentry *usb_devices_root;
> >  
> >  static void usb_debugfs_init(void)
> >  {
> > -	usb_debug_root = debugfs_create_dir("usb", NULL);
> > -	debugfs_create_file("devices", 0444, usb_debug_root, NULL,
> > -			    &usbfs_devices_fops);
> > +	usb_devices_root = debugfs_create_file("devices", 0444, usb_debug_root,
> > +					       NULL, &usbfs_devices_fops);
> >  }
> >  
> >  static void usb_debugfs_cleanup(void)
> >  {
> > -	debugfs_remove_recursive(usb_debug_root);
> > +	debugfs_remove_recursive(usb_devices_root);
> 
> That should just be debugfs_remove();
> 
> I'll fix it up after someone tests this :)
> 
> thanks,
> 
> greg k-h

Re: [PATCH] USB: move usb debugfs directory creation to the usb common core

[Chunfeng Yun]

On Tue, 2019-06-04 at 13:59 +0200, Greg Kroah-Hartman wrote:
> On Tue, Jun 04, 2019 at 11:32:58AM +0200, Greg Kroah-Hartman wrote:
> > The USB gadget subsystem wants to use the USB debugfs root directory, so
> > move it to the common "core" USB code so that it is properly initialized
> > and removed as needed.
> > 
> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > 
> > ---
> > 
> > This should be the "correct" version of this, Chunfeng, can you test
> > this to verify it works for you?
I'll test it ASAP, thanks a lot

> > 
> > 
> > diff --git a/drivers/usb/common/common.c b/drivers/usb/common/common.c
> > index 18f5dcf58b0d..3b5e4263ffef 100644
> > --- a/drivers/usb/common/common.c
> > +++ b/drivers/usb/common/common.c
> > @@ -15,6 +15,7 @@
> >  #include <linux/usb/of.h>
> >  #include <linux/usb/otg.h>
> >  #include <linux/of_platform.h>
> > +#include <linux/debugfs.h>
> >  
> >  static const char *const ep_type_names[] = {
> >  	[USB_ENDPOINT_XFER_CONTROL] = "ctrl",
> > @@ -291,4 +292,21 @@ struct device *usb_of_get_companion_dev(struct device *dev)
> >  EXPORT_SYMBOL_GPL(usb_of_get_companion_dev);
> >  #endif
> >  
> > +struct dentry *usb_debug_root;
> > +EXPORT_SYMBOL_GPL(usb_debug_root);
> > +
> > +static int usb_common_init(void)
> > +{
> > +	usb_debug_root = debugfs_create_dir("usb", NULL);
> > +	return 0;
> > +}
> > +
> > +static void usb_common_exit(void)
> > +{
> > +	debugfs_remove_recursive(usb_debug_root);
> > +}
> > +
> > +module_init(usb_common_init);
> > +module_exit(usb_common_exit);
> > +
> >  MODULE_LICENSE("GPL");
> > diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
> > index 7fcb9f782931..f3d6b1ab80cb 100644
> > --- a/drivers/usb/core/usb.c
> > +++ b/drivers/usb/core/usb.c
> > @@ -1185,19 +1185,17 @@ static struct notifier_block usb_bus_nb = {
> >  	.notifier_call = usb_bus_notify,
> >  };
> >  
> > -struct dentry *usb_debug_root;
> > -EXPORT_SYMBOL_GPL(usb_debug_root);
> > +static struct dentry *usb_devices_root;
> >  
> >  static void usb_debugfs_init(void)
> >  {
> > -	usb_debug_root = debugfs_create_dir("usb", NULL);
> > -	debugfs_create_file("devices", 0444, usb_debug_root, NULL,
> > -			    &usbfs_devices_fops);
> > +	usb_devices_root = debugfs_create_file("devices", 0444, usb_debug_root,
> > +					       NULL, &usbfs_devices_fops);
> >  }
> >  
> >  static void usb_debugfs_cleanup(void)
> >  {
> > -	debugfs_remove_recursive(usb_debug_root);
> > +	debugfs_remove_recursive(usb_devices_root);
> 
> That should just be debugfs_remove();
> 
> I'll fix it up after someone tests this :)
> 
> thanks,
> 
> greg k-h



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] USB: move usb debugfs directory creation to the usb common core

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 1511 bytes --]

Hi Greg,

I love your patch! Yet something to improve:

[auto build test ERROR on usb/usb-testing]
[also build test ERROR on v5.2-rc3 next-20190604]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Greg-Kroah-Hartman/USB-move-usb-debugfs-directory-creation-to-the-usb-common-core/20190605-114326
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
config: sh-allmodconfig (attached as .config)
compiler: sh4-linux-gcc (GCC) 7.4.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.4.0 make.cross ARCH=sh 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/usb/common/led.o: In function `ledtrig_usb_exit':
>> led.c:(.exit.text+0x0): multiple definition of `cleanup_module'
   drivers/usb/common/common.o:common.c:(.text+0x518): first defined here
   drivers/usb/common/led.o: In function `ledtrig_usb_init':
>> led.c:(.init.text+0x0): multiple definition of `init_module'
   drivers/usb/common/common.o:common.c:(.text+0x4dc): first defined here

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 52829 bytes --]

Re: [PATCH] USB: move usb debugfs directory creation to the usb common core

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 1511 bytes --]

Hi Greg,

I love your patch! Yet something to improve:

[auto build test ERROR on usb/usb-testing]
[also build test ERROR on v5.2-rc3 next-20190604]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Greg-Kroah-Hartman/USB-move-usb-debugfs-directory-creation-to-the-usb-common-core/20190605-114326
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
config: sh-allmodconfig (attached as .config)
compiler: sh4-linux-gcc (GCC) 7.4.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.4.0 make.cross ARCH=sh 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/usb/common/led.o: In function `ledtrig_usb_exit':
>> led.c:(.exit.text+0x0): multiple definition of `cleanup_module'
   drivers/usb/common/common.o:common.c:(.text+0x518): first defined here
   drivers/usb/common/led.o: In function `ledtrig_usb_init':
>> led.c:(.init.text+0x0): multiple definition of `init_module'
   drivers/usb/common/common.o:common.c:(.text+0x4dc): first defined here

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 52829 bytes --]

[-- Attachment #3: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] USB: move usb debugfs directory creation to the usb common core

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1199 bytes --]


Hi,

Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
>> Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
>> > diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
>> > index 7fcb9f782931..f3d6b1ab80cb 100644
>> > --- a/drivers/usb/core/usb.c
>> > +++ b/drivers/usb/core/usb.c
>> > @@ -1185,19 +1185,17 @@ static struct notifier_block usb_bus_nb = {
>> >  	.notifier_call = usb_bus_notify,
>> >  };
>> >  
>> > -struct dentry *usb_debug_root;
>> > -EXPORT_SYMBOL_GPL(usb_debug_root);
>> > +static struct dentry *usb_devices_root;
>> >  
>> >  static void usb_debugfs_init(void)
>> >  {
>> > -	usb_debug_root = debugfs_create_dir("usb", NULL);
>> > -	debugfs_create_file("devices", 0444, usb_debug_root, NULL,
>> > -			    &usbfs_devices_fops);
>> > +	usb_devices_root = debugfs_create_file("devices", 0444, usb_debug_root,
>> 
>> don't we have a race now? Can usbcore ever probe before usb common?
>
> How can that happen if usb_debug_root is in usb common?  The module
> loader will not let that happen.  Or it shouldn't :)

argh, indeed. The very fact that usbcore tries to resolve usb_debug_root
already forces a dependency :-p

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: [PATCH] USB: move usb debugfs directory creation to the usb common core

[Felipe Balbi]

[-- Attachment #1.1: Type: text/plain, Size: 1199 bytes --]


Hi,

Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
>> Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
>> > diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
>> > index 7fcb9f782931..f3d6b1ab80cb 100644
>> > --- a/drivers/usb/core/usb.c
>> > +++ b/drivers/usb/core/usb.c
>> > @@ -1185,19 +1185,17 @@ static struct notifier_block usb_bus_nb = {
>> >  	.notifier_call = usb_bus_notify,
>> >  };
>> >  
>> > -struct dentry *usb_debug_root;
>> > -EXPORT_SYMBOL_GPL(usb_debug_root);
>> > +static struct dentry *usb_devices_root;
>> >  
>> >  static void usb_debugfs_init(void)
>> >  {
>> > -	usb_debug_root = debugfs_create_dir("usb", NULL);
>> > -	debugfs_create_file("devices", 0444, usb_debug_root, NULL,
>> > -			    &usbfs_devices_fops);
>> > +	usb_devices_root = debugfs_create_file("devices", 0444, usb_debug_root,
>> 
>> don't we have a race now? Can usbcore ever probe before usb common?
>
> How can that happen if usb_debug_root is in usb common?  The module
> loader will not let that happen.  Or it shouldn't :)

argh, indeed. The very fact that usbcore tries to resolve usb_debug_root
already forces a dependency :-p

-- 
balbi

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] USB: move usb debugfs directory creation to the usb common core

[Chunfeng Yun]

On Tue, 2019-06-04 at 13:59 +0200, Greg Kroah-Hartman wrote:
> On Tue, Jun 04, 2019 at 11:32:58AM +0200, Greg Kroah-Hartman wrote:
> > The USB gadget subsystem wants to use the USB debugfs root directory, so
> > move it to the common "core" USB code so that it is properly initialized
> > and removed as needed.
> > 
> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > 
> > ---
> > 
> > This should be the "correct" version of this, Chunfeng, can you test
> > this to verify it works for you?
> > 
> > 
> > diff --git a/drivers/usb/common/common.c b/drivers/usb/common/common.c
> > index 18f5dcf58b0d..3b5e4263ffef 100644
> > --- a/drivers/usb/common/common.c
> > +++ b/drivers/usb/common/common.c
> > @@ -15,6 +15,7 @@
> >  #include <linux/usb/of.h>
> >  #include <linux/usb/otg.h>
> >  #include <linux/of_platform.h>
> > +#include <linux/debugfs.h>
> >  
> >  static const char *const ep_type_names[] = {
> >  	[USB_ENDPOINT_XFER_CONTROL] = "ctrl",
> > @@ -291,4 +292,21 @@ struct device *usb_of_get_companion_dev(struct device *dev)
> >  EXPORT_SYMBOL_GPL(usb_of_get_companion_dev);
> >  #endif
> >  
> > +struct dentry *usb_debug_root;
> > +EXPORT_SYMBOL_GPL(usb_debug_root);
> > +
> > +static int usb_common_init(void)
> > +{
> > +	usb_debug_root = debugfs_create_dir("usb", NULL);
> > +	return 0;
> > +}
> > +
> > +static void usb_common_exit(void)
> > +{
> > +	debugfs_remove_recursive(usb_debug_root);
> > +}
> > +
> > +module_init(usb_common_init);
I tested this patch.

Here use module_init() indeed have a race as Felipe said before.
usbcore uses subsys_initcall(), and have a higher priority than
module_init(), so when usbcore tries to create "devices" file,
usb_debug_root is not created.

after I replace it by postcore_initcall() (debugfs uses
core_initcall()), test two cases:

1. buildin usbcore/udc-core

    "usb" directory is created, and usb/devices file is also created by
usbcore

2. build both usbcore and gadget as ko

    usbcore.ko, udc-core.ko and usb-common.ko are created. 

   2.1 
       insmod usb-common.ko   // "usb" directory is created
       insmod usb-core.ko   // usb/devices file is created

   2.2
       rmmod usb-common.ko  // failed, usb_common is in use by usb-core

   2.3 
       rmmod usb-core.ko   // usb/devices file is destroyed
       rmmod usb-common.ko  // usb directory is destroyed

   2.4 
       insmod usb-common.ko   // "usb" directory is created
       insmod udc-core.ko

   2.5
       rmmod usb-common.ko  // failed, usb_common is in use by udc-core

   2.6 
       rmmod udc-core.ko
       rmmod usb-common.ko  // usb directory is destroyed

they are all in line with expectations


> > +module_exit(usb_common_exit);
> > +
> >  MODULE_LICENSE("GPL");
> > diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
> > index 7fcb9f782931..f3d6b1ab80cb 100644
> > --- a/drivers/usb/core/usb.c
> > +++ b/drivers/usb/core/usb.c
> > @@ -1185,19 +1185,17 @@ static struct notifier_block usb_bus_nb = {
> >  	.notifier_call = usb_bus_notify,
> >  };
> >  
> > -struct dentry *usb_debug_root;
> > -EXPORT_SYMBOL_GPL(usb_debug_root);
> > +static struct dentry *usb_devices_root;
> >  
> >  static void usb_debugfs_init(void)
> >  {
> > -	usb_debug_root = debugfs_create_dir("usb", NULL);
> > -	debugfs_create_file("devices", 0444, usb_debug_root, NULL,
> > -			    &usbfs_devices_fops);
> > +	usb_devices_root = debugfs_create_file("devices", 0444, usb_debug_root,
> > +					       NULL, &usbfs_devices_fops);
> >  }
> >  
> >  static void usb_debugfs_cleanup(void)
> >  {
> > -	debugfs_remove_recursive(usb_debug_root);
> > +	debugfs_remove_recursive(usb_devices_root);
> 
> That should just be debugfs_remove();
> 
> I'll fix it up after someone tests this :)
> 
> thanks,
> 
> greg k-h

Re: [PATCH] USB: move usb debugfs directory creation to the usb common core

[Chunfeng Yun]

On Tue, 2019-06-04 at 13:59 +0200, Greg Kroah-Hartman wrote:
> On Tue, Jun 04, 2019 at 11:32:58AM +0200, Greg Kroah-Hartman wrote:
> > The USB gadget subsystem wants to use the USB debugfs root directory, so
> > move it to the common "core" USB code so that it is properly initialized
> > and removed as needed.
> > 
> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > 
> > ---
> > 
> > This should be the "correct" version of this, Chunfeng, can you test
> > this to verify it works for you?
> > 
> > 
> > diff --git a/drivers/usb/common/common.c b/drivers/usb/common/common.c
> > index 18f5dcf58b0d..3b5e4263ffef 100644
> > --- a/drivers/usb/common/common.c
> > +++ b/drivers/usb/common/common.c
> > @@ -15,6 +15,7 @@
> >  #include <linux/usb/of.h>
> >  #include <linux/usb/otg.h>
> >  #include <linux/of_platform.h>
> > +#include <linux/debugfs.h>
> >  
> >  static const char *const ep_type_names[] = {
> >  	[USB_ENDPOINT_XFER_CONTROL] = "ctrl",
> > @@ -291,4 +292,21 @@ struct device *usb_of_get_companion_dev(struct device *dev)
> >  EXPORT_SYMBOL_GPL(usb_of_get_companion_dev);
> >  #endif
> >  
> > +struct dentry *usb_debug_root;
> > +EXPORT_SYMBOL_GPL(usb_debug_root);
> > +
> > +static int usb_common_init(void)
> > +{
> > +	usb_debug_root = debugfs_create_dir("usb", NULL);
> > +	return 0;
> > +}
> > +
> > +static void usb_common_exit(void)
> > +{
> > +	debugfs_remove_recursive(usb_debug_root);
> > +}
> > +
> > +module_init(usb_common_init);
I tested this patch.

Here use module_init() indeed have a race as Felipe said before.
usbcore uses subsys_initcall(), and have a higher priority than
module_init(), so when usbcore tries to create "devices" file,
usb_debug_root is not created.

after I replace it by postcore_initcall() (debugfs uses
core_initcall()), test two cases:

1. buildin usbcore/udc-core

    "usb" directory is created, and usb/devices file is also created by
usbcore

2. build both usbcore and gadget as ko

    usbcore.ko, udc-core.ko and usb-common.ko are created. 

   2.1 
       insmod usb-common.ko   // "usb" directory is created
       insmod usb-core.ko   // usb/devices file is created

   2.2
       rmmod usb-common.ko  // failed, usb_common is in use by usb-core

   2.3 
       rmmod usb-core.ko   // usb/devices file is destroyed
       rmmod usb-common.ko  // usb directory is destroyed

   2.4 
       insmod usb-common.ko   // "usb" directory is created
       insmod udc-core.ko

   2.5
       rmmod usb-common.ko  // failed, usb_common is in use by udc-core

   2.6 
       rmmod udc-core.ko
       rmmod usb-common.ko  // usb directory is destroyed

they are all in line with expectations


> > +module_exit(usb_common_exit);
> > +
> >  MODULE_LICENSE("GPL");
> > diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
> > index 7fcb9f782931..f3d6b1ab80cb 100644
> > --- a/drivers/usb/core/usb.c
> > +++ b/drivers/usb/core/usb.c
> > @@ -1185,19 +1185,17 @@ static struct notifier_block usb_bus_nb = {
> >  	.notifier_call = usb_bus_notify,
> >  };
> >  
> > -struct dentry *usb_debug_root;
> > -EXPORT_SYMBOL_GPL(usb_debug_root);
> > +static struct dentry *usb_devices_root;
> >  
> >  static void usb_debugfs_init(void)
> >  {
> > -	usb_debug_root = debugfs_create_dir("usb", NULL);
> > -	debugfs_create_file("devices", 0444, usb_debug_root, NULL,
> > -			    &usbfs_devices_fops);
> > +	usb_devices_root = debugfs_create_file("devices", 0444, usb_debug_root,
> > +					       NULL, &usbfs_devices_fops);
> >  }
> >  
> >  static void usb_debugfs_cleanup(void)
> >  {
> > -	debugfs_remove_recursive(usb_debug_root);
> > +	debugfs_remove_recursive(usb_devices_root);
> 
> That should just be debugfs_remove();
> 
> I'll fix it up after someone tests this :)
> 
> thanks,
> 
> greg k-h

Re: [PATCH] USB: move usb debugfs directory creation to the usb common core

[Chunfeng Yun]

On Tue, 2019-06-04 at 13:59 +0200, Greg Kroah-Hartman wrote:
> On Tue, Jun 04, 2019 at 11:32:58AM +0200, Greg Kroah-Hartman wrote:
> > The USB gadget subsystem wants to use the USB debugfs root directory, so
> > move it to the common "core" USB code so that it is properly initialized
> > and removed as needed.
> > 
> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > 
> > ---
> > 
> > This should be the "correct" version of this, Chunfeng, can you test
> > this to verify it works for you?
> > 
> > 
> > diff --git a/drivers/usb/common/common.c b/drivers/usb/common/common.c
> > index 18f5dcf58b0d..3b5e4263ffef 100644
> > --- a/drivers/usb/common/common.c
> > +++ b/drivers/usb/common/common.c
> > @@ -15,6 +15,7 @@
> >  #include <linux/usb/of.h>
> >  #include <linux/usb/otg.h>
> >  #include <linux/of_platform.h>
> > +#include <linux/debugfs.h>
> >  
> >  static const char *const ep_type_names[] = {
> >  	[USB_ENDPOINT_XFER_CONTROL] = "ctrl",
> > @@ -291,4 +292,21 @@ struct device *usb_of_get_companion_dev(struct device *dev)
> >  EXPORT_SYMBOL_GPL(usb_of_get_companion_dev);
> >  #endif
> >  
> > +struct dentry *usb_debug_root;
> > +EXPORT_SYMBOL_GPL(usb_debug_root);
> > +
> > +static int usb_common_init(void)
> > +{
> > +	usb_debug_root = debugfs_create_dir("usb", NULL);
> > +	return 0;
> > +}
> > +
> > +static void usb_common_exit(void)
> > +{
> > +	debugfs_remove_recursive(usb_debug_root);
> > +}
> > +
> > +module_init(usb_common_init);
I tested this patch.

Here use module_init() indeed have a race as Felipe said before.
usbcore uses subsys_initcall(), and have a higher priority than
module_init(), so when usbcore tries to create "devices" file,
usb_debug_root is not created.

after I replace it by postcore_initcall() (debugfs uses
core_initcall()), test two cases:

1. buildin usbcore/udc-core

    "usb" directory is created, and usb/devices file is also created by
usbcore

2. build both usbcore and gadget as ko

    usbcore.ko, udc-core.ko and usb-common.ko are created. 

   2.1 
       insmod usb-common.ko   // "usb" directory is created
       insmod usb-core.ko   // usb/devices file is created

   2.2
       rmmod usb-common.ko  // failed, usb_common is in use by usb-core

   2.3 
       rmmod usb-core.ko   // usb/devices file is destroyed
       rmmod usb-common.ko  // usb directory is destroyed

   2.4 
       insmod usb-common.ko   // "usb" directory is created
       insmod udc-core.ko

   2.5
       rmmod usb-common.ko  // failed, usb_common is in use by udc-core

   2.6 
       rmmod udc-core.ko
       rmmod usb-common.ko  // usb directory is destroyed

they are all in line with expectations


> > +module_exit(usb_common_exit);
> > +
> >  MODULE_LICENSE("GPL");
> > diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
> > index 7fcb9f782931..f3d6b1ab80cb 100644
> > --- a/drivers/usb/core/usb.c
> > +++ b/drivers/usb/core/usb.c
> > @@ -1185,19 +1185,17 @@ static struct notifier_block usb_bus_nb = {
> >  	.notifier_call = usb_bus_notify,
> >  };
> >  
> > -struct dentry *usb_debug_root;
> > -EXPORT_SYMBOL_GPL(usb_debug_root);
> > +static struct dentry *usb_devices_root;
> >  
> >  static void usb_debugfs_init(void)
> >  {
> > -	usb_debug_root = debugfs_create_dir("usb", NULL);
> > -	debugfs_create_file("devices", 0444, usb_debug_root, NULL,
> > -			    &usbfs_devices_fops);
> > +	usb_devices_root = debugfs_create_file("devices", 0444, usb_debug_root,
> > +					       NULL, &usbfs_devices_fops);
> >  }
> >  
> >  static void usb_debugfs_cleanup(void)
> >  {
> > -	debugfs_remove_recursive(usb_debug_root);
> > +	debugfs_remove_recursive(usb_devices_root);
> 
> That should just be debugfs_remove();
> 
> I'll fix it up after someone tests this :)
> 
> thanks,
> 
> greg k-h



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] USB: move usb debugfs directory creation to the usb common core

[Chunfeng Yun]

On Wed, 2019-06-05 at 10:28 +0300, Felipe Balbi wrote:
> Hi,
> 
> Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
> >> Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
> >> > diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
> >> > index 7fcb9f782931..f3d6b1ab80cb 100644
> >> > --- a/drivers/usb/core/usb.c
> >> > +++ b/drivers/usb/core/usb.c
> >> > @@ -1185,19 +1185,17 @@ static struct notifier_block usb_bus_nb = {
> >> >  	.notifier_call = usb_bus_notify,
> >> >  };
> >> >  
> >> > -struct dentry *usb_debug_root;
> >> > -EXPORT_SYMBOL_GPL(usb_debug_root);
> >> > +static struct dentry *usb_devices_root;
> >> >  
> >> >  static void usb_debugfs_init(void)
> >> >  {
> >> > -	usb_debug_root = debugfs_create_dir("usb", NULL);
> >> > -	debugfs_create_file("devices", 0444, usb_debug_root, NULL,
> >> > -			    &usbfs_devices_fops);
> >> > +	usb_devices_root = debugfs_create_file("devices", 0444, usb_debug_root,
> >> 
> >> don't we have a race now? Can usbcore ever probe before usb common?
> >
> > How can that happen if usb_debug_root is in usb common?  The module
> > loader will not let that happen.  Or it shouldn't :)
> 
> argh, indeed. The very fact that usbcore tries to resolve usb_debug_root
> already forces a dependency :-p
When build as module, usbcore depend on usb-common, but when buildin,
usbcore init before usb-common (use module_init)

> 

Re: [PATCH] USB: move usb debugfs directory creation to the usb common core

[Chunfeng Yun]

On Wed, 2019-06-05 at 10:28 +0300, Felipe Balbi wrote:
> Hi,
> 
> Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
> >> Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
> >> > diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
> >> > index 7fcb9f782931..f3d6b1ab80cb 100644
> >> > --- a/drivers/usb/core/usb.c
> >> > +++ b/drivers/usb/core/usb.c
> >> > @@ -1185,19 +1185,17 @@ static struct notifier_block usb_bus_nb = {
> >> >  	.notifier_call = usb_bus_notify,
> >> >  };
> >> >  
> >> > -struct dentry *usb_debug_root;
> >> > -EXPORT_SYMBOL_GPL(usb_debug_root);
> >> > +static struct dentry *usb_devices_root;
> >> >  
> >> >  static void usb_debugfs_init(void)
> >> >  {
> >> > -	usb_debug_root = debugfs_create_dir("usb", NULL);
> >> > -	debugfs_create_file("devices", 0444, usb_debug_root, NULL,
> >> > -			    &usbfs_devices_fops);
> >> > +	usb_devices_root = debugfs_create_file("devices", 0444, usb_debug_root,
> >> 
> >> don't we have a race now? Can usbcore ever probe before usb common?
> >
> > How can that happen if usb_debug_root is in usb common?  The module
> > loader will not let that happen.  Or it shouldn't :)
> 
> argh, indeed. The very fact that usbcore tries to resolve usb_debug_root
> already forces a dependency :-p
When build as module, usbcore depend on usb-common, but when buildin,
usbcore init before usb-common (use module_init)

> 

Re: [PATCH] USB: move usb debugfs directory creation to the usb common core

[Chunfeng Yun]

On Wed, 2019-06-05 at 10:28 +0300, Felipe Balbi wrote:
> Hi,
> 
> Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
> >> Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
> >> > diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
> >> > index 7fcb9f782931..f3d6b1ab80cb 100644
> >> > --- a/drivers/usb/core/usb.c
> >> > +++ b/drivers/usb/core/usb.c
> >> > @@ -1185,19 +1185,17 @@ static struct notifier_block usb_bus_nb = {
> >> >  	.notifier_call = usb_bus_notify,
> >> >  };
> >> >  
> >> > -struct dentry *usb_debug_root;
> >> > -EXPORT_SYMBOL_GPL(usb_debug_root);
> >> > +static struct dentry *usb_devices_root;
> >> >  
> >> >  static void usb_debugfs_init(void)
> >> >  {
> >> > -	usb_debug_root = debugfs_create_dir("usb", NULL);
> >> > -	debugfs_create_file("devices", 0444, usb_debug_root, NULL,
> >> > -			    &usbfs_devices_fops);
> >> > +	usb_devices_root = debugfs_create_file("devices", 0444, usb_debug_root,
> >> 
> >> don't we have a race now? Can usbcore ever probe before usb common?
> >
> > How can that happen if usb_debug_root is in usb common?  The module
> > loader will not let that happen.  Or it shouldn't :)
> 
> argh, indeed. The very fact that usbcore tries to resolve usb_debug_root
> already forces a dependency :-p
When build as module, usbcore depend on usb-common, but when buildin,
usbcore init before usb-common (use module_init)

> 



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] USB: move usb debugfs directory creation to the usb common core

[Greg Kroah-Hartman]

On Wed, Jun 05, 2019 at 03:50:31PM +0800, Chunfeng Yun wrote:
> On Tue, 2019-06-04 at 13:59 +0200, Greg Kroah-Hartman wrote:
> > On Tue, Jun 04, 2019 at 11:32:58AM +0200, Greg Kroah-Hartman wrote:
> > > The USB gadget subsystem wants to use the USB debugfs root directory, so
> > > move it to the common "core" USB code so that it is properly initialized
> > > and removed as needed.
> > > 
> > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > 
> > > ---
> > > 
> > > This should be the "correct" version of this, Chunfeng, can you test
> > > this to verify it works for you?
> > > 
> > > 
> > > diff --git a/drivers/usb/common/common.c b/drivers/usb/common/common.c
> > > index 18f5dcf58b0d..3b5e4263ffef 100644
> > > --- a/drivers/usb/common/common.c
> > > +++ b/drivers/usb/common/common.c
> > > @@ -15,6 +15,7 @@
> > >  #include <linux/usb/of.h>
> > >  #include <linux/usb/otg.h>
> > >  #include <linux/of_platform.h>
> > > +#include <linux/debugfs.h>
> > >  
> > >  static const char *const ep_type_names[] = {
> > >  	[USB_ENDPOINT_XFER_CONTROL] = "ctrl",
> > > @@ -291,4 +292,21 @@ struct device *usb_of_get_companion_dev(struct device *dev)
> > >  EXPORT_SYMBOL_GPL(usb_of_get_companion_dev);
> > >  #endif
> > >  
> > > +struct dentry *usb_debug_root;
> > > +EXPORT_SYMBOL_GPL(usb_debug_root);
> > > +
> > > +static int usb_common_init(void)
> > > +{
> > > +	usb_debug_root = debugfs_create_dir("usb", NULL);
> > > +	return 0;
> > > +}
> > > +
> > > +static void usb_common_exit(void)
> > > +{
> > > +	debugfs_remove_recursive(usb_debug_root);
> > > +}
> > > +
> > > +module_init(usb_common_init);
> I tested this patch.
> 
> Here use module_init() indeed have a race as Felipe said before.
> usbcore uses subsys_initcall(), and have a higher priority than
> module_init(), so when usbcore tries to create "devices" file,
> usb_debug_root is not created.

Ah, let me fix that, it should have the same init level and I'll ensure
it comes first in the linking.

> after I replace it by postcore_initcall() (debugfs uses
> core_initcall()), test two cases:
> 
> 1. buildin usbcore/udc-core
> 
>     "usb" directory is created, and usb/devices file is also created by
> usbcore
> 
> 2. build both usbcore and gadget as ko
> 
>     usbcore.ko, udc-core.ko and usb-common.ko are created. 
> 
>    2.1 
>        insmod usb-common.ko   // "usb" directory is created
>        insmod usb-core.ko   // usb/devices file is created
> 
>    2.2
>        rmmod usb-common.ko  // failed, usb_common is in use by usb-core
> 
>    2.3 
>        rmmod usb-core.ko   // usb/devices file is destroyed
>        rmmod usb-common.ko  // usb directory is destroyed
> 
>    2.4 
>        insmod usb-common.ko   // "usb" directory is created
>        insmod udc-core.ko
> 
>    2.5
>        rmmod usb-common.ko  // failed, usb_common is in use by udc-core
> 
>    2.6 
>        rmmod udc-core.ko
>        rmmod usb-common.ko  // usb directory is destroyed
> 
> they are all in line with expectations

Wonderful!

Let me fix up the init level, and the build issue tha kbuild found, and
post a v2 patch.

thanks,

greg k-h

Re: [PATCH] USB: move usb debugfs directory creation to the usb common core

[Greg Kroah-Hartman]

On Wed, Jun 05, 2019 at 03:50:31PM +0800, Chunfeng Yun wrote:
> On Tue, 2019-06-04 at 13:59 +0200, Greg Kroah-Hartman wrote:
> > On Tue, Jun 04, 2019 at 11:32:58AM +0200, Greg Kroah-Hartman wrote:
> > > The USB gadget subsystem wants to use the USB debugfs root directory, so
> > > move it to the common "core" USB code so that it is properly initialized
> > > and removed as needed.
> > > 
> > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > 
> > > ---
> > > 
> > > This should be the "correct" version of this, Chunfeng, can you test
> > > this to verify it works for you?
> > > 
> > > 
> > > diff --git a/drivers/usb/common/common.c b/drivers/usb/common/common.c
> > > index 18f5dcf58b0d..3b5e4263ffef 100644
> > > --- a/drivers/usb/common/common.c
> > > +++ b/drivers/usb/common/common.c
> > > @@ -15,6 +15,7 @@
> > >  #include <linux/usb/of.h>
> > >  #include <linux/usb/otg.h>
> > >  #include <linux/of_platform.h>
> > > +#include <linux/debugfs.h>
> > >  
> > >  static const char *const ep_type_names[] = {
> > >  	[USB_ENDPOINT_XFER_CONTROL] = "ctrl",
> > > @@ -291,4 +292,21 @@ struct device *usb_of_get_companion_dev(struct device *dev)
> > >  EXPORT_SYMBOL_GPL(usb_of_get_companion_dev);
> > >  #endif
> > >  
> > > +struct dentry *usb_debug_root;
> > > +EXPORT_SYMBOL_GPL(usb_debug_root);
> > > +
> > > +static int usb_common_init(void)
> > > +{
> > > +	usb_debug_root = debugfs_create_dir("usb", NULL);
> > > +	return 0;
> > > +}
> > > +
> > > +static void usb_common_exit(void)
> > > +{
> > > +	debugfs_remove_recursive(usb_debug_root);
> > > +}
> > > +
> > > +module_init(usb_common_init);
> I tested this patch.
> 
> Here use module_init() indeed have a race as Felipe said before.
> usbcore uses subsys_initcall(), and have a higher priority than
> module_init(), so when usbcore tries to create "devices" file,
> usb_debug_root is not created.

Ah, let me fix that, it should have the same init level and I'll ensure
it comes first in the linking.

> after I replace it by postcore_initcall() (debugfs uses
> core_initcall()), test two cases:
> 
> 1. buildin usbcore/udc-core
> 
>     "usb" directory is created, and usb/devices file is also created by
> usbcore
> 
> 2. build both usbcore and gadget as ko
> 
>     usbcore.ko, udc-core.ko and usb-common.ko are created. 
> 
>    2.1 
>        insmod usb-common.ko   // "usb" directory is created
>        insmod usb-core.ko   // usb/devices file is created
> 
>    2.2
>        rmmod usb-common.ko  // failed, usb_common is in use by usb-core
> 
>    2.3 
>        rmmod usb-core.ko   // usb/devices file is destroyed
>        rmmod usb-common.ko  // usb directory is destroyed
> 
>    2.4 
>        insmod usb-common.ko   // "usb" directory is created
>        insmod udc-core.ko
> 
>    2.5
>        rmmod usb-common.ko  // failed, usb_common is in use by udc-core
> 
>    2.6 
>        rmmod udc-core.ko
>        rmmod usb-common.ko  // usb directory is destroyed
> 
> they are all in line with expectations

Wonderful!

Let me fix up the init level, and the build issue tha kbuild found, and
post a v2 patch.

thanks,

greg k-h

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel