USB: dwc3: remove debugfs root dentry storage

PATCH In review Greg Kroah-Hartman 4 participants 5 messages Jun 9, 2021 – Jun 10, 2021

Subsystems (2)

USB DesignWare USB3 DRD IP driver

lore

Share & tools

lore

Greg Kroah-Hartman

author maintainer

2021-06-09 09:39 UTC #0 lore raw

PATCH 1/1 USB: dwc3: remove debugfs root dentry storage

There is no need to keep around the debugfs "root" directory for the dwc3 device. Instead, look it up anytime we need to find it. This will help when callers get out-of-order and we had the potential to have a "stale" pointer around for the root dentry, as has happened in the past.

Cc Felipe Balbi <[email protected]>

Cc Jack Pham <[email protected]>

Cc Peter Chen <[email protected]>

Signed-off-by Greg Kroah-Hartman <[email protected]>

---
 drivers/usb/dwc3/core.h    | 2 --
 drivers/usb/dwc3/debugfs.c | 8 ++++----
 drivers/usb/dwc3/gadget.c  | 4 +++-
 3 files changed, 7 insertions(+), 7 deletions(-)

drivers/usb/dwc3/core.h +0 -2

index c5d5760cdf53..dccdf13b5f9e 100644
@@ -1013,7 +1013,6 @@ struct dwc3_scratchpad_array {
  * @link_state: link state
  * @speed: device speed (super, high, full, low)
  * @hwparams: copy of hwparams registers
- * @root: debugfs root folder pointer
  * @regset: debugfs pointer to regdump file
  * @dbg_lsp_select: current debug lsp mux register selection
  * @test_mode: true when we're entering a USB test mode

@@ -1222,7 +1221,6 @@ struct dwc3 {
 	u8			num_eps;
 
 	struct dwc3_hwparams	hwparams;
-	struct dentry		*root;
 	struct debugfs_regset32	*regset;
 
 	u32			dbg_lsp_select;

drivers/usb/dwc3/debugfs.c +4 -4

index 5dbbe53269d3..f2b7675c7f62 100644
@@ -889,8 +889,10 @@ static void dwc3_debugfs_create_endpoint_files(struct dwc3_ep *dep,
 void dwc3_debugfs_create_endpoint_dir(struct dwc3_ep *dep)
 {
 	struct dentry		*dir;
+	struct dentry		*root;
 
-	dir = debugfs_create_dir(dep->name, dep->dwc->root);
+	root = debugfs_lookup(dev_name(dep->dwc->dev), usb_debug_root);
+	dir = debugfs_create_dir(dep->name, root);
 	dwc3_debugfs_create_endpoint_files(dep, dir);
 }

@@ -909,8 +911,6 @@ void dwc3_debugfs_init(struct dwc3 *dwc)
 	dwc->regset->base = dwc->regs - DWC3_GLOBALS_REGS_START;
 
 	root = debugfs_create_dir(dev_name(dwc->dev), usb_debug_root);
-	dwc->root = root;
-
 	debugfs_create_regset32("regdump", 0444, root, dwc->regset);
 	debugfs_create_file("lsp_dump", 0644, root, dwc, &dwc3_lsp_fops);

@@ -929,6 +929,6 @@ void dwc3_debugfs_init(struct dwc3 *dwc)
 
 void dwc3_debugfs_exit(struct dwc3 *dwc)
 {
-	debugfs_remove_recursive(dwc->root);
+	debugfs_remove(debugfs_lookup(dev_name(dwc->dev), usb_debug_root));
 	kfree(dwc->regset);
 }

drivers/usb/dwc3/gadget.c +3 -1

index 7cc99b6d0bfe..026a2ad0fc80 100644
@@ -2799,7 +2799,9 @@ static void dwc3_gadget_free_endpoints(struct dwc3 *dwc)
 			list_del(&dep->endpoint.ep_list);
 		}
 
-		debugfs_remove_recursive(debugfs_lookup(dep->name, dwc->root));
+		debugfs_remove_recursive(debugfs_lookup(dep->name,
+				debugfs_lookup(dev_name(dep->dwc->dev),
+					       usb_debug_root)));
 		kfree(dep);
 	}
 }

Signature · 1 line

2.32.0

Jack Pham

Tested-by on v1 1/1

2021-06-09 18:47 UTC #1 lore raw

