From: Yinghai Lu <yinghai@kernel.org>
To: Ram Pai <linuxram@us.ibm.com>, Bjorn Helgaas <bhelgaas@google.com>
Cc: linux-pci@vger.kernel.org
Subject: Re: [RFC PATCH v3 ]pci: pci resource iterator
Date: Thu, 23 Aug 2012 12:30:05 -0700 [thread overview]
Message-ID: <CAE9FiQVq=6Shv_4taNzN3h4OBs1PgoFJZqV0i3tRvmz04qZbXw@mail.gmail.com> (raw)
In-Reply-To: <20120823050958.GB2332@ram-ThinkPad-T61>
[-- Attachment #1: Type: text/plain, Size: 5709 bytes --]
Hi, Ram and Bjorn,
On Wed, Aug 22, 2012 at 10:09 PM, Ram Pai <linuxram@us.ibm.com> wrote:
> +static inline int pci_next_resource_idx(int i, int flag)
> +{
> + while (++i < PCI_NUM_RESOURCES) {
> + if ((i >= 0 && i < PCI_ROM_RESOURCE && (flag & PCI_STD_RES)) ||
> + (i == PCI_ROM_RESOURCE && (flag & PCI_ROM_RES)) ||
> +#ifdef CONFIG_PCI_IOV
> + (i <= PCI_IOV_RESOURCE_END && (flag & PCI_IOV_RES)) ||
> +#endif
> + (i <= PCI_BRIDGE_RESOURCE_END && (flag & PCI_BRIDGE_RES)))
> + return i;
> + }
> + return -1;
> +}
no, you can not merge them.
when it start as -1, and user only need the bridge resource, it will
loop from 0 to 16.
I optimized it more to skip some searching. please check v5 and v6.
v5 will store aside the mask, and use the bit map later.
v6 will still to do the local checking, but will skip some ++i loop.
it seems v5 is more efficient while it will need 16*8 bytes for mask storage.
v5: pci_next_resource_idx:
000000000000098c <pci_next_resource_idx>:
98c: 55 push %rbp
98d: 48 89 e5 mov %rsp,%rbp
990: 53 push %rbx
991: 41 50 push %r8
993: e8 00 00 00 00 callq 998 <pci_next_resource_idx+0xc>
998: 89 fb mov %edi,%ebx
99a: 89 f7 mov %esi,%edi
99c: ff c3 inc %ebx
99e: e8 d1 fe ff ff callq 874 <get_res_idx_mask>
9a3: 48 89 c7 mov %rax,%rdi
9a6: 83 c8 ff or $0xffffffffffffffff,%eax
9a9: 83 fb 10 cmp $0x10,%ebx
9ac: 7f 18 jg 9c6 <pci_next_resource_idx+0x3a>
9ae: 48 63 d3 movslq %ebx,%rdx
9b1: be 11 00 00 00 mov $0x11,%esi
9b6: e8 00 00 00 00 callq 9bb <pci_next_resource_idx+0x2f>
9bb: ba ff ff ff ff mov $0xffffffff,%edx
9c0: 83 f8 11 cmp $0x11,%eax
9c3: 0f 4d c2 cmovge %edx,%eax
9c6: 59 pop %rcx
9c7: 5b pop %rbx
9c8: 5d pop %rbp
9c9: c3 retq
v6: would need to more code in pci_next_resource_idx:
00000000000008e1 <pci_next_resource_idx>:
8e1: 55 push %rbp
8e2: 48 89 e5 mov %rsp,%rbp
8e5: e8 00 00 00 00 callq 8ea <pci_next_resource_idx+0x9>
8ea: 89 f1 mov %esi,%ecx
8ec: 89 f2 mov %esi,%edx
8ee: 89 f8 mov %edi,%eax
8f0: 89 f7 mov %esi,%edi
8f2: 83 e1 04 and $0x4,%ecx
8f5: 83 e2 02 and $0x2,%edx
8f8: 83 e7 08 and $0x8,%edi
8fb: 83 e6 01 and $0x1,%esi
8fe: eb 35 jmp 935 <pci_next_resource_idx+0x54>
900: 85 c0 test %eax,%eax
902: 78 1c js 920 <pci_next_resource_idx+0x3f>
904: 83 f8 05 cmp $0x5,%eax
907: 7f 07 jg 910 <pci_next_resource_idx+0x2f>
909: 40 84 f6 test %sil,%sil
90c: 75 38 jne 946 <pci_next_resource_idx+0x65>
90e: eb 05 jmp 915 <pci_next_resource_idx+0x34>
910: 83 f8 06 cmp $0x6,%eax
913: 75 0b jne 920 <pci_next_resource_idx+0x3f>
915: 85 d2 test %edx,%edx
917: 75 28 jne 941 <pci_next_resource_idx+0x60>
919: b8 07 00 00 00 mov $0x7,%eax
91e: eb 0a jmp 92a <pci_next_resource_idx+0x49>
920: 83 f8 06 cmp $0x6,%eax
923: 7e 10 jle 935 <pci_next_resource_idx+0x54>
925: 83 f8 0c cmp $0xc,%eax
928: 7f 1e jg 948 <pci_next_resource_idx+0x67>
92a: 85 c9 test %ecx,%ecx
92c: 75 18 jne 946 <pci_next_resource_idx+0x65>
92e: b8 0d 00 00 00 mov $0xd,%eax
933: eb 13 jmp 948 <pci_next_resource_idx+0x67>
935: ff c0 inc %eax
937: 83 f8 10 cmp $0x10,%eax
93a: 7e c4 jle 900 <pci_next_resource_idx+0x1f>
93c: 83 c8 ff or $0xffffffffffffffff,%eax
93f: eb 05 jmp 946 <pci_next_resource_idx+0x65>
941: b8 06 00 00 00 mov $0x6,%eax
946: 5d pop %rbp
947: c3 retq
948: 85 ff test %edi,%edi
94a: 75 fa jne 946 <pci_next_resource_idx+0x65>
94c: b8 11 00 00 00 mov $0x11,%eax
951: eb e2 jmp 935 <pci_next_resource_idx+0x54>
So just -v5 from now?
Please let me know which one is good, so I could rebase
for-pci-for-each-res-addon branch,
and then Ram could rebase his patches to top of new
for-pci-each-res-addon branch.
Thanks
Yinghai
[-- Attachment #2: ram_pci_it_v5.patch --]
[-- Type: application/octet-stream, Size: 3131 bytes --]
---
drivers/pci/probe.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
include/linux/pci.h | 24 ++++++++++++++++++++++++
2 files changed, 72 insertions(+)
Index: linux-2.6/include/linux/pci.h
===================================================================
--- linux-2.6.orig/include/linux/pci.h
+++ linux-2.6/include/linux/pci.h
@@ -373,6 +373,30 @@ struct pci_dev {
struct resource *pci_dev_resource_n(struct pci_dev *dev, int n);
int pci_dev_resource_idx(struct pci_dev *dev, struct resource *res);
+#define PCI_STD_RES (1<<0)
+#define PCI_ROM_RES (1<<1)
+#define PCI_IOV_RES (1<<2)
+#define PCI_BRIDGE_RES (1<<3)
+#define PCI_RES_BLOCK_NUM 4
+
+#define PCI_ALL_RES (PCI_STD_RES | PCI_ROM_RES | PCI_BRIDGE_RES | PCI_IOV_RES)
+#define PCI_NOSTD_RES (PCI_ALL_RES & ~PCI_STD_RES)
+#define PCI_NOIOV_RES (PCI_ALL_RES & ~PCI_IOV_RES)
+#define PCI_NOROM_RES (PCI_ALL_RES & ~PCI_ROM_RES)
+#define PCI_NOBRIDGE_RES (PCI_ALL_RES & ~PCI_BRIDGE_RES)
+#define PCI_STD_ROM_RES (PCI_STD_RES | PCI_ROM_RES)
+#define PCI_STD_IOV_RES (PCI_STD_RES | PCI_IOV_RES)
+#define PCI_STD_ROM_IOV_RES (PCI_STD_RES | PCI_ROM_RES | PCI_IOV_RES)
+
+int pci_next_resource_idx(int i, int flag);
+
+#define for_each_pci_resource(dev, res, i, flag) \
+ for (i = pci_next_resource_idx(-1, flag), \
+ res = pci_dev_resource_n(dev, i); \
+ res; \
+ i = pci_next_resource_idx(i, flag), \
+ res = pci_dev_resource_n(dev, i))
+
static inline struct pci_dev *pci_physfn(struct pci_dev *dev)
{
#ifdef CONFIG_PCI_IOV
Index: linux-2.6/drivers/pci/probe.c
===================================================================
--- linux-2.6.orig/drivers/pci/probe.c
+++ linux-2.6/drivers/pci/probe.c
@@ -124,6 +124,54 @@ int pci_dev_resource_idx(struct pci_dev
return -1;
}
+static void __init_res_idx_mask(unsigned long *mask, int flag)
+{
+ bitmap_zero(mask, PCI_NUM_RESOURCES);
+ if (flag & PCI_STD_RES)
+ bitmap_set(mask, PCI_STD_RESOURCES,
+ PCI_STD_RESOURCE_END - PCI_STD_RESOURCES + 1);
+ if (flag & PCI_ROM_RES)
+ bitmap_set(mask, PCI_ROM_RESOURCE, 1);
+#ifdef CONFIG_PCI_IOV
+ if (flag & PCI_IOV_RES)
+ bitmap_set(mask, PCI_IOV_RESOURCES,
+ PCI_IOV_RESOURCE_END - PCI_IOV_RESOURCES + 1);
+#endif
+ if (flag & PCI_BRIDGE_RES)
+ bitmap_set(mask, PCI_BRIDGE_RESOURCES,
+ PCI_BRIDGE_RESOURCE_END - PCI_BRIDGE_RESOURCES + 1);
+}
+
+static bool res_idx_mask_inited;
+static DECLARE_BITMAP(res_idx_mask[1 << PCI_RES_BLOCK_NUM], PCI_NUM_RESOURCES);
+static unsigned long *get_res_idx_mask(int flag)
+{
+ int i;
+
+ if (!res_idx_mask_inited) {
+ for (i = 0; i < (1 << PCI_RES_BLOCK_NUM); i++)
+ __init_res_idx_mask(res_idx_mask[i], i);
+
+ res_idx_mask_inited = true;
+ }
+
+ return res_idx_mask[flag & ((1 << PCI_RES_BLOCK_NUM) - 1)];
+}
+
+int pci_next_resource_idx(int i, int flag)
+{
+ unsigned long *mask = get_res_idx_mask(flag);
+
+ i++;
+ if (i < PCI_NUM_RESOURCES)
+ i = find_next_bit(mask, PCI_NUM_RESOURCES, i);
+
+ if (i < PCI_NUM_RESOURCES)
+ return i;
+
+ return -1;
+}
+
static u64 pci_size(u64 base, u64 maxbase, u64 mask)
{
u64 size = mask & maxbase; /* Find the significant bits */
[-- Attachment #3: ram_pci_it_v6.patch --]
[-- Type: application/octet-stream, Size: 2658 bytes --]
---
drivers/pci/probe.c | 34 ++++++++++++++++++++++++++++++++++
include/linux/pci.h | 23 +++++++++++++++++++++++
2 files changed, 57 insertions(+)
Index: linux-2.6/include/linux/pci.h
===================================================================
--- linux-2.6.orig/include/linux/pci.h
+++ linux-2.6/include/linux/pci.h
@@ -373,6 +373,29 @@ struct pci_dev {
struct resource *pci_dev_resource_n(struct pci_dev *dev, int n);
int pci_dev_resource_idx(struct pci_dev *dev, struct resource *res);
+#define PCI_STD_RES (1<<0)
+#define PCI_ROM_RES (1<<1)
+#define PCI_IOV_RES (1<<2)
+#define PCI_BRIDGE_RES (1<<3)
+
+#define PCI_ALL_RES (PCI_STD_RES | PCI_ROM_RES | PCI_BRIDGE_RES | PCI_IOV_RES)
+#define PCI_NOSTD_RES (PCI_ALL_RES & ~PCI_STD_RES)
+#define PCI_NOIOV_RES (PCI_ALL_RES & ~PCI_IOV_RES)
+#define PCI_NOROM_RES (PCI_ALL_RES & ~PCI_ROM_RES)
+#define PCI_NOBRIDGE_RES (PCI_ALL_RES & ~PCI_BRIDGE_RES)
+#define PCI_STD_ROM_RES (PCI_STD_RES | PCI_ROM_RES)
+#define PCI_STD_IOV_RES (PCI_STD_RES | PCI_IOV_RES)
+#define PCI_STD_ROM_IOV_RES (PCI_STD_RES | PCI_ROM_RES | PCI_IOV_RES)
+
+int pci_next_resource_idx(int i, int flag);
+
+#define for_each_pci_resource(dev, res, i, flag) \
+ for (i = pci_next_resource_idx(-1, flag), \
+ res = pci_dev_resource_n(dev, i); \
+ res; \
+ i = pci_next_resource_idx(i, flag), \
+ res = pci_dev_resource_n(dev, i))
+
static inline struct pci_dev *pci_physfn(struct pci_dev *dev)
{
#ifdef CONFIG_PCI_IOV
Index: linux-2.6/drivers/pci/probe.c
===================================================================
--- linux-2.6.orig/drivers/pci/probe.c
+++ linux-2.6/drivers/pci/probe.c
@@ -124,6 +124,40 @@ int pci_dev_resource_idx(struct pci_dev
return -1;
}
+static bool check_resource_idx(int *i, int start, int end, bool match)
+{
+ if (*i >= start && *i <= end) {
+ if (match)
+ return true;
+ else
+ *i = end + 1;
+ }
+ return false;
+}
+
+int pci_next_resource_idx(int i, int flag)
+{
+ while (++i < PCI_NUM_RESOURCES) {
+ if (check_resource_idx(&i, PCI_STD_RESOURCES,
+ PCI_STD_RESOURCE_END, !!(flag & PCI_STD_RES)))
+ return i;
+ if (check_resource_idx(&i, PCI_ROM_RESOURCE,
+ PCI_ROM_RESOURCE, !!(flag & PCI_ROM_RES)))
+ return i;
+#ifdef CONFIG_PCI_IOV
+ if (check_resource_idx(&i, PCI_IOV_RESOURCES,
+ PCI_IOV_RESOURCE_END, !!(flag & PCI_IOV_RES)))
+ return i;
+#endif
+ if (check_resource_idx(&i, PCI_BRIDGE_RESOURCES,
+ PCI_BRIDGE_RESOURCE_END,
+ !!(flag & PCI_BRIDGE_RES)))
+ return i;
+ }
+
+ return -1;
+}
+
static u64 pci_size(u64 base, u64 maxbase, u64 mask)
{
u64 size = mask & maxbase; /* Find the significant bits */
next prev parent reply other threads:[~2012-08-23 19:30 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-06-18 5:03 [RFC PATCH] methods to access resources of a struct pci_dev Ram Pai
2012-06-18 18:30 ` Yinghai Lu
2012-06-19 1:46 ` Ram Pai
2012-06-19 2:57 ` Yinghai Lu
2012-08-15 21:25 ` Bjorn Helgaas
2012-08-16 3:26 ` Ram Pai
2012-08-16 4:11 ` Yinghai Lu
2012-08-16 4:41 ` Ram Pai
2012-08-21 15:13 ` [RFC PATCH v2 ]pci: pci resource iterator Ram Pai
2012-08-21 23:22 ` Yinghai Lu
2012-08-22 10:15 ` Ram Pai
2012-08-22 17:31 ` Yinghai Lu
2012-08-22 17:35 ` Yinghai Lu
2012-08-23 0:28 ` Yinghai Lu
2012-08-23 5:09 ` [RFC PATCH v3 " Ram Pai
2012-08-23 19:30 ` Yinghai Lu [this message]
2012-08-27 7:33 ` Ram Pai
2012-09-03 8:07 ` Yinghai Lu
2012-09-03 9:08 ` Ram Pai
2012-09-03 18:20 ` Yinghai Lu
2012-09-04 3:27 ` Ram Pai
2012-09-18 0:03 ` Yinghai Lu
2012-09-21 6:18 ` Ram Pai
2012-09-21 6:27 ` Yinghai Lu
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='CAE9FiQVq=6Shv_4taNzN3h4OBs1PgoFJZqV0i3tRvmz04qZbXw@mail.gmail.com' \
--to=yinghai@kernel.org \
--cc=bhelgaas@google.com \
--cc=linux-pci@vger.kernel.org \
--cc=linuxram@us.ibm.com \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.