[Virtio-fs] [PATCH] fuse: refcount FORGET requests

[Tao Peng]

On Sat, Jun 1, 2019 at 3:54 AM Liu Bo <bo.liu at linux.alibaba.com> wrote:
>
> On Fri, May 31, 2019 at 02:59:07PM -0400, Vivek Goyal wrote:
> > On Fri, May 31, 2019 at 11:44:55AM -0700, Liu Bo wrote:
> > > On Fri, May 31, 2019 at 09:22:28AM -0400, Vivek Goyal wrote:
> > > > On Fri, May 31, 2019 at 05:24:03PM +0800, Peng Tao wrote:
> > > > > Right now FORGET requests are not tracked and they might be sent
> > > > > after DESTROY request.
> > > >
> > > > How does that happen?
> > > >
> > >
> > > A bit more details, it is those FORGETs that remain in HIGHPRI vq,
> > > sent before DESTROY but not yet processed by daemon before DESTROY
> > > gets back to guest, then they get processed after the 2nd INIT.
> >
> > I just posted 3 patches to make sure forget request is not sent after
> > destroy. Can you give it a try and see if it solves the problem.
> >
>
> Sorry, somehow this can only be reproduced on Peng Tao's setup.
>
> > I also pushed my changes to this branch.
> >
> > https://github.com/rhvgoyal/linux/commits/flush-forget
Hi Vivek,

I looked at your patchset and I think the main issue is that there is
already fc->num_waiting to track inflight requests. Yet you added
virtio_fs_vq->in_flight to track vq requests and it is only used by
the hiprio vq. You may think that it is only to track forget requests,
but why inventing a specific wheel when there is a general one? The
maintenance APIs for fc->num_waiting are already there and we don't
need to re-implement the busy-waiting logic as is done in
virtio_fs_flush_hiprio_queue(). The generic fuse code uses a waitqueue
for that in fuse_wait_aborted().

And another issue is that now virtiofs tracks FORGET requests while
generic fuse does not. It is better to unify the behavior.

> >
>
> The 2nd mount & INIT can only work after vq->fud is released, and
>
> virtio_kill_sb
>   fuse_kill_sb_anon
>   virtio_fs_free_devs
>     #release vq->fud
>
> So it's not necessary to have DESTROY to be the very last one, it only
> matters whether we wait for inflight FORGETS before setting vq->fud to
> NULL.
Yes, we don't necessarily need to make sure DESTROY is the last
request. We just need to make sure when kill_sb returns, all inflight
requests are cleaned up. And it is exactly what my patch is doing.

Cheers,
Tao
-- 
Into something rich and strange.