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

[Cao jin <caoj.fnst@cn.fujitsu.com>]

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 <caoj.fnst@cn.fujitsu.com>
>>>> ---
>>>>   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