LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH v3 0/3] resource: Use list_head to link sibling resource
@ 2018-04-19  0:18 Baoquan He
  2018-04-19  0:18 ` [PATCH v3 1/3] " Baoquan He
                   ` (2 more replies)
  0 siblings, 3 replies; 19+ messages in thread
From: Baoquan He @ 2018-04-19  0:18 UTC (permalink / raw)
  To: linux-kernel, akpm, robh+dt, dan.j.williams, nicolas.pitre, josh
  Cc: Baoquan He

This patchset mainly converts strut resource's sibling list from singly
linked list to doubly linked list, list_head. This is suggested by
Andrew. Since I need a reversed searching on iomem_resource's
IORESOURCE_SYSTEM_RAM children,  the old singly linked list way makes
the code in v1 post really ugly.

With this change, we only need one simple list_for_each_entry_reverse()
to do reversed iteration on sibling list. The relevant codes in
kernel/resource.c are more readable since those dazzling pointer
operation codes for singly linked list are replaced.

With the help of list_head, in patch 0002 we can have a very simple
walk_system_ram_res_rev(). And in patch 0003, will use it to search
available system RAM region for kexec_buffer of kexec_file from top to
down, just like we have been doing all along in kexec loading which is
done in kexec-tools utility.

Note:
This patchset passed testing on my kvm guest, x86_64 arch with network
enabling. The thing we need pay attetion to is that a root resource's
child member need be initialized specifically with LIST_HEAD_INIT() if
sttically defined or INIT_LIST_HEAD() for dynamically definition. Here
Just like we do for iomem_resource/ioport_resource, or the change in
get_pci_domain_busn_res().

And there are two places of change I am not very sure. One is in
drivers/hv/vmbus_drv.c, the other is in drivers/pci/host/vmd.c. So will
invite experts on these areas to help review.

V2 post and discussions can be found here:
https://lkml.org/lkml/2018/4/7/169

Remaining issue:
Rob raised issue that if we can make an common tree struct and helpers
defined on top of list_head or a new struct, quote his saying as below.
I didn't do much investigation, we may have a new struct like 

struct tlist {
        void                    *parent;
        struct list_head        sibling;
        struct list_head        child;
}

Since I have some rhel dev things right now, may consider this later, put
it in my TODO list. In kernel, like task_struct, task_group, they all have
this kind of tree list, and have used list_head. If anyone is interested,
feel free to have a try on this.

==========
The DT struct device_node also has the same tree structure with
parent, child, sibling pointers and converting to list_head had been
on the todo list for a while. ACPI also has some tree walking
functions (drivers/acpi/acpica/pstree.c). Perhaps there should be a
common tree struct and helpers defined either on top of list_head or a
new struct if that saves some size.
==========


Changelog:
v2->v3:
  Rename resource functions first_child() and sibling() to
  resource_first_chils() and resource_sibling(). Dan suggested this.

  Move resource_first_chils() and resource_sibling() to linux/ioport.h
  and make them as inline function. Rob suggested this. Accordingly add
  linux/list.h including in linux/ioport.h, please help review if this
  bring efficiency degradation or code redundancy.

  The change on struct resource {} bring two pointers of size increase,
  mention this in git log to make it more specifically, Rob suggested
  this.

v1->v2:
  Use list_head instead to link resource siblings. This is suggested by
  Andrew.

  Rewrite walk_system_ram_res_rev() after list_head is taken to link
  resouce siblings.

Baoquan He (3):
  resource: Use list_head to link sibling resource
  resource: add walk_system_ram_res_rev()
  kexec_file: Load kernel at top of system RAM if required

 arch/sparc/kernel/ioport.c                  |   2 +-
 drivers/gpu/drm/drm_memory.c                |   3 +-
 drivers/gpu/drm/gma500/gtt.c                |   5 +-
 drivers/hv/vmbus_drv.c                      |  52 +++----
 drivers/input/joystick/iforce/iforce-main.c |   4 +-
 drivers/nvdimm/e820.c                       |   2 +-
 drivers/nvdimm/namespace_devs.c             |   6 +-
 drivers/nvdimm/nd.h                         |   5 +-
 drivers/of/address.c                        |   4 +-
 drivers/parisc/lba_pci.c                    |   4 +-
 drivers/pci/host/vmd.c                      |   8 +-
 drivers/pci/probe.c                         |   2 +
 drivers/pci/setup-bus.c                     |   2 +-
 include/linux/ioport.h                      |  20 ++-
 kernel/kexec_file.c                         |   2 +
 kernel/resource.c                           | 221 ++++++++++++++++------------
 16 files changed, 193 insertions(+), 149 deletions(-)

-- 
2.13.6

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v3 1/3] resource: Use list_head to link sibling resource
  2018-04-19  0:18 [PATCH v3 0/3] resource: Use list_head to link sibling resource Baoquan He
