Re: [Qemu-devel] [PATCH] PCI: minor performance optimization

[Cao jin]

On 11/20/2015 07:26 PM, Michael S. Tsirkin wrote:
> On Fri, Nov 20, 2015 at 07:04:07PM +0800, Cao jin wrote:
> > On 11/20/2015 06:45 PM, Michael S. Tsirkin wrote:
> > > On Fri, Nov 20, 2015 at 06:45:01PM +0800, Cao jin wrote:
> > >
> > > > 2. As spec says, each capability must be DWORD aligned, so an optimization can
> > > >     be done via Loop Unrolling.
> > >
> > > Why do we want to optimize it?
> >
> > For tiny performance improvement via less loop. take pcie express
> > capability(60 bytes at most) for example, it may loop 60 times, now we just
> > need 15 times, a quarter of before.
>
> But who cares? This is not a data path operation.

It is tiny thing I found when browsing code. When found there are 
several places looks like this, I think maybe it does good to qemu to do 
this and CCed to you because it don`t look like a simple trivial patch.

So, hey Michael, if you don`t like this kind of optimization, that`t ok, 
forget it. But I think it make me little confused when determine which 
kind of patch should be CCed to you.

> > > > Signed-off-by: Cao jin <address@hidden>
> > > > ---
> > > >   hw/pci/pci.c | 12 ++++++++----
> > > >   1 file changed, 8 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> > > > index 168b9cc..1e99603 100644
> > > > --- a/hw/pci/pci.c
> > > > +++ b/hw/pci/pci.c
> > > > @@ -1924,13 +1924,15 @@ PCIDevice *pci_create_simple(PCIBus *bus, int devfn, 
> > > > const char *name)
> > > >   static uint8_t pci_find_space(PCIDevice *pdev, uint8_t size)
> > > >   {
> > > >       int offset = PCI_CONFIG_HEADER_SIZE;
> > > > -    int i;
> > > > -    for (i = PCI_CONFIG_HEADER_SIZE; i < PCI_CONFIG_SPACE_SIZE; ++i) {
> > > > +    int i = PCI_CONFIG_HEADER_SIZE;;
> > > > +
> > > > +    for (; i < PCI_CONFIG_SPACE_SIZE; i = i + 4) {
> > > >           if (pdev->used[i])
> > > > -            offset = i + 1;
> > > > -        else if (i - offset + 1 == size)
> > > > +            offset = i + 4;
> > > > +        else if (i - offset >= size)
> > > >               return offset;
> > > >       }
> > > > +
> > > >       return 0;
> > > >   }
> > > >
> > > > @@ -2144,6 +2146,8 @@ int pci_add_capability2(PCIDevice *pdev, uint8_t cap_id,
> > > >       uint8_t *config;
> > > >       int i, overlapping_cap;
> > > >
> > > > +    assert(size > 0);
> > > > +
> > > >       if (!offset) {
> > > >           offset = pci_find_space(pdev, size);
> > > >           if (!offset) {
> > > > --
> > > > 2.1.0
> > > .
> >
> > --
> > Yours Sincerely,
> >
> > Cao Jin
> .

--
Yours Sincerely,

Cao Jin