[Virtio-fs] [PATCH 3/3] virtio-fs: add tracepoints

[Masayoshi Mizuma]

On Tue, Aug 20, 2019 at 11:06:43AM +0100, Stefan Hajnoczi wrote:
> On Mon, Aug 12, 2019 at 05:04:55PM -0400, Masayoshi Mizuma wrote:
> > @@ -251,6 +259,11 @@ static void virtio_fs_hiprio_dispatch_work(struct work_struct *work)
> >  		notify = virtqueue_kick_prepare(vq);
> >  		spin_unlock(&fsvq->lock);
> >  
> > +		trace_virtio_fs_hiprio_request_dispatched(
> > +			forget->ih.opcode, forget->ih.unique,
> > +			forget->ih.nodeid, forget->ih.len,
> > +			notify);
> > +
> 
> It is safer to trace the request before placing it in the queue and
> before releasing fsvq->lock.  I'm concerned that a fast device may
> complete the request and invoke virtio_fs_hiprio_done_work() so that
> kfree(req) is called before we've finished tracing it.
> 
> (We cannot rely on virtqueue_notify() because devices may poll the
> virtqueue, so as soon as the request has been added to the virtqueue it
> may be completed.)
> 
> > @@ -914,6 +931,10 @@ static int virtio_fs_enqueue_req(struct virtqueue *vq, struct fuse_req *req)
> >  
> >  	spin_unlock(&fsvq->lock);
> >  
> > +	trace_virtio_fs_request_dispatched(
> > +		req->in.h.opcode, req->in.h.unique,
> > +		req->in.h.nodeid, req->in.h.len, req->flags, notify);
> > +
> >  	if (notify)
> >  		virtqueue_notify(vq);
> 
> Same here, we no longer control the lifetime of req so it's not safe to
> access it after virtqueue_add_sgs() + spin_unlock(&fsvq->lock).

Thank you for your review. I'll move the trace before
spin_unlock(&fsvq->lock).

Thanks!
Masa