@ 2018-04-19  0:18 ` Baoquan He
  2018-04-26  1:18   ` Wei Yang
                     ` (2 more replies)
  2018-04-19  0:18 ` [PATCH v3 2/3] resource: add walk_system_ram_res_rev() Baoquan He
  2018-04-19  0:18 ` [PATCH v3 3/3] kexec_file: Load kernel at top of system RAM if required Baoquan He
  2 siblings, 3 replies; 19+ messages in thread
From: Baoquan He @ 2018-04-19  0:18 UTC (permalink / raw)
  To: linux-kernel, akpm, robh+dt, dan.j.williams, nicolas.pitre, josh
  Cc: Brijesh Singh, devicetree, David Airlie, linux-pci, Wei Yang,
	Keith Busch, Yaowei Bai, Frank Rowand, Lorenzo Pieralisi,
	Stephen Hemminger, Baoquan He, linux-nvdimm, Patrik Jakobsson,
	linux-input, Borislav Petkov, Tom Lendacky, Haiyang Zhang,
	Jérôme Glisse, Bjorn Helgaas, Thomas Gleixner,
	Jonathan Derrick, Greg Kroah-Hartman, Dmitry Torokhov, devel

The struct resource uses singly linked list to link siblings. It's not
easy to do reverse iteration on sibling list. So replace it with list_head.

And this makes codes in kernel/resource.c more readable after refactoring
than pointer operation.

Besides, type of member variables of struct resource, sibling and child, are
changed from 'struct resource *' to 'struct list_head'. This brings two
pointers of size increase.

Suggested-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Baoquan He <bhe@redhat.com>
Cc: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
Cc: David Airlie <airlied@linux.ie>
Cc: "K. Y. Srinivasan" <kys@microsoft.com>
Cc: Haiyang Zhang <haiyangz@microsoft.com>
Cc: Stephen Hemminger <sthemmin@microsoft.com>
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Frank Rowand <frowand.list@gmail.com>
Cc: Keith Busch <keith.busch@intel.com>
Cc: Jonathan Derrick <jonathan.derrick@intel.com>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: "Jérôme Glisse" <jglisse@redhat.com>
Cc: Borislav Petkov <bp@suse.de>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Yaowei Bai <baiyaowei@cmss.chinamobile.com>
Cc: Wei Yang <richard.weiyang@gmail.com>
Cc: devel@linuxdriverproject.org
Cc: linux-input@vger.kernel.org
Cc: linux-nvdimm@lists.01.org
Cc: devicetree@vger.kernel.org
Cc: linux-pci@vger.kernel.org
---
v2->v3:
  Rename resource functions first_child() and sibling() to
  resource_first_chils() and resource_sibling(). Dan suggested this.

  Move resource_first_chils() and resource_sibling() to linux/ioport.h
  and make them as inline function. Rob suggested this. Accordingly add
  linux/list.h including in linux/ioport.h, please help review if this
  bring efficiency degradation or code redundancy.

  The change on struct resource {} bring two pointers of size increase,
  mention this in git log to make it more specifically, Rob suggested
  this.

 arch/sparc/kernel/ioport.c                  |   2 +-
 drivers/gpu/drm/drm_memory.c                |   3 +-
 drivers/gpu/drm/gma500/gtt.c                |   5 +-
 drivers/hv/vmbus_drv.c                      |  52 ++++----
 drivers/input/joystick/iforce/iforce-main.c |   4 +-
 drivers/nvdimm/e820.c                       |   2 +-
 drivers/nvdimm/namespace_devs.c             |   6 +-
 drivers/nvdimm/nd.h                         |   5 +-
 drivers/of/address.c                        |   4 +-
 drivers/parisc/lba_pci.c                    |   4 +-
 drivers/pci/host/vmd.c                      |   8 +-
 drivers/pci/probe.c                         |   2 +
 drivers/pci/setup-bus.c                     |   2 +-
 include/linux/ioport.h                      |  17 ++-
 kernel/resource.c                           | 181 +++++++++++++---------------
 15 files changed, 148 insertions(+), 149 deletions(-)

diff --git a/arch/sparc/kernel/ioport.c b/arch/sparc/kernel/ioport.c
index 3bcef9ce74df..4e91fbbbedcc 100644
--- a/arch/sparc/kernel/ioport.c
+++ b/arch/sparc/kernel/ioport.c
@@ -669,7 +669,7 @@ static int sparc_io_proc_show(struct seq_file *m, void *v)
 	struct resource *root = m->private, *r;
 	const char *nm;
 
-	for (r = root->child; r != NULL; r = r->sibling) {
+	list_for_each_entry(r, &root->child, sibling) {
 		if ((nm = r->name) == NULL) nm = "???";
 		seq_printf(m, "%016llx-%016llx: %s\n",
 				(unsigned long long)r->start,
diff --git a/drivers/gpu/drm/drm_memory.c b/drivers/gpu/drm/drm_memory.c
index 3c54044214db..53e300a993dc 100644
--- a/drivers/gpu/drm/drm_memory.c
+++ b/drivers/gpu/drm/drm_memory.c
@@ -155,9 +155,8 @@ u64 drm_get_max_iomem(void)
 	struct resource *tmp;
 	resource_size_t max_iomem = 0;
 
-	for (tmp = iomem_resource.child; tmp; tmp = tmp->sibling) {
+	list_for_each_entry(tmp, &iomem_resource.child, sibling)
 		max_iomem = max(max_iomem,  tmp->end);
-	}
 
 	return max_iomem;
 }
diff --git a/drivers/gpu/drm/gma500/gtt.c b/drivers/gpu/drm/gma500/gtt.c
index 3949b0990916..addd3bc009af 100644
--- a/drivers/gpu/drm/gma500/gtt.c
+++ b/drivers/gpu/drm/gma500/gtt.c
@@ -565,7 +565,7 @@ int psb_gtt_init(struct drm_device *dev, int resume)
 int psb_gtt_restore(struct drm_device *dev)
 {
 	struct drm_psb_private *dev_priv = dev->dev_private;
-	struct resource *r = dev_priv->gtt_mem->child;
+	struct resource *r;
 	struct gtt_range *range;
 	unsigned int restored = 0, total = 0, size = 0;
 
@@ -573,14 +573,13 @@ int psb_gtt_restore(struct drm_device *dev)
 	mutex_lock(&dev_priv->gtt_mutex);
 	psb_gtt_init(dev, 1);
 
-	while (r != NULL) {
+	list_for_each_entry(r, &dev_priv->gtt_mem->child, sibling) {
 		range = container_of(r, struct gtt_range, resource);
 		if (range->pages) {
 			psb_gtt_insert(dev, range, 1);
 			size += range->resource.end - range->resource.start;
 			restored++;
 		}
-		r = r->sibling;
 		total++;
 	}
 	mutex_unlock(&dev_priv->gtt_mutex);
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index b10fe26c4891..d87ec5a1bc4c 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -1412,9 +1412,8 @@ static acpi_status vmbus_walk_resources(struct acpi_resource *res, void *ctx)
 {
 	resource_size_t start = 0;
 	resource_size_t end = 0;
-	struct resource *new_res;
+	struct resource *new_res, *tmp;
 	struct resource **old_res = &hyperv_mmio;
-	struct resource **prev_res = NULL;
 
 	switch (res->type) {
 
@@ -1461,44 +1460,36 @@ static acpi_status vmbus_walk_resources(struct acpi_resource *res, void *ctx)
 	/*
 	 * If two ranges are adjacent, merge them.
 	 */
-	do {
-		if (!*old_res) {
-			*old_res = new_res;
-			break;
-		}
-
-		if (((*old_res)->end + 1) == new_res->start) {
-			(*old_res)->end = new_res->end;
+	if (!*old_res) {
+		*old_res = new_res;
+		return AE_OK;
+	}
+	tmp = *old_res;
+	list_for_each_entry_from(tmp, &tmp->parent->child, sibling) {
+		if ((tmp->end + 1) == new_res->start) {
+			tmp->end = new_res->end;
 			kfree(new_res);
 			break;
 		}
 
-		if ((*old_res)->start == new_res->end + 1) {
-			(*old_res)->start = new_res->start;
+		if (tmp->start == new_res->end + 1) {
+			tmp->start = new_res->start;
 			kfree(new_res);
 			break;
 		}
 
-		if ((*old_res)->start > new_res->end) {
-			new_res->sibling = *old_res;
-			if (prev_res)
-				(*prev_res)->sibling = new_res;
-			*old_res = new_res;
+		if (tmp->start > new_res->end) {
+			list_add(&new_res->sibling, tmp->sibling.prev);
 			break;
 		}
-
-		prev_res = old_res;
-		old_res = &(*old_res)->sibling;
-
-	} while (1);
+	}
 
 	return AE_OK;
 }
 
 static int vmbus_acpi_remove(struct acpi_device *device)
 {
-	struct resource *cur_res;
-	struct resource *next_res;
+	struct resource *res;
 
 	if (hyperv_mmio) {
 		if (fb_mmio) {
@@ -1507,10 +1498,9 @@ static int vmbus_acpi_remove(struct acpi_device *device)
 			fb_mmio = NULL;
 		}
 
-		for (cur_res = hyperv_mmio; cur_res; cur_res = next_res) {
-			next_res = cur_res->sibling;
-			kfree(cur_res);
-		}
+		res = hyperv_mmio;
+		list_for_each_entry_from(res, &res->parent->child, sibling)
+			kfree(res);
 	}
 
 	return 0;
@@ -1596,7 +1586,8 @@ int vmbus_allocate_mmio(struct resource **new, struct hv_device *device_obj,
 		}
 	}
 
-	for (iter = hyperv_mmio; iter; iter = iter->sibling) {
+	iter = hyperv_mmio;
+	list_for_each_entry_from(iter, &iter->parent->child, sibling) {
 		if ((iter->start >= max) || (iter->end <= min))
 			continue;
 
@@ -1639,7 +1630,8 @@ void vmbus_free_mmio(resource_size_t start, resource_size_t size)
 	struct resource *iter;
 
 	down(&hyperv_mmio_lock);
-	for (iter = hyperv_mmio; iter; iter = iter->sibling) {
+	iter = hyperv_mmio;
+	list_for_each_entry_from(iter, &iter->parent->child, sibling) {
 		if ((iter->start >= start + size) || (iter->end <= start))
 			continue;
 
diff --git a/drivers/input/joystick/iforce/iforce-main.c b/drivers/input/joystick/iforce/iforce-main.c
index daeeb4c7e3b0..5c0be27b33ff 100644
--- a/drivers/input/joystick/iforce/iforce-main.c
+++ b/drivers/input/joystick/iforce/iforce-main.c
@@ -305,8 +305,8 @@ int iforce_init_device(struct iforce *iforce)
 	iforce->device_memory.end = 200;
 	iforce->device_memory.flags = IORESOURCE_MEM;
 	iforce->device_memory.parent = NULL;
-	iforce->device_memory.child = NULL;
-	iforce->device_memory.sibling = NULL;
+	INIT_LIST_HEAD(&iforce->device_memory.child);
+	INIT_LIST_HEAD(&iforce->device_memory.sibling);
 
 /*
  * Wait until device ready - until it sends its first response.
diff --git a/drivers/nvdimm/e820.c b/drivers/nvdimm/e820.c
index 6f9a6ffd7cde..513e661bb0d8 100644
--- a/drivers/nvdimm/e820.c
+++ b/drivers/nvdimm/e820.c
@@ -53,7 +53,7 @@ static int e820_pmem_probe(struct platform_device *pdev)
 		goto err;
 	platform_set_drvdata(pdev, nvdimm_bus);
 
-	for (p = iomem_resource.child; p ; p = p->sibling) {
+	list_for_each_entry(p, &iomem_resource.child, sibling) {
 		struct nd_region_desc ndr_desc;
 
 		if (p->desc != IORES_DESC_PERSISTENT_MEMORY_LEGACY)
diff --git a/drivers/nvdimm/namespace_devs.c b/drivers/nvdimm/namespace_devs.c
index 28afdd668905..f53d410d9981 100644
--- a/drivers/nvdimm/namespace_devs.c
+++ b/drivers/nvdimm/namespace_devs.c
@@ -637,7 +637,7 @@ static resource_size_t scan_allocate(struct nd_region *nd_region,
  retry:
 	first = 0;
 	for_each_dpa_resource(ndd, res) {
-		struct resource *next = res->sibling, *new_res = NULL;
+		struct resource *next = resource_sibling(res), *new_res = NULL;
 		resource_size_t allocate, available = 0;
 		enum alloc_loc loc = ALLOC_ERR;
 		const char *action;
@@ -763,7 +763,7 @@ static resource_size_t scan_allocate(struct nd_region *nd_region,
 	 * an initial "pmem-reserve pass".  Only do an initial BLK allocation
 	 * when none of the DPA space is reserved.
 	 */
-	if ((is_pmem || !ndd->dpa.child) && n == to_allocate)
+	if ((is_pmem || list_empty(&ndd->dpa.child)) && n == to_allocate)
 		return init_dpa_allocation(label_id, nd_region, nd_mapping, n);
 	return n;
 }
@@ -779,7 +779,7 @@ static int merge_dpa(struct nd_region *nd_region,
  retry:
 	for_each_dpa_resource(ndd, res) {
 		int rc;
-		struct resource *next = res->sibling;
+		struct resource *next = resource_sibling(res);
 		resource_size_t end = res->start + resource_size(res);
 
 		if (!next || strcmp(res->name, label_id->id) != 0
diff --git a/drivers/nvdimm/nd.h b/drivers/nvdimm/nd.h
index 32e0364b48b9..da7da15e03e7 100644
--- a/drivers/nvdimm/nd.h
+++ b/drivers/nvdimm/nd.h
@@ -102,11 +102,10 @@ unsigned sizeof_namespace_label(struct nvdimm_drvdata *ndd);
 		(unsigned long long) (res ? res->start : 0), ##arg)
 
 #define for_each_dpa_resource(ndd, res) \
-	for (res = (ndd)->dpa.child; res; res = res->sibling)
+	list_for_each_entry(res, &(ndd)->dpa.child, sibling)
 
 #define for_each_dpa_resource_safe(ndd, res, next) \
-	for (res = (ndd)->dpa.child, next = res ? res->sibling : NULL; \
-			res; res = next, next = next ? next->sibling : NULL)
+	list_for_each_entry_safe(res, next, &(ndd)->dpa.child, sibling)
 
 struct nd_percpu_lane {
 	int count;
diff --git a/drivers/of/address.c b/drivers/of/address.c
index 53349912ac75..e2e25719ab52 100644
--- a/drivers/of/address.c
+++ b/drivers/of/address.c
@@ -330,7 +330,9 @@ int of_pci_range_to_resource(struct of_pci_range *range,
 {
 	int err;
 	res->flags = range->flags;
-	res->parent = res->child = res->sibling = NULL;
+	res->parent = NULL;
+	INIT_LIST_HEAD(&res->child);
+	INIT_LIST_HEAD(&res->sibling);
 	res->name = np->full_name;
 
 	if (res->flags & IORESOURCE_IO) {
diff --git a/drivers/parisc/lba_pci.c b/drivers/parisc/lba_pci.c
index 69bd98421eb1..7482bdfd1959 100644
--- a/drivers/parisc/lba_pci.c
+++ b/drivers/parisc/lba_pci.c
@@ -170,8 +170,8 @@ lba_dump_res(struct resource *r, int d)
 	for (i = d; i ; --i) printk(" ");
 	printk(KERN_DEBUG "%p [%lx,%lx]/%lx\n", r,
 		(long)r->start, (long)r->end, r->flags);
-	lba_dump_res(r->child, d+2);
-	lba_dump_res(r->sibling, d);
+	lba_dump_res(resource_first_child(&r->child), d+2);
+	lba_dump_res(resource_sibling(r), d);
 }
 
 
diff --git a/drivers/pci/host/vmd.c b/drivers/pci/host/vmd.c
index 930a8fa08bd6..c3000af903ea 100644
--- a/drivers/pci/host/vmd.c
+++ b/drivers/pci/host/vmd.c
@@ -520,14 +520,14 @@ static struct pci_ops vmd_ops = {
 
 static void vmd_attach_resources(struct vmd_dev *vmd)
 {
-	vmd->dev->resource[VMD_MEMBAR1].child = &vmd->resources[1];
-	vmd->dev->resource[VMD_MEMBAR2].child = &vmd->resources[2];
+	list_add(&vmd->resources[1].sibling, &vmd->dev->resource[VMD_MEMBAR1].child);
+	list_add(&vmd->resources[2].sibling, &vmd->dev->resource[VMD_MEMBAR2].child);
 }
 
 static void vmd_detach_resources(struct vmd_dev *vmd)
 {
-	vmd->dev->resource[VMD_MEMBAR1].child = NULL;
-	vmd->dev->resource[VMD_MEMBAR2].child = NULL;
+	INIT_LIST_HEAD(&vmd->dev->resource[VMD_MEMBAR1].child);
+	INIT_LIST_HEAD(&vmd->dev->resource[VMD_MEMBAR2].child);
 }
 
 /*
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index ac91b6fd0bcd..d162c77bec29 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -59,6 +59,8 @@ static struct resource *get_pci_domain_busn_res(int domain_nr)
 	r->res.start = 0;
 	r->res.end = 0xff;
 	r->res.flags = IORESOURCE_BUS | IORESOURCE_PCI_FIXED;
+	INIT_LIST_HEAD(&r->res.child);
+	INIT_LIST_HEAD(&r->res.sibling);
 
 	list_add_tail(&r->list, &pci_domain_busn_res_list);
 
diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 072784f55ea5..0d5e30004ca6 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -2107,7 +2107,7 @@ int pci_reassign_bridge_resources(struct pci_dev *bridge, unsigned long type)
 				continue;
 
 			/* Ignore BARs which are still in use */
-			if (res->child)
+			if (!list_empty(&res->child))
 				continue;
 
 			ret = add_to_list(&saved, bridge, res, 0, 0);
diff --git a/include/linux/ioport.h b/include/linux/ioport.h
index da0ebaec25f0..225d13d3500a 100644
--- a/include/linux/ioport.h
+++ b/include/linux/ioport.h
@@ -12,6 +12,7 @@
 #ifndef __ASSEMBLY__
 #include <linux/compiler.h>
 #include <linux/types.h>
+#include <linux/list.h>
 /*
  * Resources are tree-like, allowing
  * nesting etc..
@@ -22,7 +23,8 @@ struct resource {
 	const char *name;
 	unsigned long flags;
 	unsigned long desc;
-	struct resource *parent, *sibling, *child;
+	struct list_head child, sibling;
+	struct resource *parent;
 };
 
 /*
@@ -215,7 +217,6 @@ static inline bool resource_contains(struct resource *r1, struct resource *r2)
 	return r1->start <= r2->start && r1->end >= r2->end;
 }
 
-
 /* Convenience shorthand with allocation */
 #define request_region(start,n,name)		__request_region(&ioport_resource, (start), (n), (name), 0)
 #define request_muxed_region(start,n,name)	__request_region(&ioport_resource, (start), (n), (name), IORESOURCE_MUXED)
@@ -286,6 +287,18 @@ static inline bool resource_overlaps(struct resource *r1, struct resource *r2)
        return (r1->start <= r2->end && r1->end >= r2->start);
 }
 
+static inline struct resource *resource_sibling(struct resource *res)
+{
+	if (res->parent && !list_is_last(&res->sibling, &res->parent->child))
+		return list_next_entry(res, sibling);
+	return NULL;
+}
+
+static inline struct resource *resource_first_child(struct list_head *head)
+{
+	return list_first_entry_or_null(head, struct resource, sibling);
+}
+
 
 #endif /* __ASSEMBLY__ */
 #endif	/* _LINUX_IOPORT_H */
diff --git a/kernel/resource.c b/kernel/resource.c
index 2af6c03858b9..4f560991c130 100644
--- a/kernel/resource.c
+++ b/kernel/resource.c
@@ -31,6 +31,8 @@ struct resource ioport_resource = {
 	.start	= 0,
 	.end	= IO_SPACE_LIMIT,
 	.flags	= IORESOURCE_IO,
+	.sibling = LIST_HEAD_INIT(ioport_resource.sibling),
+	.child  = LIST_HEAD_INIT(ioport_resource.child),
 };
 EXPORT_SYMBOL(ioport_resource);
 
@@ -39,6 +41,8 @@ struct resource iomem_resource = {
 	.start	= 0,
 	.end	= -1,
 	.flags	= IORESOURCE_MEM,
+	.sibling = LIST_HEAD_INIT(iomem_resource.sibling),
+	.child  = LIST_HEAD_INIT(iomem_resource.child),
 };
 EXPORT_SYMBOL(iomem_resource);
 
@@ -57,20 +61,20 @@ static DEFINE_RWLOCK(resource_lock);
  * by boot mem after the system is up. So for reusing the resource entry
  * we need to remember the resource.
  */
-static struct resource *bootmem_resource_free;
+static struct list_head bootmem_resource_free = LIST_HEAD_INIT(bootmem_resource_free);
 static DEFINE_SPINLOCK(bootmem_resource_lock);
 
 static struct resource *next_resource(struct resource *p, bool sibling_only)
 {
 	/* Caller wants to traverse through siblings only */
 	if (sibling_only)
-		return p->sibling;
+		return resource_sibling(p);
 
-	if (p->child)
-		return p->child;
-	while (!p->sibling && p->parent)
+	if (!list_empty(&p->child))
+		return resource_first_child(&p->child);
+	while (!resource_sibling(p) && p->parent)
 		p = p->parent;
-	return p->sibling;
+	return resource_sibling(p);
 }
 
 static void *r_next(struct seq_file *m, void *v, loff_t *pos)
@@ -90,7 +94,7 @@ static void *r_start(struct seq_file *m, loff_t *pos)
 	struct resource *p = m->private;
 	loff_t l = 0;
 	read_lock(&resource_lock);
-	for (p = p->child; p && l < *pos; p = r_next(m, p, &l))
+	for (p = resource_first_child(&p->child); p && l < *pos; p = r_next(m, p, &l))
 		;
 	return p;
 }
@@ -186,8 +190,7 @@ static void free_resource(struct resource *res)
 
 	if (!PageSlab(virt_to_head_page(res))) {
 		spin_lock(&bootmem_resource_lock);
-		res->sibling = bootmem_resource_free;
-		bootmem_resource_free = res;
+		list_add(&res->sibling, &bootmem_resource_free);
 		spin_unlock(&bootmem_resource_lock);
 	} else {
 		kfree(res);
@@ -199,10 +202,9 @@ static struct resource *alloc_resource(gfp_t flags)
 	struct resource *res = NULL;
 
 	spin_lock(&bootmem_resource_lock);
-	if (bootmem_resource_free) {
-		res = bootmem_resource_free;
-		bootmem_resource_free = res->sibling;
-	}
+	res = resource_first_child(&bootmem_resource_free);
+	if (res)
+		list_del(&res->sibling);
 	spin_unlock(&bootmem_resource_lock);
 
 	if (res)
@@ -210,6 +212,8 @@ static struct resource *alloc_resource(gfp_t flags)
 	else
 		res = kzalloc(sizeof(struct resource), flags);
 
+	INIT_LIST_HEAD(&res->child);
+	INIT_LIST_HEAD(&res->sibling);
 	return res;
 }
 
@@ -218,7 +222,7 @@ static struct resource * __request_resource(struct resource *root, struct resour
 {
 	resource_size_t start = new->start;
 	resource_size_t end = new->end;
-	struct resource *tmp, **p;
+	struct resource *tmp;
 
 	if (end < start)
 		return root;
@@ -226,64 +230,62 @@ static struct resource * __request_resource(struct resource *root, struct resour
 		return root;
 	if (end > root->end)
 		return root;
-	p = &root->child;
-	for (;;) {
-		tmp = *p;
-		if (!tmp || tmp->start > end) {
-			new->sibling = tmp;
-			*p = new;
+
+	if (list_empty(&root->child)) {
+		list_add(&new->sibling, &root->child);
+		new->parent = root;
+		INIT_LIST_HEAD(&new->child);
+		return NULL;
+	}
+
+	list_for_each_entry(tmp, &root->child, sibling) {
+		if (tmp->start > end) {
+			list_add(&new->sibling, tmp->sibling.prev);
 			new->parent = root;
+			INIT_LIST_HEAD(&new->child);
 			return NULL;
 		}
-		p = &tmp->sibling;
 		if (tmp->end < start)
 			continue;
 		return tmp;
 	}
+
+	list_add_tail(&new->sibling, &root->child);
+	new->parent = root;
+	INIT_LIST_HEAD(&new->child);
+	return NULL;
 }
 
 static int __release_resource(struct resource *old, bool release_child)
 {
-	struct resource *tmp, **p, *chd;
+	struct resource *tmp, *next, *chd;
 
-	p = &old->parent->child;
-	for (;;) {
-		tmp = *p;
-		if (!tmp)
-			break;
+	list_for_each_entry_safe(tmp, next, &old->parent->child, sibling) {
 		if (tmp == old) {
-			if (release_child || !(tmp->child)) {
-				*p = tmp->sibling;
+			if (release_child || list_empty(&tmp->child)) {
+				list_del(&tmp->sibling);
 			} else {
-				for (chd = tmp->child;; chd = chd->sibling) {
+				list_for_each_entry(chd, &tmp->child, sibling)
 					chd->parent = tmp->parent;
-					if (!(chd->sibling))
-						break;
-				}
-				*p = tmp->child;
-				chd->sibling = tmp->sibling;
+				list_splice(&tmp->child, tmp->sibling.prev);
+				list_del(&tmp->sibling);
 			}
+
 			old->parent = NULL;
 			return 0;
 		}
-		p = &tmp->sibling;
 	}
 	return -EINVAL;
 }
 
 static void __release_child_resources(struct resource *r)
 {
-	struct resource *tmp, *p;
+	struct resource *tmp, *next;
 	resource_size_t size;
 
-	p = r->child;
-	r->child = NULL;
-	while (p) {
-		tmp = p;
-		p = p->sibling;
-
+	list_for_each_entry_safe(tmp, next, &r->child, sibling) {
 		tmp->parent = NULL;
-		tmp->sibling = NULL;
+		INIT_LIST_HEAD(&tmp->sibling);
 		__release_child_resources(tmp);
 
 		printk(KERN_DEBUG "release child resource %pR\n", tmp);
@@ -292,6 +294,8 @@ static void __release_child_resources(struct resource *r)
 		tmp->start = 0;
 		tmp->end = size - 1;
 	}
+
+	INIT_LIST_HEAD(&tmp->child);
 }
 
 void release_child_resources(struct resource *r)
@@ -376,7 +380,8 @@ static int find_next_iomem_res(struct resource *res, unsigned long desc,
 
 	read_lock(&resource_lock);
 
-	for (p = iomem_resource.child; p; p = next_resource(p, sibling_only)) {
+	for (p = resource_first_child(&iomem_resource.child); p;
+			p = next_resource(p, sibling_only)) {
 		if ((p->flags & res->flags) != res->flags)
 			continue;
 		if ((desc != IORES_DESC_NONE) && (desc != p->desc))
@@ -564,7 +569,7 @@ int region_intersects(resource_size_t start, size_t size, unsigned long flags,
 	struct resource *p;
 
 	read_lock(&resource_lock);
-	for (p = iomem_resource.child; p ; p = p->sibling) {
+	list_for_each_entry(p, &iomem_resource.child, sibling) {
 		bool is_type = (((p->flags & flags) == flags) &&
 				((desc == IORES_DESC_NONE) ||
 				 (desc == p->desc)));
@@ -618,7 +623,7 @@ static int __find_resource(struct resource *root, struct resource *old,
 			 resource_size_t  size,
 			 struct resource_constraint *constraint)
 {
-	struct resource *this = root->child;
+	struct resource *this = resource_first_child(&root->child);
 	struct resource tmp = *new, avail, alloc;
 
 	tmp.start = root->start;
@@ -628,7 +633,7 @@ static int __find_resource(struct resource *root, struct resource *old,
 	 */
 	if (this && this->start == root->start) {
 		tmp.start = (this == old) ? old->start : this->end + 1;
-		this = this->sibling;
+		this = resource_sibling(this);
 	}
 	for(;;) {
 		if (this)
@@ -664,7 +669,7 @@ next:		if (!this || this->end == root->end)
 
 		if (this != old)
 			tmp.start = this->end + 1;
-		this = this->sibling;
+		this = resource_sibling(this);
 	}
 	return -EBUSY;
 }
@@ -708,7 +713,7 @@ static int reallocate_resource(struct resource *root, struct resource *old,
 		goto out;
 	}
 
-	if (old->child) {
+	if (!list_empty(&old->child)) {
 		err = -EBUSY;
 		goto out;
 	}
@@ -789,7 +794,7 @@ struct resource *lookup_resource(struct resource *root, resource_size_t start)
 	struct resource *res;
 
 	read_lock(&resource_lock);
-	for (res = root->child; res; res = res->sibling) {
+	list_for_each_entry(res, &root->child, sibling) {
 		if (res->start == start)
 			break;
 	}
@@ -822,32 +827,27 @@ static struct resource * __insert_resource(struct resource *parent, struct resou
 			break;
 	}
 
-	for (next = first; ; next = next->sibling) {
+	for (next = first; ; next = resource_sibling(next)) {
 		/* Partial overlap? Bad, and unfixable */
 		if (next->start < new->start || next->end > new->end)
 			return next;
-		if (!next->sibling)
+		if (!resource_sibling(next))
 			break;
-		if (next->sibling->start > new->end)
+		if (resource_sibling(next)->start > new->end)
 			break;
 	}
-
 	new->parent = parent;
-	new->sibling = next->sibling;
-	new->child = first;
+	list_add(&new->sibling, &next->sibling);
+	INIT_LIST_HEAD(&new->child);
 
-	next->sibling = NULL;
-	for (next = first; next; next = next->sibling)
+	/*
+	 * From first to next, they all fall into new's region, so change them
+	 * as new's children.
+	 */
+	list_cut_position(&new->child, first->sibling.prev, &next->sibling);
+	list_for_each_entry(next, &new->child, sibling)
 		next->parent = new;
 
-	if (parent->child == first) {
-		parent->child = new;
-	} else {
-		next = parent->child;
-		while (next->sibling != first)
-			next = next->sibling;
-		next->sibling = new;
-	}
 	return NULL;
 }
 
@@ -969,19 +969,17 @@ static int __adjust_resource(struct resource *res, resource_size_t start,
 	if ((start < parent->start) || (end > parent->end))
 		goto out;
 
-	if (res->sibling && (res->sibling->start <= end))
+	if (resource_sibling(res) && (resource_sibling(res)->start <= end))
 		goto out;
 
-	tmp = parent->child;
-	if (tmp != res) {
-		while (tmp->sibling != res)
-			tmp = tmp->sibling;
+	if (res->sibling.prev != &parent->child) {
+		tmp = list_prev_entry(res, sibling);
 		if (start <= tmp->end)
 			goto out;
 	}
 
 skip:
-	for (tmp = res->child; tmp; tmp = tmp->sibling)
+	list_for_each_entry(tmp, &res->child, sibling)
 		if ((tmp->start < start) || (tmp->end > end))
 			goto out;
 
@@ -1206,34 +1204,32 @@ EXPORT_SYMBOL(__request_region);
 void __release_region(struct resource *parent, resource_size_t start,
 			resource_size_t n)
 {
-	struct resource **p;
+	struct resource *res;
 	resource_size_t end;
 
-	p = &parent->child;
+	res = resource_first_child(&parent->child);
 	end = start + n - 1;
 
 	write_lock(&resource_lock);
 
 	for (;;) {
-		struct resource *res = *p;
-
 		if (!res)
 			break;
 		if (res->start <= start && res->end >= end) {
 			if (!(res->flags & IORESOURCE_BUSY)) {
-				p = &res->child;
+				res = resource_first_child(&res->child);
 				continue;
 			}
 			if (res->start != start || res->end != end)
 				break;
-			*p = res->sibling;
+			list_del(&res->sibling);
 			write_unlock(&resource_lock);
 			if (res->flags & IORESOURCE_MUXED)
 				wake_up(&muxed_resource_wait);
 			free_resource(res);
 			return;
 		}
-		p = &res->sibling;
+		res = resource_sibling(res);
 	}
 
 	write_unlock(&resource_lock);
@@ -1268,9 +1264,7 @@ EXPORT_SYMBOL(__release_region);
 int release_mem_region_adjustable(struct resource *parent,
 			resource_size_t start, resource_size_t size)
 {
-	struct resource **p;
-	struct resource *res;
-	struct resource *new_res;
+	struct resource *res, *new_res;
 	resource_size_t end;
 	int ret = -EINVAL;
 
@@ -1281,16 +1275,16 @@ int release_mem_region_adjustable(struct resource *parent,
 	/* The alloc_resource() result gets checked later */
 	new_res = alloc_resource(GFP_KERNEL);
 
-	p = &parent->child;
+	res = resource_first_child(&parent->child);
 	write_lock(&resource_lock);
 
-	while ((res = *p)) {
+	while ((res)) {
 		if (res->start >= end)
 			break;
 
 		/* look for the next resource if it does not fit into */
 		if (res->start > start || res->end < end) {
-			p = &res->sibling;
+			res = resource_sibling(res);
 			continue;
 		}
 
@@ -1298,14 +1292,14 @@ int release_mem_region_adjustable(struct resource *parent,
 			break;
 
 		if (!(res->flags & IORESOURCE_BUSY)) {
-			p = &res->child;
+			res = resource_first_child(&res->child);
 			continue;
 		}
 
 		/* found the target resource; let's adjust accordingly */
 		if (res->start == start && res->end == end) {
 			/* free the whole entry */
-			*p = res->sibling;
+			list_del(&res->sibling);
 			free_resource(res);
 			ret = 0;
 		} else if (res->start == start && res->end != end) {
@@ -1328,14 +1322,13 @@ int release_mem_region_adjustable(struct resource *parent,
 			new_res->flags = res->flags;
 			new_res->desc = res->desc;
 			new_res->parent = res->parent;
-			new_res->sibling = res->sibling;
-			new_res->child = NULL;
+			INIT_LIST_HEAD(&new_res->child);
 
 			ret = __adjust_resource(res, res->start,
 						start - res->start);
 			if (ret)
 				break;
-			res->sibling = new_res;
+			list_add(&new_res->sibling, &res->sibling);
 			new_res = NULL;
 		}
 
@@ -1516,7 +1509,7 @@ static int __init reserve_setup(char *str)
 			res->end = io_start + io_num - 1;
 			res->flags |= IORESOURCE_BUSY;
 			res->desc = IORES_DESC_NONE;
-			res->child = NULL;
+			INIT_LIST_HEAD(&res->child);
 			if (request_resource(parent, res) == 0)
 				reserved = x+1;
 		}
@@ -1536,7 +1529,7 @@ int iomem_map_sanity_check(resource_size_t addr, unsigned long size)
 	loff_t l;
 
 	read_lock(&resource_lock);
-	for (p = p->child; p ; p = r_next(NULL, p, &l)) {
+	for (p = resource_first_child(&p->child); p; p = r_next(NULL, p, &l)) {
 		/*
 		 * We can probably skip the resources without
 		 * IORESOURCE_IO attribute?
@@ -1592,7 +1585,7 @@ bool iomem_is_exclusive(u64 addr)
 	addr = addr & PAGE_MASK;
 
 	read_lock(&resource_lock);
-	for (p = p->child; p ; p = r_next(NULL, p, &l)) {
+	for (p = resource_first_child(&p->child); p; p = r_next(NULL, p, &l)) {
 		/*
 		 * We can probably skip the resources without
 		 * IORESOURCE_IO attribute?
-- 
2.13.6

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v3 2/3] resource: add walk_system_ram_res_rev()
  2018-04-19  0:18 [PATCH v3 0/3] resource: Use list_head to link sibling resource Baoquan He
  2018-04-19  0:18 ` [PATCH v3 1/3] " Baoquan He
