On Sat, Jun 1, 2019 at 3:54 AM Liu Bo <[email protected]> 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.
