[RFC PATCH 0/3] PCI/ASPM: Remove unncessary linked list in aspm.c

[RFC PATCH 0/3] PCI/ASPM: Remove unncessary linked list in aspm.c

[Saheed O. Bolarinwa]

From: "Bolarinwa O. Saheed" <refactormyself@gmail.com>

An extra linked list is created inside aspm.c to keep track of devices
on which the link state was enabled. However, it is possible to access
them via existing device lists.

This series remove the extra linked list and other related members of
the struct pcie_link_state: `root`, `parent` and `downstream`. All these
are now either calculated or obtained directly when needed.

Bolarinwa O. Saheed (4):
  PCI/ASPM: Remove struct pcie_link_state.parent
  PCI/ASPM: Remove struct pcie_link_state.root
  PCI/ASPM: Remove struct pcie_link_state.downstream
  PCI/ASPM: Remove unncessary linked list defined within aspm.c

 drivers/pci/pcie/aspm.c | 121 +++++++++++++++++++++-------------------
 1 file changed, 64 insertions(+), 57 deletions(-)

-- 
2.20.1

[RFC PATCH 1/4] PCI/ASPM: Remove struct pcie_link_state.parent

[Saheed O. Bolarinwa]

From: "Bolarinwa O. Saheed" <refactormyself@gmail.com>

Information cached in struct pcie_link_state.parent is accessible
via struct pci_dev.

This patch:
 - removes *parent* from the *struct pcie_link_state*
 - adjusts all references to it to access the information directly

Signed-off-by: Bolarinwa O. Saheed <refactormyself@gmail.com>
---
OPINION: the checkpatch.pl scring warns on this line:
	`BUG_ON(root->pdev->bus->parent->self);`
however, I think if a root device reports a parent, that is serious!

 drivers/pci/pcie/aspm.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index 013a47f587ce..48b83048aa30 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -50,7 +50,6 @@ struct pcie_link_state {
 	struct pci_dev *pdev;		/* Upstream component of the Link */
 	struct pci_dev *downstream;	/* Downstream component, function 0 */
 	struct pcie_link_state *root;	/* pointer to the root port link */
-	struct pcie_link_state *parent;	/* pointer to the parent Link state */
 	struct list_head sibling;	/* node in link_list */
 
 	/* ASPM state */
@@ -379,6 +378,7 @@ static void encode_l12_threshold(u32 threshold_us, u32 *scale, u32 *value)
 static void pcie_aspm_check_latency(struct pci_dev *endpoint)
 {
 	u32 latency, l1_switch_latency = 0;
+	struct pci_dev *parent;
 	struct aspm_latency *acceptable;
 	struct pcie_link_state *link;
 
@@ -419,7 +419,8 @@ static void pcie_aspm_check_latency(struct pci_dev *endpoint)
 			link->aspm_capable &= ~ASPM_STATE_L1;
 		l1_switch_latency += 1000;
 
-		link = link->parent;
+		parent = link->pdev->bus->parent->self;
+		link = !parent ? NULL : parent->link_state;
 	}
 }
 
@@ -793,9 +794,11 @@ static void pcie_config_aspm_link(struct pcie_link_state *link, u32 state)
 
 static void pcie_config_aspm_path(struct pcie_link_state *link)
 {
+	struct pci_dev *parent;
 	while (link) {
 		pcie_config_aspm_link(link, policy_to_aspm_state(link));
-		link = link->parent;
+		parent = link->pdev->bus->parent->self;
+		link = !parent ? NULL : parent->link_state;
 	}
 }
 
@@ -872,8 +875,7 @@ static struct pcie_link_state *alloc_pcie_link_state(struct pci_dev *pdev)
 			return NULL;
 		}
 
-		link->parent = parent;
-		link->root = link->parent->root;
+		link->root = parent->root;
 	}
 
 	list_add(&link->sibling, &link_list);