@ 2018-04-19  0:18 ` Baoquan He
  2018-04-19 10:07   ` Borislav Petkov
  2018-04-19  0:18 ` [PATCH v3 3/3] kexec_file: Load kernel at top of system RAM if required Baoquan He
  2 siblings, 1 reply; 19+ messages in thread
From: Baoquan He @ 2018-04-19  0:18 UTC (permalink / raw)
  To: linux-kernel, akpm, robh+dt, dan.j.williams, nicolas.pitre, josh
  Cc: Baoquan He, Thomas Gleixner, Brijesh Singh,
	Jérôme Glisse, Borislav Petkov, Tom Lendacky, Wei Yang

This function, being a variant of walk_system_ram_res() introduced in
commit 8c86e70acead ("resource: provide new functions to walk through
resources"), walks through a list of all the resources of System RAM
in reversed order, i.e., from higher to lower.

It will be used in kexec_file code.

Signed-off-by: Baoquan He <bhe@redhat.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: "Jérôme Glisse" <jglisse@redhat.com>
Cc: Borislav Petkov <bp@suse.de>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Wei Yang <richard.weiyang@gmail.com>
---
 include/linux/ioport.h |  3 +++
 kernel/resource.c      | 40 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 43 insertions(+)

diff --git a/include/linux/ioport.h b/include/linux/ioport.h
index 225d13d3500a..5f2cfb460a14 100644
--- a/include/linux/ioport.h
+++ b/include/linux/ioport.h
@@ -278,6 +278,9 @@ extern int
 walk_system_ram_res(u64 start, u64 end, void *arg,
 		    int (*func)(struct resource *, void *));
 extern int
+walk_system_ram_res_rev(u64 start, u64 end, void *arg,
+			int (*func)(struct resource *, void *));
+extern int
 walk_iomem_res_desc(unsigned long desc, unsigned long flags, u64 start, u64 end,
 		    void *arg, int (*func)(struct resource *, void *));
 
diff --git a/kernel/resource.c b/kernel/resource.c
index 4f560991c130..6c519b06e3d6 100644
--- a/kernel/resource.c
+++ b/kernel/resource.c
@@ -23,6 +23,8 @@
 #include <linux/pfn.h>
 #include <linux/mm.h>
 #include <linux/resource_ext.h>
+#include <linux/string.h>
+#include <linux/vmalloc.h>
 #include <asm/io.h>
 
 
@@ -475,6 +477,44 @@ int walk_system_ram_res(u64 start, u64 end, void *arg,
 }
 
 /*
+ * This function, being a variant of walk_system_ram_res(), calls the @func
+ * callback against all memory ranges of type System RAM which are marked as
+ * IORESOURCE_SYSTEM_RAM and IORESOUCE_BUSY in reversed order, i.e., from
+ * higher to lower.
+ */
+int walk_system_ram_res_rev(u64 start, u64 end, void *arg,
+				int (*func)(struct resource *, void *))
+{
+	unsigned long flags;
+	struct resource *res;
+	int ret = -1;
+
+	flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY;
+
+	read_lock(&resource_lock);
+	list_for_each_entry_reverse(res, &iomem_resource.child, sibling) {
+		if (start >= end)
+			break;
+		if ((res->flags & flags) != flags)
+			continue;
+		if (res->desc != IORES_DESC_NONE)
+			continue;
+		if (res->end < start)
+			break;
+
+		if ((res->end >= start) && (res->start < end)) {
+			ret = (*func)(res, arg);
+			if (ret)
+				break;
+		}
+		end = res->start - 1;
+
+	}
+	read_unlock(&resource_lock);
+	return ret;
+}
+
+/*
  * This function calls the @func callback against all memory ranges, which
  * are ranges marked as IORESOURCE_MEM and IORESOUCE_BUSY.
  */
-- 
2.13.6

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v3 3/3] kexec_file: Load kernel at top of system RAM if required
  2018-04-19  0:18 [PATCH v3 0/3] resource: Use list_head to link sibling resource Baoquan He
  2018-04-19  0:18 ` [PATCH v3 1/3] " Baoquan He
  2018-04-19  0:18 ` [PATCH v3 2/3] resource: add walk_system_ram_res_rev() Baoquan He
@ 2018-04-19  0:18 ` Baoquan He
  2 siblings, 0 replies; 19+ messages in thread
From: Baoquan He @ 2018-04-19  0:18 UTC (permalink / raw)
  To: linux-kernel, akpm, robh+dt, dan.j.williams, nicolas.pitre, josh
  Cc: Baoquan He, Eric Biederman, Vivek Goyal, Dave Young, Yinghai Lu, kexec

For kexec_file loading, if kexec_buf.top_down is 'true', the memory which
is used to load kernel/initrd/purgatory is supposed to be allocated from
top to down. This is what we have been doing all along in the old kexec
loading interface and the kexec loading is still default setting in some
distributions. However, the current kexec_file loading interface doesn't
do likt this. The function arch_kexec_walk_mem() it calls ignores checking
kexec_buf.top_down, but calls walk_system_ram_res() directly to go through
all resources of System RAM from bottom to up, to try to find memory region
which can contain the specific kexec buffer, then call locate_mem_hole_callback()
to allocate memory in that found memory region from top to down. This brings
confusion. These two interfaces need be unified on behaviour.

Here add checking if kexec_buf.top_down is 'true' in arch_kexec_walk_mem(),
if yes, call the newly added walk_system_ram_res_rev() to find memory region
from top to down to load kernel.

Signed-off-by: Baoquan He <bhe@redhat.com>
Cc: Eric Biederman <ebiederm@xmission.com>
Cc: Vivek Goyal <vgoyal@redhat.com>
Cc: Dave Young <dyoung@redhat.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Yinghai Lu <yinghai@kernel.org>
Cc: kexec@lists.infradead.org
---
 kernel/kexec_file.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index 75d8e7cf040e..7a66d9d5a534 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -518,6 +518,8 @@ int __weak arch_kexec_walk_mem(struct kexec_buf *kbuf,
 					   IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY,
 					   crashk_res.start, crashk_res.end,
 					   kbuf, func);
+	else if (kbuf->top_down)
+		return walk_system_ram_res_rev(0, ULONG_MAX, kbuf, func);
 	else
 		return walk_system_ram_res(0, ULONG_MAX, kbuf, func);
 }
-- 
2.13.6

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v3 2/3] resource: add walk_system_ram_res_rev()
  2018-04-19  0:18 ` [PATCH v3 2/3] resource: add walk_system_ram_res_rev() Baoquan He
@ 2018-04-19 10:07   ` Borislav Petkov
  2018-04-26  8:56     ` Baoquan He
  0 siblings, 1 reply; 19+ messages in thread
From: Borislav Petkov @ 2018-04-19 10:07 UTC (permalink / raw)
  To: Baoquan He
  Cc: linux-kernel, akpm, robh+dt, dan.j.williams, nicolas.pitre, josh,
	Thomas Gleixner, Brijesh Singh, Jérôme Glisse,
	Tom Lendacky, Wei Yang

On Thu, Apr 19, 2018 at 08:18:47AM +0800, Baoquan He wrote:
> This function, being a variant of walk_system_ram_res() introduced in
> commit 8c86e70acead ("resource: provide new functions to walk through
> resources"), walks through a list of all the resources of System RAM
> in reversed order, i.e., from higher to lower.
> 
> It will be used in kexec_file code.

Of course, what is missing is the big *WHY* you need this to happen this
way...

>  /*
> + * This function, being a variant of walk_system_ram_res(), calls the @func
> + * callback against all memory ranges of type System RAM which are marked as
> + * IORESOURCE_SYSTEM_RAM and IORESOUCE_BUSY in reversed order, i.e., from
> + * higher to lower.
> + */
> +int walk_system_ram_res_rev(u64 start, u64 end, void *arg,
> +				int (*func)(struct resource *, void *))
> +{
> +	unsigned long flags;
> +	struct resource *res;
> +	int ret = -1;
> +
> +	flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY;
> +
> +	read_lock(&resource_lock);
> +	list_for_each_entry_reverse(res, &iomem_resource.child, sibling) {

... and the other thing that I'm not clear on is why are you
slapping this function just like that instead of extending
__walk_iomem_res_desc() to do reverse direction too?

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v3 1/3] resource: Use list_head to link sibling resource
  2018-04-19  0:18 ` [PATCH v3 1/3] " Baoquan He
@ 2018-04-26  1:18   ` Wei Yang
  2018-05-07  1:14     ` Baoquan He
  2018-04-26  3:01   ` kbuild test robot
  2018-04-26  3:23   ` kbuild test robot
  2 siblings, 1 reply; 19+ messages in thread
From: Wei Yang @ 2018-04-26  1:18 UTC (permalink / raw)
  To: Baoquan He
  Cc: nicolas.pitre, Brijesh Singh, devicetree, David Airlie,
	linux-pci, Wei Yang, Keith Busch, Yaowei Bai, Frank Rowand,
	dan.j.williams, Lorenzo Pieralisi, Stephen Hemminger,
	linux-nvdimm, Patrik Jakobsson, linux-input, Borislav Petkov,
	Tom Lendacky, Haiyang Zhang, josh, Jérôme Glisse,
	robh+dt, Bjorn Helgaas, Thomas Gleixner, Jonathan Derrick,
	Greg Kroah-Hartman, Dmitry Torokhov, linux-kernel, devel, akpm

On Thu, Apr 19, 2018 at 08:18:46AM +0800, Baoquan He wrote:
>The struct resource uses singly linked list to link siblings. It's not
>easy to do reverse iteration on sibling list. So replace it with list_head.
>

Hi, Baoquan

Besides changing the data structure, I have another proposal to do the reverse
iteration. Which means it would not affect other users, if you just want a
reverse iteration.

BTW, I don't think Andrew suggest to use linked-list directly. What he wants
is a better solution to your first proposal in
https://patchwork.kernel.org/patch/10300819/.

Below is my proposal of resource reverse iteration without changing current
design.

>From 5d7145d44fe48b98572a03884fa3a3aa82e3cef9 Mon Sep 17 00:00:00 2001
From: Wei Yang <richard.weiyang@gmail.com>
Date: Sat, 24 Mar 2018 23:25:46 +0800
Subject: [PATCH] kernel/resource: add walk_system_ram_res_rev()

As discussed on https://patchwork.kernel.org/patch/10300819/, this patch
comes up with a variant implementation of walk_system_ram_res_rev(), which
uses iteration instead of allocating array to store those resources.

Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
---
 include/linux/ioport.h |   3 ++
 kernel/resource.c      | 113 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 116 insertions(+)

diff --git a/include/linux/ioport.h b/include/linux/ioport.h
index da0ebaec25f0..473f1d9cb97e 100644
--- a/include/linux/ioport.h
+++ b/include/linux/ioport.h
@@ -277,6 +277,9 @@ extern int
 walk_system_ram_res(u64 start, u64 end, void *arg,
 		    int (*func)(struct resource *, void *));
 extern int
+walk_system_ram_res_rev(u64 start, u64 end, void *arg,
+		    int (*func)(struct resource *, void *));
+extern int
 walk_iomem_res_desc(unsigned long desc, unsigned long flags, u64 start, u64 end,
 		    void *arg, int (*func)(struct resource *, void *));
 
diff --git a/kernel/resource.c b/kernel/resource.c
index 769109f20fb7..d4ec5fbc6875 100644
--- a/kernel/resource.c
+++ b/kernel/resource.c
@@ -73,6 +73,38 @@ static struct resource *next_resource(struct resource *p, bool sibling_only)
 	return p->sibling;
 }
 
