[dpdk-dev] [PATCH v2 1/1] vhost: fix leak of fds and mmaps
Yuanhan Liu
yuanhan.liu at linux.intel.com
Sat Feb 6 06:23:20 CET 2016
Hey Rich,
Sorry for the long delay; I barely forgot it :(
On Tue, Jan 19, 2016 at 10:13:23AM -0800, Rich Lane wrote:
> On Sun, Jan 17, 2016 at 11:58 PM, Yuanhan Liu <yuanhan.liu at linux.intel.com>
> wrote:
>
> On Sun, Jan 17, 2016 at 11:57:18AM -0800, Rich Lane wrote:
> > +/*
> > + * Implementation-specific cleanup. Defined by vhost-cuse and
> vhost-user.
> > + */
> > +void vhost_impl_cleanup(struct virtio_net *dev);
>
> TBH, I am not quite like "_impl_"; maybe "_backend_" is better?
>
>
> If you have a strong preference I will change it. Let me know.
"backend" is just a more common word to me, as well as to QEMU.
So, I would suggest you to do such change, and if so, you could
add my ACK:
Acked-by: Yuanhan Liu <yuanhan.liu at linux.intel.com>
>
> OTOH, what I thought of has slight difference than yours: not
> necessary to export a function, but instead, call the vhost
> backend specific unmap function inside the backend itself. Say,
> call vhost_user_unmap() on RESET_OWNER and connection close.
> What do you think of that?
>
>
> The munmap must be done after the notify_ops->destroy_device callback. That
> means
> the backend can't call it before reset_owner() or destroy_device().
Well, you could:
case VHOST_USER_RESET_OWNER:
ops->reset_owner();
vhost_user_unmap();
break;
Anyway, it's not a big deal. Let's go with your solution first.
--yliu
> The munmap
> could
> be done afterwards, but that requires saving dev->mem in the caller in the case
> of
> destroy_device. The cleanest solution is for the vhost common code to ask the
> backend to clean up at the correct time.
More information about the dev
mailing list