On 2019/1/7 ??11:28, Michael S. Tsirkin wrote:> On Mon, Jan 07, 2019 at 10:19:03AM +0800, Jason Wang wrote: >> On 2019/1/3 ??4:47, Michael S. Tsirkin wrote: >>> On Sat, Dec 29, 2018 at 08:46:51PM +0800, Jason Wang wrote: >>>> This series tries to access virtqueue metadata through kernel virtual >>>> address instead of copy_user() friends since they had too much >>>> overheads like checks, spec barriers or even hardware feature >>>> toggling. >>> Will review, thanks! >>> One questions that comes to mind is whether it's all about bypassing >>> stac/clac. Could you please include a performance comparison with >>> nosmap? >>> >> On machine without SMAP (Sandy Bridge): >> >> Before: 4.8Mpps >> >> After: 5.2Mpps > OK so would you say it's really unsafe versus safe accesses? > Or would you say it's just a better written code?It's the effect of removing speculation barrier.> >> On machine with SMAP (Broadwell): >> >> Before: 5.0Mpps >> >> After: 6.1Mpps >> >> No smap: 7.5Mpps >> >> >> Thanks > > no smap being before or after? >Let me clarify: Before (SMAP on): 5.0Mpps Before (SMAP off): 7.5Mpps After (SMAP on): 6.1Mpps Thanks
On Mon, Jan 07, 2019 at 11:53:41AM +0800, Jason Wang wrote:> > On 2019/1/7 ??11:28, Michael S. Tsirkin wrote: > > On Mon, Jan 07, 2019 at 10:19:03AM +0800, Jason Wang wrote: > > > On 2019/1/3 ??4:47, Michael S. Tsirkin wrote: > > > > On Sat, Dec 29, 2018 at 08:46:51PM +0800, Jason Wang wrote: > > > > > This series tries to access virtqueue metadata through kernel virtual > > > > > address instead of copy_user() friends since they had too much > > > > > overheads like checks, spec barriers or even hardware feature > > > > > toggling. > > > > Will review, thanks! > > > > One questions that comes to mind is whether it's all about bypassing > > > > stac/clac. Could you please include a performance comparison with > > > > nosmap? > > > > > > > On machine without SMAP (Sandy Bridge): > > > > > > Before: 4.8Mpps > > > > > > After: 5.2Mpps > > OK so would you say it's really unsafe versus safe accesses? > > Or would you say it's just a better written code? > > > It's the effect of removing speculation barrier.You mean __uaccess_begin_nospec introduced by commit 304ec1b050310548db33063e567123fae8fd0301 ? So fundamentally we do access_ok checks when supplying the memory table to the kernel thread, and we should do the spec barrier there. Then we can just create and use a variant of uaccess macros that does not include the barrier? Or, how about moving the barrier into access_ok? This way repeated accesses with a single access_ok get a bit faster. CC Dan Williams on this idea.> > > > > > On machine with SMAP (Broadwell): > > > > > > Before: 5.0Mpps > > > > > > After: 6.1Mpps > > > > > > No smap: 7.5Mpps > > > > > > > > > Thanks > > > > no smap being before or after? > > > > Let me clarify: > > > Before (SMAP on): 5.0Mpps > > Before (SMAP off): 7.5Mpps > > After (SMAP on): 6.1Mpps > > > ThanksHow about after + smap off? And maybe we want a module option just for the vhost thread to keep smap off generally since almost all it does is copy stuff from userspace into kernel anyway. Because what above numbers should is that we really really want a solution that isn't limited to just meta-data access, and I really do not see how any such solution can not also be used to make meta-data access fast. -- MST
On 2019/1/7 ??12:17, Michael S. Tsirkin wrote:> On Mon, Jan 07, 2019 at 11:53:41AM +0800, Jason Wang wrote: >> On 2019/1/7 ??11:28, Michael S. Tsirkin wrote: >>> On Mon, Jan 07, 2019 at 10:19:03AM +0800, Jason Wang wrote: >>>> On 2019/1/3 ??4:47, Michael S. Tsirkin wrote: >>>>> On Sat, Dec 29, 2018 at 08:46:51PM +0800, Jason Wang wrote: >>>>>> This series tries to access virtqueue metadata through kernel virtual >>>>>> address instead of copy_user() friends since they had too much >>>>>> overheads like checks, spec barriers or even hardware feature >>>>>> toggling. >>>>> Will review, thanks! >>>>> One questions that comes to mind is whether it's all about bypassing >>>>> stac/clac. Could you please include a performance comparison with >>>>> nosmap? >>>>> >>>> On machine without SMAP (Sandy Bridge): >>>> >>>> Before: 4.8Mpps >>>> >>>> After: 5.2Mpps >>> OK so would you say it's really unsafe versus safe accesses? >>> Or would you say it's just a better written code? >> >> It's the effect of removing speculation barrier. > > You mean __uaccess_begin_nospec introduced by > commit 304ec1b050310548db33063e567123fae8fd0301 > ?Yes.> > So fundamentally we do access_ok checks when supplying > the memory table to the kernel thread, and we should > do the spec barrier there. > > Then we can just create and use a variant of uaccess macros that does > not include the barrier?The unsafe ones?> > Or, how about moving the barrier into access_ok? > This way repeated accesses with a single access_ok get a bit faster. > CC Dan Williams on this idea.The problem is, e.g for vhost control path. During mem table validation, we don't even want to access them there. So the spec barrier is not needed.> > >>>> On machine with SMAP (Broadwell): >>>> >>>> Before: 5.0Mpps >>>> >>>> After: 6.1Mpps >>>> >>>> No smap: 7.5Mpps >>>> >>>> >>>> Thanks >>> no smap being before or after? >>> >> Let me clarify: >> >> >> Before (SMAP on): 5.0Mpps >> >> Before (SMAP off): 7.5Mpps >> >> After (SMAP on): 6.1Mpps >> >> >> Thanks > How about after + smap off?After (SMAP off): 8.0Mpps> > And maybe we want a module option just for the vhost thread to keep smap > off generally since almost all it does is copy stuff from userspace into > kernel anyway. Because what above numbers should is that we really > really want a solution that isn't limited to just meta-data access, > and I really do not see how any such solution can not also be > used to make meta-data access fast.As we've discussed in another thread of previous version. This requires lots of changes, the main issues is SMAP state was not saved/restored on explicit schedule(). Even if it did, since vhost will call lots of net/block codes, any kind of uaccess in those codes needs understand this special request from vhost e.g you provably need to invent a new kinds of iov iterator that does not touch SMAP at all. And I'm not sure this is the only thing we need to deal with. So I still prefer to: 1) speedup the metadata access through vmap + MMU notifier 2) speedup the datacopy with batched copy (unsafe ones or other new interfaces) Thanks
On Sun, Jan 6, 2019 at 8:17 PM Michael S. Tsirkin <mst at redhat.com> wrote:> > On Mon, Jan 07, 2019 at 11:53:41AM +0800, Jason Wang wrote: > > > > On 2019/1/7 ??11:28, Michael S. Tsirkin wrote: > > > On Mon, Jan 07, 2019 at 10:19:03AM +0800, Jason Wang wrote: > > > > On 2019/1/3 ??4:47, Michael S. Tsirkin wrote: > > > > > On Sat, Dec 29, 2018 at 08:46:51PM +0800, Jason Wang wrote: > > > > > > This series tries to access virtqueue metadata through kernel virtual > > > > > > address instead of copy_user() friends since they had too much > > > > > > overheads like checks, spec barriers or even hardware feature > > > > > > toggling. > > > > > Will review, thanks! > > > > > One questions that comes to mind is whether it's all about bypassing > > > > > stac/clac. Could you please include a performance comparison with > > > > > nosmap? > > > > > > > > > On machine without SMAP (Sandy Bridge): > > > > > > > > Before: 4.8Mpps > > > > > > > > After: 5.2Mpps > > > OK so would you say it's really unsafe versus safe accesses? > > > Or would you say it's just a better written code? > > > > > > It's the effect of removing speculation barrier. > > > You mean __uaccess_begin_nospec introduced by > commit 304ec1b050310548db33063e567123fae8fd0301 > ? > > So fundamentally we do access_ok checks when supplying > the memory table to the kernel thread, and we should > do the spec barrier there. > > Then we can just create and use a variant of uaccess macros that does > not include the barrier? > > Or, how about moving the barrier into access_ok? > This way repeated accesses with a single access_ok get a bit faster. > CC Dan Williams on this idea.It would be interesting to see how expensive re-doing the address limit check is compared to the speculation barrier. I.e. just switch vhost_get_user() to use get_user() rather than __get_user(). That will sanitize the pointer in the speculative path without a barrier. I recall we had a convert access_ok() discussion with this result here: https://lkml.org/lkml/2018/1/17/929 ...but it sounds like you are proposing a smaller scope fixup for the vhost use case? Something like barrier_nospec() in the success path for all vhost access_ok() checks and then a get_user() variant that disables the barrier.