+static struct resource *prev_resource(struct resource *p, bool sibling_only)
+{
+	struct resource *prev;
+	if (NULL == iomem_resource.child)
+		return NULL;
+
+	if (p == NULL) {
+		prev = iomem_resource.child;
+		while (prev->sibling)
+			prev = prev->sibling;
+	} else {
+		if (p->parent->child == p) {
+			return p->parent;
+		}
+
+		for (prev = p->parent->child; prev->sibling != p;
+			prev = prev->sibling) {}
+	}
+
+	/* Caller wants to traverse through siblings only */
+	if (sibling_only)
+		return prev;
+
+	for (;prev->child;) {
+		prev = prev->child;
+
+		while (prev->sibling)
+			prev = prev->sibling;
+	}
+	return prev;
+}
+
 static void *r_next(struct seq_file *m, void *v, loff_t *pos)
 {
 	struct resource *p = v;
@@ -401,6 +433,47 @@ static int find_next_iomem_res(struct resource *res, unsigned long desc,
 	return 0;
 }
 
+/*
+ * Finds the highest iomem resource existing within [res->start.res->end).
+ * The caller must specify res->start, res->end, res->flags, and optionally
+ * desc.  If found, returns 0, res is overwritten, if not found, returns -1.
+ * This function walks the whole tree and not just first level children until
+ * and unless first_level_children_only is true.
+ */
+static int find_prev_iomem_res(struct resource *res, unsigned long desc,
+			       bool first_level_children_only)
+{
+	struct resource *p;
+
+	BUG_ON(!res);
+	BUG_ON(res->start >= res->end);
+
+	read_lock(&resource_lock);
+
+	for (p = prev_resource(NULL, first_level_children_only); p;
+		p = prev_resource(p, first_level_children_only)) {
+		if ((p->flags & res->flags) != res->flags)
+			continue;
+		if ((desc != IORES_DESC_NONE) && (desc != p->desc))
+			continue;
+		if (p->end < res->start || p->child == iomem_resource.child) {
+			p = NULL;
+			break;
+		}
+		if ((p->end >= res->start) && (p->start < res->end))
+			break;
+	}
+
+	read_unlock(&resource_lock);
+	if (!p)
+		return -1;
+	/* copy data */
+	resource_clip(res, p->start, p->end);
+	res->flags = p->flags;
+	res->desc = p->desc;
+	return 0;
+}
+
 static int __walk_iomem_res_desc(struct resource *res, unsigned long desc,
 				 bool first_level_children_only,
 				 void *arg,
@@ -422,6 +495,27 @@ static int __walk_iomem_res_desc(struct resource *res, unsigned long desc,
 	return ret;
 }
 
+static int __walk_iomem_res_rev_desc(struct resource *res, unsigned long desc,
+				 bool first_level_children_only,
+				 void *arg,
+				 int (*func)(struct resource *, void *))
+{
+	u64 orig_start = res->start;
+	int ret = -1;
+
+	while ((res->start < res->end) &&
+	       !find_prev_iomem_res(res, desc, first_level_children_only)) {
+		ret = (*func)(res, arg);
+		if (ret)
+			break;
+
+		res->end = res->start?(res->start - 1):0;
+		res->start = orig_start;
+	}
+
+	return ret;
+}
+
 /*
  * Walks through iomem resources and calls func() with matching resource
  * ranges. This walks through whole tree and not just first level children.
@@ -468,6 +562,25 @@ int walk_system_ram_res(u64 start, u64 end, void *arg,
 				     arg, func);
 }
 
+/*
+ * This function, being a variant of walk_system_ram_res(), calls the @func
+ * callback against all memory ranges of type System RAM which are marked as
+ * IORESOURCE_SYSTEM_RAM and IORESOUCE_BUSY in reversed order, i.e., from
+ * higher to lower.
+ */
+int walk_system_ram_res_rev(u64 start, u64 end, void *arg,
+				int (*func)(struct resource *, void *))
+{
+	struct resource res;
+
+	res.start = start;
+	res.end = end;
+	res.flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY;
+
+	return __walk_iomem_res_rev_desc(&res, IORES_DESC_NONE, true,
+				     arg, func);
+}
+
 /*
  * This function calls the @func callback against all memory ranges, which
  * are ranges marked as IORESOURCE_MEM and IORESOUCE_BUSY.
-- 
2.15.1


-- 
Wei Yang
Help you, Help me
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v3 1/3] resource: Use list_head to link sibling resource
  2018-04-19  0:18 ` [PATCH v3 1/3] " Baoquan He
  2018-04-26  1:18   ` Wei Yang
@ 2018-04-26  3:01   ` kbuild test robot
  2018-05-06  6:31     ` Baoquan He
  2018-04-26  3:23   ` kbuild test robot
  2 siblings, 1 reply; 19+ messages in thread
From: kbuild test robot @ 2018-04-26  3:01 UTC (permalink / raw)
  To: Baoquan He
  Cc: kbuild-all, linux-kernel, akpm, robh+dt, dan.j.williams,
	nicolas.pitre, josh, Brijesh Singh, devicetree, David Airlie,
	linux-pci, Wei Yang, Keith Busch, Yaowei Bai, Frank Rowand,
	Lorenzo Pieralisi, Stephen Hemminger, Baoquan He, linux-nvdimm,
	Patrik Jakobsson, linux-input, Borislav Petkov, Tom Lendacky,
	Haiyang Zhang, Jérôme Glisse, Bjorn Helgaas,
	Thomas Gleixner, Jonathan Derrick, Greg Kroah-Hartman,
	Dmitry Torokhov, devel

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

Hi Baoquan,

I love your patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v4.17-rc2 next-20180424]
[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/Baoquan-He/resource-Use-list_head-to-link-sibling-resource/20180419-223752
config: microblaze-mmu_defconfig (attached as .config)
compiler: microblaze-linux-gcc (GCC) 7.2.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
        make.cross ARCH=microblaze 

All errors (new ones prefixed by >>):

   arch/microblaze/pci/pci-common.c: In function 'pci_process_bridge_OF_ranges':
>> arch/microblaze/pci/pci-common.c:536:44: error: incompatible types when assigning to type 'struct list_head' from type 'void *'
       res->parent = res->child = res->sibling = NULL;
                                               ^
   arch/microblaze/pci/pci-common.c: In function 'reparent_resources':
>> arch/microblaze/pci/pci-common.c:631:10: error: assignment from incompatible pointer type [-Werror=incompatible-pointer-types]
     for (pp = &parent->child; (p = *pp) != NULL; pp = &p->sibling) {
             ^
   arch/microblaze/pci/pci-common.c:631:50: error: assignment from incompatible pointer type [-Werror=incompatible-pointer-types]
     for (pp = &parent->child; (p = *pp) != NULL; pp = &p->sibling) {
                                                     ^
>> arch/microblaze/pci/pci-common.c:644:13: error: incompatible types when assigning to type 'struct list_head' from type 'struct resource *'
     res->child = *firstpp;
                ^
   arch/microblaze/pci/pci-common.c:645:15: error: incompatible types when assigning to type 'struct list_head' from type 'struct resource *'
     res->sibling = *pp;
                  ^
>> arch/microblaze/pci/pci-common.c:648:9: error: incompatible types when assigning to type 'struct resource *' from type 'struct list_head'
     for (p = res->child; p != NULL; p = p->sibling) {
            ^
   arch/microblaze/pci/pci-common.c:648:36: error: incompatible types when assigning to type 'struct resource *' from type 'struct list_head'
     for (p = res->child; p != NULL; p = p->sibling) {
                                       ^
   cc1: some warnings being treated as errors

vim +536 arch/microblaze/pci/pci-common.c

d3afa58c Michal Simek        2010-01-18  387  
d3afa58c Michal Simek        2010-01-18  388  /**
d3afa58c Michal Simek        2010-01-18  389   * pci_process_bridge_OF_ranges - Parse PCI bridge resources from device tree
d3afa58c Michal Simek        2010-01-18  390   * @hose: newly allocated pci_controller to be setup
d3afa58c Michal Simek        2010-01-18  391   * @dev: device node of the host bridge
d3afa58c Michal Simek        2010-01-18  392   * @primary: set if primary bus (32 bits only, soon to be deprecated)
d3afa58c Michal Simek        2010-01-18  393   *
d3afa58c Michal Simek        2010-01-18  394   * This function will parse the "ranges" property of a PCI host bridge device
d3afa58c Michal Simek        2010-01-18  395   * node and setup the resource mapping of a pci controller based on its
d3afa58c Michal Simek        2010-01-18  396   * content.
d3afa58c Michal Simek        2010-01-18  397   *
d3afa58c Michal Simek        2010-01-18  398   * Life would be boring if it wasn't for a few issues that we have to deal
d3afa58c Michal Simek        2010-01-18  399   * with here:
d3afa58c Michal Simek        2010-01-18  400   *
d3afa58c Michal Simek        2010-01-18  401   *   - We can only cope with one IO space range and up to 3 Memory space
d3afa58c Michal Simek        2010-01-18  402   *     ranges. However, some machines (thanks Apple !) tend to split their
d3afa58c Michal Simek        2010-01-18  403   *     space into lots of small contiguous ranges. So we have to coalesce.
d3afa58c Michal Simek        2010-01-18  404   *
d3afa58c Michal Simek        2010-01-18  405   *   - We can only cope with all memory ranges having the same offset
d3afa58c Michal Simek        2010-01-18  406   *     between CPU addresses and PCI addresses. Unfortunately, some bridges
d3afa58c Michal Simek        2010-01-18  407   *     are setup for a large 1:1 mapping along with a small "window" which
d3afa58c Michal Simek        2010-01-18  408   *     maps PCI address 0 to some arbitrary high address of the CPU space in
d3afa58c Michal Simek        2010-01-18  409   *     order to give access to the ISA memory hole.
d3afa58c Michal Simek        2010-01-18  410   *     The way out of here that I've chosen for now is to always set the
d3afa58c Michal Simek        2010-01-18  411   *     offset based on the first resource found, then override it if we
d3afa58c Michal Simek        2010-01-18  412   *     have a different offset and the previous was set by an ISA hole.
d3afa58c Michal Simek        2010-01-18  413   *
d3afa58c Michal Simek        2010-01-18  414   *   - Some busses have IO space not starting at 0, which causes trouble with
d3afa58c Michal Simek        2010-01-18  415   *     the way we do our IO resource renumbering. The code somewhat deals with
d3afa58c Michal Simek        2010-01-18  416   *     it for 64 bits but I would expect problems on 32 bits.
d3afa58c Michal Simek        2010-01-18  417   *
d3afa58c Michal Simek        2010-01-18  418   *   - Some 32 bits platforms such as 4xx can have physical space larger than
d3afa58c Michal Simek        2010-01-18  419   *     32 bits so we need to use 64 bits values for the parsing
d3afa58c Michal Simek        2010-01-18  420   */
b881bc46 Greg Kroah-Hartman  2012-12-21  421  void pci_process_bridge_OF_ranges(struct pci_controller *hose,
b881bc46 Greg Kroah-Hartman  2012-12-21  422  				  struct device_node *dev, int primary)
d3afa58c Michal Simek        2010-01-18  423  {
d3afa58c Michal Simek        2010-01-18  424  	int memno = 0, isa_hole = -1;
d3afa58c Michal Simek        2010-01-18  425  	unsigned long long isa_mb = 0;
d3afa58c Michal Simek        2010-01-18  426  	struct resource *res;
4f7b6de4 Andrew Murray       2013-07-27  427  	struct of_pci_range range;
4f7b6de4 Andrew Murray       2013-07-27  428  	struct of_pci_range_parser parser;
d3afa58c Michal Simek        2010-01-18  429  
f2b8ae0e Rob Herring         2017-06-06  430  	pr_info("PCI host bridge %pOF %s ranges:\n",
f2b8ae0e Rob Herring         2017-06-06  431  	       dev, primary ? "(primary)" : "");
d3afa58c Michal Simek        2010-01-18  432  
4f7b6de4 Andrew Murray       2013-07-27  433  	/* Check for ranges property */
4f7b6de4 Andrew Murray       2013-07-27  434  	if (of_pci_range_parser_init(&parser, dev))
d3afa58c Michal Simek        2010-01-18  435  		return;
d3afa58c Michal Simek        2010-01-18  436  
d3afa58c Michal Simek        2010-01-18  437  	pr_debug("Parsing ranges property...\n");
4f7b6de4 Andrew Murray       2013-07-27  438  	for_each_of_pci_range(&parser, &range) {
d3afa58c Michal Simek        2010-01-18  439  		/* Read next ranges element */
6bd55f0b Michal Simek        2012-12-27  440  		pr_debug("pci_space: 0x%08x pci_addr:0x%016llx ",
4f7b6de4 Andrew Murray       2013-07-27  441  				range.pci_space, range.pci_addr);
6bd55f0b Michal Simek        2012-12-27  442  		pr_debug("cpu_addr:0x%016llx size:0x%016llx\n",
4f7b6de4 Andrew Murray       2013-07-27  443  					range.cpu_addr, range.size);
d3afa58c Michal Simek        2010-01-18  444  
d3afa58c Michal Simek        2010-01-18  445  		/* If we failed translation or got a zero-sized region
d3afa58c Michal Simek        2010-01-18  446  		 * (some FW try to feed us with non sensical zero sized regions
d3afa58c Michal Simek        2010-01-18  447  		 * such as power3 which look like some kind of attempt
d3afa58c Michal Simek        2010-01-18  448  		 * at exposing the VGA memory hole)
d3afa58c Michal Simek        2010-01-18  449  		 */
4f7b6de4 Andrew Murray       2013-07-27  450  		if (range.cpu_addr == OF_BAD_ADDR || range.size == 0)
d3afa58c Michal Simek        2010-01-18  451  			continue;
d3afa58c Michal Simek        2010-01-18  452  
d3afa58c Michal Simek        2010-01-18  453  		/* Act based on address space type */
d3afa58c Michal Simek        2010-01-18  454  		res = NULL;
4f7b6de4 Andrew Murray       2013-07-27  455  		switch (range.flags & IORESOURCE_TYPE_BITS) {
4f7b6de4 Andrew Murray       2013-07-27  456  		case IORESOURCE_IO:
6bd55f0b Michal Simek        2012-12-27  457  			pr_info("  IO 0x%016llx..0x%016llx -> 0x%016llx\n",
4f7b6de4 Andrew Murray       2013-07-27  458  				range.cpu_addr, range.cpu_addr + range.size - 1,
4f7b6de4 Andrew Murray       2013-07-27  459  				range.pci_addr);
d3afa58c Michal Simek        2010-01-18  460  
d3afa58c Michal Simek        2010-01-18  461  			/* We support only one IO range */
d3afa58c Michal Simek        2010-01-18  462  			if (hose->pci_io_size) {
6bd55f0b Michal Simek        2012-12-27  463  				pr_info(" \\--> Skipped (too many) !\n");
d3afa58c Michal Simek        2010-01-18  464  				continue;
d3afa58c Michal Simek        2010-01-18  465  			}
d3afa58c Michal Simek        2010-01-18  466  			/* On 32 bits, limit I/O space to 16MB */
4f7b6de4 Andrew Murray       2013-07-27  467  			if (range.size > 0x01000000)
4f7b6de4 Andrew Murray       2013-07-27  468  				range.size = 0x01000000;
d3afa58c Michal Simek        2010-01-18  469  
d3afa58c Michal Simek        2010-01-18  470  			/* 32 bits needs to map IOs here */
4f7b6de4 Andrew Murray       2013-07-27  471  			hose->io_base_virt = ioremap(range.cpu_addr,
4f7b6de4 Andrew Murray       2013-07-27  472  						range.size);
d3afa58c Michal Simek        2010-01-18  473  
d3afa58c Michal Simek        2010-01-18  474  			/* Expect trouble if pci_addr is not 0 */
d3afa58c Michal Simek        2010-01-18  475  			if (primary)
d3afa58c Michal Simek        2010-01-18  476  				isa_io_base =
d3afa58c Michal Simek        2010-01-18  477  					(unsigned long)hose->io_base_virt;
d3afa58c Michal Simek        2010-01-18  478  			/* pci_io_size and io_base_phys always represent IO
d3afa58c Michal Simek        2010-01-18  479  			 * space starting at 0 so we factor in pci_addr
d3afa58c Michal Simek        2010-01-18  480  			 */
4f7b6de4 Andrew Murray       2013-07-27  481  			hose->pci_io_size = range.pci_addr + range.size;
4f7b6de4 Andrew Murray       2013-07-27  482  			hose->io_base_phys = range.cpu_addr - range.pci_addr;
d3afa58c Michal Simek        2010-01-18  483  
d3afa58c Michal Simek        2010-01-18  484  			/* Build resource */
d3afa58c Michal Simek        2010-01-18  485  			res = &hose->io_resource;
4f7b6de4 Andrew Murray       2013-07-27  486  			range.cpu_addr = range.pci_addr;
4f7b6de4 Andrew Murray       2013-07-27  487  
d3afa58c Michal Simek        2010-01-18  488  			break;
4f7b6de4 Andrew Murray       2013-07-27  489  		case IORESOURCE_MEM:
6bd55f0b Michal Simek        2012-12-27  490  			pr_info(" MEM 0x%016llx..0x%016llx -> 0x%016llx %s\n",
4f7b6de4 Andrew Murray       2013-07-27  491  				range.cpu_addr, range.cpu_addr + range.size - 1,
4f7b6de4 Andrew Murray       2013-07-27  492  				range.pci_addr,
4f7b6de4 Andrew Murray       2013-07-27  493  				(range.pci_space & 0x40000000) ?
4f7b6de4 Andrew Murray       2013-07-27  494  				"Prefetch" : "");
d3afa58c Michal Simek        2010-01-18  495  
d3afa58c Michal Simek        2010-01-18  496  			/* We support only 3 memory ranges */
d3afa58c Michal Simek        2010-01-18  497  			if (memno >= 3) {
6bd55f0b Michal Simek        2012-12-27  498  				pr_info(" \\--> Skipped (too many) !\n");
d3afa58c Michal Simek        2010-01-18  499  				continue;
d3afa58c Michal Simek        2010-01-18  500  			}
d3afa58c Michal Simek        2010-01-18  501  			/* Handles ISA memory hole space here */
4f7b6de4 Andrew Murray       2013-07-27  502  			if (range.pci_addr == 0) {
4f7b6de4 Andrew Murray       2013-07-27  503  				isa_mb = range.cpu_addr;
d3afa58c Michal Simek        2010-01-18  504  				isa_hole = memno;
d3afa58c Michal Simek        2010-01-18  505  				if (primary || isa_mem_base == 0)
4f7b6de4 Andrew Murray       2013-07-27  506  					isa_mem_base = range.cpu_addr;
4f7b6de4 Andrew Murray       2013-07-27  507  				hose->isa_mem_phys = range.cpu_addr;
4f7b6de4 Andrew Murray       2013-07-27  508  				hose->isa_mem_size = range.size;
d3afa58c Michal Simek        2010-01-18  509  			}
d3afa58c Michal Simek        2010-01-18  510  
d3afa58c Michal Simek        2010-01-18  511  			/* We get the PCI/Mem offset from the first range or
d3afa58c Michal Simek        2010-01-18  512  			 * the, current one if the offset came from an ISA
d3afa58c Michal Simek        2010-01-18  513  			 * hole. If they don't match, bugger.
d3afa58c Michal Simek        2010-01-18  514  			 */
d3afa58c Michal Simek        2010-01-18  515  			if (memno == 0 ||
4f7b6de4 Andrew Murray       2013-07-27  516  			    (isa_hole >= 0 && range.pci_addr != 0 &&
d3afa58c Michal Simek        2010-01-18  517  			     hose->pci_mem_offset == isa_mb))
4f7b6de4 Andrew Murray       2013-07-27  518  				hose->pci_mem_offset = range.cpu_addr -
4f7b6de4 Andrew Murray       2013-07-27  519  							range.pci_addr;
4f7b6de4 Andrew Murray       2013-07-27  520  			else if (range.pci_addr != 0 &&
4f7b6de4 Andrew Murray       2013-07-27  521  				 hose->pci_mem_offset != range.cpu_addr -
4f7b6de4 Andrew Murray       2013-07-27  522  							range.pci_addr) {
6bd55f0b Michal Simek        2012-12-27  523  				pr_info(" \\--> Skipped (offset mismatch) !\n");
d3afa58c Michal Simek        2010-01-18  524  				continue;
d3afa58c Michal Simek        2010-01-18  525  			}
d3afa58c Michal Simek        2010-01-18  526  
d3afa58c Michal Simek        2010-01-18  527  			/* Build resource */
d3afa58c Michal Simek        2010-01-18  528  			res = &hose->mem_resources[memno++];
d3afa58c Michal Simek        2010-01-18  529  			break;
d3afa58c Michal Simek        2010-01-18  530  		}
70dcd942 Michal Simek        2014-10-27  531  		if (res != NULL) {
70dcd942 Michal Simek        2014-10-27  532  			res->name = dev->full_name;
70dcd942 Michal Simek        2014-10-27  533  			res->flags = range.flags;
70dcd942 Michal Simek        2014-10-27  534  			res->start = range.cpu_addr;
70dcd942 Michal Simek        2014-10-27  535  			res->end = range.cpu_addr + range.size - 1;
70dcd942 Michal Simek        2014-10-27 @536  			res->parent = res->child = res->sibling = NULL;
70dcd942 Michal Simek        2014-10-27  537  		}
d3afa58c Michal Simek        2010-01-18  538  	}
d3afa58c Michal Simek        2010-01-18  539  
d3afa58c Michal Simek        2010-01-18  540  	/* If there's an ISA hole and the pci_mem_offset is -not- matching
d3afa58c Michal Simek        2010-01-18  541  	 * the ISA hole offset, then we need to remove the ISA hole from
d3afa58c Michal Simek        2010-01-18  542  	 * the resource list for that brige
d3afa58c Michal Simek        2010-01-18  543  	 */
d3afa58c Michal Simek        2010-01-18  544  	if (isa_hole >= 0 && hose->pci_mem_offset != isa_mb) {
d3afa58c Michal Simek        2010-01-18  545  		unsigned int next = isa_hole + 1;
6bd55f0b Michal Simek        2012-12-27  546  		pr_info(" Removing ISA hole at 0x%016llx\n", isa_mb);
d3afa58c Michal Simek        2010-01-18  547  		if (next < memno)
d3afa58c Michal Simek        2010-01-18  548  			memmove(&hose->mem_resources[isa_hole],
d3afa58c Michal Simek        2010-01-18  549  				&hose->mem_resources[next],
d3afa58c Michal Simek        2010-01-18  550  				sizeof(struct resource) * (memno - next));
d3afa58c Michal Simek        2010-01-18  551  		hose->mem_resources[--memno].flags = 0;
d3afa58c Michal Simek        2010-01-18  552  	}
d3afa58c Michal Simek        2010-01-18  553  }
d3afa58c Michal Simek        2010-01-18  554  
9413d968 Bharat Kumar Gogada 2016-09-01  555  /* Display the domain number in /proc */
d3afa58c Michal Simek        2010-01-18  556  int pci_proc_domain(struct pci_bus *bus)
d3afa58c Michal Simek        2010-01-18  557  {
9413d968 Bharat Kumar Gogada 2016-09-01  558  	return pci_domain_nr(bus);
d3afa58c Michal Simek        2010-01-18  559  }
d3afa58c Michal Simek        2010-01-18  560  
d3afa58c Michal Simek        2010-01-18  561  /* This header fixup will do the resource fixup for all devices as they are
d3afa58c Michal Simek        2010-01-18  562   * probed, but not for bridge ranges
d3afa58c Michal Simek        2010-01-18  563   */
b881bc46 Greg Kroah-Hartman  2012-12-21  564  static void pcibios_fixup_resources(struct pci_dev *dev)
d3afa58c Michal Simek        2010-01-18  565  {
d3afa58c Michal Simek        2010-01-18  566  	struct pci_controller *hose = pci_bus_to_host(dev->bus);
d3afa58c Michal Simek        2010-01-18  567  	int i;
d3afa58c Michal Simek        2010-01-18  568  
d3afa58c Michal Simek        2010-01-18  569  	if (!hose) {
6bd55f0b Michal Simek        2012-12-27  570  		pr_err("No host bridge for PCI dev %s !\n",
d3afa58c Michal Simek        2010-01-18  571  		       pci_name(dev));
d3afa58c Michal Simek        2010-01-18  572  		return;
d3afa58c Michal Simek        2010-01-18  573  	}
d3afa58c Michal Simek        2010-01-18  574  	for (i = 0; i < DEVICE_COUNT_RESOURCE; i++) {
d3afa58c Michal Simek        2010-01-18  575  		struct resource *res = dev->resource + i;
d3afa58c Michal Simek        2010-01-18  576  		if (!res->flags)
d3afa58c Michal Simek        2010-01-18  577  			continue;
e5b36841 Bjorn Helgaas       2012-02-23  578  		if (res->start == 0) {
6bd55f0b Michal Simek        2012-12-27  579  			pr_debug("PCI:%s Resource %d %016llx-%016llx [%x]",
d3afa58c Michal Simek        2010-01-18  580  				 pci_name(dev), i,
d3afa58c Michal Simek        2010-01-18  581  				 (unsigned long long)res->start,
d3afa58c Michal Simek        2010-01-18  582  				 (unsigned long long)res->end,
d3afa58c Michal Simek        2010-01-18  583  				 (unsigned int)res->flags);
6bd55f0b Michal Simek        2012-12-27  584  			pr_debug("is unassigned\n");
d3afa58c Michal Simek        2010-01-18  585  			res->end -= res->start;
d3afa58c Michal Simek        2010-01-18  586  			res->start = 0;
d3afa58c Michal Simek        2010-01-18  587  			res->flags |= IORESOURCE_UNSET;
d3afa58c Michal Simek        2010-01-18  588  			continue;
d3afa58c Michal Simek        2010-01-18  589  		}
d3afa58c Michal Simek        2010-01-18  590  
aa23bdc0 Bjorn Helgaas       2012-02-23  591  		pr_debug("PCI:%s Resource %d %016llx-%016llx [%x]\n",
d3afa58c Michal Simek        2010-01-18  592  			 pci_name(dev), i,
6bd55f0b Michal Simek        2012-12-27  593  			 (unsigned long long)res->start,
d3afa58c Michal Simek        2010-01-18  594  			 (unsigned long long)res->end,
d3afa58c Michal Simek        2010-01-18  595  			 (unsigned int)res->flags);
d3afa58c Michal Simek        2010-01-18  596  	}
d3afa58c Michal Simek        2010-01-18  597  }
d3afa58c Michal Simek        2010-01-18  598  DECLARE_PCI_FIXUP_HEADER(PCI_ANY_ID, PCI_ANY_ID, pcibios_fixup_resources);
d3afa58c Michal Simek        2010-01-18  599  
d3afa58c Michal Simek        2010-01-18  600  /*
d3afa58c Michal Simek        2010-01-18  601   * We need to avoid collisions with `mirrored' VGA ports
d3afa58c Michal Simek        2010-01-18  602   * and other strange ISA hardware, so we always want the
d3afa58c Michal Simek        2010-01-18  603   * addresses to be allocated in the 0x000-0x0ff region
d3afa58c Michal Simek        2010-01-18  604   * modulo 0x400.
d3afa58c Michal Simek        2010-01-18  605   *
d3afa58c Michal Simek        2010-01-18  606   * Why? Because some silly external IO cards only decode
d3afa58c Michal Simek        2010-01-18  607   * the low 10 bits of the IO address. The 0x00-0xff region
d3afa58c Michal Simek        2010-01-18  608   * is reserved for motherboard devices that decode all 16
d3afa58c Michal Simek        2010-01-18  609   * bits, so it's ok to allocate at, say, 0x2800-0x28ff,
d3afa58c Michal Simek        2010-01-18  610   * but we want to try to avoid allocating at 0x2900-0x2bff
d3afa58c Michal Simek        2010-01-18  611   * which might have be mirrored at 0x0100-0x03ff..
d3afa58c Michal Simek        2010-01-18  612   */
01cf9d52 Bharat Kumar Gogada 2016-02-11  613  int pcibios_add_device(struct pci_dev *dev)
01cf9d52 Bharat Kumar Gogada 2016-02-11  614  {
01cf9d52 Bharat Kumar Gogada 2016-02-11  615  	dev->irq = of_irq_parse_and_map_pci(dev, 0, 0);
c86fac43 Michal Simek        2010-04-16  616  
01cf9d52 Bharat Kumar Gogada 2016-02-11  617  	return 0;
d3afa58c Michal Simek        2010-01-18  618  }
01cf9d52 Bharat Kumar Gogada 2016-02-11  619  EXPORT_SYMBOL(pcibios_add_device);
d3afa58c Michal Simek        2010-01-18  620  
d3afa58c Michal Simek        2010-01-18  621  /*
d3afa58c Michal Simek        2010-01-18  622   * Reparent resource children of pr that conflict with res
d3afa58c Michal Simek        2010-01-18  623   * under res, and make res replace those children.
d3afa58c Michal Simek        2010-01-18  624   */
d3afa58c Michal Simek        2010-01-18  625  static int __init reparent_resources(struct resource *parent,
d3afa58c Michal Simek        2010-01-18  626  				     struct resource *res)
d3afa58c Michal Simek        2010-01-18  627  {
d3afa58c Michal Simek        2010-01-18  628  	struct resource *p, **pp;
d3afa58c Michal Simek        2010-01-18  629  	struct resource **firstpp = NULL;
d3afa58c Michal Simek        2010-01-18  630  
d3afa58c Michal Simek        2010-01-18 @631  	for (pp = &parent->child; (p = *pp) != NULL; pp = &p->sibling) {
d3afa58c Michal Simek        2010-01-18  632  		if (p->end < res->start)
d3afa58c Michal Simek        2010-01-18  633  			continue;
d3afa58c Michal Simek        2010-01-18  634  		if (res->end < p->start)
d3afa58c Michal Simek        2010-01-18  635  			break;
d3afa58c Michal Simek        2010-01-18  636  		if (p->start < res->start || p->end > res->end)
d3afa58c Michal Simek        2010-01-18  637  			return -1;	/* not completely contained */
d3afa58c Michal Simek        2010-01-18  638  		if (firstpp == NULL)
d3afa58c Michal Simek        2010-01-18  639  			firstpp = pp;
d3afa58c Michal Simek        2010-01-18  640  	}
d3afa58c Michal Simek        2010-01-18  641  	if (firstpp == NULL)
d3afa58c Michal Simek        2010-01-18  642  		return -1;	/* didn't find any conflicting entries? */
d3afa58c Michal Simek        2010-01-18  643  	res->parent = parent;
d3afa58c Michal Simek        2010-01-18 @644  	res->child = *firstpp;
d3afa58c Michal Simek        2010-01-18  645  	res->sibling = *pp;
d3afa58c Michal Simek        2010-01-18  646  	*firstpp = res;
d3afa58c Michal Simek        2010-01-18  647  	*pp = NULL;
d3afa58c Michal Simek        2010-01-18 @648  	for (p = res->child; p != NULL; p = p->sibling) {
d3afa58c Michal Simek        2010-01-18  649  		p->parent = res;
d3afa58c Michal Simek        2010-01-18  650  		pr_debug("PCI: Reparented %s [%llx..%llx] under %s\n",
d3afa58c Michal Simek        2010-01-18  651  			 p->name,
d3afa58c Michal Simek        2010-01-18  652  			 (unsigned long long)p->start,
d3afa58c Michal Simek        2010-01-18  653  			 (unsigned long long)p->end, res->name);
d3afa58c Michal Simek        2010-01-18  654  	}
d3afa58c Michal Simek        2010-01-18  655  	return 0;
d3afa58c Michal Simek        2010-01-18  656  }
d3afa58c Michal Simek        2010-01-18  657  

:::::: The code at line 536 was first introduced by commit
:::::: 70dcd942dc4af3cc6c3dcc2ba499cd841c7f65a7 microblaze: Fix IO space breakage after of_pci_range_to_resource() change

:::::: TO: Michal Simek <michal.simek@xilinx.com>
:::::: CC: Michal Simek <michal.simek@xilinx.com>

---
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: 12274 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v3 1/3] resource: Use list_head to link sibling resource
  2018-04-19  0:18 ` [PATCH v3 1/3] " Baoquan He
  2018-04-26  1:18   ` Wei Yang
  2018-04-26  3:01   ` kbuild test robot
@ 2018-04-26  3:23   ` kbuild test robot
  2018-05-06  6:30     ` Baoquan He
  2 siblings, 1 reply; 19+ messages in thread
From: kbuild test robot @ 2018-04-26  3:23 UTC (permalink / raw)
  To: Baoquan He
  Cc: kbuild-all, linux-kernel, akpm, robh+dt, dan.j.williams,
	nicolas.pitre, josh, Brijesh Singh, devicetree, David Airlie,
	linux-pci, Wei Yang, Keith Busch, Yaowei Bai, Frank Rowand,
	Lorenzo Pieralisi, Stephen Hemminger, Baoquan He, linux-nvdimm,
	Patrik Jakobsson, linux-input, Borislav Petkov, Tom Lendacky,
	Haiyang Zhang, Jérôme Glisse, Bjorn Helgaas,
	Thomas Gleixner, Jonathan Derrick, Greg Kroah-Hartman,
	Dmitry Torokhov, devel

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

Hi Baoquan,

I love your patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v4.17-rc2 next-20180424]
[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/Baoquan-He/resource-Use-list_head-to-link-sibling-resource/20180419-223752
config: xtensa-common_defconfig (attached as .config)
compiler: xtensa-linux-gcc (GCC) 7.2.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
        make.cross ARCH=xtensa 

All errors (new ones prefixed by >>):

   In file included from arch/xtensa/lib/pci-auto.c:22:0:
   arch/xtensa/include/asm/pci-bridge.h: In function 'pcibios_init_resource':
>> arch/xtensa/include/asm/pci-bridge.h:74:15: error: incompatible types when assigning to type 'struct list_head' from type 'void *'
     res->sibling = NULL;
                  ^
   arch/xtensa/include/asm/pci-bridge.h:75:13: error: incompatible types when assigning to type 'struct list_head' from type 'void *'
     res->child = NULL;
                ^

vim +74 arch/xtensa/include/asm/pci-bridge.h

9a8fd558 include/asm-xtensa/pci-bridge.h Chris Zankel 2005-06-23  65  
9a8fd558 include/asm-xtensa/pci-bridge.h Chris Zankel 2005-06-23  66  static inline void pcibios_init_resource(struct resource *res,
9a8fd558 include/asm-xtensa/pci-bridge.h Chris Zankel 2005-06-23  67  		unsigned long start, unsigned long end, int flags, char *name)
9a8fd558 include/asm-xtensa/pci-bridge.h Chris Zankel 2005-06-23  68  {
9a8fd558 include/asm-xtensa/pci-bridge.h Chris Zankel 2005-06-23  69  	res->start = start;
9a8fd558 include/asm-xtensa/pci-bridge.h Chris Zankel 2005-06-23  70  	res->end = end;
9a8fd558 include/asm-xtensa/pci-bridge.h Chris Zankel 2005-06-23  71  	res->flags = flags;
9a8fd558 include/asm-xtensa/pci-bridge.h Chris Zankel 2005-06-23  72  	res->name = name;
9a8fd558 include/asm-xtensa/pci-bridge.h Chris Zankel 2005-06-23  73  	res->parent = NULL;
9a8fd558 include/asm-xtensa/pci-bridge.h Chris Zankel 2005-06-23 @74  	res->sibling = NULL;
9a8fd558 include/asm-xtensa/pci-bridge.h Chris Zankel 2005-06-23  75  	res->child = NULL;
9a8fd558 include/asm-xtensa/pci-bridge.h Chris Zankel 2005-06-23  76  }
9a8fd558 include/asm-xtensa/pci-bridge.h Chris Zankel 2005-06-23  77  

:::::: The code at line 74 was first introduced by commit
:::::: 9a8fd5589902153a134111ed7a40f9cca1f83254 [PATCH] xtensa: Architecture support for Tensilica Xtensa Part 6

:::::: TO: Chris Zankel <czankel@tensilica.com>
:::::: CC: Linus Torvalds <torvalds@ppc970.osdl.org>

---
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: 9949 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v3 2/3] resource: add walk_system_ram_res_rev()
  2018-04-19 10:07   ` Borislav Petkov
@ 2018-04-26  8:56     ` Baoquan He
  2018-04-26 11:09       ` Borislav Petkov
  0 siblings, 1 reply; 19+ messages in thread
From: Baoquan He @ 2018-04-26  8:56 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: linux-kernel, akpm, robh+dt, dan.j.williams, nicolas.pitre, josh,
	Thomas Gleixner, Brijesh Singh, Jérôme Glisse,
	Tom Lendacky, Wei Yang

Sorry to all reviewers, I had a redhat urgent issue, this patchset
discussing is delayed.

On 04/19/18 at 12:07pm, Borislav Petkov wrote:
> On Thu, Apr 19, 2018 at 08:18:47AM +0800, Baoquan He wrote:
> > This function, being a variant of walk_system_ram_res() introduced in
> > commit 8c86e70acead ("resource: provide new functions to walk through
> > resources"), walks through a list of all the resources of System RAM
> > in reversed order, i.e., from higher to lower.
> > 
> > It will be used in kexec_file code.
> 
> Of course, what is missing is the big *WHY* you need this to happen this
> way...

Sorry for that, I just ran scripts/get_maintainer.pl to get expert's
name and added them into each patch. The reason this change is made is
in patch 3/3. Test robot reported a code bug on the latest kernel, will
repost and CC everyone in all patches.


Rob Herring asked the same question in v2, I explained to him. The
discussion can be found here:
https://lkml.org/lkml/2018/4/10/484

> 
> >  /*
> > + * This function, being a variant of walk_system_ram_res(), calls the @func
> > + * callback against all memory ranges of type System RAM which are marked as
> > + * IORESOURCE_SYSTEM_RAM and IORESOUCE_BUSY in reversed order, i.e., from
> > + * higher to lower.
> > + */
> > +int walk_system_ram_res_rev(u64 start, u64 end, void *arg,
> > +				int (*func)(struct resource *, void *))
> > +{
> > +	unsigned long flags;
> > +	struct resource *res;
> > +	int ret = -1;
> > +
> > +	flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY;
> > +
> > +	read_lock(&resource_lock);
> > +	list_for_each_entry_reverse(res, &iomem_resource.child, sibling) {
> 
> ... and the other thing that I'm not clear on is why are you
> slapping this function just like that instead of extending
> __walk_iomem_res_desc() to do reverse direction too?

Yes, I planned to do as you said when started to write code. After investigation,
I changed to use the way of this patch. Because __walk_iomem_res_desc()
provides two interfaces by calling find_next_iomem_res(). If
'first_level_children_only' is true, it only iterates system RAM
resource; if 'first_level_children_only' is false, it does depth first
iteration in iomem_resource. The 1st interface is only used by
walk_system_ram_res() and walk_mem_res(). While the 2nd interface need
call find_next_iomem_res() which resorts to next_resource().
next_resource() will check 'sibling_only' to decide if iterate
iomem_resource's children, or walk over all resources of iomem_resource
with depth first. For walk_system_ram_res_rev(), we only need to iterate
iomem_resource's children, and seems no one has requirement to
iterate all iomem_resource's children and grand children
revresedly. For simplifying code implementation, I just did as in this
patch, surely, if any suggestion or scenario given about the reversed
iteration of all resources, I can change to add below functions, and
based on them to implement walk_system_ram_res_rev().

walk_system_ram_res_rev
	->__walk_iomem_res_desc_rev
		->find_next_iomem_res_rev
			->next_resource_rev

Thanks
Baoquan


> 
> SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
> -- 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v3 2/3] resource: add walk_system_ram_res_rev()
  2018-04-26  8:56     ` Baoquan He
@ 2018-04-26 11:09       ` Borislav Petkov
  2018-04-26 13:22         ` Baoquan He
  0 siblings, 1 reply; 19+ messages in thread
From: Borislav Petkov @ 2018-04-26 11:09 UTC (permalink / raw)
  To: Baoquan He
  Cc: linux-kernel, akpm, robh+dt, dan.j.williams, nicolas.pitre, josh,
	Thomas Gleixner, Brijesh Singh, Jérôme Glisse,
	Tom Lendacky, Wei Yang

On Thu, Apr 26, 2018 at 04:56:49PM +0800, Baoquan He wrote:
> Sorry for that, I just ran scripts/get_maintainer.pl to get expert's
> name and added them into each patch. The reason this change is made is
> in patch 3/3. Test robot reported a code bug on the latest kernel, will
> repost and CC everyone in all patches.
> 
> 
> Rob Herring asked the same question in v2, I explained to him. The
> discussion can be found here:
> https://lkml.org/lkml/2018/4/10/484

... and when I open that link, the first paragraph says:

"This is the explanation I made when Andrew helped to review the v1 post:
https://lkml.org/lkml/2018/3/23/78"

Do you see the absurdity of the link chasing of your explanation?!

Instead, the explanation *WHY* should be in the commit message of the
patch - not in mail replies when people ask you about it.

Also, do not use lkml.org when referencing a mail on lkml but
use the Message-ID of the header. We have a proper redirector at
https://lkml.kernel.org/r/<Message-ID>

Now lemme read the reason finally...

"We need unify these two interfaces on behaviour since they are the same
on essense from the users' point of view... "

That's not a good enough reason for me to cause code churn. If the only
reason is: because the one does it top-down and the other bottom-up, I'm
not convinced.

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v3 2/3] resource: add walk_system_ram_res_rev()
  2018-04-26 11:09       ` Borislav Petkov
@ 2018-04-26 13:22         ` Baoquan He
  2018-05-04 10:16           ` Borislav Petkov
  0 siblings, 1 reply; 19+ messages in thread
From: Baoquan He @ 2018-04-26 13:22 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: linux-kernel, akpm, robh+dt, dan.j.williams, nicolas.pitre, josh,
	Thomas Gleixner, Brijesh Singh, Jérôme Glisse,
	Tom Lendacky, Wei Yang

On 04/26/18 at 01:09pm, Borislav Petkov wrote:
> On Thu, Apr 26, 2018 at 04:56:49PM +0800, Baoquan He wrote:
> > Sorry for that, I just ran scripts/get_maintainer.pl to get expert's
> > name and added them into each patch. The reason this change is made is
> > in patch 3/3. Test robot reported a code bug on the latest kernel, will
> > repost and CC everyone in all patches.
> > 
> > 
> > Rob Herring asked the same question in v2, I explained to him. The
> > discussion can be found here:
> > https://lkml.org/lkml/2018/4/10/484
> 
> ... and when I open that link, the first paragraph says:
> 
> "This is the explanation I made when Andrew helped to review the v1 post:
> https://lkml.org/lkml/2018/3/23/78"
> 
> Do you see the absurdity of the link chasing of your explanation?!
> 
> Instead, the explanation *WHY* should be in the commit message of the
> patch - not in mail replies when people ask you about it.

It is in patch 3/3 and cover-letter. I often received one or several
patches of a big patchset. As I said, I used ./scripts/get_maintainer.pl
to each patch's reviewers, and will add all people in CC list.

> 
> Also, do not use lkml.org when referencing a mail on lkml but
> use the Message-ID of the header. We have a proper redirector at
> https://lkml.kernel.org/r/<Message-ID>

I noticed maintainers merged patches with this Message-ID, could you
tell how to get this Message-ID?

> 
> Now lemme read the reason finally...
> 
> "We need unify these two interfaces on behaviour since they are the same
> on essense from the users' point of view... "
> 
> That's not a good enough reason for me to cause code churn. If the only
> reason is: because the one does it top-down and the other bottom-up, I'm
> not convinced.

We have been using this top-down way for x86 64 since earlier 2013 in
the KEXEC loading with command:

	'kexec -l bzImage --initrd initramfs-xx.img'

Two years later, Vivek added a new KEXEC_FILE loading, and most of job
is done in kernel because we need do kernel image sinature verification.
The KEXEC_FILE loading mostly is used in secure boot machine. Although
more and more users like to take secure boot machine, system using KEXEC
loading are still much more. I ever asked our kernel QE who takes care
of kernel and kdump testing, they said in their testing systems, the
proportion of legacy system vs system with secure boot is 10:1.

Before long I pinged hpa, Vivek and Yinghai who discussed and wrote the
code for KEXEC loading in x86 64, asked them about the reaon they chose
top-down, no reply from them. I guess they probably worried that low
memory under 4G are mostly reserved by system or firmware, even though
kexec/kdump have tried very hard to avoid each of them if recognized,
any missed one will cause loading and booting failure. Just a guess.

In fact, there's no spec or doc to tell which one is right or not.
So I finally decide to take the same top-down way for KEXEC_FILE loading
because on statistics KEXEC loading is used longer and wider.

This is not a thing that one is top down, the other is bottom up. For
us, they might be so different on details of code, for customers, they
just think them as a same thing. They may say I just get a new machine,
and still do kexec loading, why these top-down, bottom-up things come
up.

And this is not causing code churn. You can see that by replacing
pointer operation with list_head, code in kernel/resource.c related to
child list iteration is much easier to read, e.g __request_resource(),
__release_resource(). As Rob said in v2 reviewing, they have put
the same replacing in DT struct device_node in todo list, and ACPI tree
function in drivers/acpi/acpica/pstree.c too. I think this is why Andrew
asked to try this way and see what the patches looks like.

Thanks
Baoquan

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v3 2/3] resource: add walk_system_ram_res_rev()
  2018-04-26 13:22         ` Baoquan He
@ 2018-05-04 10:16           ` Borislav Petkov
  2018-05-06  6:19             ` Baoquan He
  0 siblings, 1 reply; 19+ messages in thread
From: Borislav Petkov @ 2018-05-04 10:16 UTC (permalink / raw)
  To: Baoquan He
  Cc: linux-kernel, akpm, robh+dt, dan.j.williams, nicolas.pitre, josh,
	Thomas Gleixner, Brijesh Singh, Jérôme Glisse,
	Tom Lendacky, Wei Yang

On Thu, Apr 26, 2018 at 09:22:04PM +0800, Baoquan He wrote:
> I noticed maintainers merged patches with this Message-ID, could you
> tell how to get this Message-ID?

https://en.wikipedia.org/wiki/Message-ID

> This is not a thing that one is top down, the other is bottom up. For
> us, they might be so different on details of code, for customers, they
> just think them as a same thing. They may say I just get a new machine,
> and still do kexec loading, why these top-down, bottom-up things come
> up.

So if I read the above correctly, it doesn't matter whether top-down or
bottom-up.

> And this is not causing code churn. You can see that by replacing
> pointer operation with list_head, code in kernel/resource.c related to
> child list iteration is much easier to read,

Now *this* is starting to sound like some reason "why". If it is better
readability, then say so in the commit message.

It still doesn't justify adding walk_system_ram_res_rev() though.

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v3 2/3] resource: add walk_system_ram_res_rev()
  2018-05-04 10:16           ` Borislav Petkov
@ 2018-05-06  6:19             ` Baoquan He
  0 siblings, 0 replies; 19+ messages in thread
From: Baoquan He @ 2018-05-06  6:19 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: linux-kernel, akpm, robh+dt, dan.j.williams, nicolas.pitre, josh,
	Thomas Gleixner, Brijesh Singh, Jérôme Glisse,
	Tom Lendacky, Wei Yang

On 05/04/18 at 12:16pm, Borislav Petkov wrote:
> On Thu, Apr 26, 2018 at 09:22:04PM +0800, Baoquan He wrote:
> > I noticed maintainers merged patches with this Message-ID, could you
> > tell how to get this Message-ID?
> 
> https://en.wikipedia.org/wiki/Message-ID
thx.
> 
> > This is not a thing that one is top down, the other is bottom up. For
> > us, they might be so different on details of code, for customers, they
> > just think them as a same thing. They may say I just get a new machine,
> > and still do kexec loading, why these top-down, bottom-up things come
> > up.
> 
> So if I read the above correctly, it doesn't matter whether top-down or
> bottom-up.

Well, kexec_file loading is added because kernel signature verification
has to be done in kernel space, it's implemented by porting code of user
space utility kexc-tools to kernel. In essence both the kexec loading
and kexec_file loading are the same, they are all used to load kernel
for the furture kexec/kdump jumping. Now they have different kernel
loading position, could you tell how you get the conclusion that it
doesn't matter? And what's your theory basis?

And KASLR also changes kernel's position, I don't think users have to
know all these details to exclude unnecessary noise. 

> 
> > And this is not causing code churn. You can see that by replacing
> > pointer operation with list_head, code in kernel/resource.c related to
> > child list iteration is much easier to read,
> 
> Now *this* is starting to sound like some reason "why". If it is better
> readability, then say so in the commit message.

Yes, I can add it to log in next post. While the main reason I made
this is for the top down searching of system RAM in kexec_file
loading. 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v3 1/3] resource: Use list_head to link sibling resource
  2018-04-26  3:23   ` kbuild test robot
@ 2018-05-06  6:30     ` Baoquan He
  0 siblings, 0 replies; 19+ messages in thread
From: Baoquan He @ 2018-05-06  6:30 UTC (permalink / raw)
  To: kbuild test robot
  Cc: nicolas.pitre, Brijesh Singh, Tom Lendacky, David Airlie,
	linux-pci, Wei Yang, Keith Busch, Yaowei Bai, Frank Rowand,
	Thomas Gleixner, Lorenzo Pieralisi, Stephen Hemminger,
	linux-nvdimm, Patrik Jakobsson, linux-input, Borislav Petkov,
	devicetree, Haiyang Zhang, josh, Jérôme Glisse,
	robh+dt, Bjorn Helgaas, dan.j.williams, Jonathan Derrick,
	Greg Kroah-Hartman, Dmitry Torokhov, linux-kernel, kbuild-all,
	devel, akpm

On 04/26/18 at 11:23am, kbuild test robot wrote:
> Hi Baoquan,
> 
> I love your patch! Yet something to improve:
> 
> [auto build test ERROR on linus/master]
> [also build test ERROR on v4.17-rc2 next-20180424]
> [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/Baoquan-He/resource-Use-list_head-to-link-sibling-resource/20180419-223752
> config: xtensa-common_defconfig (attached as .config)
> compiler: xtensa-linux-gcc (GCC) 7.2.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
>         make.cross ARCH=xtensa 
> 
> All errors (new ones prefixed by >>):
> 
>    In file included from arch/xtensa/lib/pci-auto.c:22:0:
>    arch/xtensa/include/asm/pci-bridge.h: In function 'pcibios_init_resource':
> >> arch/xtensa/include/asm/pci-bridge.h:74:15: error: incompatible types when assigning to type 'struct list_head' from type 'void *'
>      res->sibling = NULL;
>                   ^
>    arch/xtensa/include/asm/pci-bridge.h:75:13: error: incompatible types when assigning to type 'struct list_head' from type 'void *'
>      res->child = NULL;

Thanks, below fix can fix it.

diff --git a/arch/xtensa/include/asm/pci-bridge.h b/arch/xtensa/include/asm/pci-bridge.h
index 0b68c76ec1e6..f487b06817df 100644
--- a/arch/xtensa/include/asm/pci-bridge.h
+++ b/arch/xtensa/include/asm/pci-bridge.h
@@ -71,8 +71,8 @@ static inline void pcibios_init_resource(struct resource *res,
 	res->flags = flags;
 	res->name = name;
 	res->parent = NULL;
-	res->sibling = NULL;
-	res->child = NULL;
+	INIT_LIST_HEAD(&res->child);
+	INIT_LIST_HEAD(&res->sibling);
 }
 
 

>                 ^
> 
> vim +74 arch/xtensa/include/asm/pci-bridge.h
> 
> 9a8fd558 include/asm-xtensa/pci-bridge.h Chris Zankel 2005-06-23  65  
> 9a8fd558 include/asm-xtensa/pci-bridge.h Chris Zankel 2005-06-23  66  static inline void pcibios_init_resource(struct resource *res,
> 9a8fd558 include/asm-xtensa/pci-bridge.h Chris Zankel 2005-06-23  67  		unsigned long start, unsigned long end, int flags, char *name)
> 9a8fd558 include/asm-xtensa/pci-bridge.h Chris Zankel 2005-06-23  68  {
> 9a8fd558 include/asm-xtensa/pci-bridge.h Chris Zankel 2005-06-23  69  	res->start = start;
> 9a8fd558 include/asm-xtensa/pci-bridge.h Chris Zankel 2005-06-23  70  	res->end = end;
> 9a8fd558 include/asm-xtensa/pci-bridge.h Chris Zankel 2005-06-23  71  	res->flags = flags;
> 9a8fd558 include/asm-xtensa/pci-bridge.h Chris Zankel 2005-06-23  72  	res->name = name;
> 9a8fd558 include/asm-xtensa/pci-bridge.h Chris Zankel 2005-06-23  73  	res->parent = NULL;
> 9a8fd558 include/asm-xtensa/pci-bridge.h Chris Zankel 2005-06-23 @74  	res->sibling = NULL;
> 9a8fd558 include/asm-xtensa/pci-bridge.h Chris Zankel 2005-06-23  75  	res->child = NULL;
> 9a8fd558 include/asm-xtensa/pci-bridge.h Chris Zankel 2005-06-23  76  }
> 9a8fd558 include/asm-xtensa/pci-bridge.h Chris Zankel 2005-06-23  77  
> 
> :::::: The code at line 74 was first introduced by commit
> :::::: 9a8fd5589902153a134111ed7a40f9cca1f83254 [PATCH] xtensa: Architecture support for Tensilica Xtensa Part 6
> 
> :::::: TO: Chris Zankel <czankel@tensilica.com>
> :::::: CC: Linus Torvalds <torvalds@ppc970.osdl.org>
> 
> ---
> 0-DAY kernel test infrastructure                Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation


_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v3 1/3] resource: Use list_head to link sibling resource
  2018-04-26  3:01   ` kbuild test robot
@ 2018-05-06  6:31     ` Baoquan He
  0 siblings, 0 replies; 19+ messages in thread
From: Baoquan He @ 2018-05-06  6:31 UTC (permalink / raw)
  To: kbuild test robot
  Cc: nicolas.pitre, Brijesh Singh, Tom Lendacky, David Airlie,
	linux-pci, Wei Yang, Keith Busch, Yaowei Bai, Frank Rowand,
	Thomas Gleixner, Lorenzo Pieralisi, Stephen Hemminger,
	linux-nvdimm, Patrik Jakobsson, linux-input, Borislav Petkov,
	devicetree, Haiyang Zhang, josh, Jérôme Glisse,
	robh+dt, Bjorn Helgaas, dan.j.williams, Jonathan Derrick,
	Greg Kroah-Hartman, Dmitry Torokhov, linux-kernel, kbuild-all,
	devel, akpm

On 04/26/18 at 11:01am, kbuild test robot wrote:
> Hi Baoquan,
> 
> I love your patch! Yet something to improve:
> 
> [auto build test ERROR on linus/master]
> [also build test ERROR on v4.17-rc2 next-20180424]
> [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/Baoquan-He/resource-Use-list_head-to-link-sibling-resource/20180419-223752
> config: microblaze-mmu_defconfig (attached as .config)
> compiler: microblaze-linux-gcc (GCC) 7.2.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
>         make.cross ARCH=microblaze 
> 
> All errors (new ones prefixed by >>):

Thanks, below patch can fix it. Will repost including the fix.


diff --git a/arch/microblaze/pci/pci-common.c b/arch/microblaze/pci/pci-common.c
index 161f9758c631..56d189cb4be4 100644
--- a/arch/microblaze/pci/pci-common.c
+++ b/arch/microblaze/pci/pci-common.c
@@ -533,7 +533,9 @@ void pci_process_bridge_OF_ranges(struct pci_controller *hose,
 			res->flags = range.flags;
 			res->start = range.cpu_addr;
 			res->end = range.cpu_addr + range.size - 1;
-			res->parent = res->child = res->sibling = NULL;
+			res->parent = NULL;
+			INIT_LIST_HEAD(&res->child);
+			INIT_LIST_HEAD(&res->sibling);
 		}
 	}
 
@@ -625,28 +627,31 @@ EXPORT_SYMBOL(pcibios_add_device);
 static int __init reparent_resources(struct resource *parent,
 				     struct resource *res)
 {
-	struct resource *p, **pp;
-	struct resource **firstpp = NULL;
+	struct resource *p, *first = NULL;
 
-	for (pp = &parent->child; (p = *pp) != NULL; pp = &p->sibling) {
+	list_for_each_entry(p, &parent->child, sibling) {
 		if (p->end < res->start)
 			continue;
 		if (res->end < p->start)
 			break;
 		if (p->start < res->start || p->end > res->end)
 			return -1;	/* not completely contained */
-		if (firstpp == NULL)
-			firstpp = pp;
+		if (first == NULL)
+			first = p;
 	}
-	if (firstpp == NULL)
+	if (first == NULL)
 		return -1;	/* didn't find any conflicting entries? */
 	res->parent = parent;
-	res->child = *firstpp;
-	res->sibling = *pp;
-	*firstpp = res;
-	*pp = NULL;
-	for (p = res->child; p != NULL; p = p->sibling) {
-		p->parent = res;
+	list_add(&res->sibling, &p->sibling.prev);
+	INIT_LIST_HEAD(&res->child);
+
+	/*
+	 * From first to p's previous sibling, they all fall into
+	 * res's region, change them as res's children.
+	 */
+	list_cut_position(&res->child, first->sibling.prev, res->sibling.prev);
+	list_for_each_entry(p, &new->child, sibling) {
+                p->parent = new;
 		pr_debug("PCI: Reparented %s [%llx..%llx] under %s\n",
 			 p->name,
 			 (unsigned long long)p->start,

> 
>    arch/microblaze/pci/pci-common.c: In function 'pci_process_bridge_OF_ranges':
> >> arch/microblaze/pci/pci-common.c:536:44: error: incompatible types when assigning to type 'struct list_head' from type 'void *'
>        res->parent = res->child = res->sibling = NULL;
>                                                ^
>    arch/microblaze/pci/pci-common.c: In function 'reparent_resources':
> >> arch/microblaze/pci/pci-common.c:631:10: error: assignment from incompatible pointer type [-Werror=incompatible-pointer-types]
>      for (pp = &parent->child; (p = *pp) != NULL; pp = &p->sibling) {
>              ^
>    arch/microblaze/pci/pci-common.c:631:50: error: assignment from incompatible pointer type [-Werror=incompatible-pointer-types]
>      for (pp = &parent->child; (p = *pp) != NULL; pp = &p->sibling) {
>                                                      ^
> >> arch/microblaze/pci/pci-common.c:644:13: error: incompatible types when assigning to type 'struct list_head' from type 'struct resource *'
>      res->child = *firstpp;
>                 ^
>    arch/microblaze/pci/pci-common.c:645:15: error: incompatible types when assigning to type 'struct list_head' from type 'struct resource *'
>      res->sibling = *pp;
>                   ^
> >> arch/microblaze/pci/pci-common.c:648:9: error: incompatible types when assigning to type 'struct resource *' from type 'struct list_head'
>      for (p = res->child; p != NULL; p = p->sibling) {
>             ^
>    arch/microblaze/pci/pci-common.c:648:36: error: incompatible types when assigning to type 'struct resource *' from type 'struct list_head'
>      for (p = res->child; p != NULL; p = p->sibling) {
>                                        ^
>    cc1: some warnings being treated as errors
> 
> vim +536 arch/microblaze/pci/pci-common.c
> 
> d3afa58c Michal Simek        2010-01-18  387  
> d3afa58c Michal Simek        2010-01-18  388  /**
> d3afa58c Michal Simek        2010-01-18  389   * pci_process_bridge_OF_ranges - Parse PCI bridge resources from device tree
> d3afa58c Michal Simek        2010-01-18  390   * @hose: newly allocated pci_controller to be setup
> d3afa58c Michal Simek        2010-01-18  391   * @dev: device node of the host bridge
> d3afa58c Michal Simek        2010-01-18  392   * @primary: set if primary bus (32 bits only, soon to be deprecated)
> d3afa58c Michal Simek        2010-01-18  393   *
> d3afa58c Michal Simek        2010-01-18  394   * This function will parse the "ranges" property of a PCI host bridge device
> d3afa58c Michal Simek        2010-01-18  395   * node and setup the resource mapping of a pci controller based on its
> d3afa58c Michal Simek        2010-01-18  396   * content.
> d3afa58c Michal Simek        2010-01-18  397   *
> d3afa58c Michal Simek        2010-01-18  398   * Life would be boring if it wasn't for a few issues that we have to deal
> d3afa58c Michal Simek        2010-01-18  399   * with here:
> d3afa58c Michal Simek        2010-01-18  400   *
> d3afa58c Michal Simek        2010-01-18  401   *   - We can only cope with one IO space range and up to 3 Memory space
> d3afa58c Michal Simek        2010-01-18  402   *     ranges. However, some machines (thanks Apple !) tend to split their
> d3afa58c Michal Simek        2010-01-18  403   *     space into lots of small contiguous ranges. So we have to coalesce.
> d3afa58c Michal Simek        2010-01-18  404   *
> d3afa58c Michal Simek        2010-01-18  405   *   - We can only cope with all memory ranges having the same offset
> d3afa58c Michal Simek        2010-01-18  406   *     between CPU addresses and PCI addresses. Unfortunately, some bridges
> d3afa58c Michal Simek        2010-01-18  407   *     are setup for a large 1:1 mapping along with a small "window" which
> d3afa58c Michal Simek        2010-01-18  408   *     maps PCI address 0 to some arbitrary high address of the CPU space in
> d3afa58c Michal Simek        2010-01-18  409   *     order to give access to the ISA memory hole.
> d3afa58c Michal Simek        2010-01-18  410   *     The way out of here that I've chosen for now is to always set the
> d3afa58c Michal Simek        2010-01-18  411   *     offset based on the first resource found, then override it if we
> d3afa58c Michal Simek        2010-01-18  412   *     have a different offset and the previous was set by an ISA hole.
> d3afa58c Michal Simek        2010-01-18  413   *
> d3afa58c Michal Simek        2010-01-18  414   *   - Some busses have IO space not starting at 0, which causes trouble with
> d3afa58c Michal Simek        2010-01-18  415   *     the way we do our IO resource renumbering. The code somewhat deals with
> d3afa58c Michal Simek        2010-01-18  416   *     it for 64 bits but I would expect problems on 32 bits.
> d3afa58c Michal Simek        2010-01-18  417   *
> d3afa58c Michal Simek        2010-01-18  418   *   - Some 32 bits platforms such as 4xx can have physical space larger than
> d3afa58c Michal Simek        2010-01-18  419   *     32 bits so we need to use 64 bits values for the parsing
> d3afa58c Michal Simek        2010-01-18  420   */
> b881bc46 Greg Kroah-Hartman  2012-12-21  421  void pci_process_bridge_OF_ranges(struct pci_controller *hose,
> b881bc46 Greg Kroah-Hartman  2012-12-21  422  				  struct device_node *dev, int primary)
> d3afa58c Michal Simek        2010-01-18  423  {
> d3afa58c Michal Simek        2010-01-18  424  	int memno = 0, isa_hole = -1;
> d3afa58c Michal Simek        2010-01-18  425  	unsigned long long isa_mb = 0;
> d3afa58c Michal Simek        2010-01-18  426  	struct resource *res;
> 4f7b6de4 Andrew Murray       2013-07-27  427  	struct of_pci_range range;
> 4f7b6de4 Andrew Murray       2013-07-27  428  	struct of_pci_range_parser parser;
> d3afa58c Michal Simek        2010-01-18  429  
> f2b8ae0e Rob Herring         2017-06-06  430  	pr_info("PCI host bridge %pOF %s ranges:\n",
> f2b8ae0e Rob Herring         2017-06-06  431  	       dev, primary ? "(primary)" : "");
> d3afa58c Michal Simek        2010-01-18  432  
> 4f7b6de4 Andrew Murray       2013-07-27  433  	/* Check for ranges property */
> 4f7b6de4 Andrew Murray       2013-07-27  434  	if (of_pci_range_parser_init(&parser, dev))
> d3afa58c Michal Simek        2010-01-18  435  		return;
> d3afa58c Michal Simek        2010-01-18  436  
> d3afa58c Michal Simek        2010-01-18  437  	pr_debug("Parsing ranges property...\n");
> 4f7b6de4 Andrew Murray       2013-07-27  438  	for_each_of_pci_range(&parser, &range) {
> d3afa58c Michal Simek        2010-01-18  439  		/* Read next ranges element */
> 6bd55f0b Michal Simek        2012-12-27  440  		pr_debug("pci_space: 0x%08x pci_addr:0x%016llx ",
> 4f7b6de4 Andrew Murray       2013-07-27  441  				range.pci_space, range.pci_addr);
> 6bd55f0b Michal Simek        2012-12-27  442  		pr_debug("cpu_addr:0x%016llx size:0x%016llx\n",
> 4f7b6de4 Andrew Murray       2013-07-27  443  					range.cpu_addr, range.size);
> d3afa58c Michal Simek        2010-01-18  444  
> d3afa58c Michal Simek        2010-01-18  445  		/* If we failed translation or got a zero-sized region
> d3afa58c Michal Simek        2010-01-18  446  		 * (some FW try to feed us with non sensical zero sized regions
> d3afa58c Michal Simek        2010-01-18  447  		 * such as power3 which look like some kind of attempt
> d3afa58c Michal Simek        2010-01-18  448  		 * at exposing the VGA memory hole)
> d3afa58c Michal Simek        2010-01-18  449  		 */
> 4f7b6de4 Andrew Murray       2013-07-27  450  		if (range.cpu_addr == OF_BAD_ADDR || range.size == 0)
> d3afa58c Michal Simek        2010-01-18  451  			continue;
> d3afa58c Michal Simek        2010-01-18  452  
> d3afa58c Michal Simek        2010-01-18  453  		/* Act based on address space type */
> d3afa58c Michal Simek        2010-01-18  454  		res = NULL;
> 4f7b6de4 Andrew Murray       2013-07-27  455  		switch (range.flags & IORESOURCE_TYPE_BITS) {
> 4f7b6de4 Andrew Murray       2013-07-27  456  		case IORESOURCE_IO:
> 6bd55f0b Michal Simek        2012-12-27  457  			pr_info("  IO 0x%016llx..0x%016llx -> 0x%016llx\n",
> 4f7b6de4 Andrew Murray       2013-07-27  458  				range.cpu_addr, range.cpu_addr + range.size - 1,
> 4f7b6de4 Andrew Murray       2013-07-27  459  				range.pci_addr);
> d3afa58c Michal Simek        2010-01-18  460  
> d3afa58c Michal Simek        2010-01-18  461  			/* We support only one IO range */
> d3afa58c Michal Simek        2010-01-18  462  			if (hose->pci_io_size) {
> 6bd55f0b Michal Simek        2012-12-27  463  				pr_info(" \\--> Skipped (too many) !\n");
> d3afa58c Michal Simek        2010-01-18  464  				continue;
> d3afa58c Michal Simek        2010-01-18  465  			}
> d3afa58c Michal Simek        2010-01-18  466  			/* On 32 bits, limit I/O space to 16MB */
> 4f7b6de4 Andrew Murray       2013-07-27  467  			if (range.size > 0x01000000)
> 4f7b6de4 Andrew Murray       2013-07-27  468  				range.size = 0x01000000;
> d3afa58c Michal Simek        2010-01-18  469  
> d3afa58c Michal Simek        2010-01-18  470  			/* 32 bits needs to map IOs here */
> 4f7b6de4 Andrew Murray       2013-07-27  471  			hose->io_base_virt = ioremap(range.cpu_addr,
> 4f7b6de4 Andrew Murray       2013-07-27  472  						range.size);
> d3afa58c Michal Simek        2010-01-18  473  
> d3afa58c Michal Simek        2010-01-18  474  			/* Expect trouble if pci_addr is not 0 */
> d3afa58c Michal Simek        2010-01-18  475  			if (primary)
> d3afa58c Michal Simek        2010-01-18  476  				isa_io_base =
> d3afa58c Michal Simek        2010-01-18  477  					(unsigned long)hose->io_base_virt;
> d3afa58c Michal Simek        2010-01-18  478  			/* pci_io_size and io_base_phys always represent IO
> d3afa58c Michal Simek        2010-01-18  479  			 * space starting at 0 so we factor in pci_addr
> d3afa58c Michal Simek        2010-01-18  480  			 */
> 4f7b6de4 Andrew Murray       2013-07-27  481  			hose->pci_io_size = range.pci_addr + range.size;
> 4f7b6de4 Andrew Murray       2013-07-27  482  			hose->io_base_phys = range.cpu_addr - range.pci_addr;
> d3afa58c Michal Simek        2010-01-18  483  
> d3afa58c Michal Simek        2010-01-18  484  			/* Build resource */
> d3afa58c Michal Simek        2010-01-18  485  			res = &hose->io_resource;
> 4f7b6de4 Andrew Murray       2013-07-27  486  			range.cpu_addr = range.pci_addr;
> 4f7b6de4 Andrew Murray       2013-07-27  487  
> d3afa58c Michal Simek        2010-01-18  488  			break;
> 4f7b6de4 Andrew Murray       2013-07-27  489  		case IORESOURCE_MEM:
> 6bd55f0b Michal Simek        2012-12-27  490  			pr_info(" MEM 0x%016llx..0x%016llx -> 0x%016llx %s\n",
> 4f7b6de4 Andrew Murray       2013-07-27  491  				range.cpu_addr, range.cpu_addr + range.size - 1,
> 4f7b6de4 Andrew Murray       2013-07-27  492  				range.pci_addr,
> 4f7b6de4 Andrew Murray       2013-07-27  493  				(range.pci_space & 0x40000000) ?
> 4f7b6de4 Andrew Murray       2013-07-27  494  				"Prefetch" : "");
> d3afa58c Michal Simek        2010-01-18  495  
> d3afa58c Michal Simek        2010-01-18  496  			/* We support only 3 memory ranges */
> d3afa58c Michal Simek        2010-01-18  497  			if (memno >= 3) {
> 6bd55f0b Michal Simek        2012-12-27  498  				pr_info(" \\--> Skipped (too many) !\n");
> d3afa58c Michal Simek        2010-01-18  499  				continue;
> d3afa58c Michal Simek        2010-01-18  500  			}
> d3afa58c Michal Simek        2010-01-18  501  			/* Handles ISA memory hole space here */
> 4f7b6de4 Andrew Murray       2013-07-27  502  			if (range.pci_addr == 0) {
> 4f7b6de4 Andrew Murray       2013-07-27  503  				isa_mb = range.cpu_addr;
> d3afa58c Michal Simek        2010-01-18  504  				isa_hole = memno;
> d3afa58c Michal Simek        2010-01-18  505  				if (primary || isa_mem_base == 0)
> 4f7b6de4 Andrew Murray       2013-07-27  506  					isa_mem_base = range.cpu_addr;
> 4f7b6de4 Andrew Murray       2013-07-27  507  				hose->isa_mem_phys = range.cpu_addr;
> 4f7b6de4 Andrew Murray       2013-07-27  508  				hose->isa_mem_size = range.size;
> d3afa58c Michal Simek        2010-01-18  509  			}
> d3afa58c Michal Simek        2010-01-18  510  
> d3afa58c Michal Simek        2010-01-18  511  			/* We get the PCI/Mem offset from the first range or
> d3afa58c Michal Simek        2010-01-18  512  			 * the, current one if the offset came from an ISA
> d3afa58c Michal Simek        2010-01-18  513  			 * hole. If they don't match, bugger.
> d3afa58c Michal Simek        2010-01-18  514  			 */
> d3afa58c Michal Simek        2010-01-18  515  			if (memno == 0 ||
> 4f7b6de4 Andrew Murray       2013-07-27  516  			    (isa_hole >= 0 && range.pci_addr != 0 &&
> d3afa58c Michal Simek        2010-01-18  517  			     hose->pci_mem_offset == isa_mb))
> 4f7b6de4 Andrew Murray       2013-07-27  518  				hose->pci_mem_offset = range.cpu_addr -
> 4f7b6de4 Andrew Murray       2013-07-27  519  							range.pci_addr;
> 4f7b6de4 Andrew Murray       2013-07-27  520  			else if (range.pci_addr != 0 &&
> 4f7b6de4 Andrew Murray       2013-07-27  521  				 hose->pci_mem_offset != range.cpu_addr -
> 4f7b6de4 Andrew Murray       2013-07-27  522  							range.pci_addr) {
> 6bd55f0b Michal Simek        2012-12-27  523  				pr_info(" \\--> Skipped (offset mismatch) !\n");
> d3afa58c Michal Simek        2010-01-18  524  				continue;
> d3afa58c Michal Simek        2010-01-18  525  			}
> d3afa58c Michal Simek        2010-01-18  526  
> d3afa58c Michal Simek        2010-01-18  527  			/* Build resource */
> d3afa58c Michal Simek        2010-01-18  528  			res = &hose->mem_resources[memno++];
> d3afa58c Michal Simek        2010-01-18  529  			break;
> d3afa58c Michal Simek        2010-01-18  530  		}
> 70dcd942 Michal Simek        2014-10-27  531  		if (res != NULL) {
> 70dcd942 Michal Simek        2014-10-27  532  			res->name = dev->full_name;
> 70dcd942 Michal Simek        2014-10-27  533  			res->flags = range.flags;
> 70dcd942 Michal Simek        2014-10-27  534  			res->start = range.cpu_addr;
> 70dcd942 Michal Simek        2014-10-27  535  			res->end = range.cpu_addr + range.size - 1;
> 70dcd942 Michal Simek        2014-10-27 @536  			res->parent = res->child = res->sibling = NULL;
> 70dcd942 Michal Simek        2014-10-27  537  		}
> d3afa58c Michal Simek        2010-01-18  538  	}
> d3afa58c Michal Simek        2010-01-18  539  
> d3afa58c Michal Simek        2010-01-18  540  	/* If there's an ISA hole and the pci_mem_offset is -not- matching
> d3afa58c Michal Simek        2010-01-18  541  	 * the ISA hole offset, then we need to remove the ISA hole from
> d3afa58c Michal Simek        2010-01-18  542  	 * the resource list for that brige
> d3afa58c Michal Simek        2010-01-18  543  	 */
> d3afa58c Michal Simek        2010-01-18  544  	if (isa_hole >= 0 && hose->pci_mem_offset != isa_mb) {
> d3afa58c Michal Simek        2010-01-18  545  		unsigned int next = isa_hole + 1;
> 6bd55f0b Michal Simek        2012-12-27  546  		pr_info(" Removing ISA hole at 0x%016llx\n", isa_mb);
> d3afa58c Michal Simek        2010-01-18  547  		if (next < memno)
> d3afa58c Michal Simek        2010-01-18  548  			memmove(&hose->mem_resources[isa_hole],
> d3afa58c Michal Simek        2010-01-18  549  				&hose->mem_resources[next],
> d3afa58c Michal Simek        2010-01-18  550  				sizeof(struct resource) * (memno - next));
> d3afa58c Michal Simek        2010-01-18  551  		hose->mem_resources[--memno].flags = 0;
> d3afa58c Michal Simek        2010-01-18  552  	}
> d3afa58c Michal Simek        2010-01-18  553  }
> d3afa58c Michal Simek        2010-01-18  554  
> 9413d968 Bharat Kumar Gogada 2016-09-01  555  /* Display the domain number in /proc */
> d3afa58c Michal Simek        2010-01-18  556  int pci_proc_domain(struct pci_bus *bus)
> d3afa58c Michal Simek        2010-01-18  557  {
> 9413d968 Bharat Kumar Gogada 2016-09-01  558  	return pci_domain_nr(bus);
> d3afa58c Michal Simek        2010-01-18  559  }
> d3afa58c Michal Simek        2010-01-18  560  
> d3afa58c Michal Simek        2010-01-18  561  /* This header fixup will do the resource fixup for all devices as they are
> d3afa58c Michal Simek        2010-01-18  562   * probed, but not for bridge ranges
> d3afa58c Michal Simek        2010-01-18  563   */
> b881bc46 Greg Kroah-Hartman  2012-12-21  564  static void pcibios_fixup_resources(struct pci_dev *dev)
> d3afa58c Michal Simek        2010-01-18  565  {
> d3afa58c Michal Simek        2010-01-18  566  	struct pci_controller *hose = pci_bus_to_host(dev->bus);
> d3afa58c Michal Simek        2010-01-18  567  	int i;
> d3afa58c Michal Simek        2010-01-18  568  
> d3afa58c Michal Simek        2010-01-18  569  	if (!hose) {
> 6bd55f0b Michal Simek        2012-12-27  570  		pr_err("No host bridge for PCI dev %s !\n",
> d3afa58c Michal Simek        2010-01-18  571  		       pci_name(dev));
> d3afa58c Michal Simek        2010-01-18  572  		return;
> d3afa58c Michal Simek        2010-01-18  573  	}
> d3afa58c Michal Simek        2010-01-18  574  	for (i = 0; i < DEVICE_COUNT_RESOURCE; i++) {
> d3afa58c Michal Simek        2010-01-18  575  		struct resource *res = dev->resource + i;
> d3afa58c Michal Simek        2010-01-18  576  		if (!res->flags)
> d3afa58c Michal Simek        2010-01-18  577  			continue;
> e5b36841 Bjorn Helgaas       2012-02-23  578  		if (res->start == 0) {
> 6bd55f0b Michal Simek        2012-12-27  579  			pr_debug("PCI:%s Resource %d %016llx-%016llx [%x]",
> d3afa58c Michal Simek        2010-01-18  580  				 pci_name(dev), i,
> d3afa58c Michal Simek        2010-01-18  581  				 (unsigned long long)res->start,
> d3afa58c Michal Simek        2010-01-18  582  				 (unsigned long long)res->end,
> d3afa58c Michal Simek        2010-01-18  583  				 (unsigned int)res->flags);
> 6bd55f0b Michal Simek        2012-12-27  584  			pr_debug("is unassigned\n");
> d3afa58c Michal Simek        2010-01-18  585  			res->end -= res->start;
> d3afa58c Michal Simek        2010-01-18  586  			res->start = 0;
> d3afa58c Michal Simek        2010-01-18  587  			res->flags |= IORESOURCE_UNSET;
> d3afa58c Michal Simek        2010-01-18  588  			continue;
> d3afa58c Michal Simek        2010-01-18  589  		}
> d3afa58c Michal Simek        2010-01-18  590  
> aa23bdc0 Bjorn Helgaas       2012-02-23  591  		pr_debug("PCI:%s Resource %d %016llx-%016llx [%x]\n",
> d3afa58c Michal Simek        2010-01-18  592  			 pci_name(dev), i,
> 6bd55f0b Michal Simek        2012-12-27  593  			 (unsigned long long)res->start,
> d3afa58c Michal Simek        2010-01-18  594  			 (unsigned long long)res->end,
> d3afa58c Michal Simek        2010-01-18  595  			 (unsigned int)res->flags);
> d3afa58c Michal Simek        2010-01-18  596  	}
> d3afa58c Michal Simek        2010-01-18  597  }
> d3afa58c Michal Simek        2010-01-18  598  DECLARE_PCI_FIXUP_HEADER(PCI_ANY_ID, PCI_ANY_ID, pcibios_fixup_resources);
> d3afa58c Michal Simek        2010-01-18  599  
> d3afa58c Michal Simek        2010-01-18  600  /*
> d3afa58c Michal Simek        2010-01-18  601   * We need to avoid collisions with `mirrored' VGA ports
> d3afa58c Michal Simek        2010-01-18  602   * and other strange ISA hardware, so we always want the
> d3afa58c Michal Simek        2010-01-18  603   * addresses to be allocated in the 0x000-0x0ff region
> d3afa58c Michal Simek        2010-01-18  604   * modulo 0x400.
> d3afa58c Michal Simek        2010-01-18  605   *
> d3afa58c Michal Simek        2010-01-18  606   * Why? Because some silly external IO cards only decode
> d3afa58c Michal Simek        2010-01-18  607   * the low 10 bits of the IO address. The 0x00-0xff region
> d3afa58c Michal Simek        2010-01-18  608   * is reserved for motherboard devices that decode all 16
> d3afa58c Michal Simek        2010-01-18  609   * bits, so it's ok to allocate at, say, 0x2800-0x28ff,
> d3afa58c Michal Simek        2010-01-18  610   * but we want to try to avoid allocating at 0x2900-0x2bff
> d3afa58c Michal Simek        2010-01-18  611   * which might have be mirrored at 0x0100-0x03ff..
> d3afa58c Michal Simek        2010-01-18  612   */
> 01cf9d52 Bharat Kumar Gogada 2016-02-11  613  int pcibios_add_device(struct pci_dev *dev)
> 01cf9d52 Bharat Kumar Gogada 2016-02-11  614  {
> 01cf9d52 Bharat Kumar Gogada 2016-02-11  615  	dev->irq = of_irq_parse_and_map_pci(dev, 0, 0);
> c86fac43 Michal Simek        2010-04-16  616  
> 01cf9d52 Bharat Kumar Gogada 2016-02-11  617  	return 0;
> d3afa58c Michal Simek        2010-01-18  618  }
> 01cf9d52 Bharat Kumar Gogada 2016-02-11  619  EXPORT_SYMBOL(pcibios_add_device);
> d3afa58c Michal Simek        2010-01-18  620  
> d3afa58c Michal Simek        2010-01-18  621  /*
> d3afa58c Michal Simek        2010-01-18  622   * Reparent resource children of pr that conflict with res
> d3afa58c Michal Simek        2010-01-18  623   * under res, and make res replace those children.
> d3afa58c Michal Simek        2010-01-18  624   */
> d3afa58c Michal Simek        2010-01-18  625  static int __init reparent_resources(struct resource *parent,
> d3afa58c Michal Simek        2010-01-18  626  				     struct resource *res)
> d3afa58c Michal Simek        2010-01-18  627  {
> d3afa58c Michal Simek        2010-01-18  628  	struct resource *p, **pp;
> d3afa58c Michal Simek        2010-01-18  629  	struct resource **firstpp = NULL;
> d3afa58c Michal Simek        2010-01-18  630  
> d3afa58c Michal Simek        2010-01-18 @631  	for (pp = &parent->child; (p = *pp) != NULL; pp = &p->sibling) {
> d3afa58c Michal Simek        2010-01-18  632  		if (p->end < res->start)
> d3afa58c Michal Simek        2010-01-18  633  			continue;
> d3afa58c Michal Simek        2010-01-18  634  		if (res->end < p->start)
> d3afa58c Michal Simek        2010-01-18  635  			break;
> d3afa58c Michal Simek        2010-01-18  636  		if (p->start < res->start || p->end > res->end)
> d3afa58c Michal Simek        2010-01-18  637  			return -1;	/* not completely contained */
> d3afa58c Michal Simek        2010-01-18  638  		if (firstpp == NULL)
> d3afa58c Michal Simek        2010-01-18  639  			firstpp = pp;
> d3afa58c Michal Simek        2010-01-18  640  	}
> d3afa58c Michal Simek        2010-01-18  641  	if (firstpp == NULL)
> d3afa58c Michal Simek        2010-01-18  642  		return -1;	/* didn't find any conflicting entries? */
> d3afa58c Michal Simek        2010-01-18  643  	res->parent = parent;
> d3afa58c Michal Simek        2010-01-18 @644  	res->child = *firstpp;
> d3afa58c Michal Simek        2010-01-18  645  	res->sibling = *pp;
> d3afa58c Michal Simek        2010-01-18  646  	*firstpp = res;
> d3afa58c Michal Simek        2010-01-18  647  	*pp = NULL;
> d3afa58c Michal Simek        2010-01-18 @648  	for (p = res->child; p != NULL; p = p->sibling) {
> d3afa58c Michal Simek        2010-01-18  649  		p->parent = res;
> d3afa58c Michal Simek        2010-01-18  650  		pr_debug("PCI: Reparented %s [%llx..%llx] under %s\n",
> d3afa58c Michal Simek        2010-01-18  651  			 p->name,
> d3afa58c Michal Simek        2010-01-18  652  			 (unsigned long long)p->start,
> d3afa58c Michal Simek        2010-01-18  653  			 (unsigned long long)p->end, res->name);
> d3afa58c Michal Simek        2010-01-18  654  	}
> d3afa58c Michal Simek        2010-01-18  655  	return 0;
> d3afa58c Michal Simek        2010-01-18  656  }
> d3afa58c Michal Simek        2010-01-18  657  
> 
> :::::: The code at line 536 was first introduced by commit
> :::::: 70dcd942dc4af3cc6c3dcc2ba499cd841c7f65a7 microblaze: Fix IO space breakage after of_pci_range_to_resource() change
> 
> :::::: TO: Michal Simek <michal.simek@xilinx.com>
> :::::: CC: Michal Simek <michal.simek@xilinx.com>
> 
> ---
> 0-DAY kernel test infrastructure                Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation


_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v3 1/3] resource: Use list_head to link sibling resource
  2018-04-26  1:18   ` Wei Yang
@ 2018-05-07  1:14     ` Baoquan He
  2018-05-08 11:48       ` Wei Yang
  0 siblings, 1 reply; 19+ messages in thread
From: Baoquan He @ 2018-05-07  1:14 UTC (permalink / raw)
  To: Wei Yang
  Cc: nicolas.pitre, Brijesh Singh, devicetree, David Airlie,
	linux-pci, Keith Busch, Yaowei Bai, Frank Rowand, dan.j.williams,
	Lorenzo Pieralisi, Stephen Hemminger, linux-nvdimm,
	Patrik Jakobsson, linux-input, Borislav Petkov, Tom Lendacky,
	Haiyang Zhang, josh, Jérôme Glisse, robh+dt,
	Bjorn Helgaas, Thomas Gleixner, Jonathan Derrick,
	Greg Kroah-Hartman, Dmitry Torokhov, linux-kernel, devel, akpm

Hi Wei Yang,

On 04/26/18 at 09:18am, Wei Yang wrote:
> On Thu, Apr 19, 2018 at 08:18:46AM +0800, Baoquan He wrote:
> >The struct resource uses singly linked list to link siblings. It's not
> >easy to do reverse iteration on sibling list. So replace it with list_head.
> >
> 
> Hi, Baoquan
> 
> Besides changing the data structure, I have another proposal to do the reverse
> iteration. Which means it would not affect other users, if you just want a
> reverse iteration.
> 
> BTW, I don't think Andrew suggest to use linked-list directly. What he wants
> is a better solution to your first proposal in
> https://patchwork.kernel.org/patch/10300819/.
> 
> Below is my proposal of resource reverse iteration without changing current
> design.

I got your mail and read it, then interrupted by other thing and forgot
replying, sorry.

I am fine with your code change. As I said before, I have tried to change
code per reviewers' comment, then let reviewers decide which way is
better. Please feel free to post formal patches and joining discussion
about this issue.

Thanks
Baoquan

> 
> From 5d7145d44fe48b98572a03884fa3a3aa82e3cef9 Mon Sep 17 00:00:00 2001
> From: Wei Yang <richard.weiyang@gmail.com>
> Date: Sat, 24 Mar 2018 23:25:46 +0800
> Subject: [PATCH] kernel/resource: add walk_system_ram_res_rev()
> 
> As discussed on https://patchwork.kernel.org/patch/10300819/, this patch
> comes up with a variant implementation of walk_system_ram_res_rev(), which
> uses iteration instead of allocating array to store those resources.
> 
> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
> ---
>  include/linux/ioport.h |   3 ++
>  kernel/resource.c      | 113 +++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 116 insertions(+)
> 
> diff --git a/include/linux/ioport.h b/include/linux/ioport.h
> index da0ebaec25f0..473f1d9cb97e 100644
> --- a/include/linux/ioport.h
> +++ b/include/linux/ioport.h
> @@ -277,6 +277,9 @@ extern int
>  walk_system_ram_res(u64 start, u64 end, void *arg,
>  		    int (*func)(struct resource *, void *));
>  extern int
> +walk_system_ram_res_rev(u64 start, u64 end, void *arg,
> +		    int (*func)(struct resource *, void *));
> +extern int
>  walk_iomem_res_desc(unsigned long desc, unsigned long flags, u64 start, u64 end,
>  		    void *arg, int (*func)(struct resource *, void *));
>  
> diff --git a/kernel/resource.c b/kernel/resource.c
> index 769109f20fb7..d4ec5fbc6875 100644
> --- a/kernel/resource.c
> +++ b/kernel/resource.c
> @@ -73,6 +73,38 @@ static struct resource *next_resource(struct resource *p, bool sibling_only)
>  	return p->sibling;
>  }
>  
> +static struct resource *prev_resource(struct resource *p, bool sibling_only)
> +{
> +	struct resource *prev;
> +	if (NULL == iomem_resource.child)
> +		return NULL;
> +
> +	if (p == NULL) {
> +		prev = iomem_resource.child;
> +		while (prev->sibling)
> +			prev = prev->sibling;
> +	} else {
> +		if (p->parent->child == p) {
> +			return p->parent;
> +		}
> +
> +		for (prev = p->parent->child; prev->sibling != p;
> +			prev = prev->sibling) {}
> +	}
> +
> +	/* Caller wants to traverse through siblings only */
> +	if (sibling_only)
> +		return prev;
> +
> +	for (;prev->child;) {
> +		prev = prev->child;
> +
> +		while (prev->sibling)
> +			prev = prev->sibling;
> +	}
> +	return prev;
> +}
> +
>  static void *r_next(struct seq_file *m, void *v, loff_t *pos)
>  {
>  	struct resource *p = v;
> @@ -401,6 +433,47 @@ static int find_next_iomem_res(struct resource *res, unsigned long desc,
>  	return 0;
>  }
>  
> +/*
> + * Finds the highest iomem resource existing within [res->start.res->end).
> + * The caller must specify res->start, res->end, res->flags, and optionally
> + * desc.  If found, returns 0, res is overwritten, if not found, returns -1.
> + * This function walks the whole tree and not just first level children until
> + * and unless first_level_children_only is true.
> + */
> +static int find_prev_iomem_res(struct resource *res, unsigned long desc,
> +			       bool first_level_children_only)
> +{
> +	struct resource *p;
> +
> +	BUG_ON(!res);
> +	BUG_ON(res->start >= res->end);
> +
> +	read_lock(&resource_lock);
> +
> +	for (p = prev_resource(NULL, first_level_children_only); p;
> +		p = prev_resource(p, first_level_children_only)) {
> +		if ((p->flags & res->flags) != res->flags)
> +			continue;
> +		if ((desc != IORES_DESC_NONE) && (desc != p->desc))
> +			continue;
> +		if (p->end < res->start || p->child == iomem_resource.child) {
> +			p = NULL;
> +			break;
> +		}
> +		if ((p->end >= res->start) && (p->start < res->end))
> +			break;
> +	}
> +
> +	read_unlock(&resource_lock);
> +	if (!p)
> +		return -1;
> +	/* copy data */
> +	resource_clip(res, p->start, p->end);
> +	res->flags = p->flags;
> +	res->desc = p->desc;
> +	return 0;
> +}
> +
>  static int __walk_iomem_res_desc(struct resource *res, unsigned long desc,
>  				 bool first_level_children_only,
>  				 void *arg,
> @@ -422,6 +495,27 @@ static int __walk_iomem_res_desc(struct resource *res, unsigned long desc,
>  	return ret;
>  }
>  
> +static int __walk_iomem_res_rev_desc(struct resource *res, unsigned long desc,
> +				 bool first_level_children_only,
> +				 void *arg,
> +				 int (*func)(struct resource *, void *))
> +{
> +	u64 orig_start = res->start;
> +	int ret = -1;
> +
> +	while ((res->start < res->end) &&
> +	       !find_prev_iomem_res(res, desc, first_level_children_only)) {
> +		ret = (*func)(res, arg);
> +		if (ret)
> +			break;
> +
> +		res->end = res->start?(res->start - 1):0;
> +		res->start = orig_start;
> +	}
> +
> +	return ret;
> +}
> +
>  /*
>   * Walks through iomem resources and calls func() with matching resource
>   * ranges. This walks through whole tree and not just first level children.
> @@ -468,6 +562,25 @@ int walk_system_ram_res(u64 start, u64 end, void *arg,
>  				     arg, func);
>  }
>  
> +/*
> + * This function, being a variant of walk_system_ram_res(), calls the @func
> + * callback against all memory ranges of type System RAM which are marked as
> + * IORESOURCE_SYSTEM_RAM and IORESOUCE_BUSY in reversed order, i.e., from
> + * higher to lower.
> + */
> +int walk_system_ram_res_rev(u64 start, u64 end, void *arg,
> +				int (*func)(struct resource *, void *))
> +{
> +	struct resource res;
> +
> +	res.start = start;
> +	res.end = end;
> +	res.flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY;
> +
> +	return __walk_iomem_res_rev_desc(&res, IORES_DESC_NONE, true,
> +				     arg, func);
> +}
> +
>  /*
>   * This function calls the @func callback against all memory ranges, which
>   * are ranges marked as IORESOURCE_MEM and IORESOUCE_BUSY.
> -- 
> 2.15.1
> 
> 
> -- 
> Wei Yang
> Help you, Help me
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v3 1/3] resource: Use list_head to link sibling resource
  2018-05-07  1:14     ` Baoquan He
@ 2018-05-08 11:48       ` Wei Yang
  2018-05-08 12:11         ` Baoquan He
  0 siblings, 1 reply; 19+ messages in thread
From: Wei Yang @ 2018-05-08 11:48 UTC (permalink / raw)
  To: Baoquan He
  Cc: nicolas.pitre, Brijesh Singh, devicetree, David Airlie,
	linux-pci, Wei Yang, Keith Busch, Yaowei Bai, Frank Rowand,
	dan.j.williams, Lorenzo Pieralisi, Stephen Hemminger,
	linux-nvdimm, Patrik Jakobsson, linux-input, Borislav Petkov,
	Tom Lendacky, Haiyang Zhang, josh, Jérôme Glisse,
	robh+dt, Bjorn Helgaas, Thomas Gleixner, Jonathan Derrick,
	Greg Kroah-Hartman, Dmitry Torokhov, linux-kernel, devel, akpm

On Mon, May 07, 2018 at 09:14:29AM +0800, Baoquan He wrote:
>Hi Wei Yang,
>
>On 04/26/18 at 09:18am, Wei Yang wrote:
>> On Thu, Apr 19, 2018 at 08:18:46AM +0800, Baoquan He wrote:
>> >The struct resource uses singly linked list to link siblings. It's not
>> >easy to do reverse iteration on sibling list. So replace it with list_head.
>> >
>> 
>> Hi, Baoquan
>> 
>> Besides changing the data structure, I have another proposal to do the reverse
>> iteration. Which means it would not affect other users, if you just want a
>> reverse iteration.
>> 
>> BTW, I don't think Andrew suggest to use linked-list directly. What he wants
>> is a better solution to your first proposal in
>> https://patchwork.kernel.org/patch/10300819/.
>> 
>> Below is my proposal of resource reverse iteration without changing current
>> design.
>
>I got your mail and read it, then interrupted by other thing and forgot
>replying, sorry.
>
>I am fine with your code change. As I said before, I have tried to change
>code per reviewers' comment, then let reviewers decide which way is
>better. Please feel free to post formal patches and joining discussion
>about this issue.

Yep, while I don't have a real requirement to add the reverse version, so what
is the proper way to send a patch?

A patch reply to this thread is ok?

>
>Thanks
>Baoquan
>
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v3 1/3] resource: Use list_head to link sibling resource
  2018-05-08 11:48       ` Wei Yang
@ 2018-05-08 12:11         ` Baoquan He
  2018-05-08 23:41           ` Wei Yang
  0 siblings, 1 reply; 19+ messages in thread
From: Baoquan He @ 2018-05-08 12:11 UTC (permalink / raw)
  To: Wei Yang
  Cc: linux-kernel, akpm, robh+dt, dan.j.williams, nicolas.pitre, josh,
	Patrik Jakobsson, David Airlie, K. Y. Srinivasan, Haiyang Zhang,
	Stephen Hemminger, Dmitry Torokhov, Frank Rowand, Keith Busch,
	Jonathan Derrick, Lorenzo Pieralisi, Bjorn Helgaas,
	Thomas Gleixner, Brijesh Singh, Jérôme Glisse,
	Borislav Petkov, Tom Lendacky, Greg Kroah-Hartman, Yaowei Bai,
	devel, linux-input, linux-nvdimm, devicetree, linux-pci

On 05/08/18 at 08:48pm, Wei Yang wrote:
> On Mon, May 07, 2018 at 09:14:29AM +0800, Baoquan He wrote:
> >Hi Wei Yang,
> >
> >On 04/26/18 at 09:18am, Wei Yang wrote:
> >> On Thu, Apr 19, 2018 at 08:18:46AM +0800, Baoquan He wrote:
> >> >The struct resource uses singly linked list to link siblings. It's not
> >> >easy to do reverse iteration on sibling list. So replace it with list_head.
> >> >
> >> 
> >> Hi, Baoquan
> >> 
> >> Besides changing the data structure, I have another proposal to do the reverse
> >> iteration. Which means it would not affect other users, if you just want a
> >> reverse iteration.
> >> 
> >> BTW, I don't think Andrew suggest to use linked-list directly. What he wants
> >> is a better solution to your first proposal in
> >> https://patchwork.kernel.org/patch/10300819/.
> >> 
> >> Below is my proposal of resource reverse iteration without changing current
> >> design.
> >
> >I got your mail and read it, then interrupted by other thing and forgot
> >replying, sorry.
> >
> >I am fine with your code change. As I said before, I have tried to change
> >code per reviewers' comment, then let reviewers decide which way is
> >better. Please feel free to post formal patches and joining discussion
> >about this issue.
> 
> Yep, while I don't have a real requirement to add the reverse version, so what
> is the proper way to send a patch?
> 
> A patch reply to this thread is ok?

I am not sure either. Since my patches are still under reviewing. And
you have pasted your patch. It depends on maintainers, mainly Andrew and
other reviewers who have concerns.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v3 1/3] resource: Use list_head to link sibling resource
  2018-05-08 12:11         ` Baoquan He
