On Wed, Nov 04, 2009 at 12:08:47PM +0100, Andi Kleen wrote:
> "Michael S. Tsirkin" <mst@xxxxxxxxxx> writes:
>
> Haven't really read the whole thing, just noticed something at a glance.
>
> > +/* Expects to be always run from workqueue - which acts as
> > + * read-size critical section for our kind of RCU. */
> > +static void handle_tx(struct vhost_net *net)
> > +{
> > + struct vhost_virtqueue *vq = &net->dev.vqs[VHOST_NET_VQ_TX];
> > + unsigned head, out, in, s;
> > + struct msghdr msg = {
> > + .msg_name = NULL,
> > + .msg_namelen = 0,
> > + .msg_control = NULL,
> > + .msg_controllen = 0,
> > + .msg_iov = vq->iov,
> > + .msg_flags = MSG_DONTWAIT,
> > + };
> > + size_t len, total_len = 0;
> > + int err, wmem;
> > + size_t hdr_size;
> > + struct socket *sock = rcu_dereference(vq->private_data);
> > + if (!sock)
> > + return;
> > +
> > + wmem = atomic_read(&sock->sk->sk_wmem_alloc);
> > + if (wmem >= sock->sk->sk_sndbuf)
> > + return;
> > +
> > + use_mm(net->dev.mm);
>
> I haven't gone over all this code in detail, but that isolated reference count
> use looks suspicious. What prevents the mm from going away before
> you increment, if it's not the current one?
We take a reference to it before we start any virtqueues,
and stop all virtqueues before we drop the reference:
/* Caller should have device mutex */
static long vhost_dev_set_owner(struct vhost_dev *dev)
{
/* Is there an owner already? */
if (dev->mm)
return -EBUSY;
/* No owner, become one */
dev->mm = get_task_mm(current);
return 0;
}
And
vhost_dev_cleanup:
....
if (dev->mm)
mmput(dev->mm);
dev->mm = NULL;
}
Fine?
> -Andi
>
> --
> ak@xxxxxxxxxxxxxxx -- Speaking for myself only.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
