On Wed, Jun 04, 2014 at 10:51:12PM +0300, Michael S. Tsirkin wrote:

On Tue, Jun 03, 2014 at 06:57:43AM -0700, Eric Dumazet wrote:

quoted

On Tue, 2014-06-03 at 14:48 +0200, Paolo Bonzini wrote:

quoted

Il 02/06/2014 23:58, Eric Dumazet ha scritto:

quoted

This looks dubious

What about using kfree_rcu() instead ?

It would lead to unbound allocation from userspace.

Look at how we did this in commit
c3059477fce2d956a0bb3e04357324780c5d8eeb

quoted

quoted

translate_desc() still uses rcu_read_lock(), its not clear if the mutex
is really held.

Yes, vhost_get_vq_desc must be called with the vq mutex held.

The rcu_read_lock/unlock in translate_desc is unnecessary.

Yep, this is what I pointed out. This is not only necessary, but
confusing and might be incorrectly copy/pasted in the future.

This patch is a partial one and leaves confusion.

Some places uses the proper 

mp = rcu_dereference_protected(dev->memory,
		 lockdep_is_held(&dev->mutex));

others use the now incorrect :

rcu_read_lock();
mp = rcu_dereference(dev->memory);
...

I agree, working on a cleanup patch on top now.

OK I just posted two cleanups as patches on top that address this.
Eric, could you please confirm that you are fine with
cleanups being patches on top?
Bisect will be fine since this hack is ugly but technically correct.

Thanks a lot for pointing out the issues!

-- 
MST