LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Baoquan He <bhe@redhat.com>
To: Wei Yang <richard.weiyang@gmail.com>
Cc: nicolas.pitre@linaro.org, "Brijesh Singh" <brijesh.singh@amd.com>,
	devicetree@vger.kernel.org, "David Airlie" <airlied@linux.ie>,
	linux-pci@vger.kernel.org, "Keith Busch" <keith.busch@intel.com>,
	"Yaowei Bai" <baiyaowei@cmss.chinamobile.com>,
	"Frank Rowand" <frowand.list@gmail.com>,
	dan.j.williams@intel.com,
	"Lorenzo Pieralisi" <lorenzo.pieralisi@arm.com>,
	"Stephen Hemminger" <sthemmin@microsoft.com>,
	linux-nvdimm@lists.01.org,
	"Patrik Jakobsson" <patrik.r.jakobsson@gmail.com>,
	linux-input@vger.kernel.org, "Borislav Petkov" <bp@suse.de>,
	"Tom Lendacky" <thomas.lendacky@amd.com>,
	"Haiyang Zhang" <haiyangz@microsoft.com>,
	josh@joshtriplett.org, "Jérôme Glisse" <jglisse@redhat.com>,
	robh+dt@kernel.org, "Bjorn Helgaas" <bhelgaas@google.com>,
	"Thomas Gleixner" <tglx@linutronix.de>,
	"Jonathan Derrick" <jonathan.derrick@intel.com>,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	"Dmitry Torokhov" <dmitry.torokhov@gmail.com>,
	linux-kernel@vger.kernel.org, devel@linuxdriverproject.org,
	akpm@linux-foundation.org
Subject: Re: [PATCH v3 1/3] resource: Use list_head to link sibling resource
Date: Mon, 7 May 2018 09:14:29 +0800	[thread overview]
Message-ID: <20180507011429.GG30581@MiWiFi-R3L-srv> (raw)
In-Reply-To: <20180426011837.GA79340@WeideMacBook-Pro.local>

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

  reply	other threads:[~2018-05-07  1:14 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-19  0:18 [PATCH v3 0/3] " 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 [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180507011429.GG30581@MiWiFi-R3L-srv \
    --to=bhe@redhat.com \
    --cc=airlied@linux.ie \
    --cc=akpm@linux-foundation.org \
    --cc=baiyaowei@cmss.chinamobile.com \
    --cc=bhelgaas@google.com \
    --cc=bp@suse.de \
    --cc=brijesh.singh@amd.com \
    --cc=dan.j.williams@intel.com \
    --cc=devel@linuxdriverproject.org \
    --cc=devicetree@vger.kernel.org \
    --cc=dmitry.torokhov@gmail.com \
    --cc=frowand.list@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=haiyangz@microsoft.com \
    --cc=jglisse@redhat.com \
    --cc=jonathan.derrick@intel.com \
    --cc=josh@joshtriplett.org \
    --cc=keith.busch@intel.com \
    --cc=linux-input@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nvdimm@lists.01.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=lorenzo.pieralisi@arm.com \
    --cc=nicolas.pitre@linaro.org \
    --cc=patrik.r.jakobsson@gmail.com \
    --cc=richard.weiyang@gmail.com \
    --cc=robh+dt@kernel.org \
    --cc=sthemmin@microsoft.com \
    --cc=tglx@linutronix.de \
    --cc=thomas.lendacky@amd.com \
    --subject='Re: [PATCH v3 1/3] resource: Use list_head to link sibling resource' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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).