@@ -962,7 +964,7 @@ void pcie_aspm_init_link_state(struct pci_dev *pdev)
 static void pcie_update_aspm_capable(struct pcie_link_state *root)
 {
 	struct pcie_link_state *link;
-	BUG_ON(root->parent);
+	BUG_ON(root->pdev->bus->parent->self);
 	list_for_each_entry(link, &link_list, sibling) {
 		if (link->root != root)
 			continue;
@@ -985,6 +987,7 @@ static void pcie_update_aspm_capable(struct pcie_link_state *root)
 /* @pdev: the endpoint device */
 void pcie_aspm_exit_link_state(struct pci_dev *pdev)
 {
+	struct pci_dev *parent_dev;
 	struct pci_dev *parent = pdev->bus->self;
 	struct pcie_link_state *link, *root, *parent_link;
 
@@ -1002,7 +1005,8 @@ void pcie_aspm_exit_link_state(struct pci_dev *pdev)
 
 	link = parent->link_state;
 	root = link->root;
-	parent_link = link->parent;
+	parent_dev = link->pdev->bus->parent->self;
+	parent_link = !parent_dev ? NULL : parent_dev->link_state;
 
 	/* All functions are removed, so just disable ASPM for the link */
 	pcie_config_aspm_link(link, 0);
-- 
2.20.1

Re: [RFC PATCH 1/4] PCI/ASPM: Remove struct pcie_link_state.parent

[Bjorn Helgaas]

On Thu, Sep 16, 2021 at 10:52:03AM +0200, Saheed O. Bolarinwa wrote:
> From: "Bolarinwa O. Saheed" <refactormyself@gmail.com>
> 
> Information cached in struct pcie_link_state.parent is accessible
> via struct pci_dev.
> 
> This patch:
>  - removes *parent* from the *struct pcie_link_state*
>  - adjusts all references to it to access the information directly
> 
> Signed-off-by: Bolarinwa O. Saheed <refactormyself@gmail.com>
> ---
> OPINION: the checkpatch.pl scring warns on this line:
> 	`BUG_ON(root->pdev->bus->parent->self);`
> however, I think if a root device reports a parent, that is serious!

Do you mean this warning?

  WARNING: Missing a blank line after declarations
  #967: FILE: drivers/pci/pcie/aspm.c:967:
  +	struct pcie_link_state *link;
  +	BUG_ON(root->pdev->bus->parent->self);

That's just complaining about a blank line, so no big deal.  You could
resolve that by adding the blank line in this patch.

The fact that we use BUG_ON() at all *is* a real problem.  See the
comments at the BUG() definition.  We should rework this so that
condition is either impossible and we can just remove the BUG_ON(), or
we can deal with it gracefully.  But this would be material for a
different patch.

>  drivers/pci/pcie/aspm.c | 18 +++++++++++-------
>  1 file changed, 11 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
> index 013a47f587ce..48b83048aa30 100644
> --- a/drivers/pci/pcie/aspm.c
> +++ b/drivers/pci/pcie/aspm.c
> @@ -50,7 +50,6 @@ struct pcie_link_state {
>  	struct pci_dev *pdev;		/* Upstream component of the Link */
>  	struct pci_dev *downstream;	/* Downstream component, function 0 */
>  	struct pcie_link_state *root;	/* pointer to the root port link */
> -	struct pcie_link_state *parent;	/* pointer to the parent Link state */
>  	struct list_head sibling;	/* node in link_list */
>  
>  	/* ASPM state */
> @@ -379,6 +378,7 @@ static void encode_l12_threshold(u32 threshold_us, u32 *scale, u32 *value)
>  static void pcie_aspm_check_latency(struct pci_dev *endpoint)
>  {
>  	u32 latency, l1_switch_latency = 0;
> +	struct pci_dev *parent;
>  	struct aspm_latency *acceptable;
>  	struct pcie_link_state *link;
>  
> @@ -419,7 +419,8 @@ static void pcie_aspm_check_latency(struct pci_dev *endpoint)
>  			link->aspm_capable &= ~ASPM_STATE_L1;
>  		l1_switch_latency += 1000;
>  
> -		link = link->parent;
> +		parent = link->pdev->bus->parent->self;
> +		link = !parent ? NULL : parent->link_state;

I love the direction of this patch, but this chain of pointers
(link->pdev->bus->parent->self) is a little over the top and is
repeated several times here.

Can we simplify it a bit by making a helper function?  It's similar
but not quite the same as pci_upstream_bridge().

And maybe reverse the condition to avoid the negation?

  link = parent ? parent->link_state : NULL;

>  	}
>  }
>  
> @@ -793,9 +794,11 @@ static void pcie_config_aspm_link(struct pcie_link_state *link, u32 state)
>  
>  static void pcie_config_aspm_path(struct pcie_link_state *link)
>  {
> +	struct pci_dev *parent;
>  	while (link) {
>  		pcie_config_aspm_link(link, policy_to_aspm_state(link));
> -		link = link->parent;
> +		parent = link->pdev->bus->parent->self;
> +		link = !parent ? NULL : parent->link_state;
>  	}
>  }
>  
> @@ -872,8 +875,7 @@ static struct pcie_link_state *alloc_pcie_link_state(struct pci_dev *pdev)
>  			return NULL;
>  		}
>  
> -		link->parent = parent;
> -		link->root = link->parent->root;
> +		link->root = parent->root;
>  	}
>  
>  	list_add(&link->sibling, &link_list);
> @@ -962,7 +964,7 @@ void pcie_aspm_init_link_state(struct pci_dev *pdev)
>  static void pcie_update_aspm_capable(struct pcie_link_state *root)
>  {
>  	struct pcie_link_state *link;
> -	BUG_ON(root->parent);
> +	BUG_ON(root->pdev->bus->parent->self);
>  	list_for_each_entry(link, &link_list, sibling) {
>  		if (link->root != root)
>  			continue;
> @@ -985,6 +987,7 @@ static void pcie_update_aspm_capable(struct pcie_link_state *root)
>  /* @pdev: the endpoint device */
>  void pcie_aspm_exit_link_state(struct pci_dev *pdev)
>  {
> +	struct pci_dev *parent_dev;
>  	struct pci_dev *parent = pdev->bus->self;
>  	struct pcie_link_state *link, *root, *parent_link;
>  
> @@ -1002,7 +1005,8 @@ void pcie_aspm_exit_link_state(struct pci_dev *pdev)
>  
>  	link = parent->link_state;
>  	root = link->root;
> -	parent_link = link->parent;
> +	parent_dev = link->pdev->bus->parent->self;
> +	parent_link = !parent_dev ? NULL : parent_dev->link_state;
>  
>  	/* All functions are removed, so just disable ASPM for the link */
>  	pcie_config_aspm_link(link, 0);
> -- 
> 2.20.1
> 

[RFC PATCH 2/4] PCI/ASPM: Remove struct pcie_link_state.root

[Saheed O. Bolarinwa]

From: "Bolarinwa O. Saheed" <refactormyself@gmail.com>

Information on the root device is cached in
struct pcie_link_state.root it obtained within
alloc_pcie_link_state().

This patch:
 - creates *pcie_get_root()* which return the root pci_dev
   of a pci_dev.
 - removes *root* from the *struct pcie_link_state*.
 - replaces references to struct pcie_link_state.root with
   a call to pcie_get_root().

Signed-off-by: Bolarinwa O. Saheed <refactormyself@gmail.com>
---
 drivers/pci/pcie/aspm.c | 63 +++++++++++++++++++++--------------------
 1 file changed, 32 insertions(+), 31 deletions(-)

diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index 48b83048aa30..8772a4ea4365 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -49,7 +49,6 @@ struct aspm_latency {
 struct pcie_link_state {
 	struct pci_dev *pdev;		/* Upstream component of the Link */
 	struct pci_dev *downstream;	/* Downstream component, function 0 */
-	struct pcie_link_state *root;	/* pointer to the root port link */
 	struct list_head sibling;	/* node in link_list */
 
 	/* ASPM state */
@@ -843,6 +842,25 @@ static int pcie_aspm_sanity_check(struct pci_dev *pdev)
 	return 0;
 }
 
+/*
+ * Root Ports and PCI/PCI-X to PCIe Bridges are roots of PCIe
+ * hierarchies.  Note that some PCIe host implementations omit
+ * the root ports entirely, in which case a downstream port on
+ * a switch may become the root of the link state chain for all
+ * its subordinate endpoints.
+ */
+static struct pci_dev *pcie_get_root(struct pci_dev *pdev)
+{
+	struct pci_dev *parent = pdev->bus->parent->self;
+
+	if (pci_pcie_type(pdev) == PCI_EXP_TYPE_ROOT_PORT ||
+	    pci_pcie_type(pdev) == PCI_EXP_TYPE_PCIE_BRIDGE || !parent) {
+		return pdev;
+	} else {
+		return pcie_get_root(parent);
+	}
+}
+
 static struct pcie_link_state *alloc_pcie_link_state(struct pci_dev *pdev)
 {
 	struct pcie_link_state *link;
@@ -855,29 +873,6 @@ static struct pcie_link_state *alloc_pcie_link_state(struct pci_dev *pdev)
 	link->pdev = pdev;
 	link->downstream = pci_function_0(pdev->subordinate);
 
-	/*
-	 * Root Ports and PCI/PCI-X to PCIe Bridges are roots of PCIe
-	 * hierarchies.  Note that some PCIe host implementations omit
-	 * the root ports entirely, in which case a downstream port on
-	 * a switch may become the root of the link state chain for all
-	 * its subordinate endpoints.
-	 */
-	if (pci_pcie_type(pdev) == PCI_EXP_TYPE_ROOT_PORT ||
-	    pci_pcie_type(pdev) == PCI_EXP_TYPE_PCIE_BRIDGE ||
-	    !pdev->bus->parent->self) {
-		link->root = link;
-	} else {
-		struct pcie_link_state *parent;
-
-		parent = pdev->bus->parent->self->link_state;
-		if (!parent) {
-			kfree(link);
-			return NULL;
-		}
-
-		link->root = parent->root;
-	}
-
 	list_add(&link->sibling, &link_list);
 	pdev->link_state = link;
 	return link;
@@ -963,18 +958,23 @@ void pcie_aspm_init_link_state(struct pci_dev *pdev)
 /* Recheck latencies and update aspm_capable for links under the root */
 static void pcie_update_aspm_capable(struct pcie_link_state *root)
 {
+	struct pci_dev *dev;
 	struct pcie_link_state *link;
 	BUG_ON(root->pdev->bus->parent->self);
 	list_for_each_entry(link, &link_list, sibling) {
-		if (link->root != root)
+		dev = pcie_get_root(link->pdev);
+		if (dev->link_state != root)
 			continue;
+
 		link->aspm_capable = link->aspm_support;
 	}
 	list_for_each_entry(link, &link_list, sibling) {
 		struct pci_dev *child;
 		struct pci_bus *linkbus = link->pdev->subordinate;
-		if (link->root != root)
+		dev = pcie_get_root(link->pdev);
+		if (dev->link_state != root)
 			continue;
+
 		list_for_each_entry(child, &linkbus->devices, bus_list) {
 			if ((pci_pcie_type(child) != PCI_EXP_TYPE_ENDPOINT) &&
 			    (pci_pcie_type(child) != PCI_EXP_TYPE_LEG_END))
@@ -987,9 +987,9 @@ static void pcie_update_aspm_capable(struct pcie_link_state *root)
 /* @pdev: the endpoint device */
 void pcie_aspm_exit_link_state(struct pci_dev *pdev)
 {
-	struct pci_dev *parent_dev;
+	struct pci_dev *parent_dev, *root_dev;
 	struct pci_dev *parent = pdev->bus->self;
-	struct pcie_link_state *link, *root, *parent_link;
+	struct pcie_link_state *link, *parent_link;
 
 	if (!parent || !parent->link_state)
 		return;
@@ -1004,7 +1004,7 @@ void pcie_aspm_exit_link_state(struct pci_dev *pdev)
 		goto out;
 
 	link = parent->link_state;
-	root = link->root;
+	root_dev = pcie_get_root(link->pdev);
 	parent_dev = link->pdev->bus->parent->self;
 	parent_link = !parent_dev ? NULL : parent_dev->link_state;
 
@@ -1016,7 +1016,7 @@ void pcie_aspm_exit_link_state(struct pci_dev *pdev)
 
 	/* Recheck latencies and configure upstream links */
 	if (parent_link) {
-		pcie_update_aspm_capable(root);
+		pcie_update_aspm_capable(root_dev->link_state);
 		pcie_config_aspm_path(parent_link);
 	}
 out:
@@ -1028,6 +1028,7 @@ void pcie_aspm_exit_link_state(struct pci_dev *pdev)
 void pcie_aspm_pm_state_change(struct pci_dev *pdev)
 {
 	struct pcie_link_state *link = pdev->link_state;
+	struct pci_dev *root = pcie_get_root(pdev);
 
 	if (aspm_disabled || !link)
 		return;
@@ -1037,7 +1038,7 @@ void pcie_aspm_pm_state_change(struct pci_dev *pdev)
 	 */
 	down_read(&pci_bus_sem);
 	mutex_lock(&aspm_lock);
-	pcie_update_aspm_capable(link->root);
+	pcie_update_aspm_capable(root->link_state);
 	pcie_config_aspm_path(link);
 	mutex_unlock(&aspm_lock);
 	up_read(&pci_bus_sem);
-- 
2.20.1

[RFC PATCH 3/4] PCI/ASPM: Remove struct pcie_link_state.downstream

[Saheed O. Bolarinwa]

From: "Bolarinwa O. Saheed" <refactormyself@gmail.com>

Information on the downstream component is cached in
struct pcie_link_state.downstream it obtained within
alloc_pcie_link_state() by calling pci_function_0()

This patch:
 - removes *downstream* from *struct pcie_link_state*.
 - replaces references to pcie_link_state.downstream with
   a call to pci_function_0(pdev->subordinate).

Signed-off-by: Bolarinwa O. Saheed <refactormyself@gmail.com>
---
 drivers/pci/pcie/aspm.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index 8772a4ea4365..516a6f07ac6e 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -48,7 +48,6 @@ struct aspm_latency {
 
 struct pcie_link_state {
 	struct pci_dev *pdev;		/* Upstream component of the Link */
-	struct pci_dev *downstream;	/* Downstream component, function 0 */
 	struct list_head sibling;	/* node in link_list */
 
 	/* ASPM state */
@@ -452,7 +451,8 @@ static void pci_clear_and_set_dword(struct pci_dev *pdev, int pos,
 static void aspm_calc_l1ss_info(struct pcie_link_state *link,
 				u32 parent_l1ss_cap, u32 child_l1ss_cap)
 {
-	struct pci_dev *child = link->downstream, *parent = link->pdev;
+	struct pci_dev *parent = link->pdev;
+	struct pci_dev *child = pci_function_0(link->pdev->subordinate);
 	u32 val1, val2, scale1, scale2;
 	u32 t_common_mode, t_power_on, l1_2_threshold, scale, value;
 	u32 ctl1 = 0, ctl2 = 0;
@@ -542,7 +542,8 @@ static void aspm_calc_l1ss_info(struct pcie_link_state *link,
 
 static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist)
 {
-	struct pci_dev *child = link->downstream, *parent = link->pdev;
+	struct pci_dev *parent = link->pdev;
+	struct pci_dev *child = pci_function_0(link->pdev->subordinate);
 	u32 parent_lnkcap, child_lnkcap;
 	u16 parent_lnkctl, child_lnkctl;
 	u32 parent_l1ss_cap, child_l1ss_cap;
@@ -684,7 +685,8 @@ static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist)
 static void pcie_config_aspm_l1ss(struct pcie_link_state *link, u32 state)
 {
 	u32 val, enable_req;
-	struct pci_dev *child = link->downstream, *parent = link->pdev;
+	struct pci_dev *parent = link->pdev;
+	struct pci_dev *child = pci_function_0(link->pdev->subordinate);
 
 	enable_req = (link->aspm_enabled ^ state) & state;
 
@@ -743,7 +745,8 @@ static void pcie_config_aspm_dev(struct pci_dev *pdev, u32 val)
 static void pcie_config_aspm_link(struct pcie_link_state *link, u32 state)
 {
 	u32 upstream = 0, dwstream = 0;
-	struct pci_dev *child = link->downstream, *parent = link->pdev;
+	struct pci_dev *parent = link->pdev;
+	struct pci_dev *child = pci_function_0(link->pdev->subordinate);
 	struct pci_bus *linkbus = parent->subordinate;
 
 	/* Enable only the states that were not explicitly disabled */
@@ -871,7 +874,6 @@ static struct pcie_link_state *alloc_pcie_link_state(struct pci_dev *pdev)
 
 	INIT_LIST_HEAD(&link->sibling);
 	link->pdev = pdev;
-	link->downstream = pci_function_0(pdev->subordinate);
 
 	list_add(&link->sibling, &link_list);
 	pdev->link_state = link;
-- 
2.20.1

[RFC PATCH 4/4] PCI/ASPM: Remove unncessary linked list defined within aspm.c

[Saheed O. Bolarinwa]

From: "Bolarinwa O. Saheed" <refactormyself@gmail.com>

aspm.c defines a linked list - `link_list` and stores each of
its node in struct pcie_link_state.sibling. This linked list
tracks devices for which the struct pcie_link_state object
was successfully created. It is used to loop through the list
for instance to set ASPM policy or update changes. However, it
is possible to access these devices via existing lists defined
inside pci.h

This patch:
- removes link_list and struct pcie_link_state.sibling
- accesses child devices via struct pci_dev.bust_list
- accesses all PCI buses via pci_root_buses on struct pci_bus.node

Signed-off-by: Bolarinwa O. Saheed <refactormyself@gmail.com>
---
 drivers/pci/pcie/aspm.c | 40 ++++++++++++++++++++--------------------
 1 file changed, 20 insertions(+), 20 deletions(-)

diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index 516a6f07ac6e..35bc16c00b32 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -48,7 +48,6 @@ struct aspm_latency {
 
 struct pcie_link_state {
 	struct pci_dev *pdev;		/* Upstream component of the Link */
-	struct list_head sibling;	/* node in link_list */
 
 	/* ASPM state */
 	u32 aspm_support:7;		/* Supported ASPM state */
@@ -76,7 +75,6 @@ struct pcie_link_state {
 static int aspm_disabled, aspm_force;
 static bool aspm_support_enabled = true;
 static DEFINE_MUTEX(aspm_lock);
-static LIST_HEAD(link_list);
 
 #define POLICY_DEFAULT 0	/* BIOS default setting */
 #define POLICY_PERFORMANCE 1	/* high performance */
@@ -872,10 +870,7 @@ static struct pcie_link_state *alloc_pcie_link_state(struct pci_dev *pdev)
 	if (!link)
 		return NULL;
 
-	INIT_LIST_HEAD(&link->sibling);
 	link->pdev = pdev;
-
-	list_add(&link->sibling, &link_list);
 	pdev->link_state = link;
 	return link;
 }
@@ -961,21 +956,16 @@ void pcie_aspm_init_link_state(struct pci_dev *pdev)
 static void pcie_update_aspm_capable(struct pcie_link_state *root)
 {
 	struct pci_dev *dev;
+	struct pci_bus *rootbus = root->pdev->bus;
 	struct pcie_link_state *link;
 	BUG_ON(root->pdev->bus->parent->self);
-	list_for_each_entry(link, &link_list, sibling) {
-		dev = pcie_get_root(link->pdev);
-		if (dev->link_state != root)
-			continue;
-
-		link->aspm_capable = link->aspm_support;
+	list_for_each_entry(dev, &rootbus->devices, bus_list) {
+		dev->link_state->aspm_capable = link->aspm_support;
 	}
-	list_for_each_entry(link, &link_list, sibling) {
+
+	list_for_each_entry(dev, &rootbus->devices, bus_list) {
 		struct pci_dev *child;
-		struct pci_bus *linkbus = link->pdev->subordinate;
-		dev = pcie_get_root(link->pdev);
-		if (dev->link_state != root)
-			continue;
+		struct pci_bus *linkbus = dev->subordinate;
 
 		list_for_each_entry(child, &linkbus->devices, bus_list) {
 			if ((pci_pcie_type(child) != PCI_EXP_TYPE_ENDPOINT) &&
@@ -1012,7 +1002,6 @@ void pcie_aspm_exit_link_state(struct pci_dev *pdev)
 
 	/* All functions are removed, so just disable ASPM for the link */
 	pcie_config_aspm_link(link, 0);
-	list_del(&link->sibling);
 	/* Clock PM is for endpoint device */
 	free_link_state(link);
 
@@ -1152,6 +1141,8 @@ static int pcie_aspm_set_policy(const char *val,
 {
 	int i;
 	struct pcie_link_state *link;
+	struct pci_bus *bus;
+	struct pci_dev *pdev;
 
 	if (aspm_disabled)
 		return -EPERM;
@@ -1164,9 +1155,18 @@ static int pcie_aspm_set_policy(const char *val,
 	down_read(&pci_bus_sem);
 	mutex_lock(&aspm_lock);
 	aspm_policy = i;
-	list_for_each_entry(link, &link_list, sibling) {
-		pcie_config_aspm_link(link, policy_to_aspm_state(link));
-		pcie_set_clkpm(link, policy_to_clkpm_state(link));
+	list_for_each_entry(bus, &pci_root_buses, node) {
+		list_for_each_entry(pdev, &bus->devices, bus_list) {
+			if (!pci_is_pcie(pdev))
+				break;
+
+			link = pdev->link_state;
+			if (!link)
+				continue;
+
+			pcie_config_aspm_link(link, policy_to_aspm_state(link));
+			pcie_set_clkpm(link, policy_to_clkpm_state(link));
+		}
 	}
 	mutex_unlock(&aspm_lock);
 	up_read(&pci_bus_sem);
-- 
2.20.1

Re: [RFC PATCH 4/4] PCI/ASPM: Remove unncessary linked list defined within aspm.c

[kernel test robot]

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

Hi "Saheed,

[FYI, it's a private test report for your RFC patch.]
[auto build test WARNING on helgaas-pci/next]
[also build test WARNING on v5.15-rc1 next-20210916]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Saheed-O-Bolarinwa/PCI-ASPM-Remove-struct-pcie_link_state-parent/20210916-175210
base:   https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next
config: i386-buildonly-randconfig-r002-20210916 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project c8b3d7d6d6de37af68b2f379d0e37304f78e115f)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/a2a494c1e1531309518be74ed2e406334b716fbe
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Saheed-O-Bolarinwa/PCI-ASPM-Remove-struct-pcie_link_state-parent/20210916-175210
        git checkout a2a494c1e1531309518be74ed2e406334b716fbe
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 ARCH=i386 

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

All warnings (new ones prefixed by >>):

>> drivers/pci/pcie/aspm.c:963:35: warning: variable 'link' is uninitialized when used here [-Wuninitialized]
                   dev->link_state->aspm_capable = link->aspm_support;
                                                   ^~~~
   drivers/pci/pcie/aspm.c:960:30: note: initialize the variable 'link' to silence this warning
           struct pcie_link_state *link;
                                       ^
                                        = NULL
   1 warning generated.


vim +/link +963 drivers/pci/pcie/aspm.c

   954	
   955	/* Recheck latencies and update aspm_capable for links under the root */
   956	static void pcie_update_aspm_capable(struct pcie_link_state *root)
   957	{
   958		struct pci_dev *dev;
   959		struct pci_bus *rootbus = root->pdev->bus;
   960		struct pcie_link_state *link;
   961		BUG_ON(root->pdev->bus->parent->self);
   962		list_for_each_entry(dev, &rootbus->devices, bus_list) {
 > 963			dev->link_state->aspm_capable = link->aspm_support;
   964		}
   965	
   966		list_for_each_entry(dev, &rootbus->devices, bus_list) {
   967			struct pci_dev *child;
   968			struct pci_bus *linkbus = dev->subordinate;
   969	
   970			list_for_each_entry(child, &linkbus->devices, bus_list) {
   971				if ((pci_pcie_type(child) != PCI_EXP_TYPE_ENDPOINT) &&
   972				    (pci_pcie_type(child) != PCI_EXP_TYPE_LEG_END))
   973					continue;
   974				pcie_aspm_check_latency(child);
   975			}
   976		}
   977	}
   978	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

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

Re: [RFC PATCH 4/4] PCI/ASPM: Remove unncessary linked list defined within aspm.c

[kernel test robot]

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

Hi "Saheed,

[FYI, it's a private test report for your RFC patch.]
[auto build test WARNING on helgaas-pci/next]
[also build test WARNING on v5.15-rc1 next-20210916]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Saheed-O-Bolarinwa/PCI-ASPM-Remove-struct-pcie_link_state-parent/20210916-175210
base:   https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next
config: i386-buildonly-randconfig-r002-20210916 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project c8b3d7d6d6de37af68b2f379d0e37304f78e115f)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/a2a494c1e1531309518be74ed2e406334b716fbe
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Saheed-O-Bolarinwa/PCI-ASPM-Remove-struct-pcie_link_state-parent/20210916-175210
        git checkout a2a494c1e1531309518be74ed2e406334b716fbe
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 ARCH=i386 

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

All warnings (new ones prefixed by >>):

>> drivers/pci/pcie/aspm.c:963:35: warning: variable 'link' is uninitialized when used here [-Wuninitialized]
                   dev->link_state->aspm_capable = link->aspm_support;
                                                   ^~~~
   drivers/pci/pcie/aspm.c:960:30: note: initialize the variable 'link' to silence this warning
           struct pcie_link_state *link;
                                       ^
                                        = NULL
   1 warning generated.


vim +/link +963 drivers/pci/pcie/aspm.c

   954	
   955	/* Recheck latencies and update aspm_capable for links under the root */
   956	static void pcie_update_aspm_capable(struct pcie_link_state *root)
   957	{
   958		struct pci_dev *dev;
   959		struct pci_bus *rootbus = root->pdev->bus;
   960		struct pcie_link_state *link;
   961		BUG_ON(root->pdev->bus->parent->self);
   962		list_for_each_entry(dev, &rootbus->devices, bus_list) {
 > 963			dev->link_state->aspm_capable = link->aspm_support;
   964		}
   965	
   966		list_for_each_entry(dev, &rootbus->devices, bus_list) {
   967			struct pci_dev *child;
   968			struct pci_bus *linkbus = dev->subordinate;
   969	
   970			list_for_each_entry(child, &linkbus->devices, bus_list) {
   971				if ((pci_pcie_type(child) != PCI_EXP_TYPE_ENDPOINT) &&
   972				    (pci_pcie_type(child) != PCI_EXP_TYPE_LEG_END))
   973					continue;
   974				pcie_aspm_check_latency(child);
   975			}
   976		}
   977	}
   978	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

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

Re: [RFC PATCH 4/4] PCI/ASPM: Remove unncessary linked list defined within aspm.c

[kernel test robot]

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

CC: llvm(a)lists.linux.dev
CC: kbuild-all(a)lists.01.org
In-Reply-To: <20210916085206.2268-5-refactormyself@gmail.com>
References: <20210916085206.2268-5-refactormyself@gmail.com>
TO: "Saheed O. Bolarinwa" <refactormyself@gmail.com>

Hi "Saheed,

[FYI, it's a private test report for your RFC patch.]
[auto build test WARNING on helgaas-pci/next]
[also build test WARNING on v5.15-rc1 next-20210917]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Saheed-O-Bolarinwa/PCI-ASPM-Remove-struct-pcie_link_state-parent/20210916-175210
base:   https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next
:::::: branch date: 27 hours ago
:::::: commit date: 27 hours ago
config: riscv-randconfig-c006-20210916 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project c8b3d7d6d6de37af68b2f379d0e37304f78e115f)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install riscv cross compiling tool for clang build
        # apt-get install binutils-riscv64-linux-gnu
        # https://github.com/0day-ci/linux/commit/a2a494c1e1531309518be74ed2e406334b716fbe
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Saheed-O-Bolarinwa/PCI-ASPM-Remove-struct-pcie_link_state-parent/20210916-175210
        git checkout a2a494c1e1531309518be74ed2e406334b716fbe
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=riscv clang-analyzer 

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


clang-analyzer warnings: (new ones prefixed by >>)
                              ^   ~~~~~~~
   Suppressed 6 warnings (6 in non-user code).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   15 warnings generated.
   Suppressed 15 warnings (8 in non-user code, 7 with check filters).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   14 warnings generated.
   Suppressed 14 warnings (7 in non-user code, 7 with check filters).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   14 warnings generated.
   Suppressed 14 warnings (7 in non-user code, 7 with check filters).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   14 warnings generated.
   Suppressed 14 warnings (7 in non-user code, 7 with check filters).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   14 warnings generated.
   Suppressed 14 warnings (7 in non-user code, 7 with check filters).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   18 warnings generated.
   drivers/regulator/mt6358-regulator.c:278:47: warning: The result of the right shift is undefined because the right operand is negative [clang-analyzer-core.UndefinedBinaryOperatorResult]
           selector = (selector & info->desc.vsel_mask) >>
                                                        ^
   drivers/regulator/mt6358-regulator.c:271:6: note: Assuming 'ret' is equal to 0
           if (ret != 0) {
               ^~~~~~~~
   drivers/regulator/mt6358-regulator.c:271:2: note: Taking false branch
           if (ret != 0) {
           ^
   drivers/regulator/mt6358-regulator.c:278:47: note: The result of the right shift is undefined because the right operand is negative
           selector = (selector & info->desc.vsel_mask) >>
                                                        ^
   drivers/regulator/mt6358-regulator.c:302:38: warning: The result of the right shift is undefined because the right operand is negative [clang-analyzer-core.UndefinedBinaryOperatorResult]
           ret = (regval & info->da_vsel_mask) >> (ffs(info->da_vsel_mask) - 1);
                                               ^  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/regulator/mt6358-regulator.c:295:6: note: Assuming 'ret' is equal to 0
           if (ret != 0) {
               ^~~~~~~~
   drivers/regulator/mt6358-regulator.c:295:2: note: Taking false branch
           if (ret != 0) {
           ^
   drivers/regulator/mt6358-regulator.c:302:38: note: The result of the right shift is undefined because the right operand is negative
           ret = (regval & info->da_vsel_mask) >> (ffs(info->da_vsel_mask) - 1);
                                               ^  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/regulator/mt6358-regulator.c:342:6: warning: Assigned value is garbage or undefined [clang-analyzer-core.uninitialized.Assign]
           val <<= ffs(info->modeset_mask) - 1;
               ^   ~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/regulator/mt6358-regulator.c:328:2: note: Control jumps to 'case 2:'  at line 332
           switch (mode) {
           ^
   drivers/regulator/mt6358-regulator.c:334:3: note:  Execution continues on line 339
                   break;
                   ^
   drivers/regulator/mt6358-regulator.c:339:2: note: Loop condition is false.  Exiting loop
           dev_dbg(&rdev->dev, "mt6358 buck set_mode %#x, %#x, %#x\n",
           ^
   include/linux/dev_printk.h:158:2: note: expanded from macro 'dev_dbg'
           dev_printk(KERN_DEBUG, dev, dev_fmt(fmt), ##__VA_ARGS__)
           ^
   include/linux/dev_printk.h:128:3: note: expanded from macro 'dev_printk'
                   dev_printk_index_emit(level, fmt);                      \
                   ^
   include/linux/dev_printk.h:105:2: note: expanded from macro 'dev_printk_index_emit'
           printk_index_subsys_emit("%s %s: ", level, fmt)
           ^
   include/linux/printk.h:413:2: note: expanded from macro 'printk_index_subsys_emit'
           __printk_index_emit(fmt, level, subsys_fmt_prefix)
           ^
   include/linux/printk.h:392:34: note: expanded from macro '__printk_index_emit'
   #define __printk_index_emit(...) do {} while (0)
                                    ^
   drivers/regulator/mt6358-regulator.c:342:6: note: Assigned value is garbage or undefined
           val <<= ffs(info->modeset_mask) - 1;
               ^   ~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/regulator/mt6358-regulator.c:360:40: warning: The result of the right shift is undefined because the right operand is negative [clang-analyzer-core.UndefinedBinaryOperatorResult]
           switch ((regval & info->modeset_mask) >> (ffs(info->modeset_mask) - 1)) {
                                                 ^  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/regulator/mt6358-regulator.c:354:6: note: Assuming 'ret' is equal to 0
           if (ret != 0) {
               ^~~~~~~~
   drivers/regulator/mt6358-regulator.c:354:2: note: Taking false branch
           if (ret != 0) {
           ^
   drivers/regulator/mt6358-regulator.c:360:40: note: The result of the right shift is undefined because the right operand is negative
           switch ((regval & info->modeset_mask) >> (ffs(info->modeset_mask) - 1)) {
                                                 ^  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   Suppressed 14 warnings (7 in non-user code, 7 with check filters).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   13 warnings generated.
   Suppressed 13 warnings (6 in non-user code, 7 with check filters).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   13 warnings generated.
   Suppressed 13 warnings (6 in non-user code, 7 with check filters).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   13 warnings generated.
   Suppressed 13 warnings (6 in non-user code, 7 with check filters).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   13 warnings generated.
   Suppressed 13 warnings (6 in non-user code, 7 with check filters).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   15 warnings generated.
>> drivers/pci/pcie/aspm.c:963:35: warning: Access to field 'aspm_support' results in a dereference of an undefined pointer value (loaded from variable 'link') [clang-analyzer-core.NullDereference]
                   dev->link_state->aspm_capable = link->aspm_support;
                                                   ^
   drivers/pci/pcie/aspm.c:1024:6: note: Assuming 'aspm_disabled' is 0
           if (aspm_disabled || !link)
               ^~~~~~~~~~~~~
   drivers/pci/pcie/aspm.c:1024:6: note: Left side of '||' is false
   drivers/pci/pcie/aspm.c:1024:23: note: Assuming 'link' is non-null
           if (aspm_disabled || !link)
                                ^~~~~
   drivers/pci/pcie/aspm.c:1024:2: note: Taking false branch
           if (aspm_disabled || !link)
           ^
   drivers/pci/pcie/aspm.c:1032:2: note: Calling 'pcie_update_aspm_capable'
           pcie_update_aspm_capable(root->link_state);
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/pci/pcie/aspm.c:960:2: note: 'link' declared without an initial value
           struct pcie_link_state *link;
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/pci/pcie/aspm.c:961:2: note: Assuming field 'self' is null
           BUG_ON(root->pdev->bus->parent->self);
           ^
   include/asm-generic/bug.h:65:36: note: expanded from macro 'BUG_ON'
   #define BUG_ON(condition) do { if (unlikely(condition)) BUG(); } while (0)
                                      ^~~~~~~~~~~~~~~~~~~
   include/linux/compiler.h:78:40: note: expanded from macro 'unlikely'
   # define unlikely(x)    __builtin_expect(!!(x), 0)
                                             ^~~~
   drivers/pci/pcie/aspm.c:961:2: note: Taking false branch
           BUG_ON(root->pdev->bus->parent->self);
           ^
   include/asm-generic/bug.h:65:32: note: expanded from macro 'BUG_ON'
   #define BUG_ON(condition) do { if (unlikely(condition)) BUG(); } while (0)
                                  ^
   drivers/pci/pcie/aspm.c:961:2: note: Loop condition is false.  Exiting loop
           BUG_ON(root->pdev->bus->parent->self);
           ^
   include/asm-generic/bug.h:65:27: note: expanded from macro 'BUG_ON'
   #define BUG_ON(condition) do { if (unlikely(condition)) BUG(); } while (0)
                             ^
   drivers/pci/pcie/aspm.c:962:2: note: Left side of '&&' is false
           list_for_each_entry(dev, &rootbus->devices, bus_list) {
           ^
   include/linux/list.h:628:13: note: expanded from macro 'list_for_each_entry'
           for (pos = list_first_entry(head, typeof(*pos), member);        \
                      ^
   include/linux/list.h:522:2: note: expanded from macro 'list_first_entry'
           list_entry((ptr)->next, type, member)
           ^
   include/linux/list.h:511:2: note: expanded from macro 'list_entry'
           container_of(ptr, type, member)
           ^
   include/linux/kernel.h:495:61: note: expanded from macro 'container_of'
           BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) &&   \
                                                                      ^
   drivers/pci/pcie/aspm.c:962:2: note: Taking false branch
           list_for_each_entry(dev, &rootbus->devices, bus_list) {
           ^
   include/linux/list.h:628:13: note: expanded from macro 'list_for_each_entry'
           for (pos = list_first_entry(head, typeof(*pos), member);        \
                      ^
   include/linux/list.h:522:2: note: expanded from macro 'list_first_entry'
           list_entry((ptr)->next, type, member)
           ^
   include/linux/list.h:511:2: note: expanded from macro 'list_entry'
           container_of(ptr, type, member)
           ^
   note: (skipping 2 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all)
   include/linux/compiler_types.h:322:2: note: expanded from macro 'compiletime_assert'
           _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
           ^
   include/linux/compiler_types.h:310:2: note: expanded from macro '_compiletime_assert'
           __compiletime_assert(condition, msg, prefix, suffix)
           ^
   include/linux/compiler_types.h:302:3: note: expanded from macro '__compiletime_assert'
                   if (!(condition))                                       \
                   ^
   drivers/pci/pcie/aspm.c:962:2: note: Loop condition is false.  Exiting loop
           list_for_each_entry(dev, &rootbus->devices, bus_list) {
           ^
   include/linux/list.h:628:13: note: expanded from macro 'list_for_each_entry'
           for (pos = list_first_entry(head, typeof(*pos), member);        \
                      ^
   include/linux/list.h:522:2: note: expanded from macro 'list_first_entry'
           list_entry((ptr)->next, type, member)
           ^
   include/linux/list.h:511:2: note: expanded from macro 'list_entry'
           container_of(ptr, type, member)
           ^
   note: (skipping 2 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all)
   include/linux/compiler_types.h:322:2: note: expanded from macro 'compiletime_assert'
           _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
           ^
   include/linux/compiler_types.h:310:2: note: expanded from macro '_compiletime_assert'
           __compiletime_assert(condition, msg, prefix, suffix)
           ^
   include/linux/compiler_types.h:300:2: note: expanded from macro '__compiletime_assert'
           do {                                                            \
           ^
   drivers/pci/pcie/aspm.c:962:2: note: Loop condition is true.  Entering loop body
           list_for_each_entry(dev, &rootbus->devices, bus_list) {

vim +963 drivers/pci/pcie/aspm.c

7d715a6c1ae578 Shaohua Li          2008-02-25  954  
07d92760d2ee54 Kenji Kaneshige     2009-08-19  955  /* Recheck latencies and update aspm_capable for links under the root */
07d92760d2ee54 Kenji Kaneshige     2009-08-19  956  static void pcie_update_aspm_capable(struct pcie_link_state *root)
07d92760d2ee54 Kenji Kaneshige     2009-08-19  957  {
b73623c75c53fb Bolarinwa O. Saheed 2021-09-16  958  	struct pci_dev *dev;
a2a494c1e15313 Bolarinwa O. Saheed 2021-09-16  959  	struct pci_bus *rootbus = root->pdev->bus;
07d92760d2ee54 Kenji Kaneshige     2009-08-19  960  	struct pcie_link_state *link;
d98a968e12577a Bolarinwa O. Saheed 2021-09-16  961  	BUG_ON(root->pdev->bus->parent->self);
a2a494c1e15313 Bolarinwa O. Saheed 2021-09-16  962  	list_for_each_entry(dev, &rootbus->devices, bus_list) {
a2a494c1e15313 Bolarinwa O. Saheed 2021-09-16 @963  		dev->link_state->aspm_capable = link->aspm_support;
07d92760d2ee54 Kenji Kaneshige     2009-08-19  964  	}
a2a494c1e15313 Bolarinwa O. Saheed 2021-09-16  965  
a2a494c1e15313 Bolarinwa O. Saheed 2021-09-16  966  	list_for_each_entry(dev, &rootbus->devices, bus_list) {
07d92760d2ee54 Kenji Kaneshige     2009-08-19  967  		struct pci_dev *child;
a2a494c1e15313 Bolarinwa O. Saheed 2021-09-16  968  		struct pci_bus *linkbus = dev->subordinate;
b73623c75c53fb Bolarinwa O. Saheed 2021-09-16  969  
07d92760d2ee54 Kenji Kaneshige     2009-08-19  970  		list_for_each_entry(child, &linkbus->devices, bus_list) {
62f87c0e31d646 Yijing Wang         2012-07-24  971  			if ((pci_pcie_type(child) != PCI_EXP_TYPE_ENDPOINT) &&
62f87c0e31d646 Yijing Wang         2012-07-24  972  			    (pci_pcie_type(child) != PCI_EXP_TYPE_LEG_END))
07d92760d2ee54 Kenji Kaneshige     2009-08-19  973  				continue;
07d92760d2ee54 Kenji Kaneshige     2009-08-19  974  			pcie_aspm_check_latency(child);
07d92760d2ee54 Kenji Kaneshige     2009-08-19  975  		}
07d92760d2ee54 Kenji Kaneshige     2009-08-19  976  	}
07d92760d2ee54 Kenji Kaneshige     2009-08-19  977  }
07d92760d2ee54 Kenji Kaneshige     2009-08-19  978  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

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

Re: [RFC PATCH 4/4] PCI/ASPM: Remove unncessary linked list defined within aspm.c

[kernel test robot]

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

Hi Saheed,

[FYI, it's a private test report for your RFC patch.]
[auto build test WARNING on helgaas-pci/next]
[also build test WARNING on v5.15-rc1 next-20210917]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Saheed-O-Bolarinwa/PCI-ASPM-Remove-struct-pcie_link_state-parent/20210916-175210
base:   https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next
:::::: branch date: 27 hours ago
:::::: commit date: 27 hours ago
config: riscv-randconfig-c006-20210916 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project c8b3d7d6d6de37af68b2f379d0e37304f78e115f)
reproduce (this is a W=1 build):
         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
         chmod +x ~/bin/make.cross
         # install riscv cross compiling tool for clang build
         # apt-get install binutils-riscv64-linux-gnu
         # https://github.com/0day-ci/linux/commit/a2a494c1e1531309518be74ed2e406334b716fbe
         git remote add linux-review https://github.com/0day-ci/linux
         git fetch --no-tags linux-review Saheed-O-Bolarinwa/PCI-ASPM-Remove-struct-pcie_link_state-parent/20210916-175210
         git checkout a2a494c1e1531309518be74ed2e406334b716fbe
         # save the attached .config to linux build tree
         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=riscv clang-analyzer

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


clang-analyzer warnings: (new ones prefixed by >>)

 >> drivers/pci/pcie/aspm.c:963:35: warning: Access to field 'aspm_support' results in a dereference of an undefined pointer value (loaded from variable 'link') [clang-analyzer-core.NullDereference]
                    dev->link_state->aspm_capable = link->aspm_support;
                                                    ^

vim +963 drivers/pci/pcie/aspm.c

7d715a6c1ae578 Shaohua Li          2008-02-25  954
07d92760d2ee54 Kenji Kaneshige     2009-08-19  955  /* Recheck latencies and update aspm_capable for links under the root */
07d92760d2ee54 Kenji Kaneshige     2009-08-19  956  static void pcie_update_aspm_capable(struct pcie_link_state *root)
07d92760d2ee54 Kenji Kaneshige     2009-08-19  957  {
b73623c75c53fb Bolarinwa O. Saheed 2021-09-16  958  	struct pci_dev *dev;
a2a494c1e15313 Bolarinwa O. Saheed 2021-09-16  959  	struct pci_bus *rootbus = root->pdev->bus;
07d92760d2ee54 Kenji Kaneshige     2009-08-19  960  	struct pcie_link_state *link;
d98a968e12577a Bolarinwa O. Saheed 2021-09-16  961  	BUG_ON(root->pdev->bus->parent->self);
a2a494c1e15313 Bolarinwa O. Saheed 2021-09-16  962  	list_for_each_entry(dev, &rootbus->devices, bus_list) {
a2a494c1e15313 Bolarinwa O. Saheed 2021-09-16 @963  		dev->link_state->aspm_capable = link->aspm_support;
07d92760d2ee54 Kenji Kaneshige     2009-08-19  964  	}
a2a494c1e15313 Bolarinwa O. Saheed 2021-09-16  965
a2a494c1e15313 Bolarinwa O. Saheed 2021-09-16  966  	list_for_each_entry(dev, &rootbus->devices, bus_list) {
07d92760d2ee54 Kenji Kaneshige     2009-08-19  967  		struct pci_dev *child;
a2a494c1e15313 Bolarinwa O. Saheed 2021-09-16  968  		struct pci_bus *linkbus = dev->subordinate;
b73623c75c53fb Bolarinwa O. Saheed 2021-09-16  969
07d92760d2ee54 Kenji Kaneshige     2009-08-19  970  		list_for_each_entry(child, &linkbus->devices, bus_list) {
62f87c0e31d646 Yijing Wang         2012-07-24  971  			if ((pci_pcie_type(child) != PCI_EXP_TYPE_ENDPOINT) &&
62f87c0e31d646 Yijing Wang         2012-07-24  972  			    (pci_pcie_type(child) != PCI_EXP_TYPE_LEG_END))
07d92760d2ee54 Kenji Kaneshige     2009-08-19  973  				continue;
07d92760d2ee54 Kenji Kaneshige     2009-08-19  974  			pcie_aspm_check_latency(child);
07d92760d2ee54 Kenji Kaneshige     2009-08-19  975  		}
07d92760d2ee54 Kenji Kaneshige     2009-08-19  976  	}
07d92760d2ee54 Kenji Kaneshige     2009-08-19  977  }
07d92760d2ee54 Kenji Kaneshige     2009-08-19  978

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

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

[-- Attachment #3: Attached Message Part --]
[-- Type: text/plain, Size: 154 bytes --]

_______________________________________________
kbuild mailing list -- kbuild@lists.01.org
To unsubscribe send an email to kbuild-leave@lists.01.org

Re: [RFC PATCH 4/4] PCI/ASPM: Remove unncessary linked list defined within aspm.c

[kernel test robot]

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

Hi Saheed,

[FYI, it's a private test report for your RFC patch.]
[auto build test WARNING on helgaas-pci/next]
[also build test WARNING on v5.15-rc1 next-20210917]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Saheed-O-Bolarinwa/PCI-ASPM-Remove-struct-pcie_link_state-parent/20210916-175210
base:   https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next
:::::: branch date: 27 hours ago
:::::: commit date: 27 hours ago
config: riscv-randconfig-c006-20210916 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project c8b3d7d6d6de37af68b2f379d0e37304f78e115f)
reproduce (this is a W=1 build):
         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
         chmod +x ~/bin/make.cross
         # install riscv cross compiling tool for clang build
         # apt-get install binutils-riscv64-linux-gnu
         # https://github.com/0day-ci/linux/commit/a2a494c1e1531309518be74ed2e406334b716fbe
         git remote add linux-review https://github.com/0day-ci/linux
         git fetch --no-tags linux-review Saheed-O-Bolarinwa/PCI-ASPM-Remove-struct-pcie_link_state-parent/20210916-175210
         git checkout a2a494c1e1531309518be74ed2e406334b716fbe
         # save the attached .config to linux build tree
         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=riscv clang-analyzer

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


clang-analyzer warnings: (new ones prefixed by >>)

 >> drivers/pci/pcie/aspm.c:963:35: warning: Access to field 'aspm_support' results in a dereference of an undefined pointer value (loaded from variable 'link') [clang-analyzer-core.NullDereference]
                    dev->link_state->aspm_capable = link->aspm_support;
                                                    ^

vim +963 drivers/pci/pcie/aspm.c

7d715a6c1ae578 Shaohua Li          2008-02-25  954
07d92760d2ee54 Kenji Kaneshige     2009-08-19  955  /* Recheck latencies and update aspm_capable for links under the root */
07d92760d2ee54 Kenji Kaneshige     2009-08-19  956  static void pcie_update_aspm_capable(struct pcie_link_state *root)
07d92760d2ee54 Kenji Kaneshige     2009-08-19  957  {
b73623c75c53fb Bolarinwa O. Saheed 2021-09-16  958  	struct pci_dev *dev;
a2a494c1e15313 Bolarinwa O. Saheed 2021-09-16  959  	struct pci_bus *rootbus = root->pdev->bus;
07d92760d2ee54 Kenji Kaneshige     2009-08-19  960  	struct pcie_link_state *link;
d98a968e12577a Bolarinwa O. Saheed 2021-09-16  961  	BUG_ON(root->pdev->bus->parent->self);
a2a494c1e15313 Bolarinwa O. Saheed 2021-09-16  962  	list_for_each_entry(dev, &rootbus->devices, bus_list) {
a2a494c1e15313 Bolarinwa O. Saheed 2021-09-16 @963  		dev->link_state->aspm_capable = link->aspm_support;
07d92760d2ee54 Kenji Kaneshige     2009-08-19  964  	}
a2a494c1e15313 Bolarinwa O. Saheed 2021-09-16  965
a2a494c1e15313 Bolarinwa O. Saheed 2021-09-16  966  	list_for_each_entry(dev, &rootbus->devices, bus_list) {
07d92760d2ee54 Kenji Kaneshige     2009-08-19  967  		struct pci_dev *child;
a2a494c1e15313 Bolarinwa O. Saheed 2021-09-16  968  		struct pci_bus *linkbus = dev->subordinate;
b73623c75c53fb Bolarinwa O. Saheed 2021-09-16  969
07d92760d2ee54 Kenji Kaneshige     2009-08-19  970  		list_for_each_entry(child, &linkbus->devices, bus_list) {
62f87c0e31d646 Yijing Wang         2012-07-24  971  			if ((pci_pcie_type(child) != PCI_EXP_TYPE_ENDPOINT) &&
62f87c0e31d646 Yijing Wang         2012-07-24  972  			    (pci_pcie_type(child) != PCI_EXP_TYPE_LEG_END))
07d92760d2ee54 Kenji Kaneshige     2009-08-19  973  				continue;
07d92760d2ee54 Kenji Kaneshige     2009-08-19  974  			pcie_aspm_check_latency(child);
07d92760d2ee54 Kenji Kaneshige     2009-08-19  975  		}
07d92760d2ee54 Kenji Kaneshige     2009-08-19  976  	}
07d92760d2ee54 Kenji Kaneshige     2009-08-19  977  }
07d92760d2ee54 Kenji Kaneshige     2009-08-19  978

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

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

[-- Attachment #3: AttachedMessagePart.ksh --]
[-- Type: text/plain, Size: 154 bytes --]

_______________________________________________
kbuild mailing list -- kbuild@lists.01.org
To unsubscribe send an email to kbuild-leave@lists.01.org