Re: [PATCH v3 1/3] resource: Use list_head to link sibling resource
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 ___ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm
Re: [PATCH v3 1/3] resource: Use list_head to link sibling resource
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. ___ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm
Re: [PATCH v3 1/3] resource: Use list_head to link sibling resource
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 > ___ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm
Re: [PATCH v3 1/3] resource: Use list_head to link sibling resource
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
> 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
> ---
> 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))
> +
Re: [PATCH v3 1/3] resource: Use list_head to link sibling resource
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
Re: [PATCH v3 1/3] resource: Use list_head to link sibling resource
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
> :: CC: Linus Torvalds
>
> ---
> 0-DAY kernel test infrastructureOpen Source Technology Center
> https://lists.01.org/pipermail/kbuild-all Intel Corporation
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
Re: [PATCH v3 1/3] resource: Use list_head to link sibling resource
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
:: CC: Linus Torvalds
---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
Re: [PATCH v3 1/3] resource: Use list_head to link sibling resource
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 Simek2010-01-18 387
d3afa58c Michal Simek2010-01-18 388 /**
d3afa58c Michal Simek2010-01-18 389 * pci_process_bridge_OF_ranges -
Parse PCI bridge resources from device tree
d3afa58c Michal Simek2010-01-18 390 * @hose: newly allocated
pci_controller to be setup
d3afa58c Michal Simek2010-01-18 391 * @dev: device node of the host
bridge
d3afa58c Michal Simek2010-01-18 392 * @primary: set if primary bus
(32 bits only, soon to be deprecated)
d3afa58c Michal Simek2010-01-18 393 *
d3afa58c Michal Simek2010-01-18 394 * This function will parse the
"ranges" property of a PCI host bridge device
d3afa58c Michal Simek2010-01-18 395 * node and setup the resource
mapping of a pci controller based on its
d3afa58c Michal Simek2010-01-18 396 * content.
d3afa58c Michal Simek2010-01-18 397 *
d3afa58c Michal Simek2010-01-18 398 * Life would be boring if it
wasn't for a few issues that we have to deal
d3afa58c Michal Simek2010-01-18 399 * with here:
d3afa58c Michal Simek2010-01-18 400 *
d3afa58c Michal Simek2010-01-18 401 * - We can only cope with one
IO space range and up to 3 Memory space
d3afa58c Michal Simek2010-01-18 402 * ranges. However, some
machines (thanks Apple !) tend to split their
d3afa58c Michal Simek2010-01-18 403 * space into lots of small
contiguous ranges. So we have to coalesce.
d3afa58c Michal Simek2010-01-18 404 *
d3afa58c Michal Simek2010-01-18 405 * - We can only cope with all
memory ranges having the same offset
d3afa58c Michal Simek2010-01-18 406 * between CPU addresses and
PCI addresses. Unfortunately, some bridges
d3afa58c Michal Simek2010-01-18 407 * are setup for a large 1:1
mapping along with a small "window" which
d3afa58c Michal Simek2010-01-18 408 * maps PCI address 0 to some
arbitrary high address of the CPU space in
d3afa58c Michal Simek2010-01-18 409 * order to give access to
the ISA memory hole.
d3afa58c Michal Simek2010-01-18 410 * The way out of here that
I've chosen for now is to always set the
d3afa58c Michal Simek2010-01-18 411 * offset based on the first
resource found, then override it if we
d3afa58c Michal Simek2010-01-18 412 * have
Re: [PATCH v3 1/3] resource: Use list_head to link sibling resource
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
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
---
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 r
[PATCH v3 1/3] resource: Use list_head to link sibling resource
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
Signed-off-by: Baoquan He
Cc: Patrik Jakobsson
Cc: David Airlie
Cc: "K. Y. Srinivasan"
Cc: Haiyang Zhang
Cc: Stephen Hemminger
Cc: Dmitry Torokhov
Cc: Dan Williams
Cc: Rob Herring
Cc: Frank Rowand
Cc: Keith Busch
Cc: Jonathan Derrick
Cc: Lorenzo Pieralisi
Cc: Bjorn Helgaas
Cc: Thomas Gleixner
Cc: Brijesh Singh
Cc: "Jérôme Glisse"
Cc: Borislav Petkov
Cc: Tom Lendacky
Cc: Greg Kroah-Hartman
Cc: Yaowei Bai
Cc: Wei Yang
Cc: de...@linuxdriverproject.org
Cc: linux-in...@vger.kernel.org
Cc: linux-nvdimm@lists.01.org
Cc: devicet...@vger.kernel.org
Cc: linux-...@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
