Message-ID: <58f4e0ed-182b-605e-1f6c-f107ffac1fa6@redhat.com>
Date:   Mon, 7 Jan 2019 14:54:19 +0800
From:   Jason Wang <jasowang@...hat.com>
To:     "Michael S. Tsirkin" <mst@...hat.com>
Cc:     netdev@...r.kernel.org
Subject: Re: thoughts stac/clac and get user for vhost


On 2019/1/7 下午1:42, Michael S. Tsirkin wrote:
> On Mon, Jan 07, 2019 at 12:26:51PM +0800, Jason Wang wrote:
>> On 2019/1/5 上午5:25, Michael S. Tsirkin wrote:
>>> On Wed, Jan 02, 2019 at 11:25:14AM +0800, Jason Wang wrote:
>>>> On 2018/12/31 上午2:40, Michael S. Tsirkin wrote:
>>>>> On Thu, Dec 27, 2018 at 05:55:52PM +0800, Jason Wang wrote:
>>>>>> On 2018/12/26 下午11:06, Michael S. Tsirkin wrote:
>>>>>>> On Wed, Dec 26, 2018 at 12:03:50PM +0800, Jason Wang wrote:
>>>>>>>> On 2018/12/26 上午12:41, Michael S. Tsirkin wrote:
>>>>>>>>> Hi!
>>>>>>>>> I was just wondering: packed ring batches things naturally.
>>>>>>>>> E.g.
>>>>>>>>>
>>>>>>>>> user_access_begin
>>>>>>>>> check descriptor valid
>>>>>>>>> smp_rmb
>>>>>>>>> copy descriptor
>>>>>>>>> user_access_end
>>>>>>>> But without speculation on the descriptor (which may only work for in-order
>>>>>>>> or even a violation of spec). Only one two access of a single descriptor
>>>>>>>> could be batched. For split ring, we can batch more since we know how many
>>>>>>>> descriptors is pending. (avail_idx - last_avail_idx).
>>>>>>>>
>>>>>>>> Anything I miss?
>>>>>>>>
>>>>>>>> Thanks
>>>>>>>>
>>>>>>> just check more descriptors in a loop:
>>>>>>>
>>>>>>>      user_access_begin
>>>>>>>      for (i = 0; i < 16; ++i) {
>>>>>>> 	 if (!descriptor valid)
>>>>>>> 		break;
>>>>>>> 	 smp_rmb
>>>>>>> 	 copy descriptor
>>>>>>>      }
>>>>>>>      user_access_end
>>>>>>>
>>>>>>> you don't really need to know how many there are
>>>>>>> ahead of the time as you still copy them 1 by one.
>>>>>> So let's see the case of split ring
>>>>>>
>>>>>>
>>>>>> user_access_begin
>>>>>>
>>>>>> n = avail_idx - last_avail_idx (1)
>>>>>>
>>>>>> n = MIN(n, 16)
>>>>>>
>>>>>> smp_rmb
>>>>>>
>>>>>> read n entries from avail_ring (2)
>>>>>>
>>>>>> for (i =0; i <n; i++)
>>>>>>
>>>>>>        copy descriptor (3)
>>>>>>
>>>>>> user_access_end
>>>>>>
>>>>>>
>>>>>> Consider for the case of heavy workload. So for packed ring, we have 32
>>>>>> times of userspace access and 16 times of smp_rmb()
>>>>>>
>>>>>> For split ring we have
>>>>>>
>>>>>> (1) 1 time
>>>>>>
>>>>>> (2) 2 times at most
>>>>>>
>>>>>> (3) 16 times
>>>>>>
>>>>>> 19 times of userspace access and 1 times of smp_rmb(). In fact 2 could be
>>>>>> eliminated with in order. 3 could be batched completely with in order and
>>>>>> partially when out of order.
>>>>>>
>>>>>> I don't see how packed ring help here especially consider lfence on x86 is
>>>>>> more than memory fence, it prevents speculation in fact.
>>>>>>
>>>>>> Thanks
>>>>> So on x86 at least RMB is free, this is why I never bothered optimizing
>>>>> it out. Is smp_rmb still worth optimizing out for ARM? Does it cost
>>>>> more than the extra indirection in the split ring?
>>>> I don't know, but obviously, RMB has a chance to damage the performance more
>>>> or less. But even on arch where the RMB is free, packed ring still does not
>>>> show obvious advantage.
>>> People do measure gains with a PMD on host+guest.
>>> So it's a question of optimizing the packed ring implementation in Linux.
>>
>> Well, 2%-3% difference is not quite a lot.
> People reported a 10% gain with tiny packets, others reported more.


Good to know this, any pointer. 2%-3% is the number I got from Jens' 
cover letter.


>
> Again, packed ring is faster sometimes by a factor of 3x but
> virtio is just virtio, there's a lot going on besides
> just passing the buffer addresses guest to host,
> and a different ring layout won't help with that.
>
>
>> I think it's not hard to let split ring faster have some small optimizations
>> on the code itself.
>>
>> Thanks
> Speed up the split ring support in virtio pmd in dpdk? There have been
> several people working on that for a while now. It seems more likely
> that we can speed up the newer packed ring code.  E.g. things like
> prefetch have a much better chance to work will with the packed layout,
> with split one it was a wash IIRC.


But what happen when in order is implemented for packed ring?

I post a patch that increase 10% of PPS with less than 10 lines of code 
for vhost (bypass the avail ring reading). I have similar patch for dpdk 
but just don't have time to test it. Similar optimization could be 
applied to used ring for TX easily.

Thanks


>
>>>
>>>>> But my point was really fundamental - if ring accesses are expensive
>>>>> then we should batch them.
>>>> I don't object the batching, the reason that they are expensive could be:
>>>>
>>>> 1) unnecessary overhead caused by speculation barrier and check likes SMAP
>>>> 2) cache contention
>>>>
>>>> So it does not conflict with the effort that I did to remove 1). My plan is:
>>>> for metadata, try to eliminate all the 1) completely. For data, we can do
>>>> batch copying to amortize its effort. For avail/descriptor batching, we can
>>>> try to it on top.
>>>>
>>>>
>>>>>     Right now we have an API that gets
>>>>> an iovec directly. That limits the optimizations you can do.
>>>>>
>>>>> The translation works like this:
>>>>>
>>>>> ring -> valid descriptors -> iovecs
>>>>>
>>>>> We should have APIs for each step that work in batches.
>>>>>
>>>> Yes.
>>>>
>>>> Thanks
>>>>
>>>>
>>>>>>>>> So packed layout should show the gain with this approach.
>>>>>>>>> That could be motivation enough to finally enable vhost packed ring
>>>>>>>>> support.
>>>>>>>>>
>>>>>>>>> Thoughts?
>>>>>>>>>

Powered by blists - more mailing lists