Quoted 73 lines · Greg Kroah-Hartman

Cc Felipe Balbi <[email protected]>

Cc Jack Pham <[email protected]>

Cc Peter Chen <[email protected]>

Signed-off-by Greg Kroah-Hartman <[email protected]>

---
 drivers/usb/dwc3/core.h    | 2 --
 drivers/usb/dwc3/debugfs.c | 8 ++++----
 drivers/usb/dwc3/gadget.c  | 4 +++-
 3 files changed, 7 insertions(+), 7 deletions(-)

drivers/usb/dwc3/core.h +0 -2

index c5d5760cdf53..dccdf13b5f9e 100644
@@ -1013,7 +1013,6 @@ struct dwc3_scratchpad_array {
  * @link_state: link state
  * @speed: device speed (super, high, full, low)
  * @hwparams: copy of hwparams registers
- * @root: debugfs root folder pointer
  * @regset: debugfs pointer to regdump file
  * @dbg_lsp_select: current debug lsp mux register selection
  * @test_mode: true when we're entering a USB test mode

@@ -1222,7 +1221,6 @@ struct dwc3 {
 	u8			num_eps;
 
 	struct dwc3_hwparams	hwparams;
-	struct dentry		*root;
 	struct debugfs_regset32	*regset;
 
 	u32			dbg_lsp_select;

drivers/usb/dwc3/debugfs.c +4 -4

index 5dbbe53269d3..f2b7675c7f62 100644
@@ -889,8 +889,10 @@ static void dwc3_debugfs_create_endpoint_files(struct dwc3_ep *dep,
 void dwc3_debugfs_create_endpoint_dir(struct dwc3_ep *dep)
 {
 	struct dentry		*dir;
+	struct dentry		*root;
 
-	dir = debugfs_create_dir(dep->name, dep->dwc->root);
+	root = debugfs_lookup(dev_name(dep->dwc->dev), usb_debug_root);
+	dir = debugfs_create_dir(dep->name, root);
 	dwc3_debugfs_create_endpoint_files(dep, dir);
 }

@@ -909,8 +911,6 @@ void dwc3_debugfs_init(struct dwc3 *dwc)
 	dwc->regset->base = dwc->regs - DWC3_GLOBALS_REGS_START;
 
 	root = debugfs_create_dir(dev_name(dwc->dev), usb_debug_root);
-	dwc->root = root;
-
 	debugfs_create_regset32("regdump", 0444, root, dwc->regset);
 	debugfs_create_file("lsp_dump", 0644, root, dwc, &dwc3_lsp_fops);

@@ -929,6 +929,6 @@ void dwc3_debugfs_init(struct dwc3 *dwc)
 
 void dwc3_debugfs_exit(struct dwc3 *dwc)
 {
-	debugfs_remove_recursive(dwc->root);
+	debugfs_remove(debugfs_lookup(dev_name(dwc->dev), usb_debug_root));
 	kfree(dwc->regset);
 }

drivers/usb/dwc3/gadget.c +0 -0

index 7cc99b6d0bfe..026a2ad0fc80 100644

drivers/usb/dwc3/gadget.c static void dwc3_gadget_free_endpoints(struct dwc3 *dwc) · line 2804

Quoted 7 lines

drivers/usb/dwc3/gadget.c +3 -1

@@ -2799,7 +2799,9 @@ static void dwc3_gadget_free_endpoints(struct dwc3 *dwc)
 			list_del(&dep->endpoint.ep_list);
 		}
 
-		debugfs_remove_recursive(debugfs_lookup(dep->name, dwc->root));
+		debugfs_remove_recursive(debugfs_lookup(dep->name,
+				debugfs_lookup(dev_name(dep->dwc->dev),
+					       usb_debug_root)));

Nested calls to debugfs_lookup() :). But it does work, and similarly avoids the out-of-order debugfs removal issue as well even without Peter's fix.

Tested-by Jack Pham <[email protected]>

Thanks
Jack

Signature · 2 lines

The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Greg Kroah-Hartman

author maintainer

2021-06-09 18:59 UTC #2 lore raw

Quoted 77 lines · Jack Pham

Quoted 76 lines · Greg Kroah-Hartman

Cc Felipe Balbi <[email protected]>

Cc Jack Pham <[email protected]>

Cc Peter Chen <[email protected]>

Signed-off-by Greg Kroah-Hartman <[email protected]>

---
 drivers/usb/dwc3/core.h    | 2 --
 drivers/usb/dwc3/debugfs.c | 8 ++++----
 drivers/usb/dwc3/gadget.c  | 4 +++-
 3 files changed, 7 insertions(+), 7 deletions(-)

drivers/usb/dwc3/core.h +0 -2

index c5d5760cdf53..dccdf13b5f9e 100644
@@ -1013,7 +1013,6 @@ struct dwc3_scratchpad_array {
  * @link_state: link state
  * @speed: device speed (super, high, full, low)
  * @hwparams: copy of hwparams registers
- * @root: debugfs root folder pointer
  * @regset: debugfs pointer to regdump file
  * @dbg_lsp_select: current debug lsp mux register selection
  * @test_mode: true when we're entering a USB test mode

@@ -1222,7 +1221,6 @@ struct dwc3 {
 	u8			num_eps;
 
 	struct dwc3_hwparams	hwparams;
-	struct dentry		*root;
 	struct debugfs_regset32	*regset;
 
 	u32			dbg_lsp_select;

drivers/usb/dwc3/debugfs.c +4 -4

index 5dbbe53269d3..f2b7675c7f62 100644
@@ -889,8 +889,10 @@ static void dwc3_debugfs_create_endpoint_files(struct dwc3_ep *dep,
 void dwc3_debugfs_create_endpoint_dir(struct dwc3_ep *dep)
 {
 	struct dentry		*dir;
+	struct dentry		*root;
 
-	dir = debugfs_create_dir(dep->name, dep->dwc->root);
+	root = debugfs_lookup(dev_name(dep->dwc->dev), usb_debug_root);
+	dir = debugfs_create_dir(dep->name, root);
 	dwc3_debugfs_create_endpoint_files(dep, dir);
 }

@@ -909,8 +911,6 @@ void dwc3_debugfs_init(struct dwc3 *dwc)
 	dwc->regset->base = dwc->regs - DWC3_GLOBALS_REGS_START;
 
 	root = debugfs_create_dir(dev_name(dwc->dev), usb_debug_root);
-	dwc->root = root;
-
 	debugfs_create_regset32("regdump", 0444, root, dwc->regset);
 	debugfs_create_file("lsp_dump", 0644, root, dwc, &dwc3_lsp_fops);

@@ -929,6 +929,6 @@ void dwc3_debugfs_init(struct dwc3 *dwc)
 
 void dwc3_debugfs_exit(struct dwc3 *dwc)
 {
-	debugfs_remove_recursive(dwc->root);
+	debugfs_remove(debugfs_lookup(dev_name(dwc->dev), usb_debug_root));
 	kfree(dwc->regset);
 }

drivers/usb/dwc3/gadget.c +0 -0

index 7cc99b6d0bfe..026a2ad0fc80 100644
@@ -2799,7 +2799,9 @@ static void dwc3_gadget_free_endpoints(struct dwc3 *dwc)
 			list_del(&dep->endpoint.ep_list);
 		}

Quoted 8 lines

drivers/usb/dwc3/gadget.c static void dwc3_gadget_free_endpoints(struct dwc3 *dwc) · line 2804
Quoted 4 lines
drivers/usb/dwc3/gadget.c +3 -1
-		debugfs_remove_recursive(debugfs_lookup(dep->name, dwc->root));
+		debugfs_remove_recursive(debugfs_lookup(dep->name,
+				debugfs_lookup(dev_name(dep->dwc->dev),
+					       usb_debug_root)));
Nested calls to debugfs_lookup() :). But it does work, and similarly avoids the out-of-order debugfs removal issue as well even without Peter's fix.

Yeah, it's funny, but it's safer than trying to store a pointer around :)

Quoted 1 line

Tested-by Jack Pham <[email protected]>

Many thanks for testing.

greg k-h

Peter Chen

Reviewed-by on v1 1/1

2021-06-10 01:14 UTC #3 lore raw

In reply to Greg Kroah-Hartman

Quoted 9 lines · Greg Kroah-Hartman

There is no need to keep around the debugfs "root" directory for the dwc3 device. Instead, look it up anytime we need to find it. This will help when callers get out-of-order and we had the potential to have a "stale" pointer around for the root dentry, as has happened in the past.
Cc Felipe Balbi <[email protected]>
Cc Jack Pham <[email protected]>
Cc Peter Chen <[email protected]>
Signed-off-by Greg Kroah-Hartman <[email protected]>

Reviewed-by Peter Chen <[email protected]>

Quoted 77 lines

---
 drivers/usb/dwc3/core.h    | 2 --
 drivers/usb/dwc3/debugfs.c | 8 ++++----
 drivers/usb/dwc3/gadget.c  | 4 +++-
 3 files changed, 7 insertions(+), 7 deletions(-)

drivers/usb/dwc3/core.h +0 -2

index c5d5760cdf53..dccdf13b5f9e 100644
@@ -1013,7 +1013,6 @@ struct dwc3_scratchpad_array {
  * @link_state: link state
  * @speed: device speed (super, high, full, low)
  * @hwparams: copy of hwparams registers
- * @root: debugfs root folder pointer
  * @regset: debugfs pointer to regdump file
  * @dbg_lsp_select: current debug lsp mux register selection
  * @test_mode: true when we're entering a USB test mode

@@ -1222,7 +1221,6 @@ struct dwc3 {
 	u8			num_eps;
 
 	struct dwc3_hwparams	hwparams;
-	struct dentry		*root;
 	struct debugfs_regset32	*regset;
 
 	u32			dbg_lsp_select;

drivers/usb/dwc3/debugfs.c +4 -4

index 5dbbe53269d3..f2b7675c7f62 100644
@@ -889,8 +889,10 @@ static void dwc3_debugfs_create_endpoint_files(struct dwc3_ep *dep,
 void dwc3_debugfs_create_endpoint_dir(struct dwc3_ep *dep)
 {
 	struct dentry		*dir;
+	struct dentry		*root;
 
-	dir = debugfs_create_dir(dep->name, dep->dwc->root);
+	root = debugfs_lookup(dev_name(dep->dwc->dev), usb_debug_root);
+	dir = debugfs_create_dir(dep->name, root);
 	dwc3_debugfs_create_endpoint_files(dep, dir);
 }

@@ -909,8 +911,6 @@ void dwc3_debugfs_init(struct dwc3 *dwc)
 	dwc->regset->base = dwc->regs - DWC3_GLOBALS_REGS_START;
 
 	root = debugfs_create_dir(dev_name(dwc->dev), usb_debug_root);
-	dwc->root = root;
-
 	debugfs_create_regset32("regdump", 0444, root, dwc->regset);
 	debugfs_create_file("lsp_dump", 0644, root, dwc, &dwc3_lsp_fops);

@@ -929,6 +929,6 @@ void dwc3_debugfs_init(struct dwc3 *dwc)
 
 void dwc3_debugfs_exit(struct dwc3 *dwc)
 {
-	debugfs_remove_recursive(dwc->root);
+	debugfs_remove(debugfs_lookup(dev_name(dwc->dev), usb_debug_root));
 	kfree(dwc->regset);
 }

drivers/usb/dwc3/gadget.c +3 -1

index 7cc99b6d0bfe..026a2ad0fc80 100644
@@ -2799,7 +2799,9 @@ static void dwc3_gadget_free_endpoints(struct dwc3 *dwc)
 			list_del(&dep->endpoint.ep_list);
 		}
 
-		debugfs_remove_recursive(debugfs_lookup(dep->name, dwc->root));
+		debugfs_remove_recursive(debugfs_lookup(dep->name,
+				debugfs_lookup(dev_name(dep->dwc->dev),
+					       usb_debug_root)));
 		kfree(dep);
 	}
 }

Signature · 1 line

2.32.0

Signature · 2 lines

Thanks,
Peter Chen

Felipe Balbi

Acked-by on v1 1/1

2021-06-10 09:29 UTC #4 lore raw

In reply to Greg Kroah-Hartman

Quoted 9 lines · Greg Kroah-Hartman

There is no need to keep around the debugfs "root" directory for the dwc3 device. Instead, look it up anytime we need to find it. This will help when callers get out-of-order and we had the potential to have a "stale" pointer around for the root dentry, as has happened in the past.
Cc Felipe Balbi <[email protected]>
Cc Jack Pham <[email protected]>
Cc Peter Chen <[email protected]>
Signed-off-by Greg Kroah-Hartman <[email protected]>

Acked-by Felipe Balbi <[email protected]>

Signature · 1 line

balbi