Messages in this thread | | | Subject | Re: [PATCH V2 7/9] vhost: do not use RCU to synchronize MMU notifier with worker | From | Jason Wang <> | Date | Wed, 7 Aug 2019 14:49:57 +0800 |
| |
On 2019/8/6 下午8:04, Jason Gunthorpe wrote: > On Mon, Aug 05, 2019 at 12:20:45PM +0800, Jason Wang wrote: >> On 2019/8/2 下午8:46, Jason Gunthorpe wrote: >>> On Fri, Aug 02, 2019 at 05:40:07PM +0800, Jason Wang wrote: >>>>> This must be a proper barrier, like a spinlock, mutex, or >>>>> synchronize_rcu. >>>> I start with synchronize_rcu() but both you and Michael raise some >>>> concern. >>> I've also idly wondered if calling synchronize_rcu() under the various >>> mm locks is a deadlock situation. >> >> Maybe, that's why I suggest to use vhost_work_flush() which is much >> lightweight can can achieve the same function. It can guarantee all previous >> work has been processed after vhost_work_flush() return. > If things are already running in a work, then yes, you can piggyback > on the existing spinlocks inside the workqueue and be Ok > > However, if that work is doing any copy_from_user, then the flush > becomes dependent on swap and it won't work again...
Yes it do copy_from_user(), so we can't do this.
> >>>> 1) spinlock: add lots of overhead on datapath, this leads 0 performance >>>> improvement. >>> I think the topic here is correctness not performance improvement> > >> But the whole series is to speed up vhost. > So? Starting with a whole bunch of crazy, possibly broken, locking and > claiming a performance win is not reasonable.
Yes, I admit this patch is tricky, I'm not going to push this. Will post a V3.
> >> Spinlock is correct but make the whole series meaningless consider it won't >> bring any performance improvement. > You can't invent a faster spinlock by opencoding some wild > scheme. There is nothing special about the usage here, it needs a > blocking lock, plain and simple. > > Jason
Will post V3. Let's see if you are happy with that version.
Thanks
| |