@ 2018-05-08 23:41           ` Wei Yang
  0 siblings, 0 replies; 19+ messages in thread
From: Wei Yang @ 2018-05-08 23:41 UTC (permalink / raw)
  To: Baoquan He
  Cc: nicolas.pitre, Brijesh Singh, devicetree, David Airlie,
	linux-pci, Wei Yang, Keith Busch, Yaowei Bai, Frank Rowand,
	dan.j.williams, Lorenzo Pieralisi, Stephen Hemminger,
	linux-nvdimm, Patrik Jakobsson, linux-input, Borislav Petkov,
	Tom Lendacky, Haiyang Zhang, josh, Jérôme Glisse,
	robh+dt, Bjorn Helgaas, Thomas Gleixner, Jonathan Derrick,
	Greg Kroah-Hartman, Dmitry Torokhov, linux-kernel, devel, akpm

On Tue, May 08, 2018 at 08:11:23PM +0800, Baoquan He wrote:
>On 05/08/18 at 08:48pm, Wei Yang wrote:
>> On Mon, May 07, 2018 at 09:14:29AM +0800, Baoquan He wrote:
>> >Hi Wei Yang,
>> >
>> >On 04/26/18 at 09:18am, Wei Yang wrote:
>> >> On Thu, Apr 19, 2018 at 08:18:46AM +0800, Baoquan He wrote:
>> >> >The struct resource uses singly linked list to link siblings. It's not
>> >> >easy to do reverse iteration on sibling list. So replace it with list_head.
>> >> >
>> >> 
>> >> Hi, Baoquan
>> >> 
>> >> Besides changing the data structure, I have another proposal to do the reverse
>> >> iteration. Which means it would not affect other users, if you just want a
>> >> reverse iteration.
>> >> 
>> >> BTW, I don't think Andrew suggest to use linked-list directly. What he wants
>> >> is a better solution to your first proposal in
>> >> https://patchwork.kernel.org/patch/10300819/.
>> >> 
>> >> Below is my proposal of resource reverse iteration without changing current
>> >> design.
>> >
>> >I got your mail and read it, then interrupted by other thing and forgot
>> >replying, sorry.
>> >
>> >I am fine with your code change. As I said before, I have tried to change
>> >code per reviewers' comment, then let reviewers decide which way is
>> >better. Please feel free to post formal patches and joining discussion
>> >about this issue.
>> 
>> Yep, while I don't have a real requirement to add the reverse version, so what
>> is the proper way to send a patch?
>> 
>> A patch reply to this thread is ok?
>
>I am not sure either. Since my patches are still under reviewing. And
>you have pasted your patch. It depends on maintainers, mainly Andrew and
>other reviewers who have concerns.

Ok, thanks.

-- 
Wei Yang
Help you, Help me
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2018-05-08 23:41 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-19  0:18 [PATCH v3 0/3] resource: Use list_head to link sibling resource Baoquan He
2018-04-19  0:18 ` [PATCH v3 1/3] " Baoquan He
2018-04-26  1:18   ` Wei Yang
2018-05-07  1:14     ` Baoquan He
2018-05-08 11:48       ` Wei Yang
2018-05-08 12:11         ` Baoquan He
2018-05-08 23:41           ` Wei Yang
2018-04-26  3:01   ` kbuild test robot
2018-05-06  6:31     ` Baoquan He
2018-04-26  3:23   ` kbuild test robot
2018-05-06  6:30     ` Baoquan He
2018-04-19  0:18 ` [PATCH v3 2/3] resource: add walk_system_ram_res_rev() Baoquan He
2018-04-19 10:07   ` Borislav Petkov
2018-04-26  8:56     ` Baoquan He
2018-04-26 11:09       ` Borislav Petkov
2018-04-26 13:22         ` Baoquan He
2018-05-04 10:16           ` Borislav Petkov
2018-05-06  6:19             ` Baoquan He
2018-04-19  0:18 ` [PATCH v3 3/3] kexec_file: Load kernel at top of system RAM if required Baoquan He

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).