thoughts stac/clac and get user for vhost

thoughts stac/clac and get user for vhost

[Michael S. Tsirkin]

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

So packed layout should show the gain with this approach.
That could be motivation enough to finally enable vhost packed ring
support.

Thoughts?

-- 
MST

Re: thoughts stac/clac and get user for vhost

[Jason Wang]

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


>
> So packed layout should show the gain with this approach.
> That could be motivation enough to finally enable vhost packed ring
> support.
>
> Thoughts?
>

Re: thoughts stac/clac and get user for vhost

[Michael S. Tsirkin]

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 packed layout should show the gain with this approach.
> > That could be motivation enough to finally enable vhost packed ring
> > support.
> > 
> > Thoughts?
> > 

Re: thoughts stac/clac and get user for vhost

[Jason Wang]

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 packed layout should show the gain with this approach.
>>> That could be motivation enough to finally enable vhost packed ring
>>> support.
>>>
>>> Thoughts?
>>>

Re: thoughts stac/clac and get user for vhost

[Michael S. Tsirkin]

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?

But my point was really fundamental - if ring accesses are expensive
then we should batch them. 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.



> 
> > 
> > 
> > > > So packed layout should show the gain with this approach.
> > > > That could be motivation enough to finally enable vhost packed ring
> > > > support.
> > > > 
> > > > Thoughts?
> > > > 

Re: thoughts stac/clac and get user for vhost

[Jason Wang]

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.


>
> 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?
>>>>>

Re: thoughts stac/clac and get user for vhost

[Michael S. Tsirkin]

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.


> 
> > 
> > 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?
> > > > > > 

Re: thoughts stac/clac and get user for vhost

[Jason Wang]

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.

I think it's not hard to let split ring faster have some small 
optimizations on the code itself.

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?
>>>>>>>

Re: thoughts stac/clac and get user for vhost

[Michael S. Tsirkin]

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.

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 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?
> > > > > > > > 

Re: thoughts stac/clac and get user for vhost

[Jason Wang]

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?
>>>>>>>>>

Re: thoughts stac/clac and get user for vhost

[Michael S. Tsirkin]

On Mon, Jan 07, 2019 at 02:54:19PM +0800, Jason Wang wrote:
> 
> 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.

Oh interesting.  Also Jens' cover letter only from an earlier version,
Jan 29. What happened between these two dates I don't know, worth
investigating.

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

Oh I have no doubt we can speed things up with interface extensions.


> 
> > 
> > > > 
> > > > > > 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?
> > > > > > > > > > 

Re: thoughts stac/clac and get user for vhost

[Jason Wang]

On 2019/1/7 下午10:45, Michael S. Tsirkin wrote:
> On Mon, Jan 07, 2019 at 02:54:19PM +0800, Jason Wang wrote:
>> 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.
> Oh interesting.  Also Jens' cover letter only from an earlier version,
> Jan 29. What happened between these two dates I don't know, worth
> investigating.


Btw, the increased times of userspace memory access looks like the root 
cause of regression of packed ring implementation in vhost. If I manage 
to do some out of spec stuffs to reduce the time, packed ring will be at 
most as fast as split ring. I wonder whether or not it's the same case 
of hardware implementation consider each PCI transaction is not free.


>
>>> 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
> Oh I have no doubt we can speed things up with interface extensions.
>

Ok, let me repost the in order series for vhost.

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?
>>>>>>>>>>>